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

Helm chart not correctly rendering when using additional annotations and azure workload identity #437

Open
tkennes opened this issue May 8, 2023 · 5 comments · May be fixed by #439
Open
Labels
bug Something isn't working

Comments

@tkennes
Copy link

tkennes commented May 8, 2023

A clear and concise description of what the bug is.

Expected Behavior

Helm template rendering

Actual Behavior

Issue. Cannot convert YAML to JSON

Steps to Reproduce the Problem

  1. Set podIdentity.azureWorkload.enabled: true
  2. Set additionalAnnotations: key: value
  3. Run a helm template dry-run

Specifications

  • KEDA Version: 2.10.2

Fix

I tried to submit a pull request myself, but I'm not allowed to push a branch.

@tkennes tkennes added the bug Something isn't working label May 8, 2023
@jared-schmidt-niceincontact
Copy link

jared-schmidt-niceincontact commented May 8, 2023

I'm trying to use additionalAnnotations from the 2.9.4 chart which has nindent in it and it isn't working either. There's something broken going on with the annotations. I'm wondering if some neighboring field is gobbling whitespace incorrectly. I'm using aws annotations instead of azure.

@tkennes
Copy link
Author

tkennes commented May 9, 2023

Running a dry-run for the sa gives me this:

# Source: nlaks-keda/charts/keda/templates/01-serviceaccount.yaml
apiVersion: v1
kind: ServiceAccount
metadata:
  labels:
    app.kubernetes.io/name: keda-operator
    azure.workload.identity/use: "true"    
    helm.sh/chart: keda-2.10.2
    app.kubernetes.io/component: operator
    app.kubernetes.io/managed-by: Helm
    app.kubernetes.io/instance: keda-deployment
    app.kubernetes.io/part-of: keda-operator
    app.kubernetes.io/version: 2.10.1
  annotations:    keda: keda
    azure.workload.identity/client-id: "8684c9da-0d13-4d14-89e3-1bef458d3940"
    azure.workload.identity/tenant-id: "ac9c9f22-b439-4f4e-848c-a4b0fe3a8215"
    azure.workload.identity/service-account-token-expiration: "3600"
  name: keda-operator
  namespace: tmp
automountServiceAccountToken: true

hence why I mentioned nindent. What do you get for AWS?

@tomkerkhove
Copy link
Member

Looks like the issue is probably here:
annotations: keda: keda

Are you willing to contribute a fix?

@tkennes
Copy link
Author

tkennes commented May 9, 2023

Exactly, that's why I proposed the nindent :).

Sure. But I'll provide a couple more fixes. The reason I have this weird annotation is because of issues on my servicemonitors, it's a bit wonky with resources in a permanent state of out-of-sync under ArgoCD management.

@jared-schmidt-niceincontact
Copy link

jared-schmidt-niceincontact commented May 15, 2023

I opened a PR (#441) that revamps annotations all over the helm chart so they will actually work correctly. There is a problem where the '{}' gets included in the annotations even when you override the value with something else. There's also an issue with empty annotations. So, I modified that in all of the templates. The PR also fixes the whitespace gobbling issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants