Add option to treat secrets as mutable #3164

MaienM · 2024-08-14T00:01:53Z

Proposed changes

This introduces the flag enableSecretMutable which does for Secrets what the existing enableConfigMapMutable flag does for ConfigMaps. See #1926 for the motivation for this flag. Changes to the type field will still trigger a replacement as this field is immutable.

Related issues (optional)

Fixes #2291.
Fixes #3181.

github-actions · 2024-08-14T00:02:12Z

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 · 2024-08-14T01:16:04Z

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

blampe · 2024-08-16T18:56:44Z

/run-acceptance-tests

pulumi-bot · 2024-08-16T18:57:10Z

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

blampe · 2024-08-16T19:27:27Z

Thank you for the contribution! A couple test failures but looks reasonable overall. We'll review this as a team next week.

=== RUN   TestPatchToDiff/Secret_resources_trigger_a_replace_when_enableSecretMutable_is_not_set.
    diff_test.go:331: 
        	Error Trace:	/home/runner/work/pulumi-kubernetes/pulumi-kubernetes/provider/pkg/provider/diff_test.go:331
        	Error:      	Not equal: 
        	            	expected: map[string]*pulumirpc.PropertyDiff{"data.property1":(*pulumirpc.PropertyDiff)(0xc000893a70)}
        	            	actual  : map[string]*pulumirpc.PropertyDiff{"data.property1":(*pulumirpc.PropertyDiff)(0xc000748360)}
        	            	
        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -13,3 +13,3 @@
        	            	   unknownFields: ([]uint8) <nil>,
        	            	-  Kind: (pulumirpc.PropertyDiff_Kind) 5,
        	            	+  Kind: (pulumirpc.PropertyDiff_Kind) 4,
        	            	   InputDiff: (bool) false
        	Test:       	TestPatchToDiff/Secret_resources_trigger_a_replace_when_enableSecretMutable_is_not_set.
=== RUN   TestPatchToDiff/Secret_resources_don't_trigger_a_replace_when_mutable.
=== RUN   TestPatchToDiff/Secret_resources_trigger_a_replace_when_type_changes_even_if_enableSecretMutable_is_set.
    diff_test.go:331: 
        	Error Trace:	/home/runner/work/pulumi-kubernetes/pulumi-kubernetes/provider/pkg/provider/diff_test.go:331
        	Error:      	Not equal: 
        	            	expected: map[string]*pulumirpc.PropertyDiff{"data.property1":(*pulumirpc.PropertyDiff)(0xc000893a70)}
        	            	actual  : map[string]*pulumirpc.PropertyDiff{"type":(*pulumirpc.PropertyDiff)(0xc0007492c0)}
        	            	
        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -1,3 +1,3 @@
        	            	 (map[string]*pulumirpc.PropertyDiff) (len=1) {
        	            	- (string) (len=14) "data.property1": (*pulumirpc.PropertyDiff)({
        	            	+ (string) (len=4) "type": (*pulumirpc.PropertyDiff)({
        	            	   state: (impl.MessageState) {
        	Test:       	TestPatchToDiff/Secret_resources_trigger_a_replace_when_type_changes_even_if_enableSecretMutable_is_set.

github-actions · 2024-08-16T21:05:29Z

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

This does for `Secret`s what the existing `enableConfigMapMutable` flag does for `ConfigMap`s.

github-actions · 2024-08-16T21:06:51Z

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

MaienM · 2024-08-16T21:09:40Z

Fixed! One of those was some sloppy copy-pasting in the test itself, the other was an actual logic error that I introduced when I re-marked the type field as always immutable.

github-actions · 2024-08-19T18:50:24Z

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

blampe · 2024-08-19T18:53:19Z

/run-acceptance-tests

pulumi-bot · 2024-08-19T18:53:46Z

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

rquitales

LGTM - thanks for the PR!

blampe · 2024-08-20T21:15:29Z

provider/pkg/provider/diff.go

+			if clients.IsSecret(obj) && !k.enableSecretMutable {
+				props = append(props, properties{".type", ".stringData", ".data"}...)
+			} else if kindFields, kindExists := version[gvk.Kind]; kindExists {


@MaienM it just occurred to me while adding some detail to #3179 that we should probably respect the immutable field.

In other words, if the secret has immutable: true it should preserve the existing behavior even if the provider has enableSecretMutable. Does that make sense?

That makes sense, yeah, it's not like trying to treat it as mutable will even work in that case. The same applies to ConfigMaps though, right? I don't think those handle that either right now, that's what I based this code on after all.

I can make a change to handle that, should I also make the same change for ConfigMaps while I'm at it?

I've made the change for this for both Secrets and ConfigMaps. I put the change for the ConfigMaps in a separate commit because I wasn't sure whether you wanted that one, so you can just get rid of that commit if you don't.

@MaienM thank you again for the great contribution! I'm very happy this prompted us to uncover the immutable incompatibility and that you were able to fix it while you were in here. I added an E2E test case partly as a sanity check and party to cement the behavior we expect here.

So much has changed with Pulumi and k8s since this behavior was originally introduced that it would make much more sense IMO to make this the default behavior at some point in the future (#3179). I added a note to the changelog to that effect, since it's a potentially disruptive behavioral change.

After I merge this it will be available as a pre-release if you'd like to play around with it.

github-actions · 2024-08-20T22:12: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

provider/pkg/clients/clients.go

github-actions · 2024-08-21T17:22:21Z

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 · 2024-08-21T17:23:21Z

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 · 2024-08-21T17:24:14Z

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

blampe · 2024-08-21T17:25:39Z

/run-acceptance-tests

pulumi-bot · 2024-08-21T17:26:10Z

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

MaienM force-pushed the feature/enable-secret-mutable-flag branch from 9f88514 to 4deb10c Compare August 14, 2024 01:15

MaienM force-pushed the feature/enable-secret-mutable-flag branch from 4deb10c to 30cc20b Compare August 16, 2024 21:05

Add enableSecretMutable flag

4001e00

This does for `Secret`s what the existing `enableConfigMapMutable` flag does for `ConfigMap`s.

MaienM force-pushed the feature/enable-secret-mutable-flag branch from 30cc20b to 4001e00 Compare August 16, 2024 21:06

mjeffryes assigned blampe Aug 19, 2024

blampe approved these changes Aug 19, 2024

View reviewed changes

changelog

0403d40

rquitales approved these changes Aug 20, 2024

View reviewed changes

blampe reviewed Aug 20, 2024

View reviewed changes

MaienM added 2 commits August 21, 2024 00:12

Always recreate secret when marked as immutable

150ab6f

Always recreate configmap when marked as immutable

b99c9fc

MaienM commented Aug 20, 2024

View reviewed changes

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

add an e2e test for immutability

3ecf7a5

blampe force-pushed the feature/enable-secret-mutable-flag branch from 5d5621e to 9affcae Compare August 21, 2024 17:23

changelog

278cc10

blampe force-pushed the feature/enable-secret-mutable-flag branch from 9affcae to 278cc10 Compare August 21, 2024 17:23

blampe merged commit 2ec7a1a into pulumi:master Aug 21, 2024
8 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add option to treat secrets as mutable #3164

Add option to treat secrets as mutable #3164

MaienM commented Aug 14, 2024 •

edited by blampe

Loading

github-actions bot commented Aug 14, 2024

github-actions bot commented Aug 14, 2024

blampe commented Aug 16, 2024

pulumi-bot commented Aug 16, 2024

blampe commented Aug 16, 2024

github-actions bot commented Aug 16, 2024

github-actions bot commented Aug 16, 2024

MaienM commented Aug 16, 2024

github-actions bot commented Aug 19, 2024

blampe commented Aug 19, 2024

pulumi-bot commented Aug 19, 2024

rquitales left a comment

blampe Aug 20, 2024

MaienM Aug 20, 2024

MaienM Aug 20, 2024

blampe Aug 21, 2024

github-actions bot commented Aug 20, 2024

github-actions bot commented Aug 21, 2024

github-actions bot commented Aug 21, 2024

github-actions bot commented Aug 21, 2024

blampe commented Aug 21, 2024

pulumi-bot commented Aug 21, 2024

Add option to treat secrets as mutable #3164

Add option to treat secrets as mutable #3164

Conversation

MaienM commented Aug 14, 2024 • edited by blampe Loading

Proposed changes

Related issues (optional)

github-actions bot commented Aug 14, 2024

github-actions bot commented Aug 14, 2024

blampe commented Aug 16, 2024

pulumi-bot commented Aug 16, 2024

blampe commented Aug 16, 2024

github-actions bot commented Aug 16, 2024

github-actions bot commented Aug 16, 2024

MaienM commented Aug 16, 2024

github-actions bot commented Aug 19, 2024

blampe commented Aug 19, 2024

pulumi-bot commented Aug 19, 2024

rquitales left a comment

Choose a reason for hiding this comment

blampe Aug 20, 2024

Choose a reason for hiding this comment

MaienM Aug 20, 2024

Choose a reason for hiding this comment

MaienM Aug 20, 2024

Choose a reason for hiding this comment

blampe Aug 21, 2024

Choose a reason for hiding this comment

github-actions bot commented Aug 20, 2024

github-actions bot commented Aug 21, 2024

github-actions bot commented Aug 21, 2024

github-actions bot commented Aug 21, 2024

blampe commented Aug 21, 2024

pulumi-bot commented Aug 21, 2024

MaienM commented Aug 14, 2024 •

edited by blampe

Loading