diff --git a/pkg/pipeline/PipelineStagePipelineConditions_test.go b/pkg/pipeline/PipelineStagePipelineConditions_test.go new file mode 100644 index 0000000000..396175f225 --- /dev/null +++ b/pkg/pipeline/PipelineStagePipelineConditions_test.go @@ -0,0 +1,75 @@ +/* + * Copyright (c) 2024. Devtron Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package pipeline + +import ( + "testing" + + "github.com/devtron-labs/devtron/pkg/pipeline/repository" + "github.com/stretchr/testify/assert" + "go.uber.org/zap" +) + +type fakePipelineStageRepoForWfRequest struct { + repository.PipelineStageRepository + variables []*repository.PipelineStageStepVariable + conditions []*repository.PipelineStageStepCondition +} + +func (f *fakePipelineStageRepoForWfRequest) GetVariablesByStepId(stepId int) ([]*repository.PipelineStageStepVariable, error) { + return f.variables, nil +} + +func (f *fakePipelineStageRepoForWfRequest) GetConditionsByStepId(stepId int) ([]*repository.PipelineStageStepCondition, error) { + return f.conditions, nil +} + +// Regression guard for the plugin-condition change: the conditions saved on the pipeline step +// (pipeline_stage_step_condition) are the ones that must keep reaching ci-runner. +func TestBuildVariableAndConditionDataForWfRequest_PipelineLevelConditionsAreSent(t *testing.T) { + repo := &fakePipelineStageRepoForWfRequest{ + variables: []*repository.PipelineStageStepVariable{ + {Id: 198, PipelineStageStepId: 225, Name: "FILE_PATH", Format: repository.PIPELINE_STAGE_STEP_VARIABLE_FORMAT_TYPE_STRING, VariableType: repository.PIPELINE_STAGE_STEP_VARIABLE_TYPE_INPUT, ValueType: repository.PIPELINE_STAGE_STEP_VARIABLE_VALUE_TYPE_NEW, Value: "/tmp/other", AllowEmptyValue: true, IsExposed: true, VariableStepIndexInPlugin: 1}, + {Id: 199, PipelineStageStepId: 225, Name: "CONTENT", Format: repository.PIPELINE_STAGE_STEP_VARIABLE_FORMAT_TYPE_STRING, VariableType: repository.PIPELINE_STAGE_STEP_VARIABLE_TYPE_OUTPUT, ValueType: repository.PIPELINE_STAGE_STEP_VARIABLE_VALUE_TYPE_NEW, IsExposed: true, VariableStepIndexInPlugin: 1}, + }, + conditions: []*repository.PipelineStageStepCondition{ + {Id: 15, PipelineStageStepId: 225, ConditionVariableId: 198, ConditionType: repository.PIPELINE_STAGE_STEP_CONDITION_TYPE_TRIGGER, ConditionalOperator: "==", ConditionalValue: "/tmp/other"}, + {Id: 16, PipelineStageStepId: 225, ConditionVariableId: 199, ConditionType: repository.PIPELINE_STAGE_STEP_CONDITION_TYPE_SUCCESS, ConditionalOperator: "==", ConditionalValue: "ok"}, + {Id: 17, PipelineStageStepId: 225, ConditionVariableId: 0, ConditionType: repository.PIPELINE_STAGE_STEP_CONDITION_TYPE_SKIP, ConditionalOperator: "!=", ConditionalValue: "x"}, + }, + } + impl := &PipelineStageServiceImpl{logger: zap.NewNop().Sugar(), pipelineStageRepository: repo} + data, err := impl.buildVariableAndConditionDataForWfRequest(225) + assert.NoError(t, err) + if assert.Len(t, data.GetInputVariables(), 1) { + assert.Equal(t, "/tmp/other", data.GetInputVariables()[0].Value) + assert.Equal(t, 1, data.GetInputVariables()[0].VariableStepIndexInPlugin) + } + assert.Len(t, data.GetOutputVariables(), 1) + triggerSkip := data.GetTriggerSkipConditions() + if assert.Len(t, triggerSkip, 2) { + assert.Equal(t, "FILE_PATH", triggerSkip[0].ConditionOnVariable) + assert.Equal(t, "TRIGGER", triggerSkip[0].ConditionType) + assert.Equal(t, "==", triggerSkip[0].ConditionalOperator) + assert.Equal(t, "/tmp/other", triggerSkip[0].ConditionalValue) + assert.Equal(t, "", triggerSkip[1].ConditionOnVariable, "condition on an unknown variable id is sent without a variable name (pre-existing behaviour)") + } + if assert.Len(t, data.GetSuccessFailureConditions(), 1) { + assert.Equal(t, "CONTENT", data.GetSuccessFailureConditions()[0].ConditionOnVariable) + assert.Equal(t, "PASS", data.GetSuccessFailureConditions()[0].ConditionType) + } +} diff --git a/pkg/pipeline/PipelineStagePluginConditions_test.go b/pkg/pipeline/PipelineStagePluginConditions_test.go new file mode 100644 index 0000000000..5169fd2c51 --- /dev/null +++ b/pkg/pipeline/PipelineStagePluginConditions_test.go @@ -0,0 +1,230 @@ +/* + * Copyright (c) 2024. Devtron Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package pipeline + +import ( + "errors" + "testing" + + commonBean "github.com/devtron-labs/common-lib/workflow" + repository2 "github.com/devtron-labs/devtron/pkg/plugin/repository" + "github.com/go-pg/pg" + "github.com/stretchr/testify/assert" + "go.uber.org/zap" +) + +// fakePluginRepoForWfRequest serves a plugin definition (steps, script, variables, conditions) to the workflow request +// builders. Any repository method not overridden here panics if called, which is intended. +type fakePluginRepoForWfRequest struct { + repository2.GlobalPluginRepository + steps []*repository2.PluginStep + scripts map[int]*repository2.PluginPipelineScript + variables map[int][]*repository2.PluginStepVariable + variablesErr error + conditions map[int][]*repository2.PluginStepCondition + conditionCalls int +} + +func (f *fakePluginRepoForWfRequest) GetStepsByPluginIds(pluginIds []int) ([]*repository2.PluginStep, error) { + return f.steps, nil +} + +func (f *fakePluginRepoForWfRequest) GetScriptDetailById(id int) (*repository2.PluginPipelineScript, error) { + if script, ok := f.scripts[id]; ok { + return script, nil + } + return nil, pg.ErrNoRows +} + +func (f *fakePluginRepoForWfRequest) GetScriptMappingDetailByScriptId(scriptId int) ([]*repository2.ScriptPathArgPortMapping, error) { + return nil, nil +} + +func (f *fakePluginRepoForWfRequest) GetVariablesByStepId(stepId int) ([]*repository2.PluginStepVariable, error) { + if f.variablesErr != nil { + return nil, f.variablesErr + } + return f.variables[stepId], nil +} + +func (f *fakePluginRepoForWfRequest) GetConditionsByStepId(stepId int) ([]*repository2.PluginStepCondition, error) { + f.conditionCalls++ + return f.conditions[stepId], nil +} + +func newPluginConditionsTestService(repo *fakePluginRepoForWfRequest) *PipelineStageServiceImpl { + return &PipelineStageServiceImpl{logger: zap.NewNop().Sugar(), globalPluginRepository: repo} +} + +func pluginStepVariable(name string, variableType repository2.PluginStepVariableType, exposed bool, value, defaultValue string) *repository2.PluginStepVariable { + return &repository2.PluginStepVariable{ + Id: 1, + Name: name, + Format: repository2.PLUGIN_VARIABLE_FORMAT_TYPE_STRING, + VariableType: variableType, + ValueType: repository2.PLUGIN_VARIABLE_VALUE_TYPE_NEW, + IsExposed: exposed, + Value: value, + DefaultValue: defaultValue, + VariableStepIndex: 1, + } +} + +// pluginWithConditions models a custom plugin saved from the dashboard: one shell step, an exposed input variable +// FILE_PATH with a TRIGGER condition on it and an exposed output variable CONTENT with a PASS condition on it. +func pluginWithConditions() *fakePluginRepoForWfRequest { + input := pluginStepVariable("FILE_PATH", repository2.PLUGIN_VARIABLE_TYPE_INPUT, true, "/tmp/shivam", "") + input.Id = 386 + output := pluginStepVariable("CONTENT", repository2.PLUGIN_VARIABLE_TYPE_OUTPUT, true, "", "") + output.Id = 387 + return &fakePluginRepoForWfRequest{ + steps: []*repository2.PluginStep{ + {Id: 69, PluginId: 71, Name: "Task 1", Index: 1, StepType: repository2.PLUGIN_STEP_TYPE_INLINE, ScriptId: 5}, + }, + scripts: map[int]*repository2.PluginPipelineScript{5: {Type: repository2.SCRIPT_TYPE_SHELL, Script: "echo hi > $FILE_PATH"}}, + variables: map[int][]*repository2.PluginStepVariable{69: {input, output}}, + conditions: map[int][]*repository2.PluginStepCondition{69: { + {Id: 23, PluginStepId: 69, ConditionVariableId: 386, ConditionType: repository2.PLUGIN_CONDITION_TYPE_TRIGGER, ConditionalOperator: "==", ConditionalValue: "/tmp/shivam"}, + {Id: 24, PluginStepId: 69, ConditionVariableId: 387, ConditionType: repository2.PLUGIN_CONDITION_TYPE_SUCCESS, ConditionalOperator: "==", ConditionalValue: "ok"}, + }}, + } +} + +func TestBuildPluginStepDataForWfRequest_PluginConditionsAreNotSentToCiRunner(t *testing.T) { + t.Run("inline plugin step: variables and script are sent, plugin-defined conditions are not", func(t *testing.T) { + repo := pluginWithConditions() + stepData, err := newPluginConditionsTestService(repo).BuildPluginStepDataForWfRequest(repo.steps[0]) + assert.NoError(t, err) + assert.Equal(t, "Task 1", stepData.Name) + assert.Equal(t, 1, stepData.Index) + assert.Equal(t, string(repository2.PLUGIN_STEP_TYPE_INLINE), stepData.StepType) + assert.Equal(t, string(repository2.SCRIPT_TYPE_SHELL), stepData.ExecutorType) + assert.Equal(t, "echo hi > $FILE_PATH", stepData.Script) + if assert.Len(t, stepData.InputVars, 1) { + assert.Equal(t, "FILE_PATH", stepData.InputVars[0].Name) + assert.Equal(t, "/tmp/shivam", stepData.InputVars[0].Value) + } + if assert.Len(t, stepData.OutputVars, 1) { + assert.Equal(t, "CONTENT", stepData.OutputVars[0].Name) + } + assert.Nil(t, stepData.TriggerSkipConditions, "plugin-level trigger/skip conditions must not reach ci-runner") + assert.Nil(t, stepData.SuccessFailureConditions, "plugin-level pass/fail conditions must not reach ci-runner") + assert.Equal(t, 0, repo.conditionCalls, "plugin_step_condition must not even be read for the workflow request") + }) + t.Run("plugin step referencing another plugin: no conditions either", func(t *testing.T) { + repo := pluginWithConditions() + nested := &repository2.PluginStep{Id: 69, PluginId: 71, Name: "nested", Index: 1, StepType: repository2.PLUGIN_STEP_TYPE_REF_PLUGIN, RefPluginId: 42} + stepData, err := newPluginConditionsTestService(repo).BuildPluginStepDataForWfRequest(nested) + assert.NoError(t, err) + assert.Equal(t, "PLUGIN", stepData.ExecutorType) + assert.Equal(t, 42, stepData.RefPluginId) + assert.Len(t, stepData.InputVars, 1) + assert.Nil(t, stepData.TriggerSkipConditions) + assert.Nil(t, stepData.SuccessFailureConditions) + assert.Equal(t, 0, repo.conditionCalls) + }) + t.Run("variable lookup error is propagated", func(t *testing.T) { + repo := pluginWithConditions() + repo.variablesErr = errors.New("db down") + stepData, err := newPluginConditionsTestService(repo).BuildPluginStepDataForWfRequest(repo.steps[0]) + assert.EqualError(t, err, "db down") + assert.Nil(t, stepData) + }) +} + +func TestGetRefPluginStepsByIds_NoPluginConditionOnAnyStep(t *testing.T) { + repo := pluginWithConditions() + pluginIdStepsMap, err := newPluginConditionsTestService(repo).GetRefPluginStepsByIds([]int{71}, map[int]bool{71: true}) + assert.NoError(t, err) + if assert.Len(t, pluginIdStepsMap[71], 1) { + assert.Nil(t, pluginIdStepsMap[71][0].TriggerSkipConditions) + assert.Nil(t, pluginIdStepsMap[71][0].SuccessFailureConditions) + assert.Len(t, pluginIdStepsMap[71][0].InputVars, 1) + } + assert.Equal(t, 0, repo.conditionCalls) +} + +func TestBuildPluginVariableDataForWfRequest(t *testing.T) { + newRepo := func(vars ...*repository2.PluginStepVariable) *fakePluginRepoForWfRequest { + return &fakePluginRepoForWfRequest{variables: map[int][]*repository2.PluginStepVariable{69: vars}} + } + t.Run("exposed input without default uses the stored value, non-exposed input uses its default", func(t *testing.T) { + exposed := pluginStepVariable("A", repository2.PLUGIN_VARIABLE_TYPE_INPUT, true, "user", "") + exposed.VariableStepIndexInPlugin = 3 // only meaningful for plugins nested in plugins; must be passed through untouched + internal := pluginStepVariable("B", repository2.PLUGIN_VARIABLE_TYPE_INPUT, false, "ignored", "default") + exposedWithDefault := pluginStepVariable("C", repository2.PLUGIN_VARIABLE_TYPE_INPUT, true, "", "default") + inputs, outputs, err := newPluginConditionsTestService(newRepo(exposed, internal, exposedWithDefault)).BuildPluginVariableDataForWfRequest(69) + assert.NoError(t, err) + assert.Empty(t, outputs) + if assert.Len(t, inputs, 3) { + assert.Equal(t, "user", inputs[0].Value) + assert.Equal(t, "default", inputs[1].Value) + assert.Equal(t, "", inputs[2].Value, "exposed variable value is filled from the pipeline step by ci-runner") + assert.Equal(t, commonBean.VariableTypeValue, inputs[0].VariableType) + assert.Equal(t, commonBean.FormatTypeString, inputs[0].Format) + assert.Equal(t, 3, inputs[0].VariableStepIndexInPlugin, "nested-plugin step index is passed through") + assert.Equal(t, 0, inputs[1].VariableStepIndexInPlugin) + } + }) + t.Run("value types map to ci-runner variable types", func(t *testing.T) { + global := pluginStepVariable("G", repository2.PLUGIN_VARIABLE_TYPE_INPUT, false, "", "") + global.ValueType = repository2.PLUGIN_VARIABLE_VALUE_TYPE_GLOBAL + global.ReferenceVariableName = "DOCKER_IMAGE" + previous := pluginStepVariable("P", repository2.PLUGIN_VARIABLE_TYPE_INPUT, false, "", "") + previous.ValueType = repository2.PLUGIN_VARIABLE_VALUE_TYPE_PREVIOUS + previous.PreviousStepIndex = 1 + previous.ReferenceVariableName = "OUT" + exposedPrevious := pluginStepVariable("E", repository2.PLUGIN_VARIABLE_TYPE_INPUT, true, "", "") + exposedPrevious.ValueType = repository2.PLUGIN_VARIABLE_VALUE_TYPE_PREVIOUS + inputs, _, err := newPluginConditionsTestService(newRepo(global, previous, exposedPrevious)).BuildPluginVariableDataForWfRequest(69) + assert.NoError(t, err) + if assert.Len(t, inputs, 3) { + assert.Equal(t, commonBean.VariableTypeRefGlobal, inputs[0].VariableType) + assert.Equal(t, "DOCKER_IMAGE", inputs[0].ReferenceVariableName) + assert.Equal(t, commonBean.VariableTypeRefPlugin, inputs[1].VariableType) + assert.Equal(t, 1, inputs[1].ReferenceVariableStepIndex) + assert.Equal(t, commonBean.VariableTypeValue, inputs[2].VariableType, "exposed previous-step variables are resolved at pipeline level") + } + }) + t.Run("output variables are returned separately", func(t *testing.T) { + out := pluginStepVariable("OUT", repository2.PLUGIN_VARIABLE_TYPE_OUTPUT, true, "", "") + inputs, outputs, err := newPluginConditionsTestService(newRepo(out)).BuildPluginVariableDataForWfRequest(69) + assert.NoError(t, err) + assert.Empty(t, inputs) + if assert.Len(t, outputs, 1) { + assert.Equal(t, "OUT", outputs[0].Name) + } + }) + t.Run("no variables (pg.ErrNoRows) yields empty lists", func(t *testing.T) { + repo := &fakePluginRepoForWfRequest{variablesErr: pg.ErrNoRows} + inputs, outputs, err := newPluginConditionsTestService(repo).BuildPluginVariableDataForWfRequest(69) + assert.NoError(t, err) + assert.Empty(t, inputs) + assert.Empty(t, outputs) + }) + t.Run("repository error is propagated", func(t *testing.T) { + repo := &fakePluginRepoForWfRequest{variablesErr: errors.New("db down")} + _, _, err := newPluginConditionsTestService(repo).BuildPluginVariableDataForWfRequest(69) + assert.EqualError(t, err, "db down") + }) + t.Run("invalid variable format is an error", func(t *testing.T) { + bad := pluginStepVariable("X", repository2.PLUGIN_VARIABLE_TYPE_INPUT, true, "", "") + bad.Format = "BOGUS" + _, _, err := newPluginConditionsTestService(newRepo(bad)).BuildPluginVariableDataForWfRequest(69) + assert.Error(t, err) + }) +} diff --git a/pkg/pipeline/PipelineStageService.go b/pkg/pipeline/PipelineStageService.go index a3a2812c1d..c3e0449a37 100644 --- a/pkg/pipeline/PipelineStageService.go +++ b/pkg/pipeline/PipelineStageService.go @@ -2118,34 +2118,38 @@ func (impl *PipelineStageServiceImpl) BuildPluginStepDataForWfRequest(step *repo stepData.ExecutorType = "PLUGIN" //added only to avoid un-marshaling issues at ci-runner side, will not be used stepData.RefPluginId = step.RefPluginId } - inputVars, outputVars, triggerSkipConditions, successFailureConditions, err := impl.BuildPluginVariableAndConditionDataForWfRequest(step.Id) + inputVars, outputVars, err := impl.BuildPluginVariableDataForWfRequest(step.Id) if err != nil { - impl.logger.Errorw("error in getting variable and conditions data for wf request", "err", err, "stepId", step.Id) + impl.logger.Errorw("error in getting variable data for wf request", "err", err, "stepId", step.Id) return nil, err } stepData.InputVars = inputVars stepData.OutputVars = outputVars - stepData.TriggerSkipConditions = triggerSkipConditions - stepData.SuccessFailureConditions = successFailureConditions + // Conditions saved with the plugin definition (plugin_step_condition) are intentionally not sent for the + // plugin's internal steps. They are only defaults that pre-fill the task when the plugin is attached to a + // pipeline; the conditions to evaluate at trigger time are the ones saved on the pipeline step + // (pipeline_stage_step_condition), which are sent with the pre/post stage step referencing this plugin. return stepData, nil } -func (impl *PipelineStageServiceImpl) BuildPluginVariableAndConditionDataForWfRequest(stepId int) ([]*commonBean.VariableObject, []*commonBean.VariableObject, []*bean.ConditionObject, []*bean.ConditionObject, error) { +// BuildPluginVariableDataForWfRequest builds the input and output variables of a plugin's internal step for the +// workflow request. Plugin step conditions (plugin_step_condition) are deliberately not part of the workflow request: +// they are defaults used to pre-fill a task when the plugin is attached to a pipeline, and the effective conditions +// are the ones saved on the pipeline step (pipeline_stage_step_condition). +func (impl *PipelineStageServiceImpl) BuildPluginVariableDataForWfRequest(stepId int) ([]*commonBean.VariableObject, []*commonBean.VariableObject, error) { var inputVariables []*commonBean.VariableObject var outputVariables []*commonBean.VariableObject //getting all variables in the step variables, err := impl.globalPluginRepository.GetVariablesByStepId(stepId) if err != nil && err != pg.ErrNoRows { impl.logger.Errorw("error in getting variables by stepId", "err", err, "stepId", stepId) - return nil, nil, nil, nil, err + return nil, nil, err } - variableNameIdMap := make(map[int]string) for _, variable := range variables { - variableNameIdMap[variable.Id] = variable.Name format, err := commonBean.NewFormat(variable.Format.String()) if err != nil { impl.logger.Errorw("error in creating format object", "err", err, "format", variable.Format) - return nil, nil, nil, nil, err + return nil, nil, err } variableData := &commonBean.VariableObject{ Name: variable.Name, @@ -2181,30 +2185,7 @@ func (impl *PipelineStageServiceImpl) BuildPluginVariableAndConditionDataForWfRe outputVariables = append(outputVariables, variableData) } } - conditions, err := impl.globalPluginRepository.GetConditionsByStepId(stepId) - if err != nil && err != pg.ErrNoRows { - impl.logger.Errorw("error in getting conditions by stepId", "err", err, "stepId", stepId) - return nil, nil, nil, nil, err - } - var triggerSkipConditions []*bean.ConditionObject - var successFailureConditions []*bean.ConditionObject - for _, condition := range conditions { - conditionData := &bean.ConditionObject{ - ConditionalOperator: condition.ConditionalOperator, - ConditionalValue: condition.ConditionalValue, - ConditionType: string(condition.ConditionType), - } - varName, ok := variableNameIdMap[condition.ConditionVariableId] - if ok { - conditionData.ConditionOnVariable = varName - } - if condition.ConditionType == repository2.PLUGIN_CONDITION_TYPE_TRIGGER || condition.ConditionType == repository2.PLUGIN_CONDITION_TYPE_SKIP { - triggerSkipConditions = append(triggerSkipConditions, conditionData) - } else if condition.ConditionType == repository2.PLUGIN_CONDITION_TYPE_SUCCESS || condition.ConditionType == repository2.PLUGIN_CONDITION_TYPE_FAIL { - successFailureConditions = append(successFailureConditions, conditionData) - } - } - return inputVariables, outputVariables, triggerSkipConditions, successFailureConditions, nil + return inputVariables, outputVariables, nil } //BuildPrePostAndRefPluginStepsDataForWfRequest and related methods ends