Pulumi Helm Deployments: Allow Null Values in Configs #2220

cdibble · 2022-10-27T17:23:59Z

Proposed changes

I need to be able to pass ingress.tls=[] to the Apache Superset helm chart, which won't deploy unless that config is populated, even if it is with a null entry. At this time, null values like that are scrubbed out of helm configs in the mergeMaps function, so you can not pass a null value to your helm deployment.

This PR would add a boolean to allow null values through mergeMaps in helm_release.go and expose that option in the SDKs.

Related issues (optional)

Addresses #2089

…scrubbing them during helm value merges

…values in values.yamls in the python api

…source schema. this commit is the result of running make ensure build install given the schema updates

Helm provider/allow null values

github-actions · 2022-10-27T17:24:15Z

PR is now waiting for a maintainer to run the acceptance tests.
Note for the maintainer: To run the acceptance tests, please comment /run-acceptance-tests on the PR

cdibble · 2022-10-27T17:25:53Z

@lblackstone I went ahead and opened this PR after merging it in my fork (cdibble#1). Please LMK if you need additional input from me :)

lblackstone · 2022-10-28T01:50:54Z

/run-acceptance-tests

pulumi-bot · 2022-10-28T01:51:28Z

Please view the PR build: https://github.com/pulumi/pulumi-kubernetes/actions/runs/3342207244

lblackstone

Thanks for the PR! I think the code changes look good, and I've kicked off the CI tests to make sure that's passing. Just one minor update needed on the changelog, and I think this will be ready to merge.

CHANGELOG.md

github-actions · 2022-10-31T18:42:41Z

PR is now waiting for a maintainer to run the acceptance tests.
Note for the maintainer: To run the acceptance tests, please comment /run-acceptance-tests on the PR

github-actions · 2022-10-31T18:44:01Z

PR is now waiting for a maintainer to run the acceptance tests.
Note for the maintainer: To run the acceptance tests, please comment /run-acceptance-tests on the PR

cdibble · 2022-10-31T18:45:09Z

Thanks for the PR! I think the code changes look good, and I've kicked off the CI tests to make sure that's passing. Just one minor update needed on the changelog, and I think this will be ready to merge.

@lblackstone Just resolved that comment- thank you! Please feel free to ping me if you need anything else from me. Thanks for the quick review :)

lblackstone · 2022-10-31T19:31:46Z

/run-acceptance-tests

pulumi-bot · 2022-10-31T19:32:13Z

Please view the PR build: https://github.com/pulumi/pulumi-kubernetes/actions/runs/3364176538

lblackstone · 2022-10-31T19:33:30Z

@cdibble The CI tests caught an issue: https://github.com/pulumi/pulumi-kubernetes/actions/runs/3342207244/jobs/5534197567#step:20:477

Looks like you'll need to update the function signature in a test.

CONTRIBUTING.md

github-actions · 2022-10-31T20:55:39Z

PR is now waiting for a maintainer to run the acceptance tests.
Note for the maintainer: To run the acceptance tests, please comment /run-acceptance-tests on the PR

cdibble · 2022-10-31T20:58:16Z

@cdibble The CI tests caught an issue: https://github.com/pulumi/pulumi-kubernetes/actions/runs/3342207244/jobs/5534197567#step:20:477

Looks like you'll need to update the function signature in a test.

@lblackstone Ok I just updated that test, hopefully that takes care of it. This does just maintain the previous test coverage, rather than extending it to include coverage of this functionality. I imagine the latter is preferable- I'll take a closer look to see if I can work out how to do that and put another commit on here.

github-actions · 2022-10-31T21:26:40Z

PR is now waiting for a maintainer to run the acceptance tests.
Note for the maintainer: To run the acceptance tests, please comment /run-acceptance-tests on the PR

cdibble · 2022-10-31T21:27:52Z

@lblackstone I just pushed a commit that adds one more test with this functionality turned on. We can revert that if it's not desired of course. TBH I am not totally confident I got the patterns right and don't have a dev environment to try it out locally. Let me know if you have any comments :)

lblackstone · 2022-11-01T00:51:28Z

/run-acceptance-tests

pulumi-bot · 2022-11-01T00:52:01Z

Please view the PR build: https://github.com/pulumi/pulumi-kubernetes/actions/runs/3365898906

provider/pkg/provider/helm_release_test.go

github-actions · 2022-11-02T15:34:55Z

PR is now waiting for a maintainer to run the acceptance tests.
Note for the maintainer: To run the acceptance tests, please comment /run-acceptance-tests on the PR

lblackstone · 2022-11-02T16:07:52Z

/run-acceptance-tests

pulumi-bot · 2022-11-02T16:08:20Z

Please view the PR build: https://github.com/pulumi/pulumi-kubernetes/actions/runs/3379190809

cdibble · 2022-11-04T19:01:53Z

FYI I see the error and will try to revise this to get it working. Might take a minute as I think I need to improve my understanding of Go fundamentals and figure out how to run these tests locally.

…ue to merge is nil (where IsValid() fails)

github-actions · 2022-11-07T22:15:16Z

PR is now waiting for a maintainer to run the acceptance tests.
Note for the maintainer: To run the acceptance tests, please comment /run-acceptance-tests on the PR

lblackstone

Ah, I see the problem. I think my suggested change should fix it.

provider/pkg/provider/helm_release.go

github-actions · 2022-11-07T22:29:02Z

PR is now waiting for a maintainer to run the acceptance tests.
Note for the maintainer: To run the acceptance tests, please comment /run-acceptance-tests on the PR

lblackstone · 2022-11-07T22:29:13Z

/run-acceptance-tests

pulumi-bot · 2022-11-07T22:29:37Z

Please view the PR build: https://github.com/pulumi/pulumi-kubernetes/actions/runs/3414606305

cdibble · 2022-11-07T22:42:44Z

@lblackstone Alrighty- I've enjoyed getting to know Go a bit better and I think that the latest commit should allow these tests to pass with the expected merged map. Please let me know if you have a moment to re-run those CI tests and I'd as always be happy to address any questions or comments.

IsValid was erroneously skipping nil values, so inverted the check

github-actions · 2022-11-07T23:32:15Z

PR is now waiting for a maintainer to run the acceptance tests.
Note for the maintainer: To run the acceptance tests, please comment /run-acceptance-tests on the PR

lblackstone · 2022-11-07T23:32:28Z

/run-acceptance-tests

pulumi-bot · 2022-11-07T23:32:58Z

Please view the PR build: https://github.com/pulumi/pulumi-kubernetes/actions/runs/3414969238

lblackstone · 2022-11-07T23:33:57Z

@lblackstone Alrighty- I've enjoyed getting to know Go a bit better and I think that the latest commit should allow these tests to pass with the expected merged map. Please let me know if you have a moment to re-run those CI tests and I'd as always be happy to address any questions or comments.

Thanks for your patience on this. I couldn't figure out why the test was still failing, so I went ahead and debugged it. Turns out that the IsValid function returns false for nil values, so it still wasn't working quite right. I pushed a new commit that passes the tests.

cdibble · 2022-11-08T00:15:23Z

Thanks for your patience on this. I couldn't figure out why the test was still failing, so I went ahead and debugged it. Turns out that the IsValid function returns false for nil values, so it still wasn't working quite right. I pushed a new commit that passes the tests.

No problem at all. Thanks for your help. Hopefully we're good to go now but please do tag me if this needs any further attention.

cdibble and others added 9 commits September 27, 2022 15:40

allow null values in helm release optionally rather than mandatorily …

6778a1c

…scrubbing them during helm value merges

provide a pathway to set the new helm_release argument to allow null …

196c107

…values in values.yamls in the python api

alphabetize the Options

1e09311

adds the new allowNullValues option to the Resource Schema

b75d81f

updates changelong and contributing readdme

16e3102

adds the new allowNullValues to the Resource Schema

030ac6b

updated SDK builds which include the allowNullValues option in the re…

a099953

…source schema. this commit is the result of running make ensure build install given the schema updates

Merge branch 'master' into helm_provider/allow_null_values

e6665a5

Merge pull request #1 from cdibble/helm_provider/allow_null_values

d86e8b9

Helm provider/allow null values

lblackstone reviewed Oct 28, 2022

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

move changelog entry to section UNRELEASED

0da1d60

Merge branch 'master' into master

4d979bd

cdibble commented Oct 31, 2022

View reviewed changes

CONTRIBUTING.md Show resolved Hide resolved

pass false for the allowNullValues arguemnt to mergeMaps in testing

1353a66

expands Test_MergeMaps to include allowNullValues = false|true

7833327

lblackstone reviewed Nov 1, 2022

View reviewed changes

provider/pkg/provider/helm_release_test.go Show resolved Hide resolved

add allowNil param to struct definition for helm_release_test

5a4e129

mapToInterface should assign nil not make recursive call when the val…

1ac180d

…ue to merge is nil (where IsValid() fails)

lblackstone requested changes Nov 7, 2022

View reviewed changes

provider/pkg/provider/helm_release.go Outdated Show resolved Hide resolved

Update mapToInterface logic

f0e78dd

Additional fixes

3adb1ea

IsValid was erroneously skipping nil values, so inverted the check

lblackstone approved these changes Nov 8, 2022

View reviewed changes

lblackstone merged commit e1d68db into pulumi:master Nov 8, 2022

lblackstone mentioned this pull request Nov 15, 2022

Empty array value ignored in Helm Chart #2089

Closed

cdibble mentioned this pull request Dec 6, 2022

Make allowNullValues an InputProperty #2255

Merged

kwohlfahrt mentioned this pull request Oct 14, 2023

Unable to delete a Chart default key #2034

Closed

EronWright mentioned this pull request Nov 2, 2023

AllowNullValues causes a panic when there's a null value #2622

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pulumi Helm Deployments: Allow Null Values in Configs #2220

Pulumi Helm Deployments: Allow Null Values in Configs #2220

cdibble commented Oct 27, 2022 •

edited

Loading

github-actions bot commented Oct 27, 2022

cdibble commented Oct 27, 2022

lblackstone commented Oct 28, 2022

pulumi-bot commented Oct 28, 2022

lblackstone left a comment

github-actions bot commented Oct 31, 2022

github-actions bot commented Oct 31, 2022

cdibble commented Oct 31, 2022

lblackstone commented Oct 31, 2022

pulumi-bot commented Oct 31, 2022

lblackstone commented Oct 31, 2022

github-actions bot commented Oct 31, 2022

cdibble commented Oct 31, 2022

github-actions bot commented Oct 31, 2022

cdibble commented Oct 31, 2022

lblackstone commented Nov 1, 2022

pulumi-bot commented Nov 1, 2022

github-actions bot commented Nov 2, 2022

lblackstone commented Nov 2, 2022

pulumi-bot commented Nov 2, 2022

cdibble commented Nov 4, 2022

github-actions bot commented Nov 7, 2022

lblackstone left a comment

github-actions bot commented Nov 7, 2022

lblackstone commented Nov 7, 2022

pulumi-bot commented Nov 7, 2022

cdibble commented Nov 7, 2022

github-actions bot commented Nov 7, 2022

lblackstone commented Nov 7, 2022

pulumi-bot commented Nov 7, 2022

lblackstone commented Nov 7, 2022

cdibble commented Nov 8, 2022

Pulumi Helm Deployments: Allow Null Values in Configs #2220

Pulumi Helm Deployments: Allow Null Values in Configs #2220

Conversation

cdibble commented Oct 27, 2022 • edited Loading

Proposed changes

Related issues (optional)

github-actions bot commented Oct 27, 2022

cdibble commented Oct 27, 2022

lblackstone commented Oct 28, 2022

pulumi-bot commented Oct 28, 2022

lblackstone left a comment

Choose a reason for hiding this comment

github-actions bot commented Oct 31, 2022

github-actions bot commented Oct 31, 2022

cdibble commented Oct 31, 2022

lblackstone commented Oct 31, 2022

pulumi-bot commented Oct 31, 2022

lblackstone commented Oct 31, 2022

github-actions bot commented Oct 31, 2022

cdibble commented Oct 31, 2022

github-actions bot commented Oct 31, 2022

cdibble commented Oct 31, 2022

lblackstone commented Nov 1, 2022

pulumi-bot commented Nov 1, 2022

github-actions bot commented Nov 2, 2022

lblackstone commented Nov 2, 2022

pulumi-bot commented Nov 2, 2022

cdibble commented Nov 4, 2022

github-actions bot commented Nov 7, 2022

lblackstone left a comment

Choose a reason for hiding this comment

github-actions bot commented Nov 7, 2022

lblackstone commented Nov 7, 2022

pulumi-bot commented Nov 7, 2022

cdibble commented Nov 7, 2022

github-actions bot commented Nov 7, 2022

lblackstone commented Nov 7, 2022

pulumi-bot commented Nov 7, 2022

lblackstone commented Nov 7, 2022

cdibble commented Nov 8, 2022

cdibble commented Oct 27, 2022 •

edited

Loading