From 70e4d2e24c77b2be5e90ff62624ffc9eabe354c1 Mon Sep 17 00:00:00 2001 From: fktym <1465910+fktym@users.noreply.github.com> Date: Thu, 12 Dec 2024 14:02:53 +0900 Subject: [PATCH] fix: make labels handle -f flag same as commonLabels --- kustomize/commands/edit/add/addmetadata.go | 70 +++++++- .../commands/edit/add/addmetadata_test.go | 165 ++++++++++++++---- 2 files changed, 192 insertions(+), 43 deletions(-) diff --git a/kustomize/commands/edit/add/addmetadata.go b/kustomize/commands/edit/add/addmetadata.go index 93cf91a006..25b1a5aaca 100644 --- a/kustomize/commands/edit/add/addmetadata.go +++ b/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{ - Pairs: make(map[string]string), - IncludeSelectors: false, - IncludeTemplates: o.includeTemplates, - }) - return o.writeToMap(m.Labels[len(m.Labels)-1].Pairs, label) + return o.writeToLabels(m, 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 { - return fmt.Errorf("%s %s already in kustomization file", kind, k) + if err := o.writeToMapEntry(m, k, v, kind); err != nil { + 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 +} diff --git a/kustomize/commands/edit/add/addmetadata_test.go b/kustomize/commands/edit/add/addmetadata_test.go index ec840ffb3d..9a52e133e0 100644 --- a/kustomize/commands/edit/add/addmetadata_test.go +++ b/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) { - var o addMetadataOptions - o.metadata = map[string]string{"owls": "cute", "otters": "adorable"} - o.labelsWithoutSelector = true - - m := makeKustomization(t) - require.NoError(t, o.addLabels(m)) - // adding new labels should work - 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{"owls": "cute", "otters": "adorable"}}) - assert.Equal(t, m.Labels[1], types.Label{Pairs: map[string]string{"new": "label"}}) +func TestAddLabelWithoutSelector(t *testing.T) { + tests := []struct { + name string + baseLabels []types.Label // original labels + options addMetadataOptions // add labels + metadata map[string]string // added labels + expected []types.Label // expected labels + expectedErr string + }{ + { + name: "add to empty labels", + baseLabels: []types.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) + } + }) + } }