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

fix: 🐛 annotations leaking between aliased subcharts #829

Merged
merged 2 commits into from
Apr 18, 2023

Conversation

romeuhcf
Copy link
Contributor

@romeuhcf romeuhcf commented Apr 18, 2023

What does this PR do?

When using multiple traefik setups as subcharts, this prevents annotations to be leaking from one alias to another.

Motivation

Relates to helm/helm#11891 .

Merge mutates its first parameter and this is not isolated between multiple aliased subcharts.

I have this Chart.yaml

apiVersion: v2
name: inf-traefik
description: A Helm chart for Kubernetes
type: application
version: 0.1.0
dependencies:
  - name: traefik
    version: ">0"
    repository: https://helm.traefik.io/traefik
    alias: traefik-external
  - name: traefik
    version: ">0"
    repository: https://helm.traefik.io/traefik
    alias: traefik-internal

And this values.yaml

traefik-external:
  ingressRoute:
    dashboard:
      annotations:
        kubernetes.io/ingress.class: traefik-ext

  service:
    annotations:
      external-dns.alpha.kubernetes.io/hostname: "*.lb-ext.alpha.example.com"


traefik-internal:
  ingressRoute:
    dashboard:
      annotations:
        kubernetes.io/ingress.class: traefik-int

  service:
    name: traefik-int
    annotations:
      external-dns.alpha.kubernetes.io/hostname: "*.lb-int.alpha.example.com"

When running template I would expect that each aliased subchart receives only its specific values.

❯ helm3 template . | grep hostname:
    external-dns.alpha.kubernetes.io/hostname: '*.lb-ext.alpha.example.com' # not an actual result
    external-dns.alpha.kubernetes.io/hostname: '*.lb-int.alpha.example.com'

But rendering the template, I get both services with same value.

❯ helm template . | grep hostname:
    external-dns.alpha.kubernetes.io/hostname: '*.lb-int.alpha.example.com'
    external-dns.alpha.kubernetes.io/hostname: '*.lb-int.alpha.example.com'

More

  • Yes, I updated the tests accordingly
  • Yes, I ran make test and all the tests passed

Relates to helm/helm#11891 .

Merge mutates its first parameter and this is not isolated between multiple aliased subcharts.
@romeuhcf romeuhcf changed the title Update service.yaml Annotations leaking between aliased subcharts Apr 18, 2023
@romeuhcf romeuhcf marked this pull request as ready for review April 18, 2023 02:44
@mloiseleur mloiseleur changed the title Annotations leaking between aliased subcharts fix: 🐛 annotations leaking between aliased subcharts Apr 18, 2023
@mloiseleur
Copy link
Contributor

Hello @romeuhcf,

Thanks for this PR. It looks good.

Copy link
Contributor

@project0 project0 left a comment

Choose a reason for hiding this comment

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

can you add some tests for this? We have already something similar, but this case is not really covered.

Copy link
Contributor

@project0 project0 left a comment

Choose a reason for hiding this comment

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

I understand testing is difficult in this case.
Just to make it clear: This is actually a workaround for a bug of the helm/spring merge function as it results into some cached values.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants