tryretool / retool-helm

MIT License
47 stars 59 forks source link

Feature request: add support for secretRefs in all database values #177

Open criskurtin opened 5 months ago

criskurtin commented 5 months ago

Details

As per previous discussion, I'm opening an issue here with added details.

Current setup

We handle our RDS provisioning via IaaC (pulumi) and export the relevant values into k8s secrets so they can later be referenced in various helm chart values. These values are usually the database hostname, dbname, username and password. Using the existing secret to set the password value already seems to be supported, but other values (hostname, etc.) can currently only be set as a string. To work around this, we set the env var values directly in the environmentSecrets section of the retool values file:

---
config:
  postgresql:
    port: 5432
    ssl_enabled: true
    passwordSecretName: CENSORED
    passwordSecretKey: password
environmentSecrets:
  - name: POSTGRES_HOST
    secretKeyRef:
      name: CENSORED
      key: hostname
  - name: POSTGRES_DB
    secretKeyRef:
      name: CENSORED
      key: database
  - name: POSTGRES_USER
    secretKeyRef:
      name: CENSORED
      key: username

Problem

This works fine for the initial deployment. However, probably because the environment variables are now set twice (once by the deployment template and then again by us in the environmentSecrets section), every subsequent upgrade fails with the following error:

  Error: UPGRADE FAILED: an error occurred while rolling back the release. original upgrade error: failed to create patch: The order in patch list:
  [map[name:DEPLOYMENT_TEMPLATE_VERSION value:6.2.1] map[name:CLIENT_ID value:<nil>] map[name:POSTGRES_HOST value:<nil>] map[name:POSTGRES_DB valueFrom:map[secretKeyRef:map[key:database name:CENSORED]]] map[name:POSTGRES_DB value:<nil>] map[name:POSTGRES_USER value:<nil>] map[name:POSTGRES_USER valueFrom:map[secretKeyRef:map[key:username name:CENSORED]]] map[name:DISABLE_IMAGE_PROXY value:true] map[name:DISABLE_PUBLIC_PAGES value:true] map[name:VERSION_CONTROL_LOCKED value:false] map[name:USE_SHORT_SESSIONS value:true]]
   doesn't match $setElementOrder list:
  [map[name:DEPLOYMENT_TEMPLATE_TYPE] map[name:DEPLOYMENT_TEMPLATE_VERSION] map[name:NODE_ENV] map[name:SERVICE_TYPE] map[name:CLIENT_ID] map[name:COOKIE_INSECURE] map[name:POSTGRES_HOST] map[name:POSTGRES_PORT] map[name:POSTGRES_DB] map[name:POSTGRES_USER] map[name:POSTGRES_SSL_ENABLED] map[name:WORKFLOW_TEMPORAL_CLUSTER_FRONTEND_HOST] map[name:WORKFLOW_TEMPORAL_CLUSTER_FRONTEND_PORT] map[name:WORKFLOW_TEMPORAL_CLUSTER_NAMESPACE] map[name:WORKFLOW_BACKEND_HOST] map[name:CODE_EXECUTOR_INGRESS_DOMAIN] map[name:LICENSE_KEY] map[name:JWT_SECRET] map[name:ENCRYPTION_KEY] map[name:POSTGRES_PASSWORD] map[name:CLIENT_SECRET] map[name:POSTGRES_HOST] map[name:POSTGRES_DB] map[name:POSTGRES_USER] map[name:DISABLE_IMAGE_PROXY] map[name:DISABLE_PUBLIC_PAGES] map[name:VERSION_CONTROL_LOCKED] map[name:USE_SHORT_SESSIONS]]
  : cannot patch "retool" with kind Deployment: The order in patch list:
  [map[name:CLIENT_ID value:<nil>] map[name:POSTGRES_HOST value:<nil>] map[name:POSTGRES_DB value:<nil> valueFrom:<nil>] map[name:POSTGRES_DB value:<nil>] map[name:POSTGRES_USER value:<nil>] map[name:POSTGRES_USER valueFrom:<nil>] map[$patch:delete name:DISABLE_IMAGE_PROXY] map[$patch:delete name:DISABLE_PUBLIC_PAGES] map[$patch:delete name:USE_SHORT_SESSIONS] map[$patch:delete name:VERSION_CONTROL_LOCKED]]
   doesn't match $setElementOrder list:
  [map[name:DEPLOYMENT_TEMPLATE_TYPE] map[name:DEPLOYMENT_TEMPLATE_VERSION] map[name:NODE_ENV] map[name:SERVICE_TYPE] map[name:CLIENT_ID] map[name:COOKIE_INSECURE] map[name:POSTGRES_HOST] map[name:POSTGRES_PORT] map[name:POSTGRES_DB] map[name:POSTGRES_USER] map[name:POSTGRES_SSL_ENABLED] map[name:WORKFLOW_TEMPORAL_CLUSTER_FRONTEND_HOST] map[name:WORKFLOW_TEMPORAL_CLUSTER_FRONTEND_PORT] map[name:WORKFLOW_TEMPORAL_CLUSTER_NAMESPACE] map[name:WORKFLOW_BACKEND_HOST] map[name:CODE_EXECUTOR_INGRESS_DOMAIN] map[name:LICENSE_KEY] map[name:JWT_SECRET] map[name:ENCRYPTION_KEY] map[name:POSTGRES_PASSWORD] map[name:CLIENT_SECRET] map[name:POSTGRES_HOST] map[name:POSTGRES_DB] map[name:POSTGRES_USER]]
   && failed to create patch: The order in patch list:
  [map[name:DEPLOYMENT_TEMPLATE_VERSION value:6.2.0] map[name:CLIENT_ID value:<nil>] map[name:POSTGRES_HOST value:<nil>] map[name:POSTGRES_DB valueFrom:map[secretKeyRef:map[key:database name:CENSORED]]] map[name:POSTGRES_DB value:<nil>] map[name:POSTGRES_USER valueFrom:map[secretKeyRef:map[key:username name:CENSORED]]] map[name:POSTGRES_USER value:<nil>]]
   doesn't match $setElementOrder list:
  [map[name:DEPLOYMENT_TEMPLATE_TYPE] map[name:DEPLOYMENT_TEMPLATE_VERSION] map[name:NODE_ENV] map[name:SERVICE_TYPE] map[name:CLIENT_ID] map[name:COOKIE_INSECURE] map[name:POSTGRES_HOST] map[name:POSTGRES_PORT] map[name:POSTGRES_DB] map[name:POSTGRES_USER] map[name:POSTGRES_SSL_ENABLED] map[name:LICENSE_KEY] map[name:JWT_SECRET] map[name:ENCRYPTION_KEY] map[name:POSTGRES_PASSWORD] map[name:CLIENT_SECRET] map[name:POSTGRES_HOST] map[name:POSTGRES_DB] map[name:POSTGRES_USER]]
   && failed to create patch: The order in patch list:
  [map[name:DEPLOYMENT_TEMPLATE_VERSION value:6.2.0] map[name:CLIENT_ID value:<nil>] map[name:POSTGRES_HOST valueFrom:map[secretKeyRef:map[key:AURORA_HOSTNAME name:CENSORED]]] map[name:POSTGRES_HOST value:<nil>] map[name:POSTGRES_DB value:<nil>] map[name:POSTGRES_USER valueFrom:map[secretKeyRef:map[key:username name:CENSORED]]] map[name:POSTGRES_USER value:<nil>]]
   doesn't match $setElementOrder list:
  [map[name:DEPLOYMENT_TEMPLATE_TYPE] map[name:DEPLOYMENT_TEMPLATE_VERSION] map[name:NODE_ENV] map[name:SERVICE_TYPE] map[name:DBCONNECTOR_POSTGRES_POOL_MAX_SIZE] map[name:DBCONNECTOR_QUERY_TIMEOUT_MS] map[name:DISABLE_DATABASE_MIGRATIONS] map[name:WORKFLOW_TEMPORAL_CLUSTER_FRONTEND_HOST] map[name:WORKFLOW_TEMPORAL_CLUSTER_FRONTEND_PORT] map[name:WORKFLOW_TEMPORAL_CLUSTER_NAMESPACE] map[name:CLIENT_ID] map[name:COOKIE_INSECURE] map[name:POSTGRES_HOST] map[name:POSTGRES_PORT] map[name:POSTGRES_DB] map[name:POSTGRES_USER] map[name:POSTGRES_SSL_ENABLED] map[name:WORKFLOW_BACKEND_HOST] map[name:LICENSE_KEY] map[name:JWT_SECRET] map[name:ENCRYPTION_KEY] map[name:POSTGRES_PASSWORD] map[name:CLIENT_SECRET] map[name:POSTGRES_HOST] map[name:POSTGRES_DB] map[name:POSTGRES_USER]]

As a result, every time we want to modify some values or update our Retool deployment, we have to delete the releases in the k8s cluster and redeploy "from scratch". It's not that big of an issue, as all settings are persisted in the database, but it does cause a couple of minutes of downtime and it's an annoyance.

Proposed solution

It would be great if the secrets could be referenced in the config.postgresql section, not only for the password, but for other database connection string values.

Here are two examples of how this would be implemented in the values file. The first one would perhaps be a bit more familiar to anyone usually referencing the secrets in the env section, while the second one would be aligned with the current implementation with password:

---
config:
  postgresql:
    host:
      secretKeyRef:
        name: CENSORED
        key: hostname
    port: 5432
    db:
      secretKeyRef:
        name: CENSORED
        key: database
    user:
      secretKeyRef:
        name: CENSORED
        key: username
    password:
      secretKeyRef:
        name: CENSORED
        key: password
    ssl_enabled: true
---
config:
  postgresql:
    hostSecretName: CENSORED
    hostSecretKey: hostname
    dbSecretName: CENSORED
    dbSecretKey: database
    userSecretName: CENSORED
    userSecretKey: username
    passwordSecretName: CENSORED
    passwordSecretKey: password
    port: 5432
    ssl_enabled: true

If you need any additional info, I'll be watching this issue and replying to any comments.