Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: "edit add labels" make labels handle -f flag same as commonLabels #5809

Merged
merged 1 commit into from
Dec 13, 2024
Merged

fix: "edit add labels" make labels handle -f flag same as commonLabels #5809

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 61 additions & 9 deletions kustomize/commands/edit/add/addmetadata.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
}
165 changes: 131 additions & 34 deletions kustomize/commands/edit/add/addmetadata_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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)
}
})
}
}