Add support for Orphan management policy
Description of your changes
Added an Orphan management policy value which maps to ['Observe', 'Create', 'Update', 'LateInitialize'] in order to simplify the migration from deletionPolicy to managementPolicies and facilitate controlled orphaning of resources.
Fixes #6694 (Crossplane)
Tested using provider-kubernetes running a private branch of crossplane-runtime. Created an Object containing a Secret with the managementPolicies: ['Orphan'] and verified that when the Object was deleted the Secret remained.
Docs PR is https://github.com/crossplane/docs/pull/993
Fixes https://github.com/crossplane/crossplane/issues/6694
I have:
- [X] Read and followed Crossplane's contribution process.
- [X] Run
earthly +reviewableto ensure this PR is ready for review. - [X] Added or updated unit tests.
- [X] Linked a PR or a docs tracking issue to document this change.
~- [ ] Added
backport release-x.ylabels to auto-backport this PR.~
Need help with this checklist? See the cheat sheet.
IMO the idea of management policies is that we define primitives like Observe, Create, Update, Delete that clearly state what will be done with the resources. I intentionally didnt put LateInitialize here as its a bit of an outlier and in hindsight should have been a separate feature outside of management policies.
Adding more policies when we can do the same with the existing ones seems the move in the wrong direction. IMO it would just complicate things more. As its not a primitive, but a policy like * which contains more policies it would be another special case that cannot be combined with other policies.
If we really need a simplifier for management policies, maybe we should introduce a separate field:
managementPolicyAlias which would be translated into managementPolicies.
So you could just set managementPolicyAlias: Orphan which would fill in managementPolicies: ['Observe', 'Create', 'Update', 'LateInitialize'].
@lsviben I don't disagree that adding Orphan is a workaround, but I'm not sure that adding another field is a better solution. deletionPolicy was a simple and clean interface - maybe managementPolicies should have been a map instead of a list, which would have made it easier to convert from deletionPolicy, and would also make it easier to do things like mustCreate:
managementPolicies:
observe: true # true/false
create: required # true/false/requried
update: false # true/false
delete: false # true/false
@lsviben I don't disagree that adding
Orphanis a workaround, but I'm not sure that adding another field is a better solution.deletionPolicywas a simple and clean interface - maybemanagementPoliciesshould have been a map instead of a list, which would have made it easier to convert fromdeletionPolicy, and would also make it easier to do things likemustCreate:managementPolicies: observe: true # true/false create: required # true/false/requried update: false # true/false delete: false # true/false
In hindsight and with the context we have today, maybe it would be easier. Although I am wondering if there would be even more things popipng up along with required.
But we should work with what we have today unless we want to make breaking changes. I just added some propositions that came to my mind how to overcome these issues without making changes to the managementpolicies API itself.