From 662ccf19154c6d4b0d5a1575e7dd2cf72e8229fe Mon Sep 17 00:00:00 2001 From: Agustina Barbetta Date: Tue, 20 Sep 2022 13:03:58 -0300 Subject: [PATCH 01/10] add test for resources with no template --- api/krusty/inlinelabels_test.go | 156 ++++++++++++++++++++++++++++++++ 1 file changed, 156 insertions(+) diff --git a/api/krusty/inlinelabels_test.go b/api/krusty/inlinelabels_test.go index 2979c27e4f..a1729e3cab 100644 --- a/api/krusty/inlinelabels_test.go +++ b/api/krusty/inlinelabels_test.go @@ -160,3 +160,159 @@ spec: foo: bar `) } + +func TestKustomizationLabelsInTemplateWhenLabelsIsNil(t *testing.T) { + th := kusttest_test.MakeHarness(t) + th.WriteF("app/deployment.yaml", ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: deployment +spec: + replicas: 1 + template: + spec: + containers: + - name: test-server + image: test-server +`) + th.WriteK("/app", ` +resources: +- deployment.yaml + +commonLabels: + app: test-server + +labels: +- pairs: + app.kubernetes.io/component: a + app.kubernetes.io/instance: b + app.kubernetes.io/name: c + app.kubernetes.io/part-of: d + includeSelectors: false + includeTemplates: true +`) + m := th.Run("/app", th.MakeDefaultOptions()) + th.AssertActualEqualsExpected(m, ` +apiVersion: apps/v1 +kind: Deployment +metadata: + labels: + app: test-server + app.kubernetes.io/component: a + app.kubernetes.io/instance: b + app.kubernetes.io/name: c + app.kubernetes.io/part-of: d + name: deployment +spec: + replicas: 1 + selector: + matchLabels: + app: test-server + template: + metadata: + labels: + app: test-server + app.kubernetes.io/component: a + app.kubernetes.io/instance: b + app.kubernetes.io/name: c + app.kubernetes.io/part-of: d + spec: + containers: + - image: test-server + name: test-server +`) +} + +func TestKustomizationLabelsInTemplateWhenResourceHasNoTemplate(t *testing.T) { + th := kusttest_test.MakeHarness(t) + th.WriteF("app/deployment.yaml", ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: deployment +spec: + replicas: 1 + template: + spec: + containers: + - name: test-server + image: test-server +`) + th.WriteF("app/service.yaml", ` +apiVersion: v1 +kind: Service +metadata: + name: service +spec: + ports: + - port: 80 + protocol: TCP + targetPort: 9376 +`) + th.WriteK("/app", ` +resources: +- deployment.yaml +- service.yaml + +commonLabels: + app: test-server + +labels: +- pairs: + app.kubernetes.io/component: a + app.kubernetes.io/instance: b + app.kubernetes.io/name: c + app.kubernetes.io/part-of: d + includeSelectors: false + includeTemplates: true +`) + m := th.Run("/app", th.MakeDefaultOptions()) + th.AssertActualEqualsExpected(m, ` +apiVersion: apps/v1 +kind: Deployment +metadata: + labels: + app: test-server + app.kubernetes.io/component: a + app.kubernetes.io/instance: b + app.kubernetes.io/name: c + app.kubernetes.io/part-of: d + name: deployment +spec: + replicas: 1 + selector: + matchLabels: + app: test-server + template: + metadata: + labels: + app: test-server + app.kubernetes.io/component: a + app.kubernetes.io/instance: b + app.kubernetes.io/name: c + app.kubernetes.io/part-of: d + spec: + containers: + - image: test-server + name: test-server +--- +apiVersion: v1 +kind: Service +metadata: + labels: + app: test-server + app.kubernetes.io/component: a + app.kubernetes.io/instance: b + app.kubernetes.io/name: c + app.kubernetes.io/part-of: d + name: service +spec: + ports: + - port: 80 + protocol: TCP + targetPort: 9376 + selector: + app: test-server +`) +} From 4dcc040ec1a41b6e0c77200da0be7515fd4447cd Mon Sep 17 00:00:00 2001 From: Agustina Barbetta Date: Tue, 20 Sep 2022 13:33:07 -0300 Subject: [PATCH 02/10] create template/metadata in includeTemplates if not present --- api/internal/target/kusttarget_configplugin.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/internal/target/kusttarget_configplugin.go b/api/internal/target/kusttarget_configplugin.go index 0991c5fac7..f05c6584dd 100644 --- a/api/internal/target/kusttarget_configplugin.go +++ b/api/internal/target/kusttarget_configplugin.go @@ -288,7 +288,7 @@ var transformerConfigurators = map[builtinhelpers.BuiltinPluginType]func( } else { // merge spec/template/metadata fieldSpec if includeTemplate flag is true if label.IncludeTemplates { - fss, err = fss.MergeOne(types.FieldSpec{Path: "spec/template/metadata/labels", CreateIfNotPresent: false}) + fss, err = fss.MergeOne(types.FieldSpec{Path: "spec/template/metadata/labels", CreateIfNotPresent: true}) if err != nil { return nil, errors.Wrap(err, "failed to merge template fieldSpec") } From e2196d9bd1a2fe97a9f54a125ece16e0e819a4e0 Mon Sep 17 00:00:00 2001 From: Agustina Barbetta Date: Fri, 23 Sep 2022 15:11:59 -0300 Subject: [PATCH 03/10] separate template specific field specs and use in includeTemplates --- .../builtinconfig/transformerconfig.go | 6 +++ .../target/kusttarget_configplugin.go | 4 +- .../builtinpluginconsts/commonlabels.go | 48 +---------------- .../builtinpluginconsts/defaultconfig.go | 2 + .../builtinpluginconsts/metadatalabels.go | 51 +++++++++++++++++++ .../builtinpluginconsts/templatelabels.go | 8 +++ 6 files changed, 70 insertions(+), 49 deletions(-) create mode 100644 api/konfig/builtinpluginconsts/metadatalabels.go create mode 100644 api/konfig/builtinpluginconsts/templatelabels.go diff --git a/api/internal/plugins/builtinconfig/transformerconfig.go b/api/internal/plugins/builtinconfig/transformerconfig.go index a28627a13f..867a04d208 100644 --- a/api/internal/plugins/builtinconfig/transformerconfig.go +++ b/api/internal/plugins/builtinconfig/transformerconfig.go @@ -18,6 +18,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"` + 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"` VarReference types.FsSlice `json:"varReference,omitempty" yaml:"varReference,omitempty"` @@ -60,6 +61,7 @@ func (t *TransformerConfig) sortFields() { sort.Sort(t.NamePrefix) sort.Sort(t.NameSpace) sort.Sort(t.CommonLabels) + sort.Sort(t.TemplateLabels) sort.Sort(t.CommonAnnotations) sort.Sort(t.NameReference) sort.Sort(t.VarReference) @@ -127,6 +129,10 @@ func (t *TransformerConfig) Merge(input *TransformerConfig) ( if err != nil { return nil, err } + merged.TemplateLabels, err = t.TemplateLabels.MergeAll(input.TemplateLabels) + if err != nil { + return nil, err + } merged.VarReference, err = t.VarReference.MergeAll(input.VarReference) if err != nil { return nil, err diff --git a/api/internal/target/kusttarget_configplugin.go b/api/internal/target/kusttarget_configplugin.go index f05c6584dd..9a3c63086f 100644 --- a/api/internal/target/kusttarget_configplugin.go +++ b/api/internal/target/kusttarget_configplugin.go @@ -286,9 +286,9 @@ var transformerConfigurators = map[builtinhelpers.BuiltinPluginType]func( if label.IncludeSelectors { fss, err = fss.MergeAll(tc.CommonLabels) } else { - // merge spec/template/metadata fieldSpec if includeTemplate flag is true + // merge spec/template/metadata fieldSpecs if includeTemplate flag is true if label.IncludeTemplates { - fss, err = fss.MergeOne(types.FieldSpec{Path: "spec/template/metadata/labels", CreateIfNotPresent: true}) + fss, err = fss.MergeAll(tc.TemplateLabels) if err != nil { return nil, errors.Wrap(err, "failed to merge template fieldSpec") } diff --git a/api/konfig/builtinpluginconsts/commonlabels.go b/api/konfig/builtinpluginconsts/commonlabels.go index 7775a544ff..b2a78f5681 100644 --- a/api/konfig/builtinpluginconsts/commonlabels.go +++ b/api/konfig/builtinpluginconsts/commonlabels.go @@ -5,9 +5,6 @@ package builtinpluginconsts const commonLabelFieldSpecs = ` commonLabels: -- path: metadata/labels - create: true - - path: spec/selector create: true version: v1 @@ -17,20 +14,10 @@ commonLabels: create: true version: v1 kind: ReplicationController - -- path: spec/template/metadata/labels - create: true - version: v1 - kind: ReplicationController - - path: spec/selector/matchLabels create: true kind: Deployment -- path: spec/template/metadata/labels - create: true - kind: Deployment - - path: spec/template/spec/affinity/podAffinity/preferredDuringSchedulingIgnoredDuringExecution/podAffinityTerm/labelSelector/matchLabels create: false group: apps @@ -60,28 +47,15 @@ commonLabels: create: true kind: ReplicaSet -- path: spec/template/metadata/labels - create: true - kind: ReplicaSet - - path: spec/selector/matchLabels create: true kind: DaemonSet -- path: spec/template/metadata/labels - create: true - kind: DaemonSet - - path: spec/selector/matchLabels create: true group: apps kind: StatefulSet -- path: spec/template/metadata/labels - create: true - group: apps - kind: StatefulSet - - path: spec/template/spec/affinity/podAffinity/preferredDuringSchedulingIgnoredDuringExecution/podAffinityTerm/labelSelector/matchLabels create: false group: apps @@ -107,36 +81,16 @@ commonLabels: group: apps kind: StatefulSet -- path: spec/volumeClaimTemplates[]/metadata/labels - create: true - group: apps - kind: StatefulSet - - path: spec/selector/matchLabels create: false group: batch kind: Job -- path: spec/template/metadata/labels - create: true - group: batch - kind: Job - - path: spec/jobTemplate/spec/selector/matchLabels create: false group: batch kind: CronJob -- path: spec/jobTemplate/metadata/labels - create: true - group: batch - kind: CronJob - -- path: spec/jobTemplate/spec/template/metadata/labels - create: true - group: batch - kind: CronJob - - path: spec/selector/matchLabels create: false group: policy @@ -156,4 +110,4 @@ commonLabels: create: false group: networking.k8s.io kind: NetworkPolicy -` +` + metadataLabelsFieldSpecs diff --git a/api/konfig/builtinpluginconsts/defaultconfig.go b/api/konfig/builtinpluginconsts/defaultconfig.go index 29673d76a7..2f220cb29a 100644 --- a/api/konfig/builtinpluginconsts/defaultconfig.go +++ b/api/konfig/builtinpluginconsts/defaultconfig.go @@ -13,6 +13,7 @@ func GetDefaultFieldSpecs() []byte { []byte(namePrefixFieldSpecs), []byte(nameSuffixFieldSpecs), []byte(commonLabelFieldSpecs), + []byte(templateLabelFieldSpecs), []byte(commonAnnotationFieldSpecs), []byte(namespaceFieldSpecs), []byte(varReferenceFieldSpecs), @@ -30,6 +31,7 @@ func GetDefaultFieldSpecsAsMap() map[string]string { result["nameprefix"] = namePrefixFieldSpecs result["namesuffix"] = nameSuffixFieldSpecs result["commonlabels"] = commonLabelFieldSpecs + result["templatelabels"] = templateLabelFieldSpecs result["commonannotations"] = commonAnnotationFieldSpecs result["namespace"] = namespaceFieldSpecs result["varreference"] = varReferenceFieldSpecs diff --git a/api/konfig/builtinpluginconsts/metadatalabels.go b/api/konfig/builtinpluginconsts/metadatalabels.go new file mode 100644 index 0000000000..d070cca4b8 --- /dev/null +++ b/api/konfig/builtinpluginconsts/metadatalabels.go @@ -0,0 +1,51 @@ +// Copyright 2019 The Kubernetes Authors. +// SPDX-License-Identifier: Apache-2.0 + +package builtinpluginconsts + +const metadataLabelsFieldSpecs = ` +- path: metadata/labels + create: true + +- path: spec/template/metadata/labels + create: true + version: v1 + kind: ReplicationController + +- path: spec/template/metadata/labels + create: true + kind: Deployment + +- path: spec/template/metadata/labels + create: true + kind: ReplicaSet + +- path: spec/template/metadata/labels + create: true + kind: DaemonSet + +- path: spec/template/metadata/labels + create: true + group: apps + kind: StatefulSet + +- path: spec/volumeClaimTemplates[]/metadata/labels + create: true + group: apps + kind: StatefulSet + +- path: spec/template/metadata/labels + create: true + group: batch + kind: Job + +- path: spec/jobTemplate/metadata/labels + create: true + group: batch + kind: CronJob + +- path: spec/jobTemplate/spec/template/metadata/labels + create: true + group: batch + kind: CronJob +` diff --git a/api/konfig/builtinpluginconsts/templatelabels.go b/api/konfig/builtinpluginconsts/templatelabels.go new file mode 100644 index 0000000000..5d3c9c1957 --- /dev/null +++ b/api/konfig/builtinpluginconsts/templatelabels.go @@ -0,0 +1,8 @@ +// Copyright 2019 The Kubernetes Authors. +// SPDX-License-Identifier: Apache-2.0 + +package builtinpluginconsts + +const templateLabelFieldSpecs = ` +templateLabels: +` + metadataLabelsFieldSpecs From 344b257c1f823ee9f1cbf242da9935701102c1d7 Mon Sep 17 00:00:00 2001 From: Agustina Barbetta Date: Fri, 23 Sep 2022 15:54:25 -0300 Subject: [PATCH 04/10] add tests for other kinds --- api/krusty/inlinelabels_test.go | 271 +++++++++++++++++++++++++++++++- 1 file changed, 270 insertions(+), 1 deletion(-) diff --git a/api/krusty/inlinelabels_test.go b/api/krusty/inlinelabels_test.go index a1729e3cab..ce93d2d794 100644 --- a/api/krusty/inlinelabels_test.go +++ b/api/krusty/inlinelabels_test.go @@ -92,7 +92,7 @@ spec: `) } -func TestKustomizationLabelsInTemplate(t *testing.T) { +func TestKustomizationLabelsInDeploymentTemplate(t *testing.T) { th := kusttest_test.MakeHarness(t) th.WriteF("app/deployment.yaml", ` apiVersion: apps/v1 @@ -316,3 +316,272 @@ spec: app: test-server `) } + +func TestKustomizationLabelsInDaemonSetTemplate(t *testing.T) { + th := kusttest_test.MakeHarness(t) + th.WriteF("app/ds.yaml", ` +apiVersion: apps/v1 +kind: DaemonSet +metadata: + labels: + app.kubernetes.io/component: a + app.kubernetes.io/instance: b + app.kubernetes.io/name: c + app.kubernetes.io/part-of: d + name: daemon +spec: + selector: + matchLabels: + app.kubernetes.io/component: a + app.kubernetes.io/instance: b + app.kubernetes.io/name: c + app.kubernetes.io/part-of: d + template: + metadata: + labels: + app.kubernetes.io/component: a + app.kubernetes.io/instance: b + app.kubernetes.io/name: c + app.kubernetes.io/part-of: d +`) + th.WriteK("/app", ` +resources: +- ds.yaml + +labels: +- pairs: + foo: bar + includeSelectors: false + includeTemplates: true +`) + m := th.Run("/app", th.MakeDefaultOptions()) + th.AssertActualEqualsExpected(m, ` +apiVersion: apps/v1 +kind: DaemonSet +metadata: + labels: + app.kubernetes.io/component: a + app.kubernetes.io/instance: b + app.kubernetes.io/name: c + app.kubernetes.io/part-of: d + foo: bar + name: daemon +spec: + selector: + matchLabels: + app.kubernetes.io/component: a + app.kubernetes.io/instance: b + app.kubernetes.io/name: c + app.kubernetes.io/part-of: d + template: + metadata: + labels: + app.kubernetes.io/component: a + app.kubernetes.io/instance: b + app.kubernetes.io/name: c + app.kubernetes.io/part-of: d + foo: bar +`) +} + +func TestKustomizationLabelsInStatefulSetTemplate(t *testing.T) { + th := kusttest_test.MakeHarness(t) + th.WriteF("app/sts.yaml", ` +apiVersion: apps/v1 +kind: StatefulSet +metadata: + labels: + app.kubernetes.io/component: a + app.kubernetes.io/instance: b + app.kubernetes.io/name: c + app.kubernetes.io/part-of: d + name: set +spec: + replicas: 3 + selector: + matchLabels: + app.kubernetes.io/component: a + app.kubernetes.io/instance: b + app.kubernetes.io/name: c + app.kubernetes.io/part-of: d + serviceName: set + template: + metadata: + labels: + app.kubernetes.io/component: a + app.kubernetes.io/instance: b + app.kubernetes.io/name: c + app.kubernetes.io/part-of: d +`) + th.WriteK("/app", ` +resources: +- sts.yaml + +labels: +- pairs: + foo: bar + includeSelectors: false + includeTemplates: true +`) + m := th.Run("/app", th.MakeDefaultOptions()) + th.AssertActualEqualsExpected(m, ` +apiVersion: apps/v1 +kind: StatefulSet +metadata: + labels: + app.kubernetes.io/component: a + app.kubernetes.io/instance: b + app.kubernetes.io/name: c + app.kubernetes.io/part-of: d + foo: bar + name: set +spec: + replicas: 3 + selector: + matchLabels: + app.kubernetes.io/component: a + app.kubernetes.io/instance: b + app.kubernetes.io/name: c + app.kubernetes.io/part-of: d + serviceName: set + template: + metadata: + labels: + app.kubernetes.io/component: a + app.kubernetes.io/instance: b + app.kubernetes.io/name: c + app.kubernetes.io/part-of: d + foo: bar +`) +} + +func TestKustomizationLabelsInCronJobTemplate(t *testing.T) { + th := kusttest_test.MakeHarness(t) + th.WriteF("app/cjob.yaml", ` +apiVersion: batch/v1 +kind: CronJob +metadata: + labels: + app.kubernetes.io/component: a + app.kubernetes.io/instance: b + app.kubernetes.io/name: c + app.kubernetes.io/part-of: d + name: job +spec: + jobTemplate: + spec: + backoffLimit: 4 + template: + metadata: + labels: + app.kubernetes.io/component: a + app.kubernetes.io/instance: b + app.kubernetes.io/name: c + app.kubernetes.io/part-of: d + spec: + restartPolicy: Never + schedule: '* * * * *' +`) + th.WriteK("/app", ` +resources: +- cjob.yaml + +labels: +- pairs: + foo: bar + includeSelectors: false + includeTemplates: true +`) + m := th.Run("/app", th.MakeDefaultOptions()) + th.AssertActualEqualsExpected(m, ` +apiVersion: batch/v1 +kind: CronJob +metadata: + labels: + app.kubernetes.io/component: a + app.kubernetes.io/instance: b + app.kubernetes.io/name: c + app.kubernetes.io/part-of: d + foo: bar + name: job +spec: + jobTemplate: + metadata: + labels: + foo: bar + spec: + backoffLimit: 4 + template: + metadata: + labels: + app.kubernetes.io/component: a + app.kubernetes.io/instance: b + app.kubernetes.io/name: c + app.kubernetes.io/part-of: d + foo: bar + spec: + restartPolicy: Never + schedule: '* * * * *' +`) +} + +func TestKustomizationLabelsInJobTemplate(t *testing.T) { + th := kusttest_test.MakeHarness(t) + th.WriteF("app/job.yaml", ` +apiVersion: batch/v1 +kind: Job +metadata: + labels: + app.kubernetes.io/component: a + app.kubernetes.io/instance: b + app.kubernetes.io/name: c + app.kubernetes.io/part-of: d + name: job +spec: + backoffLimit: 4 + template: + metadata: + labels: + app.kubernetes.io/component: a + app.kubernetes.io/instance: b + app.kubernetes.io/name: c + app.kubernetes.io/part-of: d + spec: + restartPolicy: Never +`) + th.WriteK("/app", ` +resources: +- job.yaml + +labels: +- pairs: + foo: bar + includeSelectors: false + includeTemplates: true +`) + m := th.Run("/app", th.MakeDefaultOptions()) + th.AssertActualEqualsExpected(m, ` +apiVersion: batch/v1 +kind: Job +metadata: + labels: + app.kubernetes.io/component: a + app.kubernetes.io/instance: b + app.kubernetes.io/name: c + app.kubernetes.io/part-of: d + foo: bar + name: job +spec: + backoffLimit: 4 + template: + metadata: + labels: + app.kubernetes.io/component: a + app.kubernetes.io/instance: b + app.kubernetes.io/name: c + app.kubernetes.io/part-of: d + foo: bar + spec: + restartPolicy: Never +`) +} From 2b9ef61f88cb9ae5fbb6427fa95d1576b882d4c8 Mon Sep 17 00:00:00 2001 From: Agustina Barbetta Date: Fri, 23 Sep 2022 18:30:00 -0300 Subject: [PATCH 05/10] error wraps for linter --- .../builtinconfig/transformerconfig.go | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/api/internal/plugins/builtinconfig/transformerconfig.go b/api/internal/plugins/builtinconfig/transformerconfig.go index 867a04d208..761ae7e5fc 100644 --- a/api/internal/plugins/builtinconfig/transformerconfig.go +++ b/api/internal/plugins/builtinconfig/transformerconfig.go @@ -7,6 +7,7 @@ import ( "log" "sort" + "github.com/pkg/errors" "sigs.k8s.io/kustomize/api/ifc" "sigs.k8s.io/kustomize/api/konfig/builtinpluginconsts" "sigs.k8s.io/kustomize/api/types" @@ -110,44 +111,44 @@ func (t *TransformerConfig) Merge(input *TransformerConfig) ( merged = &TransformerConfig{} merged.NamePrefix, err = t.NamePrefix.MergeAll(input.NamePrefix) if err != nil { - return nil, err + return nil, errors.Wrap(err, "failed to merge NamePrefix fieldSpec") } merged.NameSuffix, err = t.NameSuffix.MergeAll(input.NameSuffix) if err != nil { - return nil, err + return nil, errors.Wrap(err, "failed to merge NameSuffix fieldSpec") } merged.NameSpace, err = t.NameSpace.MergeAll(input.NameSpace) if err != nil { - return nil, err + return nil, errors.Wrap(err, "failed to merge NameSpace fieldSpec") } merged.CommonAnnotations, err = t.CommonAnnotations.MergeAll( input.CommonAnnotations) if err != nil { - return nil, err + return nil, errors.Wrap(err, "failed to merge CommonAnnotations fieldSpec") } merged.CommonLabels, err = t.CommonLabels.MergeAll(input.CommonLabels) if err != nil { - return nil, err + return nil, errors.Wrap(err, "failed to merge CommonLabels fieldSpec") } merged.TemplateLabels, err = t.TemplateLabels.MergeAll(input.TemplateLabels) if err != nil { - return nil, err + return nil, errors.Wrap(err, "failed to merge TemplateLabels fieldSpec") } merged.VarReference, err = t.VarReference.MergeAll(input.VarReference) if err != nil { - return nil, err + return nil, errors.Wrap(err, "failed to merge VarReference fieldSpec") } merged.NameReference, err = t.NameReference.mergeAll(input.NameReference) if err != nil { - return nil, err + return nil, errors.Wrap(err, "failed to merge NameReference fieldSpec") } merged.Images, err = t.Images.MergeAll(input.Images) if err != nil { - return nil, err + return nil, errors.Wrap(err, "failed to merge Images fieldSpec") } merged.Replicas, err = t.Replicas.MergeAll(input.Replicas) if err != nil { - return nil, err + return nil, errors.Wrap(err, "failed to merge Replicas fieldSpec") } merged.sortFields() return merged, nil From c750c0089d3e1bc6cacb7d7e9775c76607da92d5 Mon Sep 17 00:00:00 2001 From: Agustina Barbetta Date: Sun, 25 Sep 2022 09:07:47 -0300 Subject: [PATCH 06/10] use fmt instead of errors --- .../builtinconfig/transformerconfig.go | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/api/internal/plugins/builtinconfig/transformerconfig.go b/api/internal/plugins/builtinconfig/transformerconfig.go index 761ae7e5fc..84830fa642 100644 --- a/api/internal/plugins/builtinconfig/transformerconfig.go +++ b/api/internal/plugins/builtinconfig/transformerconfig.go @@ -4,10 +4,10 @@ package builtinconfig import ( + "fmt" "log" "sort" - "github.com/pkg/errors" "sigs.k8s.io/kustomize/api/ifc" "sigs.k8s.io/kustomize/api/konfig/builtinpluginconsts" "sigs.k8s.io/kustomize/api/types" @@ -111,44 +111,44 @@ func (t *TransformerConfig) Merge(input *TransformerConfig) ( merged = &TransformerConfig{} merged.NamePrefix, err = t.NamePrefix.MergeAll(input.NamePrefix) if err != nil { - return nil, errors.Wrap(err, "failed to merge NamePrefix fieldSpec") + return nil, fmt.Errorf("failed to merge NamePrefix fieldSpec: %v", err) } merged.NameSuffix, err = t.NameSuffix.MergeAll(input.NameSuffix) if err != nil { - return nil, errors.Wrap(err, "failed to merge NameSuffix fieldSpec") + return nil, fmt.Errorf("failed to merge NameSuffix fieldSpec: %v", err) } merged.NameSpace, err = t.NameSpace.MergeAll(input.NameSpace) if err != nil { - return nil, errors.Wrap(err, "failed to merge NameSpace fieldSpec") + return nil, fmt.Errorf("failed to merge NameSpace fieldSpec: %v", err) } merged.CommonAnnotations, err = t.CommonAnnotations.MergeAll( input.CommonAnnotations) if err != nil { - return nil, errors.Wrap(err, "failed to merge CommonAnnotations fieldSpec") + return nil, fmt.Errorf("failed to merge CommonAnnotations fieldSpec: %v", err) } merged.CommonLabels, err = t.CommonLabels.MergeAll(input.CommonLabels) if err != nil { - return nil, errors.Wrap(err, "failed to merge CommonLabels fieldSpec") + return nil, fmt.Errorf("failed to merge CommonLabels fieldSpec: %v", err) } merged.TemplateLabels, err = t.TemplateLabels.MergeAll(input.TemplateLabels) if err != nil { - return nil, errors.Wrap(err, "failed to merge TemplateLabels fieldSpec") + return nil, fmt.Errorf("failed to merge TemplateLabels fieldSpec: %v", err) } merged.VarReference, err = t.VarReference.MergeAll(input.VarReference) if err != nil { - return nil, errors.Wrap(err, "failed to merge VarReference fieldSpec") + return nil, fmt.Errorf("failed to merge VarReference fieldSpec: %v", err) } merged.NameReference, err = t.NameReference.mergeAll(input.NameReference) if err != nil { - return nil, errors.Wrap(err, "failed to merge NameReference fieldSpec") + return nil, fmt.Errorf("failed to merge NameReference fieldSpec: %v", err) } merged.Images, err = t.Images.MergeAll(input.Images) if err != nil { - return nil, errors.Wrap(err, "failed to merge Images fieldSpec") + return nil, fmt.Errorf("failed to merge Images fieldSpec: %v", err) } merged.Replicas, err = t.Replicas.MergeAll(input.Replicas) if err != nil { - return nil, errors.Wrap(err, "failed to merge Replicas fieldSpec") + return nil, fmt.Errorf("failed to merge Replicas fieldSpec: %v", err) } merged.sortFields() return merged, nil From eb1529b5162f4973fab19ba6e4a3ce5ad7522ccf Mon Sep 17 00:00:00 2001 From: Agustina Barbetta Date: Sun, 25 Sep 2022 09:19:31 -0300 Subject: [PATCH 07/10] use %w to format errors --- .../builtinconfig/transformerconfig.go | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/api/internal/plugins/builtinconfig/transformerconfig.go b/api/internal/plugins/builtinconfig/transformerconfig.go index 84830fa642..e8cac9befd 100644 --- a/api/internal/plugins/builtinconfig/transformerconfig.go +++ b/api/internal/plugins/builtinconfig/transformerconfig.go @@ -111,44 +111,44 @@ func (t *TransformerConfig) Merge(input *TransformerConfig) ( merged = &TransformerConfig{} merged.NamePrefix, err = t.NamePrefix.MergeAll(input.NamePrefix) if err != nil { - return nil, fmt.Errorf("failed to merge NamePrefix fieldSpec: %v", err) + return nil, fmt.Errorf("failed to merge NamePrefix fieldSpec: %w", err) } merged.NameSuffix, err = t.NameSuffix.MergeAll(input.NameSuffix) if err != nil { - return nil, fmt.Errorf("failed to merge NameSuffix fieldSpec: %v", err) + return nil, fmt.Errorf("failed to merge NameSuffix fieldSpec: %w", err) } merged.NameSpace, err = t.NameSpace.MergeAll(input.NameSpace) if err != nil { - return nil, fmt.Errorf("failed to merge NameSpace fieldSpec: %v", err) + return nil, fmt.Errorf("failed to merge NameSpace fieldSpec: %w", err) } merged.CommonAnnotations, err = t.CommonAnnotations.MergeAll( input.CommonAnnotations) if err != nil { - return nil, fmt.Errorf("failed to merge CommonAnnotations fieldSpec: %v", err) + return nil, fmt.Errorf("failed to merge CommonAnnotations fieldSpec: %w", err) } merged.CommonLabels, err = t.CommonLabels.MergeAll(input.CommonLabels) if err != nil { - return nil, fmt.Errorf("failed to merge CommonLabels fieldSpec: %v", err) + return nil, fmt.Errorf("failed to merge CommonLabels fieldSpec: %w", err) } merged.TemplateLabels, err = t.TemplateLabels.MergeAll(input.TemplateLabels) if err != nil { - return nil, fmt.Errorf("failed to merge TemplateLabels fieldSpec: %v", err) + return nil, fmt.Errorf("failed to merge TemplateLabels fieldSpec: %w", err) } merged.VarReference, err = t.VarReference.MergeAll(input.VarReference) if err != nil { - return nil, fmt.Errorf("failed to merge VarReference fieldSpec: %v", err) + return nil, fmt.Errorf("failed to merge VarReference fieldSpec: %w", err) } merged.NameReference, err = t.NameReference.mergeAll(input.NameReference) if err != nil { - return nil, fmt.Errorf("failed to merge NameReference fieldSpec: %v", err) + return nil, fmt.Errorf("failed to merge NameReference fieldSpec: %w", err) } merged.Images, err = t.Images.MergeAll(input.Images) if err != nil { - return nil, fmt.Errorf("failed to merge Images fieldSpec: %v", err) + return nil, fmt.Errorf("failed to merge Images fieldSpec: %w", err) } merged.Replicas, err = t.Replicas.MergeAll(input.Replicas) if err != nil { - return nil, fmt.Errorf("failed to merge Replicas fieldSpec: %v", err) + return nil, fmt.Errorf("failed to merge Replicas fieldSpec: %w", err) } merged.sortFields() return merged, nil From acba8fff62b1ecc5d5846d2026007afb3e908ad8 Mon Sep 17 00:00:00 2001 From: Agustina Barbetta Date: Sat, 8 Oct 2022 20:18:57 -0300 Subject: [PATCH 08/10] Update api/krusty/inlinelabels_test.go Co-authored-by: Katrina Verey --- api/krusty/inlinelabels_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/krusty/inlinelabels_test.go b/api/krusty/inlinelabels_test.go index ce93d2d794..95254c8dc6 100644 --- a/api/krusty/inlinelabels_test.go +++ b/api/krusty/inlinelabels_test.go @@ -224,7 +224,7 @@ spec: `) } -func TestKustomizationLabelsInTemplateWhenResourceHasNoTemplate(t *testing.T) { +func TestKustomizationLabelsDoesNotCreateInvalidTemplatePaths(t *testing.T) { th := kusttest_test.MakeHarness(t) th.WriteF("app/deployment.yaml", ` apiVersion: apps/v1 From d14dfb604c026cc1524ab11d44ed05ee6df670e1 Mon Sep 17 00:00:00 2001 From: Agustina Barbetta Date: Sat, 8 Oct 2022 20:37:24 -0300 Subject: [PATCH 09/10] Use WrapPrefixf --- .../builtinconfig/transformerconfig.go | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/api/internal/plugins/builtinconfig/transformerconfig.go b/api/internal/plugins/builtinconfig/transformerconfig.go index e8cac9befd..50f162f993 100644 --- a/api/internal/plugins/builtinconfig/transformerconfig.go +++ b/api/internal/plugins/builtinconfig/transformerconfig.go @@ -4,13 +4,13 @@ package builtinconfig import ( - "fmt" "log" "sort" "sigs.k8s.io/kustomize/api/ifc" "sigs.k8s.io/kustomize/api/konfig/builtinpluginconsts" "sigs.k8s.io/kustomize/api/types" + "sigs.k8s.io/kustomize/kyaml/errors" ) // TransformerConfig holds the data needed to perform transformations. @@ -111,44 +111,44 @@ func (t *TransformerConfig) Merge(input *TransformerConfig) ( merged = &TransformerConfig{} merged.NamePrefix, err = t.NamePrefix.MergeAll(input.NamePrefix) if err != nil { - return nil, fmt.Errorf("failed to merge NamePrefix fieldSpec: %w", err) + return nil, errors.WrapPrefixf(err, "failed to merge NamePrefix fieldSpec") } merged.NameSuffix, err = t.NameSuffix.MergeAll(input.NameSuffix) if err != nil { - return nil, fmt.Errorf("failed to merge NameSuffix fieldSpec: %w", err) + return nil, errors.WrapPrefixf(err, "failed to merge NameSuffix fieldSpec") } merged.NameSpace, err = t.NameSpace.MergeAll(input.NameSpace) if err != nil { - return nil, fmt.Errorf("failed to merge NameSpace fieldSpec: %w", err) + return nil, errors.WrapPrefixf(err, "failed to merge NameSpace fieldSpec") } merged.CommonAnnotations, err = t.CommonAnnotations.MergeAll( input.CommonAnnotations) if err != nil { - return nil, fmt.Errorf("failed to merge CommonAnnotations fieldSpec: %w", err) + return nil, errors.WrapPrefixf(err, "failed to merge CommonAnnotations fieldSpec") } merged.CommonLabels, err = t.CommonLabels.MergeAll(input.CommonLabels) if err != nil { - return nil, fmt.Errorf("failed to merge CommonLabels fieldSpec: %w", err) + return nil, errors.WrapPrefixf(err, "failed to merge CommonLabels fieldSpec") } merged.TemplateLabels, err = t.TemplateLabels.MergeAll(input.TemplateLabels) if err != nil { - return nil, fmt.Errorf("failed to merge TemplateLabels fieldSpec: %w", err) + return nil, errors.WrapPrefixf(err, "failed to merge TemplateLabels fieldSpec") } merged.VarReference, err = t.VarReference.MergeAll(input.VarReference) if err != nil { - return nil, fmt.Errorf("failed to merge VarReference fieldSpec: %w", err) + return nil, errors.WrapPrefixf(err, "failed to merge VarReference fieldSpec") } merged.NameReference, err = t.NameReference.mergeAll(input.NameReference) if err != nil { - return nil, fmt.Errorf("failed to merge NameReference fieldSpec: %w", err) + return nil, errors.WrapPrefixf(err, "failed to merge NameReference fieldSpec") } merged.Images, err = t.Images.MergeAll(input.Images) if err != nil { - return nil, fmt.Errorf("failed to merge Images fieldSpec: %w", err) + return nil, errors.WrapPrefixf(err, "failed to merge Images fieldSpec") } merged.Replicas, err = t.Replicas.MergeAll(input.Replicas) if err != nil { - return nil, fmt.Errorf("failed to merge Replicas fieldSpec: %w", err) + return nil, errors.WrapPrefixf(err, "failed to merge Replicas fieldSpec") } merged.sortFields() return merged, nil From 7b84613ad1f0817c1f8519bb7ac5c3d799c7c937 Mon Sep 17 00:00:00 2001 From: Agustina Barbetta Date: Sat, 8 Oct 2022 20:38:44 -0300 Subject: [PATCH 10/10] Use fewer labels to ease reading --- api/krusty/inlinelabels_test.go | 100 +++++++------------------------- 1 file changed, 20 insertions(+), 80 deletions(-) diff --git a/api/krusty/inlinelabels_test.go b/api/krusty/inlinelabels_test.go index 95254c8dc6..e5b82af160 100644 --- a/api/krusty/inlinelabels_test.go +++ b/api/krusty/inlinelabels_test.go @@ -324,25 +324,16 @@ apiVersion: apps/v1 kind: DaemonSet metadata: labels: - app.kubernetes.io/component: a - app.kubernetes.io/instance: b - app.kubernetes.io/name: c - app.kubernetes.io/part-of: d + app.kubernetes.io/name: daemon name: daemon spec: selector: matchLabels: - app.kubernetes.io/component: a - app.kubernetes.io/instance: b - app.kubernetes.io/name: c - app.kubernetes.io/part-of: d + app.kubernetes.io/name: daemon template: metadata: labels: - app.kubernetes.io/component: a - app.kubernetes.io/instance: b - app.kubernetes.io/name: c - app.kubernetes.io/part-of: d + app.kubernetes.io/name: daemon `) th.WriteK("/app", ` resources: @@ -360,26 +351,17 @@ apiVersion: apps/v1 kind: DaemonSet metadata: labels: - app.kubernetes.io/component: a - app.kubernetes.io/instance: b - app.kubernetes.io/name: c - app.kubernetes.io/part-of: d + app.kubernetes.io/name: daemon foo: bar name: daemon spec: selector: matchLabels: - app.kubernetes.io/component: a - app.kubernetes.io/instance: b - app.kubernetes.io/name: c - app.kubernetes.io/part-of: d + app.kubernetes.io/name: daemon template: metadata: labels: - app.kubernetes.io/component: a - app.kubernetes.io/instance: b - app.kubernetes.io/name: c - app.kubernetes.io/part-of: d + app.kubernetes.io/name: daemon foo: bar `) } @@ -391,27 +373,18 @@ apiVersion: apps/v1 kind: StatefulSet metadata: labels: - app.kubernetes.io/component: a - app.kubernetes.io/instance: b - app.kubernetes.io/name: c - app.kubernetes.io/part-of: d + app.kubernetes.io/name: set name: set spec: replicas: 3 selector: matchLabels: - app.kubernetes.io/component: a - app.kubernetes.io/instance: b - app.kubernetes.io/name: c - app.kubernetes.io/part-of: d + app.kubernetes.io/name: set serviceName: set template: metadata: labels: - app.kubernetes.io/component: a - app.kubernetes.io/instance: b - app.kubernetes.io/name: c - app.kubernetes.io/part-of: d + app.kubernetes.io/name: set `) th.WriteK("/app", ` resources: @@ -429,28 +402,19 @@ apiVersion: apps/v1 kind: StatefulSet metadata: labels: - app.kubernetes.io/component: a - app.kubernetes.io/instance: b - app.kubernetes.io/name: c - app.kubernetes.io/part-of: d + app.kubernetes.io/name: set foo: bar name: set spec: replicas: 3 selector: matchLabels: - app.kubernetes.io/component: a - app.kubernetes.io/instance: b - app.kubernetes.io/name: c - app.kubernetes.io/part-of: d + app.kubernetes.io/name: set serviceName: set template: metadata: labels: - app.kubernetes.io/component: a - app.kubernetes.io/instance: b - app.kubernetes.io/name: c - app.kubernetes.io/part-of: d + app.kubernetes.io/name: set foo: bar `) } @@ -462,10 +426,7 @@ apiVersion: batch/v1 kind: CronJob metadata: labels: - app.kubernetes.io/component: a - app.kubernetes.io/instance: b - app.kubernetes.io/name: c - app.kubernetes.io/part-of: d + app.kubernetes.io/name: job name: job spec: jobTemplate: @@ -474,10 +435,7 @@ spec: template: metadata: labels: - app.kubernetes.io/component: a - app.kubernetes.io/instance: b - app.kubernetes.io/name: c - app.kubernetes.io/part-of: d + app.kubernetes.io/name: job spec: restartPolicy: Never schedule: '* * * * *' @@ -498,10 +456,7 @@ apiVersion: batch/v1 kind: CronJob metadata: labels: - app.kubernetes.io/component: a - app.kubernetes.io/instance: b - app.kubernetes.io/name: c - app.kubernetes.io/part-of: d + app.kubernetes.io/name: job foo: bar name: job spec: @@ -514,10 +469,7 @@ spec: template: metadata: labels: - app.kubernetes.io/component: a - app.kubernetes.io/instance: b - app.kubernetes.io/name: c - app.kubernetes.io/part-of: d + app.kubernetes.io/name: job foo: bar spec: restartPolicy: Never @@ -532,20 +484,14 @@ apiVersion: batch/v1 kind: Job metadata: labels: - app.kubernetes.io/component: a - app.kubernetes.io/instance: b - app.kubernetes.io/name: c - app.kubernetes.io/part-of: d + app.kubernetes.io/name: job name: job spec: backoffLimit: 4 template: metadata: labels: - app.kubernetes.io/component: a - app.kubernetes.io/instance: b - app.kubernetes.io/name: c - app.kubernetes.io/part-of: d + app.kubernetes.io/name: job spec: restartPolicy: Never `) @@ -565,10 +511,7 @@ apiVersion: batch/v1 kind: Job metadata: labels: - app.kubernetes.io/component: a - app.kubernetes.io/instance: b - app.kubernetes.io/name: c - app.kubernetes.io/part-of: d + app.kubernetes.io/name: job foo: bar name: job spec: @@ -576,10 +519,7 @@ spec: template: metadata: labels: - app.kubernetes.io/component: a - app.kubernetes.io/instance: b - app.kubernetes.io/name: c - app.kubernetes.io/part-of: d + app.kubernetes.io/name: job foo: bar spec: restartPolicy: Never