Skip to content

Commit 671de16

Browse files
feat: support labels key in transformer configuration (#5556)
* feat: support labels key in transformer configuration Allow the usage of a separate transformer configuration for the labels key, similar to what is currently available for commonLabels and commonAnnotations. This aims to provide the same functionality that commonLabels currently provide for labels, since commonLabels is deprecated and slated for removal in a future release. * chore(transformerconfig): add nolint hint Add a nolint hint to the new method so the returns can stay consistent with one another. * fix: changes from code review * Rename methods `AddCommonLabelFieldSpec` and `AddLabelFieldSpec` to `AddCommonLabelsFieldSpec` and `AddLabelsFieldSpec`. * Add extra test to verify scenarios applying labels to Custom Resource Definitions.
1 parent 2e6171a commit 671de16

File tree

8 files changed

+478
-120
lines changed

8 files changed

+478
-120
lines changed

api/internal/accumulator/loadconfigfromcrds.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ func loadCrdIntoConfig(
144144
}
145145
_, label := property.Extensions.GetString(xLabelSelector)
146146
if label {
147-
err = theConfig.AddLabelFieldSpec(
147+
err = theConfig.AddCommonLabelsFieldSpec(
148148
makeFs(theGvk, append(path, propName)))
149149
if err != nil {
150150
return

api/internal/plugins/builtinconfig/loaddefaultconfig_test.go

Lines changed: 0 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -70,30 +70,3 @@ namoPrefix:
7070
t.Fatalf("expected error %s, but got %s", errMsg, err)
7171
}
7272
}
73-
74-
// please remove this failing test after implements the labels support
75-
func TestLoadDefaultConfigsFromFilesWithMissingFieldsLabels(t *testing.T) {
76-
fSys := filesys.MakeFsInMemory()
77-
filePathContainsTypo := "config_contains_typo.yaml"
78-
if err := fSys.WriteFile(filePathContainsTypo, []byte(`
79-
labels:
80-
- path: spec/podTemplate/metadata/labels
81-
create: true
82-
kind: FlinkDeployment
83-
`)); err != nil {
84-
t.Fatal(err)
85-
}
86-
ldr, err := loader.NewLoader(
87-
loader.RestrictionRootOnly, filesys.Separator, fSys)
88-
if err != nil {
89-
t.Fatal(err)
90-
}
91-
errMsg := "error unmarshaling JSON: while decoding JSON: json: unknown field"
92-
_, err = loadDefaultConfig(ldr, []string{filePathContainsTypo})
93-
if err == nil {
94-
t.Fatalf("expected to fail unmarshal yaml, but got nil %s", filePathContainsTypo)
95-
}
96-
if !strings.Contains(err.Error(), errMsg) {
97-
t.Fatalf("expected error %s, but got %s", errMsg, err)
98-
}
99-
}

api/internal/plugins/builtinconfig/transformerconfig.go

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ type TransformerConfig struct {
2121
NameSuffix types.FsSlice `json:"nameSuffix,omitempty" yaml:"nameSuffix,omitempty"`
2222
NameSpace types.FsSlice `json:"namespace,omitempty" yaml:"namespace,omitempty"`
2323
CommonLabels types.FsSlice `json:"commonLabels,omitempty" yaml:"commonLabels,omitempty"`
24+
Labels types.FsSlice `json:"labels,omitempty" yaml:"labels,omitempty"`
2425
TemplateLabels types.FsSlice `json:"templateLabels,omitempty" yaml:"templateLabels,omitempty"`
2526
CommonAnnotations types.FsSlice `json:"commonAnnotations,omitempty" yaml:"commonAnnotations,omitempty"`
2627
NameReference nbrSlice `json:"nameReference,omitempty" yaml:"nameReference,omitempty"`
@@ -41,6 +42,7 @@ func (t *TransformerConfig) DeepCopy() *TransformerConfig {
4142
NameSuffix: t.NameSuffix.DeepCopy(),
4243
NameSpace: t.NameSpace.DeepCopy(),
4344
CommonLabels: t.CommonLabels.DeepCopy(),
45+
Labels: t.Labels.DeepCopy(),
4446
TemplateLabels: t.TemplateLabels.DeepCopy(),
4547
CommonAnnotations: t.CommonAnnotations.DeepCopy(),
4648
NameReference: t.NameReference.DeepCopy(),
@@ -94,6 +96,7 @@ func (t *TransformerConfig) sortFields() {
9496
sort.Sort(t.NameSuffix)
9597
sort.Sort(t.NameSpace)
9698
sort.Sort(t.CommonLabels)
99+
sort.Sort(t.Labels)
97100
sort.Sort(t.TemplateLabels)
98101
sort.Sort(t.CommonAnnotations)
99102
sort.Sort(t.NameReference)
@@ -114,12 +117,18 @@ func (t *TransformerConfig) AddSuffixFieldSpec(fs types.FieldSpec) (err error) {
114117
return err
115118
}
116119

117-
// AddLabelFieldSpec adds a FieldSpec to CommonLabels
118-
func (t *TransformerConfig) AddLabelFieldSpec(fs types.FieldSpec) (err error) {
120+
// AddCommonLabelsFieldSpec adds a FieldSpec to CommonLabels
121+
func (t *TransformerConfig) AddCommonLabelsFieldSpec(fs types.FieldSpec) (err error) {
119122
t.CommonLabels, err = t.CommonLabels.MergeOne(fs)
120123
return err
121124
}
122125

126+
// AddLabelsFieldSpec adds a FieldSpec to Labels
127+
func (t *TransformerConfig) AddLabelsFieldSpec(fs types.FieldSpec) (err error) {
128+
t.Labels, err = t.Labels.MergeOne(fs)
129+
return err //nolint:wrapcheck
130+
}
131+
123132
// AddAnnotationFieldSpec adds a FieldSpec to CommonAnnotations
124133
func (t *TransformerConfig) AddAnnotationFieldSpec(fs types.FieldSpec) (err error) {
125134
t.CommonAnnotations, err = t.CommonAnnotations.MergeOne(fs)
@@ -162,6 +171,10 @@ func (t *TransformerConfig) Merge(input *TransformerConfig) (
162171
if err != nil {
163172
return nil, errors.WrapPrefixf(err, "failed to merge CommonLabels fieldSpec")
164173
}
174+
merged.Labels, err = t.Labels.MergeAll(input.Labels)
175+
if err != nil {
176+
return nil, errors.WrapPrefixf(err, "failed to merge Labels fieldSpec")
177+
}
165178
merged.TemplateLabels, err = t.TemplateLabels.MergeAll(input.TemplateLabels)
166179
if err != nil {
167180
return nil, errors.WrapPrefixf(err, "failed to merge TemplateLabels fieldSpec")

api/internal/plugins/builtinconfig/transformerconfig_test.go

Lines changed: 42 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,10 @@
44
package builtinconfig_test
55

66
import (
7-
"reflect"
87
"testing"
98

9+
"github.com/stretchr/testify/assert"
10+
"github.com/stretchr/testify/require"
1011
. "sigs.k8s.io/kustomize/api/internal/plugins/builtinconfig"
1112
"sigs.k8s.io/kustomize/api/types"
1213
"sigs.k8s.io/kustomize/kyaml/resid"
@@ -35,13 +36,8 @@ func TestAddNamereferenceFieldSpec(t *testing.T) {
3536
},
3637
}
3738

38-
err := cfg.AddNamereferenceFieldSpec(nbrs)
39-
if err != nil {
40-
t.Fatalf("unexpected err: %v", err)
41-
}
42-
if len(cfg.NameReference) != 1 {
43-
t.Fatal("failed to add namereference FieldSpec")
44-
}
39+
require.NoError(t, cfg.AddNamereferenceFieldSpec(nbrs))
40+
require.Len(t, cfg.NameReference, 1, "failed to add namereference FieldSpec")
4541
}
4642

4743
func TestAddFieldSpecs(t *testing.T) {
@@ -53,34 +49,14 @@ func TestAddFieldSpecs(t *testing.T) {
5349
CreateIfNotPresent: true,
5450
}
5551

56-
err := cfg.AddPrefixFieldSpec(fieldSpec)
57-
if err != nil {
58-
t.Fatalf("unexpected err: %v", err)
59-
}
60-
if len(cfg.NamePrefix) != 1 {
61-
t.Fatalf("failed to add nameprefix FieldSpec")
62-
}
63-
err = cfg.AddSuffixFieldSpec(fieldSpec)
64-
if err != nil {
65-
t.Fatalf("unexpected err: %v", err)
66-
}
67-
if len(cfg.NameSuffix) != 1 {
68-
t.Fatalf("failed to add namesuffix FieldSpec")
69-
}
70-
err = cfg.AddLabelFieldSpec(fieldSpec)
71-
if err != nil {
72-
t.Fatalf("unexpected err: %v", err)
73-
}
74-
if len(cfg.CommonLabels) != 1 {
75-
t.Fatalf("failed to add nameprefix FieldSpec")
76-
}
77-
err = cfg.AddAnnotationFieldSpec(fieldSpec)
78-
if err != nil {
79-
t.Fatalf("unexpected err: %v", err)
80-
}
81-
if len(cfg.CommonAnnotations) != 1 {
82-
t.Fatalf("failed to add nameprefix FieldSpec")
83-
}
52+
require.NoError(t, cfg.AddPrefixFieldSpec(fieldSpec))
53+
require.Len(t, cfg.NamePrefix, 1, "failed to add nameprefix FieldSpec")
54+
require.NoError(t, cfg.AddSuffixFieldSpec(fieldSpec))
55+
require.Len(t, cfg.NameSuffix, 1, "failed to add namesuffix FieldSpec")
56+
require.NoError(t, cfg.AddCommonLabelsFieldSpec(fieldSpec))
57+
require.Len(t, cfg.CommonLabels, 1, "failed to add labels FieldSpec")
58+
require.NoError(t, cfg.AddAnnotationFieldSpec(fieldSpec))
59+
require.Len(t, cfg.CommonAnnotations, 1, "failed to add nameprefix FieldSpec")
8460
}
8561

8662
func TestMerge(t *testing.T) {
@@ -127,51 +103,43 @@ func TestMerge(t *testing.T) {
127103
},
128104
}
129105
cfga := &TransformerConfig{}
130-
cfga.AddNamereferenceFieldSpec(nameReference[0])
131-
cfga.AddPrefixFieldSpec(fieldSpecs[0])
132-
cfga.AddSuffixFieldSpec(fieldSpecs[0])
106+
require.NoError(t, cfga.AddNamereferenceFieldSpec(nameReference[0]))
107+
require.NoError(t, cfga.AddPrefixFieldSpec(fieldSpecs[0]))
108+
require.NoError(t, cfga.AddSuffixFieldSpec(fieldSpecs[0]))
109+
require.NoError(t, cfga.AddCommonLabelsFieldSpec(fieldSpecs[0]))
110+
require.NoError(t, cfga.AddLabelsFieldSpec(fieldSpecs[0]))
133111

134112
cfgb := &TransformerConfig{}
135-
cfgb.AddNamereferenceFieldSpec(nameReference[1])
136-
cfgb.AddPrefixFieldSpec(fieldSpecs[1])
137-
cfga.AddSuffixFieldSpec(fieldSpecs[1])
113+
require.NoError(t, cfgb.AddNamereferenceFieldSpec(nameReference[1]))
114+
require.NoError(t, cfgb.AddPrefixFieldSpec(fieldSpecs[1]))
115+
require.NoError(t, cfgb.AddSuffixFieldSpec(fieldSpecs[1]))
116+
require.NoError(t, cfgb.AddCommonLabelsFieldSpec(fieldSpecs[1]))
117+
require.NoError(t, cfgb.AddLabelsFieldSpec(fieldSpecs[1]))
138118

139119
actual, err := cfga.Merge(cfgb)
140-
if err != nil {
141-
t.Fatalf("unexpected err: %v", err)
142-
}
143-
144-
if len(actual.NamePrefix) != 2 {
145-
t.Fatal("merge failed for namePrefix FieldSpec")
146-
}
147-
148-
if len(actual.NameSuffix) != 2 {
149-
t.Fatal("merge failed for nameSuffix FieldSpec")
150-
}
151-
152-
if len(actual.NameReference) != 1 {
153-
t.Fatal("merge failed for namereference FieldSpec")
154-
}
120+
require.NoError(t, err)
121+
require.Len(t, actual.NamePrefix, 2, "merge failed for namePrefix FieldSpec")
122+
require.Len(t, actual.NameSuffix, 2, "merge failed for nameSuffix FieldSpec")
123+
require.Len(t, actual.NameReference, 1, "merge failed for nameReference FieldSpec")
124+
require.Len(t, actual.Labels, 2, "merge failed for labels FieldSpec")
125+
require.Len(t, actual.CommonLabels, 2, "merge failed for commonLabels FieldSpec")
155126

156127
expected := &TransformerConfig{}
157-
expected.AddNamereferenceFieldSpec(nameReference[0])
158-
expected.AddNamereferenceFieldSpec(nameReference[1])
159-
expected.AddPrefixFieldSpec(fieldSpecs[0])
160-
expected.AddPrefixFieldSpec(fieldSpecs[1])
161-
expected.AddSuffixFieldSpec(fieldSpecs[0])
162-
expected.AddSuffixFieldSpec(fieldSpecs[1])
163-
164-
if !reflect.DeepEqual(actual, expected) {
165-
t.Fatalf("expected: %v\n but got: %v\n", expected, actual)
166-
}
128+
require.NoError(t, expected.AddNamereferenceFieldSpec(nameReference[0]))
129+
require.NoError(t, expected.AddNamereferenceFieldSpec(nameReference[1]))
130+
require.NoError(t, expected.AddPrefixFieldSpec(fieldSpecs[0]))
131+
require.NoError(t, expected.AddPrefixFieldSpec(fieldSpecs[1]))
132+
require.NoError(t, expected.AddSuffixFieldSpec(fieldSpecs[0]))
133+
require.NoError(t, expected.AddSuffixFieldSpec(fieldSpecs[1]))
134+
require.NoError(t, expected.AddCommonLabelsFieldSpec(fieldSpecs[0]))
135+
require.NoError(t, expected.AddCommonLabelsFieldSpec(fieldSpecs[1]))
136+
require.NoError(t, expected.AddLabelsFieldSpec(fieldSpecs[0]))
137+
require.NoError(t, expected.AddLabelsFieldSpec(fieldSpecs[1]))
138+
require.Equal(t, expected, actual)
167139

168140
actual, err = cfga.Merge(nil)
169-
if err != nil {
170-
t.Fatalf("unexpected err: %v", err)
171-
}
172-
if !reflect.DeepEqual(actual, cfga) {
173-
t.Fatalf("expected: %v\n but got: %v\n", cfga, actual)
174-
}
141+
require.NoError(t, err)
142+
require.Equal(t, cfga, actual)
175143
}
176144

177145
func TestMakeDefaultConfig_mutation(t *testing.T) {
@@ -182,9 +150,7 @@ func TestMakeDefaultConfig_mutation(t *testing.T) {
182150
a.NameReference = a.NameReference[:1]
183151

184152
clean := MakeDefaultConfig()
185-
if clean.NameReference[0].Kind == "mutated" {
186-
t.Errorf("MakeDefaultConfig() did not return a clean copy: %+v", clean.NameReference)
187-
}
153+
assert.NotEqualf(t, "mutated", clean.NameReference[0].Kind, "MakeDefaultConfig() did not return a clean copy: %+v", clean.NameReference)
188154
}
189155

190156
func BenchmarkMakeDefaultConfig(b *testing.B) {

api/internal/target/kusttarget_configplugin.go

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -275,13 +275,25 @@ var transformerConfigurators = map[builtinhelpers.BuiltinPluginType]func(
275275
if len(kt.kustomization.Labels) == 0 && len(kt.kustomization.CommonLabels) == 0 {
276276
return
277277
}
278+
279+
type labelStruct struct {
280+
Labels map[string]string
281+
FieldSpecs []types.FieldSpec
282+
}
283+
278284
for _, label := range kt.kustomization.Labels {
279-
var c struct {
280-
Labels map[string]string
281-
FieldSpecs []types.FieldSpec
282-
}
285+
var c labelStruct
286+
283287
c.Labels = label.Pairs
284288
fss := types.FsSlice(label.FieldSpecs)
289+
290+
// merge labels specified in the label section of transformer configs
291+
// these apply to selectors and templates
292+
fss, err := fss.MergeAll(tc.Labels)
293+
if err != nil {
294+
return nil, fmt.Errorf("failed to merge labels: %w", err)
295+
}
296+
285297
// merge the custom fieldSpecs with the default
286298
if label.IncludeSelectors {
287299
fss, err = fss.MergeAll(tc.CommonLabels)
@@ -297,7 +309,7 @@ var transformerConfigurators = map[builtinhelpers.BuiltinPluginType]func(
297309
fss, err = fss.MergeOne(types.FieldSpec{Path: "metadata/labels", CreateIfNotPresent: true})
298310
}
299311
if err != nil {
300-
return nil, err
312+
return nil, fmt.Errorf("failed to merge labels: %w", err)
301313
}
302314
c.FieldSpecs = fss
303315
p := f()
@@ -307,10 +319,9 @@ var transformerConfigurators = map[builtinhelpers.BuiltinPluginType]func(
307319
}
308320
result = append(result, p)
309321
}
310-
var c struct {
311-
Labels map[string]string
312-
FieldSpecs []types.FieldSpec
313-
}
322+
323+
var c labelStruct
324+
314325
c.Labels = kt.kustomization.CommonLabels
315326
c.FieldSpecs = tc.CommonLabels
316327
p := f()

api/internal/target/kusttarget_test.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -295,9 +295,7 @@ metadata:
295295

296296
expected := resmap.New()
297297
for _, r := range resources {
298-
if err := expected.Append(r); err != nil {
299-
t.Fatalf("failed to append resource: %v", err)
300-
}
298+
require.NoError(t, expected.Append(r), "failed to append resource: %v")
301299
}
302300
expected.RemoveBuildAnnotations()
303301
expYaml, err := expected.AsYaml()

0 commit comments

Comments
 (0)