Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

enhance(starlark): make execution step limit configurable #947

Merged
merged 11 commits into from
Aug 31, 2023
6 changes: 6 additions & 0 deletions cmd/vela-server/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,12 @@ func main() {
Name: "github-token",
Usage: "github token, used by compiler, for pulling registry templates",
},
&cli.Uint64Flag{
EnvVars: []string{"VELA_COMPILER_STARLARK_EXEC_LIMIT", "COMPILER_STARLARK_EXEC_LIMIT"},
Name: "compiler-starlark-exec-limit",
Usage: "set the starlark execution step limit for compiling starlark pipelines",
Value: 7500,
},
&cli.StringFlag{
EnvVars: []string{"VELA_MODIFICATION_ADDR", "MODIFICATION_ADDR"},
Name: "modification-addr",
Expand Down
2 changes: 1 addition & 1 deletion compiler/native/expand.go
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ func (c *client) mergeTemplate(bytes []byte, tmpl *yaml.Template, step *yaml.Ste
return native.Render(string(bytes), step.Name, step.Template.Name, step.Environment, step.Template.Variables)
case constants.PipelineTypeStarlark:
//nolint:lll // ignore long line length due to return
return starlark.Render(string(bytes), step.Name, step.Template.Name, step.Environment, step.Template.Variables)
return starlark.Render(string(bytes), step.Name, step.Template.Name, step.Environment, step.Template.Variables, c.StarlarkExecLimit)
default:
//nolint:lll // ignore long line length due to return
return &yaml.Build{}, fmt.Errorf("format of %s is unsupported", tmpl.Format)
Expand Down
6 changes: 6 additions & 0 deletions compiler/native/native.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ type client struct {
ModificationService ModificationConfig
CloneImage string
TemplateDepth int
StarlarkExecLimit uint64

build *library.Build
comment string
Expand Down Expand Up @@ -73,8 +74,12 @@ func New(ctx *cli.Context) (*client, error) {
// set the clone image to use for the injected clone step
c.CloneImage = ctx.String("clone-image")

// set the template depth to use for nested templates
c.TemplateDepth = ctx.Int("max-template-depth")

// set the starlark execution step limit for compiling starlark pipelines
c.StarlarkExecLimit = ctx.Uint64("compiler-starlark-exec-limit")

if ctx.Bool("github-driver") {
logrus.Tracef("setting up Private GitHub Client for %s", ctx.String("github-url"))
// setup private github service
Expand Down Expand Up @@ -115,6 +120,7 @@ func (c *client) Duplicate() compiler.Engine {
cc.ModificationService = c.ModificationService
cc.CloneImage = c.CloneImage
cc.TemplateDepth = c.TemplateDepth
cc.StarlarkExecLimit = c.StarlarkExecLimit

return cc
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/native/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func (c *client) Parse(v interface{}, pipelineType string, template *types.Templ
// capture the raw pipeline configuration
raw = []byte(parsedRaw)

p, err = starlark.RenderBuild(template.Name, parsedRaw, c.EnvironmentBuild(), template.Variables)
p, err = starlark.RenderBuild(template.Name, parsedRaw, c.EnvironmentBuild(), template.Variables, c.StarlarkExecLimit)
if err != nil {
return nil, raw, err
}
Expand Down
9 changes: 5 additions & 4 deletions compiler/template/starlark/render.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"bytes"
"errors"
"fmt"

"github.com/go-vela/types/raw"
"go.starlark.net/starlarkstruct"

Expand All @@ -31,14 +32,14 @@ var (
)

// Render combines the template with the step in the yaml pipeline.
func Render(tmpl string, name string, tName string, environment raw.StringSliceMap, variables map[string]interface{}) (*types.Build, error) {
func Render(tmpl string, name string, tName string, environment raw.StringSliceMap, variables map[string]interface{}, limit uint64) (*types.Build, error) {
config := new(types.Build)

thread := &starlark.Thread{Name: name}
// arbitrarily limiting the steps of the thread to 5000 to help prevent infinite loops
// may need to further investigate spawning a separate POSIX process if user input is problematic
// see https://github.com/google/starlark-go/issues/160#issuecomment-466794230 for further details
thread.SetMaxExecutionSteps(5000)
thread.SetMaxExecutionSteps(limit)

predeclared := starlark.StringDict{"struct": starlark.NewBuiltin("struct", starlarkstruct.Make)}

Expand Down Expand Up @@ -139,14 +140,14 @@ func Render(tmpl string, name string, tName string, environment raw.StringSliceM
// RenderBuild renders the templated build.
//
//nolint:lll // ignore function length due to input args
func RenderBuild(tmpl string, b string, envs map[string]string, variables map[string]interface{}) (*types.Build, error) {
func RenderBuild(tmpl string, b string, envs map[string]string, variables map[string]interface{}, limit uint64) (*types.Build, error) {
config := new(types.Build)

thread := &starlark.Thread{Name: "templated-base"}
// arbitrarily limiting the steps of the thread to 5000 to help prevent infinite loops
// may need to further investigate spawning a separate POSIX process if user input is problematic
// see https://github.com/google/starlark-go/issues/160#issuecomment-466794230 for further details
thread.SetMaxExecutionSteps(5000)
thread.SetMaxExecutionSteps(limit)

predeclared := starlark.StringDict{"struct": starlark.NewBuiltin("struct", starlarkstruct.Make)}

Expand Down
126 changes: 110 additions & 16 deletions compiler/template/starlark/render_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,51 @@ func TestStarlark_Render(t *testing.T) {
wantFile string
wantErr bool
}{
{"basic", args{velaFile: "testdata/step/basic/step.yml", starlarkFile: "testdata/step/basic/template.py"}, "testdata/step/basic/want.yml", false},
{"with method", args{velaFile: "testdata/step/with_method/step.yml", starlarkFile: "testdata/step/with_method/template.star"}, "testdata/step/with_method/want.yml", false},
{"user vars", args{velaFile: "testdata/step/with_vars/step.yml", starlarkFile: "testdata/step/with_vars/template.star"}, "testdata/step/with_vars/want.yml", false},
{"platform vars", args{velaFile: "testdata/step/with_vars_plat/step.yml", starlarkFile: "testdata/step/with_vars_plat/template.star"}, "testdata/step/with_vars_plat/want.yml", false},
{"cancel due to complexity", args{velaFile: "testdata/step/cancel/step.yml", starlarkFile: "testdata/step/cancel/template.star"}, "", true},
{
name: "basic",
args: args{
velaFile: "testdata/step/basic/step.yml",
starlarkFile: "testdata/step/basic/template.py",
},
wantFile: "testdata/step/basic/want.yml",
wantErr: false,
},
{
name: "with method",
args: args{
velaFile: "testdata/step/with_method/step.yml",
starlarkFile: "testdata/step/with_method/template.star",
},
wantFile: "testdata/step/with_method/want.yml",
wantErr: false,
},
{
name: "user vars",
args: args{
velaFile: "testdata/step/with_vars/step.yml",
starlarkFile: "testdata/step/with_vars/template.star",
},
wantFile: "testdata/step/with_vars/want.yml",
wantErr: false,
},
{
name: "platform vars",
args: args{
velaFile: "testdata/step/with_vars_plat/step.yml",
starlarkFile: "testdata/step/with_vars_plat/template.star",
},
wantFile: "testdata/step/with_vars_plat/want.yml",
wantErr: false,
},
{
name: "cancel due to complexity",
args: args{
velaFile: "testdata/step/cancel/step.yml",
starlarkFile: "testdata/step/cancel/template.star",
},
wantFile: "",
wantErr: true,
},
}

for _, tt := range tests {
Expand All @@ -53,7 +93,7 @@ func TestStarlark_Render(t *testing.T) {
t.Error(err)
}

tmplBuild, err := Render(string(tmpl), b.Steps[0].Name, b.Steps[0].Template.Name, b.Steps[0].Environment, b.Steps[0].Template.Variables)
tmplBuild, err := Render(string(tmpl), b.Steps[0].Name, b.Steps[0].Template.Name, b.Steps[0].Environment, b.Steps[0].Template.Variables, 7500)
if (err != nil) != tt.wantErr {
t.Errorf("Render() error = %v, wantErr %v", err, tt.wantErr)
return
Expand Down Expand Up @@ -92,20 +132,73 @@ func TestStarlark_Render(t *testing.T) {
}

func TestNative_RenderBuild(t *testing.T) {
noWantFile := "none"

type args struct {
velaFile string
}

tests := []struct {
name string
args args
wantFile string
wantErr bool
name string
args args
wantFile string
wantErr bool
execLimit uint64
}{
{"steps", args{velaFile: "testdata/build/basic/build.star"}, "testdata/build/basic/want.yml", false},
{"stages", args{velaFile: "testdata/build/basic_stages/build.star"}, "testdata/build/basic_stages/want.yml", false},
{"conditional match", args{velaFile: "testdata/build/conditional/build.star"}, "testdata/build/conditional/want.yml", false},
{"steps, with structs", args{velaFile: "testdata/build/with_struct/build.star"}, "testdata/build/with_struct/want.yml", false},
{
name: "steps",
args: args{
velaFile: "testdata/build/basic/build.star",
},
wantFile: "testdata/build/basic/want.yml",
wantErr: false,
execLimit: 7500,
},
{
name: "stages",
args: args{
velaFile: "testdata/build/basic_stages/build.star",
},
wantFile: "testdata/build/basic_stages/want.yml",
wantErr: false,
execLimit: 7500,
},
{
name: "conditional match",
args: args{
velaFile: "testdata/build/conditional/build.star",
},
wantFile: "testdata/build/conditional/want.yml",
wantErr: false,
execLimit: 7500,
},
{
name: "steps, with structs",
args: args{
velaFile: "testdata/build/with_struct/build.star",
},
wantFile: "testdata/build/with_struct/want.yml",
wantErr: false,
execLimit: 7500,
},
{
name: "large build - exec step limit good",
args: args{
velaFile: "testdata/build/large/build.star",
},
wantFile: "testdata/build/large/want.yml",
wantErr: false,
execLimit: 7500,
},
{
name: "large build - exec step limit too low",
args: args{
velaFile: "testdata/build/large/build.star",
},
wantFile: "",
wantErr: true,
execLimit: 5000,
},
}

for _, tt := range tests {
Expand All @@ -118,13 +211,14 @@ func TestNative_RenderBuild(t *testing.T) {
got, err := RenderBuild("build", string(sFile), map[string]string{
"VELA_REPO_FULL_NAME": "octocat/hello-world",
"VELA_BUILD_BRANCH": "master",
}, map[string]interface{}{})
"VELA_REPO_ORG": "octocat",
}, map[string]interface{}{}, tt.execLimit)
if (err != nil) != tt.wantErr {
t.Errorf("RenderBuild() error = %v, wantErr %v", err, tt.wantErr)
return
}

if tt.wantErr != true {
if tt.wantErr != true && tt.wantFile != noWantFile {
wFile, err := os.ReadFile(tt.wantFile)
if err != nil {
t.Error(err)
Expand Down
Loading