Revert "Fix sidecar conditionals (#4672)" (#4727)

This reverts commit cd8c93e.
SAP · Dec 18, 2023 · 0b585ed · 0b585ed
1 parent c3d420a
commit 0b585ed
Show file tree

Hide file tree

Showing 6 changed files with 31 additions and 101 deletions.
diff --git a/cmd/getConfig.go b/cmd/getConfig.go
@@ -295,10 +295,10 @@ func applyContextConditions(metadata config.StepData, stepConfig *config.StepCon
 	// consider conditions for context configuration
 
 	// containers
-	config.ApplyContainerConditions("container", metadata.Spec.Containers, stepConfig)
+	config.ApplyContainerConditions(metadata.Spec.Containers, stepConfig)
 
 	// sidecars
-	config.ApplyContainerConditions("sidecar", metadata.Spec.Sidecars, stepConfig)
+	config.ApplyContainerConditions(metadata.Spec.Sidecars, stepConfig)
 
 	// ToDo: remove all unnecessary sub maps?
 	// e.g. extract delete() from applyContainerConditions - loop over all stepConfig.Config[param.Value] and remove ...

diff --git a/cmd/getConfig_test.go b/cmd/getConfig_test.go
@@ -122,11 +122,12 @@ func TestApplyContextConditions(t *testing.T) {
 				},
 			}}},
 			conf: config.StepConfig{Config: map[string]interface{}{
-				"param1":                    "val1",
-				`container[param1=="val2"]`: map[string]interface{}{"dockerImage": "myTestImage:latest"},
+				"param1": "val1",
+				"val1":   map[string]interface{}{"dockerImage": "myTestImage:latest"},
 			}},
 			expected: map[string]interface{}{
 				"param1": "val1",
+				"val1":   map[string]interface{}{"dockerImage": "myTestImage:latest"},
 			},
 		},
 		{
@@ -145,8 +146,8 @@ func TestApplyContextConditions(t *testing.T) {
 				},
 			}}},
 			conf: config.StepConfig{Config: map[string]interface{}{
-				"param1":                    "val1",
-				`container[param1=="val1"]`: map[string]interface{}{"dockerImage": "myTestImage:latest"},
+				"param1": "val1",
+				"val1":   map[string]interface{}{"dockerImage": "myTestImage:latest"},
 			}},
 			expected: map[string]interface{}{
 				"param1":      "val1",
@@ -193,9 +194,9 @@ func TestApplyContextConditions(t *testing.T) {
 				},
 			}}},
 			conf: config.StepConfig{Config: map[string]interface{}{
-				"param1":                    "val1",
-				`container[param1=="val1"]`: map[string]interface{}{"dockerImage": "mySubTestImage:latest"},
-				"dockerImage":               "myTestImage:latest",
+				"param1":      "val1",
+				"val1":        map[string]interface{}{"dockerImage": "mySubTestImage:latest"},
+				"dockerImage": "myTestImage:latest",
 			}},
 			expected: map[string]interface{}{
 				"param1":      "val1",
@@ -226,6 +227,7 @@ func TestApplyContextConditions(t *testing.T) {
 				"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{
@@ -242,8 +244,8 @@ func TestApplyContextConditions(t *testing.T) {
 				},
 			}}},
 			conf: config.StepConfig{Config: map[string]interface{}{
-				"param1":                  "val1",
-				`sidecar[param1=="val1"]`: map[string]interface{}{"dockerImage": "myTestImage:latest"},
+				"param1": "val1",
+				"val1":   map[string]interface{}{"dockerImage": "myTestImage:latest"},
 			}},
 			expected: map[string]interface{}{
 				"param1":      "val1",

diff --git a/pkg/config/config.go b/pkg/config/config.go
@@ -444,23 +444,22 @@ func (s *StepConfig) mixInStepDefaults(stepParams []StepParameters) {
 }
 
 // ApplyContainerConditions evaluates conditions in step yaml container definitions
-func ApplyContainerConditions(name string, containers []Container, stepConfig *StepConfig) {
+func ApplyContainerConditions(containers []Container, stepConfig *StepConfig) {
 	for _, container := range containers {
 		if len(container.Conditions) > 0 {
 			for _, param := range container.Conditions[0].Params {
-				key := fmt.Sprintf("%s[%s==%q]", name, param.Name, param.Value)
 				if container.Conditions[0].ConditionRef == "strings-equal" && stepConfig.Config[param.Name] == param.Value {
 					var containerConf map[string]interface{}
-					if stepConfig.Config[key] != nil {
-						containerConf = stepConfig.Config[key].(map[string]interface{})
+					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, key)
 			}
 		}
 	}

diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go
@@ -976,34 +976,3 @@ func TestCloneConfig(t *testing.T) {
 	testConfig.General["p0"] = "new_value"
 	assert.NotEqual(t, testConfig.General, clone.General)
 }
-
-func TestApplyContainerConditions(t *testing.T) {
-	testConfig := &StepConfig{
-		Config: map[string]interface{}{
-			"key": "test",
-			`test[key=="test"]`: map[string]interface{}{
-				"merge-me": "please",
-			},
-			`test[key=="not-test"]`: map[string]interface{}{
-				"ignore-me": "please",
-			},
-		},
-	}
-
-	ApplyContainerConditions("test", []Container{{
-		Conditions: []Condition{{
-			ConditionRef: "strings-equal",
-			Params:       []Param{{Name: "key", Value: "test"}},
-		}},
-	}, {
-		Conditions: []Condition{{
-			ConditionRef: "strings-equal",
-			Params:       []Param{{Name: "key", Value: "not-test"}},
-		}},
-	}}, testConfig)
-
-	assert.Equal(t, map[string]interface{}{
-		"key":      "test",
-		"merge-me": "please",
-	}, testConfig.Config)
-}
diff --git a/pkg/config/stepmeta.go b/pkg/config/stepmeta.go
@@ -236,17 +236,8 @@ func (m *StepData) GetContextParameterFilters() StepFilters {
 	}
 	if len(m.Spec.Sidecars) > 0 {
 		//ToDo: support fallback for "dockerName" configuration property -> via aliasing?
-		parameterKeys := []string{"containerName", "containerPortMappings", "dockerName", "sidecarEnvVars", "sidecarImage", "sidecarName", "sidecarOptions", "sidecarPullImage", "sidecarReadyCommand", "sidecarVolumeBind", "sidecarWorkspace"}
-		for _, container := range m.Spec.Sidecars {
-			for _, condition := range container.Conditions {
-				for _, dependentParam := range condition.Params {
-					parameterKeys = append(parameterKeys, dependentParam.Value)
-					parameterKeys = append(parameterKeys, dependentParam.Name)
-				}
-			}
-		}
-		// ToDo: append dependentParam.Value & dependentParam.Name only according to correct parameter scope and not generally
-		contextFilters = append(contextFilters, parameterKeys...)
+		contextFilters = append(contextFilters, []string{"containerName", "containerPortMappings", "dockerName", "sidecarEnvVars", "sidecarImage", "sidecarName", "sidecarOptions", "sidecarPullImage", "sidecarReadyCommand", "sidecarVolumeBind", "sidecarWorkspace"}...)
+		//ToDo: add condition param.Value and param.Name to filter as for Containers
 	}
 
 	contextFilters = addVaultContextParametersFilter(m, contextFilters)
@@ -286,7 +277,7 @@ func (m *StepData) GetContextDefaults(stepName string) (io.ReadCloser, error) {
 			}
 			p := map[string]interface{}{}
 			if key != "" {
-				root[fmt.Sprintf("container[%s==%q]", conditionParam, key)] = p
+				root[key] = p
 				//add default for condition parameter if available
 				for _, inputParam := range m.Spec.Inputs.Parameters {
 					if inputParam.Name == conditionParam {
@@ -311,35 +302,14 @@ func (m *StepData) GetContextDefaults(stepName string) (io.ReadCloser, error) {
 	}
 
 	if len(m.Spec.Sidecars) > 0 {
-		for _, sidecar := range m.Spec.Sidecars {
-			key := ""
-			conditionParam := ""
-			if len(sidecar.Conditions) > 0 {
-				key = sidecar.Conditions[0].Params[0].Value
-				conditionParam = sidecar.Conditions[0].Params[0].Name
-			}
-			p := map[string]interface{}{}
-			if key != "" {
-				root[fmt.Sprintf("sidecar[%s==%q]", conditionParam, key)] = p
-				//add default for condition parameter if available
-				for _, inputParam := range m.Spec.Inputs.Parameters {
-					if inputParam.Name == conditionParam {
-						root[conditionParam] = inputParam.Default
-					}
-				}
-			} else {
-				p = root
-			}
-
-			if len(sidecar.Command) > 0 {
-				p["sidecarCommand"] = sidecar.Command[0]
-			}
-			sidecar.commonConfiguration("sidecar", &p)
-			putStringIfNotEmpty(p, "sidecarReadyCommand", sidecar.ReadyCommand)
-
-			// not filled for now since this is not relevant in Kubernetes case
-			//putStringIfNotEmpty(root, "containerPortMappings", sidecar.)
+		if len(m.Spec.Sidecars[0].Command) > 0 {
+			root["sidecarCommand"] = m.Spec.Sidecars[0].Command[0]
 		}
+		m.Spec.Sidecars[0].commonConfiguration("sidecar", &root)
+		putStringIfNotEmpty(root, "sidecarReadyCommand", m.Spec.Sidecars[0].ReadyCommand)
+
+		// not filled for now since this is not relevant in Kubernetes case
+		//putStringIfNotEmpty(root, "containerPortMappings", m.Spec.Sidecars[0].)
 	}
 
 	if len(m.Spec.Inputs.Resources) > 0 {

diff --git a/pkg/config/stepmeta_test.go b/pkg/config/stepmeta_test.go
@@ -255,17 +255,7 @@ func TestGetContextParameterFilters(t *testing.T) {
 	metadata3 := StepData{
 		Spec: StepSpec{
 			Sidecars: []Container{
-				{Name: "testsidecar",
-					Conditions: []Condition{
-						{
-							Params: []Param{
-								{
-									Name:  "conditionParam",
-									Value: "conditionValue",
-								},
-							},
-						},
-					}},
+				{Name: "testsidecar"},
 			},
 		},
 	}
@@ -315,7 +305,7 @@ func TestGetContextParameterFilters(t *testing.T) {
 
 	t.Run("Sidecars", func(t *testing.T) {
 		filters := metadata3.GetContextParameterFilters()
-		params := defaultParams("containerName", "containerPortMappings", "dockerName", "sidecarEnvVars", "sidecarImage", "sidecarName", "sidecarOptions", "sidecarPullImage", "sidecarReadyCommand", "sidecarVolumeBind", "sidecarWorkspace", "conditionValue", "conditionParam")
+		params := defaultParams("containerName", "containerPortMappings", "dockerName", "sidecarEnvVars", "sidecarImage", "sidecarName", "sidecarOptions", "sidecarPullImage", "sidecarReadyCommand", "sidecarVolumeBind", "sidecarWorkspace")
 		assert.Equal(t, params, filters.All, "incorrect filter All")
 		assert.Equal(t, params, filters.General, "incorrect filter General")
 		assert.Equal(t, params, filters.Steps, "incorrect filter Steps")
@@ -516,10 +506,10 @@ func TestGetContextDefaults(t *testing.T) {
 		assert.Equal(t, "testConditionMet", d.Defaults[0].Steps["testStep"]["testConditionParameter"])
 		assert.Nil(t, d.Defaults[0].Steps["testStep"]["dockerImage"])
 
-		metParameter := d.Defaults[0].Steps["testStep"][`container[testConditionParameter=="testConditionMet"]`].(map[string]interface{})
+		metParameter := d.Defaults[0].Steps["testStep"]["testConditionMet"].(map[string]interface{})
 		assert.Equal(t, "testImage2:tag", metParameter["dockerImage"])
 
-		notMetParameter := d.Defaults[0].Steps["testStep"][`container[testConditionParameter=="testConditionNotMet"]`].(map[string]interface{})
+		notMetParameter := d.Defaults[0].Steps["testStep"]["testConditionNotMet"].(map[string]interface{})
 		assert.Equal(t, "testImage1:tag", notMetParameter["dockerImage"])
 	})