From d7f8401ab1f0f810d4576d6c6a36fcaca9e985c3 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Fri, 28 Oct 2022 10:44:00 -0700 Subject: [PATCH 1/5] internal/config: Update Step and hclStep to be lazy decoded This commit updates the pipeline parsing for steps to store step values in the Step struct rather than the hclStep raw struct. This is intended to support lazy loading for when Waypoint resolves input variables later on during config parsing. --- internal/config/pipeline.go | 83 +++++++++++++++++++------------- internal/config/pipeline_test.go | 1 + internal/config/stages.go | 25 ---------- internal/config/validate.go | 12 ++--- internal/core/project.go | 3 +- 5 files changed, 58 insertions(+), 66 deletions(-) diff --git a/internal/config/pipeline.go b/internal/config/pipeline.go index 5e03eef9972..9560e344504 100644 --- a/internal/config/pipeline.go +++ b/internal/config/pipeline.go @@ -32,8 +32,12 @@ type hclPipeline struct { Remain hcl.Body `hcl:",remain"` } -// hclStep represents a raw HCL version of a step stanza in a pipeline config -type hclStep struct { +// Step are the step settings for pipelines +type Step struct { + Labels map[string]string `hcl:"labels,optional"` + Use *Use `hcl:"use,block"` + + // Give this step a name Name string `hcl:",label"` // If set, this step will depend on the defined step. The default step @@ -44,14 +48,24 @@ type hclStep struct { // The OCI image to use for executing this step ImageURL string `hcl:"image_url,optional"` - // The plugin to use for this Step - Use *Use `hcl:"use,block"` + // An optional embedded pipeline stanza + Pipeline *Pipeline `hcl:"pipeline,block"` + + ctx *hcl.EvalContext + + // Optional workspace scoping + Workspace string `hcl:"workspace,optional"` +} + +// hclStep represents a raw HCL version of a step stanza in a pipeline config +type hclStep struct { + Name string `hcl:",label"` // An optional embedded pipeline stanza PipelineRaw *hclPipeline `hcl:"pipeline,block"` - // An optional embedded pipeline stanza - Workspace string `hcl:"workspace,optional"` + Body hcl.Body `hcl:",body"` + Remain hcl.Body `hcl:",remain"` } // Pipelines returns the id of all the defined pipelines @@ -99,14 +113,12 @@ func (c *Config) Pipeline(id string, ctx *hcl.EvalContext) (*Pipeline, error) { var steps []*Step for _, stepRaw := range pipeline.StepRaw { // turn stepRaw into a staged Step - s := Step{ - ctx: ctx, - Name: stepRaw.Name, - DependsOn: stepRaw.DependsOn, - ImageURL: stepRaw.ImageURL, - Use: stepRaw.Use, - Workspace: stepRaw.Workspace, + var step Step + if diag := gohcl.DecodeBody(stepRaw.Body, finalizeContext(ctx), &step); diag.HasErrors() { + return nil, diag } + step.ctx = ctx + step.Name = stepRaw.Name // Parse a nested pipeline step if defined // TODO(briancain): At the moment, we're supporting singly nestested Pipeline @@ -118,13 +130,13 @@ func (c *Config) Pipeline(id string, ctx *hcl.EvalContext) (*Pipeline, error) { return nil, diag } - s.Pipeline = &Pipeline{ + step.Pipeline = &Pipeline{ Name: stepRaw.PipelineRaw.Name, ctx: ctx, Config: c, } - if s.Pipeline.Config != nil { - s.Pipeline.Config.ctx = ctx + if step.Pipeline.Config != nil { + step.Pipeline.Config.ctx = ctx } // Parse all the steps @@ -154,21 +166,19 @@ func (c *Config) Pipeline(id string, ctx *hcl.EvalContext) (*Pipeline, error) { } // turn stepRaw into a staged Step - s := Step{ - ctx: ctx, - Name: embedStepRaw.Name, - DependsOn: embedStepRaw.DependsOn, - ImageURL: embedStepRaw.ImageURL, - Use: embedStepRaw.Use, - Workspace: embedStepRaw.Workspace, + var embedStep Step + if diag := gohcl.DecodeBody(embedStepRaw.Body, finalizeContext(ctx), &embedStep); diag.HasErrors() { + return nil, diag } - embSteps = append(embSteps, &s) + embedStep.ctx = ctx + embedStep.Name = embedStepRaw.Name + embSteps = append(embSteps, &embedStep) } - s.Pipeline.Steps = embSteps + step.Pipeline.Steps = embSteps } - steps = append(steps, &s) + steps = append(steps, &step) } pipeline.Steps = steps @@ -191,7 +201,12 @@ func (c *Config) PipelineProtos() ([]*pb.Pipeline, error) { // Load HCL config and convert to a Pipeline proto var result []*pb.Pipeline for _, pl := range c.hclConfig.Pipelines { - pipes, err := c.buildPipelineProto(pl) + pipeline, err := c.Pipeline(pl.Name, c.ctx) + if err != nil { + return nil, err + } + + pipes, err := c.buildPipelineProto(pipeline) if err != nil { return nil, err } @@ -206,7 +221,7 @@ func (c *Config) PipelineProtos() ([]*pb.Pipeline, error) { // buildPipelineProto will recursively translate an hclPipeline into a protobuf // Pipeline message. -func (c *Config) buildPipelineProto(pl *hclPipeline) ([]*pb.Pipeline, error) { +func (c *Config) buildPipelineProto(pl *Pipeline) ([]*pb.Pipeline, error) { var result []*pb.Pipeline pipe := &pb.Pipeline{ Name: pl.Name, @@ -218,7 +233,7 @@ func (c *Config) buildPipelineProto(pl *hclPipeline) ([]*pb.Pipeline, error) { } steps := make(map[string]*pb.Pipeline_Step) - for i, step := range pl.StepRaw { + for i, step := range pl.Steps { s := &pb.Pipeline_Step{ Name: step.Name, DependsOn: step.DependsOn, @@ -233,23 +248,23 @@ func (c *Config) buildPipelineProto(pl *hclPipeline) ([]*pb.Pipeline, error) { // If no dependency was explictily set, we rely on the previous step if i != 0 && len(step.DependsOn) == 0 { - s.DependsOn = []string{pl.StepRaw[i-1].Name} + s.DependsOn = []string{pl.Steps[i-1].Name} } // We have an embeded pipeline for this step. This can either be an hclPipeline // defined directly in the step, or a pipeline reference to another pipeline // defined else where. If this is a ref, the raw hcl for the pipeline should // be a "built-in" step of type "pipeline" - if step.PipelineRaw != nil { + if step.Pipeline != nil { // Parse the embedded pipeline assuming it has steps - if len(step.PipelineRaw.StepRaw) > 0 { + if len(step.Pipeline.Steps) > 0 { // This means this is an embedded pipeline, i.e. the HCL definition // is nested within the step PipelineRaw. we parse that pipeline // directly and store it as a separate pipeline, and make _this_ step // a reference to the pipeline // Parse nested pipeline steps - pipelines, err := c.buildPipelineProto(step.PipelineRaw) + pipelines, err := c.buildPipelineProto(step.Pipeline) if err != nil { return nil, err } @@ -257,7 +272,7 @@ func (c *Config) buildPipelineProto(pl *hclPipeline) ([]*pb.Pipeline, error) { result = append(result, pipelines...) // We check if this step references a separate pipeline by Owner - pipeName := step.PipelineRaw.Name + pipeName := step.Pipeline.Name pipeProject := c.hclConfig.Project // Add pipeline reference as a pipeline ref step for parent pipeline diff --git a/internal/config/pipeline_test.go b/internal/config/pipeline_test.go index 3b623aafeb8..edb3d90d730 100644 --- a/internal/config/pipeline_test.go +++ b/internal/config/pipeline_test.go @@ -306,6 +306,7 @@ func TestPipelineProtos(t *testing.T) { require.Len(pipelines[0].Steps, 1) nestedStep := pipelines[0].Steps["test_nested"] + require.NotNil(nestedStep) require.Equal(nestedStep.Name, "test_nested") }, }, diff --git a/internal/config/stages.go b/internal/config/stages.go index 544c75ca369..c2561e779f9 100644 --- a/internal/config/stages.go +++ b/internal/config/stages.go @@ -41,31 +41,6 @@ type scopedStage struct { Remain hcl.Body `hcl:",remain"` } -// Step are the step settings for pipelines -type Step struct { - Labels map[string]string `hcl:"labels,optional"` - Use *Use `hcl:"use,block"` - - // Give this step a name - Name string `hcl:",label"` - - // If set, this step will depend on the defined step. The default step - // will be the previously defined step in order that it was defined - // in a waypoint.hcl - DependsOn []string `hcl:"depends_on,optional"` - - // The OCI image to use for executing this step - ImageURL string `hcl:"image_url,optional"` - - // An optional embedded pipeline stanza - Pipeline *Pipeline `hcl:"pipeline,block"` - - ctx *hcl.EvalContext - - // Optional workspace scoping - Workspace string `hcl:"workspace,optional"` -} - // Build are the build settings. type Build struct { Labels map[string]string `hcl:"labels,optional"` diff --git a/internal/config/validate.go b/internal/config/validate.go index 32198ce5300..8420fa40ad4 100644 --- a/internal/config/validate.go +++ b/internal/config/validate.go @@ -326,20 +326,20 @@ func (c *Config) validatePipeline(b *hcl.Block) []ValidationResult { func (c *Pipeline) Validate() error { var result error - for _, stepRaw := range c.StepRaw { - if stepRaw == nil { + for _, step := range c.Steps { + if step == nil { result = multierror.Append(result, fmt.Errorf( "step stage in pipeline is nil, this is an internal error")) - } else if stepRaw != nil && (stepRaw.Use == nil && stepRaw.PipelineRaw == nil) { + } else if step != nil && (step.Use == nil && step.Pipeline == nil) { result = multierror.Append(result, fmt.Errorf( "step stage with a default 'use' stanza or a 'pipeline' stanza is required")) - } else if stepRaw.Use != nil && stepRaw.PipelineRaw != nil { + } else if step.Use != nil && step.Pipeline != nil { result = multierror.Append(result, fmt.Errorf( "step stage with both a 'use' stanza and pipeline stanza is not valid")) - } else if stepRaw.PipelineRaw == nil && (stepRaw.Use == nil || stepRaw.Use.Type == "") { + } else if step.Pipeline == nil && (step.Use == nil || step.Use.Type == "") { result = multierror.Append(result, fmt.Errorf( "step stage %q is required to define a 'use' stanza and label or a "+ - "pipeline stanza but neither were found", stepRaw.Name)) + "pipeline stanza but neither were found", step.Name)) } // else, other step validations? diff --git a/internal/core/project.go b/internal/core/project.go index cc9b18b8a0a..17be51c5998 100644 --- a/internal/core/project.go +++ b/internal/core/project.go @@ -152,8 +152,9 @@ func NewProject(ctx context.Context, os ...Option) (*Project, error) { // configure pipelines for project and its apps for _, name := range opts.Config.Pipelines() { + // Set input variables for pipelines and steps in context evalCtx := config.EvalContext(nil, p.dir.DataDir()).NewChild() - // TODO: Add variables + config.AddVariables(evalCtx, p.variables) pipelineConfig, err := opts.Config.Pipeline(name, evalCtx) if err != nil { From 771c681a4ec56a8fcbddbfdce3333c37f1dcd9fe Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Fri, 28 Oct 2022 14:34:31 -0700 Subject: [PATCH 2/5] config: Add input variable test for pipelines config parsing --- internal/config/pipeline_test.go | 31 ++++++++++++++++++- .../testdata/pipelines/pipeline_input_var.hcl | 28 +++++++++++++++++ 2 files changed, 58 insertions(+), 1 deletion(-) create mode 100644 internal/config/testdata/pipelines/pipeline_input_var.hcl diff --git a/internal/config/pipeline_test.go b/internal/config/pipeline_test.go index edb3d90d730..0425779d4be 100644 --- a/internal/config/pipeline_test.go +++ b/internal/config/pipeline_test.go @@ -4,6 +4,8 @@ import ( "path/filepath" "testing" + "github.com/hashicorp/go-hclog" + "github.com/hashicorp/waypoint/internal/config/variables" pb "github.com/hashicorp/waypoint/pkg/server/gen" "github.com/stretchr/testify/require" "google.golang.org/grpc/codes" @@ -59,6 +61,29 @@ func TestPipeline(t *testing.T) { }, }, + { + "pipeline_input_var.hcl", + "foo", + func(t *testing.T, c *Pipeline) { + require := require.New(t) + + require.NotNil(t, c) + require.Equal("foo", c.Name) + + steps := c.Steps + s := steps[0] + + var p testStepPluginConfig + diag := s.Configure(&p, nil) + if diag.HasErrors() { + t.Fatal(diag.Error()) + } + + require.NotEmpty(t, p.config.Foo) + require.Equal("example.com/test", s.ImageURL) + }, + }, + { "pipeline_multi_step.hcl", "foo", @@ -222,7 +247,11 @@ func TestPipeline(t *testing.T) { }) require.NoError(err) - pipeline, err := cfg.Pipeline(tt.Pipeline, nil) + evalCtx := EvalContext(nil, "").NewChild() + inputVars, _, _ := variables.EvaluateVariables(hclog.L(), nil, cfg.InputVariables, "") + AddVariables(evalCtx, inputVars) + + pipeline, err := cfg.Pipeline(tt.Pipeline, evalCtx) require.NoError(err) tt.Func(t, pipeline) diff --git a/internal/config/testdata/pipelines/pipeline_input_var.hcl b/internal/config/testdata/pipelines/pipeline_input_var.hcl new file mode 100644 index 00000000000..3031d78b9ff --- /dev/null +++ b/internal/config/testdata/pipelines/pipeline_input_var.hcl @@ -0,0 +1,28 @@ +project = "foo" + +pipeline "foo" { + step "test" { + image_url = var.image_url + + use "test" { + foo = "bar" + } + } +} + +app "web" { + config { + env = { + static = "hello" + } + } + + build {} + + deploy {} +} + +variable "image_url" { + default = "example.com/test" + type = string +} From f8c820a53927c457d8356a54b160c74c1e04737a Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Fri, 28 Oct 2022 14:35:20 -0700 Subject: [PATCH 3/5] cli/init: Update how init upserts pipelines Prior to this commit, the init CLI would upsert the full protobuf structure of a pipeline. This was fine before input variables, but now given how Waypoint internally lazy-evaluates input variables later, this approach no logner works. This commit updates the init cli to instead behave more like how the init CLI handles syncing Applications: The smallest structure possible i.e. a name, owner, and a root step. This data gets re-upserted on configsync and before running a pipeline, so this init simply upserts a basic structure on initialiation. --- internal/cli/init.go | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/internal/cli/init.go b/internal/cli/init.go index d4eb2721fbd..47ca91ea2a7 100644 --- a/internal/cli/init.go +++ b/internal/cli/init.go @@ -404,17 +404,29 @@ func (c *InitCommand) validateProject() bool { } sp := sg.Add("Registering all pipelines for project %q", ref.Project) - protoPipes, err := c.cfg.PipelineProtos() - if err != nil { - c.stepError(s, initStepPipeline, err) - return false - } + pipeNames := c.cfg.Pipelines() + + // We do a shallow sync of pipelines on the init phase in the same way we + // shallow sync Applications. Prior to running a pipeline - we do a full + // HCL eval and protobuf sync that will upsert over this data. + for _, pn := range pipeNames { + sp.Update("Registering Pipeline %q with the server...", pn) - for _, pipeline := range protoPipes { - sp.Update("Registering Pipeline %q with the server...", pipeline.Name) + baseStep := map[string]*pb.Pipeline_Step{"root": &pb.Pipeline_Step{ + Name: "root", + Kind: &pb.Pipeline_Step_Pipeline_{}, + }} _, err := client.UpsertPipeline(c.Ctx, &pb.UpsertPipelineRequest{ - Pipeline: pipeline, + Pipeline: &pb.Pipeline{ + Name: pn, + Owner: &pb.Pipeline_Project{ + Project: &pb.Ref_Project{ + Project: ref.Project, + }, + }, + Steps: baseStep, + }, }) if err != nil { c.stepError(sp, initStepPipeline, err) From 324d3434188c70bf31f6eb7522108236660b71c4 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Fri, 28 Oct 2022 14:45:20 -0700 Subject: [PATCH 4/5] linter: gofmt --- internal/cli/init.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/cli/init.go b/internal/cli/init.go index 47ca91ea2a7..4638b052e97 100644 --- a/internal/cli/init.go +++ b/internal/cli/init.go @@ -412,7 +412,7 @@ func (c *InitCommand) validateProject() bool { for _, pn := range pipeNames { sp.Update("Registering Pipeline %q with the server...", pn) - baseStep := map[string]*pb.Pipeline_Step{"root": &pb.Pipeline_Step{ + baseStep := map[string]*pb.Pipeline_Step{"root": { Name: "root", Kind: &pb.Pipeline_Step_Pipeline_{}, }} From 0d9884cda8f74b21acc9f0805badbf248a574252 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Fri, 28 Oct 2022 14:55:19 -0700 Subject: [PATCH 5/5] Add changelog --- .changelog/4132.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/4132.txt diff --git a/.changelog/4132.txt b/.changelog/4132.txt new file mode 100644 index 00000000000..e5acae29a88 --- /dev/null +++ b/.changelog/4132.txt @@ -0,0 +1,3 @@ +```release-note:improvement +pipelines: Add ability to evaluate input variables in pipelines stanzas. +```