From e890687d7227e5f5e95c3b5629a5e0722151abf8 Mon Sep 17 00:00:00 2001 From: NopeCharon <115794254+NopeCharon@users.noreply.github.com> Date: Tue, 27 Oct 2026 16:17:43 +0530 Subject: [PATCH] fix: do not panic when overriding attributes with non-identifier keys Overriding a parent or plugin devfile element whose attributes contain an object or list value under a key that is not a valid Go identifier, such as the commonly used `pod-overrides` attribute, panicked with: panic: reflect.StructOf: field 0 has invalid name During the strategic merge, mapEnabledPatchMetaFromStruct describes a map type to the strategicpatch library by building a struct type with a single field, named after the map key with strings.Title. Map keys like `pod-overrides` or `app.kubernetes.io/part-of` are not valid struct field names, so reflect.StructOf panics as soon as the merge descends into such an attribute value. The synthesized struct only ever has one field, so the field name and its JSON tag do not need to carry the map key. Use a fixed `Value` field with the `value` JSON name, and look up the patch metadata with that fixed key whenever the type being described is a map. The resulting patch strategy and value types are unchanged, so attribute values are still replaced as a whole as before. Add an overriding test fixture covering object and list attributes with non-identifier keys at the top level and in a component, including the scenario from the issue. Fixes #1268 Signed-off-by: NopeCharon <115794254+NopeCharon@users.noreply.github.com> --- pkg/utils/overriding/overriding.go | 23 +++++++++++++++---- .../original.yaml | 15 ++++++++++++ .../patch.yaml | 12 ++++++++++ .../result.yaml | 14 +++++++++++ 4 files changed, 59 insertions(+), 5 deletions(-) create mode 100644 pkg/utils/overriding/test-fixtures/patches/attributes-with-non-identifier-keys/original.yaml create mode 100644 pkg/utils/overriding/test-fixtures/patches/attributes-with-non-identifier-keys/patch.yaml create mode 100644 pkg/utils/overriding/test-fixtures/patches/attributes-with-non-identifier-keys/result.yaml diff --git a/pkg/utils/overriding/overriding.go b/pkg/utils/overriding/overriding.go index 16fea8f89..ebf809754 100644 --- a/pkg/utils/overriding/overriding.go +++ b/pkg/utils/overriding/overriding.go @@ -155,7 +155,18 @@ type mapEnabledPatchMetaFromStruct struct { var _ strategicpatch.LookupPatchMeta = (*mapEnabledPatchMetaFromStruct)(nil) -func (s *mapEnabledPatchMetaFromStruct) replaceMapWithSingleKeyStruct(key string, elementIsSlice bool) { +// mapValueFieldKey is the JSON name of the single field of the struct that +// replaces a map type during patch metadata lookup. +// A fixed key is used, instead of the map key itself, since map keys +// (such as attribute names like `pod-overrides`) are not necessarily valid +// Go struct field names or JSON tag names. +const mapValueFieldKey = "value" + +// replaceMapWithSingleKeyStruct replaces a map type with a struct type +// containing a single field that describes the map values. +// It returns the key that should be used to look up the patch metadata: +// mapValueFieldKey if the type was a map, or the given key otherwise. +func (s *mapEnabledPatchMetaFromStruct) replaceMapWithSingleKeyStruct(key string, elementIsSlice bool) string { t := s.delegate.T if t.Kind() == reflect.Ptr { t = t.Elem() @@ -163,7 +174,7 @@ func (s *mapEnabledPatchMetaFromStruct) replaceMapWithSingleKeyStruct(key string if t.Kind() == reflect.Map { typeName := t.Name() - tag := `json:"` + key + `"` + tag := `json:"` + mapValueFieldKey + `"` elemType := t.Elem() if typeName == "Attributes" { if elementIsSlice { @@ -174,12 +185,14 @@ func (s *mapEnabledPatchMetaFromStruct) replaceMapWithSingleKeyStruct(key string } } - s.delegate.T = reflect.StructOf([]reflect.StructField{{Name: strings.Title(key), Type: elemType, Tag: reflect.StructTag(tag)}}) + s.delegate.T = reflect.StructOf([]reflect.StructField{{Name: "Value", Type: elemType, Tag: reflect.StructTag(tag)}}) + return mapValueFieldKey } + return key } func (s mapEnabledPatchMetaFromStruct) LookupPatchMetadataForStruct(key string) (strategicpatch.LookupPatchMeta, strategicpatch.PatchMeta, error) { - s.replaceMapWithSingleKeyStruct(key, false) + key = s.replaceMapWithSingleKeyStruct(key, false) schema, patchMeta, err := s.delegate.LookupPatchMetadataForStruct(key) var lookupPatchMeta strategicpatch.LookupPatchMeta = nil @@ -190,7 +203,7 @@ func (s mapEnabledPatchMetaFromStruct) LookupPatchMetadataForStruct(key string) } func (s mapEnabledPatchMetaFromStruct) LookupPatchMetadataForSlice(key string) (strategicpatch.LookupPatchMeta, strategicpatch.PatchMeta, error) { - s.replaceMapWithSingleKeyStruct(key, true) + key = s.replaceMapWithSingleKeyStruct(key, true) schema, patchMeta, err := s.delegate.LookupPatchMetadataForSlice(key) var lookupPatchMeta strategicpatch.LookupPatchMeta = nil diff --git a/pkg/utils/overriding/test-fixtures/patches/attributes-with-non-identifier-keys/original.yaml b/pkg/utils/overriding/test-fixtures/patches/attributes-with-non-identifier-keys/original.yaml new file mode 100644 index 000000000..2c1989bcd --- /dev/null +++ b/pkg/utils/overriding/test-fixtures/patches/attributes-with-non-identifier-keys/original.yaml @@ -0,0 +1,15 @@ +attributes: + app.kubernetes.io/part-of: + name: original-name + version: 1.0.0 + list-attribute: + - originalElement +components: + - name: runtime + container: + image: python:3.11 + attributes: + pod-overrides: + spec: + imagePullSecrets: + - name: regcred diff --git a/pkg/utils/overriding/test-fixtures/patches/attributes-with-non-identifier-keys/patch.yaml b/pkg/utils/overriding/test-fixtures/patches/attributes-with-non-identifier-keys/patch.yaml new file mode 100644 index 000000000..9377906f0 --- /dev/null +++ b/pkg/utils/overriding/test-fixtures/patches/attributes-with-non-identifier-keys/patch.yaml @@ -0,0 +1,12 @@ +attributes: + app.kubernetes.io/part-of: + name: patched-name + list-attribute: + - patchedElement +components: + - name: runtime + attributes: + pod-overrides: + spec: + imagePullSecrets: + - name: abcd diff --git a/pkg/utils/overriding/test-fixtures/patches/attributes-with-non-identifier-keys/result.yaml b/pkg/utils/overriding/test-fixtures/patches/attributes-with-non-identifier-keys/result.yaml new file mode 100644 index 000000000..3b9c211e1 --- /dev/null +++ b/pkg/utils/overriding/test-fixtures/patches/attributes-with-non-identifier-keys/result.yaml @@ -0,0 +1,14 @@ +attributes: + app.kubernetes.io/part-of: + name: patched-name + list-attribute: + - patchedElement +components: + - name: runtime + container: + image: python:3.11 + attributes: + pod-overrides: + spec: + imagePullSecrets: + - name: abcd