Improve handling of missing namespace in SecretKeyReference
#146
Conversation
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>
|
Do we have any sanity check on if any use cases have a dependency on the secret being in |
|
@jlbutler Good call - all the tests we have around secret references, explicitly set a namespace. e.g RDS DBCluster https://github.com/aws-controllers-k8s/rds-controller/blob/main/test/e2e/tests/test_db_cluster.py#L52-L76 Ideally we want to test the defaulting behavior in the runtime package. Should we deal with it during the refactoring, as we will have easier testing (probably using envtest)? rather than just investing on more mocking. |
|
Yes I think that's ok to look at this for refactoring. I think this is the correct default (look to the namespace in context by default, not the general default). I'm just invoking Hyrum's Law here and ensuring we consider this as a potentially breaking change, even if depending on this would be a bit of an anti-pattern. Let's go with it, and keep an eye on issues for the corner case, and we can roll back if our instinct is wrong. |
|
/retest |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: a-hilaly, jlbutler The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
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 codepreviously 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.