Skip to content

Commit

Permalink
Improve handling of missing namespace in SecretKeyReference (#146)
Browse files Browse the repository at this point in the history
Fixes aws-controllers-k8s/community#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 <hilalyamine@gmail.com>

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
  • Loading branch information
a-hilaly authored Mar 27, 2024
1 parent f67ec48 commit e566808
Showing 1 changed file with 13 additions and 2 deletions.
15 changes: 13 additions & 2 deletions pkg/runtime/reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit e566808

Please sign in to comment.