search

pulumi / pulumi-kubernetes

A Pulumi resource provider for Kubernetes to manage API resources and workloads in running clusters
https://www.pulumi.com/docs/reference/clouds/kubernetes/
Apache License 2.0
406 stars 115 forks source link

Propagate RetainOnDelete to child resources #3078

Closed blampe closed 2 months ago

blampe commented 3 months ago

WIP - needs tests.

Closes #3065

github-actions[bot] commented 3 months ago

Does the PR have any schema changes?

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

codecov[bot] commented 3 months ago

Codecov Report

Attention: Patch coverage is 69.23077% with 4 lines in your changes missing coverage. Please review.

Project coverage is 36.64%. Comparing base (d418c9a) to head (0e18243).

Files Patch % Lines
provider/pkg/provider/yaml/v2/yaml.go 33.33% 2 Missing and 2 partials :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #3078 +/- ## ========================================== + Coverage 36.60% 36.64% +0.03% ========================================== Files 71 71 Lines 9259 9268 +9 ========================================== + Hits 3389 3396 +7 - Misses 5531 5532 +1 - Partials 339 340 +1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

EronWright commented 3 months ago

Here's a thought-experiment to argue against taking this change.

Imagine you are deploying a third-party Chart and, over time, the chart version changes and, over time, the chart's resources change. For example, a RoleBinding isn't needed anymore.

Now, imagine the user has marked the chart as protect and/or retainOnDelete. Should the user get errors over time about how the child resources cannot be deleted? Should the old RoleBinding be "retained"? I think no.

I believe that the engine is in a better position to 'get it right'. Imagine the children aren't marked as retainOnDelete, but the chart is, and the user attempts to delete the chart. If the engine did propagation, and if the propagation logic was applied only at deletion time, Pulumi would correctly retain all the children. In other words, an inherited retain may have a slightly different semantic, e.g. inherited only at deletion time. This would allow the internal structure of the component to vary over time without any UX issues.

EronWright commented 2 months ago

@blampe can we close this PR?

blampe commented 2 months ago

Recommendation will be to use transforms to apply this to child resources.