From 30a149a1d1f3ad62e18b5aa9b91090549849ec20 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A1bor=20Szak=C3=A1cs?= Date: Mon, 16 Dec 2024 14:40:00 +0100 Subject: [PATCH] Understand workflow variants when validating the pipeline [CI-4035] (#1029) * Understand workflow variants when validating the pipeline * Add more validation * Rename source to uses * Fix linting issues --- cli/agent.go | 2 +- cli/docker/container_manager.go | 19 +++++++++--------- cli/run_util_pipeline_test.go | 35 +++++++++++++++++++++++++++++++++ log/log.go | 6 +++--- models/models.go | 5 +++-- models/models_methods.go | 22 +++++++++++++++------ 6 files changed, 68 insertions(+), 21 deletions(-) diff --git a/cli/agent.go b/cli/agent.go index 2a4ba1a35..ac0654e0f 100644 --- a/cli/agent.go +++ b/cli/agent.go @@ -114,7 +114,7 @@ func cleanupDirs(dirs []string) error { if err := os.RemoveAll(absPath); err != nil { return fmt.Errorf("cleaning up %s: %w", dir, err) } - log.Donef("- Cleaned %s", colorstring.Cyan(expandedPath)) + log.Donef("- Cleaned %s", colorstring.Cyan("%s", expandedPath)) } return nil diff --git a/cli/docker/container_manager.go b/cli/docker/container_manager.go index ff51a021f..3efa01ff2 100644 --- a/cli/docker/container_manager.go +++ b/cli/docker/container_manager.go @@ -11,13 +11,14 @@ import ( "sync" "time" + "github.com/docker/docker/api/types" + "github.com/docker/docker/api/types/filters" + "github.com/docker/docker/client" + "github.com/bitrise-io/bitrise/log" "github.com/bitrise-io/bitrise/models" "github.com/bitrise-io/go-utils/command" "github.com/bitrise-io/go-utils/v2/redactwriter" - "github.com/docker/docker/api/types" - "github.com/docker/docker/api/types/filters" - "github.com/docker/docker/client" ) type RunningContainer struct { @@ -267,7 +268,7 @@ func (cm *ContainerManager) login(container models.Container, envs map[string]st out, err := command.New("docker", args...).RunAndReturnTrimmedCombinedOutput() if err != nil { - cm.logger.Errorf(out) + cm.logger.Errorf("%s", out) return fmt.Errorf("run docker login: %w", err) } } @@ -328,7 +329,7 @@ func (cm *ContainerManager) startContainer(options containerCreateOptions) (*Run cm.logger.Infof("ℹ️ Running command: docker start %s", options.name) out, err := command.New("docker", "start", options.name).RunAndReturnTrimmedCombinedOutput() if err != nil { - cm.logger.Errorf(out) + cm.logger.Errorf("%s", out) return runningContainer, fmt.Errorf("start docker container (%s): %w", options.name, err) } @@ -349,7 +350,8 @@ func (cm *ContainerManager) createContainer( options containerCreateOptions, envs map[string]string, ) error { - dockerRunArgs := []string{"create", + dockerRunArgs := []string{ + "create", "--platform", "linux/amd64", fmt.Sprintf("--network=%s", bitriseNetwork), } @@ -406,7 +408,7 @@ func (cm *ContainerManager) createContainer( out, err := command.New("docker", dockerRunArgs...).RunAndReturnTrimmedCombinedOutput() if err != nil { - cm.logger.Errorf(out) + cm.logger.Errorf("%s", out) return fmt.Errorf("create container (%s): %w", options.name, err) } @@ -460,7 +462,7 @@ func (cm *ContainerManager) pullImage(container models.Container) error { cm.logger.Infof("ℹ️ Running command: docker %s", strings.Join(dockerRunArgs, " ")) out, err := command.New("docker", dockerRunArgs...).RunAndReturnTrimmedCombinedOutput() if err != nil { - cm.logger.Errorf(out) + cm.logger.Errorf("%s", out) return fmt.Errorf("pull container (%s): %w", container.Image, err) } return nil @@ -495,7 +497,6 @@ func (cm *ContainerManager) getRunningContainer(ctx context.Context, name string return &containers[0], fmt.Errorf("container (%s) is not running", name) } return &containers[0], nil - } func (cm *ContainerManager) healthCheckContainer(ctx context.Context, container *RunningContainer) error { diff --git a/cli/run_util_pipeline_test.go b/cli/run_util_pipeline_test.go index 170b1f132..9a58d8c66 100644 --- a/cli/run_util_pipeline_test.go +++ b/cli/run_util_pipeline_test.go @@ -41,6 +41,7 @@ pipelines: dag: workflows: a: {} + a1: { uses: a, inputs: [key: value] } b: { depends_on: [a] } c: { depends_on: [a] } d: { depends_on: [a] } @@ -102,6 +103,30 @@ workflows: b: {} ` +const missingWorkflowInWorkflowVariantDefinitionForDAGPipeline = ` +format_version: '13' +pipelines: + dag: + workflows: + a: {} + b: { uses: c } +workflows: + a: {} +` + +const workflowVariantHasTheSameNameAsAnExistingWorkflowForDAGPipeline = ` +format_version: '13' +pipelines: + dag: + workflows: + a: {} + b: { uses: c } +workflows: + a: {} + b: {} + c: {} +` + const duplicatedDependencyDAGPipeline = ` format_version: '13' pipelines: @@ -170,6 +195,16 @@ func TestValidation(t *testing.T) { config: missingWorkflowInWorkflowDefinitionForDAGPipeline, wantErr: "failed to get Bitrise config (bitrise.yml) from base 64 data: Failed to parse bitrise config, error: workflow (c) defined in pipeline (dag) is not found in the workflow definitions", }, + { + name: "Workflow is missing from the Workflow Variant definition", + config: missingWorkflowInWorkflowVariantDefinitionForDAGPipeline, + wantErr: "failed to get Bitrise config (bitrise.yml) from base 64 data: Failed to parse bitrise config, error: workflow (c) referenced in pipeline (dag) in workflow variant (b) is not found in the workflow definitions", + }, + { + name: "Workflow variant has the same name as an existing workflow", + config: workflowVariantHasTheSameNameAsAnExistingWorkflowForDAGPipeline, + wantErr: "failed to get Bitrise config (bitrise.yml) from base 64 data: Failed to parse bitrise config, error: workflow (b) defined in pipeline (dag) is a variant of another workflow, but it is also defined as a workflow", + }, { name: "Utility workflow is referenced in the DAG pipeline", config: utilityWorkflowDAGPipeline, diff --git a/log/log.go b/log/log.go index c7354c64c..ee216a963 100644 --- a/log/log.go +++ b/log/log.go @@ -145,8 +145,8 @@ func (m *defaultLogger) PrintBitriseStartedEvent(plan models.WorkflowRunPlan) { }) } else { m.Print() - m.Printf("Invocation started at %s", colorstring.Cyan(m.opts.TimeProvider().Format(consoleTimeLayout))) - m.Printf("Bitrise CLI version: %s", colorstring.Cyan(plan.Version)) + m.Printf("Invocation started at %s", colorstring.Cyan("%s", m.opts.TimeProvider().Format(consoleTimeLayout))) + m.Printf("Bitrise CLI version: %s", colorstring.Cyan("%s", plan.Version)) m.Print() m.Infof("Run modes:") m.Printf("CI mode: %v", colorstring.Cyan("%v", plan.CIMode)) @@ -169,7 +169,7 @@ func (m *defaultLogger) PrintBitriseStartedEvent(plan models.WorkflowRunPlan) { prefix = "Running workflows" } - m.Printf("%s: %s", prefix, colorstring.Cyan(strings.Join(workflowIDs, " → "))) + m.Printf("%s: %s", prefix, colorstring.Cyan("%s", strings.Join(workflowIDs, " → "))) } } diff --git a/models/models.go b/models/models.go index 89212f562..824c9170b 100644 --- a/models/models.go +++ b/models/models.go @@ -82,6 +82,7 @@ type GraphPipelineWorkflowModel struct { AbortOnFail bool `json:"abort_on_fail,omitempty" yaml:"abort_on_fail,omitempty"` RunIf GraphPipelineRunIfModel `json:"run_if,omitempty" yaml:"run_if,omitempty"` ShouldAlwaysRun GraphPipelineAlwaysRunMode `json:"should_always_run,omitempty" yaml:"should_always_run,omitempty"` + Uses string `json:"uses,omitempty" yaml:"uses,omitempty"` } type GraphPipelineRunIfModel struct { @@ -289,14 +290,14 @@ func (s StepRunResultsModel) error() []StepError { } func formatStatusReasonTimeInterval(timeInterval time.Duration) string { - var remaining = int(timeInterval / time.Second) + remaining := int(timeInterval / time.Second) h := int(remaining / 3600) remaining = remaining - h*3600 m := int(remaining / 60) remaining = remaining - m*60 s := remaining - var formattedTimeInterval = "" + formattedTimeInterval := "" if h > 0 { formattedTimeInterval += fmt.Sprintf("%dh ", h) } diff --git a/models/models_methods.go b/models/models_methods.go index e530a7a9d..c56df3400 100644 --- a/models/models_methods.go +++ b/models/models_methods.go @@ -365,7 +365,6 @@ func (with *WithModel) Validate(workflowID string, containers, services map[stri } return warnings, nil - } func (workflow *WorkflowModel) Validate() error { @@ -605,8 +604,19 @@ func validateDAGPipeline(pipelineID string, pipeline *PipelineModel, config *Bit return fmt.Errorf("workflow (%s) defined in pipeline (%s) is a utility workflow", pipelineWorkflowID, pipelineID) } - if _, ok := config.Workflows[pipelineWorkflowID]; !ok { - return fmt.Errorf("workflow (%s) defined in pipeline (%s) is not found in the workflow definitions", pipelineWorkflowID, pipelineID) + isWorkflowVariant := pipelineWorkflow.Uses != "" + if isWorkflowVariant { + if _, ok := config.Workflows[pipelineWorkflow.Uses]; !ok { + return fmt.Errorf("workflow (%s) referenced in pipeline (%s) in workflow variant (%s) is not found in the workflow definitions", pipelineWorkflow.Uses, pipelineID, pipelineWorkflowID) + } + + if _, ok := config.Workflows[pipelineWorkflowID]; ok { + return fmt.Errorf("workflow (%s) defined in pipeline (%s) is a variant of another workflow, but it is also defined as a workflow", pipelineWorkflowID, pipelineID) + } + } else { + if _, ok := config.Workflows[pipelineWorkflowID]; !ok { + return fmt.Errorf("workflow (%s) defined in pipeline (%s) is not found in the workflow definitions", pipelineWorkflowID, pipelineID) + } } uniqueItems := make(map[string]bool) @@ -895,7 +905,7 @@ func removeEnvironmentRedundantFields(env *envmanModels.EnvironmentItemModel) er hasOptions = true } } - if options.ValueOptions != nil && len(options.ValueOptions) > 0 { + if len(options.ValueOptions) > 0 { hasOptions = true } if options.IsRequired != nil { @@ -919,7 +929,7 @@ func removeEnvironmentRedundantFields(env *envmanModels.EnvironmentItemModel) er hasOptions = true } } - if options.Meta != nil && len(options.Meta) > 0 { + if len(options.Meta) > 0 { hasOptions = true } @@ -988,7 +998,7 @@ func MergeEnvironmentWith(env *envmanModels.EnvironmentItemModel, otherEnv envma (*env)[key] = otherValue - //merge options + // merge options options, err := env.GetOptions() if err != nil { return err