From 0e9691000c504bcb4375adfc284f4d3093bceec7 Mon Sep 17 00:00:00 2001 From: yxxhero Date: Tue, 2 Apr 2024 20:44:40 +0800 Subject: [PATCH 1/2] fix normalieze manifests issue Signed-off-by: yxxhero --- manifest/parse.go | 31 ++++++++++++++++++----------- manifest/parse_test.go | 44 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 11 deletions(-) diff --git a/manifest/parse.go b/manifest/parse.go index 552a5ba3..ce784de6 100644 --- a/manifest/parse.go +++ b/manifest/parse.go @@ -144,18 +144,11 @@ func parseContent(content string, defaultNamespace string, normalizeManifests bo } if normalizeManifests { - // Unmarshal and marshal again content to normalize yaml structure - // This avoids style differences to show up as diffs but it can - // make the output different from the original template (since it is in normalized form) - var object map[interface{}]interface{} - if err := yaml.Unmarshal([]byte(content), &object); err != nil { - log.Fatalf("YAML unmarshal error: %s\nCan't unmarshal %s", err, content) - } - normalizedContent, err := yaml.Marshal(object) - if err != nil { - log.Fatalf("YAML marshal error: %s\nCan't marshal %v", err, object) + var normalizeErr error + content, normalizeErr = ContentNormalizeManifests(content) + if normalizeErr != nil { + log.Fatalf("Error normalizing manifests: %v", normalizeErr) } - content = string(normalizedContent) } if isHook(parsedMetadata, excludedHooks...) { @@ -176,6 +169,22 @@ func parseContent(content string, defaultNamespace string, normalizeManifests bo }, nil } +func ContentNormalizeManifests(content string) (string, error) { + // Unmarshal and marshal again content to normalize yaml structure + // This avoids style differences to show up as diffs but it can + // make the output different from the original template (since it is in normalized form) + log.Printf("Normalizing content: \n%s", content) + var object map[interface{}]interface{} + if err := yaml.Unmarshal([]byte(content), &object); err != nil { + return "", err + } + normalizedContent, err := yaml.Marshal(object) + if err != nil { + return "", err + } + return string(normalizedContent), nil +} + func isHook(metadata metadata, hooks ...string) bool { for _, hook := range hooks { if metadata.Metadata.Annotations[hookAnnotation] == hook { diff --git a/manifest/parse_test.go b/manifest/parse_test.go index 1ae65f28..e22a7cc5 100644 --- a/manifest/parse_test.go +++ b/manifest/parse_test.go @@ -111,3 +111,47 @@ func TestBaseNameAnnotation(t *testing.T) { foundObjects(Parse(string(spec), "default", false)), ) } +func TestContentNormalizeManifests(t *testing.T) { + tests := []struct { + name string + content string + expectedOutput string + expectedError error + }{ + { + name: "Valid content", + content: `apiVersion: v1 +kind: Pod +metadata: + name: my-pod +spec: + containers: + - name: my-container + image: nginx`, + expectedOutput: `apiVersion: v1 +kind: Pod +metadata: + name: my-pod +spec: + containers: + - image: nginx + name: my-container +`, + expectedError: nil, + }, + { + name: "Empty content", + content: "", + expectedOutput: "{}\n", + expectedError: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + output, err := ContentNormalizeManifests(tt.content) + require.Equal(t, tt.expectedError, err) + require.Equal(t, tt.expectedOutput, output) + }) + } +} From 194a3e428404c7f17ed971ac0918bdf4843c2f9c Mon Sep 17 00:00:00 2001 From: yxxhero Date: Mon, 15 Jun 2026 08:19:15 +0800 Subject: [PATCH 2/2] refactor: unexport normalize helper and harden tests - Rename ContentNormalizeManifests to unexported normalizeContent (avoids widening the public API; name avoids clash with the normalizeManifests bool parameter) - Move the test into an internal package (manifest) to access the unexported helper - Add an error-path case (sequence cannot unmarshal into map) - Use require.NoError/require.Error and direct []byte comparison Signed-off-by: yxxhero --- manifest/normalize_test.go | 62 ++++++++++++++++++++++++++++++++++++++ manifest/parse.go | 4 +-- manifest/parse_test.go | 44 --------------------------- 3 files changed, 64 insertions(+), 46 deletions(-) create mode 100644 manifest/normalize_test.go diff --git a/manifest/normalize_test.go b/manifest/normalize_test.go new file mode 100644 index 00000000..fb635fb0 --- /dev/null +++ b/manifest/normalize_test.go @@ -0,0 +1,62 @@ +package manifest + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestNormalizeContent(t *testing.T) { + tests := []struct { + name string + content []byte + expectedOutput []byte + expectError bool + }{ + { + name: "Valid content", + content: []byte(`apiVersion: v1 +kind: Pod +metadata: + name: my-pod +spec: + containers: + - name: my-container + image: nginx`), + expectedOutput: []byte(`apiVersion: v1 +kind: Pod +metadata: + name: my-pod +spec: + containers: + - image: nginx + name: my-container +`), + }, + { + // yaml.v2 marshals a nil map to "{}\n". Empty content never + // reaches this path in practice (Parse filters it earlier), + // but document the behavior regardless. + name: "Empty content", + content: []byte(""), + expectedOutput: []byte("{}\n"), + }, + { + name: "Sequence cannot unmarshal into map", + content: []byte("- a\n- b"), + expectError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + output, err := normalizeContent(tt.content) + if tt.expectError { + require.Error(t, err) + return + } + require.NoError(t, err) + require.Equal(t, tt.expectedOutput, output) + }) + } +} diff --git a/manifest/parse.go b/manifest/parse.go index 35aac2da..c6277f01 100644 --- a/manifest/parse.go +++ b/manifest/parse.go @@ -191,7 +191,7 @@ func parseContent(content []byte, defaultNamespace string, normalizeManifests bo if normalizeManifests { var normalizeErr error - content, normalizeErr = ContentNormalizeManifests(content) + content, normalizeErr = normalizeContent(content) if normalizeErr != nil { log.Fatalf("Error normalizing manifests: %v", normalizeErr) } @@ -216,7 +216,7 @@ func parseContent(content []byte, defaultNamespace string, normalizeManifests bo }, nil } -func ContentNormalizeManifests(content []byte) ([]byte, error) { +func normalizeContent(content []byte) ([]byte, error) { // Unmarshal and marshal again content to normalize yaml structure // This avoids style differences to show up as diffs but it can // make the output different from the original template (since it is in normalized form) diff --git a/manifest/parse_test.go b/manifest/parse_test.go index c960040d..efe50fd5 100644 --- a/manifest/parse_test.go +++ b/manifest/parse_test.go @@ -139,50 +139,6 @@ func TestBaseNameAnnotation(t *testing.T) { foundObjects(Parse(spec, "default", false)), ) } -func TestContentNormalizeManifests(t *testing.T) { - tests := []struct { - name string - content []byte - expectedOutput []byte - expectedError error - }{ - { - name: "Valid content", - content: []byte(`apiVersion: v1 -kind: Pod -metadata: - name: my-pod -spec: - containers: - - name: my-container - image: nginx`), - expectedOutput: []byte(`apiVersion: v1 -kind: Pod -metadata: - name: my-pod -spec: - containers: - - image: nginx - name: my-container -`), - expectedError: nil, - }, - { - name: "Empty content", - content: []byte(""), - expectedOutput: []byte("{}\n"), - expectedError: nil, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - output, err := ContentNormalizeManifests(tt.content) - require.Equal(t, tt.expectedError, err) - require.Equal(t, string(tt.expectedOutput), string(output)) - }) - } -} func TestParseObject(t *testing.T) { for _, tt := range []struct {