-
Notifications
You must be signed in to change notification settings - Fork 107
Open
Labels
bugSomething isn't workingSomething isn't working
Description
What happened?
When using an upjet-based provider like the AzureAD provider, I can observe that critical annotations (crossplane.io/external-name) are not updated when LateInitialize is not set although that was introduced in #224 here:
upjet/pkg/controller/external.go
Lines 252 to 273 in 4c6bfc2
| // NOTE(lsviben) although the annotations were supposed to be set and the | |
| // managed resource updated during the Create step, we are checking and | |
| // updating the annotations here due to the fact that in most cases, the | |
| // Create step is done asynchronously and the managed resource is not | |
| // updated with the annotations. That is why below we are prioritizing the | |
| // annotations update before anything else. We are setting lateInitialized | |
| // to true so that the reconciler updates the managed resource. This | |
| // behavior conflicts with management policies in which LateInitialize is | |
| // turned off. To circumvent this, we are checking if the management policy | |
| // does not contain LateInitialize and if it does not, we are updating the | |
| // annotations manually. | |
| annotationsUpdated, err := resource.SetCriticalAnnotations(tr, e.config, tfstate, string(res.State.GetPrivateRaw())) | |
| if err != nil { | |
| return managed.ExternalObservation{}, errors.Wrap(err, "cannot set critical annotations") | |
| } | |
| policyHasLateInit := policySet.HasAny(xpv1.ManagementActionLateInitialize, xpv1.ManagementActionAll) | |
| if annotationsUpdated && !policyHasLateInit { | |
| if err := e.kube.Update(ctx, mg); err != nil { | |
| return managed.ExternalObservation{}, errors.Wrap(err, errUpdateAnnotations) | |
| } | |
| annotationsUpdated = false | |
| } |
This leads to issues with non-deterministic names. I already filed a PR to address that in the crossplane-runtime (crossplane/crossplane-runtime#850), but for the sake of completeness as there must be some bug also in upjet, here aswell.
How can we reproduce it?
Apply the following group using the AzureAD provider:
apiVersion: groups.azuread.upbound.io/v1beta2
kind: Group
metadata:
name: test-group-for-lateinit
spec:
deletionPolicy: Delete
forProvider:
description: Test group for late init
displayName: test-group
owners:
- <some owner id>
securityEnabled: true
managementPolicies:
- Observe
- Create
- Update
- Delete
providerConfigRef:
name: defaultThen take a look at the annotations after the observe phase. It is missing the crossplane.io/external-name annotation.
Metadata
Metadata
Assignees
Labels
bugSomething isn't workingSomething isn't working