From a0ded365c51a59604d2d6321bf3338269fd6bcd4 Mon Sep 17 00:00:00 2001 From: Oliver Nocon <33484802+OliverNocon@users.noreply.github.com> Date: Wed, 20 May 2020 10:50:35 +0200 Subject: [PATCH] Fix container condition handling (#1562) * Add additional tests * Update handling of containers * Properly consider existing config at root level * Fix parameter filtering * Fix tests for adapted filter handling --- cmd/getConfig.go | 17 ++++--- cmd/getConfig_test.go | 89 +++++++++++++++++++++++++++++++++++-- pkg/config/stepmeta.go | 2 + pkg/config/stepmeta_test.go | 4 +- 4 files changed, 100 insertions(+), 12 deletions(-) diff --git a/cmd/getConfig.go b/cmd/getConfig.go index f59c553f0..c37614b68 100644 --- a/cmd/getConfig.go +++ b/cmd/getConfig.go @@ -2,11 +2,12 @@ package cmd import ( "fmt" + "io" + "os" + "github.com/SAP/jenkins-library/pkg/config" "github.com/pkg/errors" "github.com/spf13/cobra" - "io" - "os" ) type configCommandOptions struct { @@ -148,11 +149,15 @@ func applyContainerConditions(containers []config.Container, stepConfig *config. for _, param := range container.Conditions[0].Params { if container.Conditions[0].ConditionRef == "strings-equal" && stepConfig.Config[param.Name] == param.Value { var containerConf map[string]interface{} - containerConf = stepConfig.Config[param.Value].(map[string]interface{}) - for key, value := range containerConf { - stepConfig.Config[key] = value + if stepConfig.Config[param.Value] != nil { + containerConf = stepConfig.Config[param.Value].(map[string]interface{}) + for key, value := range containerConf { + if stepConfig.Config[key] == nil { + stepConfig.Config[key] = value + } + } + delete(stepConfig.Config, param.Value) } - delete(stepConfig.Config, param.Value) } } } diff --git a/cmd/getConfig_test.go b/cmd/getConfig_test.go index a0fdaefb4..0ec319884 100644 --- a/cmd/getConfig_test.go +++ b/cmd/getConfig_test.go @@ -92,19 +92,22 @@ func TestDefaultsAndFilters(t *testing.T) { func TestApplyContextConditions(t *testing.T) { tt := []struct { + name string metadata config.StepData conf config.StepConfig expected map[string]interface{} }{ { + name: "no context conditions", metadata: config.StepData{Spec: config.StepSpec{Containers: []config.Container{}}}, conf: config.StepConfig{Config: map[string]interface{}{}}, expected: map[string]interface{}{}, }, { + name: "context condition not met", metadata: config.StepData{Spec: config.StepSpec{Containers: []config.Container{ { - Image: "myTestImage:latest", + Image: "myDefaultImage:latest", Conditions: []config.Condition{ { ConditionRef: "strings-equal", @@ -125,9 +128,10 @@ func TestApplyContextConditions(t *testing.T) { }, }, { + name: "context condition met", metadata: config.StepData{Spec: config.StepSpec{Containers: []config.Container{ { - Image: "myTestImage:latest", + Image: "myDefaultImage:latest", Conditions: []config.Condition{ { ConditionRef: "strings-equal", @@ -148,6 +152,81 @@ func TestApplyContextConditions(t *testing.T) { }, }, { + name: "context condition met - root defined already", + metadata: config.StepData{Spec: config.StepSpec{Containers: []config.Container{ + { + Image: "myDefaultImage:latest", + Conditions: []config.Condition{ + { + ConditionRef: "strings-equal", + Params: []config.Param{ + {Name: "param1", Value: "val1"}, + }, + }, + }, + }, + }}}, + conf: config.StepConfig{Config: map[string]interface{}{ + "param1": "val1", + "dockerImage": "myTestImage:latest", + }}, + expected: map[string]interface{}{ + "param1": "val1", + "dockerImage": "myTestImage:latest", + }, + }, + { + name: "context condition met - root defined and deep value defined", + metadata: config.StepData{Spec: config.StepSpec{Containers: []config.Container{ + { + Image: "myDefaultImage:latest", + Conditions: []config.Condition{ + { + ConditionRef: "strings-equal", + Params: []config.Param{ + {Name: "param1", Value: "val1"}, + }, + }, + }, + }, + }}}, + conf: config.StepConfig{Config: map[string]interface{}{ + "param1": "val1", + "val1": map[string]interface{}{"dockerImage": "mySubTestImage:latest"}, + "dockerImage": "myTestImage:latest", + }}, + expected: map[string]interface{}{ + "param1": "val1", + "dockerImage": "myTestImage:latest", + }, + }, + { + name: "context condition met - root defined as empty", + metadata: config.StepData{Spec: config.StepSpec{Containers: []config.Container{ + { + Image: "myDefaultImage:latest", + Conditions: []config.Condition{ + { + ConditionRef: "strings-equal", + Params: []config.Param{ + {Name: "param1", Value: "val1"}, + }, + }, + }, + }, + }}}, + conf: config.StepConfig{Config: map[string]interface{}{ + "param1": "val1", + "dockerImage": "", + }}, + expected: map[string]interface{}{ + "param1": "val1", + "dockerImage": "", + }, + }, + //ToDo: Sidecar behavior not properly working, expects sidecarImage, ... parameters and not dockerImage + { + name: "sidecar context condition met", metadata: config.StepData{Spec: config.StepSpec{Sidecars: []config.Container{ { Image: "myTestImage:latest", @@ -173,7 +252,9 @@ func TestApplyContextConditions(t *testing.T) { } for run, test := range tt { - applyContextConditions(test.metadata, &test.conf) - assert.Equalf(t, test.expected, test.conf.Config, fmt.Sprintf("Run %v failed", run)) + t.Run(test.name, func(t *testing.T) { + applyContextConditions(test.metadata, &test.conf) + assert.Equalf(t, test.expected, test.conf.Config, fmt.Sprintf("Run %v failed", run)) + }) } } diff --git a/pkg/config/stepmeta.go b/pkg/config/stepmeta.go index 175b3e0e1..1eb125e42 100644 --- a/pkg/config/stepmeta.go +++ b/pkg/config/stepmeta.go @@ -216,6 +216,7 @@ func (m *StepData) GetContextParameterFilters() StepFilters { } } } + // ToDo: append dependentParam.Value & dependentParam.Name only according to correct parameter scope and not generally containerFilters = append(containerFilters, parameterKeys...) } if len(m.Spec.Sidecars) > 0 { @@ -225,6 +226,7 @@ func (m *StepData) GetContextParameterFilters() StepFilters { } if len(containerFilters) > 0 { filters.All = append(filters.All, containerFilters...) + filters.General = append(filters.General, containerFilters...) filters.Steps = append(filters.Steps, containerFilters...) filters.Stages = append(filters.Stages, containerFilters...) filters.Parameters = append(filters.Parameters, containerFilters...) diff --git a/pkg/config/stepmeta_test.go b/pkg/config/stepmeta_test.go index 3d7eb3834..04e504692 100644 --- a/pkg/config/stepmeta_test.go +++ b/pkg/config/stepmeta_test.go @@ -268,7 +268,7 @@ func TestGetContextParameterFilters(t *testing.T) { t.Run("Containers", func(t *testing.T) { filters := metadata2.GetContextParameterFilters() assert.Equal(t, []string{"containerCommand", "containerShell", "dockerEnvVars", "dockerImage", "dockerOptions", "dockerPullImage", "dockerVolumeBind", "dockerWorkspace", "pip", "scanType"}, filters.All, "incorrect filter All") - assert.NotEqual(t, []string{"containerCommand", "containerShell", "dockerEnvVars", "dockerImage", "dockerOptions", "dockerPullImage", "dockerVolumeBind", "dockerWorkspace", "pip", "scanType"}, filters.General, "incorrect filter General") + assert.Equal(t, []string{"containerCommand", "containerShell", "dockerEnvVars", "dockerImage", "dockerOptions", "dockerPullImage", "dockerVolumeBind", "dockerWorkspace", "pip", "scanType"}, filters.General, "incorrect filter General") assert.Equal(t, []string{"containerCommand", "containerShell", "dockerEnvVars", "dockerImage", "dockerOptions", "dockerPullImage", "dockerVolumeBind", "dockerWorkspace", "pip", "scanType"}, filters.Steps, "incorrect filter Steps") assert.Equal(t, []string{"containerCommand", "containerShell", "dockerEnvVars", "dockerImage", "dockerOptions", "dockerPullImage", "dockerVolumeBind", "dockerWorkspace", "pip", "scanType"}, filters.Stages, "incorrect filter Stages") assert.Equal(t, []string{"containerCommand", "containerShell", "dockerEnvVars", "dockerImage", "dockerOptions", "dockerPullImage", "dockerVolumeBind", "dockerWorkspace", "pip", "scanType"}, filters.Parameters, "incorrect filter Parameters") @@ -278,7 +278,7 @@ func TestGetContextParameterFilters(t *testing.T) { t.Run("Sidecars", func(t *testing.T) { filters := metadata3.GetContextParameterFilters() assert.Equal(t, []string{"containerName", "containerPortMappings", "dockerName", "sidecarEnvVars", "sidecarImage", "sidecarName", "sidecarOptions", "sidecarPullImage", "sidecarReadyCommand", "sidecarVolumeBind", "sidecarWorkspace"}, filters.All, "incorrect filter All") - assert.NotEqual(t, []string{"containerName", "containerPortMappings", "dockerName", "sidecarEnvVars", "sidecarImage", "sidecarName", "sidecarOptions", "sidecarPullImage", "sidecarReadyCommand", "sidecarVolumeBind", "sidecarWorkspace"}, filters.General, "incorrect filter General") + assert.Equal(t, []string{"containerName", "containerPortMappings", "dockerName", "sidecarEnvVars", "sidecarImage", "sidecarName", "sidecarOptions", "sidecarPullImage", "sidecarReadyCommand", "sidecarVolumeBind", "sidecarWorkspace"}, filters.General, "incorrect filter General") assert.Equal(t, []string{"containerName", "containerPortMappings", "dockerName", "sidecarEnvVars", "sidecarImage", "sidecarName", "sidecarOptions", "sidecarPullImage", "sidecarReadyCommand", "sidecarVolumeBind", "sidecarWorkspace"}, filters.Steps, "incorrect filter Steps") assert.Equal(t, []string{"containerName", "containerPortMappings", "dockerName", "sidecarEnvVars", "sidecarImage", "sidecarName", "sidecarOptions", "sidecarPullImage", "sidecarReadyCommand", "sidecarVolumeBind", "sidecarWorkspace"}, filters.Stages, "incorrect filter Stages") assert.Equal(t, []string{"containerName", "containerPortMappings", "dockerName", "sidecarEnvVars", "sidecarImage", "sidecarName", "sidecarOptions", "sidecarPullImage", "sidecarReadyCommand", "sidecarVolumeBind", "sidecarWorkspace"}, filters.Parameters, "incorrect filter Parameters")