grafana / crossplane-provider-grafana

Crossplane provider of https://github.com/grafana/terraform-provider-grafana. Generated by https://github.com/upbound/upjet
Apache License 2.0
27 stars 14 forks source link

feat(grafana_data_source): added passwordSecretRef again #19

Closed haarchri closed 9 months ago

haarchri commented 1 year ago

Description of your changes

we used the old terrajet grafana provider and migrated to the upjet implementation - and passwordSecretRefis missing - so we added password for grafana_data_source again. for crossplane eco-system its much more easier with password field - because we can directly use the generated connectionSecrets from rds for example - is this an option for this deprecated field ? we can't find why this field is deprecated in terraform provider

---
apiVersion: oss.grafana.crossplane.io/v1alpha1
kind: DataSource
metadata:
  name: sonarqube-postgresql
spec:
  forProvider:
    accessMode: proxy
    name: test
    type: postgres
    databaseName: sonarqube
    url: sonarqube.xxxx.eu-central-1.rds.amazonaws.com:5432
    username: user123
    passwordSecretRef: 
      key: password
      name: sonarqube-dbinstance
      namespace: sonarqube 
    jsonDataEncoded: |
      {
      "connMaxLifetime": "14400",
      "maxIdleConns": "2",
      "maxOpenConns": "0",
      "postgresVersion": "1000",
      "sslMode": "disable",
      "timeInterval": "1m",
      "timescaledb": "false"
      }
  providerConfigRef:
    name: example

Fixes #

I have:

How has this code been tested

NAME                                                                        READY   SYNCED   EXTERNAL-NAME   AGE
datasource.oss.grafana.crossplane.io/sonarqube-postgresql                   True    True    16              11m
julienduchesne commented 1 year ago

This field is going away in Terraform for sure, so it's not going to stay here in the Crossplane provider.

The reason it's deprecated is because it used to be top-level attribute in the Grafana API too but it's not anymore. Not sure what the solution is but the password field probably isn't it

Maybe, we can have an attribute that's a secret ref, takes the value from that ref and tranposes it to a secure data json payload?

Can this be done from a combine patch?

haarchri commented 1 year ago

problem here is that we can't use a combine patch from a secretRef ... https://github.com/crossplane/crossplane/issues/2772

options:

btw all the "new" encoded fields are pain in daily doing

CLAassistant commented 1 year ago

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Christopher Paul Haar seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

abacus3 commented 1 year ago

I'd suggest to generally allow specification of YAML objects in claims. The actual transformation into a JSON string should be a provider internal concern and not part of the API. I am willing to create an issue on this subject.

julienduchesne commented 9 months ago

Going to have to close this one sorry. The field has been removed from the TF provider (and the API)