Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updating templates to remove blank annotations #486

Merged
merged 1 commit into from
Jun 29, 2023

Conversation

emctl
Copy link
Contributor

@emctl emctl commented Jun 23, 2023

When using the keda chart, I noticed that annotations can generate empty key's when no additional annotations are passed in e.g.

# Source: keda/templates/manager/service.yaml
apiVersion: v1
kind: Service
metadata:
  annotations:
  labels:
    app.kubernetes.io/name: keda-operator    
    helm.sh/chart: keda-2.11.0
    app.kubernetes.io/component: operator
    app.kubernetes.io/managed-by: Helm
    app.kubernetes.io/instance: release-name
    app.kubernetes.io/part-of: keda-operator
    app.kubernetes.io/version: 2.11.0
  name: keda-operator
  namespace: default
spec:
  ports:
  - name: metricsservice
    port: 9666
    targetPort: 9666
  selector:
    app: keda-operator
---

I am attempted to just alter the helm template logic to only create the annotation key should there be a value passed in. I have added a before and after yaml from running helm template locally. I carried out some local testing where I passed in a number of different values (i.e. not just AdditionalAnnotations) and seemed ok to me.

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)

Fixes #

Files for comparison:
beforeAfter.zip

@emctl emctl force-pushed the annotationKeyRemoval branch 5 times, most recently from 3e4dd79 to 6127f2e Compare June 23, 2023 19:29
Signed-off-by: emctl <8000237+emctl@users.noreply.github.com>
@emctl emctl force-pushed the annotationKeyRemoval branch from 6127f2e to 3e08287 Compare June 23, 2023 19:30
@emctl emctl marked this pull request as ready for review June 23, 2023 19:35
@emctl emctl requested a review from a team as a code owner June 23, 2023 19:35
Copy link
Member

@JorTurFer JorTurFer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the improvement! ❤️

@JorTurFer JorTurFer merged commit 008b720 into kedacore:main Jun 29, 2023
@tomkerkhove tomkerkhove mentioned this pull request Jun 29, 2023
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants