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.
This commit is contained in:
Mauren
2024-04-25 03:40:44 -04:00
committed by GitHub
parent 2e6171a9ea
commit 671de1662d
8 changed files with 478 additions and 120 deletions

View File

@@ -144,7 +144,7 @@ func loadCrdIntoConfig(
}
_, label := property.Extensions.GetString(xLabelSelector)
if label {
err = theConfig.AddLabelFieldSpec(
err = theConfig.AddCommonLabelsFieldSpec(
makeFs(theGvk, append(path, propName)))
if err != nil {
return

View File

@@ -70,30 +70,3 @@ namoPrefix:
t.Fatalf("expected error %s, but got %s", errMsg, err)
}
}
// please remove this failing test after implements the labels support
func TestLoadDefaultConfigsFromFilesWithMissingFieldsLabels(t *testing.T) {
fSys := filesys.MakeFsInMemory()
filePathContainsTypo := "config_contains_typo.yaml"
if err := fSys.WriteFile(filePathContainsTypo, []byte(`
labels:
- path: spec/podTemplate/metadata/labels
create: true
kind: FlinkDeployment
`)); err != nil {
t.Fatal(err)
}
ldr, err := loader.NewLoader(
loader.RestrictionRootOnly, filesys.Separator, fSys)
if err != nil {
t.Fatal(err)
}
errMsg := "error unmarshaling JSON: while decoding JSON: json: unknown field"
_, err = loadDefaultConfig(ldr, []string{filePathContainsTypo})
if err == nil {
t.Fatalf("expected to fail unmarshal yaml, but got nil %s", filePathContainsTypo)
}
if !strings.Contains(err.Error(), errMsg) {
t.Fatalf("expected error %s, but got %s", errMsg, err)
}
}

View File

@@ -21,6 +21,7 @@ type TransformerConfig struct {
NameSuffix types.FsSlice `json:"nameSuffix,omitempty" yaml:"nameSuffix,omitempty"`
NameSpace types.FsSlice `json:"namespace,omitempty" yaml:"namespace,omitempty"`
CommonLabels types.FsSlice `json:"commonLabels,omitempty" yaml:"commonLabels,omitempty"`
Labels types.FsSlice `json:"labels,omitempty" yaml:"labels,omitempty"`
TemplateLabels types.FsSlice `json:"templateLabels,omitempty" yaml:"templateLabels,omitempty"`
CommonAnnotations types.FsSlice `json:"commonAnnotations,omitempty" yaml:"commonAnnotations,omitempty"`
NameReference nbrSlice `json:"nameReference,omitempty" yaml:"nameReference,omitempty"`
@@ -41,6 +42,7 @@ func (t *TransformerConfig) DeepCopy() *TransformerConfig {
NameSuffix: t.NameSuffix.DeepCopy(),
NameSpace: t.NameSpace.DeepCopy(),
CommonLabels: t.CommonLabels.DeepCopy(),
Labels: t.Labels.DeepCopy(),
TemplateLabels: t.TemplateLabels.DeepCopy(),
CommonAnnotations: t.CommonAnnotations.DeepCopy(),
NameReference: t.NameReference.DeepCopy(),
@@ -94,6 +96,7 @@ func (t *TransformerConfig) sortFields() {
sort.Sort(t.NameSuffix)
sort.Sort(t.NameSpace)
sort.Sort(t.CommonLabels)
sort.Sort(t.Labels)
sort.Sort(t.TemplateLabels)
sort.Sort(t.CommonAnnotations)
sort.Sort(t.NameReference)
@@ -114,12 +117,18 @@ func (t *TransformerConfig) AddSuffixFieldSpec(fs types.FieldSpec) (err error) {
return err
}
// AddLabelFieldSpec adds a FieldSpec to CommonLabels
func (t *TransformerConfig) AddLabelFieldSpec(fs types.FieldSpec) (err error) {
// AddCommonLabelsFieldSpec adds a FieldSpec to CommonLabels
func (t *TransformerConfig) AddCommonLabelsFieldSpec(fs types.FieldSpec) (err error) {
t.CommonLabels, err = t.CommonLabels.MergeOne(fs)
return err
}
// AddLabelsFieldSpec adds a FieldSpec to Labels
func (t *TransformerConfig) AddLabelsFieldSpec(fs types.FieldSpec) (err error) {
t.Labels, err = t.Labels.MergeOne(fs)
return err //nolint:wrapcheck
}
// AddAnnotationFieldSpec adds a FieldSpec to CommonAnnotations
func (t *TransformerConfig) AddAnnotationFieldSpec(fs types.FieldSpec) (err error) {
t.CommonAnnotations, err = t.CommonAnnotations.MergeOne(fs)
@@ -162,6 +171,10 @@ func (t *TransformerConfig) Merge(input *TransformerConfig) (
if err != nil {
return nil, errors.WrapPrefixf(err, "failed to merge CommonLabels fieldSpec")
}
merged.Labels, err = t.Labels.MergeAll(input.Labels)
if err != nil {
return nil, errors.WrapPrefixf(err, "failed to merge Labels fieldSpec")
}
merged.TemplateLabels, err = t.TemplateLabels.MergeAll(input.TemplateLabels)
if err != nil {
return nil, errors.WrapPrefixf(err, "failed to merge TemplateLabels fieldSpec")

View File

@@ -4,9 +4,10 @@
package builtinconfig_test
import (
"reflect"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
. "sigs.k8s.io/kustomize/api/internal/plugins/builtinconfig"
"sigs.k8s.io/kustomize/api/types"
"sigs.k8s.io/kustomize/kyaml/resid"
@@ -35,13 +36,8 @@ func TestAddNamereferenceFieldSpec(t *testing.T) {
},
}
err := cfg.AddNamereferenceFieldSpec(nbrs)
if err != nil {
t.Fatalf("unexpected err: %v", err)
}
if len(cfg.NameReference) != 1 {
t.Fatal("failed to add namereference FieldSpec")
}
require.NoError(t, cfg.AddNamereferenceFieldSpec(nbrs))
require.Len(t, cfg.NameReference, 1, "failed to add namereference FieldSpec")
}
func TestAddFieldSpecs(t *testing.T) {
@@ -53,34 +49,14 @@ func TestAddFieldSpecs(t *testing.T) {
CreateIfNotPresent: true,
}
err := cfg.AddPrefixFieldSpec(fieldSpec)
if err != nil {
t.Fatalf("unexpected err: %v", err)
}
if len(cfg.NamePrefix) != 1 {
t.Fatalf("failed to add nameprefix FieldSpec")
}
err = cfg.AddSuffixFieldSpec(fieldSpec)
if err != nil {
t.Fatalf("unexpected err: %v", err)
}
if len(cfg.NameSuffix) != 1 {
t.Fatalf("failed to add namesuffix FieldSpec")
}
err = cfg.AddLabelFieldSpec(fieldSpec)
if err != nil {
t.Fatalf("unexpected err: %v", err)
}
if len(cfg.CommonLabels) != 1 {
t.Fatalf("failed to add nameprefix FieldSpec")
}
err = cfg.AddAnnotationFieldSpec(fieldSpec)
if err != nil {
t.Fatalf("unexpected err: %v", err)
}
if len(cfg.CommonAnnotations) != 1 {
t.Fatalf("failed to add nameprefix FieldSpec")
}
require.NoError(t, cfg.AddPrefixFieldSpec(fieldSpec))
require.Len(t, cfg.NamePrefix, 1, "failed to add nameprefix FieldSpec")
require.NoError(t, cfg.AddSuffixFieldSpec(fieldSpec))
require.Len(t, cfg.NameSuffix, 1, "failed to add namesuffix FieldSpec")
require.NoError(t, cfg.AddCommonLabelsFieldSpec(fieldSpec))
require.Len(t, cfg.CommonLabels, 1, "failed to add labels FieldSpec")
require.NoError(t, cfg.AddAnnotationFieldSpec(fieldSpec))
require.Len(t, cfg.CommonAnnotations, 1, "failed to add nameprefix FieldSpec")
}
func TestMerge(t *testing.T) {
@@ -127,51 +103,43 @@ func TestMerge(t *testing.T) {
},
}
cfga := &TransformerConfig{}
cfga.AddNamereferenceFieldSpec(nameReference[0])
cfga.AddPrefixFieldSpec(fieldSpecs[0])
cfga.AddSuffixFieldSpec(fieldSpecs[0])
require.NoError(t, cfga.AddNamereferenceFieldSpec(nameReference[0]))
require.NoError(t, cfga.AddPrefixFieldSpec(fieldSpecs[0]))
require.NoError(t, cfga.AddSuffixFieldSpec(fieldSpecs[0]))
require.NoError(t, cfga.AddCommonLabelsFieldSpec(fieldSpecs[0]))
require.NoError(t, cfga.AddLabelsFieldSpec(fieldSpecs[0]))
cfgb := &TransformerConfig{}
cfgb.AddNamereferenceFieldSpec(nameReference[1])
cfgb.AddPrefixFieldSpec(fieldSpecs[1])
cfga.AddSuffixFieldSpec(fieldSpecs[1])
require.NoError(t, cfgb.AddNamereferenceFieldSpec(nameReference[1]))
require.NoError(t, cfgb.AddPrefixFieldSpec(fieldSpecs[1]))
require.NoError(t, cfgb.AddSuffixFieldSpec(fieldSpecs[1]))
require.NoError(t, cfgb.AddCommonLabelsFieldSpec(fieldSpecs[1]))
require.NoError(t, cfgb.AddLabelsFieldSpec(fieldSpecs[1]))
actual, err := cfga.Merge(cfgb)
if err != nil {
t.Fatalf("unexpected err: %v", err)
}
if len(actual.NamePrefix) != 2 {
t.Fatal("merge failed for namePrefix FieldSpec")
}
if len(actual.NameSuffix) != 2 {
t.Fatal("merge failed for nameSuffix FieldSpec")
}
if len(actual.NameReference) != 1 {
t.Fatal("merge failed for namereference FieldSpec")
}
require.NoError(t, err)
require.Len(t, actual.NamePrefix, 2, "merge failed for namePrefix FieldSpec")
require.Len(t, actual.NameSuffix, 2, "merge failed for nameSuffix FieldSpec")
require.Len(t, actual.NameReference, 1, "merge failed for nameReference FieldSpec")
require.Len(t, actual.Labels, 2, "merge failed for labels FieldSpec")
require.Len(t, actual.CommonLabels, 2, "merge failed for commonLabels FieldSpec")
expected := &TransformerConfig{}
expected.AddNamereferenceFieldSpec(nameReference[0])
expected.AddNamereferenceFieldSpec(nameReference[1])
expected.AddPrefixFieldSpec(fieldSpecs[0])
expected.AddPrefixFieldSpec(fieldSpecs[1])
expected.AddSuffixFieldSpec(fieldSpecs[0])
expected.AddSuffixFieldSpec(fieldSpecs[1])
if !reflect.DeepEqual(actual, expected) {
t.Fatalf("expected: %v\n but got: %v\n", expected, actual)
}
require.NoError(t, expected.AddNamereferenceFieldSpec(nameReference[0]))
require.NoError(t, expected.AddNamereferenceFieldSpec(nameReference[1]))
require.NoError(t, expected.AddPrefixFieldSpec(fieldSpecs[0]))
require.NoError(t, expected.AddPrefixFieldSpec(fieldSpecs[1]))
require.NoError(t, expected.AddSuffixFieldSpec(fieldSpecs[0]))
require.NoError(t, expected.AddSuffixFieldSpec(fieldSpecs[1]))
require.NoError(t, expected.AddCommonLabelsFieldSpec(fieldSpecs[0]))
require.NoError(t, expected.AddCommonLabelsFieldSpec(fieldSpecs[1]))
require.NoError(t, expected.AddLabelsFieldSpec(fieldSpecs[0]))
require.NoError(t, expected.AddLabelsFieldSpec(fieldSpecs[1]))
require.Equal(t, expected, actual)
actual, err = cfga.Merge(nil)
if err != nil {
t.Fatalf("unexpected err: %v", err)
}
if !reflect.DeepEqual(actual, cfga) {
t.Fatalf("expected: %v\n but got: %v\n", cfga, actual)
}
require.NoError(t, err)
require.Equal(t, cfga, actual)
}
func TestMakeDefaultConfig_mutation(t *testing.T) {
@@ -182,9 +150,7 @@ func TestMakeDefaultConfig_mutation(t *testing.T) {
a.NameReference = a.NameReference[:1]
clean := MakeDefaultConfig()
if clean.NameReference[0].Kind == "mutated" {
t.Errorf("MakeDefaultConfig() did not return a clean copy: %+v", clean.NameReference)
}
assert.NotEqualf(t, "mutated", clean.NameReference[0].Kind, "MakeDefaultConfig() did not return a clean copy: %+v", clean.NameReference)
}
func BenchmarkMakeDefaultConfig(b *testing.B) {

View File

@@ -275,13 +275,25 @@ var transformerConfigurators = map[builtinhelpers.BuiltinPluginType]func(
if len(kt.kustomization.Labels) == 0 && len(kt.kustomization.CommonLabels) == 0 {
return
}
type labelStruct struct {
Labels map[string]string
FieldSpecs []types.FieldSpec
}
for _, label := range kt.kustomization.Labels {
var c struct {
Labels map[string]string
FieldSpecs []types.FieldSpec
}
var c labelStruct
c.Labels = label.Pairs
fss := types.FsSlice(label.FieldSpecs)
// merge labels specified in the label section of transformer configs
// these apply to selectors and templates
fss, err := fss.MergeAll(tc.Labels)
if err != nil {
return nil, fmt.Errorf("failed to merge labels: %w", err)
}
// merge the custom fieldSpecs with the default
if label.IncludeSelectors {
fss, err = fss.MergeAll(tc.CommonLabels)
@@ -297,7 +309,7 @@ var transformerConfigurators = map[builtinhelpers.BuiltinPluginType]func(
fss, err = fss.MergeOne(types.FieldSpec{Path: "metadata/labels", CreateIfNotPresent: true})
}
if err != nil {
return nil, err
return nil, fmt.Errorf("failed to merge labels: %w", err)
}
c.FieldSpecs = fss
p := f()
@@ -307,10 +319,9 @@ var transformerConfigurators = map[builtinhelpers.BuiltinPluginType]func(
}
result = append(result, p)
}
var c struct {
Labels map[string]string
FieldSpecs []types.FieldSpec
}
var c labelStruct
c.Labels = kt.kustomization.CommonLabels
c.FieldSpecs = tc.CommonLabels
p := f()

View File

@@ -295,9 +295,7 @@ metadata:
expected := resmap.New()
for _, r := range resources {
if err := expected.Append(r); err != nil {
t.Fatalf("failed to append resource: %v", err)
}
require.NoError(t, expected.Append(r), "failed to append resource: %v")
}
expected.RemoveBuildAnnotations()
expYaml, err := expected.AsYaml()

View File

@@ -335,3 +335,400 @@ spec:
location: Arizona
`)
}
func TestLabelTransformerConfig(t *testing.T) {
testCases := []struct {
name string
kustomization string
transformerConfig string
expectedResult string
}{
{
name: "includeSelectors=false, includeTemplates=false, include template via transformerConfig",
kustomization: `configurations:
- config/configurations.yaml
labels:
- includeSelectors: false
includeTemplates: false
pairs:
location: planet-earth
environment: dev
resources:
- resources/deployment.yaml
`,
transformerConfig: `labels:
- path: spec/template/metadata/labels
create: true
kind: Deployment
`,
expectedResult: `apiVersion: apps/v1
kind: Deployment
metadata:
creationTimestamp: null
labels:
app: sample-deploy
environment: dev
location: planet-earth
name: sample-deploy
spec:
replicas: 1
selector:
matchLabels:
app: sample-deploy
strategy: {}
template:
metadata:
creationTimestamp: null
labels:
app: sample-deploy
environment: dev
location: planet-earth
spec:
containers:
- image: hello-world:latest
name: hello-world
`,
},
{
name: "includeSelectors=true, includeTemplates=false, include template via transformerConfig",
kustomization: `configurations:
- config/configurations.yaml
labels:
- includeSelectors: true
includeTemplates: false
pairs:
location: planet-earth
environment: dev
resources:
- resources/deployment.yaml
`,
transformerConfig: `labels:
- path: spec/template/metadata/labels
create: true
kind: Deployment
`,
expectedResult: `apiVersion: apps/v1
kind: Deployment
metadata:
creationTimestamp: null
labels:
app: sample-deploy
environment: dev
location: planet-earth
name: sample-deploy
spec:
replicas: 1
selector:
matchLabels:
app: sample-deploy
environment: dev
location: planet-earth
strategy: {}
template:
metadata:
creationTimestamp: null
labels:
app: sample-deploy
environment: dev
location: planet-earth
spec:
containers:
- image: hello-world:latest
name: hello-world
`,
},
{
name: "includeSelectors=false, includeTemplates=true, no transformerConfig",
kustomization: `labels:
- includeSelectors: false
includeTemplates: true
pairs:
location: planet-earth
environment: dev
resources:
- resources/deployment.yaml
`,
expectedResult: `apiVersion: apps/v1
kind: Deployment
metadata:
creationTimestamp: null
labels:
app: sample-deploy
environment: dev
location: planet-earth
name: sample-deploy
spec:
replicas: 1
selector:
matchLabels:
app: sample-deploy
strategy: {}
template:
metadata:
creationTimestamp: null
labels:
app: sample-deploy
environment: dev
location: planet-earth
spec:
containers:
- image: hello-world:latest
name: hello-world
`,
},
{
name: "includeSelectors=false, includeTemplates=false, no transformerConfig",
kustomization: `labels:
- includeSelectors: false
includeTemplates: false
pairs:
location: planet-earth
environment: dev
resources:
- resources/deployment.yaml
`,
expectedResult: `apiVersion: apps/v1
kind: Deployment
metadata:
creationTimestamp: null
labels:
app: sample-deploy
environment: dev
location: planet-earth
name: sample-deploy
spec:
replicas: 1
selector:
matchLabels:
app: sample-deploy
strategy: {}
template:
metadata:
creationTimestamp: null
labels:
app: sample-deploy
spec:
containers:
- image: hello-world:latest
name: hello-world
`,
},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
th := kusttest_test.MakeHarness(t)
th.WriteK(".", tc.kustomization)
th.WriteF("resources/deployment.yaml",
`apiVersion: apps/v1
kind: Deployment
metadata:
creationTimestamp: null
labels:
app: sample-deploy
name: sample-deploy
spec:
replicas: 1
selector:
matchLabels:
app: sample-deploy
strategy: {}
template:
metadata:
creationTimestamp: null
labels:
app: sample-deploy
spec:
containers:
- image: hello-world:latest
name: hello-world
`)
if tc.transformerConfig != "" {
th.WriteF("config/configurations.yaml", tc.transformerConfig)
}
output := th.Run(".", th.MakeDefaultOptions())
th.AssertActualEqualsExpected(output, tc.expectedResult)
})
}
}
func TestLabelTransformerConfigWithCustomResources(t *testing.T) {
testCases := []struct {
name string
kustomization string
transformerConfig string
expectedResult string
}{
{
name: "include template via transformerConfig",
kustomization: `configurations:
- config/configurations.yaml
labels:
- includeSelectors: false
includeTemplates: false
pairs:
location: planet-earth
environment: dev
resources:
- resources/custom-resource.yaml
`,
transformerConfig: `labels:
- path: spec/template/metadata/labels
create: true
kind: SampleResource
`,
expectedResult: `apiVersion: custom.example.org/v1
kind: SampleResource
metadata:
labels:
environment: dev
location: planet-earth
name: sample-resource
namespace: sample-namespace
spec:
template:
metadata:
labels:
environment: dev
location: planet-earth
spec:
containers:
- env:
- name: VARIABLE
value: value
image: index.docker.io/library/hello-world
`,
},
{
name: "include selector via transformerConfig",
kustomization: `configurations:
- config/configurations.yaml
labels:
- includeSelectors: false
includeTemplates: false
pairs:
location: planet-earth
environment: dev
resources:
- resources/custom-resource.yaml
`,
transformerConfig: `labels:
- path: spec/selectors/labels
create: true
kind: SampleResource
`,
expectedResult: `apiVersion: custom.example.org/v1
kind: SampleResource
metadata:
labels:
environment: dev
location: planet-earth
name: sample-resource
namespace: sample-namespace
spec:
selectors:
labels:
environment: dev
location: planet-earth
template:
spec:
containers:
- env:
- name: VARIABLE
value: value
image: index.docker.io/library/hello-world
`,
},
{
name: "include selectors and labels via transformerConfig",
kustomization: `configurations:
- config/configurations.yaml
labels:
- includeSelectors: false
includeTemplates: false
pairs:
location: planet-earth
environment: dev
resources:
- resources/custom-resource.yaml
`,
transformerConfig: `
labels:
- path: spec/selectors/labels
create: true
kind: SampleResource
- path: spec/template/metadata/labels
create: true
kind: SampleResource
`,
expectedResult: `apiVersion: custom.example.org/v1
kind: SampleResource
metadata:
labels:
environment: dev
location: planet-earth
name: sample-resource
namespace: sample-namespace
spec:
selectors:
labels:
environment: dev
location: planet-earth
template:
metadata:
labels:
environment: dev
location: planet-earth
spec:
containers:
- env:
- name: VARIABLE
value: value
image: index.docker.io/library/hello-world
`,
},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
th := kusttest_test.MakeHarness(t)
th.WriteK(".", tc.kustomization)
th.WriteF("resources/custom-resource.yaml",
`apiVersion: custom.example.org/v1
kind: SampleResource
metadata:
name: sample-resource
namespace: sample-namespace
spec:
template:
spec:
containers:
- image: index.docker.io/library/hello-world
env:
- name: VARIABLE
value: value
`)
th.WriteF("config/configurations.yaml", tc.transformerConfig)
output := th.Run(".", th.MakeDefaultOptions())
th.AssertActualEqualsExpected(output, tc.expectedResult)
})
}
}

View File

@@ -6,12 +6,12 @@ package types
type Label struct {
// Pairs contains the key-value pairs for labels to add
Pairs map[string]string `json:"pairs,omitempty" yaml:"pairs,omitempty"`
// IncludeSelectors inidicates should transformer include the
// IncludeSelectors indicates whether the transformer should include the
// fieldSpecs for selectors. Custom fieldSpecs specified by
// FieldSpecs will be merged with builtin fieldSpecs if this
// is true.
IncludeSelectors bool `json:"includeSelectors,omitempty" yaml:"includeSelectors,omitempty"`
// IncludeTemplates inidicates should transformer include the
// IncludeTemplates indicates whether the transformer should include the
// spec/template/metadata fieldSpec. Custom fieldSpecs specified by
// FieldSpecs will be merged with spec/template/metadata fieldSpec if this
// is true. If IncludeSelectors is true, IncludeTemplates is not needed.