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

feat(pipeline)!: add warnings to pipelines that use legacy YAML lib or have dupl anchors in map #1232

Merged
merged 3 commits into from
Jan 6, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion api/pipeline/template.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))

Expand Down
57 changes: 43 additions & 14 deletions api/types/pipeline.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -436,6 +463,7 @@ func (p *Pipeline) String() string {
Templates: %t,
Type: %s,
Version: %s,
Warnings: %v,
}`,
p.GetCommit(),
p.GetData(),
Expand All @@ -452,5 +480,6 @@ func (p *Pipeline) String() string {
p.GetTemplates(),
p.GetType(),
p.GetVersion(),
p.GetWarnings(),
)
}
12 changes: 12 additions & 0 deletions api/types/pipeline_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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())
}
Expand All @@ -207,6 +216,7 @@ func TestAPI_Pipeline_String(t *testing.T) {
Templates: %t,
Type: %s,
Version: %s,
Warnings: %v,
}`,
p.GetCommit(),
p.GetData(),
Expand All @@ -223,6 +233,7 @@ func TestAPI_Pipeline_String(t *testing.T) {
p.GetTemplates(),
p.GetType(),
p.GetVersion(),
p.GetWarnings(),
)

// run test
Expand Down Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
8 changes: 5 additions & 3 deletions compiler/native/compile.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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)
Expand Down Expand Up @@ -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
}
Expand All @@ -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())
Expand Down Expand Up @@ -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
}
Expand Down
12 changes: 6 additions & 6 deletions compiler/native/expand.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@
// 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
}
Expand Down Expand Up @@ -247,7 +247,7 @@
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
}
Expand Down Expand Up @@ -391,9 +391,9 @@
}

//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", "":

Check failure on line 396 in compiler/native/expand.go

View workflow job for this annotation

GitHub Actions / diff-review

string `golang` has 3 occurrences, make it a constant (goconst)

Check failure on line 396 in compiler/native/expand.go

View workflow job for this annotation

GitHub Actions / diff-review

string `golang` has 3 occurrences, make it a constant (goconst)

Check failure on line 396 in compiler/native/expand.go

View workflow job for this annotation

GitHub Actions / full-review

string `golang` has 3 occurrences, make it a constant (goconst)

Check failure on line 396 in compiler/native/expand.go

View workflow job for this annotation

GitHub Actions / full-review

string `golang` has 3 occurrences, make it a constant (goconst)

Check failure on line 396 in compiler/native/expand.go

View workflow job for this annotation

GitHub Actions / golangci

[golangci] compiler/native/expand.go#L396

string `golang` has 3 occurrences, make it a constant (goconst)
Raw output
compiler/native/expand.go:396:33: string `golang` has 3 occurrences, make it a constant (goconst)
	case constants.PipelineTypeGo, "golang", "":
	                               ^

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 [golangci] reported by reviewdog 🐶
string golang has 3 occurrences, make it a constant (goconst)

//nolint:lll // ignore long line length due to return
return native.Render(string(bytes), step.Name, step.Template.Name, step.Environment, step.Template.Variables)
case constants.PipelineTypeStarlark:
Expand All @@ -401,11 +401,11 @@
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
Expand All @@ -415,7 +415,7 @@
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)
}
}

Expand Down
47 changes: 23 additions & 24 deletions compiler/native/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,40 +40,41 @@ 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 {
case constants.PipelineTypeGo, "golang":
// 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) {
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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)
}

Expand All @@ -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()
Expand All @@ -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)
Expand All @@ -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))
}
Loading
Loading