zarf-dev / zarf

DevSecOps for Air Gap & Limited-Connection Systems. https://zarf.dev/
Apache License 2.0
1.34k stars 162 forks source link

helm override variables are limited to strings #2783

Open Michael-Kruggel opened 1 month ago

Michael-Kruggel commented 1 month ago

Steps to reproduce

  1. Add helm override variable to component
  2. Set that variable to something other than a string (e.g an array) in the zarf config
  3. Deploy package using said config

Expected result

Value of variable is set in chart values

Actual Result

No value is set, chart's default value is used

Visual Proof (screenshots, videos, text, etc)

Example zarf.yaml component:

components:
  - name: jenkins
    required: true
    charts:
      - name: jenkins
        namespace: jenkins
        url: https://charts.jenkins.io
        version: 5.4.3
        repoName: jenkins
        releaseName: jenkins
        valuesFiles:
          - ../values/common.yaml
        variables:
          - name: JENKINS_PLUGINS
            description: "List of plugins to install on Jenkins startup"
            path: controller.installPlugins

Example zarf-config.yaml:

package:
  deploy:
    components: 'jenkins'
    set:
      jenkins_plugins:
        - kubernetes:4238.v41b_3ef14a_5d8
        - workflow-aggregator:596.v8c21c963d92d
        - git:5.2.2
        - configuration-as-code:1810.v9b_c30a_249a_4c
        - oic-auth:4.269.va_7526f34f306
        - prometheus:773.v3b_62d8178eec
        - cloudbees-disk-usage-simple:203.v3f46a_7462b_1a_
        - saml:4.464.vea_cb_75d7f5e0

Values in k9s: image

Racer159 commented 1 month ago

This is relatively simple to accomplish internally to Zarf by changing string to interface{} on the SetVariables field however the wall this is effectively blocked on (https://github.com/spf13/viper/issues/1014) would need a workaround like what UDS CLI has done: https://github.com/defenseunicorns/uds-cli/blob/main/src/cmd/root.go#L151 (which as-written there only supports YAML) or would need to swap around libraries to something else.

Racer159 commented 1 month ago

The bright side is that Zarf does not process config files like this:

package:
  deploy:
    set:
      test:
        aT: thingT
        bT: to-beT

so it would be easier to stay away from making this a breaking change.

mjnagel commented 1 week ago

Tacking onto this existing issue - besides maps/lists there are also issues with passing boolean values in (they end up getting cast into strings which can cause issues in certain helm templates due to "false" being a truthy value).