fix: make labels handle -f flag same as commonLabels

kustomize/commands/edit/add/addmetadata.go

@@ -139,12 +139,7 @@ func (o *addMetadataOptions) addAnnotations(m *types.Kustomization) error {
 
 func (o *addMetadataOptions) addLabels(m *types.Kustomization) error {
 	if o.labelsWithoutSelector {
-		m.Labels = append(m.Labels, types.Label{
+		return o.writeToLabels(m, label)
-			Pairs:            make(map[string]string),
-			IncludeSelectors: false,
-			IncludeTemplates: o.includeTemplates,
-		})
-		return o.writeToMap(m.Labels[len(m.Labels)-1].Pairs, label)
 	}
 	if m.CommonLabels == nil {
 		m.CommonLabels = make(map[string]string)
@@ -154,10 +149,67 @@ func (o *addMetadataOptions) addLabels(m *types.Kustomization) error {
 
 func (o *addMetadataOptions) writeToMap(m map[string]string, kind kindOfAdd) error {
 	for k, v := range o.metadata {
-		if _, ok := m[k]; ok && !o.force {
+		if err := o.writeToMapEntry(m, k, v, kind); err != nil {
-			return fmt.Errorf("%s %s already in kustomization file", kind, k)
+			return err
 		}
-		m[k] = v
 	}
 	return nil
 }
+
+func (o *addMetadataOptions) writeToMapEntry(m map[string]string, k, v string, kind kindOfAdd) error {
+	if _, ok := m[k]; ok && !o.force {
+		return fmt.Errorf("%s %s already in kustomization file. Use --force to override.", kind, k)
+	}
+	m[k] = v
+	return nil
+}
+
+func (o *addMetadataOptions) writeToLabels(m *types.Kustomization, kind kindOfAdd) error {
+	lbl := types.Label{
+		Pairs:            make(map[string]string),
+		IncludeSelectors: false,
+		IncludeTemplates: o.includeTemplates,
+	}
+	for k, v := range o.metadata {
+		if i, ok := o.findLabelKeyIndex(m, lbl, k); ok {
+			if err := o.writeToMapEntry(m.Labels[i].Pairs, k, v, kind); err != nil {
+				return err
+			}
+			continue
+		}
+		if i, ok := o.findLabelIndex(m, lbl); ok {
+			if err := o.writeToMapEntry(m.Labels[i].Pairs, k, v, kind); err != nil {
+				return err
+			}
+			continue
+		}
+		if err := o.writeToMap(lbl.Pairs, kind); err != nil {
+			return err
+		}
+		m.Labels = append(m.Labels, lbl)
+	}
+	return nil
+}
+
+func (o *addMetadataOptions) matchLabelSettings(lbl1, lbl2 types.Label) bool {
+	return lbl1.IncludeSelectors == lbl2.IncludeSelectors &&
+		lbl1.IncludeTemplates == lbl2.IncludeTemplates
+}
+
+func (o *addMetadataOptions) findLabelIndex(m *types.Kustomization, lbl types.Label) (int, bool) {
+	for i, ml := range m.Labels {
+		if o.matchLabelSettings(ml, lbl) {
+			return i, true
+		}
+	}
+	return 0, false
+}
+
+func (o *addMetadataOptions) findLabelKeyIndex(m *types.Kustomization, lbl types.Label, key string) (int, bool) {
+	if i, found := o.findLabelIndex(m, lbl); found {
+		if _, ok := m.Labels[i].Pairs[key]; ok {
+			return i, true
+		}
+	}
+	return 0, false
+}

kustomize/commands/edit/add/addmetadata_test.go

@@ -161,7 +161,7 @@ func TestAddAnnotationForce(t *testing.T) {
 	err := cmd.RunE(cmd, args)
 	v.VerifyCall()
 	require.Error(t, err)
-	assert.Equal(t, "annotation key already in kustomization file", err.Error())
+	assert.Equal(t, "annotation key already in kustomization file. Use --force to override.", err.Error())
 	// but trying to add it with --force should
 	v = valtest_test.MakeHappyMapValidator(t)
 	cmd = newCmdAddAnnotation(fSys, v.Validator)
@@ -265,7 +265,7 @@ func TestAddLabelForce(t *testing.T) {
 	err := cmd.RunE(cmd, args)
 	v.VerifyCall()
 	require.Error(t, err)
-	assert.Equal(t, "label key already in kustomization file", err.Error())
+	assert.Equal(t, "label key already in kustomization file. Use --force to override.", err.Error())
 	// but trying to add it with --force should
 	v = valtest_test.MakeHappyMapValidator(t)
 	cmd = newCmdAddLabel(fSys, v.Validator)
@@ -275,25 +275,6 @@ func TestAddLabelForce(t *testing.T) {
 	v.VerifyCall()
 }
 
-func TestAddLabelWithoutSelector(t *testing.T) {
-	var o addMetadataOptions
-	o.labelsWithoutSelector = true
-	m := makeKustomization(t)
-	o.metadata = map[string]string{"new": "label"}
-	require.NoError(t, o.addLabels(m))
-	assert.Equal(t, m.Labels[0], types.Label{Pairs: map[string]string{"new": "label"}})
-}
-
-func TestAddLabelWithoutSelectorIncludeTemplates(t *testing.T) {
-	var o addMetadataOptions
-	o.labelsWithoutSelector = true
-	m := makeKustomization(t)
-	o.metadata = map[string]string{"new": "label"}
-	o.includeTemplates = true
-	require.NoError(t, o.addLabels(m))
-	assert.Equal(t, m.Labels[0], types.Label{Pairs: map[string]string{"new": "label"}, IncludeTemplates: true})
-}
-
 func TestAddLabelIncludeTemplatesWithoutRequiredFlag(t *testing.T) {
 	fSys := filesys.MakeFsInMemory()
 	v := valtest_test.MakeHappyMapValidator(t)
@@ -307,17 +288,133 @@ func TestAddLabelIncludeTemplatesWithoutRequiredFlag(t *testing.T) {
 	require.Containsf(t, err.Error(), "--without-selector flag must be specified for --include-templates to work", "incorrect error: %s", err.Error())
 }
 
-func TestAddLabelWithoutSelectorAddLabel(t *testing.T) {
+func TestAddLabelWithoutSelector(t *testing.T) {
-	var o addMetadataOptions
+	tests := []struct {
-	o.metadata = map[string]string{"owls": "cute", "otters": "adorable"}
+		name        string
-	o.labelsWithoutSelector = true
+		baseLabels  []types.Label      // original labels
-
+		options     addMetadataOptions // add labels
-	m := makeKustomization(t)
+		metadata    map[string]string  // added labels
-	require.NoError(t, o.addLabels(m))
+		expected    []types.Label      // expected labels
-	// adding new labels should work
+		expectedErr string
-	o.metadata = map[string]string{"new": "label"}
+	}{
-	require.NoError(t, o.addLabels(m))
+		{
-
+			name:       "add to empty labels",
-	assert.Equal(t, m.Labels[0], types.Label{Pairs: map[string]string{"owls": "cute", "otters": "adorable"}})
+			baseLabels: []types.Label{},
-	assert.Equal(t, m.Labels[1], types.Label{Pairs: map[string]string{"new": "label"}})
+			options: addMetadataOptions{
+				labelsWithoutSelector: true,
+				metadata:              map[string]string{"new": "label"},
+			},
+			expected: []types.Label{
+				{
+					Pairs: map[string]string{"new": "label"},
+				},
+			},
+		},
+		{
+			name:       "add to empty labels with IncludeTemplates",
+			baseLabels: []types.Label{},
+			options: addMetadataOptions{
+				labelsWithoutSelector: true,
+				includeTemplates:      true,
+				metadata:              map[string]string{"new": "label"},
+			},
+			expected: []types.Label{
+				{
+					Pairs:            map[string]string{"new": "label"},
+					IncludeTemplates: true,
+				},
+			},
+		},
+		{
+			name: "overwrite label requires force",
+			baseLabels: []types.Label{
+				{
+					Pairs: map[string]string{"key1": "old-value1"},
+				},
+			},
+			options: addMetadataOptions{
+				labelsWithoutSelector: true,
+				force:                 false,
+				metadata:              map[string]string{"key1": "new-value1"},
+			},
+			expectedErr: "label key1 already in kustomization file",
+		},
+		{
+			name: "overwrite label",
+			baseLabels: []types.Label{
+				{
+					Pairs: map[string]string{"key1": "old-value1"},
+				},
+			},
+			options: addMetadataOptions{
+				labelsWithoutSelector: true,
+				force:                 true,
+				metadata:              map[string]string{"key1": "new-value1"},
+			},
+			expected: []types.Label{
+				{
+					Pairs: map[string]string{"key1": "new-value1"},
+				},
+			},
+		},
+		{
+			name: "overwrite and add label",
+			baseLabels: []types.Label{
+				{
+					Pairs: map[string]string{"key1": "old-value1"},
+				},
+			},
+			options: addMetadataOptions{
+				labelsWithoutSelector: true,
+				force:                 true,
+				metadata:              map[string]string{"key1": "new-value1", "key2": "value2"},
+			},
+			expected: []types.Label{
+				{
+					Pairs: map[string]string{"key1": "new-value1", "key2": "value2"},
+				},
+			},
+		},
+		{
+			name: "overwrite label with same settings",
+			baseLabels: []types.Label{
+				{
+					Pairs: map[string]string{"key": "old-value"},
+				},
+				{
+					IncludeTemplates: true,
+					Pairs:            map[string]string{"key": "old-value"},
+				},
+			},
+			options: addMetadataOptions{
+				labelsWithoutSelector: true,
+				force:                 true,
+				metadata:              map[string]string{"key": "new-value"},
+				includeTemplates:      true,
+			},
+			expected: []types.Label{
+				{
+					Pairs: map[string]string{"key": "old-value"},
+				},
+				{
+					IncludeTemplates: true,
+					Pairs:            map[string]string{"key": "new-value"},
+				},
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			m := makeKustomization(t)
+			m.Labels = tt.baseLabels
+			err := tt.options.addLabels(m)
+			if tt.expectedErr != "" {
+				require.Error(t, err)
+				assert.Contains(t, err.Error(), tt.expectedErr)
+			} else {
+				require.NoError(t, err)
+				assert.Equal(t, tt.expected, m.Labels)
+			}
+		})
+	}
 }