Fix sidecar conditionals (#4672)

* fix sidecar conditionals

Co-authored-by: Philipp Stehle <philipp.stehle@sap.com>

* Fix unit tests

Co-authored-by: Ralf Pannemans <ralf.pannemans@sap.com>

* Consider parameter used in conditions of sidecars

Co-authored-by: Philipp Stehle <philipp.stehle@sap.com>

---------

Co-authored-by: Johannes Dillmann <j.dillmann@sap.com>
Co-authored-by: Philipp Stehle <philipp.stehle@sap.com>
This commit is contained in:
Ralf Pannemans
2023-12-18 16:03:58 +04:00
committed by GitHub
co-authored by Philipp Stehle Johannes Dillmann
parent 6587808062
commit cd8c93ea6c
6 changed files with 101 additions and 31 deletions
+2 -2
View File
@@ -295,10 +295,10 @@ func applyContextConditions(metadata config.StepData, stepConfig *config.StepCon
// consider conditions for context configuration
// containers
config.ApplyContainerConditions(metadata.Spec.Containers, stepConfig)
config.ApplyContainerConditions("container", metadata.Spec.Containers, stepConfig)
// sidecars
config.ApplyContainerConditions(metadata.Spec.Sidecars, stepConfig)
config.ApplyContainerConditions("sidecar", 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 ...
+9 -11
View File
@@ -122,12 +122,11 @@ func TestApplyContextConditions(t *testing.T) {
},
}}},
conf: config.StepConfig{Config: map[string]interface{}{
"param1": "val1",
"val1": map[string]interface{}{"dockerImage": "myTestImage:latest"},
"param1": "val1",
`container[param1=="val2"]`: map[string]interface{}{"dockerImage": "myTestImage:latest"},
}},
expected: map[string]interface{}{
"param1": "val1",
"val1": map[string]interface{}{"dockerImage": "myTestImage:latest"},
},
},
{
@@ -146,8 +145,8 @@ func TestApplyContextConditions(t *testing.T) {
},
}}},
conf: config.StepConfig{Config: map[string]interface{}{
"param1": "val1",
"val1": map[string]interface{}{"dockerImage": "myTestImage:latest"},
"param1": "val1",
`container[param1=="val1"]`: map[string]interface{}{"dockerImage": "myTestImage:latest"},
}},
expected: map[string]interface{}{
"param1": "val1",
@@ -194,9 +193,9 @@ func TestApplyContextConditions(t *testing.T) {
},
}}},
conf: config.StepConfig{Config: map[string]interface{}{
"param1": "val1",
"val1": map[string]interface{}{"dockerImage": "mySubTestImage:latest"},
"dockerImage": "myTestImage:latest",
"param1": "val1",
`container[param1=="val1"]`: map[string]interface{}{"dockerImage": "mySubTestImage:latest"},
"dockerImage": "myTestImage:latest",
}},
expected: map[string]interface{}{
"param1": "val1",
@@ -227,7 +226,6 @@ 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{
@@ -244,8 +242,8 @@ func TestApplyContextConditions(t *testing.T) {
},
}}},
conf: config.StepConfig{Config: map[string]interface{}{
"param1": "val1",
"val1": map[string]interface{}{"dockerImage": "myTestImage:latest"},
"param1": "val1",
`sidecar[param1=="val1"]`: map[string]interface{}{"dockerImage": "myTestImage:latest"},
}},
expected: map[string]interface{}{
"param1": "val1",
+5 -4
View File
@@ -444,22 +444,23 @@ func (s *StepConfig) mixInStepDefaults(stepParams []StepParameters) {
}
// ApplyContainerConditions evaluates conditions in step yaml container definitions
func ApplyContainerConditions(containers []Container, stepConfig *StepConfig) {
func ApplyContainerConditions(name string, 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[param.Value] != nil {
containerConf = stepConfig.Config[param.Value].(map[string]interface{})
if stepConfig.Config[key] != nil {
containerConf = stepConfig.Config[key].(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)
}
}
}
+31
View File
@@ -976,3 +976,34 @@ 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)
}
+40 -10
View File
@@ -236,8 +236,17 @@ func (m *StepData) GetContextParameterFilters() StepFilters {
}
if len(m.Spec.Sidecars) > 0 {
//ToDo: support fallback for "dockerName" configuration property -> via aliasing?
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
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 = addVaultContextParametersFilter(m, contextFilters)
@@ -277,7 +286,7 @@ func (m *StepData) GetContextDefaults(stepName string) (io.ReadCloser, error) {
}
p := map[string]interface{}{}
if key != "" {
root[key] = p
root[fmt.Sprintf("container[%s==%q]", conditionParam, key)] = p
//add default for condition parameter if available
for _, inputParam := range m.Spec.Inputs.Parameters {
if inputParam.Name == conditionParam {
@@ -302,14 +311,35 @@ func (m *StepData) GetContextDefaults(stepName string) (io.ReadCloser, error) {
}
if len(m.Spec.Sidecars) > 0 {
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)
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
}
// not filled for now since this is not relevant in Kubernetes case
//putStringIfNotEmpty(root, "containerPortMappings", m.Spec.Sidecars[0].)
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.Inputs.Resources) > 0 {
+14 -4
View File
@@ -255,7 +255,17 @@ func TestGetContextParameterFilters(t *testing.T) {
metadata3 := StepData{
Spec: StepSpec{
Sidecars: []Container{
{Name: "testsidecar"},
{Name: "testsidecar",
Conditions: []Condition{
{
Params: []Param{
{
Name: "conditionParam",
Value: "conditionValue",
},
},
},
}},
},
},
}
@@ -305,7 +315,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")
params := defaultParams("containerName", "containerPortMappings", "dockerName", "sidecarEnvVars", "sidecarImage", "sidecarName", "sidecarOptions", "sidecarPullImage", "sidecarReadyCommand", "sidecarVolumeBind", "sidecarWorkspace", "conditionValue", "conditionParam")
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")
@@ -506,10 +516,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"]["testConditionMet"].(map[string]interface{})
metParameter := d.Defaults[0].Steps["testStep"][`container[testConditionParameter=="testConditionMet"]`].(map[string]interface{})
assert.Equal(t, "testImage2:tag", metParameter["dockerImage"])
notMetParameter := d.Defaults[0].Steps["testStep"]["testConditionNotMet"].(map[string]interface{})
notMetParameter := d.Defaults[0].Steps["testStep"][`container[testConditionParameter=="testConditionNotMet"]`].(map[string]interface{})
assert.Equal(t, "testImage1:tag", notMetParameter["dockerImage"])
})