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

"needs:" dependency does not work if kubecontext contains a forward slash #203

Closed
fbordallo opened this issue Jun 30, 2022 · 5 comments · Fixed by #268
Closed

"needs:" dependency does not work if kubecontext contains a forward slash #203

fbordallo opened this issue Jun 30, 2022 · 5 comments · Fixed by #268
Labels
enhancement New feature or request

Comments

@fbordallo
Copy link

Operating system

Ubuntu 20.04

Helmfile Version

v0.144.0

Helm Version

version.BuildInfo{Version:"v3.7.0", GitCommit:"eeac83883cb4014fe60267ec6373570374ce770b", GitTreeState:"clean", GoVersion:"go1.16.8"}

Bug description

This issue was mentioned at roboll/helmfile#2048 (comment) .

Helmfile fails to evaluate a needs: dependency correctly if the entry has KUBECONTEXT/NAMESPACE/RELEASE_NAME structure and KUBECONTEXT contains a forward slash.

AWS EKS Cluster context names usually have this structure: arn:aws:eks:AWS_REGION:AWS_ACCOUNT:cluster/CLUSTER_NAME.

Example helmfile.yaml

  - name: releaseB
    namespace: namespaceA
    kubeContext: arn:aws:eks:us-east-1:1234567890:cluster/myekscluster
    needs:
      - arn:aws:eks:us-east-1:1234567890:cluster/myekscluster/namespaceA/releaseA

Error message you've seen (if any)

in ./helmfile.yaml: release(s) "arn:aws:eks:us-east-1:1234567890:cluster/myekscluster/namespaceA/releaseB" depend(s) on an undefined release "myekscluster/namespaceA/releaseA". Perhaps you made a typo in "needs" or forgot defining a release named "cluster" with appropriate "namespace" and "kubeContext"?

Steps to reproduce

N/A

Relevant discussion

No response

@yxxhero
Copy link
Member

yxxhero commented Jun 30, 2022

@fbordallo please show your command?

@fbordallo
Copy link
Author

Yes, sorry. I thought it was enough info.

helmfilebug.yaml:

releases:
  - name: releaseA
    namespace: kube-system
    kubeContext: arn:aws:eks:eu-central-1:123456789:cluster/mycluster
    installed: true
    chart: ../../../../charts/cluster
    labels:
      name: cluster-support
    version: 0.1.0
  - name: releaseB
    namespace: prod
    kubeContext: arn:aws:eks:eu-central-1:123456789:cluster/mycluster
    installed: true
    chart: ../../../../charts/namespace
    labels:
      name: namespace-support
    version: 0.1.0
    needs:
      - arn:aws:eks:eu-central-1:123456789:cluster/mycluster/kube-system/releaseA

Command run:

$ helmfile --debug -i -f helmfilebug.yaml apply
processing file "helmfilebug.yaml" in directory "."
first-pass rendering starting for "helmfilebug.yaml.part.0": inherited=&{default map[] map[]}, overrode=<nil>
first-pass uses: &{default map[] map[]}
first-pass rendering output of "helmfilebug.yaml.part.0":
 0: releases:
 1:   - name: releaseA
 2:     namespace: kube-system
 3:     kubeContext: arn:aws:eks:eu-central-1:123456789:cluster/mycluster
 4:     installed: true
 5:     chart: ../../../../charts/cluster
 6:     labels:
 7:       name: cluster-support
 8:     version: 0.1.0
 9:   - name: releaseB
10:     namespace: prod
11:     kubeContext: arn:aws:eks:eu-central-1:123456789:cluster/mycluster
12:     installed: true
13:     chart: ../../../../charts/namespace
14:     labels:
15:       name: namespace-support
16:     version: 0.1.0
17:     needs:
18:       - arn:aws:eks:eu-central-1:123456789:cluster/mycluster/kube-system/releaseA
19: 

first-pass produced: &{default map[] map[]}
first-pass rendering result of "helmfilebug.yaml.part.0": {default map[] map[]}
vals:
map[]
defaultVals:[]
second-pass rendering result of "helmfilebug.yaml.part.0":
 0: releases:
 1:   - name: releaseA
 2:     namespace: kube-system
 3:     kubeContext: arn:aws:eks:eu-central-1:123456789:cluster/mycluster
 4:     installed: true
 5:     chart: ../../../../charts/cluster
 6:     labels:
 7:       name: cluster-support
 8:     version: 0.1.0
 9:   - name: releaseB
10:     namespace: prod
11:     kubeContext: arn:aws:eks:eu-central-1:123456789:cluster/mycluster
12:     installed: true
13:     chart: ../../../../charts/namespace
14:     labels:
15:       name: namespace-support
16:     version: 0.1.0
17:     needs:
18:       - arn:aws:eks:eu-central-1:123456789:cluster/mycluster/kube-system/releaseA
19: 

merged environment: &{default map[] map[]}
helm:MZVSg> v3.7.0+geeac838
Building dependency release=releaseA, chart=../../../../charts/cluster
exec: helm dependency build ../../../../charts/cluster
helm:pWRNL> Hang tight while we grab the latest from your chart repositories...
helm:pWRNL> ...Successfully got an update from the "docker-mailserver" chart repository
helm:pWRNL> ...Successfully got an update from the "nginx-stable" chart repository
helm:pWRNL> ...Successfully got an update from the "haproxy-ingress" chart repository
helm:pWRNL> ...Successfully got an update from the "sonatype" chart repository
helm:pWRNL> ...Successfully got an update from the "vmware-tanzu" chart repository
helm:pWRNL> ...Successfully got an update from the "ingress-nginx" chart repository
helm:pWRNL> ...Successfully got an update from the "eks" chart repository
helm:pWRNL> ...Successfully got an update from the "grafana" chart repository
helm:pWRNL> ...Successfully got an update from the "jenkins" chart repository
helm:pWRNL> ...Successfully got an update from the "bitnami" chart repository
helm:pWRNL> ...Successfully got an update from the "prometheus-community" chart repository
helm:pWRNL> ...Successfully got an update from the "stable" chart repository
Update Complete. ⎈Happy Helming!⎈
helm:pWRNL> Saving 2 charts
Downloading aws-load-balancer-controller from repo https://aws.github.io/eks-charts
helm:pWRNL> Downloading velero from repo https://vmware-tanzu.github.io/helm-charts/
helm:pWRNL> Deleting outdated charts
Hang tight while we grab the latest from your chart repositories...
...Successfully got an update from the "docker-mailserver" chart repository
...Successfully got an update from the "nginx-stable" chart repository
...Successfully got an update from the "haproxy-ingress" chart repository
...Successfully got an update from the "sonatype" chart repository
...Successfully got an update from the "vmware-tanzu" chart repository
...Successfully got an update from the "ingress-nginx" chart repository
...Successfully got an update from the "eks" chart repository
...Successfully got an update from the "grafana" chart repository
...Successfully got an update from the "jenkins" chart repository
...Successfully got an update from the "bitnami" chart repository
...Successfully got an update from the "prometheus-community" chart repository
...Successfully got an update from the "stable" chart repository
Update Complete. ⎈Happy Helming!⎈
Saving 2 charts
Downloading aws-load-balancer-controller from repo https://aws.github.io/eks-charts
Downloading velero from repo https://vmware-tanzu.github.io/helm-charts/
Deleting outdated charts

Building dependency release=releaseB, chart=../../../../charts/namespace
exec: helm dependency build ../../../../charts/namespace
helm:zlKEn> Hang tight while we grab the latest from your chart repositories...
helm:zlKEn> ...Successfully got an update from the "docker-mailserver" chart repository
helm:zlKEn> ...Successfully got an update from the "nginx-stable" chart repository
helm:zlKEn> ...Successfully got an update from the "haproxy-ingress" chart repository
helm:zlKEn> ...Successfully got an update from the "sonatype" chart repository
helm:zlKEn> ...Successfully got an update from the "ingress-nginx" chart repository
helm:zlKEn> ...Successfully got an update from the "eks" chart repository
helm:zlKEn> ...Successfully got an update from the "vmware-tanzu" chart repository
helm:zlKEn> ...Successfully got an update from the "jenkins" chart repository
helm:zlKEn> ...Successfully got an update from the "fh" chart repository
helm:zlKEn> ...Successfully got an update from the "grafana" chart repository
helm:zlKEn> ...Successfully got an update from the "prometheus-community" chart repository
helm:zlKEn> ...Successfully got an update from the "bitnami" chart repository
helm:zlKEn> ...Successfully got an update from the "stable" chart repository
Update Complete. ⎈Happy Helming!⎈
helm:zlKEn> Saving 1 charts
helm:zlKEn> Downloading external-dns from repo https://charts.bitnami.com/bitnami
helm:zlKEn> Deleting outdated charts
Hang tight while we grab the latest from your chart repositories...
...Successfully got an update from the "docker-mailserver" chart repository
...Successfully got an update from the "nginx-stable" chart repository
...Successfully got an update from the "haproxy-ingress" chart repository
...Successfully got an update from the "sonatype" chart repository
...Successfully got an update from the "ingress-nginx" chart repository
...Successfully got an update from the "eks" chart repository
...Successfully got an update from the "vmware-tanzu" chart repository
...Successfully got an update from the "jenkins" chart repository
...Successfully got an update from the "grafana" chart repository
...Successfully got an update from the "prometheus-community" chart repository
...Successfully got an update from the "bitnami" chart repository
...Successfully got an update from the "stable" chart repository
Update Complete. ⎈Happy Helming!⎈
Saving 1 charts
Downloading external-dns from repo https://charts.bitnami.com/bitnami
Deleting outdated charts

2 release(s) found in helmfilebug.yaml

err: release(s) "arn:aws:eks:eu-central-1:123456789:cluster/mycluster/prod/releaseB" depend(s) on an undefined release "mycluster/kube-system/releaseA". Perhaps you made a typo in "needs" or forgot defining a release named "releaseA" with appropriate "namespace" and "kubeContext"?
in ./helmfilebug.yaml: release(s) "arn:aws:eks:eu-central-1:123456789:cluster/mycluster/prod/releaseB" depend(s) on an undefined release "mycluster/kube-system/releaseA". Perhaps you made a typo in "needs" or forgot defining a release named "releaseA" with appropriate "namespace" and "kubeContext"?

@mumoshu
Copy link
Contributor

mumoshu commented Jul 6, 2022

Thanks for reporting!

Helmfile usese $kubecontext/$ns/$release_name as the internal identifier of every node in the dependency graph build from needs. It will certainly break if you had a slash within the kubecontext name- Good catch.

I think a way to fix this would be to change the separator between the internal id components to something that doesn't intersect with the characters allowed in kubecontext, ns name, and the release name.. or maybe make it possible to use a Go struct rather than a Go string as the identifier.

For the latter, we might need to somehow enhance https://github.com/variantdev/dag, which Helmfile uses under the hood to calculate the dep graph of needs.

@mumoshu mumoshu added the enhancement New feature or request label Jul 7, 2022
@stale
Copy link

stale bot commented Jul 21, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix This will not be worked on label Jul 21, 2022
@yxxhero yxxhero removed the wontfix This will not be worked on label Jul 21, 2022
@stale
Copy link

stale bot commented Aug 4, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix This will not be worked on label Aug 4, 2022
@yxxhero yxxhero removed the wontfix This will not be worked on label Aug 4, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 6, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants