From 957c2d29ef3cec7e49e35f24c7548cc918f60fae Mon Sep 17 00:00:00 2001 From: Jeeva Kandasamy Date: Wed, 15 Nov 2023 15:38:56 +0530 Subject: [PATCH] ignore nested struct in AddConfigMapValues transformer Signed-off-by: Jeeva Kandasamy --- .../testdata/test-add-configmap-values.yaml | 22 ++ pkg/reconciler/common/transformers.go | 58 +++-- pkg/reconciler/common/transformers_test.go | 212 +++++++++++++++++- 3 files changed, 269 insertions(+), 23 deletions(-) create mode 100644 pkg/reconciler/common/testdata/test-add-configmap-values.yaml diff --git a/pkg/reconciler/common/testdata/test-add-configmap-values.yaml b/pkg/reconciler/common/testdata/test-add-configmap-values.yaml new file mode 100644 index 0000000000..957692411c --- /dev/null +++ b/pkg/reconciler/common/testdata/test-add-configmap-values.yaml @@ -0,0 +1,22 @@ +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: test-add-config + namespace: tekton-pipelines + +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: test-add-config-with-data + namespace: tekton-pipelines +data: + foo: bar + +--- +apiVersion: v1 +kind: Deployment +metadata: + name: foo-deployment + namespace: foo diff --git a/pkg/reconciler/common/transformers.go b/pkg/reconciler/common/transformers.go index a311308195..497fe96ef7 100644 --- a/pkg/reconciler/common/transformers.go +++ b/pkg/reconciler/common/transformers.go @@ -527,11 +527,7 @@ func replaceNamespaceInContainerArg(container *corev1.Container, targetNamespace // with key as json tag of the struct field func AddConfigMapValues(configMapName string, prop interface{}) mf.Transformer { return func(u *unstructured.Unstructured) error { - kind := strings.ToLower(u.GetKind()) - if kind != "configmap" { - return nil - } - if u.GetName() != configMapName { + if u.GetKind() != "ConfigMap" || u.GetName() != configMapName || prop == nil { return nil } @@ -545,32 +541,54 @@ func AddConfigMapValues(configMapName string, prop interface{}) mf.Transformer { } values := reflect.ValueOf(prop) - types := values.Type() + // if the given properties is not struct type, do not proceed + if values.Kind() != reflect.Struct { + return nil + } + + for index := 0; index < values.NumField(); index++ { + key := values.Type().Field(index).Tag.Get("json") + if strings.Contains(key, ",") { + key = strings.Split(key, ",")[0] + } - for i := 0; i < values.NumField(); i++ { - key := strings.Split(types.Field(i).Tag.Get("json"), ",")[0] if key == "" { continue } - if values.Field(i).Kind() == reflect.Ptr { - innerElem := values.Field(i).Elem() - if !innerElem.IsValid() { + element := values.Field(index) + if element.Kind() == reflect.Ptr { + if element.IsNil() { continue } + // extract the actual element from the pointer + element = values.Field(index).Elem() + } - if innerElem.Kind() == reflect.Bool { - cm.Data[key] = strconv.FormatBool(innerElem.Bool()) - } else if innerElem.Kind() == reflect.Uint { - cm.Data[key] = strconv.FormatUint(innerElem.Uint(), 10) - } else if innerElem.Kind() == reflect.String { - cm.Data[key] = innerElem.String() - } + if !element.IsValid() { continue } - if value := values.Field(i).String(); value != "" { - cm.Data[key] = value + _value := "" + switch element.Kind() { + case reflect.Bool: + _value = strconv.FormatBool(element.Bool()) + + case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64: + _value = strconv.FormatInt(element.Int(), 10) + + case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64: + _value = strconv.FormatUint(element.Uint(), 10) + + case reflect.Float32, reflect.Float64: + _value = strconv.FormatFloat(element.Float(), 'f', 6, 64) + + case reflect.String: + _value = element.String() + } + + if _value != "" { + cm.Data[key] = _value } } diff --git a/pkg/reconciler/common/transformers_test.go b/pkg/reconciler/common/transformers_test.go index f3fb5f5c64..30848636bc 100644 --- a/pkg/reconciler/common/transformers_test.go +++ b/pkg/reconciler/common/transformers_test.go @@ -498,6 +498,212 @@ func TestAddConfigMapValues_OptionalPipelineProperties(t *testing.T) { assert.Equal(t, cm.Data["default-pod-template"], "") } +func TestAddConfigMapValues(t *testing.T) { + configMapName := "test-add-config" + configMapNameWithData := "test-add-config-with-data" + testData := path.Join("testdata", "test-add-configmap-values.yaml") + uintPtr := uint(2048) + + tests := []struct { + name string + targetConfigMapName string + props interface{} + expectedData map[string]string + keysShouldNotBeIn []string + doesConfigMapExists bool + }{ + { + name: "verify-values", + targetConfigMapName: configMapName, + props: struct { + StringValue string `json:"stringValue"` + StringPtr *string `json:"stringPtr"` + Int int `json:"int"` + Int8 int8 `json:"int8"` + Int16 int16 `json:"int16"` + Int32 int32 `json:"int32"` + Int64 int64 `json:"int64"` + Int64Ptr *int64 `json:"int64Ptr"` + Uint uint `json:"uint"` + Uint8 uint8 `json:"uint8"` + Uint16 uint16 `json:"uint16"` + Uint32 uint32 `json:"uint32"` + Uint64 uint64 `json:"uint64"` + UintPtr *uint `json:"uintPtr"` + Float32 float32 `json:"float32"` + Float64 float64 `json:"float64"` + Float64Ptr *float64 `json:"float64Ptr"` + Bool bool `json:"bool"` + BoolPtr *bool `json:"boolPtr"` + NestedStruct struct { + Foo string `json:"foo"` + } + }{ + StringValue: "foo", + StringPtr: ptr.String("fooPtr"), + Int: -256, + Int8: -128, + Int16: 512, + Int32: 2048, + Int64: 4096, + Uint: 256, + Uint8: 254, + Uint16: 512, + Uint32: 1024, + Uint64: 2048, + Int64Ptr: ptr.Int64(-2049), + UintPtr: &uintPtr, + Float32: 512.512, + Float64: 1024.1024567, + Float64Ptr: ptr.Float64(-1024.1023), + Bool: true, + BoolPtr: ptr.Bool(true), + NestedStruct: struct { + Foo string `json:"foo"` + }{ + Foo: "level-1", + }, + }, + doesConfigMapExists: true, + }, + { + name: "verify-with-nil-values", + targetConfigMapName: configMapName, + props: struct { + StringPtr *string `json:"stringPtr"` + StringPtr2 *string `json:"stringPtr2"` + Int32Ptr *int32 `json:"int32Ptr"` + Int32Ptr2 *int32 `json:"int32Ptr2"` + Int64Ptr *int64 `json:"int64Ptr"` + Int64Ptr2 *int64 `json:"int64Ptr2"` + UintPtr *uint `json:"uint"` + Uint8Ptr *uint8 `json:"uint8"` + Uint16Ptr *uint16 `json:"uint16"` + Uint32Ptr *uint32 `json:"uint32"` + Uint64Ptr *uint64 `json:"uint64"` + Float32Ptr *float32 `json:"float32"` + Float64Ptr *float64 `json:"float64Ptr"` + BoolPtr *bool `json:"boolPtr"` + NestedStruct interface{} `json:"interfaceNil"` + }{ + StringPtr2: ptr.String("hi"), + Int32Ptr2: ptr.Int32(21), + Int64Ptr2: ptr.Int64(22), + }, + expectedData: map[string]string{ + "stringPtr2": "hi", + "int32Ptr2": "21", + "int64Ptr2": "22", + }, + keysShouldNotBeIn: []string{ + "stringPtr", + "int32Ptr", + "int64Ptr", + "uint", + "uint8", + "uint16", + "uint32", + "uint64", + "float32", + "float64Ptr", + "boolPtr", + "interfaceNil", + }, + doesConfigMapExists: true, + }, + { + name: "verify-values-with-existing-data", + targetConfigMapName: configMapNameWithData, + props: struct { + Hello string `json:"hello"` + }{ + Hello: "hi", + }, + expectedData: map[string]string{ + "foo": "bar", // existing data in the map + "hello": "hi", + }, + keysShouldNotBeIn: []string{}, + doesConfigMapExists: true, + }, + { + name: "verify-configmap-not-found", + targetConfigMapName: "not-found-cm", // config map not exists + props: struct { + Name string `json:"name"` + }{ + Name: "test", + }, + doesConfigMapExists: false, + }, + { + name: "verify-props-as-map", + targetConfigMapName: configMapNameWithData, + props: map[string]string{"name": "hi"}, + expectedData: map[string]string{"foo": "bar"}, + keysShouldNotBeIn: []string{"name"}, + doesConfigMapExists: true, + }, + { + name: "verify-nil-props", + targetConfigMapName: configMapNameWithData, + props: nil, + expectedData: map[string]string{"foo": "bar"}, + doesConfigMapExists: true, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + // get a manifest + manifest, err := mf.ManifestFrom(mf.Recursive(testData)) + assert.NilError(t, err) + + manifest, err = manifest.Transform(AddConfigMapValues(test.targetConfigMapName, test.props)) + assert.NilError(t, err) + + var cm *corev1.ConfigMap + for _, resource := range manifest.Resources() { + if resource.GetKind() == "ConfigMap" && resource.GetName() == test.targetConfigMapName { + cm = &corev1.ConfigMap{} + err = runtime.DefaultUnstructuredConverter.FromUnstructured(resource.Object, cm) + assert.NilError(t, err) + } + } + + if test.doesConfigMapExists { + assert.Equal(t, false, cm == nil, fmt.Sprintf("configMap[%s] not found and not loaded", test.targetConfigMapName)) + } else { + assert.Equal(t, true, cm == nil, fmt.Sprintf("configMap[%s] found and loaded", test.targetConfigMapName)) + // return from here. no assertion needed + return + } + + for key, value := range test.expectedData { + keyFound := false + for targetMapKey, targetMapValue := range cm.Data { + if targetMapKey == key { + assert.Equal(t, value, targetMapValue, fmt.Sprintf("value not matching. key:[%s], value:[%s], expected:[%s], configMap:[%s]", targetMapKey, targetMapValue, value, test.targetConfigMapName)) + keyFound = true + break + } + } + assert.Equal(t, true, keyFound, fmt.Sprintf("key[%s] not found in configMap[%s]", key, test.targetConfigMapName)) + } + + // keys should not be in + for targetMapKey, targetMapValue := range cm.Data { + for _, keyShouldNotBeIn := range test.keysShouldNotBeIn { + if keyShouldNotBeIn == targetMapKey { + assert.Equal(t, false, keyShouldNotBeIn == targetMapKey, fmt.Sprintf("key should not be in, but found. key:[%s], value:[%s], configMap:[%s]", targetMapKey, targetMapValue, test.targetConfigMapName)) + } + } + } + + }) + } +} + func TestInjectLabelOnNamespace(t *testing.T) { t.Run("TestInjectLabel", func(t *testing.T) { testData := path.Join("testdata", "test-namespace-inject.yaml") @@ -800,9 +1006,9 @@ func TestCopyConfigMap(t *testing.T) { for key, value := range test.data { keyFound := false - for mapKey, mapValue := range cm.Data { - if mapKey == key { - assert.Equal(t, value, mapValue) + for targetMapKey, targetMapValue := range cm.Data { + if targetMapKey == key { + assert.Equal(t, value, targetMapValue) keyFound = true } }