From a029f3ebf3c04e8db47945ed27be97e711738515 Mon Sep 17 00:00:00 2001 From: AdamMagued Date: Fri, 2 Oct 2026 00:52:09 +0000 Subject: [PATCH] fix: prevent duplicate app.kubernetes.io/name in selector labels --- pkg/app/app_e2e_test.go | 65 +++++++++++++++++ pkg/processor/daemonset/daemonset.go | 30 +++++--- pkg/processor/daemonset/daemonset_test.go | 70 +++++++++++++++++++ pkg/processor/deployment/deployment.go | 30 +++++--- pkg/processor/deployment/deployment_test.go | 70 +++++++++++++++++++ pkg/processor/poddisruptionbudget/pdb.go | 14 +++- pkg/processor/poddisruptionbudget/pdb_test.go | 48 +++++++++++++ pkg/processor/service/service.go | 14 ++-- pkg/processor/service/service_test.go | 48 +++++++++++++ 9 files changed, 365 insertions(+), 24 deletions(-) diff --git a/pkg/app/app_e2e_test.go b/pkg/app/app_e2e_test.go index 41436828..4514467f 100644 --- a/pkg/app/app_e2e_test.go +++ b/pkg/app/app_e2e_test.go @@ -3,6 +3,7 @@ package app import ( "bufio" "os" + "strings" "testing" "github.com/arttor/helmify/pkg/config" @@ -58,3 +59,67 @@ func TestApp(t *testing.T) { assert.NoError(t, err) } } + +func TestDuplicateSelectorLabelKustomize(t *testing.T) { + const chartName = "test-dup-selector" + input := `apiVersion: apps/v1 +kind: Deployment +metadata: + name: platform-operator + namespace: default + labels: + app.kubernetes.io/name: platform-operator + control-plane: controller-manager +spec: + selector: + matchLabels: + app.kubernetes.io/name: platform-operator + control-plane: controller-manager + template: + metadata: + labels: + app.kubernetes.io/name: platform-operator + control-plane: controller-manager + spec: + containers: + - name: manager + image: controller:latest +--- +apiVersion: v1 +kind: Service +metadata: + name: platform-operator-service + labels: + app.kubernetes.io/name: platform-operator +spec: + selector: + app.kubernetes.io/name: platform-operator + control-plane: controller-manager + ports: + - port: 8080 +` + reader := bufio.NewReader(strings.NewReader(input)) + err := Start(reader, config.Config{ChartName: chartName}) + assert.NoError(t, err) + + t.Cleanup(func() { + _ = os.RemoveAll(chartName) + }) + + helmLint := action.NewLint() + helmLint.Strict = true + helmLint.Namespace = "test-ns" + result := helmLint.Run([]string{chartName}, nil) + for _, err = range result.Errors { + assert.NoError(t, err) + } + assert.Empty(t, result.Errors) + + deplContent, err := os.ReadFile(chartName + "/templates/deployment.yaml") + assert.NoError(t, err) + assert.NotContains(t, string(deplContent), "app.kubernetes.io/name: platform-operator") + + svcContent, err := os.ReadFile(chartName + "/templates/service.yaml") + assert.NoError(t, err) + assert.NotContains(t, string(svcContent), "app.kubernetes.io/name: platform-operator") +} diff --git a/pkg/processor/daemonset/daemonset.go b/pkg/processor/daemonset/daemonset.go index 7caac534..328b93b0 100644 --- a/pkg/processor/daemonset/daemonset.go +++ b/pkg/processor/daemonset/daemonset.go @@ -67,12 +67,20 @@ func (d daemonset) Process(appMeta helmify.AppMetadata, obj *unstructured.Unstru name := appMeta.TrimName(obj.GetName()) - matchLabels, err := yamlformat.Marshal(map[string]interface{}{"matchLabels": dae.Spec.Selector.MatchLabels}, 0) - if err != nil { - return true, nil, err + if dae.Spec.Selector != nil && dae.Spec.Selector.MatchLabels != nil { + delete(dae.Spec.Selector.MatchLabels, "app.kubernetes.io/name") + } + var matchLabels string + if dae.Spec.Selector != nil && len(dae.Spec.Selector.MatchLabels) > 0 { + matchLabels, err = yamlformat.Marshal(map[string]interface{}{"matchLabels": dae.Spec.Selector.MatchLabels}, 0) + if err != nil { + return true, nil, err + } + } else { + matchLabels = "matchLabels:" } matchExpr := "" - if dae.Spec.Selector.MatchExpressions != nil { + if dae.Spec.Selector != nil && dae.Spec.Selector.MatchExpressions != nil { matchExpr, err = yamlformat.Marshal(map[string]interface{}{"matchExpressions": dae.Spec.Selector.MatchExpressions}, 0) if err != nil { return true, nil, err @@ -82,11 +90,17 @@ func (d daemonset) Process(appMeta helmify.AppMetadata, obj *unstructured.Unstru selector = strings.Trim(selector, " \n") selector = string(yamlformat.Indent([]byte(selector), 4)) - podLabels, err := yamlformat.Marshal(dae.Spec.Template.ObjectMeta.Labels, 8) - if err != nil { - return true, nil, err + delete(dae.Spec.Template.ObjectMeta.Labels, "app.kubernetes.io/name") + var podLabels string + if len(dae.Spec.Template.ObjectMeta.Labels) > 0 { + podLabels, err = yamlformat.Marshal(dae.Spec.Template.ObjectMeta.Labels, 8) + if err != nil { + return true, nil, err + } + podLabels += fmt.Sprintf("\n {{- include \"%s.selectorLabels\" . | nindent 8 }}", appMeta.ChartName()) + } else { + podLabels = fmt.Sprintf(" {{- include \"%s.selectorLabels\" . | nindent 8 }}", appMeta.ChartName()) } - podLabels += fmt.Sprintf("\n {{- include \"%s.selectorLabels\" . | nindent 8 }}", appMeta.ChartName()) podAnnotations := "" if len(dae.Spec.Template.ObjectMeta.Annotations) != 0 { diff --git a/pkg/processor/daemonset/daemonset_test.go b/pkg/processor/daemonset/daemonset_test.go index 28abe1f3..6e2dfb6e 100644 --- a/pkg/processor/daemonset/daemonset_test.go +++ b/pkg/processor/daemonset/daemonset_test.go @@ -1,6 +1,7 @@ package daemonset import ( + "bytes" "testing" "github.com/arttor/helmify/pkg/metadata" @@ -73,4 +74,73 @@ func Test_daemonset_Process(t *testing.T) { assert.NoError(t, err) assert.Equal(t, false, processed) }) + + t.Run("processed with duplicate app.kubernetes.io/name in selector and pod labels", func(t *testing.T) { + const daeWithDuplicateYaml = `apiVersion: apps/v1 +kind: DaemonSet +metadata: + name: platform-operator-daemon + labels: + app.kubernetes.io/name: platform-operator + control-plane: controller-manager +spec: + selector: + matchLabels: + app.kubernetes.io/name: platform-operator + control-plane: controller-manager + template: + metadata: + labels: + app.kubernetes.io/name: platform-operator + control-plane: controller-manager + spec: + containers: + - name: manager + image: controller:latest` + obj := internal.GenerateObj(daeWithDuplicateYaml) + processed, template, err := testInstance.Process(&metadata.Service{}, obj) + assert.NoError(t, err) + assert.True(t, processed) + var buf bytes.Buffer + err = template.Write(&buf) + assert.NoError(t, err) + out := buf.String() + assert.NotContains(t, out, "app.kubernetes.io/name: platform-operator") + assert.Contains(t, out, "control-plane: controller-manager") + assert.Contains(t, out, `{{- include ".selectorLabels" . | nindent 6 }}`) + assert.Contains(t, out, `{{- include ".selectorLabels" . | nindent 8 }}`) + }) + + t.Run("processed with only app.kubernetes.io/name in selector and pod labels", func(t *testing.T) { + const daeOnlyNameYaml = `apiVersion: apps/v1 +kind: DaemonSet +metadata: + name: platform-operator-daemon + labels: + app.kubernetes.io/name: platform-operator +spec: + selector: + matchLabels: + app.kubernetes.io/name: platform-operator + template: + metadata: + labels: + app.kubernetes.io/name: platform-operator + spec: + containers: + - name: manager + image: controller:latest` + obj := internal.GenerateObj(daeOnlyNameYaml) + processed, template, err := testInstance.Process(&metadata.Service{}, obj) + assert.NoError(t, err) + assert.True(t, processed) + var buf bytes.Buffer + err = template.Write(&buf) + assert.NoError(t, err) + out := buf.String() + assert.NotContains(t, out, "app.kubernetes.io/name: platform-operator") + assert.NotContains(t, out, "matchLabels: {}") + assert.Contains(t, out, "matchLabels:\n {{- include \".selectorLabels\" . | nindent 6 }}") + assert.Contains(t, out, "labels:\n {{- include \".selectorLabels\" . | nindent 8 }}") + }) } diff --git a/pkg/processor/deployment/deployment.go b/pkg/processor/deployment/deployment.go index ad7c9234..3bc6d031 100644 --- a/pkg/processor/deployment/deployment.go +++ b/pkg/processor/deployment/deployment.go @@ -93,12 +93,20 @@ func (d deployment) Process(appMeta helmify.AppMetadata, obj *unstructured.Unstr return true, nil, err } - matchLabels, err := yamlformat.Marshal(map[string]interface{}{"matchLabels": depl.Spec.Selector.MatchLabels}, 0) - if err != nil { - return true, nil, err + if depl.Spec.Selector != nil && depl.Spec.Selector.MatchLabels != nil { + delete(depl.Spec.Selector.MatchLabels, "app.kubernetes.io/name") + } + var matchLabels string + if depl.Spec.Selector != nil && len(depl.Spec.Selector.MatchLabels) > 0 { + matchLabels, err = yamlformat.Marshal(map[string]interface{}{"matchLabels": depl.Spec.Selector.MatchLabels}, 0) + if err != nil { + return true, nil, err + } + } else { + matchLabels = "matchLabels:" } matchExpr := "" - if depl.Spec.Selector.MatchExpressions != nil { + if depl.Spec.Selector != nil && depl.Spec.Selector.MatchExpressions != nil { matchExpr, err = yamlformat.Marshal(map[string]interface{}{"matchExpressions": depl.Spec.Selector.MatchExpressions}, 0) if err != nil { return true, nil, err @@ -108,11 +116,17 @@ func (d deployment) Process(appMeta helmify.AppMetadata, obj *unstructured.Unstr selector = strings.Trim(selector, " \n") selector = string(yamlformat.Indent([]byte(selector), 4)) - podLabels, err := yamlformat.Marshal(depl.Spec.Template.ObjectMeta.Labels, 8) - if err != nil { - return true, nil, err + delete(depl.Spec.Template.ObjectMeta.Labels, "app.kubernetes.io/name") + var podLabels string + if len(depl.Spec.Template.ObjectMeta.Labels) > 0 { + podLabels, err = yamlformat.Marshal(depl.Spec.Template.ObjectMeta.Labels, 8) + if err != nil { + return true, nil, err + } + podLabels += fmt.Sprintf("\n {{- include \"%s.selectorLabels\" . | nindent 8 }}", appMeta.ChartName()) + } else { + podLabels = fmt.Sprintf(" {{- include \"%s.selectorLabels\" . | nindent 8 }}", appMeta.ChartName()) } - podLabels += fmt.Sprintf("\n {{- include \"%s.selectorLabels\" . | nindent 8 }}", appMeta.ChartName()) podAnnotations := "" if len(depl.Spec.Template.ObjectMeta.Annotations) != 0 { diff --git a/pkg/processor/deployment/deployment_test.go b/pkg/processor/deployment/deployment_test.go index 07a2046f..c3e1ad93 100644 --- a/pkg/processor/deployment/deployment_test.go +++ b/pkg/processor/deployment/deployment_test.go @@ -1,6 +1,7 @@ package deployment import ( + "bytes" "testing" "github.com/arttor/helmify/pkg/metadata" @@ -135,6 +136,75 @@ func Test_deployment_Process(t *testing.T) { assert.NoError(t, err) assert.Equal(t, false, processed) }) + + t.Run("processed with duplicate app.kubernetes.io/name in selector and pod labels", func(t *testing.T) { + const deplWithDuplicateYaml = `apiVersion: apps/v1 +kind: Deployment +metadata: + name: platform-operator + labels: + app.kubernetes.io/name: platform-operator + control-plane: controller-manager +spec: + selector: + matchLabels: + app.kubernetes.io/name: platform-operator + control-plane: controller-manager + template: + metadata: + labels: + app.kubernetes.io/name: platform-operator + control-plane: controller-manager + spec: + containers: + - name: manager + image: controller:latest` + obj := internal.GenerateObj(deplWithDuplicateYaml) + processed, template, err := testInstance.Process(&metadata.Service{}, obj) + assert.NoError(t, err) + assert.True(t, processed) + var buf bytes.Buffer + err = template.Write(&buf) + assert.NoError(t, err) + out := buf.String() + assert.NotContains(t, out, "app.kubernetes.io/name: platform-operator") + assert.Contains(t, out, "control-plane: controller-manager") + assert.Contains(t, out, `{{- include ".selectorLabels" . | nindent 6 }}`) + assert.Contains(t, out, `{{- include ".selectorLabels" . | nindent 8 }}`) + }) + + t.Run("processed with only app.kubernetes.io/name in selector and pod labels", func(t *testing.T) { + const deplOnlyNameYaml = `apiVersion: apps/v1 +kind: Deployment +metadata: + name: platform-operator + labels: + app.kubernetes.io/name: platform-operator +spec: + selector: + matchLabels: + app.kubernetes.io/name: platform-operator + template: + metadata: + labels: + app.kubernetes.io/name: platform-operator + spec: + containers: + - name: manager + image: controller:latest` + obj := internal.GenerateObj(deplOnlyNameYaml) + processed, template, err := testInstance.Process(&metadata.Service{}, obj) + assert.NoError(t, err) + assert.True(t, processed) + var buf bytes.Buffer + err = template.Write(&buf) + assert.NoError(t, err) + out := buf.String() + assert.NotContains(t, out, "app.kubernetes.io/name: platform-operator") + assert.NotContains(t, out, "matchLabels: {}") + assert.Contains(t, out, "matchLabels:\n {{- include \".selectorLabels\" . | nindent 6 }}") + assert.Contains(t, out, "labels:\n {{- include \".selectorLabels\" . | nindent 8 }}") + }) } var singleQuotesTest = []struct { diff --git a/pkg/processor/poddisruptionbudget/pdb.go b/pkg/processor/poddisruptionbudget/pdb.go index 99635d14..e98f4a9e 100644 --- a/pkg/processor/poddisruptionbudget/pdb.go +++ b/pkg/processor/poddisruptionbudget/pdb.go @@ -61,9 +61,17 @@ func (r pdb) Process(appMeta helmify.AppMetadata, obj *unstructured.Unstructured name := appMeta.TrimName(obj.GetName()) nameCamel := strcase.ToLowerCamel(name) - selector, _ := yaml.Marshal(pdb.Spec.Selector) - selector = yamlformat.Indent(selector, 4) - selector = bytes.TrimRight(selector, "\n ") + if pdb.Spec.Selector != nil && pdb.Spec.Selector.MatchLabels != nil { + delete(pdb.Spec.Selector.MatchLabels, "app.kubernetes.io/name") + } + var selector string + if pdb.Spec.Selector == nil || (len(pdb.Spec.Selector.MatchLabels) == 0 && len(pdb.Spec.Selector.MatchExpressions) == 0) { + selector = " matchLabels:" + } else { + s, _ := yaml.Marshal(pdb.Spec.Selector) + s = yamlformat.Indent(s, 4) + selector = string(bytes.TrimRight(s, "\n ")) + } if spec.MaxUnavailable != nil { _, err := values.Add(spec.MaxUnavailable.IntValue(), nameCamel, "maxUnavailable") diff --git a/pkg/processor/poddisruptionbudget/pdb_test.go b/pkg/processor/poddisruptionbudget/pdb_test.go index 2645506b..0f687277 100644 --- a/pkg/processor/poddisruptionbudget/pdb_test.go +++ b/pkg/processor/poddisruptionbudget/pdb_test.go @@ -1,6 +1,7 @@ package poddisruptionbudget import ( + "bytes" "os" "testing" @@ -39,4 +40,51 @@ func Test_pdb_Process(t *testing.T) { assert.NoError(t, err) assert.Equal(t, false, processed) }) + + t.Run("processed with duplicate app.kubernetes.io/name in selector", func(t *testing.T) { + const pdbWithDuplicateYaml = `apiVersion: policy/v1 +kind: PodDisruptionBudget +metadata: + name: my-pdb +spec: + minAvailable: 1 + selector: + matchLabels: + app.kubernetes.io/name: platform-operator + control-plane: controller-manager` + obj := internal.GenerateObj(pdbWithDuplicateYaml) + processed, template, err := testInstance.Process(&metadata.Service{}, obj) + assert.NoError(t, err) + assert.True(t, processed) + var buf bytes.Buffer + err = template.Write(&buf) + assert.NoError(t, err) + out := buf.String() + assert.NotContains(t, out, "app.kubernetes.io/name: platform-operator") + assert.Contains(t, out, "control-plane: controller-manager") + assert.Contains(t, out, `{{- include ".selectorLabels" . | nindent 6 }}`) + }) + + t.Run("processed with only app.kubernetes.io/name in selector", func(t *testing.T) { + const pdbOnlyNameYaml = `apiVersion: policy/v1 +kind: PodDisruptionBudget +metadata: + name: my-pdb +spec: + minAvailable: 1 + selector: + matchLabels: + app.kubernetes.io/name: platform-operator` + obj := internal.GenerateObj(pdbOnlyNameYaml) + processed, template, err := testInstance.Process(&metadata.Service{}, obj) + assert.NoError(t, err) + assert.True(t, processed) + var buf bytes.Buffer + err = template.Write(&buf) + assert.NoError(t, err) + out := buf.String() + assert.NotContains(t, out, "app.kubernetes.io/name: platform-operator") + assert.NotContains(t, out, "{}") + assert.Contains(t, out, "matchLabels:\n {{- include \".selectorLabels\" . | nindent 6 }}") + }) } diff --git a/pkg/processor/service/service.go b/pkg/processor/service/service.go index 0cc77354..54e0a9b9 100644 --- a/pkg/processor/service/service.go +++ b/pkg/processor/service/service.go @@ -23,8 +23,7 @@ const ( svcTempSpec = ` spec: type: {{ .Values.%[1]s.type }} - selector: -%[2]s + selector:%[2]s {{- include "%[3]s.selectorLabels" . | nindent 4 }}%[4]s ports: {{- .Values.%[1]s.ports | toYaml | nindent 2 }}` @@ -80,9 +79,14 @@ func (r svc) Process(appMeta helmify.AppMetadata, obj *unstructured.Unstructured shortName := strings.TrimPrefix(name, "controller-manager-") shortNameCamel := strcase.ToLowerCamel(shortName) - selector, _ := yaml.Marshal(service.Spec.Selector) - selector = yamlformat.Indent(selector, 4) - selector = bytes.TrimRight(selector, "\n ") + delete(service.Spec.Selector, "app.kubernetes.io/name") + var selector string + if len(service.Spec.Selector) > 0 { + s, _ := yaml.Marshal(service.Spec.Selector) + s = yamlformat.Indent(s, 4) + s = bytes.TrimRight(s, "\n ") + selector = "\n" + string(s) + } values := helmify.Values{} svcType := service.Spec.Type diff --git a/pkg/processor/service/service_test.go b/pkg/processor/service/service_test.go index 134e6c91..e4c4493a 100644 --- a/pkg/processor/service/service_test.go +++ b/pkg/processor/service/service_test.go @@ -1,6 +1,7 @@ package service import ( + "bytes" "testing" "github.com/arttor/helmify/internal" @@ -77,4 +78,51 @@ func Test_svc_Process(t *testing.T) { assert.True(t, found) assert.Len(t, ipFamilies, 2) }) + + t.Run("processed with duplicate app.kubernetes.io/name in selector", func(t *testing.T) { + const svcWithDuplicateYaml = `apiVersion: v1 +kind: Service +metadata: + name: my-service +spec: + ports: + - port: 8080 + selector: + app.kubernetes.io/name: platform-operator + control-plane: controller-manager` + obj := internal.GenerateObj(svcWithDuplicateYaml) + processed, template, err := testInstance.Process(&metadata.Service{}, obj) + assert.NoError(t, err) + assert.True(t, processed) + var buf bytes.Buffer + err = template.Write(&buf) + assert.NoError(t, err) + out := buf.String() + assert.NotContains(t, out, "app.kubernetes.io/name: platform-operator") + assert.Contains(t, out, "control-plane: controller-manager") + assert.Contains(t, out, `{{- include ".selectorLabels" . | nindent 4 }}`) + }) + + t.Run("processed with only app.kubernetes.io/name in selector", func(t *testing.T) { + const svcOnlyNameYaml = `apiVersion: v1 +kind: Service +metadata: + name: my-service +spec: + ports: + - port: 8080 + selector: + app.kubernetes.io/name: platform-operator` + obj := internal.GenerateObj(svcOnlyNameYaml) + processed, template, err := testInstance.Process(&metadata.Service{}, obj) + assert.NoError(t, err) + assert.True(t, processed) + var buf bytes.Buffer + err = template.Write(&buf) + assert.NoError(t, err) + out := buf.String() + assert.NotContains(t, out, "app.kubernetes.io/name: platform-operator") + assert.NotContains(t, out, "{}") + assert.Contains(t, out, "selector:\n {{- include \".selectorLabels\" . | nindent 4 }}") + }) }