Skip to content

Commit b908027

Browse files
authored
fix normalieze manifests issue (#585)
* fix normalieze manifests issue Signed-off-by: yxxhero <aiopsclub@163.com> * 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 <aiopsclub@163.com> --------- Signed-off-by: yxxhero <aiopsclub@163.com>
1 parent 5546534 commit b908027

2 files changed

Lines changed: 81 additions & 8 deletions

File tree

manifest/normalize_test.go

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
package manifest
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/require"
7+
)
8+
9+
func TestNormalizeContent(t *testing.T) {
10+
tests := []struct {
11+
name string
12+
content []byte
13+
expectedOutput []byte
14+
expectError bool
15+
}{
16+
{
17+
name: "Valid content",
18+
content: []byte(`apiVersion: v1
19+
kind: Pod
20+
metadata:
21+
name: my-pod
22+
spec:
23+
containers:
24+
- name: my-container
25+
image: nginx`),
26+
expectedOutput: []byte(`apiVersion: v1
27+
kind: Pod
28+
metadata:
29+
name: my-pod
30+
spec:
31+
containers:
32+
- image: nginx
33+
name: my-container
34+
`),
35+
},
36+
{
37+
// yaml.v2 marshals a nil map to "{}\n". Empty content never
38+
// reaches this path in practice (Parse filters it earlier),
39+
// but document the behavior regardless.
40+
name: "Empty content",
41+
content: []byte(""),
42+
expectedOutput: []byte("{}\n"),
43+
},
44+
{
45+
name: "Sequence cannot unmarshal into map",
46+
content: []byte("- a\n- b"),
47+
expectError: true,
48+
},
49+
}
50+
51+
for _, tt := range tests {
52+
t.Run(tt.name, func(t *testing.T) {
53+
output, err := normalizeContent(tt.content)
54+
if tt.expectError {
55+
require.Error(t, err)
56+
return
57+
}
58+
require.NoError(t, err)
59+
require.Equal(t, tt.expectedOutput, output)
60+
})
61+
}
62+
}

manifest/parse.go

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -190,15 +190,11 @@ func parseContent(content []byte, defaultNamespace string, normalizeManifests bo
190190
}
191191

192192
if normalizeManifests {
193-
var object map[interface{}]interface{}
194-
if err := yaml.Unmarshal(content, &object); err != nil {
195-
log.Fatalf("YAML unmarshal error: %s\nCan't unmarshal %s", err, content)
196-
}
197-
normalizedContent, err := yaml.Marshal(object)
198-
if err != nil {
199-
log.Fatalf("YAML marshal error: %s\nCan't marshal %v", err, object)
193+
var normalizeErr error
194+
content, normalizeErr = normalizeContent(content)
195+
if normalizeErr != nil {
196+
log.Fatalf("Error normalizing manifests: %v", normalizeErr)
200197
}
201-
content = normalizedContent
202198
}
203199

204200
if isHook(parsedMetadata, excludedHooks...) {
@@ -220,6 +216,21 @@ func parseContent(content []byte, defaultNamespace string, normalizeManifests bo
220216
}, nil
221217
}
222218

219+
func normalizeContent(content []byte) ([]byte, error) {
220+
// Unmarshal and marshal again content to normalize yaml structure
221+
// This avoids style differences to show up as diffs but it can
222+
// make the output different from the original template (since it is in normalized form)
223+
var object map[interface{}]interface{}
224+
if err := yaml.Unmarshal(content, &object); err != nil {
225+
return nil, err
226+
}
227+
normalizedContent, err := yaml.Marshal(object)
228+
if err != nil {
229+
return nil, err
230+
}
231+
return normalizedContent, nil
232+
}
233+
223234
func isHook(metadata metadata, hooks ...string) bool {
224235
for _, hook := range hooks {
225236
if metadata.Metadata.Annotations[hookAnnotation] == hook {

0 commit comments

Comments
 (0)