Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DBInstance] Needs namespace "crossplane-system" to store values in cache (version 0.42) #1834

Closed
WolfGanGeRTech opened this issue Aug 7, 2023 · 3 comments · Fixed by #1893
Labels
bug Something isn't working

Comments

@WolfGanGeRTech
Copy link

What happened?

After the upgrade for version 0.42 I am not able to create new RDS due to error:

    "Message: create failed: pre-create failed: cannot cache password: cannot create object: namespaces "crossplane-system" not found"

This is probably due to the refactor of the password cache ( #1756 ) cc. @schroeder-paul / @MisterMX . It seems the Cache namespace is not fetching the namespace from "masterUserPasswordSecretRef", so the fix seems to be something simple.

I am using:

    masterUserPasswordSecretRef:
      key: password
      name: example-dbinstance
      namespace: default

So, I guess Crossplane should use that namespace for caching.

How can we reproduce it?

Here is the manifest applied

apiVersion: rds.aws.crossplane.io/v1alpha1
kind: DBInstance
metadata:
  name: example-dbinstance
spec:
  forProvider:
    region: eu-central-1
    allocatedStorage: 20
    dbName: example
    engine: postgres
    engineVersion: "12.9"
    dbInstanceClass: db.t3.micro
    masterUsername: adminuser
    masterUserPasswordSecretRef:
      key: password
      name: example-dbinstance
      namespace: default
  writeConnectionSecretToRef:
    name: example-dbinstance-out
    namespace: default
  providerConfigRef:
    name: example

---
apiVersion: v1
kind: Secret
metadata:
  name: example-dbinstance
  namespace: default
type: Opaque
data:
  password: dGVzdFBhc3N3b3JkITEyMw== # testPassword!123

What environment did it happen in?

Crossplane version: 0.42

Thanks for the help! :)

@WolfGanGeRTech WolfGanGeRTech added the bug Something isn't working label Aug 7, 2023
@valdemon
Copy link

valdemon commented Aug 27, 2023

@WolfGanGeRTech the masterUserPasswordSecretRef is optional and must not be provided when using the manageMasterUserPassword option (my case) .
So I guess the namespace for the password caching must be provided in another way.
I've worked around this problem by creating the artificial namespace crossplane-system in the target cluster as my Crossplane is installed in the crossplane ns.
Anyways it's a bug.

@MisterMX
Copy link
Collaborator

MisterMX commented Sep 5, 2023

While it may seem trivial, this issue is not so easy to resolve as it may seem at first glance. Some heads-up thought:

masterUserPasswordSecretRef is indeed an optional field. The last used password is stored in a cache secret. It is not something that should be specified for MR individually since it is only provider internal. #1756 was written with the assumption that providers are always running in the crossplane-system namespace, so it got hardcoded.

However, it is not so easy to dynamically resolve the namespace to be used since controller-runtime's Client provides no direct way to get the default namespace from a config.

To solve this issue we need to lookup the target namespace manually. There are a bunch of possible locations to fetch it from:

  • POD_NAMESPACE env var
  • local kubeconfig context
  • --namespace CLI arg (we already have that one but it is used in a different way)

After we retrieved the namespace, we need to set it at the cache secret in the controller. It can be done directly via secret.SetNamespace or making it more abstract by using a client.NewNamespacedClient wrapper.

@okgolove
Copy link

okgolove commented Oct 3, 2023

probably /var/run/secrets/kubernetes.io/serviceaccount/namespace file is an option as well?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants