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 template appends api/versions to compiled in api/versions instead of replacing #7291

Open
1 of 5 tasks
sathieu opened this issue Sep 24, 2021 · 7 comments
Open
1 of 5 tasks
Labels
bug Something isn't working

Comments

@sathieu
Copy link
Contributor

sathieu commented Sep 24, 2021

Describe the bug

There are several bugs about .Capabilities.APIVersions.

Here is my understanding of the current behavior, with identified filled issues:

To Reproduce

With a given chart, repo-server will log:

time="2021-09-08T07:55:59Z"
level=info
msg=Trace
args="[helm template . --name-template gatekeeper --namespace gatekeeper --kube-version 1.20 --values values.yaml --api-versions acme.cert-manager.io/v1 --api-versions acme.cert-manager.io/v1alpha2 --api-versions acme.cert-manager.io/v1alpha3 --api-versions acme.cert-manager.io/v1beta1 --api-versions admissionregistration.k8s.io/v1 --api-versions admissionregistration.k8s.io/v1beta1 --api-versions apiextensions.k8s.io/v1 --api-versions apiextensions.k8s.io/v1beta1 --api-versions apiregistration.k8s.io/v1 --api-versions apiregistration.k8s.io/v1beta1 --api-versions apps/v1 --api-versions argoproj.io/v1alpha1 --api-versions authentication.k8s.io/v1 --api-versions authentication.k8s.io/v1beta1 --api-versions authorization.k8s.io/v1 --api-versions authorization.k8s.io/v1beta1 --api-versions autoscaling/v1 --api-versions autoscaling/v2beta1 --api-versions autoscaling/v2beta2 --api-versions batch/v1 --api-versions batch/v1beta1 --api-versions bitnami.com/v1alpha1 --api-versions ceph.rook.io/v1 --api-versions cert-manager.io/v1 --api-versions cert-manager.io/v1alpha2 --api-versions cert-manager.io/v1alpha3 --api-versions cert-manager.io/v1beta1 --api-versions certificates.k8s.io/v1 --api-versions certificates.k8s.io/v1beta1 --api-versions config.gatekeeper.sh/v1alpha1 --api-versions coordination.k8s.io/v1 --api-versions coordination.k8s.io/v1beta1 --api-versions crd.projectcalico.org/v1 --api-versions discovery.k8s.io/v1beta1 --api-versions events.k8s.io/v1 --api-versions events.k8s.io/v1beta1 --api-versions extensions/v1beta1 --api-versions flowcontrol.apiserver.k8s.io/v1beta1 --api-versions install.istio.io/v1alpha1 --api-versions kubitus-project.gitlab.io/v1alpha1 --api-versions metacontroller.k8s.io/v1alpha1 --api-versions metrics.k8s.io/v1beta1 --api-versions monitoring.coreos.com/v1 --api-versions monitoring.coreos.com/v1alpha1 --api-versions mutations.gatekeeper.sh/v1alpha1 --api-versions networking.istio.io/v1alpha3 --api-versions networking.istio.io/v1beta1 --api-versions networking.k8s.io/v1 --api-versions networking.k8s.io/v1beta1 --api-versions node.k8s.io/v1 --api-versions node.k8s.io/v1beta1 --api-versions objectbucket.io/v1alpha1 --api-versions operators.coreos.com/v1 --api-versions operators.coreos.com/v1alpha1 --api-versions operators.coreos.com/v1alpha2 --api-versions operators.coreos.com/v2 --api-versions packages.operators.coreos.com/v1 --api-versions policy/v1beta1 --api-versions rbac.authorization.k8s.io/v1 --api-versions rbac.authorization.k8s.io/v1beta1 --api-versions replication.storage.openshift.io/v1alpha1 --api-versions rook.io/v1alpha2 --api-versions scheduling.k8s.io/v1 --api-versions scheduling.k8s.io/v1beta1 --api-versions security.istio.io/v1beta1 --api-versions status.gatekeeper.sh/v1beta1 --api-versions storage.k8s.io/v1 --api-versions storage.k8s.io/v1beta1 --api-versions telemetry.istio.io/v1alpha1 --api-versions templates.gatekeeper.sh/v1 --api-versions templates.gatekeeper.sh/v1alpha1 --api-versions templates.gatekeeper.sh/v1beta1 --api-versions v1 --include-crds]"
dir=/tmp/https___gitlab.example.org_gitops_infra/gatekeeper
operation_name="exec helm"
time_ms=265.434214

ArgoCD is correctly passing k8s version and api/versions, (but not api/version/kinds - problem 2),
however while only policy/v1beta1 is passed, policy/v1 will also be in .Capabilities.APIVersions,
because Helm was compiled with k8s 1.21. As such, a template will have the following evaluated incorrectly:

{{- if .Capabilities.APIVersions.Has "policy/v1" }}
apiVersion: policy/v1
{{ else }}
apiVersion: policy/v1beta1
{{ end -}}

As a workaround, and thanks to problem 4, changing the condition will evaluate correctly:

{{- if .Capabilities.APIVersions.Has "policy/v1/PodDisruptionBudget" }}
apiVersion: policy/v1
{{ else }}
apiVersion: policy/v1beta1
{{ end -}}

Expected behavior

.Capabilities.APIVersions to not be prefilled with values depending on the k8s version used during the Helm build.

Some cooperation is needed between ArgoCD devs and Helm devs about this, we can work on the implementation.

Proposals:

  1. Add --strict-api-versions option to Helm (add strict apiVersions, allowing templating without added versions from kube version helm/helm#10108)
  2. Only add StrictAPIVersions in the struct, and change ArgoCD to build against Helm (see here)
  3. Hardcode api/version/kind for each k8s version (rejected by Helm maintainer)
  4. Allow helm to access k8s API to fill .Capabilities.APIVersions.
  5. ... other ideas?

Version

argocd: v2.1.2+7af9dfb
  BuildDate: 2021-09-02T18:05:23Z
  GitCommit: 7af9dfb3524c13e941ab604e36e49a617fe47d2e
  GitTreeState: clean
  GoVersion: go1.16.5
  Compiler: gc
  Platform: linux/amd64
@jannfis
Copy link
Member

jannfis commented Oct 1, 2021

@rbreeze Are you sure about cherry-picking the change of this behavior into 2.1? Frankly, I think this is not a bug, but rather an enhancement to existing functionality. I agree with the v2.2 milestone, but I'd oppose cherry-picking a behavioral change like a possible fix for this into 2.1.

@alexmt alexmt removed the cherry-pick/2.1 Candidate for cherry picking into the 2.1 release branch label Oct 19, 2021
@alexmt
Copy link
Collaborator

alexmt commented Oct 19, 2021

I confused @rbreeze and asked to add cherry-pick label. Removing it

@alexmt
Copy link
Collaborator

alexmt commented Oct 19, 2021

Thanks a lot for the investigation and preparing the summary @sathieu ! It helps a lot.

I think problem 1 is solved already. The #3594 was reported for v1.5.4 which indeed did not have support for --api-versions

We can fix problems 2 and 4 in v2.2! Working on pull request.

@sathieu
Copy link
Contributor Author

sathieu commented Oct 20, 2021

Thanks @alexmt. Looking forward for those fixes!

@pjaak
Copy link

pjaak commented Dec 2, 2021

We are facing the same issues at the moment when we try to update our ingress API versions. Looking forward to the changes aswell @alexmt

andrzej-stencel added a commit to SumoLogic/sumologic-kubernetes-collection that referenced this issue Dec 7, 2021
andrzej-stencel added a commit to SumoLogic/sumologic-kubernetes-collection that referenced this issue Dec 7, 2021
andrzej-stencel added a commit to SumoLogic/sumologic-kubernetes-collection that referenced this issue Dec 7, 2021
andrzej-stencel added a commit to SumoLogic/sumologic-kubernetes-collection that referenced this issue Dec 7, 2021
@alexmt alexmt modified the milestones: v2.3, v2.4 Jan 18, 2022
@gdsoumya
Copy link
Member

gdsoumya commented Feb 4, 2022

@alexmt @sathieu can you confirm if this is what is expected for issue #3594 - PR #8371

@sathieu
Copy link
Contributor Author

sathieu commented Feb 4, 2022

@gdsoumya Don't know. I'm more interested in problem 3.

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

No branches or pull requests

7 participants