diff --git a/cmd/checkIfStepActive.go b/cmd/checkIfStepActive.go index 840c2dffe..473984fa1 100644 --- a/cmd/checkIfStepActive.go +++ b/cmd/checkIfStepActive.go @@ -85,7 +85,7 @@ func checkIfStepActive(utils piperutils.FileUtils) error { if checkStepActiveOptions.v1Active { runConfig := config.RunConfig{StageConfigFile: stageConfigFile} runConfigV1 := &config.RunConfigV1{RunConfig: runConfig} - err = runConfigV1.InitRunConfigV1(projectConfig, nil, nil, nil, nil, utils, GeneralConfig.EnvRootPath) + err = runConfigV1.InitRunConfigV1(projectConfig, utils, GeneralConfig.EnvRootPath) if err != nil { return err } diff --git a/pkg/config/evaluation.go b/pkg/config/evaluation.go index b8e01cd64..150d74e1a 100644 --- a/pkg/config/evaluation.go +++ b/pkg/config/evaluation.go @@ -21,89 +21,110 @@ const ( npmScriptsCondition = "npmScripts" ) -// EvaluateConditionsV1 validates stage conditions and updates runSteps in runConfig according to V1 schema -func (r *RunConfigV1) evaluateConditionsV1(config *Config, filters map[string]StepFilters, parameters map[string][]StepParameters, - secrets map[string][]StepSecrets, stepAliases map[string][]Alias, utils piperutils.FileUtils, envRootPath string) error { - - // initialize in case not initialized - if r.RunConfig.RunSteps == nil { - r.RunConfig.RunSteps = map[string]map[string]bool{} +// evaluateConditionsV1 validates stage conditions and updates runSteps in runConfig according to V1 schema. +// Priority of step activation/deactivation is follow: +// - stepNotActiveCondition (highest, if any) +// - explicit activation/deactivation (medium, if any) +// - stepActiveConditions (lowest, step is active by default if no conditions are configured) +func (r *RunConfigV1) evaluateConditionsV1(config *Config, utils piperutils.FileUtils, envRootPath string) error { + if r.RunSteps == nil { + r.RunSteps = make(map[string]map[string]bool, len(r.PipelineConfig.Spec.Stages)) } - if r.RunConfig.RunStages == nil { - r.RunConfig.RunStages = map[string]bool{} + if r.RunStages == nil { + r.RunStages = make(map[string]bool, len(r.PipelineConfig.Spec.Stages)) } + currentOrchestrator := orchestrator.DetectOrchestrator().String() for _, stage := range r.PipelineConfig.Spec.Stages { - runStep := map[string]bool{} - stageActive := false - - // currently displayName is used, may need to consider to use technical name as well + // Currently, the displayName is being used, but it may be necessary + // to also consider using the technical name. stageName := stage.DisplayName + // Check #1: Apply explicit activation/deactivation from config file (if any) + // and then evaluate stepActive conditions + runStep := make(map[string]bool, len(stage.Steps)) + stepConfigCache := make(map[string]StepConfig, len(stage.Steps)) for _, step := range stage.Steps { - // Only consider orchestrator-specific steps in case orchestrator limitation is set - currentOrchestrator := orchestrator.DetectOrchestrator().String() + // Consider only orchestrator-specific steps if the orchestrator limitation is set. if len(step.Orchestrators) > 0 && !piperutils.ContainsString(step.Orchestrators, currentOrchestrator) { continue } - stepActive := false - stepNotActive := false - - stepConfig, err := r.getStepConfig(config, stageName, step.Name, filters, parameters, secrets, stepAliases) + stepConfig, err := r.getStepConfig(config, stageName, step.Name, nil, nil, nil, nil) if err != nil { return err } + stepConfigCache[step.Name] = stepConfig + // Respect explicit activation/deactivation if available. + // Note that this has higher priority than step conditions if active, ok := stepConfig.Config[step.Name].(bool); ok { - // respect explicit activation/de-activation if available - stepActive = active - } else { - if step.Conditions == nil || len(step.Conditions) == 0 { - // if no condition is available, step will be active by default - stepActive = true - } else { - for _, condition := range step.Conditions { - stepActive, err = condition.evaluateV1(stepConfig, utils, step.Name, envRootPath) - if err != nil { - return fmt.Errorf("failed to evaluate stage conditions: %w", err) - } - if stepActive { - // first condition which matches will be considered to activate the step - break - } - } - } + runStep[step.Name] = active + continue } - // TODO: PART 1 : if explicit activation/de-activation is available should notActiveConditions be checked ? - // Fortify has no anchor, so if we explicitly set it to true then it may run even during commit pipelines, if we implement TODO PART 1?? - for _, condition := range step.NotActiveConditions { - stepNotActive, err = condition.evaluateV1(stepConfig, utils, step.Name, envRootPath) + // If no condition is available, the step will be active by default. + stepActive := true + for _, condition := range step.Conditions { + stepActive, err = condition.evaluateV1(stepConfig, utils, step.Name, envRootPath, runStep) if err != nil { - return fmt.Errorf("failed to evaluate not active stage conditions: %w", err) + return fmt.Errorf("failed to evaluate step conditions: %w", err) } - if stepNotActive { - // first condition which matches will be considered to not activate the step + if stepActive { + // The first condition that matches will be considered to activate the step. break } } - // final decision is when step is activated and negate when not active is true - stepActive = stepActive && !stepNotActive + runStep[step.Name] = stepActive + } - if stepActive { + // Check #2: Evaluate stepNotActive conditions (if any) and deactivate the step if the condition is met. + // + // TODO: PART 1 : if explicit activation/de-activation is available should notActiveConditions be checked ? + // Fortify has no anchor, so if we explicitly set it to true then it may run even during commit pipelines, if we implement TODO PART 1?? + for _, step := range stage.Steps { + stepConfig, found := stepConfigCache[step.Name] + if !found { + // If no stepConfig exists here, it means that this step was skipped in previous checks. + continue + } + + for _, condition := range step.NotActiveConditions { + stepNotActive, err := condition.evaluateV1(stepConfig, utils, step.Name, envRootPath, runStep) + if err != nil { + return fmt.Errorf("failed to evaluate not active step conditions: %w", err) + } + + // Deactivate the step if the notActive condition is met. + if stepNotActive { + runStep[step.Name] = false + break + } + } + } + + r.RunSteps[stageName] = runStep + + stageActive := false + for _, anyStepIsActive := range r.RunSteps[stageName] { + if anyStepIsActive { stageActive = true } - runStep[step.Name] = stepActive - r.RunSteps[stageName] = runStep } r.RunStages[stageName] = stageActive } + return nil } -func (s *StepCondition) evaluateV1(config StepConfig, utils piperutils.FileUtils, stepName string, envRootPath string) (bool, error) { +func (s *StepCondition) evaluateV1( + config StepConfig, + utils piperutils.FileUtils, + stepName string, + envRootPath string, + runSteps map[string]bool, +) (bool, error) { // only the first condition will be evaluated. // if multiple conditions should be checked they need to provided via the Conditions list @@ -189,6 +210,14 @@ func (s *StepCondition) evaluateV1(config StepConfig, utils piperutils.FileUtils return false, nil } + if s.OnlyActiveStepInStage { + // Used only in NotActiveConditions. + // Returns true if all other steps are inactive, so step will be deactivated + // if it's the only active step in stage. + // For example, sapCumulusUpload step must be deactivated in a stage where others steps are inactive. + return !anyOtherStepIsActive(stepName, runSteps), nil + } + // needs to be checked last: // if none of the other conditions matches, step will be active unless set to inactive if s.Inactive == true { @@ -491,3 +520,15 @@ func checkForNpmScriptsInPackagesV1(npmScript string, config StepConfig, utils p } return false, nil } + +// anyOtherStepIsActive loops through previous steps active states and returns true +// if at least one of them is active, otherwise result is false. Ignores the step that is being checked. +func anyOtherStepIsActive(targetStep string, runSteps map[string]bool) bool { + for step, isActive := range runSteps { + if isActive && step != targetStep { + return true + } + } + + return false +} diff --git a/pkg/config/evaluation_test.go b/pkg/config/evaluation_test.go index ae42f07a0..91468d0b8 100644 --- a/pkg/config/evaluation_test.go +++ b/pkg/config/evaluation_test.go @@ -49,200 +49,191 @@ func evaluateConditionsOpenFileMock(name string, _ map[string]string) (io.ReadCl return fileContent, nil } -func TestEvaluateConditionsV1(t *testing.T) { - filesMock := mock.FilesMock{} - - runConfig := RunConfigV1{ - PipelineConfig: PipelineDefinitionV1{ - Spec: Spec{ - Stages: []Stage{ - { - Name: "stage1", - DisplayName: "Test Stage 1", - Steps: []Step{ - { - Name: "step1_1", - Conditions: []StepCondition{}, - Orchestrators: []string{"Jenkins"}, - }, - { - Name: "step1_2", - Conditions: []StepCondition{ - {ConfigKey: "testKey"}, - }, - }, - { - Name: "step1_3", - Conditions: []StepCondition{}, - }, - { - Name: "step1_4", - Conditions: []StepCondition{ - {ConfigKey: "firstKey/nextKey"}, - }, - }, - }, - }, - { - Name: "stage2", - DisplayName: "Test Stage 2", - Steps: []Step{ - { - Name: "step2_1", - Conditions: []StepCondition{ - {ConfigKey: "testKeyNotExisting"}, - {ConfigKey: "testKey"}, - }, - }, - { - Name: "step2_2", - }, - }, - }, - { - Name: "stage3", - DisplayName: "Test Stage 3", - Steps: []Step{ - { - Name: "step3_1", - Conditions: []StepCondition{ - {ConfigKey: "testKeyNotExisting"}, - {ConfigKey: "testKey"}, - }, - }, - }, - }, - }, - }, - }, - } +func TestRunConfigV1EvaluateConditionsV1(t *testing.T) { config := Config{Stages: map[string]map[string]interface{}{ - "Test Stage 1": {"step1_3": false, "testKey": "testVal", "firstKey": map[string]interface{}{"nextKey": "dummy"}}, - "Test Stage 2": {"testKey": "testVal"}, - }} - - expectedSteps := map[string]map[string]bool{ "Test Stage 1": { - "step1_2": true, - "step1_3": false, - "step1_4": true, + "step1": true, // explicit activate + "step5": true, // explicit activate + "step2": false, // explicit deactivate + "testKey": "testVal", // some condition 1 + "testKey2": "testVal2", // some condition 2 }, - "Test Stage 2": { - "step2_1": true, - "step2_2": true, - }, - "Test Stage 3": { - "step3_1": false, - }, - } - - expectedStages := map[string]bool{ - "Test Stage 1": true, - "Test Stage 2": true, - "Test Stage 3": false, - } - - err := runConfig.evaluateConditionsV1(&config, nil, nil, nil, nil, &filesMock, ".pipeline") - assert.NoError(t, err) - assert.Equal(t, expectedSteps, runConfig.RunSteps) - assert.Equal(t, expectedStages, runConfig.RunStages) - -} - -func TestNotActiveEvaluateConditionsV1(t *testing.T) { + }} filesMock := mock.FilesMock{} + envRootPath := ".pipeline" - runConfig := RunConfigV1{ - PipelineConfig: PipelineDefinitionV1{ - Spec: Spec{ - Stages: []Stage{ - { - Name: "stage1", - DisplayName: "Test Stage 1", - Steps: []Step{ - { - Name: "step1_1", - Conditions: []StepCondition{}, - Orchestrators: []string{"Jenkins"}, - }, - { - Name: "step1_2", - Conditions: []StepCondition{ - {ConfigKey: "testKey"}, - }, - NotActiveConditions: []StepCondition{ - {ConfigKey: "testKeyNotExisting"}, - }, - }, - { - Name: "step1_3", - Conditions: []StepCondition{}, - NotActiveConditions: []StepCondition{ - {ConfigKey: "testKeyNotExisting"}, - {ConfigKey: "testKey"}, - }, - }, - }, - }, - { - Name: "stage2", - DisplayName: "Test Stage 2", - Steps: []Step{ - { - Name: "step2_1", - Conditions: []StepCondition{ - {ConfigKey: "testKeyNotExisting"}, - {ConfigKey: "testKey"}, - }, - }, - }, - }, - { - Name: "stage3", - DisplayName: "Test Stage 3", - Steps: []Step{ - { - Name: "step3_1", - NotActiveConditions: []StepCondition{ - {ConfigKey: "testKey"}, - }, - }, - }, - }, - }, + tests := []struct { + name string + pipelineConfig PipelineDefinitionV1 + wantRunSteps map[string]map[string]bool + wantRunStages map[string]bool + }{ + { + name: "all steps in stage are inactive", + pipelineConfig: PipelineDefinitionV1{Spec: Spec{Stages: []Stage{{DisplayName: "Test Stage 1", + Steps: []Step{{ + Name: "step1", + NotActiveConditions: []StepCondition{{ConfigKey: "testKey"}}, + }, { + Name: "step2", + }, { + Name: "step3", + NotActiveConditions: []StepCondition{{ConfigKey: "testKey"}}, + }}, }, + }}}, + wantRunSteps: map[string]map[string]bool{ + "Test Stage 1": { + "step1": false, + "step2": false, + "step3": false, + }}, + wantRunStages: map[string]bool{"Test Stage 1": false}, + }, + { + name: "simple stepActive conditions", + pipelineConfig: PipelineDefinitionV1{Spec: Spec{Stages: []Stage{{DisplayName: "Test Stage 1", + Steps: []Step{{ + Name: "step3", + Conditions: []StepCondition{{ConfigKey: "testKey"}}, + }, { + Name: "step4", + Conditions: []StepCondition{{ConfigKey: "notExistentKey"}}, + }}, + }, + }}}, + wantRunSteps: map[string]map[string]bool{ + "Test Stage 1": { + "step3": true, + "step4": false, + }}, + wantRunStages: map[string]bool{"Test Stage 1": true}, + }, + { + name: "explicit active/deactivate over stepActiveCondition", + pipelineConfig: PipelineDefinitionV1{Spec: Spec{Stages: []Stage{{DisplayName: "Test Stage 1", + Steps: []Step{{ + Name: "step1", + Conditions: []StepCondition{{ConfigKey: "notExistentKey"}}, + }, { + Name: "step2", + Conditions: []StepCondition{{ConfigKey: "testKey"}}, + }}, + }, + }}}, + wantRunSteps: map[string]map[string]bool{ + "Test Stage 1": { + "step1": true, + "step2": false, + }}, + wantRunStages: map[string]bool{"Test Stage 1": true}, + }, + { + name: "stepNotActiveCondition over stepActiveCondition", + pipelineConfig: PipelineDefinitionV1{Spec: Spec{Stages: []Stage{{DisplayName: "Test Stage 1", + Steps: []Step{{ + Name: "step3", + Conditions: []StepCondition{{ConfigKey: "testKey"}}, + NotActiveConditions: []StepCondition{{ConfigKey: "testKey2"}}, + }, { + // false notActive condition + Name: "step4", + Conditions: []StepCondition{{ConfigKey: "testKey"}}, + NotActiveConditions: []StepCondition{{ConfigKey: "notExistentKey"}}, + }}, + }, + }}}, + wantRunSteps: map[string]map[string]bool{ + "Test Stage 1": { + "step3": false, + "step4": true, + }}, + wantRunStages: map[string]bool{"Test Stage 1": true}, + }, + { + name: "stepNotActiveCondition over explicitly activated step", + pipelineConfig: PipelineDefinitionV1{Spec: Spec{Stages: []Stage{{DisplayName: "Test Stage 1", + Steps: []Step{{ + Name: "step1", + NotActiveConditions: []StepCondition{{ConfigKey: "testKey"}}, + }, { + Name: "step5", + NotActiveConditions: []StepCondition{{ConfigKey: "notExistentKey"}}, + }}, + }, + }}}, + wantRunSteps: map[string]map[string]bool{ + "Test Stage 1": { + "step1": false, + "step5": true, + }}, + wantRunStages: map[string]bool{"Test Stage 1": true}, + }, + { + name: "deactivate if only active step in stage", + pipelineConfig: PipelineDefinitionV1{Spec: Spec{Stages: []Stage{{DisplayName: "Test Stage 1", + Steps: []Step{{ + Name: "step1", + NotActiveConditions: []StepCondition{{ConfigKey: "testKey"}}, + }, { + Name: "step2", + }, { + Name: "step3", + NotActiveConditions: []StepCondition{{OnlyActiveStepInStage: true}}, + }, { + Name: "step4", + Conditions: []StepCondition{{ConfigKey: "keyNotExist"}}, + }}, + }, + }}}, + wantRunSteps: map[string]map[string]bool{ + "Test Stage 1": { + "step1": false, + "step2": false, + "step3": false, + "step4": false, + }}, + wantRunStages: map[string]bool{"Test Stage 1": false}, + }, + { + name: "OnlyActiveStepInStage: one of the next steps is active", + pipelineConfig: PipelineDefinitionV1{Spec: Spec{Stages: []Stage{{DisplayName: "Test Stage 1", + Steps: []Step{{ + Name: "step1", + NotActiveConditions: []StepCondition{{ConfigKey: "testKey"}}, + }, { + Name: "step2", + }, { + Name: "step3", + Conditions: []StepCondition{{ConfigKey: "testKey"}}, + NotActiveConditions: []StepCondition{{OnlyActiveStepInStage: true}}, + }, { + Name: "step4", + Conditions: []StepCondition{{ConfigKey: "testKey2"}}, + }}, + }, + }}}, + wantRunSteps: map[string]map[string]bool{ + "Test Stage 1": { + "step1": false, + "step2": false, + "step3": true, + "step4": true, + }}, + wantRunStages: map[string]bool{"Test Stage 1": true}, }, } - config := Config{Stages: map[string]map[string]interface{}{ - "Test Stage 1": {"testKey": "testVal"}, - "Test Stage 2": {"testKey": "testVal"}, - "Test Stage 3": {"testKey": "testVal"}, - }} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + r := &RunConfigV1{PipelineConfig: tt.pipelineConfig} + assert.NoError(t, r.evaluateConditionsV1(&config, &filesMock, envRootPath), + fmt.Sprintf("evaluateConditionsV1() err, pipelineConfig = %v", tt.pipelineConfig), + ) - expectedSteps := map[string]map[string]bool{ - "Test Stage 1": { - "step1_2": true, - "step1_3": false, - }, - "Test Stage 2": { - "step2_1": true, - }, - "Test Stage 3": { - "step3_1": false, - }, + assert.Equal(t, tt.wantRunSteps, r.RunSteps, "RunSteps mismatch") + assert.Equal(t, tt.wantRunStages, r.RunStages, "RunStages mismatch") + }) } - - expectedStages := map[string]bool{ - "Test Stage 1": true, - "Test Stage 2": true, - "Test Stage 3": false, - } - - err := runConfig.evaluateConditionsV1(&config, nil, nil, nil, nil, &filesMock, ".pipeline") - assert.NoError(t, err) - assert.Equal(t, expectedSteps, runConfig.RunSteps) - assert.Equal(t, expectedStages, runConfig.RunStages) - } func TestEvaluateV1(t *testing.T) { @@ -250,6 +241,7 @@ func TestEvaluateV1(t *testing.T) { name string config StepConfig stepCondition StepCondition + runSteps map[string]bool expected bool expectedError error }{ @@ -399,6 +391,20 @@ func TestEvaluateV1(t *testing.T) { stepCondition: StepCondition{PipelineEnvironmentFilled: "custom/notMyCpeTrueFile"}, expected: false, }, + { + name: "NotActiveCondition: all previous steps are inactive", + config: StepConfig{Config: map[string]interface{}{}}, + stepCondition: StepCondition{OnlyActiveStepInStage: true}, + runSteps: map[string]bool{"step1": false, "step2": false}, + expected: true, + }, + { + name: "NotActiveCondition: one of the previous steps is active", + config: StepConfig{Config: map[string]interface{}{}}, + stepCondition: StepCondition{OnlyActiveStepInStage: true}, + runSteps: map[string]bool{"step1": false, "step2": false, "step3": true}, + expected: false, + }, { name: "No condition - true", config: StepConfig{Config: map[string]interface{}{}}, @@ -429,7 +435,7 @@ func TestEvaluateV1(t *testing.T) { for _, test := range tt { t.Run(test.name, func(t *testing.T) { - active, err := test.stepCondition.evaluateV1(test.config, &filesMock, "dummy", dir) + active, err := test.stepCondition.evaluateV1(test.config, &filesMock, "dummy", dir, test.runSteps) if test.expectedError == nil { assert.NoError(t, err) } else { @@ -505,7 +511,7 @@ stages: testStage1: stepConditions: firstStep: - config: + config: - testGeneral `)), runStepsExpected: map[string]map[string]bool{}, @@ -711,7 +717,7 @@ stages: - '**/conf.js' - 'myCollection.json' secondStep: - filePattern: + filePattern: - '**/conf.jsx' `)), runStepsExpected: map[string]map[string]bool{ @@ -945,3 +951,59 @@ stages: }) } } + +func TestAnyOtherStepIsActive(t *testing.T) { + targetStep := "step3" + + tests := []struct { + name string + runSteps map[string]bool + want bool + }{ + { + name: "all steps are inactive (target active)", + runSteps: map[string]bool{ + "step1": false, + "step2": false, + "step3": true, + "step4": false, + }, + want: false, + }, + { + name: "all steps are inactive (target inactive)", + runSteps: map[string]bool{ + "step1": false, + "step2": false, + "step3": false, + "step4": false, + }, + want: false, + }, + { + name: "some previous step is active", + runSteps: map[string]bool{ + "step1": false, + "step2": true, + "step3": false, + "step4": false, + }, + want: true, + }, + { + name: "some next step is active", + runSteps: map[string]bool{ + "step1": false, + "step2": false, + "step3": true, + "step4": true, + }, + want: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equalf(t, tt.want, anyOtherStepIsActive(targetStep, tt.runSteps), "anyOtherStepIsActive(%v, %v)", targetStep, tt.runSteps) + }) + } +} diff --git a/pkg/config/run.go b/pkg/config/run.go index 6eb197684..8800332a3 100644 --- a/pkg/config/run.go +++ b/pkg/config/run.go @@ -73,13 +73,13 @@ type StepCondition struct { FilePattern string `json:"filePattern,omitempty"` FilePatternFromConfig string `json:"filePatternFromConfig,omitempty"` Inactive bool `json:"inactive,omitempty"` + OnlyActiveStepInStage bool `json:"onlyActiveStepInStage,omitempty"` NpmScript string `json:"npmScript,omitempty"` CommonPipelineEnvironment map[string]interface{} `json:"commonPipelineEnvironment,omitempty"` PipelineEnvironmentFilled string `json:"pipelineEnvironmentFilled,omitempty"` } -func (r *RunConfigV1) InitRunConfigV1(config *Config, filters map[string]StepFilters, parameters map[string][]StepParameters, - secrets map[string][]StepSecrets, stepAliases map[string][]Alias, utils piperutils.FileUtils, envRootPath string) error { +func (r *RunConfigV1) InitRunConfigV1(config *Config, utils piperutils.FileUtils, envRootPath string) error { if len(r.PipelineConfig.Spec.Stages) == 0 { if err := r.LoadConditionsV1(); err != nil { @@ -87,7 +87,7 @@ func (r *RunConfigV1) InitRunConfigV1(config *Config, filters map[string]StepFil } } - err := r.evaluateConditionsV1(config, filters, parameters, secrets, stepAliases, utils, envRootPath) + err := r.evaluateConditionsV1(config, utils, envRootPath) if err != nil { return fmt.Errorf("failed to evaluate step conditions: %w", err) } diff --git a/pkg/config/run_test.go b/pkg/config/run_test.go index 98bbedd59..f7759d6e5 100644 --- a/pkg/config/run_test.go +++ b/pkg/config/run_test.go @@ -63,7 +63,7 @@ func TestInitRunConfigV1(t *testing.T) { stageConfig := ioutil.NopCloser(strings.NewReader(test.stageConfig)) runConfig := RunConfig{StageConfigFile: stageConfig} runConfigV1 := RunConfigV1{RunConfig: runConfig} - err := runConfigV1.InitRunConfigV1(&test.config, nil, nil, nil, nil, &filesMock, ".pipeline") + err := runConfigV1.InitRunConfigV1(&test.config, &filesMock, ".pipeline") if len(test.errorContains) > 0 { assert.Contains(t, fmt.Sprint(err), test.errorContains) } else {