Add option to treat configmaps as mutable #1926

viveklak · 2022-03-10T20:18:55Z

Proposed changes

This is a workaround for configmap in particular while #1775 awaits some additional support from the schema (pulumi/pulumi#9158)

Enabling the enableConfigMapMutable flag will treat configmaps as mutable - i.e. convert what would previously be a replacement to an update when mutated. If the provider flag is enabled, the previous behavior can be opted-in on a resource by resource basis by specifying the replaceOnChanges: [*] option. If the flag is not enabled (default), existing replacement behavior would be retained.

Related issues (optional)

Fixes #1847

github-actions · 2022-03-10T20:21:56Z

Does the PR have any schema changes?

Looking good! No breaking changes found.
No new resources/functions.

github-actions · 2022-03-10T21:09:48Z

Does the PR have any schema changes?

Looking good! No breaking changes found.
No new resources/functions.

github-actions · 2022-03-10T21:58:57Z

Does the PR have any schema changes?

Looking good! No breaking changes found.
No new resources/functions.

lblackstone · 2022-03-11T00:22:44Z

The implementation looks fine, but I'm concerned about two things with this change:

I'm leery about changing the default replace behavior. The intent with the replaceOnChanges approach was to keep the current behavior for now, but allow overrides. This seems like more of a major release change since it might break workloads that were expecting the current behavior.
The immutable flag wasn't introduced until k8s 1.18 (stable in 1.21), so users of older cluster versions wouldn't have any way to opt out of the changed behavior.

I'd be ok gating the behavior change behind an opt-in provider flag, but wonder if we'd want to explicitly tie it to the immutable flag. Alternatively, we could use a provider flag to change the default from replace to update, and support replacements explicitly with the replaceOnChanges resource option.

viveklak · 2022-03-11T06:20:35Z

The implementation looks fine, but I'm concerned about two things with this change:

I'm leery about changing the default replace behavior. The intent with the replaceOnChanges approach was to keep the current behavior for now, but allow overrides. This seems like more of a major release change since it might break workloads that were expecting the current behavior.

Yes, I had the same impression. Perhaps I am missing something obvious here though. Based on my reading of pulumi/pulumi#7226 there is no way currently to undo the replacement (marking replaceOnChanges undefined etc.) as you suggested in pulumi/pulumi#9158 (comment). So sounds like regardless, we would have to flip the default interpretation of configMaps from being immutable to mutable to address this problem. replaceOnChanges can be used to support the old behavior though (i.e. add replaceOnChanges: [".data"] instead of using the immutable field). Nonetheless this is still a break right? The only way to avoid that I can see is to fix the engine's step_generator handling of replaceOnChanges to also handle the "unsetting" case (e.g. treating undefined replaceOnChanges to mean no replacement to perform). This might be still complicated since the interpretation will vary based on what version of the CLI is running.

I'd be ok gating the behavior change behind an opt-in provider flag, but wonder if we'd want to explicitly tie it to the immutable flag. Alternatively, we could use a provider flag to change the default from replace to update, and support replacements explicitly with the replaceOnChanges resource option.

I am fine with the latter approach which I alluded to above as well (use replaceOnChanges to trigger replacement instead of immutable field).

lblackstone · 2022-03-11T13:33:25Z

there is no way currently to undo the replacement

It looks like the mergeOpts function should support overriding something specified in the SDK. i.e., if the SDK has replaceOnChanges: [".data"] set by default, it could be overridden by the user setting replaceOnChanges: [], but perhaps I'm mistaken about that.

This might be still complicated since the interpretation will vary based on what version of the CLI is running.

Good point; I hadn't considered that. My first thought is that we could check the engine version at runtime, but I'm not sure if the provider has access to that information. That would be good to find out if that's a possibility.

I am fine with the latter approach which I alluded to above as well (use replaceOnChanges to trigger replacement instead of immutable field).

Let's try the provider flag approach and see how that looks. We could flip the default replacement behavior for both ConfigMap and Secret, and indicate how to use replaceOnChanges to opt into the previous behavior on a per-resource basis.

viveklak · 2022-03-11T18:08:27Z

Let's try the provider flag approach and see how that looks. We could flip the default replacement behavior for both ConfigMap and Secret, and indicate how to use replaceOnChanges to opt into the previous behavior on a per-resource basis.

Yeah that sounds good! Will move forward with that approach.

viveklak · 2022-03-11T22:29:38Z

@lblackstone this is now ready for a review.

github-actions · 2022-03-11T22:30:36Z

Does the PR have any schema changes?

Looking good! No breaking changes found.
No new resources/functions.

lblackstone

Implementation looks good. I don't see any SDK changes, so you'll need to make sure that you regenerate those as well with the schema changes.

CHANGELOG.md

provider/pkg/gen/schema.go

Co-authored-by: Levi Blackstone <levi@pulumi.com>

github-actions · 2022-03-14T17:15:44Z

Does the PR have any schema changes?

Looking good! No breaking changes found.
No new resources/functions.

github-actions · 2022-03-14T17:15:47Z

Does the PR have any schema changes?

Looking good! No breaking changes found.
No new resources/functions.

github-actions · 2022-03-14T17:16:17Z

Does the PR have any schema changes?

Looking good! No breaking changes found.
No new resources/functions.

github-actions · 2022-03-14T17:16:21Z

Does the PR have any schema changes?

Looking good! No breaking changes found.
No new resources/functions.

github-actions · 2022-03-14T17:30:05Z

Does the PR have any schema changes?

Looking good! No breaking changes found.
No new resources/functions.

DrorBuhnik · 2023-01-15T00:48:46Z

Hi,
Can we also have enableSecretMutable flag to so the same thing for secrets?

I applied the flag for cm and it works as expected, but there is no comparable workaround for secrets

Thank you!

lblackstone · 2023-01-18T17:56:05Z

I opened #2291 to track the option for Secrets.

This introduces the flag `enableSecretMutable` which does for `Secret`s what the existing `enableConfigMapMutable` flag does for `ConfigMap`s. See #1926 for the motivation for this flag. Changes to the `type` field will still trigger a replacement as this field is immutable. Fixes #2291. Fixes #3181. --------- Co-authored-by: Bryce Lampe <bryce@pulumi.com>

viveklak marked this pull request as draft March 10, 2022 20:29

viveklak requested a review from lblackstone March 10, 2022 20:29

viveklak marked this pull request as ready for review March 11, 2022 22:17

viveklak added 5 commits March 11, 2022 14:27

Make configmaps mutable unless marked explicitly

45709b0

Fix test

c1a264e

Fix formatting

431eb25

Add provider flag to treat configmaps as mutable

07bb63d

Changelog

d2856cd

viveklak force-pushed the vl/1847 branch from dfea91f to d2856cd Compare March 11, 2022 22:27

viveklak changed the title ~~Make configmaps mutable unless marked explicitly~~ Add option to treat configmaps as mutable Mar 11, 2022

lblackstone requested changes Mar 14, 2022

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

CHANGELOG.md Outdated Show resolved Hide resolved

CHANGELOG.md Outdated Show resolved Hide resolved

provider/pkg/gen/schema.go Outdated Show resolved Hide resolved

provider/pkg/gen/schema.go Outdated Show resolved Hide resolved

viveklak and others added 4 commits March 14, 2022 10:12

Update provider/pkg/gen/schema.go

0b6315f

Co-authored-by: Levi Blackstone <levi@pulumi.com>

Update provider/pkg/gen/schema.go

a000f8b

Co-authored-by: Levi Blackstone <levi@pulumi.com>

Update CHANGELOG.md

9d62c60

Co-authored-by: Levi Blackstone <levi@pulumi.com>

Update CHANGELOG.md

ec6b9b6

Co-authored-by: Levi Blackstone <levi@pulumi.com>

viveklak added 2 commits March 14, 2022 10:23

Add SDK/schema changes

403633a

Make recommended changelog changes

3e394aa

viveklak requested a review from lblackstone March 14, 2022 17:27

lblackstone approved these changes Mar 14, 2022

View reviewed changes

viveklak merged commit 056b832 into master Mar 14, 2022

pulumi-bot deleted the vl/1847 branch March 14, 2022 18:09

viveklak mentioned this pull request Mar 14, 2022

Update dependencies to address dependabot warnings #1930

Merged

viveklak mentioned this pull request Jun 6, 2022

Document immutable ConfigMap/Secret behavior for our provider #1568

Open

lblackstone mentioned this pull request Jan 18, 2023

Add option to treat Secrets as mutable #2291

Closed

MaienM mentioned this pull request Aug 14, 2024

Add option to treat secrets as mutable #3164

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add option to treat configmaps as mutable #1926

Add option to treat configmaps as mutable #1926

viveklak commented Mar 10, 2022 •

edited

Loading

github-actions bot commented Mar 10, 2022

github-actions bot commented Mar 10, 2022

github-actions bot commented Mar 10, 2022

lblackstone commented Mar 11, 2022

viveklak commented Mar 11, 2022

lblackstone commented Mar 11, 2022 •

edited

Loading

viveklak commented Mar 11, 2022

viveklak commented Mar 11, 2022

github-actions bot commented Mar 11, 2022

lblackstone left a comment

github-actions bot commented Mar 14, 2022

github-actions bot commented Mar 14, 2022

github-actions bot commented Mar 14, 2022

github-actions bot commented Mar 14, 2022

github-actions bot commented Mar 14, 2022

DrorBuhnik commented Jan 15, 2023

lblackstone commented Jan 18, 2023

Add option to treat configmaps as mutable #1926

Add option to treat configmaps as mutable #1926

Conversation

viveklak commented Mar 10, 2022 • edited Loading

Proposed changes

Related issues (optional)

github-actions bot commented Mar 10, 2022

Does the PR have any schema changes?

github-actions bot commented Mar 10, 2022

Does the PR have any schema changes?

github-actions bot commented Mar 10, 2022

Does the PR have any schema changes?

lblackstone commented Mar 11, 2022

viveklak commented Mar 11, 2022

lblackstone commented Mar 11, 2022 • edited Loading

viveklak commented Mar 11, 2022

viveklak commented Mar 11, 2022

github-actions bot commented Mar 11, 2022

Does the PR have any schema changes?

lblackstone left a comment

Choose a reason for hiding this comment

github-actions bot commented Mar 14, 2022

Does the PR have any schema changes?

github-actions bot commented Mar 14, 2022

Does the PR have any schema changes?

github-actions bot commented Mar 14, 2022

Does the PR have any schema changes?

github-actions bot commented Mar 14, 2022

Does the PR have any schema changes?

github-actions bot commented Mar 14, 2022

Does the PR have any schema changes?

DrorBuhnik commented Jan 15, 2023

lblackstone commented Jan 18, 2023

viveklak commented Mar 10, 2022 •

edited

Loading

lblackstone commented Mar 11, 2022 •

edited

Loading