Make starlark size limit configurable (#3291)

cmd/drone-server/config/config.go

@@ -148,6 +148,7 @@ type (
 	Starlark struct {
 		Enabled   bool   `envconfig:"DRONE_STARLARK_ENABLED"`
 		StepLimit uint64 `envconfig:"DRONE_STARLARK_STEP_LIMIT"`
+		SizeLimit uint64 `envconfig:"DRONE_STARLARK_SIZE_LIMIT" default:"0"`
 	}
 
 	// License provides license configuration

cmd/drone-server/inject_plugin.go

@@ -83,6 +83,7 @@ func provideConvertPlugin(client *scm.Client, fileService core.FileService, conf
 		converter.Starlark(
 			conf.Starlark.Enabled,
 			conf.Starlark.StepLimit,
+			conf.Starlark.SizeLimit,
 		),
 		converter.Jsonnet(
 			conf.Jsonnet.Enabled,
@@ -92,6 +93,7 @@ func provideConvertPlugin(client *scm.Client, fileService core.FileService, conf
 		converter.Template(
 			templateStore,
 			conf.Starlark.StepLimit,
+			conf.Starlark.SizeLimit,
 		),
 		converter.Memoize(
 			converter.Remote(

plugin/converter/starlark.go

@@ -26,16 +26,18 @@ import (
 
 // Starlark returns a conversion service that converts the
 // starlark file to a yaml file.
-func Starlark(enabled bool, stepLimit uint64) core.ConvertService {
+func Starlark(enabled bool, stepLimit uint64, sizeLimit uint64) core.ConvertService {
 	return &starlarkPlugin{
 		enabled: enabled,
 		stepLimit: stepLimit,
+		sizeLimit: sizeLimit,
 	}
 }
 
 type starlarkPlugin struct {
 	enabled bool
 	stepLimit uint64
+	sizeLimit uint64
 }
 
 func (p *starlarkPlugin) Convert(ctx context.Context, req *core.ConvertArgs) (*core.Config, error) {
@@ -53,7 +55,7 @@ func (p *starlarkPlugin) Convert(ctx context.Context, req *core.ConvertArgs) (*c
 		return nil, nil
 	}
 
-	file, err := starlark.Parse(req, nil, nil, p.stepLimit)
+	file, err := starlark.Parse(req, nil, nil, p.stepLimit, p.sizeLimit)
 	if err != nil {
 		return nil, err
 	}

plugin/converter/starlark/starlark.go

@@ -29,8 +29,8 @@ const (
 	newline   = "\n"
 )
 
-// limits generated configuration file size.
+// default limit for generated configuration file size.
-const limit = 1000000
+const defaultSizeLimit = 1000000
 
 var (
 	// ErrMainMissing indicates the starlark script is missing
@@ -54,7 +54,7 @@ var (
 	ErrCannotLoad = errors.New("starlark: cannot load external scripts")
 )
 
-func Parse(req *core.ConvertArgs, template *core.Template, templateData map[string]interface{}, stepLimit uint64) (string, error) {
+func Parse(req *core.ConvertArgs, template *core.Template, templateData map[string]interface{}, stepLimit uint64, sizeLimit uint64) (string, error) {
 	thread := &starlark.Thread{
 		Name: "drone",
 		Load: noLoad,
@@ -132,9 +132,13 @@ func Parse(req *core.ConvertArgs, template *core.Template, templateData map[stri
 		return "", ErrMainReturn
 	}
 
+	if sizeLimit == 0 {
+		sizeLimit = defaultSizeLimit
+	}
+
 	// this is a temporary workaround until we
 	// implement a LimitWriter.
-	if b := buf.Bytes(); len(b) > limit {
+	if b := buf.Bytes(); uint64(len(b)) > sizeLimit {
 		return "", ErrMaximumSize
 	}
 	return buf.String(), nil

plugin/converter/starlark/starlark_test.go

@@ -57,7 +57,7 @@ func TestParseStarlark(t *testing.T) {
 
 	req.Config.Data = string(before)
 
-	parsedFile, err := Parse(req, template, templateData, 0)
+	parsedFile, err := Parse(req, template, templateData, 0, 0)
 	if err != nil {
 		t.Error(err)
 		return
@@ -95,7 +95,7 @@ func TestParseStarlarkNotTemplateFile(t *testing.T) {
 	req.Repo.Config = "plugin.starlark.star"
 	req.Config.Data = string(before)
 
-	parsedFile, err := Parse(req, nil, nil, 0)
+	parsedFile, err := Parse(req, nil, nil, 0, 0)
 	if err != nil {
 		t.Error(err)
 		return

plugin/converter/starlark_oss.go

@@ -18,6 +18,6 @@ package converter
 
 import "github.com/drone/drone/core"
 
-func Starlark(enabled bool, stepLimit uint64) core.ConvertService {
+func Starlark(enabled bool, stepLimit uint64, sizeLimit uint64) core.ConvertService {
 	return new(noop)
 }

plugin/converter/starlark_test.go

@@ -24,7 +24,7 @@ import (
 )
 
 func TestStarlarkConvert(t *testing.T) {
-	plugin := Starlark(true, 0)
+	plugin := Starlark(true, 0, 0)
 
 	req := &core.ConvertArgs{
 		Build: &core.Build{
@@ -102,7 +102,7 @@ func TestConvert_Multi(t *testing.T) {
 		},
 	}
 
-	plugin := Starlark(true, 0)
+	plugin := Starlark(true, 0, 0)
 	config, err := plugin.Convert(noContext, req)
 	if err != nil {
 		t.Error(err)
@@ -134,7 +134,7 @@ func TestConvert_Multi(t *testing.T) {
 // this test verifies the plugin is skipped when it has
 // not been explicitly enabled.
 func TestConvert_Skip(t *testing.T) {
-	plugin := Starlark(false, 0)
+	plugin := Starlark(false, 0, 0)
 	config, err := plugin.Convert(noContext, nil)
 	if err != nil {
 		t.Error(err)
@@ -154,7 +154,7 @@ func TestConvert_SkipYaml(t *testing.T) {
 		},
 	}
 
-	plugin := Starlark(true, 0)
+	plugin := Starlark(true, 0, 0)
 	config, err := plugin.Convert(noContext, req)
 	if err != nil {
 		t.Error(err)
@@ -164,3 +164,35 @@ func TestConvert_SkipYaml(t *testing.T) {
 		t.Errorf("Expect nil config returned for non-starlark files")
 	}
 }
+
+// this test verifies the plugin returns error
+// if the generated file size is exceeded.
+func TestConvert_SizeLimit(t *testing.T) {
+	smallFileSizeLimit := uint64(1)
+	plugin := Starlark(true, 0, smallFileSizeLimit)
+
+	req := &core.ConvertArgs{
+		Build: &core.Build{
+			After: "3d21ec53a331a6f037a91c368710b99387d012c1",
+		},
+		Repo: &core.Repository{
+			Slug:   "octocat/hello-world",
+			Config: ".drone.yml",
+		},
+		Config: &core.Config{},
+	}
+
+	before, err := ioutil.ReadFile("testdata/single.star")
+	if err != nil {
+		t.Error(err)
+		return
+	}
+
+	req.Repo.Config = "single.star"
+	req.Config.Data = string(before)
+	_, expectedError := plugin.Convert(noContext, req)
+	if expectedError == nil {
+		t.Error("Expected 'starlark: maximum file size exceeded' error")
+		return
+	}
+}

plugin/converter/template.go

@@ -41,16 +41,18 @@ var (
 	errTemplateExtensionInvalid = errors.New("template extension invalid. must be yaml, starlark or jsonnet")
 )
 
-func Template(templateStore core.TemplateStore, stepLimit uint64) core.ConvertService {
+func Template(templateStore core.TemplateStore, stepLimit uint64, sizeLimit uint64) core.ConvertService {
 	return &templatePlugin{
 		templateStore: templateStore,
 		stepLimit: stepLimit,
+		sizeLimit: sizeLimit,
 	}
 }
 
 type templatePlugin struct {
 	templateStore core.TemplateStore
 	stepLimit uint64
+	sizeLimit uint64
 }
 
 func (p *templatePlugin) Convert(ctx context.Context, req *core.ConvertArgs) (*core.Config, error) {
@@ -84,7 +86,7 @@ func (p *templatePlugin) Convert(ctx context.Context, req *core.ConvertArgs) (*c
 	case ".yml", ".yaml":
 		return parseYaml(req, template, templateArgs)
 	case ".star", ".starlark", ".script":
-		return parseStarlark(req, template, templateArgs, p.stepLimit)
+		return parseStarlark(req, template, templateArgs, p.stepLimit, p.sizeLimit)
 	case ".jsonnet":
 		return parseJsonnet(req, template, templateArgs)
 	default:
@@ -122,8 +124,8 @@ func parseJsonnet(req *core.ConvertArgs, template *core.Template, templateArgs c
 	}, nil
 }
 
-func parseStarlark(req *core.ConvertArgs, template *core.Template, templateArgs core.TemplateArgs, stepLimit uint64) (*core.Config, error) {
+func parseStarlark(req *core.ConvertArgs, template *core.Template, templateArgs core.TemplateArgs, stepLimit uint64, sizeLimit uint64) (*core.Config, error) {
-	file, err := starlark.Parse(req, template, templateArgs.Data, stepLimit)
+	file, err := starlark.Parse(req, template, templateArgs.Data, stepLimit, sizeLimit)
 	if err != nil {
 		return nil, err
 	}

plugin/converter/template_oss.go

@@ -22,7 +22,7 @@ import (
 	"github.com/drone/drone/core"
 )
 
-func Template(templateStore core.TemplateStore, stepLimit uint64) core.ConvertService {
+func Template(templateStore core.TemplateStore, stepLimit uint64, sizeLimit uint64) core.ConvertService {
 	return &templatePlugin{
 		templateStore: templateStore,
 	}

plugin/converter/template_test.go

@@ -73,7 +73,7 @@ func TestTemplatePluginConvertStarlark(t *testing.T) {
 	templates := mock.NewMockTemplateStore(controller)
 	templates.EXPECT().FindName(gomock.Any(), template.Name, req.Repo.Namespace).Return(template, nil)
 
-	plugin := Template(templates, 0)
+	plugin := Template(templates, 0, 0)
 	config, err := plugin.Convert(noContext, req)
 	if err != nil {
 		t.Error(err)
@@ -92,7 +92,7 @@ func TestTemplatePluginConvertStarlark(t *testing.T) {
 
 func TestTemplatePluginConvertNotYamlFile(t *testing.T) {
 
-	plugin := Template(nil, 0)
+	plugin := Template(nil, 0, 0)
 	req := &core.ConvertArgs{
 		Build: &core.Build{
 			After: "3d21ec53a331a6f037a91c368710b99387d012c1",
@@ -120,7 +120,7 @@ func TestTemplatePluginConvertDroneFileTypePipeline(t *testing.T) {
 		t.Error(err)
 		return
 	}
-	plugin := Template(nil, 0)
+	plugin := Template(nil, 0, 0)
 	req := &core.ConvertArgs{
 		Build: &core.Build{
 			After: "3d21ec53a331a6f037a91c368710b99387d012c1",
@@ -161,7 +161,7 @@ func TestTemplatePluginConvertDroneFileYamlExtensions(t *testing.T) {
 			templates := mock.NewMockTemplateStore(controller)
 			templates.EXPECT().FindName(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, dummyErr)
 
-			plugin := Template(templates, 0)
+			plugin := Template(templates, 0, 0)
 			req := &core.ConvertArgs{
 				Build: &core.Build{
 					After: "3d21ec53a331a6f037a91c368710b99387d012c1",
@@ -214,7 +214,7 @@ func TestTemplatePluginConvertTemplateNotFound(t *testing.T) {
 	templates := mock.NewMockTemplateStore(controller)
 	templates.EXPECT().FindName(gomock.Any(), template.Name, req.Repo.Namespace).Return(nil, nil)
 
-	plugin := Template(templates, 0)
+	plugin := Template(templates, 0, 0)
 
 	config, err := plugin.Convert(noContext, req)
 	if config != nil {
@@ -267,7 +267,7 @@ func TestTemplatePluginConvertJsonnet(t *testing.T) {
 	templates := mock.NewMockTemplateStore(controller)
 	templates.EXPECT().FindName(gomock.Any(), template.Name, req.Repo.Namespace).Return(template, nil)
 
-	plugin := Template(templates, 0)
+	plugin := Template(templates, 0, 0)
 	config, err := plugin.Convert(noContext, req)
 	if err != nil {
 		t.Error(err)
@@ -346,7 +346,7 @@ func TestTemplateNestedValuesPluginConvertStarlark(t *testing.T) {
 	templates := mock.NewMockTemplateStore(controller)
 	templates.EXPECT().FindName(gomock.Any(), template.Name, req.Repo.Namespace).Return(template, nil)
 
-	plugin := Template(templates, 0)
+	plugin := Template(templates, 0, 0)
 	config, err := plugin.Convert(noContext, req)
 	if err != nil {
 		t.Error(err)
@@ -424,7 +424,7 @@ func TestTemplatePluginConvertYaml(t *testing.T) {
 	templates := mock.NewMockTemplateStore(controller)
 	templates.EXPECT().FindName(gomock.Any(), template.Name, req.Repo.Namespace).Return(template, nil)
 
-	plugin := Template(templates, 0)
+	plugin := Template(templates, 0, 0)
 	config, err := plugin.Convert(noContext, req)
 	if err != nil {
 		t.Error(err)
@@ -483,7 +483,7 @@ func TestTemplatePluginConvertInvalidTemplateExtension(t *testing.T) {
 	templates := mock.NewMockTemplateStore(controller)
 	templates.EXPECT().FindName(gomock.Any(), template.Name, req.Repo.Namespace).Return(template, nil)
 
-	plugin := Template(templates, 0)
+	plugin := Template(templates, 0, 0)
 	config, err := plugin.Convert(noContext, req)
 	if config != nil {
 		t.Errorf("template extension invalid. must be yaml, starlark or jsonnet")
@@ -535,7 +535,7 @@ func TestTemplatePluginConvertYamlWithComment(t *testing.T) {
 	templates := mock.NewMockTemplateStore(controller)
 	templates.EXPECT().FindName(gomock.Any(), template.Name, req.Repo.Namespace).Return(template, nil)
 
-	plugin := Template(templates, 0)
+	plugin := Template(templates, 0, 0)
 	config, err := plugin.Convert(noContext, req)
 	if err != nil {
 		t.Error(err)