community Namespace references should default to the current namespace

Is your feature request related to a problem?

Currently if you don't specify a namespace then you get the default namespace. This is rarely desired and means there's a easy place to forget changing a namespace as you deploy resources to a different namespace e.g. in a rds.services.k8s.aws/v1alpha1 DBInstance the masterUserPassword is a reference to a Secret

apiVersion: rds.services.k8s.aws/v1alpha1
kind: DBInstance
metadata:
  name: mydb
  namespace: foo
spec:
  dbInstanceIdentifier: mydb
  dbSubnetGroupName: mysubnet
  engine: postgres
  engineVersion: "14.6"
  masterUsername: root
  masterUserPassword:
    namespace: foo # if not provided, uses the `default` namespace
    name: mysecret
    key: PGPASS

Describe the solution you'd like When there is a namespace field in a reference and it is not set, use the namespace of the current resource.

Describe alternatives you've considered

Make the namespace field required instead of optional.

Feb 16 '23 23:02 james-callahan

Hmm, this is a bug, since what you're describing is what is expected to happen :)

Feb 17 '23 13:02 jaypipes

I think the existing behaviour is coded at https://github.com/aws-controllers-k8s/runtime/blob/a8bd391d5cd30605770ce780044813113eb6694f/pkg/runtime/reconciler.go#L118-L120

Should SecretValueFromReference be changed to also take a "current_namespace"? Or should the caller of SecretValueFromReference fill in the SecretKeyReference with its namespace at some earlier point in time? If the latter, should that be filled in at parse time? or somewhere else?

Feb 20 '23 23:02 james-callahan

Should SecretValueFromReference be changed to also take a "current_namespace"? Or should the caller of SecretValueFromReference fill in the SecretKeyReference with its namespace at some earlier point in time?

@james-callahan, both solutions are viable options.

Solution 1 would require a modification to the ack.Reconciler interface and the generated code. This modification would likely be a one-liner in each repository. (famous last words)
Solution 2, on the other hand, would only require changes to the generated code, such as in this example: https://github.com/aws-controllers-k8s/rds-controller/blob/2d2be9ee69ff468f737cdb16670e87087cdfd33d/pkg/resource/db_cluster/custom_update.go#L563.

Despite the fact that the first solution requires more changes (and breaks an interface), I believe it is still a cleaner solution than attempting to modify the reference object at the "sdk" level of controllers.

Feb 21 '23 01:02 a-hilaly

Issues go stale after 90d of inactivity. Mark the issue as fresh with /remove-lifecycle stale. Stale issues rot after an additional 30d of inactivity and eventually close. If this issue is safe to close now please do so with /close. Provide feedback via https://github.com/aws-controllers-k8s/community. /lifecycle stale

Jun 07 '23 09:06 ack-bot

/remove-lifecycle stale

Jun 07 '23 17:06 jljaco

Issues go stale after 180d of inactivity. Mark the issue as fresh with /remove-lifecycle stale. Stale issues rot after an additional 60d of inactivity and eventually close. If this issue is safe to close now please do so with /close. Provide feedback via https://github.com/aws-controllers-k8s/community. /lifecycle stale

Dec 04 '23 17:12 ack-bot

/remove-lifecycle stale

Dec 05 '23 06:12 james-callahan

@a-hilaly do you have any intention to work on this?

Mar 22 '24 02:03 james-callahan

@james-callahan Yesir! just opened https://github.com/aws-controllers-k8s/runtime/pull/146 - we'll try to include it in our next controllers release.

Mar 26 '24 15:03 a-hilaly

@james-callahan We just fixed this in the runtime, next controller patches will include this fix

Mar 27 '24 14:03 a-hilaly

@james-callahan The patch have made it to all the controllers - can you please give it a shot?

Apr 02 '24 21:04 a-hilaly

community community copied to clipboard

Namespace references should default to the current namespace

community
community copied to clipboard