Skip to content

Conversation

@bobh66
Copy link
Contributor

@bobh66 bobh66 commented Aug 6, 2025

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 crossplane/docs#993

Fixes crossplane/crossplane#6694

I have:

Need help with this checklist? See the cheat sheet.

Fixes #6694

Signed-off-by: Bob Haddleton <[email protected]>
@bobh66 bobh66 requested a review from a team as a code owner August 6, 2025 21:08
@bobh66 bobh66 requested a review from jbw976 August 6, 2025 21:08
@bobh66 bobh66 marked this pull request as draft August 6, 2025 21:08
Copy link
Member

@jbw976 jbw976 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a nice convenience policy that streamlines the experience for enabling the orphaning of resources. Thanks @bobh66! just a couple questions

// PLEASE UPDATE THIS WHEN RELEASING.
"baseBranches": [
'main',
'release-1.18',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was already done in #865, so it's odd to see here again 🤔

},
want: want{delete: false},
},
"DeletionDeleteManagementActionOrphan": {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what do you think about another test case for TestShouldDelete too? e.g. make sure deletion is not allowed when this new orphan policy is set?

@lsviben
Copy link
Contributor

lsviben commented Oct 28, 2025

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 lsviben mentioned this pull request Oct 28, 2025
4 tasks
@bobh66
Copy link
Contributor Author

bobh66 commented Nov 4, 2025

@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add support for an Orphan managementPolicy

4 participants