Fix Remove app.kubernetes.io/instance label in crd

[choisungwook]

When deploying keda with argocd, changing the releasename keep causes a warning in the argocd application. for example, argocd appplication name is keda-test and releaseName: keda, keep causes a warning

apiVersion: argoproj.io/v1alpha1
kind: Application
metadata:
  name: keda-test
  namespace: argocd
spec:
  source:
    helm:
      releaseName: keda

Argocd offical document say that
Please note that overriding the Helm release name might cause problems when the chart you are deploying is using the app.kubernetes.io/instance.

And, I use argo rollout helm charts, karpenter helm charts same method. but argocd application has not warning. because these helm charts have no app.kubernetes.io/instance label in crd.

So. I think that remove app.kubernetes.io/instance label in crd. thank you.

Checklist

• I have verified that my change is according to the deprecations & breaking changes policy
• Commits are signed with Developer Certificate of Origin (DCO - learn more)
• README is updated with new configuration values (if applicable) learn more
• A PR is opened to update KEDA core (repo) (if applicable, ie. when deployment manifests are modified)

[JorTurFer]

I see your point, but the proposed change in the tpl file will affect all the places where keda.labels is used (I'd say that everywhere). I guess that we can create other group of labels for CRD , something like keda.crd-labels, with just the basic labels. We could generate keda.labels appending the extra labels to keda.crd-labels.
WDYT?

[choisungwook]

you're right. How about define the labels directly in the CRD. For example

{{- if .Values.crds.install }}
apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:  
  labels:
    app.kubernetes.io/name: {{ .Values.operator.name }}
    app.kubernetes.io/component: operator
    app.kubernetes.io/managed-by: {{ .Release.Service }} 
    # Remove
    # app.kubernetes.io/instance: {{ .Release.Name }}
    app.kubernetes.io/part-of: {{ .Values.operator.name }}
  

[JorTurFer]

Currently, we use a tpl helper:

{{/*
Generate basic labels
*/}}
{{- define "keda.labels" }}
helm.sh/chart: {{ include "keda.chart" . }}
app.kubernetes.io/component: operator
app.kubernetes.io/managed-by: {{ .Release.Service }}
app.kubernetes.io/instance: {{ .Release.Name }}
app.kubernetes.io/part-of: {{ .Values.operator.name }}
{{- if .Chart.AppVersion }}
app.kubernetes.io/version: {{ .Chart.AppVersion }}
{{- end }}
{{- if .Values.additionalLabels }}
{{ toYaml .Values.additionalLabels }}
{{- end }}
{{- end }}

that's included on CRDs:

  labels:
    app.kubernetes.io/name: {{ .Values.operator.name }}
    {{- include "keda.labels" . | indent 4 }}

I suggest creating other helper, for example:

{{/*
Generate basic labels
*/}}
{{- define "keda.basic-labels" }}
helm.sh/chart: {{ include "keda.chart" . }}
{{- if .Chart.AppVersion }}
app.kubernetes.io/version: {{ .Chart.AppVersion }}
{{- end }}
{{- if .Values.additionalLabels }}
{{ toYaml .Values.additionalLabels }}
{{- end }}
{{- end }}

{{/*
Generate labels
*/}}
{{- define "keda.labels" }}
{{ include "basic-labels" . }}
app.kubernetes.io/component: operator
app.kubernetes.io/managed-by: {{ .Release.Service }}
app.kubernetes.io/instance: {{ .Release.Name }}
app.kubernetes.io/part-of: {{ .Values.operator.name }}
{{- end }}

And change the CRD labels:

  labels:
    {{- include "keda.basic-labels" . | indent 4 }}

I've written the code on the fly, so maybe there are errors, but I hope that the idea is more clear

[choisungwook]

@JorTurFer Could you review code? and The workflow "WhiteSource Security Check" was stuck.

After some time, I realized that the issue I PR'd was an ArgoCD issue. So I think it's okay to close the PR.

However, I think it needs to be refactored to use tpl for CRD .metadata.labels. Other open source helm charts don't use tpl for CRD .metadata.labels. I don't know much about CRDs yet, so I will study more and post a PR later if I can.

[JorTurFer]

this LGTM. WDYT @tomkerkhove ?