Skip to content

Commit

Permalink
fix(api): make PromotionStep input field a list
Browse files Browse the repository at this point in the history 
Signed-off-by: Hidde Beydals <[email protected]>
  • Loading branch information
@hiddeco
hiddeco committed Dec 13, 2024
1 parent 3557742 commit 4261f9e
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 35 deletions.
1 change: 1 addition & 0 deletions api/v1alpha1/promotion_task_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ type PromotionTaskInput struct {
//
// +kubebuilder:validation:Required
// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:Pattern=^[a-zA-Z_]\w*$
Name string `json:"name" protobuf:"bytes,1,opt,name=name"`
// Default specifies a default value for the parameter. This value will be
// used if the parameter is not specified in the PromotionTemplate.
Expand Down
16 changes: 14 additions & 2 deletions api/v1alpha1/promotion_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,18 @@ func (r *PromotionStepRetry) GetErrorThreshold(fallback uint32) uint32 {
return r.ErrorThreshold
}

// PromotionStepInput describes a single input value for a PromotionStep that may
// be referenced by expressions in the step.
type PromotionStepInput struct {
// Name is the name of the input to which the value is assigned.
//
// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:Pattern=^[a-zA-Z_]\w*$
Name string `json:"name" protobuf:"bytes,1,opt,name=name"`
// Value is the input value.
Value string `json:"value" protobuf:"bytes,2,opt,name=value"`
}

// PromotionStep describes a directive to be executed as part of a Promotion.
//
// +kubebuilder:validation:XValidation:message="inputs must not be set when task is set",rule="!(has(self.task) && self.inputs.size() > 0)"
Expand All @@ -211,13 +223,13 @@ type PromotionStep struct {
As string `json:"as,omitempty" protobuf:"bytes,2,opt,name=as"`
// Retry is the retry policy for this step.
Retry *PromotionStepRetry `json:"retry,omitempty" protobuf:"bytes,4,opt,name=retry"`
// Inputs is a map of inputs that can used to parameterize the execution
// Inputs is a list of inputs that can used to parameterize the execution
// of the PromotionStep and can be referenced by expressions in the Config.
//
// When a PromotionStep is inflated from a PromotionTask, the inputs
// specified in the PromotionTask are set based on the inputs specified
// in the Config of the PromotionStep that references the PromotionTask.
Inputs map[string]string `json:"inputs,omitempty" protobuf:"bytes,6,rep,name=inputs" protobuf_key:"bytes,1,opt,name=key" protobuf_val:"bytes,2,opt,name=value"`
Inputs []PromotionStepInput `json:"inputs,omitempty" protobuf:"bytes,6,rep,name=inputs"`
// Config is opaque configuration for the PromotionStep that is understood
// only by each PromotionStep's implementation. It is legal to utilize
// expressions in defining values at any level of this block.
Expand Down
15 changes: 9 additions & 6 deletions internal/kargo/promotion_builder.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ func (b *PromotionBuilder) inflateTaskSteps(
return nil, err
}

inputs, err := validateAndMapTaskInputs(task.Inputs, taskStep.Config)
inputs, err := promotionTaskInputsToStepInputs(task.Inputs, taskStep.Config)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -215,12 +215,12 @@ func generatePromotionTaskStepAlias(taskAlias, stepAlias string) string {
return fmt.Sprintf("%s%s%s", taskAlias, aliasSeparator, stepAlias)
}

// validateAndMapTaskInputs validates the task step config against the task
// promotionTaskInputsToStepInputs validates the task step config against the task
// inputs, and maps the config to inputs for the inflated steps.
func validateAndMapTaskInputs(
func promotionTaskInputsToStepInputs(
taskInputs []kargoapi.PromotionTaskInput,
stepConfig *apiextensionsv1.JSON,
) (map[string]string, error) {
) ([]kargoapi.PromotionStepInput, error) {
if len(taskInputs) == 0 {
return nil, nil
}
Expand All @@ -234,7 +234,7 @@ func validateAndMapTaskInputs(
return nil, fmt.Errorf("unmarshal step config: %w", err)
}

inputs := make(map[string]string, len(taskInputs))
inputs := make([]kargoapi.PromotionStepInput, 0, len(taskInputs))
for _, input := range taskInputs {
iv := input.Default
if cv, exists := config[input.Name]; exists {
Expand All @@ -247,7 +247,10 @@ func validateAndMapTaskInputs(
if iv == "" {
return nil, fmt.Errorf("missing required input %q", input.Name)
}
inputs[input.Name] = iv
inputs = append(inputs, kargoapi.PromotionStepInput{
Name: input.Name,
Value: iv,
})
}
return inputs, nil
}
66 changes: 39 additions & 27 deletions internal/kargo/promotion_builder_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,9 @@ func TestPromotionBuilder_Build(t *testing.T) {
require.Len(t, promotion.Spec.Steps, 1)
assert.Equal(t, "task-step::sub-step", promotion.Spec.Steps[0].As)
assert.Equal(t, "other-fake-step", promotion.Spec.Steps[0].Uses)
assert.Equal(t, map[string]string{"input1": "value1"}, promotion.Spec.Steps[0].Inputs)
assert.ElementsMatch(t, []kargoapi.PromotionStepInput{
{Name: "input1", Value: "value1"},
}, promotion.Spec.Steps[0].Inputs)
},
},
}
Expand Down Expand Up @@ -331,7 +333,9 @@ func TestPromotionBuilder_buildSteps(t *testing.T) {
// Check inflated task step
assert.Equal(t, "task-step::sub-step", steps[1].As)
assert.Equal(t, "other-fake-step", steps[1].Uses)
assert.Equal(t, map[string]string{"input1": "value1"}, steps[1].Inputs)
assert.ElementsMatch(t, []kargoapi.PromotionStepInput{
{Name: "input1", Value: "value1"},
}, steps[1].Inputs)
},
},
{
Expand Down Expand Up @@ -410,11 +414,15 @@ func TestPromotionBuilder_buildSteps(t *testing.T) {

assert.Equal(t, "task1::step1", steps[0].As)
assert.Equal(t, "fake-step", steps[0].Uses)
assert.Equal(t, map[string]string{"input1": "value1"}, steps[0].Inputs)
assert.ElementsMatch(t, []kargoapi.PromotionStepInput{
{Name: "input1", Value: "value1"},
}, steps[0].Inputs)

assert.Equal(t, "task2::step2", steps[1].As)
assert.Equal(t, "other-fake-step", steps[1].Uses)
assert.Equal(t, map[string]string{"input2": "value2"}, steps[1].Inputs)
assert.ElementsMatch(t, []kargoapi.PromotionStepInput{
{Name: "input2", Value: "value2"},
}, steps[1].Inputs)
},
},
}
Expand Down Expand Up @@ -530,16 +538,16 @@ func TestPromotionBuilder_inflateTaskSteps(t *testing.T) {

assert.Equal(t, "task-1::step1", steps[0].As)
assert.Equal(t, "fake-step", steps[0].Uses)
assert.Equal(t, map[string]string{
"input1": "value1",
"input2": "value2",
assert.ElementsMatch(t, []kargoapi.PromotionStepInput{
{Name: "input1", Value: "value1"},
{Name: "input2", Value: "value2"},
}, steps[0].Inputs)

assert.Equal(t, "task-1::step2", steps[1].As)
assert.Equal(t, "other-fake-step", steps[1].Uses)
assert.Equal(t, map[string]string{
"input1": "value1",
"input2": "value2",
assert.ElementsMatch(t, []kargoapi.PromotionStepInput{
{Name: "input1", Value: "value1"},
{Name: "input2", Value: "value2"},
}, steps[1].Inputs)
},
},
Expand Down Expand Up @@ -961,18 +969,18 @@ func Test_generatePromotionTaskStepName(t *testing.T) {
}
}

func Test_validateAndMapTaskInputs(t *testing.T) {
func Test_promotionTaskInputsToStepInputs(t *testing.T) {
tests := []struct {
name string
taskInputs []kargoapi.PromotionTaskInput
config map[string]any
assertions func(t *testing.T, result map[string]string, err error)
assertions func(t *testing.T, result []kargoapi.PromotionStepInput, err error)
}{
{
name: "nil inputs returns nil map and no error",
taskInputs: nil,
config: nil,
assertions: func(t *testing.T, result map[string]string, err error) {
assertions: func(t *testing.T, result []kargoapi.PromotionStepInput, err error) {
require.NoError(t, err)
assert.Nil(t, result)
},
Expand All @@ -981,7 +989,7 @@ func Test_validateAndMapTaskInputs(t *testing.T) {
name: "empty inputs returns nil map and no error",
taskInputs: []kargoapi.PromotionTaskInput{},
config: nil,
assertions: func(t *testing.T, result map[string]string, err error) {
assertions: func(t *testing.T, result []kargoapi.PromotionStepInput, err error) {
require.NoError(t, err)
assert.Nil(t, result)
},
Expand All @@ -992,7 +1000,7 @@ func Test_validateAndMapTaskInputs(t *testing.T) {
{Name: "input1"},
},
config: nil,
assertions: func(t *testing.T, result map[string]string, err error) {
assertions: func(t *testing.T, result []kargoapi.PromotionStepInput, err error) {
assert.ErrorContains(t, err, "missing step config")
assert.Nil(t, result)
},
Expand All @@ -1005,7 +1013,7 @@ func Test_validateAndMapTaskInputs(t *testing.T) {
config: map[string]any{
"input1": 123, // number instead of string
},
assertions: func(t *testing.T, result map[string]string, err error) {
assertions: func(t *testing.T, result []kargoapi.PromotionStepInput, err error) {
assert.ErrorContains(t, err, "input \"input1\" must be a string")
assert.Nil(t, result)
},
Expand All @@ -1018,7 +1026,7 @@ func Test_validateAndMapTaskInputs(t *testing.T) {
config: map[string]any{
"input1": "", // empty string is not allowed without default
},
assertions: func(t *testing.T, result map[string]string, err error) {
assertions: func(t *testing.T, result []kargoapi.PromotionStepInput, err error) {
assert.ErrorContains(t, err, "missing required input \"input1\"")
assert.Nil(t, result)
},
Expand All @@ -1029,9 +1037,11 @@ func Test_validateAndMapTaskInputs(t *testing.T) {
{Name: "input1", Default: "default1"},
},
config: map[string]any{},
assertions: func(t *testing.T, result map[string]string, err error) {
assertions: func(t *testing.T, result []kargoapi.PromotionStepInput, err error) {
require.NoError(t, err)
assert.Equal(t, map[string]string{"input1": "default1"}, result)
assert.ElementsMatch(t, []kargoapi.PromotionStepInput{
{Name: "input1", Value: "default1"},
}, result)
},
},
{
Expand All @@ -1042,9 +1052,11 @@ func Test_validateAndMapTaskInputs(t *testing.T) {
config: map[string]any{
"input1": "override1",
},
assertions: func(t *testing.T, result map[string]string, err error) {
assertions: func(t *testing.T, result []kargoapi.PromotionStepInput, err error) {
require.NoError(t, err)
assert.Equal(t, map[string]string{"input1": "override1"}, result)
assert.ElementsMatch(t, []kargoapi.PromotionStepInput{
{Name: "input1", Value: "override1"},
}, result)
},
},
{
Expand All @@ -1058,12 +1070,12 @@ func Test_validateAndMapTaskInputs(t *testing.T) {
"input1": "override1",
"input3": "value3",
},
assertions: func(t *testing.T, result map[string]string, err error) {
assertions: func(t *testing.T, result []kargoapi.PromotionStepInput, err error) {
require.NoError(t, err)
assert.Equal(t, map[string]string{
"input1": "override1",
"input2": "default2",
"input3": "value3",
assert.ElementsMatch(t, []kargoapi.PromotionStepInput{
{Name: "input1", Value: "override1"},
{Name: "input2", Value: "default2"},
{Name: "input3", Value: "value3"},
}, result)
},
},
Expand All @@ -1078,7 +1090,7 @@ func Test_validateAndMapTaskInputs(t *testing.T) {
configJSON = &apiextensionsv1.JSON{Raw: configBytes}
}

result, err := validateAndMapTaskInputs(tt.taskInputs, configJSON)
result, err := promotionTaskInputsToStepInputs(tt.taskInputs, configJSON)
tt.assertions(t, result, err)
})
}
Expand Down

0 comments on commit 4261f9e

Please sign in to comment.