Skip to content

Commit

Permalink
Understand workflow variants when validating the pipeline [CI-4035] (#…
Browse files Browse the repository at this point in the history
…1029)

* Understand workflow variants when validating the pipeline

* Add more validation

* Rename source to uses

* Fix linting issues
  • Loading branch information
gaborszakacs authored Dec 16, 2024
1 parent 91e698e commit 30a149a
Show file tree
Hide file tree
Showing 6 changed files with 68 additions and 21 deletions.
2 changes: 1 addition & 1 deletion cli/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
19 changes: 10 additions & 9 deletions cli/docker/container_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
}
}
Expand Down Expand Up @@ -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)
}

Expand All @@ -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),
}
Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
35 changes: 35 additions & 0 deletions cli/run_util_pipeline_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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] }
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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,
Expand Down
6 changes: 3 additions & 3 deletions log/log.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand All @@ -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, " → ")))
}
}

Expand Down
5 changes: 3 additions & 2 deletions models/models.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
}
Expand Down
22 changes: 16 additions & 6 deletions models/models_methods.go
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,6 @@ func (with *WithModel) Validate(workflowID string, containers, services map[stri
}

return warnings, nil

}

func (workflow *WorkflowModel) Validate() error {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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 {
Expand All @@ -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
}

Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 30a149a

Please sign in to comment.