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

Empty array value ignored in Helm Chart #2089

Closed
kwohlfahrt opened this issue Jul 18, 2022 · 3 comments
Closed

Empty array value ignored in Helm Chart #2089

kwohlfahrt opened this issue Jul 18, 2022 · 3 comments
Assignees
Labels
kind/bug Some behavior is incorrect or out of spec resolution/fixed This issue was fixed
Milestone

Comments

@kwohlfahrt
Copy link

kwohlfahrt commented Jul 18, 2022

What happened?

I tried to deploy a helm chart, setting a config key to an empty array value (i.e. []). The key was omitted entirely.

Steps to reproduce

Only the second step is really needed, but doing the first one makes the output of pulumi preview --diff show the issue clearly.

Deploy a Helm chart, where a key is set to a non-empty value. For example, this one:

new k8s.helm.v3.Release(
  "aws-efs-csi-driver",
  {
    chart: "aws-efs-csi-driver",
    version: "2.2.7",
    repositoryOpts: { repo: "https://kubernetes-sigs.github.io/aws-efs-csi-driver" },
    values: { node: { tolerations: [{key: "foo"}] } },
  },
);

Then, set the key to an empty value:

new k8s.helm.v3.Release(
  "aws-efs-csi-driver",
  {
    chart: "aws-efs-csi-driver",
    version: "2.2.7",
    repositoryOpts: { repo: "https://kubernetes-sigs.github.io/aws-efs-csi-driver" },
    values: { node: { tolerations: [] } }, // THIS LINE CHANGED!
  },
);

Preview the second change, with pulumi preview --diff.

Expected Behavior

I expect to see the tolerations key replaced with an empty array, probably like so (though I can't generate actual output due to this bug):

  pulumi:pulumi:Stack: (same)
    [urn=urn:pulumi:kai::k8s::pulumi:pulumi:Stack::k8s-kai]
    ~ kubernetes:helm.sh/v3:Release: (update)
        [id=aws/aws-efs-csi-driver-bd73cca4]
        [urn=urn:pulumi:kai::k8s::kubernetes:helm.sh/v3:Release::aws-efs-csi-driver]
        [provider=urn:pulumi:kai::k8s::pulumi:providers:kubernetes::k8s::3c98ce34-0966-4049-9301-d9ec94fa8194]
      ~ values: {
          ~ node: {
              ~ tolerations: [
              -     [0]: {
                      - key: "foo"
                    }
                ]
            }
        }

Actual Behavior

The tolerations key is deleted entirely (note the symbol in front of tolerations is a -, I expected a ~).

  pulumi:pulumi:Stack: (same)
    [urn=urn:pulumi:kai::k8s::pulumi:pulumi:Stack::k8s-kai]
    ~ kubernetes:helm.sh/v3:Release: (update)
        [id=aws/aws-efs-csi-driver-bd73cca4]
        [urn=urn:pulumi:kai::k8s::kubernetes:helm.sh/v3:Release::aws-efs-csi-driver]
        [provider=urn:pulumi:kai::k8s::pulumi:providers:kubernetes::k8s::3c98ce34-0966-4049-9301-d9ec94fa8194]
      ~ values: {
          ~ node: {
              - tolerations: [
              -     [0]: {
                      - key: "foo"
                    }
                ]
            }
        }

Versions used

CLI          
Version      3.36.0
Go Version   go1.17.11
Go Compiler  gc

Plugins
NAME        VERSION
aws         5.10.0
kubernetes  3.20.0
nodejs      unknown
tls         4.6.0

Host     
OS       darwin
Version  12.4
Arch     arm64

This project is written in nodejs: executable='/Users/kai/.nix-profile/bin/node' version='v16.15.0'

Backend        
Name           pulumi.com
URL            https://app.pulumi.com/kwohlfahrt
User           kwohlfahrt
Organizations  kwohlfahrt, CHARM-Tx

Dependencies:
NAME                VERSION
@pulumi/aws         5.10.0
@pulumi/kubernetes  3.20.0
@pulumi/pulumi      3.36.0
@pulumi/tls         4.6.0
@types/node         14.18.22
ts-node             10.9.1
typescript          4.7.4
zod                 3.17.3

Pulumi locates its logs in /var/folders/3b/r11hff0x1335_th38vhwwmbh0000gn/T/ by default

Additional context

This may be caused by the same underlying issue as #2034.

Contributing

Vote on this issue by adding a 👍 reaction.
To contribute a fix for this issue, leave a comment (and link to your pull request, if you've opened one already).

@kwohlfahrt kwohlfahrt added kind/bug Some behavior is incorrect or out of spec needs-triage Needs attention from the triage team labels Jul 18, 2022
@viveklak
Copy link
Contributor

Thanks for the bug report! This seems to be caused due to our exclusion of nulls in values:

func mergeMaps(a, b map[string]interface{}) (map[string]interface{}, error) {
. We can probably relax this but will need to test and validate that is true.

@viveklak viveklak removed the needs-triage Needs attention from the triage team label Jul 19, 2022
@viveklak viveklak self-assigned this Jul 19, 2022
@viveklak viveklak added this to the 0.76 milestone Jul 19, 2022
@viveklak viveklak modified the milestones: 0.76, 0.77 Aug 12, 2022
@lukehoban lukehoban modified the milestones: 0.77, 0.78 Sep 8, 2022
@cdibble
Copy link
Contributor

cdibble commented Sep 27, 2022

This issue is affecting me as I attempt to use Pulumi to manage helm deployments of Apache Superset. That chart has an ingress.tls value that must be set even if it is empty.

Noting the comment above, I thought I'd see if I could get a patch working to allow one to optionally pass null values through mergeMaps. I'm unable to get tests passing on the master branch at the moment (though the build and install steps all seem to work fine), so I definitely haven't figured out how I can install and test the patched branch locally.

However, I figured I'd put this link up- if anyone has any advice or comments, I'm all ears. Please bear with me as I am not a Go developer :)

https://github.com/cdibble/pulumi-kubernetes/pull/1/files

@lblackstone
Copy link
Member

lblackstone commented Nov 15, 2022

This was fixed in #2220 -- thanks to @cdibble for the PR!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Some behavior is incorrect or out of spec resolution/fixed This issue was fixed
Projects
None yet
Development

No branches or pull requests

5 participants