From ccb95ab2695945069b2c23c7db6cd17d7fc061d2 Mon Sep 17 00:00:00 2001
From: Donny Xia <donnyxia@google.com>
Date: Wed, 24 Mar 2021 10:41:37 -0700
Subject: [PATCH] add conflict check

---
 api/types/kustomization.go              | 20 +++++++++++++++++---
 kustomize/commands/edit/fix/fix.go      |  5 ++++-
 kustomize/commands/edit/fix/fix_test.go | 23 +++++++++++++++++++++++
 3 files changed, 44 insertions(+), 4 deletions(-)

diff --git a/api/types/kustomization.go b/api/types/kustomization.go
index da019f075..95f47da9a 100644
--- a/api/types/kustomization.go
+++ b/api/types/kustomization.go
@@ -6,6 +6,7 @@ package types
 import (
 	"bytes"
 	"encoding/json"
+	"fmt"
 
 	"sigs.k8s.io/yaml"
 )
@@ -189,15 +190,28 @@ func (k *Kustomization) FixKustomizationPostUnmarshalling() {
 // FixKustomizationPreMarshalling fixes things
 // that should occur after the kustomization file
 // has been processed.
-func (k *Kustomization) FixKustomizationPreMarshalling() {
+func (k *Kustomization) FixKustomizationPreMarshalling() error {
 	// PatchesJson6902 should be under the Patches field.
 	k.Patches = append(k.Patches, k.PatchesJson6902...)
 	k.PatchesJson6902 = nil
 
-	if l := labelFromCommonLabels(k.CommonLabels); l != nil {
-		k.Labels = append(k.Labels, *l)
+	// this fix is not in FixKustomizationPostUnmarshalling because
+	// it will break some commands like `create` and `add`. those
+	// commands depend on 'commonLabels' field
+	if cl := labelFromCommonLabels(k.CommonLabels); cl != nil {
+		// check conflicts between commonLabels and labels
+		for _, l := range k.Labels {
+			for k := range l.Pairs {
+				if _, exist := cl.Pairs[k]; exist {
+					return fmt.Errorf("label name '%s' exists in both commonLabels and labels", k)
+				}
+			}
+		}
+		k.Labels = append(k.Labels, *cl)
 		k.CommonLabels = nil
 	}
+
+	return nil
 }
 
 func (k *Kustomization) EnforceFields() []string {
diff --git a/kustomize/commands/edit/fix/fix.go b/kustomize/commands/edit/fix/fix.go
index 11aef8a38..1149e6908 100644
--- a/kustomize/commands/edit/fix/fix.go
+++ b/kustomize/commands/edit/fix/fix.go
@@ -39,6 +39,9 @@ func RunFix(fSys filesys.FileSystem) error {
 		return err
 	}
 
-	m.FixKustomizationPreMarshalling()
+	err = m.FixKustomizationPreMarshalling()
+	if err != nil {
+		return err
+	}
 	return mf.Write(m)
 }
diff --git a/kustomize/commands/edit/fix/fix_test.go b/kustomize/commands/edit/fix/fix_test.go
index c3195b9c7..92f249965 100644
--- a/kustomize/commands/edit/fix/fix_test.go
+++ b/kustomize/commands/edit/fix/fix_test.go
@@ -175,3 +175,26 @@ kind: Kustomization
 		t.Errorf("Mismatch (-expected, +actual):\n%s", diff)
 	}
 }
+
+func TestFixOutdatedCommonLabelsDuplicate(t *testing.T) {
+	kustomizationContentWithOutdatedCommonLabels := []byte(`
+commonLabels:
+  foo: bar
+labels:
+- pairs:
+    foo: baz
+    a: b
+`)
+
+	fSys := filesys.MakeFsInMemory()
+	testutils_test.WriteTestKustomizationWith(fSys, kustomizationContentWithOutdatedCommonLabels)
+	cmd := NewCmdFix(fSys)
+	err := cmd.RunE(cmd, nil)
+	if err == nil {
+		t.Fatalf("expect error")
+	}
+	expectedErr := "label name 'foo' exists in both commonLabels and labels"
+	if err.Error() != expectedErr {
+		t.Fatalf("error message '%s' doesn't match expected", err.Error())
+	}
+}