Track resources that are pending external name assignment #279
Conversation
negz
force-pushed
the
impending
branch
2 times, most recently
from
d4cf5ec
to
7ec0887
Compare
Mostly just to get one more green line of code coverage. :) Signed-off-by: Nic Cope <negz@rk0n.org>
Some Crossplane resources have non-deterministic external names that are returned at Create time. We must record those names and rely on them to determine whether the external resources exist during subsequent Observe calls. The absence of an external name does not guarantee that we didn't create an external resource. It's possible that we created the resource but failed to update its name. It's also possible that we did update the name but have since read a stale, unnamed, version of the resource from cache. To deal with this we set a 'pending external name' annotation immediately before creating our external resource. If we get to the Create call because we have a stale view of our managed resource the Update call that persists the annotation will fail. If we are already pending external name assignment when we get to the Create call we know it's possible that we created an external resource but did not persist its name, so we refuse to proceed rather than creating a duplicate. Signed-off-by: Nic Cope <negz@rk0n.org>
Signed-off-by: Nic Cope <negz@rk0n.org>
| // annotation should be set before an external resource is created. | ||
| // Crossplane uses this annotation to ensure it creates at most one | ||
| // external resource. | ||
| AnnotationKeyExternalNamePending = "crossplane.io/external-name-pending" |
There was a problem hiding this comment.
I am wondering if we could combine this PR with #280 by using an annotation like crossplane.io/external-name-pending-since given we also set timestamp as a value here.
There was a problem hiding this comment.
I can't see a clean way to combine the two annotations into one, if that's what you're thinking? In this case it's important that we write the annotation immediately before calling create, in order to guarantee that we have the latest version of the managed resource before we create the external resource. In #280 we want to record the time that the external resource was successfully created, i.e. after the creation of the external resource.
Perhaps though we could combine them in some other way, i.e. rather than adding an external-name-pending annotation then removing it when the name was no longer pending, we could add an annotation that indicated a create was about to happen, then another one indicating it succeeded? 🤔
|
Closing in favor of #283. |
Description of your changes
Edit: I had originally hoped this would fix crossplane-contrib/provider-aws#802, but it appears there was a simpler fix for that specific case. Nonetheless, I do believe we're hypothetically vulnerable to the issue this PR addresses, so we may want to consider merging it anyhow.
Some Crossplane resources have non-deterministic external names that are returned at Create time. We must record those names and rely on them to determine whether the external resources exist during subsequent Observe calls.
The absence of an external name does not guarantee that we didn't create an external resource. It's possible that we created the resource but failed to update its name. It's also possible that we did update the name but have since read a stale, unnamed, version of the resource from cache.
To deal with this we set a 'pending external name' annotation immediately before creating our external resource. If we get to the Create call because we have a stale view of our managed resource the Update call that persists the annotation will fail. If we are already pending external name assignment when we get to the Create call we know it's possible that we created an external resource but did not persist its name, so we refuse to proceed rather than creating a duplicate.
I have:
make reviewable testto ensure this PR is ready for review.How has this code been tested
It has not yet. I'll move it out of draft once it has been tested.