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

Improve ProviderConfigRef Secret #159

Open
mircea-pavel-anton opened this issue Sep 12, 2024 · 4 comments · May be fixed by #176
Open

Improve ProviderConfigRef Secret #159

mircea-pavel-anton opened this issue Sep 12, 2024 · 4 comments · May be fixed by #176
Labels

Comments

@mircea-pavel-anton
Copy link

In the documentation, it is mentioned that additional fields supported by the upstream Terraform provider are supported.

I think that having the JSON structure to the connection secret is quite limiting. When I deploy Keycloak via the bitnami helm chart, for example, I need to provide a secret with the password. Having this in json format poses some challenges as to extracting and processing that data to format it nicely

I see that the Terraform provider also supports environment variables. That would be much better UX in allowing me to pick and choose keys from multiple sources (configmaps/secrets).

Is this supported?

@Breee
Copy link
Collaborator

Breee commented Sep 12, 2024

Make an example please.
One thing you have to keep in mind is, that a crossplane provider potentially can configure multiple keycloak instances.
So a provider config is used for exactly one instance of keycloak. Just passing environment variables to the provider will not be good enough

@mircea-pavel-anton
Copy link
Author

Sure thing!

I didn't mean injecting env vars into the provider deployment itself. I was talking more about restructuring the provider config credential secret into individual keys as opposed to a single json value, and I was referencing the names of the env vars to be used as keys as a possibility.

Currently, it is defined like this:

---
apiVersion: keycloak.crossplane.io/v1beta1
kind: ProviderConfig
metadata:
  name: keycloak-provider-config
spec:
  credentials:
    source: Secret
    secretRef:
      name: keycloak-credentials
      key: credentials
      namespace: crossplane-system
---
apiVersion: v1
kind: Secret
metadata:
  name: keycloak-credentials
  namespace: crossplane-system
  labels: 
    type: provider-credentials
type: Opaque
stringData:
  credentials: |
    {
      "client_id":"admin-cli",
      "username": "admin",
      "password": "admin",
      "url": "https://keycloak.example.com",
      "base_path": "/auth",
      "realm": "master"
    }

However what I was proposing is the ability to define it either like this:

---
apiVersion: v1
kind: Secret
metadata:
  name: keycloak-credentials
  namespace: crossplane-system
  labels: 
    type: provider-credentials
type: Opaque
stringData:
      client_id: "admin-cli"
      username: "admin"
      password: "admin"
      url: "https://keycloak.example.com"
      base_path: "/auth"
      realm: "master"

Or like this:

---
apiVersion: v1
kind: Secret
metadata:
  name: keycloak-credentials
  namespace: crossplane-system
  labels: 
    type: provider-credentials
type: Opaque
stringData:
      KEYCLOAK_CLIENT_ID: "admin-cli"
      KEYCLOAK_USER: "admin"
      KEYCLOAK_PASSWORD: "admin"
      KEYCLOAK_URL: "https://keycloak.example.com"
      KEYCLOAK_BASE_PATH: "/auth"
      KEYCLOAK_REALM: "master

And then rearrange this data into the required format at runtime.

Additionally, sourcing these pieces of information from more than one source could be helpful. For example, having a configMap with the url, client id, realm name and base path and then a secret just with the password or client secret.

@mircea-pavel-anton mircea-pavel-anton changed the title Improve ProviderConfigRed Improve ProviderConfigRef Secret Sep 12, 2024
@Breee Breee added the feature label Sep 20, 2024
@m-hofmann-a9s
Copy link

I'm currently struggling with the same issue.

Are you accepting contributions? I've created a small patch to how provider-keycloak's config is handled that should allow using both the old "JSON in a secret" as well as a plain k8s key-value-map Secret. I'd be happy to hear some feedback!

I've also raised the question as to why all Upjet-generated providers deal with credentials this way with Crossplane, as I cannot figure out the reasoning here.

@Breee
Copy link
Collaborator

Breee commented Oct 15, 2024

I accept contributions.
Just make a pull request and i will review it.

In principle i think this makes sense and i prefer objects over json

I think people do it like that because in the provider-template / docs / guides they do it like this

@m-hofmann-a9s m-hofmann-a9s linked a pull request Oct 16, 2024 that will close this issue
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants