Set namespace when looking up resource with externalName #246
Conversation
Fixes crossplane#245 Signed-off-by: Cyrill Troxler <cyrill@nine.ch>
| @@ -687,7 +687,7 @@ func (r *Reconciler) Reconcile(_ context.Context, req reconcile.Request) (reconc | |||
| // of the loop. In that case, we warn the user that the external resource | |||
| // might be leaked. | |||
| err := retry.OnError(retry.DefaultRetry, resource.IsAPIError, func() error { | |||
| nn := types.NamespacedName{Name: managed.GetName()} | |||
| nn := types.NamespacedName{Name: managed.GetName(), Namespace: managed.GetNamespace()} | |||
There was a problem hiding this comment.
Managed resources are intended to be cluster-scoped and the machinery here is intended to accommodate that fact. Using namespace-scoped managed resources could impact other aspects of the managed reconciler, such as cross-resource references. Do you have an example of where you are currently using the managed reconciler successfully for a namespaced resource?
There was a problem hiding this comment.
I was not aware of that :) I know that all included Crossplane providers only use cluster-scoped resources for their managed resources but so far nothing had stopped us using namespaced resources in our own provider. So far everything has worked but tbh we have not had to use cross-resource refs yet. If this is completely unsupported, I think there should be a more obvious way to tell us (docs or maybe even some check on controller start).
But of course my preference would be that this is somewhat supported so we can keep using it. I'm aware of all the other constructs that Crossplane provides to have namespaced resources (compositions, publications etc.) but sometimes that's just a bit too much configuration overhead for simple resources. So anyway, I don't have an example ready right now but there is not much to it besides setting +kubebuilder:resource:scope=Namespaced. I would prefer it if crossplane-runtime did not limit this use-case but maybe instead pointed at the limitations of it, like for example that cross-resource refs would just work if the other resource is cluster-scoped.
|
This turned out not to be a bug, I addressed the wider issue here: #251 |
Description of your changes
Set namespace when looking up resource with externalName.
Fixes #245
I have:
make reviewable testto ensure this PR is ready for review.