From a686d1eabae2294755f27f780b9a2054380ac406 Mon Sep 17 00:00:00 2001 From: Gerd Oberlechner Date: Wed, 18 Dec 2024 11:48:45 +0100 Subject: [PATCH 1/6] introduce step action based structs Signed-off-by: Gerd Oberlechner --- dev-infrastructure/.gitignore | 2 + .../templatize/cmd/pipeline/run/options.go | 2 +- tooling/templatize/internal/end2end/e2e.go | 2 +- .../templatize/internal/end2end/e2e_test.go | 147 +++++---------- tooling/templatize/pkg/ev2/pipeline.go | 22 +-- tooling/templatize/pkg/pipeline/arm.go | 6 +- .../templatize/pkg/pipeline/common_test.go | 5 +- tooling/templatize/pkg/pipeline/inspect.go | 14 +- .../templatize/pkg/pipeline/inspect_test.go | 50 ++---- tooling/templatize/pkg/pipeline/run.go | 49 ++--- tooling/templatize/pkg/pipeline/run_test.go | 97 +++------- tooling/templatize/pkg/pipeline/shell.go | 6 +- tooling/templatize/pkg/pipeline/shell_test.go | 30 ++-- tooling/templatize/pkg/pipeline/types.go | 167 ++++++++++++++++-- tooling/templatize/pkg/pipeline/validation.go | 12 +- .../pkg/pipeline/validation_test.go | 38 ++-- 16 files changed, 336 insertions(+), 313 deletions(-) diff --git a/dev-infrastructure/.gitignore b/dev-infrastructure/.gitignore index 31fd022f0..a7199467a 100644 --- a/dev-infrastructure/.gitignore +++ b/dev-infrastructure/.gitignore @@ -15,3 +15,5 @@ configurations/global-acr.bicepparam configurations/global-roles.bicepparam configurations/global-infra.bicepparam config.mk + +istio-* diff --git a/tooling/templatize/cmd/pipeline/run/options.go b/tooling/templatize/cmd/pipeline/run/options.go index 5a866f37c..b9669c59d 100644 --- a/tooling/templatize/cmd/pipeline/run/options.go +++ b/tooling/templatize/cmd/pipeline/run/options.go @@ -100,7 +100,7 @@ func (o *RunOptions) RunPipeline(ctx context.Context) error { if err != nil { return err } - return o.PipelineOptions.Pipeline.Run(ctx, &pipeline.PipelineRunOptions{ + return pipeline.RunPipeline(o.PipelineOptions.Pipeline, ctx, &pipeline.PipelineRunOptions{ DryRun: o.DryRun, Vars: variables, Region: rolloutOptions.Region, diff --git a/tooling/templatize/internal/end2end/e2e.go b/tooling/templatize/internal/end2end/e2e.go index ea2ad9bca..11c664560 100644 --- a/tooling/templatize/internal/end2end/e2e.go +++ b/tooling/templatize/internal/end2end/e2e.go @@ -146,7 +146,7 @@ func (e *e2eImpl) SetAKSName(aksName string) { } func (e *e2eImpl) AddStep(step pipeline.Step, rg int) { - e.pipeline.ResourceGroups[rg].Steps = append(e.pipeline.ResourceGroups[rg].Steps, &step) + e.pipeline.ResourceGroups[rg].Steps = append(e.pipeline.ResourceGroups[rg].Steps, step) } func (e *e2eImpl) SetConfig(updates config.Variables) { diff --git a/tooling/templatize/internal/end2end/e2e_test.go b/tooling/templatize/internal/end2end/e2e_test.go index ffe9a46bf..36346c2da 100644 --- a/tooling/templatize/internal/end2end/e2e_test.go +++ b/tooling/templatize/internal/end2end/e2e_test.go @@ -35,17 +35,13 @@ func TestE2EMake(t *testing.T) { tmpDir := t.TempDir() e2eImpl := newE2E(tmpDir) - e2eImpl.AddStep(pipeline.Step{ - Name: "test", - Action: "Shell", - Command: "make test", - Variables: []pipeline.Variable{ - { - Name: "TEST_ENV", - ConfigRef: "test_env", - }, - }, - }, 0) + e2eImpl.AddStep( + pipeline.NewShellStep("test", "make test").WithVariables(pipeline.Variable{ + Name: "TEST_ENV", + ConfigRef: "test_env", + }), + 0, + ) e2eImpl.SetConfig(config.Variables{"defaults": config.Variables{"test_env": "test_env"}}) @@ -68,11 +64,7 @@ func TestE2EKubernetes(t *testing.T) { tmpDir := t.TempDir() e2eImpl := newE2E(tmpDir) - e2eImpl.AddStep(pipeline.Step{ - Name: "test", - Action: "Shell", - Command: "kubectl get namespaces", - }, 0) + e2eImpl.AddStep(pipeline.NewShellStep("test", "kubectl get namespaces"), 0) e2eImpl.SetAKSName("aro-hcp-aks") e2eImpl.SetConfig(config.Variables{"defaults": config.Variables{"rg": "hcp-underlay-dev-svc"}}) @@ -88,13 +80,7 @@ func TestE2EArmDeploy(t *testing.T) { tmpDir := t.TempDir() e2eImpl := newE2E(tmpDir) - e2eImpl.AddStep(pipeline.Step{ - Name: "test", - Action: "ARM", - Template: "test.bicep", - Parameters: "test.bicepparm", - }, 0) - + e2eImpl.AddStep(pipeline.NewARMStep("test", "test.bicep", "test.bicepparm"), 0) cleanup := e2eImpl.UseRandomRG() defer func() { err := cleanup() @@ -140,11 +126,10 @@ func TestE2EShell(t *testing.T) { e2eImpl := newE2E(tmpDir) - e2eImpl.AddStep(pipeline.Step{ - Name: "readInput", - Action: "Shell", - Command: "/bin/echo ${PWD} > env.txt", - }, 0) + e2eImpl.AddStep( + pipeline.NewShellStep("readInput", "/bin/echo ${PWD} > env.txt"), + 0, + ) persistAndRun(t, &e2eImpl) @@ -161,27 +146,20 @@ func TestE2EArmDeployWithOutput(t *testing.T) { tmpDir := t.TempDir() e2eImpl := newE2E(tmpDir) - e2eImpl.AddStep(pipeline.Step{ - Name: "createZone", - Action: "ARM", - Template: "test.bicep", - Parameters: "test.bicepparm", - }, 0) - - e2eImpl.AddStep(pipeline.Step{ - Name: "readInput", - Action: "Shell", - Command: "echo ${zoneName} > env.txt", - Variables: []pipeline.Variable{ - { + + e2eImpl.AddStep(pipeline.NewARMStep("createZone", "test.bicep", "test.bicepparm"), 0) + + e2eImpl.AddStep(pipeline.NewShellStep( + "readInput", "echo ${zoneName} > env.txt", + ).WithVariables( + pipeline.Variable{ + Name: "zoneName", + Input: &pipeline.Input{ Name: "zoneName", - Input: &pipeline.Input{ - Name: "zoneName", - Step: "createZone", - }, + Step: "createZone", }, }, - }, 0) + ), 0) cleanup := e2eImpl.UseRandomRG() defer func() { @@ -212,43 +190,26 @@ func TestE2EArmDeployWithOutputToArm(t *testing.T) { tmpDir := t.TempDir() e2eImpl := newE2E(tmpDir) - e2eImpl.AddStep(pipeline.Step{ - Name: "parameterA", - Action: "ARM", - Template: "testa.bicep", - Parameters: "testa.bicepparm", - }, 0) - - e2eImpl.AddStep(pipeline.Step{ - Name: "parameterB", - Action: "ARM", - Template: "testb.bicep", - Parameters: "testb.bicepparm", - Variables: []pipeline.Variable{ - { - Name: "parameterB", - Input: &pipeline.Input{ - Name: "parameterA", - Step: "parameterA", - }, - }, + e2eImpl.AddStep(pipeline.NewARMStep("parameterA", "testa.bicep", "testa.bicepparm"), 0) + e2eImpl.AddStep(pipeline.NewARMStep("parameterB", "testb.bicep", "testb.bicepparm").WithVariables(pipeline.Variable{ + Name: "parameterB", + Input: &pipeline.Input{ + Name: "parameterA", + Step: "parameterA", }, - }, 0) - - e2eImpl.AddStep(pipeline.Step{ - Name: "readInput", - Action: "Shell", - Command: "echo ${end} > env.txt", - Variables: []pipeline.Variable{ - { - Name: "end", - Input: &pipeline.Input{ - Name: "parameterC", - Step: "parameterB", - }, + }), 0) + + e2eImpl.AddStep(pipeline.NewShellStep( + "readInput", "echo ${end} > env.txt", + ).WithVariables( + pipeline.Variable{ + Name: "end", + Input: &pipeline.Input{ + Name: "parameterC", + Step: "parameterB", }, }, - }, 0) + ), 0) e2eImpl.AddBicepTemplate(` param parameterA string @@ -290,29 +251,19 @@ func TestE2EArmDeployWithOutputRGOverlap(t *testing.T) { tmpDir := t.TempDir() e2eImpl := newE2E(tmpDir) - e2eImpl.AddStep(pipeline.Step{ - Name: "parameterA", - Action: "ARM", - Template: "testa.bicep", - Parameters: "testa.bicepparm", - }, 0) + e2eImpl.AddStep(pipeline.NewARMStep("parameterA", "testa.bicep", "testa.bicepparm"), 0) e2eImpl.AddResourceGroup() - e2eImpl.AddStep(pipeline.Step{ - Name: "readInput", - Action: "Shell", - Command: "echo ${end} > env.txt", - Variables: []pipeline.Variable{ - { - Name: "end", - Input: &pipeline.Input{ - Name: "parameterA", - Step: "parameterA", - }, + e2eImpl.AddStep(pipeline.NewShellStep("readInput", "echo ${end} > env.txt").WithVariables( + pipeline.Variable{ + Name: "end", + Input: &pipeline.Input{ + Name: "parameterA", + Step: "parameterA", }, }, - }, 1) + ), 1) e2eImpl.AddBicepTemplate(` param parameterA string diff --git a/tooling/templatize/pkg/ev2/pipeline.go b/tooling/templatize/pkg/ev2/pipeline.go index a51b5c28e..cb8681c58 100644 --- a/tooling/templatize/pkg/ev2/pipeline.go +++ b/tooling/templatize/pkg/ev2/pipeline.go @@ -68,16 +68,17 @@ func readReferencedPipelineFiles(p *pipeline.Pipeline) (map[string][]byte, error referencedFiles := make(map[string][]byte) for _, rg := range p.ResourceGroups { for _, step := range rg.Steps { - if step.Parameters != "" { - absFilePath, err := p.AbsoluteFilePath(step.Parameters) + switch concreteStep := step.(type) { + case *pipeline.ARMStep: + absFilePath, err := p.AbsoluteFilePath(concreteStep.Parameters) if err != nil { - return nil, fmt.Errorf("failed to get absolute file path for %q: %w", step.Parameters, err) + return nil, fmt.Errorf("failed to get absolute file path for %q: %w", concreteStep.Parameters, err) } paramFileContent, err := os.ReadFile(absFilePath) if err != nil { - return nil, fmt.Errorf("failed to read parameter file %q: %w", step.Parameters, err) + return nil, fmt.Errorf("failed to read parameter file %q: %w", concreteStep.Parameters, err) } - referencedFiles[step.Parameters] = paramFileContent + referencedFiles[concreteStep.Parameters] = paramFileContent } } } @@ -94,18 +95,19 @@ func processPipelineForEV2(p *pipeline.Pipeline, referencedFiles map[string][]by for _, rg := range processingPipeline.ResourceGroups { for _, step := range rg.Steps { // preprocess the parameters file with scopebinding variables - if step.Parameters != "" { - paramFileContent, ok := referencedFiles[step.Parameters] + switch concreteStep := step.(type) { + case *pipeline.ARMStep: + paramFileContent, ok := referencedFiles[concreteStep.Parameters] if !ok { - return nil, nil, fmt.Errorf("parameter file %q not found", step.Parameters) + return nil, nil, fmt.Errorf("parameter file %q not found", concreteStep.Parameters) } preprocessedBytes, err := config.PreprocessContent(paramFileContent, scopeBoundBicepParamVars) if err != nil { return nil, nil, err } - newParameterFilePath := buildPrefixedFilePath(step.Parameters, precompiledPrefix) + newParameterFilePath := buildPrefixedFilePath(concreteStep.Parameters, precompiledPrefix) processedFiles[newParameterFilePath] = preprocessedBytes - step.Parameters = newParameterFilePath + concreteStep.Parameters = newParameterFilePath } } } diff --git a/tooling/templatize/pkg/pipeline/arm.go b/tooling/templatize/pkg/pipeline/arm.go index 469854921..9e0ab0210 100644 --- a/tooling/templatize/pkg/pipeline/arm.go +++ b/tooling/templatize/pkg/pipeline/arm.go @@ -31,7 +31,7 @@ func newArmClient(subscriptionID, region string) *armClient { } } -func (a *armClient) runArmStep(ctx context.Context, options *PipelineRunOptions, rgName string, step *Step, input map[string]output) (output, error) { +func (a *armClient) runArmStep(ctx context.Context, options *PipelineRunOptions, rgName string, step *ARMStep, input map[string]output) (output, error) { // Ensure resourcegroup exists err := a.ensureResourceGroupExists(ctx, rgName, options.NoPersist) if err != nil { @@ -69,7 +69,7 @@ func printChanges(t armresources.ChangeType, changes []*armresources.WhatIfChang } } } -func doDryRun(ctx context.Context, client *armresources.DeploymentsClient, rgName string, step *Step, vars config.Variables, input map[string]output) (output, error) { +func doDryRun(ctx context.Context, client *armresources.DeploymentsClient, rgName string, step *ARMStep, vars config.Variables, input map[string]output) (output, error) { logger := logr.FromContextOrDiscard(ctx) @@ -126,7 +126,7 @@ func doDryRun(ctx context.Context, client *armresources.DeploymentsClient, rgNam return nil, nil } -func doWaitForDeployment(ctx context.Context, client *armresources.DeploymentsClient, rgName string, step *Step, vars config.Variables, input map[string]output) (output, error) { +func doWaitForDeployment(ctx context.Context, client *armresources.DeploymentsClient, rgName string, step *ARMStep, vars config.Variables, input map[string]output) (output, error) { logger := logr.FromContextOrDiscard(ctx) inputValues, err := getInputValues(step.Variables, input) diff --git a/tooling/templatize/pkg/pipeline/common_test.go b/tooling/templatize/pkg/pipeline/common_test.go index 7b5d15b65..cb8493797 100644 --- a/tooling/templatize/pkg/pipeline/common_test.go +++ b/tooling/templatize/pkg/pipeline/common_test.go @@ -4,9 +4,10 @@ import ( "path/filepath" "testing" + "gotest.tools/v3/assert" + "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" - "gotest.tools/v3/assert" "github.com/Azure/ARO-HCP/tooling/templatize/pkg/config" ) @@ -31,7 +32,7 @@ func TestDeepCopy(t *testing.T) { assert.Assert(t, pipeline != pipelineCopy, "expected pipeline and copy to be different") assert.Equal(t, pipelineCopy.PipelineFilePath(), newPipelinePath, "expected pipeline copy to have new path") - if diff := cmp.Diff(pipeline, pipelineCopy, cmpopts.IgnoreUnexported(Pipeline{}, Step{})); diff != "" { + if diff := cmp.Diff(pipeline, pipelineCopy, cmpopts.IgnoreUnexported(Pipeline{}, ShellStep{}, ARMStep{})); diff != "" { t.Errorf("got diffs after pipeline deep copy: %v", diff) } } diff --git a/tooling/templatize/pkg/pipeline/inspect.go b/tooling/templatize/pkg/pipeline/inspect.go index 7576b8277..af80f0107 100644 --- a/tooling/templatize/pkg/pipeline/inspect.go +++ b/tooling/templatize/pkg/pipeline/inspect.go @@ -8,7 +8,7 @@ import ( "github.com/Azure/ARO-HCP/tooling/templatize/pkg/config" ) -type StepInspectScope func(*Step, *InspectOptions, io.Writer) error +type StepInspectScope func(Step, *InspectOptions, io.Writer) error func NewStepInspectScopes() map[string]StepInspectScope { return map[string]StepInspectScope{ @@ -41,7 +41,7 @@ func NewInspectOptions(vars config.Variables, region, step, scope, format string func (p *Pipeline) Inspect(ctx context.Context, options *InspectOptions, writer io.Writer) error { for _, rg := range p.ResourceGroups { for _, step := range rg.Steps { - if step.Name == options.Step { + if step.StepName() == options.Step { if inspectFunc, ok := options.ScopeFunctions[options.Scope]; ok { err := inspectFunc(step, options, writer) if err != nil { @@ -57,14 +57,14 @@ func (p *Pipeline) Inspect(ctx context.Context, options *InspectOptions, writer return fmt.Errorf("step %q not found", options.Step) } -func inspectVars(s *Step, options *InspectOptions, writer io.Writer) error { +func inspectVars(s Step, options *InspectOptions, writer io.Writer) error { var envVars map[string]string var err error - switch s.Action { - case "Shell": - envVars, err = s.mapStepVariables(options.Vars) + switch step := s.(type) { + case *ShellStep: + envVars, err = step.mapStepVariables(options.Vars) default: - return fmt.Errorf("inspecting step variables not implemented for action type %s", s.Action) + return fmt.Errorf("inspecting step variables not implemented for action type %s", s.ActionType()) } if err != nil { return err diff --git a/tooling/templatize/pkg/pipeline/inspect_test.go b/tooling/templatize/pkg/pipeline/inspect_test.go index 744ec4707..602bdf2e0 100644 --- a/tooling/templatize/pkg/pipeline/inspect_test.go +++ b/tooling/templatize/pkg/pipeline/inspect_test.go @@ -14,22 +14,17 @@ import ( func TestInspectVars(t *testing.T) { testCases := []struct { name string - caseStep *Step + caseStep Step options *InspectOptions expected string err string }{ { name: "basic", - caseStep: &Step{ - Action: "Shell", - Variables: []Variable{ - { - Name: "FOO", - ConfigRef: "foo", - }, - }, - }, + caseStep: NewShellStep("step", "echo hello").WithVariables(Variable{ + Name: "FOO", + ConfigRef: "foo", + }), options: &InspectOptions{ Vars: config.Variables{ "foo": "bar", @@ -40,15 +35,10 @@ func TestInspectVars(t *testing.T) { }, { name: "makefile", - caseStep: &Step{ - Action: "Shell", - Variables: []Variable{ - { - Name: "FOO", - ConfigRef: "foo", - }, - }, - }, + caseStep: NewShellStep("step", "echo hello").WithVariables(Variable{ + Name: "FOO", + ConfigRef: "foo", + }), options: &InspectOptions{ Vars: config.Variables{ "foo": "bar", @@ -59,12 +49,12 @@ func TestInspectVars(t *testing.T) { }, { name: "failed action", - caseStep: &Step{Action: "Unknown"}, - err: "inspecting step variables not implemented for action type Unknown", + caseStep: NewARMStep("step", "test.bicep", "test.bicepparam"), + err: "inspecting step variables not implemented for action type ARM", }, { name: "failed format", - caseStep: &Step{Action: "Shell"}, + caseStep: NewShellStep("step", "echo hello"), options: &InspectOptions{Format: "unknown"}, err: "unknown output format \"unknown\"", }, @@ -87,10 +77,8 @@ func TestInspectVars(t *testing.T) { func TestInspect(t *testing.T) { p := Pipeline{ ResourceGroups: []*ResourceGroup{{ - Steps: []*Step{ - { - Name: "step1", - }, + Steps: []Step{ + NewShellStep("step1", "echo hello"), }, }, }, @@ -98,8 +86,8 @@ func TestInspect(t *testing.T) { opts := NewInspectOptions(config.Variables{}, "", "step1", "scope", "format") opts.ScopeFunctions = map[string]StepInspectScope{ - "scope": func(s *Step, o *InspectOptions, w io.Writer) error { - assert.Equal(t, s.Name, "step1") + "scope": func(s Step, o *InspectOptions, w io.Writer) error { + assert.Equal(t, s.StepName(), "step1") return nil }, } @@ -111,10 +99,8 @@ func TestInspect(t *testing.T) { func TestInspectWrongScope(t *testing.T) { p := Pipeline{ ResourceGroups: []*ResourceGroup{{ - Steps: []*Step{ - { - Name: "step1", - }, + Steps: []Step{ + NewShellStep("step1", "echo hello"), }, }, }, diff --git a/tooling/templatize/pkg/pipeline/run.go b/tooling/templatize/pkg/pipeline/run.go index 4cb6a239f..989bdb6ab 100644 --- a/tooling/templatize/pkg/pipeline/run.go +++ b/tooling/templatize/pkg/pipeline/run.go @@ -5,7 +5,6 @@ import ( "fmt" "os" "path/filepath" - "strings" "gopkg.in/yaml.v3" @@ -77,7 +76,7 @@ func (o armOutput) GetValue(key string) (*outPutValue, error) { return nil, fmt.Errorf("key %q not found", key) } -func (p *Pipeline) Run(ctx context.Context, options *PipelineRunOptions) error { +func RunPipeline(pipeline *Pipeline, ctx context.Context, options *PipelineRunOptions) error { logger := logr.FromContextOrDiscard(ctx) outPuts := make(map[string]output) @@ -89,7 +88,7 @@ func (p *Pipeline) Run(ctx context.Context, options *PipelineRunOptions) error { if err != nil { return err } - dir := filepath.Dir(p.pipelineFilePath) + dir := filepath.Dir(pipeline.pipelineFilePath) logger.V(7).Info("switch current dir to pipeline file directory", "path", dir) err = os.Chdir(dir) if err != nil { @@ -103,7 +102,7 @@ func (p *Pipeline) Run(ctx context.Context, options *PipelineRunOptions) error { } }() - for _, rg := range p.ResourceGroups { + for _, rg := range pipeline.ResourceGroups { // prepare execution context subscriptionID, err := options.SubsciptionLookupFunc(ctx, rg.Subscription) if err != nil { @@ -116,7 +115,7 @@ func (p *Pipeline) Run(ctx context.Context, options *PipelineRunOptions) error { resourceGroup: rg.Name, aksClusterName: rg.AKSCluster, } - err = rg.run(ctx, options, &executionTarget, outPuts) + err = RunResourceGroup(rg, ctx, options, &executionTarget, outPuts) if err != nil { return err } @@ -124,7 +123,7 @@ func (p *Pipeline) Run(ctx context.Context, options *PipelineRunOptions) error { return nil } -func (rg *ResourceGroup) run(ctx context.Context, options *PipelineRunOptions, executionTarget ExecutionTarget, outputs map[string]output) error { +func RunResourceGroup(rg *ResourceGroup, ctx context.Context, options *PipelineRunOptions, executionTarget ExecutionTarget, outputs map[string]output) error { logger := logr.FromContextOrDiscard(ctx) kubeconfigFile, err := executionTarget.KubeConfig(ctx) @@ -140,11 +139,12 @@ func (rg *ResourceGroup) run(ctx context.Context, options *PipelineRunOptions, e for _, step := range rg.Steps { // execute - output, err := step.run( + output, err := RunStep( + step, logr.NewContext( ctx, logger.WithValues( - "step", step.Name, + "step", step.StepName(), "subscription", executionTarget.GetSubscriptionID(), "resourceGroup", executionTarget.GetResourceGroup(), "aksCluster", executionTarget.GetAkSClusterName(), @@ -158,14 +158,15 @@ func (rg *ResourceGroup) run(ctx context.Context, options *PipelineRunOptions, e return err } if output != nil { - outputs[step.Name] = output + + outputs[step.StepName()] = output } } return nil } -func (s *Step) run(ctx context.Context, kubeconfigFile string, executionTarget ExecutionTarget, options *PipelineRunOptions, outPuts map[string]output) (output, error) { - if options.Step != "" && s.Name != options.Step { +func RunStep(s Step, ctx context.Context, kubeconfigFile string, executionTarget ExecutionTarget, options *PipelineRunOptions, outPuts map[string]output) (output, error) { + if options.Step != "" && s.StepName() != options.Step { // skip steps that don't match the specified step name return nil, nil } @@ -173,37 +174,25 @@ func (s *Step) run(ctx context.Context, kubeconfigFile string, executionTarget E if options.DryRun { fmt.Println("This is a dry run!") } - fmt.Println(s.description()) + fmt.Println(s.Description()) fmt.Print("\n") - switch s.Action { - case "Shell": - return nil, s.runShellStep(ctx, kubeconfigFile, options, outPuts) - case "ARM": + switch step := s.(type) { + case *ShellStep: + return nil, runShellStep(step, ctx, kubeconfigFile, options, outPuts) + case *ARMStep: a := newArmClient(executionTarget.GetSubscriptionID(), executionTarget.GetRegion()) if a == nil { return nil, fmt.Errorf("failed to create ARM client") } - output, err := a.runArmStep(ctx, options, executionTarget.GetResourceGroup(), s, outPuts) + output, err := a.runArmStep(ctx, options, executionTarget.GetResourceGroup(), step, outPuts) if err != nil { return nil, fmt.Errorf("failed to run ARM step: %w", err) } return output, nil default: - return nil, fmt.Errorf("unsupported action type %q", s.Action) - } -} - -func (s *Step) description() string { - var details []string - switch s.Action { - case "Shell": - details = append(details, fmt.Sprintf("Command: %s", s.Command)) - case "ARM": - details = append(details, fmt.Sprintf("Template: %s", s.Template)) - details = append(details, fmt.Sprintf("Parameters: %s", s.Parameters)) + return nil, fmt.Errorf("unsupported action type %q", s.ActionType()) } - return fmt.Sprintf("Step %s\n Kind: %s\n %s", s.Name, s.Action, strings.Join(details, "\n ")) } func getInputValues(configuredVariables []Variable, inputs map[string]output) (map[string]any, error) { diff --git a/tooling/templatize/pkg/pipeline/run_test.go b/tooling/templatize/pkg/pipeline/run_test.go index 9874cb71e..738974fbe 100644 --- a/tooling/templatize/pkg/pipeline/run_test.go +++ b/tooling/templatize/pkg/pipeline/run_test.go @@ -8,83 +8,46 @@ import ( ) func TestStepRun(t *testing.T) { - fooundOutput := "" - s := &Step{ - Name: "test", - Action: "Shell", - Command: "echo hello", - outputFunc: func(output string) { - fooundOutput = output + foundOutput := "" + s := NewShellStep("step", "echo hello").WithOutputFunc( + func(output string) { + foundOutput = output }, - } - _, err := s.run(context.Background(), "", &executionTargetImpl{}, &PipelineRunOptions{}, nil) - assert.NilError(t, err) - assert.Equal(t, fooundOutput, "hello\n") -} - -func TestStepRunSkip(t *testing.T) { - s := &Step{ - Name: "step", - } - // this should skip - _, err := s.run(context.Background(), "", &executionTargetImpl{}, &PipelineRunOptions{Step: "skip"}, nil) + ) + _, err := RunStep(s, context.Background(), "", &executionTargetImpl{}, &PipelineRunOptions{}, nil) assert.NilError(t, err) - - // this should fail - _, err = s.run(context.Background(), "", &executionTargetImpl{}, &PipelineRunOptions{Step: "step"}, nil) - assert.Error(t, err, "unsupported action type \"\"") + assert.Equal(t, foundOutput, "hello\n") } func TestResourceGroupRun(t *testing.T) { foundOutput := "" rg := &ResourceGroup{ - Steps: []*Step{ - { - Name: "step", - Action: "Shell", - Command: "echo hello", - outputFunc: func(output string) { + Steps: []Step{ + NewShellStep("step", "echo hello").WithOutputFunc( + func(output string) { foundOutput = output }, - }, + ), }, } - err := rg.run(context.Background(), &PipelineRunOptions{}, &executionTargetImpl{}, make(map[string]output)) + err := RunResourceGroup(rg, context.Background(), &PipelineRunOptions{}, &executionTargetImpl{}, make(map[string]output)) assert.NilError(t, err) assert.Equal(t, foundOutput, "hello\n") } func TestResourceGroupError(t *testing.T) { tmpVals := make([]string, 0) + outputFunc := func(output string) { + tmpVals = append(tmpVals, output) + } rg := &ResourceGroup{ - Steps: []*Step{ - { - Name: "step", - Action: "Shell", - Command: "echo hello", - outputFunc: func(output string) { - tmpVals = append(tmpVals, output) - }, - }, - { - Name: "step", - Action: "Shell", - Command: "faaaaafffaa", - outputFunc: func(output string) { - tmpVals = append(tmpVals, output) - }, - }, - { - Name: "step", - Action: "Shell", - Command: "echo hallo", - outputFunc: func(output string) { - tmpVals = append(tmpVals, output) - }, - }, + Steps: []Step{ + NewShellStep("step1", "echo hello").WithOutputFunc(outputFunc), + NewShellStep("step2", "faaaaafffaa").WithOutputFunc(outputFunc), + NewShellStep("step3", "echo hallo").WithOutputFunc(outputFunc), }, } - err := rg.run(context.Background(), &PipelineRunOptions{}, &executionTargetImpl{}, make(map[string]output)) + err := RunResourceGroup(rg, context.Background(), &PipelineRunOptions{}, &executionTargetImpl{}, make(map[string]output)) assert.ErrorContains(t, err, "faaaaafffaa: command not found\n exit status 127") // Test processing ends after first error assert.Equal(t, len(tmpVals), 1) @@ -101,9 +64,8 @@ func (t *testExecutionTarget) GetResourceGroup() string { return "test" } func (t *testExecutionTarget) GetRegion() string { return "test" } func TestResourceGroupRunRequireKubeconfig(t *testing.T) { - - rg := &ResourceGroup{Steps: []*Step{}} - err := rg.run(context.Background(), &PipelineRunOptions{}, &testExecutionTarget{}, make(map[string]output)) + rg := &ResourceGroup{Steps: []Step{}} + err := RunResourceGroup(rg, context.Background(), &PipelineRunOptions{}, &testExecutionTarget{}, make(map[string]output)) assert.NilError(t, err) } @@ -114,21 +76,18 @@ func TestPipelineRun(t *testing.T) { { Name: "test", Subscription: "test", - Steps: []*Step{ - { - Name: "step", - Action: "Shell", - Command: "echo hello", - outputFunc: func(output string) { + Steps: []Step{ + NewShellStep("step", "echo hello").WithOutputFunc( + func(output string) { foundOutput = output }, - }, + ), }, }, }, } - err := pipeline.Run(context.Background(), &PipelineRunOptions{ + err := RunPipeline(pipeline, context.Background(), &PipelineRunOptions{ SubsciptionLookupFunc: func(_ context.Context, _ string) (string, error) { return "test", nil }, @@ -160,7 +119,7 @@ func TestAddInputVars(t *testing.T) { "value": "value1", }, } - s := &Step{ + s := &ShellStep{ Variables: []Variable{{ Name: "input1", Input: &Input{ diff --git a/tooling/templatize/pkg/pipeline/shell.go b/tooling/templatize/pkg/pipeline/shell.go index 0427c8dc0..4761a2ef0 100644 --- a/tooling/templatize/pkg/pipeline/shell.go +++ b/tooling/templatize/pkg/pipeline/shell.go @@ -12,7 +12,7 @@ import ( "github.com/Azure/ARO-HCP/tooling/templatize/pkg/utils" ) -func (s *Step) createCommand(ctx context.Context, dryRun bool, envVars map[string]string) (*exec.Cmd, bool) { +func (s *ShellStep) createCommand(ctx context.Context, dryRun bool, envVars map[string]string) (*exec.Cmd, bool) { var scriptCommand string = s.Command if dryRun { if s.DryRun.Command == "" && s.DryRun.Variables == nil { @@ -34,7 +34,7 @@ func buildBashScript(command string) string { return fmt.Sprintf("set -o errexit -o nounset -o pipefail\n%s", command) } -func (s *Step) runShellStep(ctx context.Context, kubeconfigFile string, options *PipelineRunOptions, inputs map[string]output) error { +func runShellStep(s *ShellStep, ctx context.Context, kubeconfigFile string, options *PipelineRunOptions, inputs map[string]output) error { if s.outputFunc == nil { s.outputFunc = func(output string) { fmt.Println(output) @@ -82,7 +82,7 @@ func (s *Step) runShellStep(ctx context.Context, kubeconfigFile string, options return nil } -func (s *Step) mapStepVariables(vars config.Variables) (map[string]string, error) { +func (s *ShellStep) mapStepVariables(vars config.Variables) (map[string]string, error) { envVars := make(map[string]string) for _, e := range s.Variables { if e.ConfigRef != "" { // not all Variables are Environment variables diff --git a/tooling/templatize/pkg/pipeline/shell_test.go b/tooling/templatize/pkg/pipeline/shell_test.go index bee5ff4de..831ba8d39 100644 --- a/tooling/templatize/pkg/pipeline/shell_test.go +++ b/tooling/templatize/pkg/pipeline/shell_test.go @@ -15,7 +15,7 @@ func TestCreateCommand(t *testing.T) { ctx := context.Background() testCases := []struct { name string - step *Step + step *ShellStep dryRun bool envVars map[string]string expectedScript string @@ -24,14 +24,14 @@ func TestCreateCommand(t *testing.T) { }{ { name: "basic", - step: &Step{ + step: &ShellStep{ Command: "/bin/echo hello", }, expectedScript: buildBashScript("/bin/echo hello"), }, { name: "dry-run", - step: &Step{ + step: &ShellStep{ Command: "/bin/echo hello", DryRun: DryRun{ Command: "/bin/echo dry-run", @@ -42,7 +42,7 @@ func TestCreateCommand(t *testing.T) { }, { name: "dry-run-env", - step: &Step{ + step: &ShellStep{ Command: "/bin/echo", DryRun: DryRun{ Variables: []Variable{ @@ -60,7 +60,7 @@ func TestCreateCommand(t *testing.T) { }, { name: "dry-run fail", - step: &Step{ + step: &ShellStep{ Command: "/bin/echo", }, dryRun: true, @@ -86,7 +86,7 @@ func TestMapStepVariables(t *testing.T) { testCases := []struct { name string vars config.Variables - step Step + step *ShellStep expected map[string]string err string }{ @@ -95,7 +95,7 @@ func TestMapStepVariables(t *testing.T) { vars: config.Variables{ "FOO": "bar", }, - step: Step{ + step: &ShellStep{ Variables: []Variable{ { Name: "BAZ", @@ -110,7 +110,7 @@ func TestMapStepVariables(t *testing.T) { { name: "missing", vars: config.Variables{}, - step: Step{ + step: &ShellStep{ Variables: []Variable{ { ConfigRef: "FOO", @@ -124,7 +124,7 @@ func TestMapStepVariables(t *testing.T) { vars: config.Variables{ "FOO": 42, }, - step: Step{ + step: &ShellStep{ Variables: []Variable{ { Name: "BAZ", @@ -155,20 +155,20 @@ func TestRunShellStep(t *testing.T) { testCases := []struct { name string vars config.Variables - step *Step + step *ShellStep err string }{ { name: "basic", vars: config.Variables{}, - step: &Step{ + step: &ShellStep{ Command: "echo hello", }, }, { name: "test nounset", vars: config.Variables{}, - step: &Step{ + step: &ShellStep{ Command: "echo $DOES_NOT_EXIST", }, err: "DOES_NOT_EXIST: unbound variable\n exit status 1", @@ -176,7 +176,7 @@ func TestRunShellStep(t *testing.T) { { name: "test errexit", vars: config.Variables{}, - step: &Step{ + step: &ShellStep{ Command: "false ; echo hello", }, err: "failed to execute shell command: exit status 1", @@ -184,7 +184,7 @@ func TestRunShellStep(t *testing.T) { { name: "test pipefail", vars: config.Variables{}, - step: &Step{ + step: &ShellStep{ Command: "false | echo", }, err: "failed to execute shell command: \n exit status 1", @@ -192,7 +192,7 @@ func TestRunShellStep(t *testing.T) { } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - err := tc.step.runShellStep(context.Background(), "", &PipelineRunOptions{}, map[string]output{}) + err := runShellStep(tc.step, context.Background(), "", &PipelineRunOptions{}, map[string]output{}) if tc.err != "" { assert.ErrorContains(t, err, tc.err) } else { diff --git a/tooling/templatize/pkg/pipeline/types.go b/tooling/templatize/pkg/pipeline/types.go index a24dcfae4..acafe0bfd 100644 --- a/tooling/templatize/pkg/pipeline/types.go +++ b/tooling/templatize/pkg/pipeline/types.go @@ -2,6 +2,8 @@ package pipeline import ( "context" + "fmt" + "strings" ) type subsciptionLookup func(context.Context, string) (string, error) @@ -14,25 +16,170 @@ type Pipeline struct { } type ResourceGroup struct { - Name string `yaml:"name"` - Subscription string `yaml:"subscription"` - AKSCluster string `yaml:"aksCluster,omitempty"` - Steps []*Step `yaml:"steps"` + Name string `yaml:"name"` + Subscription string `yaml:"subscription"` + AKSCluster string `yaml:"aksCluster,omitempty"` + Steps []Step `yaml:"steps"` +} + +func (rg *ResourceGroup) UnmarshalYAML(unmarshal func(interface{}) error) error { + rawRg := &struct { + Name string `yaml:"name"` + Subscription string `yaml:"subscription"` + AKSCluster string `yaml:"aksCluster,omitempty"` + Steps []rawStep `yaml:"steps"` + }{} + if err := unmarshal(&rawRg); err != nil { + return err + } + rg.Name = rawRg.Name + rg.Subscription = rawRg.Subscription + rg.AKSCluster = rawRg.AKSCluster + rg.Steps = make([]Step, len(rawRg.Steps)) + for i, rawStep := range rawRg.Steps { + switch rawStep.meta.Action { + case "Shell": + rg.Steps[i] = &ShellStep{} + case "ARM": + rg.Steps[i] = &ARMStep{} + default: + return fmt.Errorf("unknown action type %s", rawStep.meta.Action) + } + err := rawStep.unmarshal(rg.Steps[i]) + if err != nil { + return err + } + } + return nil } type outPutHandler func(string) -type Step struct { - Name string `yaml:"name"` - Action string `yaml:"action"` +type StepMeta struct { + Name string `yaml:"name"` + Action string `yaml:"action"` + DependsOn []string `yaml:"dependsOn,omitempty"` +} + +func (m *StepMeta) StepName() string { + return m.Name +} + +func (m *StepMeta) ActionType() string { + return m.Action +} + +func (m *StepMeta) Dependencies() []string { + return m.DependsOn +} + +type rawStep struct { + meta *StepMeta + unmarshal func(interface{}) error +} + +func (msg *rawStep) UnmarshalYAML(unmarshal func(interface{}) error) error { + msg.meta = &StepMeta{} + if err := unmarshal(msg.meta); err != nil { + return err + } + msg.unmarshal = unmarshal + return nil +} + +func (msg *rawStep) Unmarshal(v interface{}) error { + return msg.unmarshal(v) +} + +type Step interface { + StepName() string + ActionType() string + Description() string + Dependencies() []string +} + +func NewShellStep(name string, command string) *ShellStep { + return &ShellStep{ + StepMeta: StepMeta{ + Name: name, + Action: "Shell", + }, + Command: command, + } +} + +type ShellStep struct { + StepMeta `yaml:",inline"` + Command string `yaml:"command,omitempty"` + Variables []Variable `yaml:"variables,omitempty"` + DryRun DryRun `yaml:"dryRun,omitempty"` + outputFunc outPutHandler +} + +func (s *ShellStep) Description() string { + return fmt.Sprintf("Step %s\n Kind: %s\n Command: %s\n", s.Name, s.Action, s.Command) +} + +func (s *ShellStep) WithDependsOn(dependsOn ...string) *ShellStep { + s.DependsOn = dependsOn + return s +} + +func (s *ShellStep) WithVariables(variables ...Variable) *ShellStep { + s.Variables = variables + return s +} + +func (s *ShellStep) WithDryRun(dryRun DryRun) *ShellStep { + s.DryRun = dryRun + return s +} + +func (s *ShellStep) WithOutputFunc(outputFunc outPutHandler) *ShellStep { + s.outputFunc = outputFunc + return s +} + +func NewARMStep(name string, template string, parameters string) *ARMStep { + return &ARMStep{ + StepMeta: StepMeta{ + Name: name, + Action: "ARM", + }, + Template: template, + Parameters: parameters, + } +} + +func (s *ARMStep) WithDependsOn(dependsOn ...string) *ARMStep { + s.DependsOn = dependsOn + return s +} + +func (s *ARMStep) WithVariables(variables ...Variable) *ARMStep { + s.Variables = variables + return s +} + +func (s *ARMStep) WithDeploymentLevel(deploymentLevel string) *ARMStep { + s.DeploymentLevel = deploymentLevel + return s +} + +type ARMStep struct { + StepMeta `yaml:",inline"` Command string `yaml:"command,omitempty"` Variables []Variable `yaml:"variables,omitempty"` Template string `yaml:"template,omitempty"` Parameters string `yaml:"parameters,omitempty"` - DependsOn []string `yaml:"dependsOn,omitempty"` - DryRun DryRun `yaml:"dryRun,omitempty"` DeploymentLevel string `yaml:"deploymentLevel,omitempty"` - outputFunc outPutHandler +} + +func (s *ARMStep) Description() string { + var details []string + details = append(details, fmt.Sprintf("Template: %s", s.Template)) + details = append(details, fmt.Sprintf("Parameters: %s", s.Parameters)) + return fmt.Sprintf("Step %s\n Kind: %s\n %s", s.Name, s.Action, strings.Join(details, "\n ")) } type DryRun struct { diff --git a/tooling/templatize/pkg/pipeline/validation.go b/tooling/templatize/pkg/pipeline/validation.go index ec70460ab..99877a654 100644 --- a/tooling/templatize/pkg/pipeline/validation.go +++ b/tooling/templatize/pkg/pipeline/validation.go @@ -75,21 +75,21 @@ func compileSchema(schemaRef string, schemaBytes []byte) (*jsonschema.Schema, er func (p *Pipeline) Validate() error { // collect all steps from all resourcegroups and fail if there are duplicates - stepMap := make(map[string]*Step) + stepMap := make(map[string]Step) for _, rg := range p.ResourceGroups { for _, step := range rg.Steps { - if _, ok := stepMap[step.Name]; ok { - return fmt.Errorf("duplicate step name %q", step.Name) + if _, ok := stepMap[step.StepName()]; ok { + return fmt.Errorf("duplicate step name %q", step.StepName()) } - stepMap[step.Name] = step + stepMap[step.StepName()] = step } } // validate dependsOn for a step exists for _, step := range stepMap { - for _, dep := range step.DependsOn { + for _, dep := range step.Dependencies() { if _, ok := stepMap[dep]; !ok { - return fmt.Errorf("invalid dependency on step %s: dependency %s does not exist", step.Name, dep) + return fmt.Errorf("invalid dependency on step %s: dependency %s does not exist", step.StepName(), dep) } } } diff --git a/tooling/templatize/pkg/pipeline/validation_test.go b/tooling/templatize/pkg/pipeline/validation_test.go index 71a31b755..3855e0db8 100644 --- a/tooling/templatize/pkg/pipeline/validation_test.go +++ b/tooling/templatize/pkg/pipeline/validation_test.go @@ -64,20 +64,15 @@ func TestPipelineValidate(t *testing.T) { { Name: "rg1", Subscription: "sub1", - Steps: []*Step{ - { - Name: "step1", - }, + Steps: []Step{ + NewShellStep("step1", "echo foo"), }, }, { Name: "rg2", Subscription: "sub1", - Steps: []*Step{ - { - Name: "step2", - DependsOn: []string{"step3"}, - }, + Steps: []Step{ + NewShellStep("step2", "echo bar").WithDependsOn("step3"), }, }, }, @@ -91,19 +86,15 @@ func TestPipelineValidate(t *testing.T) { { Name: "rg1", Subscription: "sub1", - Steps: []*Step{ - { - Name: "step1", - }, + Steps: []Step{ + NewShellStep("step1", "echo foo"), }, }, { Name: "rg2", Subscription: "sub1", - Steps: []*Step{ - { - Name: "step1", - }, + Steps: []Step{ + NewShellStep("step1", "echo bar").WithDependsOn("step1"), }, }, }, @@ -117,20 +108,15 @@ func TestPipelineValidate(t *testing.T) { { Name: "rg1", Subscription: "sub1", - Steps: []*Step{ - { - Name: "step1", - }, + Steps: []Step{ + NewShellStep("step1", "echo foo"), }, }, { Name: "rg2", Subscription: "sub1", - Steps: []*Step{ - { - Name: "step2", - DependsOn: []string{"step1"}, - }, + Steps: []Step{ + NewShellStep("step2", "echo bar").WithDependsOn("step1"), }, }, }, From 8f7147d63a8ac27bd1c8d275547850f9d7b1f203 Mon Sep 17 00:00:00 2001 From: Gerd Oberlechner Date: Wed, 18 Dec 2024 17:02:09 +0100 Subject: [PATCH 2/6] remove parsing indirection on step level Signed-off-by: Gerd Oberlechner --- tooling/templatize/pkg/pipeline/types.go | 51 ++++++++++++------------ 1 file changed, 26 insertions(+), 25 deletions(-) diff --git a/tooling/templatize/pkg/pipeline/types.go b/tooling/templatize/pkg/pipeline/types.go index acafe0bfd..7ed3a1a0b 100644 --- a/tooling/templatize/pkg/pipeline/types.go +++ b/tooling/templatize/pkg/pipeline/types.go @@ -4,6 +4,8 @@ import ( "context" "fmt" "strings" + + "gopkg.in/yaml.v3" ) type subsciptionLookup func(context.Context, string) (string, error) @@ -24,10 +26,10 @@ type ResourceGroup struct { func (rg *ResourceGroup) UnmarshalYAML(unmarshal func(interface{}) error) error { rawRg := &struct { - Name string `yaml:"name"` - Subscription string `yaml:"subscription"` - AKSCluster string `yaml:"aksCluster,omitempty"` - Steps []rawStep `yaml:"steps"` + Name string `yaml:"name"` + Subscription string `yaml:"subscription"` + AKSCluster string `yaml:"aksCluster,omitempty"` + Steps []any `yaml:"steps"` }{} if err := unmarshal(&rawRg); err != nil { return err @@ -37,15 +39,21 @@ func (rg *ResourceGroup) UnmarshalYAML(unmarshal func(interface{}) error) error rg.AKSCluster = rawRg.AKSCluster rg.Steps = make([]Step, len(rawRg.Steps)) for i, rawStep := range rawRg.Steps { - switch rawStep.meta.Action { + // unmarshal the map into a StepMeta + stepMeta := &StepMeta{} + err := mapToStruct(rawStep, stepMeta) + if err != nil { + return err + } + switch stepMeta.Action { case "Shell": rg.Steps[i] = &ShellStep{} case "ARM": rg.Steps[i] = &ARMStep{} default: - return fmt.Errorf("unknown action type %s", rawStep.meta.Action) + return fmt.Errorf("unknown action type %s", stepMeta.Action) } - err := rawStep.unmarshal(rg.Steps[i]) + err = mapToStruct(rawStep, rg.Steps[i]) if err != nil { return err } @@ -53,6 +61,17 @@ func (rg *ResourceGroup) UnmarshalYAML(unmarshal func(interface{}) error) error return nil } +func mapToStruct(m any, s interface{}) error { + bytes, err := yaml.Marshal(m) + if err != nil { + return err + } + if err := yaml.Unmarshal(bytes, s); err != nil { + return err + } + return nil +} + type outPutHandler func(string) type StepMeta struct { @@ -73,24 +92,6 @@ func (m *StepMeta) Dependencies() []string { return m.DependsOn } -type rawStep struct { - meta *StepMeta - unmarshal func(interface{}) error -} - -func (msg *rawStep) UnmarshalYAML(unmarshal func(interface{}) error) error { - msg.meta = &StepMeta{} - if err := unmarshal(msg.meta); err != nil { - return err - } - msg.unmarshal = unmarshal - return nil -} - -func (msg *rawStep) Unmarshal(v interface{}) error { - return msg.unmarshal(v) -} - type Step interface { StepName() string ActionType() string From 90698b709a80e1154da16a5a46ca110a11d34210 Mon Sep 17 00:00:00 2001 From: Gerd Oberlechner Date: Wed, 18 Dec 2024 17:34:13 +0100 Subject: [PATCH 3/6] remove RG parsing magic func Signed-off-by: Gerd Oberlechner --- tooling/templatize/pkg/pipeline/common.go | 5 +- tooling/templatize/pkg/pipeline/run.go | 7 +- tooling/templatize/pkg/pipeline/types.go | 79 ++++++++++++++--------- 3 files changed, 51 insertions(+), 40 deletions(-) diff --git a/tooling/templatize/pkg/pipeline/common.go b/tooling/templatize/pkg/pipeline/common.go index 2a2584000..8d5d62577 100644 --- a/tooling/templatize/pkg/pipeline/common.go +++ b/tooling/templatize/pkg/pipeline/common.go @@ -8,16 +8,15 @@ import ( ) func (p *Pipeline) DeepCopy(newPipelineFilePath string) (*Pipeline, error) { - copy := new(Pipeline) data, err := yaml.Marshal(p) if err != nil { return nil, fmt.Errorf("failed to marshal pipeline: %v", err) } - err = yaml.Unmarshal(data, copy) + + copy, err := NewPlainPipelineFromBytes(newPipelineFilePath, data) if err != nil { return nil, fmt.Errorf("failed to unmarshal pipeline: %v", err) } - copy.pipelineFilePath = newPipelineFilePath return copy, nil } diff --git a/tooling/templatize/pkg/pipeline/run.go b/tooling/templatize/pkg/pipeline/run.go index 989bdb6ab..0589f468a 100644 --- a/tooling/templatize/pkg/pipeline/run.go +++ b/tooling/templatize/pkg/pipeline/run.go @@ -6,8 +6,6 @@ import ( "os" "path/filepath" - "gopkg.in/yaml.v3" - "github.com/go-logr/logr" "github.com/Azure/ARO-HCP/tooling/templatize/pkg/config" @@ -29,10 +27,7 @@ func NewPipelineFromFile(pipelineFilePath string, vars config.Variables) (*Pipel return nil, fmt.Errorf("failed to get absolute path for pipeline file %q: %w", pipelineFilePath, err) } - pipeline := &Pipeline{ - pipelineFilePath: absPath, - } - err = yaml.Unmarshal(bytes, pipeline) + pipeline, err := NewPlainPipelineFromBytes(absPath, bytes) if err != nil { return nil, fmt.Errorf("failed to unmarshal pipeline file %w", err) } diff --git a/tooling/templatize/pkg/pipeline/types.go b/tooling/templatize/pkg/pipeline/types.go index 7ed3a1a0b..30d6c50a5 100644 --- a/tooling/templatize/pkg/pipeline/types.go +++ b/tooling/templatize/pkg/pipeline/types.go @@ -24,41 +24,58 @@ type ResourceGroup struct { Steps []Step `yaml:"steps"` } -func (rg *ResourceGroup) UnmarshalYAML(unmarshal func(interface{}) error) error { - rawRg := &struct { - Name string `yaml:"name"` - Subscription string `yaml:"subscription"` - AKSCluster string `yaml:"aksCluster,omitempty"` - Steps []any `yaml:"steps"` +func NewPlainPipelineFromBytes(filepath string, bytes []byte) (*Pipeline, error) { + rawPipeline := &struct { + ServiceGroup string `yaml:"serviceGroup"` + RolloutName string `yaml:"rolloutName"` + ResourceGroups []struct { + Name string `yaml:"name"` + Subscription string `yaml:"subscription"` + AKSCluster string `yaml:"aksCluster,omitempty"` + Steps []any `yaml:"steps"` + } `yaml:"resourceGroups"` }{} - if err := unmarshal(&rawRg); err != nil { - return err + err := yaml.Unmarshal(bytes, rawPipeline) + if err != nil { + return nil, err } - rg.Name = rawRg.Name - rg.Subscription = rawRg.Subscription - rg.AKSCluster = rawRg.AKSCluster - rg.Steps = make([]Step, len(rawRg.Steps)) - for i, rawStep := range rawRg.Steps { - // unmarshal the map into a StepMeta - stepMeta := &StepMeta{} - err := mapToStruct(rawStep, stepMeta) - if err != nil { - return err - } - switch stepMeta.Action { - case "Shell": - rg.Steps[i] = &ShellStep{} - case "ARM": - rg.Steps[i] = &ARMStep{} - default: - return fmt.Errorf("unknown action type %s", stepMeta.Action) - } - err = mapToStruct(rawStep, rg.Steps[i]) - if err != nil { - return err + pipeline := &Pipeline{ + pipelineFilePath: filepath, + ServiceGroup: rawPipeline.ServiceGroup, + RolloutName: rawPipeline.RolloutName, + ResourceGroups: make([]*ResourceGroup, len(rawPipeline.ResourceGroups)), + } + + for i, rawRg := range rawPipeline.ResourceGroups { + rg := &ResourceGroup{} + pipeline.ResourceGroups[i] = rg + rg.Name = rawRg.Name + rg.Subscription = rawRg.Subscription + rg.AKSCluster = rawRg.AKSCluster + rg.Steps = make([]Step, len(rawRg.Steps)) + for i, rawStep := range rawRg.Steps { + // unmarshal the map into a StepMeta + stepMeta := &StepMeta{} + err := mapToStruct(rawStep, stepMeta) + if err != nil { + return nil, err + } + switch stepMeta.Action { + case "Shell": + rg.Steps[i] = &ShellStep{} + case "ARM": + rg.Steps[i] = &ARMStep{} + default: + return nil, fmt.Errorf("unknown action type %s", stepMeta.Action) + } + err = mapToStruct(rawStep, rg.Steps[i]) + if err != nil { + return nil, err + } } } - return nil + + return pipeline, nil } func mapToStruct(m any, s interface{}) error { From 809d83b839895d9a3afb91ca8cacc2ea660edc40 Mon Sep 17 00:00:00 2001 From: Gerd Oberlechner Date: Wed, 18 Dec 2024 15:11:43 +0100 Subject: [PATCH 4/6] new types Signed-off-by: Gerd Oberlechner --- .../pkg/pipeline/pipeline.schema.v1.json | 114 +++++++++++++++++- tooling/templatize/pkg/pipeline/run.go | 3 +- tooling/templatize/pkg/pipeline/types.go | 31 +++++ tooling/templatize/testdata/pipeline.yaml | 16 +++ ...ure_TestProcessPipelineForEV2pipeline.yaml | 16 +++ .../testdata/zz_fixture_TestRawOptions.yaml | 16 +++ 6 files changed, 194 insertions(+), 2 deletions(-) diff --git a/tooling/templatize/pkg/pipeline/pipeline.schema.v1.json b/tooling/templatize/pkg/pipeline/pipeline.schema.v1.json index 3c1bb8225..25c20e238 100644 --- a/tooling/templatize/pkg/pipeline/pipeline.schema.v1.json +++ b/tooling/templatize/pkg/pipeline/pipeline.schema.v1.json @@ -3,6 +3,50 @@ "title": "pipeline.schema.v1", "type": "object", "definitions": { + "variableRef": { + "type": "object", + "properties": { + "input": { + "type": "object", + "additionalProperties": false, + "properties": { + "step": { + "type": "string" + }, + "name": { + "type": "string" + } + }, + "required": [ + "step", + "name" + ] + }, + "configRef": { + "type": "string" + }, + "value": { + "type": "string" + } + }, + "oneOf": [ + { + "required": [ + "input" + ] + }, + { + "required": [ + "configRef" + ] + }, + { + "required": [ + "value" + ] + } + ] + }, "variable": { "type": "object", "properties": { @@ -86,7 +130,7 @@ }, "action": { "type": "string", - "enum": ["ARM", "Shell"] + "enum": ["ARM", "Shell", "DelegateChildZoneExtension", "SetCertificateIssuer"] }, "template": { "type": "string" @@ -115,6 +159,18 @@ }, "dryRun": { "type": "object" + }, + "vaultBaseUrl": { + "$ref": "#/definitions/variableRef" + }, + "provider": { + "$ref": "#/definitions/variableRef" + }, + "parentZoneName": { + "$ref": "#/definitions/variableRef" + }, + "childZoneName": { + "$ref": "#/definitions/variableRef" } }, "oneOf": [ @@ -200,6 +256,62 @@ "required": [ "command" ] + }, + { + "additionalProperties": false, + "properties": { + "name": { + "type": "string" + }, + "action": { + "type": "string", + "enum": ["DelegateChildZoneExtension"] + }, + "parentZoneName": { + "$ref": "#/definitions/variableRef" + }, + "childZoneName": { + "$ref": "#/definitions/variableRef" + }, + "dependsOn": { + "type": "array", + "items": { + "type": "string" + } + } + }, + "required": [ + "parentZoneName", + "childZoneName" + ] + }, + { + "additionalProperties": false, + "properties": { + "name": { + "type": "string" + }, + "action": { + "type": "string", + "enum": ["SetCertificateIssuer"] + }, + "vaultBaseUrl": { + "$ref": "#/definitions/variableRef" + }, + "provider": { + "$ref": "#/definitions/variableRef" + }, + "dependsOn": { + "type": "array", + "items": { + "type": "string" + } + } + }, + "required": [ + "vaultBaseUrl", + "provider" + ] } ], "required": [ diff --git a/tooling/templatize/pkg/pipeline/run.go b/tooling/templatize/pkg/pipeline/run.go index 0589f468a..279e4e682 100644 --- a/tooling/templatize/pkg/pipeline/run.go +++ b/tooling/templatize/pkg/pipeline/run.go @@ -186,7 +186,8 @@ func RunStep(s Step, ctx context.Context, kubeconfigFile string, executionTarget } return output, nil default: - return nil, fmt.Errorf("unsupported action type %q", s.ActionType()) + fmt.Println("No implementation for action type - skip", s.ActionType()) + return nil, nil } } diff --git a/tooling/templatize/pkg/pipeline/types.go b/tooling/templatize/pkg/pipeline/types.go index 30d6c50a5..5e04ee19b 100644 --- a/tooling/templatize/pkg/pipeline/types.go +++ b/tooling/templatize/pkg/pipeline/types.go @@ -65,6 +65,10 @@ func NewPlainPipelineFromBytes(filepath string, bytes []byte) (*Pipeline, error) rg.Steps[i] = &ShellStep{} case "ARM": rg.Steps[i] = &ARMStep{} + case "DelegateChildZoneExtension": + rg.Steps[i] = &DelegateChildZoneStep{} + case "SetCertificateIssuer": + rg.Steps[i] = &SetCertificateIssuerStep{} default: return nil, fmt.Errorf("unknown action type %s", stepMeta.Action) } @@ -85,6 +89,7 @@ func mapToStruct(m any, s interface{}) error { } if err := yaml.Unmarshal(bytes, s); err != nil { return err + } return nil } @@ -200,6 +205,26 @@ func (s *ARMStep) Description() string { return fmt.Sprintf("Step %s\n Kind: %s\n %s", s.Name, s.Action, strings.Join(details, "\n ")) } +type DelegateChildZoneStep struct { + StepMeta `yaml:",inline"` + ParentZoneName VariableRef `yaml:"parentZoneName"` + ChildZoneName VariableRef `yaml:"childZoneName"` +} + +func (s *DelegateChildZoneStep) Description() string { + return fmt.Sprintf("Step %s\n Kind: %s", s.Name, s.Action) +} + +type SetCertificateIssuerStep struct { + StepMeta `yaml:",inline"` + VaultBaseUrl VariableRef `yaml:"vaultBaseUrl"` + Provider VariableRef `yaml:"provider"` +} + +func (s *SetCertificateIssuerStep) Description() string { + return fmt.Sprintf("Step %s\n Kind: %s", s.Name, s.Action) +} + type DryRun struct { Variables []Variable `yaml:"variables,omitempty"` Command string `yaml:"command,omitempty"` @@ -212,6 +237,12 @@ type Variable struct { Input *Input `yaml:"input,omitempty"` } +type VariableRef struct { + ConfigRef string `yaml:"configRef,omitempty"` + Value string `yaml:"value,omitempty"` + Input *Input `yaml:"input,omitempty"` +} + type Input struct { Name string `yaml:"name"` Step string `yaml:"step"` diff --git a/tooling/templatize/testdata/pipeline.yaml b/tooling/templatize/testdata/pipeline.yaml index 6ac26dcbb..d1f7139a2 100644 --- a/tooling/templatize/testdata/pipeline.yaml +++ b/tooling/templatize/testdata/pipeline.yaml @@ -23,3 +23,19 @@ resourceGroups: action: ARM template: templates/svc-cluster.bicep parameters: test.bicepparam + - name: DelegateChildZoneBase + action: DelegateChildZoneExtension + parentZoneName: + value: bla + childZoneName: + value: blub + dependsOn: + - deploy + - name: issuerTest + action: SetCertificateIssuer + vaultBaseUrl: + value: bla + provider: + value: blub + dependsOn: + - deploy diff --git a/tooling/templatize/testdata/zz_fixture_TestProcessPipelineForEV2pipeline.yaml b/tooling/templatize/testdata/zz_fixture_TestProcessPipelineForEV2pipeline.yaml index 7448f9363..b77f958a3 100644 --- a/tooling/templatize/testdata/zz_fixture_TestProcessPipelineForEV2pipeline.yaml +++ b/tooling/templatize/testdata/zz_fixture_TestProcessPipelineForEV2pipeline.yaml @@ -22,3 +22,19 @@ resourceGroups: action: ARM template: templates/svc-cluster.bicep parameters: ev2-precompiled-test.bicepparam + - name: DelegateChildZoneBase + action: DelegateChildZoneExtension + dependsOn: + - deploy + parentZoneName: + value: bla + childZoneName: + value: blub + - name: issuerTest + action: SetCertificateIssuer + dependsOn: + - deploy + vaultBaseUrl: + value: bla + provider: + value: blub diff --git a/tooling/templatize/testdata/zz_fixture_TestRawOptions.yaml b/tooling/templatize/testdata/zz_fixture_TestRawOptions.yaml index dbe5f20d8..80c3b541d 100644 --- a/tooling/templatize/testdata/zz_fixture_TestRawOptions.yaml +++ b/tooling/templatize/testdata/zz_fixture_TestRawOptions.yaml @@ -23,3 +23,19 @@ resourceGroups: action: ARM template: templates/svc-cluster.bicep parameters: test.bicepparam + - name: DelegateChildZoneBase + action: DelegateChildZoneExtension + parentZoneName: + value: bla + childZoneName: + value: blub + dependsOn: + - deploy + - name: issuerTest + action: SetCertificateIssuer + vaultBaseUrl: + value: bla + provider: + value: blub + dependsOn: + - deploy From 985e87d0987fca0279e882ebe85bfb2a44d79547 Mon Sep 17 00:00:00 2001 From: Gerd Oberlechner Date: Wed, 18 Dec 2024 20:23:26 +0100 Subject: [PATCH 5/6] update testdata for configref Signed-off-by: Gerd Oberlechner --- tooling/templatize/pkg/ev2/utils_test.go | 4 ++++ tooling/templatize/testdata/config.yaml | 4 ++++ tooling/templatize/testdata/pipeline.yaml | 8 ++++---- .../zz_fixture_TestProcessPipelineForEV2pipeline.yaml | 8 ++++---- .../templatize/testdata/zz_fixture_TestRawOptions.yaml | 8 ++++---- 5 files changed, 20 insertions(+), 12 deletions(-) diff --git a/tooling/templatize/pkg/ev2/utils_test.go b/tooling/templatize/pkg/ev2/utils_test.go index e99c524e1..8296d6345 100644 --- a/tooling/templatize/pkg/ev2/utils_test.go +++ b/tooling/templatize/pkg/ev2/utils_test.go @@ -16,16 +16,20 @@ func TestScopeBindingVariables(t *testing.T) { } expectedVars := map[string]string{ "__aksName__": "$config(aksName)", + "__childZone__": "$config(childZone)", "__globalRG__": "$config(globalRG)", "__imageSyncRG__": "$config(imageSyncRG)", "__maestro_helm_chart__": "$config(maestro_helm_chart)", "__maestro_image__": "$config(maestro_image)", "__managementClusterRG__": "$config(managementClusterRG)", "__managementClusterSubscription__": "$config(managementClusterSubscription)", + "__parentZone__": "$config(parentZone)", + "__provider__": "$config(provider)", "__region__": "$config(region)", "__regionRG__": "$config(regionRG)", "__serviceClusterRG__": "$config(serviceClusterRG)", "__serviceClusterSubscription__": "$config(serviceClusterSubscription)", + "__vaultBaseUrl__": "$config(vaultBaseUrl)", "__clusterService.imageTag__": "$config(clusterService.imageTag)", "__clusterService.replicas__": "$config(clusterService.replicas)", } diff --git a/tooling/templatize/testdata/config.yaml b/tooling/templatize/testdata/config.yaml index 4928b3565..ea16ffb32 100644 --- a/tooling/templatize/testdata/config.yaml +++ b/tooling/templatize/testdata/config.yaml @@ -12,6 +12,10 @@ defaults: clusterService: imageTag: abcdef replicas: 3 + parentZone: example.com + childZone: child.example.com + vaultBaseUrl: myvault.azure.com + provider: Self clouds: fairfax: defaults: diff --git a/tooling/templatize/testdata/pipeline.yaml b/tooling/templatize/testdata/pipeline.yaml index d1f7139a2..a7e2e48b5 100644 --- a/tooling/templatize/testdata/pipeline.yaml +++ b/tooling/templatize/testdata/pipeline.yaml @@ -26,16 +26,16 @@ resourceGroups: - name: DelegateChildZoneBase action: DelegateChildZoneExtension parentZoneName: - value: bla + configRef: parentZone childZoneName: - value: blub + value: childZone dependsOn: - deploy - name: issuerTest action: SetCertificateIssuer vaultBaseUrl: - value: bla + configRef: vaultBaseUrl provider: - value: blub + configRef: provider dependsOn: - deploy diff --git a/tooling/templatize/testdata/zz_fixture_TestProcessPipelineForEV2pipeline.yaml b/tooling/templatize/testdata/zz_fixture_TestProcessPipelineForEV2pipeline.yaml index b77f958a3..c909a934f 100644 --- a/tooling/templatize/testdata/zz_fixture_TestProcessPipelineForEV2pipeline.yaml +++ b/tooling/templatize/testdata/zz_fixture_TestProcessPipelineForEV2pipeline.yaml @@ -27,14 +27,14 @@ resourceGroups: dependsOn: - deploy parentZoneName: - value: bla + configRef: parentZone childZoneName: - value: blub + value: childZone - name: issuerTest action: SetCertificateIssuer dependsOn: - deploy vaultBaseUrl: - value: bla + configRef: vaultBaseUrl provider: - value: blub + configRef: provider diff --git a/tooling/templatize/testdata/zz_fixture_TestRawOptions.yaml b/tooling/templatize/testdata/zz_fixture_TestRawOptions.yaml index 80c3b541d..cf06a11c3 100644 --- a/tooling/templatize/testdata/zz_fixture_TestRawOptions.yaml +++ b/tooling/templatize/testdata/zz_fixture_TestRawOptions.yaml @@ -26,16 +26,16 @@ resourceGroups: - name: DelegateChildZoneBase action: DelegateChildZoneExtension parentZoneName: - value: bla + configRef: parentZone childZoneName: - value: blub + value: childZone dependsOn: - deploy - name: issuerTest action: SetCertificateIssuer vaultBaseUrl: - value: bla + configRef: vaultBaseUrl provider: - value: blub + configRef: provider dependsOn: - deploy From a3a57287ac9e9a984c6e2cc970604628a1abb480 Mon Sep 17 00:00:00 2001 From: Gerd Oberlechner Date: Thu, 19 Dec 2024 10:01:44 +0100 Subject: [PATCH 6/6] json array handling Signed-off-by: Gerd Oberlechner --- tooling/templatize/cmd/generate/options.go | 12 +---- tooling/templatize/pkg/config/config.go | 37 +++++++++++++--- tooling/templatize/pkg/config/config_test.go | 46 ++++++++++++++++++++ tooling/templatize/pkg/ev2/mapping_test.go | 8 ++++ 4 files changed, 86 insertions(+), 17 deletions(-) diff --git a/tooling/templatize/cmd/generate/options.go b/tooling/templatize/cmd/generate/options.go index 88b48b18d..ebac5e6b9 100644 --- a/tooling/templatize/cmd/generate/options.go +++ b/tooling/templatize/cmd/generate/options.go @@ -6,12 +6,11 @@ import ( "io/fs" "os" "path/filepath" - "text/template" - "github.com/Masterminds/sprig/v3" "github.com/spf13/cobra" options "github.com/Azure/ARO-HCP/tooling/templatize/cmd" + "github.com/Azure/ARO-HCP/tooling/templatize/pkg/config" ) func DefaultGenerationOptions() *RawGenerationOptions { @@ -113,16 +112,9 @@ func (o *ValidatedGenerationOptions) Complete() (*GenerationOptions, error) { } func (opts *GenerationOptions) ExecuteTemplate() error { - tmpl := template.New(opts.InputFile).Funcs(sprig.FuncMap()) content, err := fs.ReadFile(opts.InputFS, opts.InputFile) if err != nil { return err } - - tmpl, err = tmpl.Parse(string(content)) - if err != nil { - return err - } - - return tmpl.Option("missingkey=error").ExecuteTemplate(opts.OutputFile, opts.InputFile, opts.RolloutOptions.Config) + return config.PreprocessContentIntoWriter(content, opts.RolloutOptions.Config, opts.OutputFile) } diff --git a/tooling/templatize/pkg/config/config.go b/tooling/templatize/pkg/config/config.go index 1ea492b53..b084731d3 100644 --- a/tooling/templatize/pkg/config/config.go +++ b/tooling/templatize/pkg/config/config.go @@ -2,7 +2,9 @@ package config import ( "bytes" + "encoding/json" "fmt" + "io" "os" "path/filepath" "reflect" @@ -260,15 +262,36 @@ func PreprocessFile(templateFilePath string, vars map[string]any) ([]byte, error // PreprocessContent processes a gotemplate from memory func PreprocessContent(content []byte, vars map[string]any) ([]byte, error) { - tmpl := template.New("file") - tmpl, err := tmpl.Parse(string(content)) + var tmplBytes bytes.Buffer + if err := PreprocessContentIntoWriter(content, vars, &tmplBytes); err != nil { + return nil, err + } + return tmplBytes.Bytes(), nil +} + +func PreprocessContentIntoWriter(content []byte, vars map[string]any, writer io.Writer) error { + funcMap := template.FuncMap{ + "json": jsonEncoder, + } + tmpl, err := template.New("file").Funcs(funcMap).Parse(string(content)) if err != nil { - return nil, fmt.Errorf("failed to parse template: %w", err) + return fmt.Errorf("failed to parse template: %w", err) } - var tmplBytes bytes.Buffer - if err := tmpl.Option("missingkey=error").Execute(&tmplBytes, vars); err != nil { - return nil, fmt.Errorf("failed to execute template: %w", err) + if err := tmpl.Option("missingkey=error").Execute(writer, vars); err != nil { + return fmt.Errorf("failed to execute template: %w", err) } - return tmplBytes.Bytes(), nil + return nil +} + +func jsonEncoder(value interface{}) (string, error) { + valueType := reflect.TypeOf(value) + if valueType.Kind() == reflect.String { + return value.(string), nil + } + jsonBytes, err := json.Marshal(value) + if err != nil { + return "", err + } + return string(jsonBytes), nil } diff --git a/tooling/templatize/pkg/config/config_test.go b/tooling/templatize/pkg/config/config_test.go index e999ec7db..a73b0f800 100644 --- a/tooling/templatize/pkg/config/config_test.go +++ b/tooling/templatize/pkg/config/config_test.go @@ -5,6 +5,7 @@ import ( "os" "testing" + "github.com/google/go-cmp/cmp" "github.com/stretchr/testify/assert" "github.com/Azure/ARO-HCP/tooling/templatize/internal/testutil" @@ -308,3 +309,48 @@ func TestPreprocessContentMissingKey(t *testing.T) { }) } } + +func TestPreprocessContentJson(t *testing.T) { + templateContent := "{{ .variable | json }}" + + testCases := []struct { + name string + value any + expectedResult string + }{ + { + name: "string value", + value: "foo", + expectedResult: "foo", + }, + { + name: "array value", + value: []string{"foo", "bar"}, + expectedResult: "[\"foo\",\"bar\"]", + }, + { + name: "int value", + value: 42, + expectedResult: "42", + }, + { + name: "bool value", + value: true, + expectedResult: "true", + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + processed, err := PreprocessContent( + []byte(templateContent), + map[string]any{ + "variable": tc.value, + }, + ) + assert.Nil(t, err) + if diff := cmp.Diff(string(processed), string(tc.expectedResult)); diff != "" { + t.Errorf("got diff between expected and actual result\ndiff:\n%s\n\nIf this is expected, re-run the test with `UPDATE=true go test ./...` to update the fixtures.", diff) + } + }) + } +} diff --git a/tooling/templatize/pkg/ev2/mapping_test.go b/tooling/templatize/pkg/ev2/mapping_test.go index 8b334604b..f6975714f 100644 --- a/tooling/templatize/pkg/ev2/mapping_test.go +++ b/tooling/templatize/pkg/ev2/mapping_test.go @@ -128,6 +128,14 @@ func TestPlaceholderGenerators(t *testing.T) { expectedFlattened: "__foo.bar__", expectedReplace: "any('__foo.bar__')", }, + { + name: "bicep array param", + generator: NewBicepParamPlaceholders(), + key: []string{"foo", "bar"}, + valueType: reflect.TypeOf([]any{}), + expectedFlattened: "__foo.bar__", + expectedReplace: "any('__foo.bar__')", + }, } for _, tc := range tests {