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

Deploying Helm charts that use strict value validation breaks with Garden #3962

Closed
twelvemo opened this issue Mar 15, 2023 · 5 comments · Fixed by #5827
Closed

Deploying Helm charts that use strict value validation breaks with Garden #3962

twelvemo opened this issue Mar 15, 2023 · 5 comments · Fixed by #5827
Assignees

Comments

@twelvemo
Copy link
Collaborator

Bug

Current Behavior

It is not possible to install helm charts with Garden that use strict values validation via a values.schema.json that include the additionalProperties: false stanza. For an overview of json schema validation for helm values see here.
Garden adds custom values to a chart deployed with Garden to add some metadata to the helm release. This is an example of a values file that Garden produces:

service:
  type: LoadBalancer
  ports:
    - name: https
      port: 443
      protocol: TCP
      targetPort: 443
.garden:
  moduleName: istio-gateway
  projectName: istio-example
  version: v-5f013b4127

When additionalProperties: false is set in the values.schema.json of a chart the deploy will fail with following error:

Error: INSTALLATION FAILED: values don't meet the specifications of the schema(s) in the following chart(s):
gateway:
- (root): Additional property .garden is not allowed

Expected behavior

Garden deploys the helm chart.

Reproducible example

Deploy the istio gateway Helm Chart with Garden e.g.:

kind: Deploy
name: istio-gateway
description: Install istio gateway helm chart
type: helm
spec:
  chart:
    name: gateway
    version: 1.17.1
    repo: https://istio-release.storage.googleapis.com/charts

Workaround

If you maintain the chart, consider setting additionalProperties: true in the validation schema. If it is a public chart the only way to install it is to download it and store it locally and set additionalProperties: true.
There is an open issue in Helm like helm/helm#10398 to override the validation client-side.

Suggested solution(s)

Store the Garden related metadata in a configmap or secret in the same namespace similar to test results instead of Helm values.

Additional context

If you run into this error please give the issue a thumbsup, currently we only saw this with the istio chart.

Your environment

  • OS:
  • How I'm running Kubernetes:

garden version

@worldofgeese
Copy link
Contributor

A community member was bitten by this bug just now when attempting to deploy jupyterhub/jupyterhub. I have validated this bug myself using the following action provided by the community member:

kind: Deploy
type: helm
name: platformhub-helm
spec:
  releaseName: jupyterhub
  namespace: jupyterhub
  chart:
    repo: https://jupyterhub.github.io/helm-chart/
    name: jupyterhub
    version: "1.2.0"

The Discord forum post is available here.

@worldofgeese worldofgeese added the priority:high High priority issue or feature label Jul 21, 2023
@worldofgeese worldofgeese moved this to Candidate in Core Weekly Jul 21, 2023
@Walther
Copy link
Contributor

Walther commented Jul 25, 2023

This depends on upstream helm:

@vvagaytsev vvagaytsev removed this from Core Weekly Aug 17, 2023
@DnMllr
Copy link

DnMllr commented Sep 7, 2023

Hello, I've also run into this issue with regards to the helm chart for the open telemetry collector. As you can see, additionalProperties are set to false in values.schema.json. While helm/helm#11510 seems like it should get in sometime soon, it will also be an all or nothing solution regarding schema validation. It might be nice to benefit from both schema validation and also use garden to manage values, which, if I understand correctly, will require garden to move its metadata into a configmap or secret regardless of what helm does upstream, as @twelvemo suggested.

@jamesylgan
Copy link

+1 - having the same OpenTelemetry Collector <> Garden issue.

@salotz
Copy link

salotz commented Jan 23, 2024

Hitting this myself for Jupyterhub. Upstream looks like they are dragging their feet. Starting to see a pattern with Helm features.
I think the only option is to assume Helm developers will do nothing, so I'm +1 on the more robust ConfigMap value mgmt.

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

Successfully merging a pull request may close this issue.

7 participants