Merge pull request #4923 from koba1t/chore/remove_FixKustomizationPreUnmarshalling

Remove fix kustomization step before Unmarshalling the yaml structure

api/internal/localizer/localizer.go

@@ -101,10 +101,7 @@ func (lc *localizer) load() (*types.Kustomization, string, error) {
 	if err != nil {
 		return nil, "", errors.Wrap(err)
 	}
-	content, err = types.FixKustomizationPreUnmarshalling(content)
-	if err != nil {
-		return nil, "", errors.WrapPrefixf(err, "invalid kustomization")
-	}
+
 	var kust types.Kustomization
 	err = (&kust).Unmarshal(content)
 	if err != nil {

api/internal/localizer/localizer_test.go

@@ -210,7 +210,7 @@ bases:
 - beta
 configMapGenerator:
 - env: env.properties
-images:
+imageTags:
 - name: postgres
   newName: my-registry/my-postgres
   newTag: v1

api/internal/target/kusttarget.go

@@ -58,25 +58,21 @@ func (kt *KustTarget) Load() error {
 	if err != nil {
 		return err
 	}
-	content, err = types.FixKustomizationPreUnmarshalling(content)
-	if err != nil {
-		return err
-	}
+
 	var k types.Kustomization
-	err = k.Unmarshal(content)
-	if err != nil {
+	if err := k.Unmarshal(content); err != nil {
 		return err
 	}
 
 	// show warning message when using deprecated fields.
-	warningMessages := k.CheckDeprecatedFields()
-	if warningMessages != nil {
+	if warningMessages := k.CheckDeprecatedFields(); warningMessages != nil {
 		for _, msg := range *warningMessages {
 			fmt.Fprintf(os.Stderr, "%v\n", msg)
 		}
 	}
 
-	k.FixKustomizationPostUnmarshalling()
+	k.FixKustomization()
+
 	errs := k.EnforceFields()
 	if len(errs) > 0 {
 		return fmt.Errorf(

api/types/fix.go

@@ -1,22 +0,0 @@
-// Copyright 2019 The Kubernetes Authors.
-// SPDX-License-Identifier: Apache-2.0
-
-package types
-
-import (
-	"regexp"
-)
-
-// FixKustomizationPreUnmarshalling modifies the raw data
-// before marshalling - e.g. changes old field names to
-// new field names.
-func FixKustomizationPreUnmarshalling(data []byte) ([]byte, error) {
-	deprecatedFieldsMap := map[string]string{
-		"imageTags:": "images:",
-	}
-	for oldname, newname := range deprecatedFieldsMap {
-		pattern := regexp.MustCompile(oldname)
-		data = pattern.ReplaceAll(data, []byte(newname))
-	}
-	return data, nil
-}

api/types/kustomization.go

@@ -83,6 +83,9 @@ type Kustomization struct {
 	// patch, but this operator is simpler to specify.
 	Images []Image `json:"images,omitempty" yaml:"images,omitempty"`
 
+	// Deprecated: Use the Images field instead.
+	ImageTags []Image `json:"imageTags,omitempty" yaml:"imageTags,omitempty"`
+
 	// Replacements is a list of replacements, which will copy nodes from a
 	// specified source to N specified targets.
 	Replacements []ReplacementField `json:"replacements,omitempty" yaml:"replacements,omitempty"`
@@ -181,6 +184,7 @@ const (
 	deprecatedWarningToRunEditFix              = "Run 'kustomize edit fix' to update your Kustomization automatically."
 	deprecatedWarningToRunEditFixExperimential = "[EXPERIMENTAL] Run 'kustomize edit fix' to update your Kustomization automatically."
 	deprecatedBaseWarningMessage               = "# Warning: 'bases' is deprecated. Please use 'resources' instead." + " " + deprecatedWarningToRunEditFix
+	deprecatedImageTagsWarningMessage          = "# Warning: 'imageTags' is deprecated. Please use 'images' instead." + " " + deprecatedWarningToRunEditFix
 	deprecatedPatchesJson6902Message           = "# Warning: 'patchesJson6902' is deprecated. Please use 'patches' instead." + " " + deprecatedWarningToRunEditFix
 	deprecatedPatchesStrategicMergeMessage     = "# Warning: 'patchesStrategicMerge' is deprecated. Please use 'patches' instead." + " " + deprecatedWarningToRunEditFix
 	deprecatedVarsMessage                      = "# Warning: 'vars' is deprecated. Please use 'replacements' instead." + " " + deprecatedWarningToRunEditFixExperimential
@@ -192,6 +196,9 @@ func (k *Kustomization) CheckDeprecatedFields() *[]string {
 	if k.Bases != nil {
 		warningMessages = append(warningMessages, deprecatedBaseWarningMessage)
 	}
+	if k.ImageTags != nil {
+		warningMessages = append(warningMessages, deprecatedImageTagsWarningMessage)
+	}
 	if k.PatchesJson6902 != nil {
 		warningMessages = append(warningMessages, deprecatedPatchesJson6902Message)
 	}
@@ -204,11 +211,11 @@ func (k *Kustomization) CheckDeprecatedFields() *[]string {
 	return &warningMessages
 }
 
-// FixKustomizationPostUnmarshalling fixes things
+// FixKustomization fixes things
 // like empty fields that should not be empty, or
 // moving content of deprecated fields to newer
 // fields.
-func (k *Kustomization) FixKustomizationPostUnmarshalling() {
+func (k *Kustomization) FixKustomization() {
 	if k.Kind == "" {
 		k.Kind = KustomizationKind
 	}
@@ -224,6 +231,10 @@ func (k *Kustomization) FixKustomizationPostUnmarshalling() {
 	k.Resources = append(k.Resources, k.Bases...)
 	k.Bases = nil
 
+	// 'imageTags' field was deprecated in favor of the 'images' field.
+	k.Images = append(k.Images, k.ImageTags...)
+	k.ImageTags = nil
+
 	for i, g := range k.ConfigMapGenerator {
 		if g.EnvSource != "" {
 			k.ConfigMapGenerator[i].EnvSources =

api/types/kustomization_test.go

@@ -29,6 +29,13 @@ func TestKustomization_CheckDeprecatedFields(t *testing.T) {
 			},
 			want: &[]string{deprecatedBaseWarningMessage},
 		},
+		{
+			name: "using_ImageTags",
+			k: Kustomization{
+				ImageTags: []Image{},
+			},
+			want: &[]string{deprecatedImageTagsWarningMessage},
+		},
 		{
 			name: "usingPatchesJson6902",
 			k: Kustomization{
@@ -54,12 +61,14 @@ func TestKustomization_CheckDeprecatedFields(t *testing.T) {
 			name: "usingAll",
 			k: Kustomization{
 				Bases:                 []string{"base"},
+				ImageTags:             []Image{},
 				PatchesJson6902:       []Patch{},
 				PatchesStrategicMerge: []PatchStrategicMerge{},
 				Vars:                  []Var{},
 			},
 			want: &[]string{
 				deprecatedBaseWarningMessage,
+				deprecatedImageTagsWarningMessage,
 				deprecatedPatchesJson6902Message,
 				deprecatedPatchesStrategicMergeMessage,
 				deprecatedVarsMessage,
@@ -88,7 +97,7 @@ func TestFixKustomizationPostUnmarshalling(t *testing.T) {
 	k.CommonLabels = map[string]string{
 		"foo": "bar",
 	}
-	k.FixKustomizationPostUnmarshalling()
+	k.FixKustomization()
 
 	expected := Kustomization{
 		TypeMeta: TypeMeta{
@@ -120,7 +129,7 @@ func TestFixKustomizationPostUnmarshalling_2(t *testing.T) {
 		},
 	}
 	k.Bases = append(k.Bases, "foo")
-	k.FixKustomizationPostUnmarshalling()
+	k.FixKustomization()
 
 	expected := Kustomization{
 		TypeMeta: TypeMeta{

kustomize/commands/internal/kustfile/kustomizationfile.go

@@ -165,21 +165,18 @@ func (mf *kustomizationFile) Read() (*types.Kustomization, error) {
 	if err != nil {
 		return nil, err
 	}
-	data, err = types.FixKustomizationPreUnmarshalling(data)
-	if err != nil {
-		return nil, err
-	}
+
 	var k types.Kustomization
-	err = k.Unmarshal(data)
-	if err != nil {
+	if err := k.Unmarshal(data); err != nil {
 		return nil, err
 	}
-	k.FixKustomizationPostUnmarshalling()
-	err = mf.parseCommentedFields(data)
-	if err != nil {
+
+	k.FixKustomization()
+
+	if err := mf.parseCommentedFields(data); err != nil {
 		return nil, err
 	}
-	return &k, err
+	return &k, nil
 }
 
 func (mf *kustomizationFile) Write(kustomization *types.Kustomization) error {
@@ -264,12 +261,13 @@ func (mf *kustomizationFile) hasField(name string) bool {
 }
 
 /*
- isCommentOrBlankLine determines if a line is a comment or blank line
- Return true for following lines
- # This line is a comment
-       # This line is also a comment with several leading white spaces
+isCommentOrBlankLine determines if a line is a comment or blank line
+Return true for following lines
+# This line is a comment
 
- (The line above is a blank line)
+	# This line is also a comment with several leading white spaces
+
+(The line above is a blank line)
 */
 func isCommentOrBlankLine(line []byte) bool {
 	s := bytes.TrimRight(bytes.TrimLeft(line, " "), "\n")

kustomize/commands/internal/kustfile/kustomizationfile_test.go

@@ -82,7 +82,7 @@ func TestWriteAndRead(t *testing.T) {
 	if err != nil {
 		t.Fatalf("Couldn't read kustomization file: %v\n", err)
 	}
-	kustomization.FixKustomizationPostUnmarshalling()
+	kustomization.FixKustomization()
 	if !reflect.DeepEqual(kustomization, content) {
 		t.Fatal("Read kustomization is different from written kustomization")
 	}
@@ -189,7 +189,7 @@ patchesStrategicMerge:
 func TestPreserveCommentsWithAdjust(t *testing.T) {
 	kustomizationContentWithComments := []byte(`
 
-    
+
 
 # Some comments
 # This is some comment we should preserve
@@ -225,7 +225,7 @@ generatorOptions:
 
 	expected := []byte(`
 
-    
+
 
 # Some comments
 # This is some comment we should preserve
@@ -326,7 +326,7 @@ kind: Kustomization
 func TestCommentsWithDocumentSeperatorAtBeginning(t *testing.T) {
 	kustomizationContentWithComments := []byte(`
 
-    
+
 # Some comments
 # This is some comment we should preserve
 # don't delete it
@@ -339,7 +339,7 @@ namespace: mynamespace
 
 	expected := []byte(`
 
-    
+
 # Some comments
 # This is some comment we should preserve
 # don't delete it