From 8d15c2181f66848639854c55f2351a96de30a53f Mon Sep 17 00:00:00 2001 From: ecrupper Date: Mon, 30 Dec 2024 20:59:41 -0600 Subject: [PATCH 1/2] feat(pipeline): add warnings to pipelines that use legacy YAML lib or have dupl anchors in map --- api/pipeline/template.go | 2 +- api/types/pipeline.go | 57 ++++++++++++---- api/types/pipeline_test.go | 12 ++++ compiler/engine.go | 2 +- compiler/native/compile.go | 8 ++- compiler/native/expand.go | 12 ++-- compiler/native/parse.go | 47 +++++++------ compiler/native/parse_test.go | 46 ++++++------- compiler/template/native/render.go | 36 ++++++---- compiler/template/native/render_test.go | 4 +- compiler/template/starlark/render.go | 74 +++++++++++---------- compiler/template/starlark/render_test.go | 4 +- constants/limit.go | 3 + database/integration_test.go | 2 + database/pipeline/create_test.go | 6 +- database/pipeline/table.go | 2 + database/pipeline/update_test.go | 6 +- database/testutils/api_resources.go | 1 + database/types/pipeline.go | 23 +++++++ database/types/pipeline_test.go | 4 ++ internal/yaml.go | 32 +++++---- internal/yaml_test.go | 54 +++++++++------ mock/server/pipeline.go | 3 + router/middleware/pipeline/pipeline_test.go | 1 + 24 files changed, 278 insertions(+), 163 deletions(-) diff --git a/api/pipeline/template.go b/api/pipeline/template.go index 0ff2a54bc..c1b1bb6fd 100644 --- a/api/pipeline/template.go +++ b/api/pipeline/template.go @@ -101,7 +101,7 @@ func GetTemplates(c *gin.Context) { compiler := compiler.FromContext(c).Duplicate().WithCommit(p.GetCommit()).WithMetadata(m).WithRepo(r).WithUser(u) // parse the pipeline configuration - pipeline, _, err := compiler.Parse(p.GetData(), p.GetType(), new(yaml.Template)) + pipeline, _, _, err := compiler.Parse(p.GetData(), p.GetType(), new(yaml.Template)) if err != nil { util.HandleError(c, http.StatusBadRequest, fmt.Errorf("unable to parse pipeline %s: %w", entry, err)) diff --git a/api/types/pipeline.go b/api/types/pipeline.go index f626e5c6e..dd0ff617d 100644 --- a/api/types/pipeline.go +++ b/api/types/pipeline.go @@ -10,20 +10,21 @@ import ( // // swagger:model Pipeline type Pipeline struct { - ID *int64 `json:"id,omitempty"` - Repo *Repo `json:"repo,omitempty"` - Commit *string `json:"commit,omitempty"` - Flavor *string `json:"flavor,omitempty"` - Platform *string `json:"platform,omitempty"` - Ref *string `json:"ref,omitempty"` - Type *string `json:"type,omitempty"` - Version *string `json:"version,omitempty"` - ExternalSecrets *bool `json:"external_secrets,omitempty"` - InternalSecrets *bool `json:"internal_secrets,omitempty"` - Services *bool `json:"services,omitempty"` - Stages *bool `json:"stages,omitempty"` - Steps *bool `json:"steps,omitempty"` - Templates *bool `json:"templates,omitempty"` + ID *int64 `json:"id,omitempty"` + Repo *Repo `json:"repo,omitempty"` + Commit *string `json:"commit,omitempty"` + Flavor *string `json:"flavor,omitempty"` + Platform *string `json:"platform,omitempty"` + Ref *string `json:"ref,omitempty"` + Type *string `json:"type,omitempty"` + Version *string `json:"version,omitempty"` + ExternalSecrets *bool `json:"external_secrets,omitempty"` + InternalSecrets *bool `json:"internal_secrets,omitempty"` + Services *bool `json:"services,omitempty"` + Stages *bool `json:"stages,omitempty"` + Steps *bool `json:"steps,omitempty"` + Templates *bool `json:"templates,omitempty"` + Warnings *[]string `json:"warnings,omitempty"` // swagger:strfmt base64 Data *[]byte `json:"data,omitempty"` } @@ -210,6 +211,19 @@ func (p *Pipeline) GetTemplates() bool { return *p.Templates } +// GetWarnings returns the Warnings field. +// +// When the provided Pipeline type is nil, or the field within +// the type is nil, it returns the zero value for the field. +func (p *Pipeline) GetWarnings() []string { + // return zero value if Pipeline type or Warnings field is nil + if p == nil || p.Warnings == nil { + return []string{} + } + + return *p.Warnings +} + // GetData returns the Data field. // // When the provided Pipeline type is nil, or the field within @@ -405,6 +419,19 @@ func (p *Pipeline) SetTemplates(v bool) { p.Templates = &v } +// SetWarnings sets the Warnings field. +// +// When the provided Pipeline type is nil, it +// will set nothing and immediately return. +func (p *Pipeline) SetWarnings(v []string) { + // return if Pipeline type is nil + if p == nil { + return + } + + p.Warnings = &v +} + // SetData sets the Data field. // // When the provided Pipeline type is nil, it @@ -436,6 +463,7 @@ func (p *Pipeline) String() string { Templates: %t, Type: %s, Version: %s, + Warnings: %v, }`, p.GetCommit(), p.GetData(), @@ -452,5 +480,6 @@ func (p *Pipeline) String() string { p.GetTemplates(), p.GetType(), p.GetVersion(), + p.GetWarnings(), ) } diff --git a/api/types/pipeline_test.go b/api/types/pipeline_test.go index a925d9940..8c14c5af2 100644 --- a/api/types/pipeline_test.go +++ b/api/types/pipeline_test.go @@ -82,6 +82,10 @@ func TestAPI_Pipeline_Getters(t *testing.T) { t.Errorf("GetTemplates is %v, want %v", test.pipeline.GetTemplates(), test.want.GetTemplates()) } + if !reflect.DeepEqual(test.pipeline.GetWarnings(), test.want.GetWarnings()) { + t.Errorf("GetWarnings is %v, want %v", test.pipeline.GetWarnings(), test.want.GetWarnings()) + } + if !reflect.DeepEqual(test.pipeline.GetData(), test.want.GetData()) { t.Errorf("GetData is %v, want %v", test.pipeline.GetData(), test.want.GetData()) } @@ -123,6 +127,7 @@ func TestAPI_Pipeline_Setters(t *testing.T) { test.pipeline.SetStages(test.want.GetStages()) test.pipeline.SetSteps(test.want.GetSteps()) test.pipeline.SetTemplates(test.want.GetTemplates()) + test.pipeline.SetWarnings(test.want.GetWarnings()) test.pipeline.SetData(test.want.GetData()) if test.pipeline.GetID() != test.want.GetID() { @@ -181,6 +186,10 @@ func TestAPI_Pipeline_Setters(t *testing.T) { t.Errorf("SetTemplates is %v, want %v", test.pipeline.GetTemplates(), test.want.GetTemplates()) } + if !reflect.DeepEqual(test.pipeline.GetWarnings(), test.want.GetWarnings()) { + t.Errorf("SetWarnings is %v, want %v", test.pipeline.GetWarnings(), test.want.GetWarnings()) + } + if !reflect.DeepEqual(test.pipeline.GetData(), test.want.GetData()) { t.Errorf("SetData is %v, want %v", test.pipeline.GetData(), test.want.GetData()) } @@ -207,6 +216,7 @@ func TestAPI_Pipeline_String(t *testing.T) { Templates: %t, Type: %s, Version: %s, + Warnings: %v, }`, p.GetCommit(), p.GetData(), @@ -223,6 +233,7 @@ func TestAPI_Pipeline_String(t *testing.T) { p.GetTemplates(), p.GetType(), p.GetVersion(), + p.GetWarnings(), ) // run test @@ -253,6 +264,7 @@ func testPipeline() *Pipeline { p.SetSteps(true) p.SetTemplates(false) p.SetData(testPipelineData()) + p.SetWarnings([]string{"42:this is a warning"}) return p } diff --git a/compiler/engine.go b/compiler/engine.go index b3503b77a..fd38599ef 100644 --- a/compiler/engine.go +++ b/compiler/engine.go @@ -36,7 +36,7 @@ type Engine interface { // Parse defines a function that converts // an object to a yaml configuration. - Parse(interface{}, string, *yaml.Template) (*yaml.Build, []byte, error) + Parse(interface{}, string, *yaml.Template) (*yaml.Build, []byte, []string, error) // ParseRaw defines a function that converts // an object to a string. diff --git a/compiler/native/compile.go b/compiler/native/compile.go index 7f0a71f41..e8152d81b 100644 --- a/compiler/native/compile.go +++ b/compiler/native/compile.go @@ -39,7 +39,7 @@ type ModifyResponse struct { // Compile produces an executable pipeline from a yaml configuration. func (c *client) Compile(ctx context.Context, v interface{}) (*pipeline.Build, *api.Pipeline, error) { - p, data, err := c.Parse(v, c.repo.GetPipelineType(), new(yaml.Template)) + p, data, warnings, err := c.Parse(v, c.repo.GetPipelineType(), new(yaml.Template)) if err != nil { return nil, nil, err } @@ -61,6 +61,7 @@ func (c *client) Compile(ctx context.Context, v interface{}) (*pipeline.Build, * _pipeline := p.ToPipelineAPI() _pipeline.SetData(data) _pipeline.SetType(c.repo.GetPipelineType()) + _pipeline.SetWarnings(warnings) // create map of templates for easy lookup templates := mapFromTemplates(p.Templates) @@ -117,7 +118,7 @@ func (c *client) Compile(ctx context.Context, v interface{}) (*pipeline.Build, * // CompileLite produces a partial of an executable pipeline from a yaml configuration. func (c *client) CompileLite(ctx context.Context, v interface{}, ruleData *pipeline.RuleData, substitute bool) (*yaml.Build, *api.Pipeline, error) { - p, data, err := c.Parse(v, c.repo.GetPipelineType(), new(yaml.Template)) + p, data, warnings, err := c.Parse(v, c.repo.GetPipelineType(), new(yaml.Template)) if err != nil { return nil, nil, err } @@ -126,6 +127,7 @@ func (c *client) CompileLite(ctx context.Context, v interface{}, ruleData *pipel _pipeline := p.ToPipelineAPI() _pipeline.SetData(data) _pipeline.SetType(c.repo.GetPipelineType()) + _pipeline.SetWarnings(warnings) if p.Metadata.RenderInline { newPipeline, err := c.compileInline(ctx, p, c.GetTemplateDepth()) @@ -267,7 +269,7 @@ func (c *client) compileInline(ctx context.Context, p *yaml.Build, depth int) (* // inject template name into variables template.Variables["VELA_TEMPLATE_NAME"] = template.Name - parsed, _, err := c.Parse(bytes, format, template) + parsed, _, _, err := c.Parse(bytes, format, template) if err != nil { return nil, err } diff --git a/compiler/native/expand.go b/compiler/native/expand.go index ac86d09ce..df3de903b 100644 --- a/compiler/native/expand.go +++ b/compiler/native/expand.go @@ -138,7 +138,7 @@ func (c *client) ExpandSteps(ctx context.Context, s *yaml.Build, tmpls map[strin // inject template name into variables step.Template.Variables["VELA_TEMPLATE_NAME"] = step.Template.Name - tmplBuild, err := c.mergeTemplate(bytes, tmpl, step) + tmplBuild, _, err := c.mergeTemplate(bytes, tmpl, step) if err != nil { return s, err } @@ -247,7 +247,7 @@ func (c *client) ExpandDeployment(ctx context.Context, b *yaml.Build, tmpls map[ b.Deployment.Template.Variables = make(map[string]interface{}) } - tmplBuild, err := c.mergeDeployTemplate(bytes, tmpl, &b.Deployment) + tmplBuild, _, err := c.mergeDeployTemplate(bytes, tmpl, &b.Deployment) if err != nil { return b, err } @@ -391,7 +391,7 @@ func (c *client) getTemplate(ctx context.Context, tmpl *yaml.Template, name stri } //nolint:lll // ignore long line length due to input arguments -func (c *client) mergeTemplate(bytes []byte, tmpl *yaml.Template, step *yaml.Step) (*yaml.Build, error) { +func (c *client) mergeTemplate(bytes []byte, tmpl *yaml.Template, step *yaml.Step) (*yaml.Build, []string, error) { switch tmpl.Format { case constants.PipelineTypeGo, "golang", "": //nolint:lll // ignore long line length due to return @@ -401,11 +401,11 @@ func (c *client) mergeTemplate(bytes []byte, tmpl *yaml.Template, step *yaml.Ste return starlark.Render(string(bytes), step.Name, step.Template.Name, step.Environment, step.Template.Variables, c.GetStarlarkExecLimit()) default: //nolint:lll // ignore long line length due to return - return &yaml.Build{}, fmt.Errorf("format of %s is unsupported", tmpl.Format) + return &yaml.Build{}, nil, fmt.Errorf("format of %s is unsupported", tmpl.Format) } } -func (c *client) mergeDeployTemplate(bytes []byte, tmpl *yaml.Template, d *yaml.Deployment) (*yaml.Build, error) { +func (c *client) mergeDeployTemplate(bytes []byte, tmpl *yaml.Template, d *yaml.Deployment) (*yaml.Build, []string, error) { switch tmpl.Format { case constants.PipelineTypeGo, "golang", "": //nolint:lll // ignore long line length due to return @@ -415,7 +415,7 @@ func (c *client) mergeDeployTemplate(bytes []byte, tmpl *yaml.Template, d *yaml. return starlark.Render(string(bytes), "", d.Template.Name, make(raw.StringSliceMap), d.Template.Variables, c.GetStarlarkExecLimit()) default: //nolint:lll // ignore long line length due to return - return &yaml.Build{}, fmt.Errorf("format of %s is unsupported", tmpl.Format) + return &yaml.Build{}, nil, fmt.Errorf("format of %s is unsupported", tmpl.Format) } } diff --git a/compiler/native/parse.go b/compiler/native/parse.go index 317c52600..b9f47e4dc 100644 --- a/compiler/native/parse.go +++ b/compiler/native/parse.go @@ -40,10 +40,11 @@ func (c *client) ParseRaw(v interface{}) (string, error) { } // Parse converts an object to a yaml configuration. -func (c *client) Parse(v interface{}, pipelineType string, template *yaml.Template) (*yaml.Build, []byte, error) { +func (c *client) Parse(v interface{}, pipelineType string, template *yaml.Template) (*yaml.Build, []byte, []string, error) { var ( - p *yaml.Build - raw []byte + p *yaml.Build + warnings []string + raw []byte ) switch pipelineType { @@ -51,29 +52,29 @@ func (c *client) Parse(v interface{}, pipelineType string, template *yaml.Templa // expand the base configuration parsedRaw, err := c.ParseRaw(v) if err != nil { - return nil, nil, err + return nil, nil, nil, err } // capture the raw pipeline configuration raw = []byte(parsedRaw) - p, err = native.RenderBuild(template.Name, parsedRaw, c.EnvironmentBuild(), template.Variables) + p, warnings, err = native.RenderBuild(template.Name, parsedRaw, c.EnvironmentBuild(), template.Variables) if err != nil { - return nil, raw, err + return nil, raw, nil, err } case constants.PipelineTypeStarlark: // expand the base configuration parsedRaw, err := c.ParseRaw(v) if err != nil { - return nil, nil, err + return nil, nil, nil, err } // capture the raw pipeline configuration raw = []byte(parsedRaw) - p, err = starlark.RenderBuild(template.Name, parsedRaw, c.EnvironmentBuild(), template.Variables, c.GetStarlarkExecLimit()) + p, warnings, err = starlark.RenderBuild(template.Name, parsedRaw, c.EnvironmentBuild(), template.Variables, c.GetStarlarkExecLimit()) if err != nil { - return nil, raw, err + return nil, raw, nil, err } case constants.PipelineTypeYAML, "": switch v := v.(type) { @@ -90,14 +91,13 @@ func (c *client) Parse(v interface{}, pipelineType string, template *yaml.Templa // parse string as path to yaml configuration return ParsePath(v) } - // parse string as yaml configuration return ParseString(v) default: - return nil, nil, fmt.Errorf("unable to parse yaml: unrecognized type %T", v) + return nil, nil, nil, fmt.Errorf("unable to parse yaml: unrecognized type %T", v) } default: - return nil, nil, fmt.Errorf("unable to parse config: unrecognized pipeline_type of %s", c.repo.GetPipelineType()) + return nil, nil, nil, fmt.Errorf("unable to parse config: unrecognized pipeline_type of %s", c.repo.GetPipelineType()) } // initializing Environment to prevent nil error @@ -107,14 +107,14 @@ func (c *client) Parse(v interface{}, pipelineType string, template *yaml.Templa p.Environment = typesRaw.StringSliceMap{} } - return p, raw, nil + return p, raw, warnings, nil } // ParseBytes converts a byte slice to a yaml configuration. -func ParseBytes(data []byte) (*yaml.Build, []byte, error) { - config, err := internal.ParseYAML(data) +func ParseBytes(data []byte) (*yaml.Build, []byte, []string, error) { + config, warnings, err := internal.ParseYAML(data) if err != nil { - return nil, nil, err + return nil, nil, nil, err } // initializing Environment to prevent nil error @@ -124,11 +124,11 @@ func ParseBytes(data []byte) (*yaml.Build, []byte, error) { config.Environment = typesRaw.StringSliceMap{} } - return config, data, nil + return config, data, warnings, nil } // ParseFile converts an os.File into a yaml configuration. -func ParseFile(f *os.File) (*yaml.Build, []byte, error) { +func ParseFile(f *os.File) (*yaml.Build, []byte, []string, error) { return ParseReader(f) } @@ -138,11 +138,11 @@ func ParseFileRaw(f *os.File) (string, error) { } // ParsePath converts a file path into a yaml configuration. -func ParsePath(p string) (*yaml.Build, []byte, error) { +func ParsePath(p string) (*yaml.Build, []byte, []string, error) { // open the file for reading f, err := os.Open(p) if err != nil { - return nil, nil, fmt.Errorf("unable to open yaml file %s: %w", p, err) + return nil, nil, nil, fmt.Errorf("unable to open yaml file %s: %w", p, err) } defer f.Close() @@ -157,18 +157,17 @@ func ParsePathRaw(p string) (string, error) { if err != nil { return "", fmt.Errorf("unable to open yaml file %s: %w", p, err) } - defer f.Close() return ParseReaderRaw(f) } // ParseReader converts an io.Reader into a yaml configuration. -func ParseReader(r io.Reader) (*yaml.Build, []byte, error) { +func ParseReader(r io.Reader) (*yaml.Build, []byte, []string, error) { // read all the bytes from the reader data, err := io.ReadAll(r) if err != nil { - return nil, nil, fmt.Errorf("unable to read bytes for yaml: %w", err) + return nil, nil, nil, fmt.Errorf("unable to read bytes for yaml: %w", err) } return ParseBytes(data) @@ -186,6 +185,6 @@ func ParseReaderRaw(r io.Reader) (string, error) { } // ParseString converts a string into a yaml configuration. -func ParseString(s string) (*yaml.Build, []byte, error) { +func ParseString(s string) (*yaml.Build, []byte, []string, error) { return ParseBytes([]byte(s)) } diff --git a/compiler/native/parse_test.go b/compiler/native/parse_test.go index 1ac5953b9..4ae17ce11 100644 --- a/compiler/native/parse_test.go +++ b/compiler/native/parse_test.go @@ -38,7 +38,7 @@ func TestNative_Parse_Metadata_Bytes(t *testing.T) { t.Errorf("Reading file returned err: %v", err) } - got, _, err := client.Parse(b, "", new(yaml.Template)) + got, _, _, err := client.Parse(b, "", new(yaml.Template)) if err != nil { t.Errorf("Parse returned err: %v", err) } @@ -69,7 +69,7 @@ func TestNative_Parse_Metadata_File(t *testing.T) { defer f.Close() - got, _, err := client.Parse(f, "", new(yaml.Template)) + got, _, _, err := client.Parse(f, "", new(yaml.Template)) if err != nil { t.Errorf("Parse returned err: %v", err) } @@ -84,7 +84,7 @@ func TestNative_Parse_Metadata_Invalid(t *testing.T) { client, _ := FromCLIContext(cli.NewContext(nil, flag.NewFlagSet("test", 0), nil)) // run test - got, _, err := client.Parse(nil, "", new(yaml.Template)) + got, _, _, err := client.Parse(nil, "", new(yaml.Template)) if err == nil { t.Error("Parse should have returned err") @@ -109,7 +109,7 @@ func TestNative_Parse_Metadata_Path(t *testing.T) { } // run test - got, _, err := client.Parse("testdata/metadata.yml", "", new(yaml.Template)) + got, _, _, err := client.Parse("testdata/metadata.yml", "", new(yaml.Template)) if err != nil { t.Errorf("Parse returned err: %v", err) } @@ -138,7 +138,7 @@ func TestNative_Parse_Metadata_Reader(t *testing.T) { t.Errorf("Reading file returned err: %v", err) } - got, _, err := client.Parse(bytes.NewReader(b), "", new(yaml.Template)) + got, _, _, err := client.Parse(bytes.NewReader(b), "", new(yaml.Template)) if err != nil { t.Errorf("Parse returned err: %v", err) } @@ -167,7 +167,7 @@ func TestNative_Parse_Metadata_String(t *testing.T) { t.Errorf("Reading file returned err: %v", err) } - got, _, err := client.Parse(string(b), "", new(yaml.Template)) + got, _, _, err := client.Parse(string(b), "", new(yaml.Template)) if err != nil { t.Errorf("Parse returned err: %v", err) } @@ -215,7 +215,7 @@ func TestNative_Parse_Parameters(t *testing.T) { t.Errorf("Reading file returned err: %v", err) } - got, _, err := client.Parse(b, "", new(yaml.Template)) + got, _, _, err := client.Parse(b, "", new(yaml.Template)) if err != nil { t.Errorf("Parse returned err: %v", err) } @@ -343,7 +343,7 @@ func TestNative_Parse_StagesPipeline(t *testing.T) { t.Errorf("Reading file returned err: %v", err) } - got, _, err := client.Parse(b, "", new(yaml.Template)) + got, _, _, err := client.Parse(b, "", new(yaml.Template)) if err != nil { t.Errorf("Parse returned err: %v", err) } @@ -446,7 +446,7 @@ func TestNative_Parse_StepsPipeline(t *testing.T) { t.Errorf("Reading file returned err: %v", err) } - got, _, err := client.Parse(b, "", new(yaml.Template)) + got, _, _, err := client.Parse(b, "", new(yaml.Template)) if err != nil { t.Errorf("Parse returned err: %v", err) } @@ -516,7 +516,7 @@ func TestNative_Parse_Secrets(t *testing.T) { t.Errorf("Reading file returned err: %v", err) } - got, _, err := client.Parse(b, "", new(yaml.Template)) + got, _, _, err := client.Parse(b, "", new(yaml.Template)) if err != nil { t.Errorf("Parse returned err: %v", err) @@ -593,7 +593,7 @@ func TestNative_Parse_Stages(t *testing.T) { t.Errorf("Reading file returned err: %v", err) } - got, _, err := client.Parse(b, "", new(yaml.Template)) + got, _, _, err := client.Parse(b, "", new(yaml.Template)) if err != nil { t.Errorf("Parse returned err: %v", err) @@ -671,7 +671,7 @@ func TestNative_Parse_StagesLegacyMergeAnchor(t *testing.T) { t.Errorf("Reading file returned err: %v", err) } - got, _, err := client.Parse(b, "", new(yaml.Template)) + got, _, _, err := client.Parse(b, "", new(yaml.Template)) if err != nil { t.Errorf("Parse returned err: %v", err) @@ -730,7 +730,7 @@ func TestNative_Parse_Steps(t *testing.T) { t.Errorf("Reading file returned err: %v", err) } - got, _, err := client.Parse(b, "", new(yaml.Template)) + got, _, _, err := client.Parse(b, "", new(yaml.Template)) if err != nil { t.Errorf("Parse returned err: %v", err) @@ -759,7 +759,7 @@ func TestNative_ParseBytes_Metadata(t *testing.T) { t.Errorf("Reading file returned err: %v", err) } - got, _, err := ParseBytes(b) + got, _, _, err := ParseBytes(b) if err != nil { t.Errorf("ParseBytes returned err: %v", err) @@ -777,7 +777,7 @@ func TestNative_ParseBytes_Invalid(t *testing.T) { t.Errorf("Reading file returned err: %v", err) } - got, _, err := ParseBytes(b) + got, _, _, err := ParseBytes(b) if err == nil { t.Error("ParseBytes should have returned err") @@ -808,7 +808,7 @@ func TestNative_ParseFile_Metadata(t *testing.T) { defer f.Close() - got, _, err := ParseFile(f) + got, _, _, err := ParseFile(f) if err != nil { t.Errorf("ParseFile returned err: %v", err) @@ -828,7 +828,7 @@ func TestNative_ParseFile_Invalid(t *testing.T) { f.Close() - got, _, err := ParseFile(f) + got, _, _, err := ParseFile(f) if err == nil { t.Error("ParseFile should have returned err") @@ -852,7 +852,7 @@ func TestNative_ParsePath_Metadata(t *testing.T) { } // run test - got, _, err := ParsePath("testdata/metadata.yml") + got, _, _, err := ParsePath("testdata/metadata.yml") if err != nil { t.Errorf("ParsePath returned err: %v", err) @@ -865,7 +865,7 @@ func TestNative_ParsePath_Metadata(t *testing.T) { func TestNative_ParsePath_Invalid(t *testing.T) { // run test - got, _, err := ParsePath("testdata/foobar.yml") + got, _, _, err := ParsePath("testdata/foobar.yml") if err == nil { t.Error("ParsePath should have returned err") @@ -894,7 +894,7 @@ func TestNative_ParseReader_Metadata(t *testing.T) { t.Errorf("Reading file returned err: %v", err) } - got, _, err := ParseReader(bytes.NewReader(b)) + got, _, _, err := ParseReader(bytes.NewReader(b)) if err != nil { t.Errorf("ParseReader returned err: %v", err) @@ -907,7 +907,7 @@ func TestNative_ParseReader_Metadata(t *testing.T) { func TestNative_ParseReader_Invalid(t *testing.T) { // run test - got, _, err := ParseReader(FailReader{}) + got, _, _, err := ParseReader(FailReader{}) if err == nil { t.Error("ParseFile should have returned err") @@ -936,7 +936,7 @@ func TestNative_ParseString_Metadata(t *testing.T) { t.Errorf("Reading file returned err: %v", err) } - got, _, err := ParseString(string(b)) + got, _, _, err := ParseString(string(b)) if err != nil { t.Errorf("ParseString returned err: %v", err) @@ -1012,7 +1012,7 @@ func Test_client_Parse(t *testing.T) { } } - got, _, err := c.Parse(content, tt.args.pipelineType, new(yaml.Template)) + got, _, _, err := c.Parse(content, tt.args.pipelineType, new(yaml.Template)) if (err != nil) != tt.wantErr { t.Errorf("Parse() error = %v, wantErr %v", err, tt.wantErr) return diff --git a/compiler/template/native/render.go b/compiler/template/native/render.go index 1004d62d1..41ff0560a 100644 --- a/compiler/template/native/render.go +++ b/compiler/template/native/render.go @@ -15,7 +15,7 @@ import ( ) // 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{}) (*types.Build, []string, error) { buffer := new(bytes.Buffer) velaFuncs := funcHandler{envs: convertPlatformVars(environment, name)} @@ -30,25 +30,24 @@ func Render(tmpl string, name string, tName string, environment raw.StringSliceM sf := sprig.TxtFuncMap() delete(sf, "env") delete(sf, "expandenv") - // parse the template with Masterminds/sprig functions // // https://pkg.go.dev/github.com/Masterminds/sprig?tab=doc#TxtFuncMap t, err := template.New(name).Funcs(sf).Funcs(templateFuncMap).Parse(tmpl) if err != nil { - return nil, fmt.Errorf("unable to parse template %s: %w", tName, err) + return nil, nil, fmt.Errorf("unable to parse template %s: %w", tName, err) } // apply the variables to the parsed template err = t.Execute(buffer, variables) if err != nil { - return nil, fmt.Errorf("unable to execute template %s: %w", tName, err) + return nil, nil, fmt.Errorf("unable to execute template %s: %w", tName, err) } // unmarshal the template to the pipeline - config, err := internal.ParseYAML(buffer.Bytes()) + config, warnings, err := internal.ParseYAML(buffer.Bytes()) if err != nil { - return nil, fmt.Errorf("unable to unmarshal yaml: %w", err) + return nil, nil, fmt.Errorf("unable to unmarshal yaml: %w", err) } // ensure all templated steps have template prefix @@ -56,11 +55,21 @@ func Render(tmpl string, name string, tName string, environment raw.StringSliceM config.Steps[index].Name = fmt.Sprintf("%s_%s", name, newStep.Name) } - return &types.Build{Metadata: config.Metadata, Steps: config.Steps, Secrets: config.Secrets, Services: config.Services, Environment: config.Environment, Templates: config.Templates, Deployment: config.Deployment}, nil + return &types.Build{ + Metadata: config.Metadata, + Steps: config.Steps, + Secrets: config.Secrets, + Services: config.Services, + Environment: config.Environment, + Templates: config.Templates, + Deployment: config.Deployment, + }, + warnings, + nil } // RenderBuild renders the templated build. -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{}) (*types.Build, []string, error) { buffer := new(bytes.Buffer) velaFuncs := funcHandler{envs: convertPlatformVars(envs, tmpl)} @@ -75,26 +84,25 @@ func RenderBuild(tmpl string, b string, envs map[string]string, variables map[st sf := sprig.TxtFuncMap() delete(sf, "env") delete(sf, "expandenv") - // parse the template with Masterminds/sprig functions // // https://pkg.go.dev/github.com/Masterminds/sprig?tab=doc#TxtFuncMap t, err := template.New(tmpl).Funcs(sf).Funcs(templateFuncMap).Parse(b) if err != nil { - return nil, err + return nil, nil, err } // execute the template err = t.Execute(buffer, variables) if err != nil { - return nil, fmt.Errorf("unable to execute template: %w", err) + return nil, nil, fmt.Errorf("unable to execute template: %w", err) } // unmarshal the template to the pipeline - config, err := internal.ParseYAML(buffer.Bytes()) + config, warnings, err := internal.ParseYAML(buffer.Bytes()) if err != nil { - return nil, fmt.Errorf("unable to unmarshal yaml: %w", err) + return nil, nil, fmt.Errorf("unable to unmarshal yaml: %w", err) } - return config, nil + return config, warnings, nil } diff --git a/compiler/template/native/render_test.go b/compiler/template/native/render_test.go index d204f7572..822af98ae 100644 --- a/compiler/template/native/render_test.go +++ b/compiler/template/native/render_test.go @@ -59,7 +59,7 @@ func TestNative_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) if (err != nil) != tt.wantErr { t.Errorf("Render() error = %v, wantErr %v", err, tt.wantErr) return @@ -120,7 +120,7 @@ func TestNative_RenderBuild(t *testing.T) { t.Error(err) } - got, err := RenderBuild("build", string(sFile), map[string]string{ + got, _, err := RenderBuild("build", string(sFile), map[string]string{ "VELA_REPO_FULL_NAME": "octocat/hello-world", "VELA_BUILD_BRANCH": "main", }, map[string]interface{}{}) diff --git a/compiler/template/starlark/render.go b/compiler/template/starlark/render.go index 1abc52319..d9c1e4bd0 100644 --- a/compiler/template/starlark/render.go +++ b/compiler/template/starlark/render.go @@ -31,11 +31,11 @@ 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{}, limit int64) (*types.Build, error) { +func Render(tmpl string, name string, tName string, environment raw.StringSliceMap, variables map[string]interface{}, limit int64) (*types.Build, []string, error) { thread := &starlark.Thread{Name: name} if limit < 0 { - return nil, fmt.Errorf("starlark exec limit must be non-negative") + return nil, nil, fmt.Errorf("starlark exec limit must be non-negative") } thread.SetMaxExecutionSteps(uint64(limit)) @@ -44,31 +44,31 @@ func Render(tmpl string, name string, tName string, environment raw.StringSliceM globals, err := starlark.ExecFileOptions(syntax.LegacyFileOptions(), thread, "templated-base", tmpl, predeclared) if err != nil { - return nil, err + return nil, nil, err } // check the provided template has a main function mainVal, ok := globals["main"] if !ok { - return nil, fmt.Errorf("%w: %s", ErrMissingMainFunc, tName) + return nil, nil, fmt.Errorf("%w: %s", ErrMissingMainFunc, tName) } // check the provided main is a function main, ok := mainVal.(starlark.Callable) if !ok { - return nil, fmt.Errorf("%w: %s", ErrInvalidMainFunc, tName) + return nil, nil, fmt.Errorf("%w: %s", ErrInvalidMainFunc, tName) } // load the user provided vars into a starlark type userVars, err := convertTemplateVars(variables) if err != nil { - return nil, err + return nil, nil, err } // load the platform provided vars into a starlark type velaVars, err := convertPlatformVars(environment, name) if err != nil { - return nil, err + return nil, nil, err } // add the user and platform vars to a context to be used @@ -77,12 +77,12 @@ func Render(tmpl string, name string, tName string, environment raw.StringSliceM err = context.SetKey(starlark.String("vela"), velaVars) if err != nil { - return nil, err + return nil, nil, err } err = context.SetKey(starlark.String("vars"), userVars) if err != nil { - return nil, err + return nil, nil, err } args := starlark.Tuple([]starlark.Value{context}) @@ -90,11 +90,10 @@ func Render(tmpl string, name string, tName string, environment raw.StringSliceM // execute Starlark program from Go. mainVal, err = starlark.Call(thread, main, args, nil) if err != nil { - return nil, err + return nil, nil, err } buf := new(bytes.Buffer) - // extract the pipeline from the starlark program switch v := mainVal.(type) { case *starlark.List: @@ -105,7 +104,7 @@ func Render(tmpl string, name string, tName string, environment raw.StringSliceM err = writeJSON(buf, item) if err != nil { - return nil, err + return nil, nil, err } buf.WriteString("\n") @@ -115,16 +114,16 @@ func Render(tmpl string, name string, tName string, environment raw.StringSliceM err = writeJSON(buf, v) if err != nil { - return nil, err + return nil, nil, err } default: - return nil, fmt.Errorf("%w: %s", ErrInvalidPipelineReturn, mainVal.Type()) + return nil, nil, fmt.Errorf("%w: %s", ErrInvalidPipelineReturn, mainVal.Type()) } // unmarshal the template to the pipeline - config, err := internal.ParseYAML(buf.Bytes()) + config, warnings, err := internal.ParseYAML(buf.Bytes()) if err != nil { - return nil, fmt.Errorf("unable to unmarshal yaml: %w", err) + return nil, nil, fmt.Errorf("unable to unmarshal yaml: %w", err) } // ensure all templated steps have template prefix @@ -132,17 +131,25 @@ func Render(tmpl string, name string, tName string, environment raw.StringSliceM config.Steps[index].Name = fmt.Sprintf("%s_%s", name, newStep.Name) } - return &types.Build{Steps: config.Steps, Secrets: config.Secrets, Services: config.Services, Environment: config.Environment}, nil + return &types.Build{ + Steps: config.Steps, + Secrets: config.Secrets, + Services: config.Services, + Environment: config.Environment, + Deployment: config.Deployment, + }, + warnings, + nil } // 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{}, limit int64) (*types.Build, error) { +func RenderBuild(tmpl string, b string, envs map[string]string, variables map[string]interface{}, limit int64) (*types.Build, []string, error) { thread := &starlark.Thread{Name: "templated-base"} if limit < 0 { - return nil, fmt.Errorf("starlark exec limit must be non-negative") + return nil, nil, fmt.Errorf("starlark exec limit must be non-negative") } thread.SetMaxExecutionSteps(uint64(limit)) @@ -151,31 +158,31 @@ func RenderBuild(tmpl string, b string, envs map[string]string, variables map[st globals, err := starlark.ExecFileOptions(syntax.LegacyFileOptions(), thread, "templated-base", b, predeclared) if err != nil { - return nil, err + return nil, nil, err } // check the provided template has a main function mainVal, ok := globals["main"] if !ok { - return nil, fmt.Errorf("%w: %s", ErrMissingMainFunc, "templated-base") + return nil, nil, fmt.Errorf("%w: %s", ErrMissingMainFunc, "templated-base") } // check the provided main is a function main, ok := mainVal.(starlark.Callable) if !ok { - return nil, fmt.Errorf("%w: %s", ErrInvalidMainFunc, "templated-base") + return nil, nil, fmt.Errorf("%w: %s", ErrInvalidMainFunc, "templated-base") } // load the user provided vars into a starlark type userVars, err := convertTemplateVars(variables) if err != nil { - return nil, err + return nil, nil, err } // load the platform provided vars into a starlark type velaVars, err := convertPlatformVars(envs, tmpl) if err != nil { - return nil, err + return nil, nil, err } // add the user and platform vars to a context to be used @@ -184,12 +191,12 @@ func RenderBuild(tmpl string, b string, envs map[string]string, variables map[st err = context.SetKey(starlark.String("vela"), velaVars) if err != nil { - return nil, err + return nil, nil, err } err = context.SetKey(starlark.String("vars"), userVars) if err != nil { - return nil, err + return nil, nil, err } args := starlark.Tuple([]starlark.Value{context}) @@ -197,11 +204,10 @@ func RenderBuild(tmpl string, b string, envs map[string]string, variables map[st // execute Starlark program from Go. mainVal, err = starlark.Call(thread, main, args, nil) if err != nil { - return nil, err + return nil, nil, err } buf := new(bytes.Buffer) - // extract the pipeline from the starlark program switch v := mainVal.(type) { case *starlark.List: @@ -212,7 +218,7 @@ func RenderBuild(tmpl string, b string, envs map[string]string, variables map[st err = writeJSON(buf, item) if err != nil { - return nil, err + return nil, nil, err } buf.WriteString("\n") @@ -222,17 +228,17 @@ func RenderBuild(tmpl string, b string, envs map[string]string, variables map[st err = writeJSON(buf, v) if err != nil { - return nil, err + return nil, nil, err } default: - return nil, fmt.Errorf("%w: %s", ErrInvalidPipelineReturn, mainVal.Type()) + return nil, nil, fmt.Errorf("%w: %s", ErrInvalidPipelineReturn, mainVal.Type()) } // unmarshal the template to the pipeline - config, err := internal.ParseYAML(buf.Bytes()) + config, warnings, err := internal.ParseYAML(buf.Bytes()) if err != nil { - return nil, fmt.Errorf("unable to unmarshal yaml: %w", err) + return nil, nil, fmt.Errorf("unable to unmarshal yaml: %w", err) } - return config, nil + return config, warnings, nil } diff --git a/compiler/template/starlark/render_test.go b/compiler/template/starlark/render_test.go index b9d184dae..fdab8657d 100644 --- a/compiler/template/starlark/render_test.go +++ b/compiler/template/starlark/render_test.go @@ -92,7 +92,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, 7500) + 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 @@ -207,7 +207,7 @@ func TestNative_RenderBuild(t *testing.T) { t.Error(err) } - got, err := RenderBuild("build", string(sFile), map[string]string{ + got, _, err := RenderBuild("build", string(sFile), map[string]string{ "VELA_REPO_FULL_NAME": "octocat/hello-world", "VELA_BUILD_BRANCH": "main", "VELA_REPO_ORG": "octocat", diff --git a/constants/limit.go b/constants/limit.go index e3806a153..aad72e75c 100644 --- a/constants/limit.go +++ b/constants/limit.go @@ -54,4 +54,7 @@ const ( // DashboardAdminMaxSize defines the maximum size in characters for dashboard admins. DashboardAdminMaxSize = 5000 + + // PipelineWarningsMaxSize defines the maximum size in characters for the pipeline warnings. + PipelineWarningsMaxSize = 5000 ) diff --git a/database/integration_test.go b/database/integration_test.go index a0f1ad90c..c1073df7d 100644 --- a/database/integration_test.go +++ b/database/integration_test.go @@ -2788,6 +2788,7 @@ func newResources() *Resources { pipelineOne.SetStages(false) pipelineOne.SetSteps(true) pipelineOne.SetTemplates(false) + pipelineOne.SetWarnings([]string{}) pipelineOne.SetData([]byte("version: 1")) pipelineTwo := new(api.Pipeline) @@ -2805,6 +2806,7 @@ func newResources() *Resources { pipelineTwo.SetStages(false) pipelineTwo.SetSteps(true) pipelineTwo.SetTemplates(false) + pipelineTwo.SetWarnings([]string{"42:this is a warning"}) pipelineTwo.SetData([]byte("version: 1")) currTime := time.Now().UTC() diff --git a/database/pipeline/create_test.go b/database/pipeline/create_test.go index 5768e6f88..a01d7d605 100644 --- a/database/pipeline/create_test.go +++ b/database/pipeline/create_test.go @@ -34,9 +34,9 @@ func TestPipeline_Engine_CreatePipeline(t *testing.T) { // ensure the mock expects the query _mock.ExpectQuery(`INSERT INTO "pipelines" -("repo_id","commit","flavor","platform","ref","type","version","external_secrets","internal_secrets","services","stages","steps","templates","data","id") -VALUES ($1,$2,$3,$4,$5,$6,$7,$8,$9,$10,$11,$12,$13,$14,$15) RETURNING "id"`). - WithArgs(1, "48afb5bdc41ad69bf22588491333f7cf71135163", nil, nil, "refs/heads/main", "yaml", "1", false, false, false, false, false, false, AnyArgument{}, 1). +("repo_id","commit","flavor","platform","ref","type","version","external_secrets","internal_secrets","services","stages","steps","templates","warnings","data","id") +VALUES ($1,$2,$3,$4,$5,$6,$7,$8,$9,$10,$11,$12,$13,$14,$15,$16) RETURNING "id"`). + WithArgs(1, "48afb5bdc41ad69bf22588491333f7cf71135163", nil, nil, "refs/heads/main", "yaml", "1", false, false, false, false, false, false, nil, AnyArgument{}, 1). WillReturnRows(_rows) _sqlite := testSqlite(t) diff --git a/database/pipeline/table.go b/database/pipeline/table.go index 4bd874c83..f09aa96ac 100644 --- a/database/pipeline/table.go +++ b/database/pipeline/table.go @@ -28,6 +28,7 @@ pipelines ( stages BOOLEAN, steps BOOLEAN, templates BOOLEAN, + warnings VARCHAR(5000), data BYTEA, UNIQUE(repo_id, commit) ); @@ -52,6 +53,7 @@ pipelines ( stages BOOLEAN, steps BOOLEAN, templates BOOLEAN, + warnings TEXT, data BLOB, UNIQUE(repo_id, 'commit') ); diff --git a/database/pipeline/update_test.go b/database/pipeline/update_test.go index e8be0e457..ae5de556e 100644 --- a/database/pipeline/update_test.go +++ b/database/pipeline/update_test.go @@ -31,9 +31,9 @@ func TestPipeline_Engine_UpdatePipeline(t *testing.T) { // ensure the mock expects the query _mock.ExpectExec(`UPDATE "pipelines" -SET "repo_id"=$1,"commit"=$2,"flavor"=$3,"platform"=$4,"ref"=$5,"type"=$6,"version"=$7,"external_secrets"=$8,"internal_secrets"=$9,"services"=$10,"stages"=$11,"steps"=$12,"templates"=$13,"data"=$14 -WHERE "id" = $15`). - WithArgs(1, "48afb5bdc41ad69bf22588491333f7cf71135163", nil, nil, "refs/heads/main", "yaml", "1", false, false, false, false, false, false, AnyArgument{}, 1). +SET "repo_id"=$1,"commit"=$2,"flavor"=$3,"platform"=$4,"ref"=$5,"type"=$6,"version"=$7,"external_secrets"=$8,"internal_secrets"=$9,"services"=$10,"stages"=$11,"steps"=$12,"templates"=$13,"warnings"=$14,"data"=$15 +WHERE "id" = $16`). + WithArgs(1, "48afb5bdc41ad69bf22588491333f7cf71135163", nil, nil, "refs/heads/main", "yaml", "1", false, false, false, false, false, false, nil, AnyArgument{}, 1). WillReturnResult(sqlmock.NewResult(1, 1)) _sqlite := testSqlite(t) diff --git a/database/testutils/api_resources.go b/database/testutils/api_resources.go index 7689a5f4e..c33ab256c 100644 --- a/database/testutils/api_resources.go +++ b/database/testutils/api_resources.go @@ -270,6 +270,7 @@ func APIPipeline() *api.Pipeline { Stages: new(bool), Steps: new(bool), Templates: new(bool), + Warnings: new([]string), Data: new([]byte), } } diff --git a/database/types/pipeline.go b/database/types/pipeline.go index 425f193e0..b76657058 100644 --- a/database/types/pipeline.go +++ b/database/types/pipeline.go @@ -6,7 +6,10 @@ import ( "database/sql" "errors" + "github.com/lib/pq" + api "github.com/go-vela/server/api/types" + "github.com/go-vela/server/constants" "github.com/go-vela/server/util" ) @@ -30,6 +33,10 @@ var ( // ErrEmptyPipelineVersion defines the error type when a // Pipeline type has an empty Version field provided. ErrEmptyPipelineVersion = errors.New("empty pipeline version provided") + + // ErrExceededWarningsLimit defines the error type when a + // Pipeline warnings field has too many total characters. + ErrExceededWarningsLimit = errors.New("exceeded character limit for pipeline warnings") ) // Pipeline is the database representation of a pipeline. @@ -48,6 +55,7 @@ type Pipeline struct { Stages sql.NullBool `sql:"stages"` Steps sql.NullBool `sql:"steps"` Templates sql.NullBool `sql:"templates"` + Warnings pq.StringArray `sql:"warnings" gorm:"type:varchar(5000)"` Data []byte `sql:"data"` Repo Repo `gorm:"foreignKey:RepoID"` @@ -160,6 +168,7 @@ func (p *Pipeline) ToAPI() *api.Pipeline { pipeline.SetStages(p.Stages.Bool) pipeline.SetSteps(p.Steps.Bool) pipeline.SetTemplates(p.Templates.Bool) + pipeline.SetWarnings(p.Warnings) pipeline.SetData(p.Data) return pipeline @@ -193,6 +202,19 @@ func (p *Pipeline) Validate() error { return ErrEmptyPipelineVersion } + // calculate total size of favorites + total := 0 + for _, w := range p.Warnings { + total += len(w) + } + + // verify the Favorites field is within the database constraints + // len is to factor in number of comma separators included in the database field, + // removing 1 due to the last item not having an appended comma + if (total + len(p.Warnings) - 1) > constants.PipelineWarningsMaxSize { + return ErrExceededWarningsLimit + } + // ensure that all Pipeline string fields // that can be returned as JSON are sanitized // to avoid unsafe HTML content @@ -224,6 +246,7 @@ func PipelineFromAPI(p *api.Pipeline) *Pipeline { Stages: sql.NullBool{Bool: p.GetStages(), Valid: true}, Steps: sql.NullBool{Bool: p.GetSteps(), Valid: true}, Templates: sql.NullBool{Bool: p.GetTemplates(), Valid: true}, + Warnings: pq.StringArray(p.GetWarnings()), Data: p.GetData(), } diff --git a/database/types/pipeline_test.go b/database/types/pipeline_test.go index 659e3fb9c..2a03afff1 100644 --- a/database/types/pipeline_test.go +++ b/database/types/pipeline_test.go @@ -285,6 +285,7 @@ func TestDatabase_Pipeline_ToAPI(t *testing.T) { want.SetStages(false) want.SetSteps(true) want.SetTemplates(false) + want.SetWarnings([]string{"42:this is a warning"}) want.SetData(testPipelineData()) // run test @@ -393,6 +394,7 @@ func TestDatabase_PipelineFromAPI(t *testing.T) { Stages: sql.NullBool{Bool: false, Valid: true}, Steps: sql.NullBool{Bool: true, Valid: true}, Templates: sql.NullBool{Bool: false, Valid: true}, + Warnings: []string{"42:this is a warning"}, Data: testPipelineData(), } @@ -412,6 +414,7 @@ func TestDatabase_PipelineFromAPI(t *testing.T) { p.SetStages(false) p.SetSteps(true) p.SetTemplates(false) + p.SetWarnings([]string{"42:this is a warning"}) p.SetData(testPipelineData()) // run test @@ -440,6 +443,7 @@ func testPipeline() *Pipeline { Stages: sql.NullBool{Bool: false, Valid: true}, Steps: sql.NullBool{Bool: true, Valid: true}, Templates: sql.NullBool{Bool: false, Valid: true}, + Warnings: []string{"42:this is a warning"}, Data: testPipelineData(), Repo: *testRepo(), diff --git a/internal/yaml.go b/internal/yaml.go index 363e9c9c3..34add6d9d 100644 --- a/internal/yaml.go +++ b/internal/yaml.go @@ -14,19 +14,20 @@ import ( ) // ParseYAML is a helper function for transitioning teams away from legacy buildkite YAML parser. -func ParseYAML(data []byte) (*types.Build, error) { +func ParseYAML(data []byte) (*types.Build, []string, error) { var ( rootNode yaml.Node + warnings []string version string ) err := yaml.Unmarshal(data, &rootNode) if err != nil { - return nil, fmt.Errorf("unable to unmarshal pipeline version yaml: %w", err) + return nil, nil, fmt.Errorf("unable to unmarshal pipeline version yaml: %w", err) } if len(rootNode.Content) == 0 || rootNode.Content[0].Kind != yaml.MappingNode { - return nil, fmt.Errorf("unable to find pipeline version in yaml") + return nil, nil, fmt.Errorf("unable to find pipeline version in yaml") } for i, subNode := range rootNode.Content[0].Content { @@ -47,11 +48,13 @@ func ParseYAML(data []byte) (*types.Build, error) { err := bkYaml.Unmarshal(data, legacyConfig) if err != nil { - return nil, fmt.Errorf("unable to unmarshal legacy yaml: %w", err) + return nil, nil, fmt.Errorf("unable to unmarshal legacy yaml: %w", err) } config = legacyConfig.ToYAML() + warnings = append(warnings, "using legacy version. Upgrade to go-yaml v3") + default: // unmarshal the bytes into the yaml configuration err := yaml.Unmarshal(data, config) @@ -63,31 +66,31 @@ func ParseYAML(data []byte) (*types.Build, error) { if err := yaml.Unmarshal(data, root); err != nil { fmt.Println("error unmarshalling YAML:", err) - return nil, err + return nil, nil, err } - collapseMergeAnchors(root.Content[0]) + warnings = collapseMergeAnchors(root.Content[0], warnings) newData, err := yaml.Marshal(root) if err != nil { - return nil, err + return nil, nil, err } err = yaml.Unmarshal(newData, config) if err != nil { - return nil, fmt.Errorf("unable to unmarshal yaml: %w", err) + return nil, nil, fmt.Errorf("unable to unmarshal yaml: %w", err) } } else { - return nil, fmt.Errorf("unable to unmarshal yaml: %w", err) + return nil, nil, fmt.Errorf("unable to unmarshal yaml: %w", err) } } } - return config, nil + return config, warnings, nil } // collapseMergeAnchors traverses the entire pipeline and replaces duplicate `<<` keys with a single key->sequence. -func collapseMergeAnchors(node *yaml.Node) { +func collapseMergeAnchors(node *yaml.Node, warnings []string) []string { // only replace on maps if node.Kind == yaml.MappingNode { var ( @@ -129,17 +132,20 @@ func collapseMergeAnchors(node *yaml.Node) { for i := len(keysToRemove) - 1; i >= 0; i-- { index := keysToRemove[i] + warnings = append(warnings, fmt.Sprintf("%d:duplicate << keys in single YAML map", node.Content[index].Line)) node.Content = append(node.Content[:index], node.Content[index+2:]...) } } // go to next level for _, content := range node.Content { - collapseMergeAnchors(content) + warnings = collapseMergeAnchors(content, warnings) } } else if node.Kind == yaml.SequenceNode { for _, item := range node.Content { - collapseMergeAnchors(item) + warnings = collapseMergeAnchors(item, warnings) } } + + return warnings } diff --git a/internal/yaml_test.go b/internal/yaml_test.go index 0a12c6e44..e91ccaa21 100644 --- a/internal/yaml_test.go +++ b/internal/yaml_test.go @@ -4,6 +4,7 @@ package internal import ( "os" + "reflect" "testing" "github.com/google/go-cmp/cmp" @@ -35,30 +36,39 @@ func TestInternal_ParseYAML(t *testing.T) { // set up tests tests := []struct { - file string - want *yaml.Build - wantErr bool + name string + file string + wantBuild *yaml.Build + wantWarnings []string + wantErr bool }{ { - file: "testdata/go-yaml.yml", - want: wantBuild, + name: "go-yaml", + file: "testdata/go-yaml.yml", + wantBuild: wantBuild, }, { - file: "testdata/buildkite.yml", - want: wantBuild, + name: "buildkite legacy", + file: "testdata/buildkite.yml", + wantBuild: wantBuild, + wantWarnings: []string{"using legacy version. Upgrade to go-yaml v3"}, }, { - file: "testdata/buildkite_new_version.yml", - want: wantBuild, + name: "anchor collapse", + file: "testdata/buildkite_new_version.yml", + wantBuild: wantBuild, + wantWarnings: []string{"16:duplicate << keys in single YAML map"}, }, { - file: "testdata/no_version.yml", - want: wantBuild, + name: "no version", + file: "testdata/no_version.yml", + wantBuild: wantBuild, }, { - file: "testdata/invalid.yml", - want: nil, - wantErr: true, + name: "invalid yaml", + file: "testdata/invalid.yml", + wantBuild: nil, + wantErr: true, }, } @@ -66,16 +76,16 @@ func TestInternal_ParseYAML(t *testing.T) { for _, test := range tests { bytes, err := os.ReadFile(test.file) if err != nil { - t.Errorf("unable to read file: %v", err) + t.Errorf("unable to read file for test %s: %v", test.name, err) } - gotBuild, err := ParseYAML(bytes) + gotBuild, gotWarnings, err := ParseYAML(bytes) if err != nil && !test.wantErr { - t.Errorf("ParseYAML returned err: %v", err) + t.Errorf("ParseYAML for test %s returned err: %v", test.name, err) } if err == nil && test.wantErr { - t.Errorf("ParseYAML returned nil error") + t.Errorf("ParseYAML for test %s returned nil error", test.name) } if err != nil && test.wantErr { @@ -85,8 +95,12 @@ func TestInternal_ParseYAML(t *testing.T) { // different versions expected wantBuild.Version = gotBuild.Version - if diff := cmp.Diff(gotBuild, test.want); diff != "" { - t.Errorf("ParseYAML returned diff (-got +want):\n%s", diff) + if diff := cmp.Diff(gotBuild, test.wantBuild); diff != "" { + t.Errorf("ParseYAML for test %s returned diff (-got +want):\n%s", test.name, diff) + } + + if !reflect.DeepEqual(gotWarnings, test.wantWarnings) { + t.Errorf("ParseYAML for test %s returned warnings %v, want %v", test.name, gotWarnings, test.wantWarnings) } } } diff --git a/mock/server/pipeline.go b/mock/server/pipeline.go index f4d65c596..a55fc1e93 100644 --- a/mock/server/pipeline.go +++ b/mock/server/pipeline.go @@ -169,6 +169,9 @@ templates: "stages": false, "steps": true, "templates": false, + "warnings": [ + "42:this is a warning" + ], "data": "LS0tCnZlcnNpb246ICIxIgoKc3RlcHM6CiAgLSBuYW1lOiBlY2hvCiAgICBpbWFnZTogYWxwaW5lOmxhdGVzdAogICAgY29tbWFuZHM6IFtlY2hvIGZvb10=" }` diff --git a/router/middleware/pipeline/pipeline_test.go b/router/middleware/pipeline/pipeline_test.go index a12dd544b..3d305225c 100644 --- a/router/middleware/pipeline/pipeline_test.go +++ b/router/middleware/pipeline/pipeline_test.go @@ -98,6 +98,7 @@ func TestPipeline_Establish(t *testing.T) { want.SetStages(false) want.SetSteps(false) want.SetTemplates(false) + want.SetWarnings([]string{}) want.SetData([]byte{}) got := new(api.Pipeline) From 685bb5a0dafc2354c08e407fabbf96cd576cb0be Mon Sep 17 00:00:00 2001 From: ecrupper Date: Tue, 31 Dec 2024 10:24:15 -0600 Subject: [PATCH 2/2] fix typos --- database/types/pipeline.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/database/types/pipeline.go b/database/types/pipeline.go index b76657058..f5edfd7c7 100644 --- a/database/types/pipeline.go +++ b/database/types/pipeline.go @@ -202,13 +202,13 @@ func (p *Pipeline) Validate() error { return ErrEmptyPipelineVersion } - // calculate total size of favorites + // calculate total size of warnings total := 0 for _, w := range p.Warnings { total += len(w) } - // verify the Favorites field is within the database constraints + // verify the Warnings field is within the database constraints // len is to factor in number of comma separators included in the database field, // removing 1 due to the last item not having an appended comma if (total + len(p.Warnings) - 1) > constants.PipelineWarningsMaxSize {