From e5668082edc3bf40d7d2d6bdcf43e0189426d528 Mon Sep 17 00:00:00 2001 From: Amine Date: Wed, 27 Mar 2024 15:25:51 +0100 Subject: [PATCH] Improve handling of missing namespace in `SecretKeyReference` (#146) Fixes https://github.com/aws-controllers-k8s/community/issues/1699 When reconciling a resource that references a Kubernetes Secret via a `SecretKeyReference`, if the namespace field is not specified, the code previously defaulted to the "default" namespace. This commit changes that behavior to instead use the namespace of the resource being reconciled, if available in the reconcile context. Signed-off-by: Amine Hilaly By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license. --- pkg/runtime/reconciler.go | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/pkg/runtime/reconciler.go b/pkg/runtime/reconciler.go index d5956d3..9a2409b 100644 --- a/pkg/runtime/reconciler.go +++ b/pkg/runtime/reconciler.go @@ -126,8 +126,18 @@ func (r *reconciler) SecretValueFromReference( } namespace := ref.Namespace - if namespace == "" { - namespace = "default" + // During the reconcile process, the resourceNamespace is stored in the context + // and can be used to fetch the secret if the namespace is not provided in the + // SecretKeyReference. + // + // NOTE(a-hilaly): When refactoring the runtime, we might want to consider passing + // the ObjectMeta in the context. + ctxResourceNamespace := ctx.Value("resourceNamespace") + if namespace == "" && ctxResourceNamespace != nil { + ctxNamespace, ok := ctxResourceNamespace.(string) + if ok { + namespace = ctxNamespace + } } nsn := client.ObjectKey{ @@ -175,6 +185,7 @@ func (r *resourceReconciler) Reconcile(ctx context.Context, req ctrlrt.Request) // We're storing a logger pointer in the context, so that any changes to the logger // will be reflected in the context. ctx = context.WithValue(ctx, ackrtlog.ContextKey, rlog) + ctx = context.WithValue(ctx, "resourceNamespace", req.Namespace) // If a user has specified a namespace that is annotated with the // an owner account ID, we need an appropriate role ARN to assume