Add MustCreate management policy #873
Conversation
Signed-off-by: Bob Haddleton <[email protected]>
Signed-off-by: Bob Haddleton <[email protected]>
| // All actions explicitly set, the same as default. | ||
| sets.New[xpv1.ManagementAction](xpv1.ManagementActionObserve, xpv1.ManagementActionCreate, xpv1.ManagementActionUpdate, xpv1.ManagementActionLateInitialize, xpv1.ManagementActionDelete), | ||
| // All actions explicitly set with MustCreate instead of Create. | ||
| sets.New[xpv1.ManagementAction](xpv1.ManagementActionObserve, xpv1.ManagementActionMustCreate, xpv1.ManagementActionUpdate, xpv1.ManagementActionLateInitialize, xpv1.ManagementActionDelete), |
There was a problem hiding this comment.
the combinatorics are starting to get cumbersome here - are we already tracking loosening up our requirement to define every possible allowed combination of policies? i couldn't find a tracking issue for that just looking now...
There was a problem hiding this comment.
This was to ensure folks dont shoot themselves in the foot by adding unsupported/untested combinations, or combinations that dont make sense. Most of the sensible ones are here already.
Adding more policies will make this worse and if we do we should rethink this.
| never := time.Time{} | ||
| // If the resource already exists, the MustCreate policy is set, and there are no create annotations then | ||
| // this MR did not create the resource and an error is raised. | ||
| if observation.ResourceExists && policy.MustCreate() && meta.GetExternalCreatePending(managed).Equal(never) && meta.GetExternalCreateSucceeded(managed).Equal(never) && meta.GetExternalCreateFailed(managed).Equal(never) { |
There was a problem hiding this comment.
should these checks for all the external create annotations be made into a helper function? e.g. similar to ExternalCreateIncomplete and ExternalCreateSucceededDuring
|
Similarly to the comment on adding the Orphan management policy, I am vary of adding new policies to the management policies except for the primitives we have. Adding In general I think the basic policies should follow the methods of the external managed client. However, I do understand where the issue is coming from and what we are trying to achive here, I just don't think adding a new management policy is the right way. I wonder should we introduce some management options field? We could add |
3 participants
Description of your changes
Added a
MustCreatemanagement policy that requires the external resource to NOT exist before the MR can create it.Fixes #872
Tested using provider-kubernetes running a private branch of crossplane-runtime. Created a
Secretand also anObjectcontaining aSecretwith the same name but different data, andmanagementPolicies: ['MustCreate', 'Observe', 'Update', 'LateInitialize', 'Delete']and verified that when theObjectwas created theSecretremained unchanged and theObjectreported an error that theSecretit was trying to create was already in existence.Docs PR is crossplane/docs#993
I have:
earthly +reviewableto ensure this PR is ready for review.- [ ] Addedbackport release-x.ylabels to auto-backport this PR.Need help with this checklist? See the cheat sheet.