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

Add SSL chart options for PostgreSQL & Redis databases #88

Closed
safehome-jdev opened this issue Apr 12, 2024 · 8 comments
Closed

Add SSL chart options for PostgreSQL & Redis databases #88

safehome-jdev opened this issue Apr 12, 2024 · 8 comments

Comments

@safehome-jdev
Copy link

Chart Version: v0.6.0
Microservices Version: v1.101.0
Server Version: v1.101.0

Current chart doesn't accommodate instances where SSL is used for databases, instead, it defaults to no encryption. Potential fixes could be to allow a ca.crt by providing an existing secret/configMap or an allowInsecure flag.

By default, the Zalando Postgres Operator enables SSL and prevents the immich-microservices & immich-server from booting.

Log snippet from immich-microservices:

[Nest] 8  - 04/12/2024, 8:29:32 PM   ERROR [TypeOrmModule] Unable to connect to the database. Retrying (4)...
error: pg_hba.conf rejects connection for host "10.42.0.79", user "immich", database "immich", no encryption
    at Parser.parseErrorMessage (/usr/src/app/node_modules/pg-protocol/dist/parser.js:287:98)
    at Parser.handlePacket (/usr/src/app/node_modules/pg-protocol/dist/parser.js:126:29)
    at Parser.parse (/usr/src/app/node_modules/pg-protocol/dist/parser.js:39:38)
    at Socket.<anonymous> (/usr/src/app/node_modules/pg-protocol/dist/index.js:11:42)
    at Socket.emit (node:events:518:28)
    at addChunk (node:internal/streams/readable:559:12)
    at readableAddChunkPushByteMode (node:internal/streams/readable:510:3)
    at Readable.push (node:internal/streams/readable:390:5)
    at TCP.onStreamRead (node:internal/stream_base_commons:190:23)

Postgresql snippet configuration :

/run/postgres.yml

[...]
postgresql:
  authentication:
    replication:
      password: [REDACTED]
      username: standby
    superuser:
      password: [REDACTED]
      username: postgres
  basebackup_fast_xlog:
    command: /scripts/basebackup.sh
    retries: 2
  bin_dir: /usr/lib/postgresql/14/bin
  callbacks:
    on_role_change: /scripts/on_role_change.sh zalandos true
  connect_address: 10.42.0.77:5432
  create_replica_method:
  - basebackup_fast_xlog
  data_dir: /home/postgres/pgdata/pgroot/data
  listen: '*:5432'
  name: postgres-0
  parameters:
    archive_command: /bin/true
    bg_mon.history_buckets: 120
    bg_mon.listen_address: '::'
    extwlist.custom_path: /scripts
    extwlist.extensions: btree_gin,btree_gist,citext,extra_window_functions,first_last_agg,hll,hstore,hypopg,intarray,ltree,pgcrypto,pgq,pgq_node,pg_trgm,postgres_fdw,tablefunc,uuid-ossp,timescaledb,pg_partman
    log_destination: csvlog
    log_directory: ../pg_log
    log_file_mode: '0644'
    log_filename: postgresql-%u.log
    log_rotation_age: 1d
    log_truncate_on_rotation: 'on'
    logging_collector: 'on'
    pg_stat_statements.track_utility: 'off'
    shared_buffers: 100MB
    shared_preload_libraries: bg_mon,pg_stat_statements,pgextwlist,pg_auth_mon,set_user,timescaledb,pg_cron,pg_stat_kcache
    ssl: 'on'
    ssl_cert_file: /run/certs/server.crt
    ssl_key_file: /run/certs/server.key
[...]

Trying to use the DB_URL env variable as a workaround, prevents resolving shell variables and returns a string literal instead:

DB_URL='postgresql://$(DB_USERNAME):$(DB_PASSWORD)@postgreshost:postgresport/$(DB_DATABASE_NAME)?sslmode=require'

@bo0tzz
Copy link
Member

bo0tzz commented Apr 13, 2024

Can you post the actual values.yaml etc that you're using?

You can already mount an extra configmap/secret/etc to the deployments, the chart doesn't need any changes for that.

How are you setting the DB_URL env var? I think this should work: https://kubernetes.io/docs/tasks/inject-data-application/define-interdependent-environment-variables/

@safehome-jdev
Copy link
Author

safehome-jdev commented Apr 17, 2024

What's interesting is leaving blank values for DB_(HOSTNAME/PASSWORD/USERNAME) will result with empty valued ENV's attached to the container. Filling in these values by using a secretKeyRef or any value, makes them disappear (likely due to the DB_URL var).

env:
  DB_DATABASE_NAME: 
    valueFrom:
      secretKeyRef:
        key: username
        name: immich.postgres.credentials.postgresql.acid.zalan.do
  DB_HOSTNAME: postgres.immich.svc.cluster.local
  DB_PASSWORD: 
    valueFrom:
    secretKeyRef:
      key: password
      name: immich.postgres.credentials.postgresql.acid.zalan.do
  DB_USERNAME:
    valueFrom:
    secretKeyRef:
      key: username
      name: immich.postgres.credentials.postgresql.acid.zalan.do
  IMMICH_MACHINE_LEARNING_URL: '{{ printf "http://%s-machine-learning:3003" .Release.Name }}'
  REDIS_HOSTNAME: redis-sentinel.immich.svc.cluster.local
  DB_URL: >-
postgresql://$(PGSQL_DB_USERNAME):$(PGSQL_DB_PASSWORD)@postgres.immich.svc.cluster.local:5432/$(PGSQL_DB_DATABASE_NAME)?sslmode=no-verify
  PGSQL_DB_DATABASE_NAME:
    valueFrom:
      secretKeyRef:
        key: username
        name: immich.postgres.credentials.postgresql.acid.zalan.do
  PGSQL_DB_PASSWORD:
    valueFrom:
      secretKeyRef:
        key: password
        name: immich.postgres.credentials.postgresql.acid.zalan.do
  PGSQL_DB_USERNAME:
    valueFrom:
      secretKeyRef:
        key: username
        name: immich.postgres.credentials.postgresql.acid.zalan.do
  REDIS_PASSWORD:
    valueFrom:
      secretKeyRef:
        key: password
        name: redis-secret
  REDIS_PORT: "6379"
image:
  tag: v1.101.0
immich:
  configuration: {}
  metrics:
    enabled: false
  persistence:
    library:
      existingClaim: immich-data
machine-learning:
  enabled: true
  env:
    TRANSFORMERS_CACHE: /cache
  image:
    pullPolicy: IfNotPresent
    repository: ghcr.io/immich-app/immich-machine-learning
  persistence:
    cache:
      accessMode: ReadWriteMany
      enabled: true
      size: 512Mi
      type: emptyDir
      medium: Memory
microservices:
  enabled: true
  image:
    pullPolicy: IfNotPresent
    repository: ghcr.io/immich-app/immich-server
  persistence:
    geodata-cache:
      storageClass: nfs-emc-client
      type: pvc
postgresql:
  enabled: false
  global:
    postgresql:
      auth:
        database: immich
        password: immich
        username: immich
  image:
    repository: tensorchord/pgvecto-rs
    tag: pg14-v0.2.0
  primary:
    initdb:
      scripts:
        create-extensions.sql: |
          CREATE EXTENSION cube;
          CREATE EXTENSION earthdistance;
          CREATE EXTENSION vectors;
  persistence:
    accessModes:
      - ReadWriteMany
    enabled: false
    size: 10Gi
    type: pvc
    volumeName: data
redis:
  architecture: standalone
  auth:
    enabled: false
  enabled: false
server:
  enabled: true
  image:
    pullPolicy: IfNotPresent
    repository: ghcr.io/immich-app/immich-server
  ingress:
    main:
      annotations:
        nginx.ingress.kubernetes.io/proxy-body-size: '0'
      enabled: false
      hosts:
        - host: immich.local
          paths:
            - path: /
      tls: []
global:
  cattle:
    systemProjectId: p-ngj57

To get around this, I created 3 additional env's that aren't watched by the chart (prefixed with PGSQL_), and reference them in the DB_URL env. This makes them appear within the container config, however, somewhere along the way within the container, the vars lose their value and wind up being a string literal.

Here's a snippet of the container log

2024-04-16T19:10:38.011905498-07:00 [Nest] 7  - 04/17/2024, 2:10:38 AM   ERROR [TypeOrmModule] Unable to connect to the database. Retrying (1)...
error: password authentication failed for user "$(PGSQL_DB_USERNAME)"
2024-04-16T19:10:38.011944065-07:00     at Parser.parseErrorMessage (/usr/src/app/node_modules/pg-protocol/dist/parser.js:287:98)

@safehome-jdev
Copy link
Author

safehome-jdev commented Apr 17, 2024

Here's the deployment YAML that results from this:

apiVersion: apps/v1
kind: Deployment
metadata:
  annotations:
    meta.helm.sh/release-name: immich
    meta.helm.sh/release-namespace: immich
  labels:
    app.kubernetes.io/instance: immich
    app.kubernetes.io/managed-by: Helm
    app.kubernetes.io/name: microservices
    app.kubernetes.io/version: v1.100.0
    helm.sh/chart: immich-0.6.0
  name: immich-microservices
  namespace: immich
spec:
  progressDeadlineSeconds: 600
  replicas: 1
  revisionHistoryLimit: 3
  selector:
    matchLabels:
      app.kubernetes.io/instance: immich
      app.kubernetes.io/name: microservices
  strategy:
    type: Recreate
  template:
    metadata:
      creationTimestamp: null
      labels:
        app.kubernetes.io/instance: immich
        app.kubernetes.io/name: microservices
      namespace: immich
    spec:
      automountServiceAccountToken: true
      containers:
        - args:
            - ./start-microservices.sh
          command:
            - /bin/sh
          env:
            - name: DB_URL
              value: >-
                postgresql://$(PGSQL_DB_USERNAME):$(PGSQL_DB_PASSWORD)@postgres.immich.svc.cluster.local:5432/$(PGSQL_DB_DATABASE_NAME)?sslmode=no-verify
            - name: IMMICH_MACHINE_LEARNING_URL
              value: http://immich-machine-learning:3003
            - name: PGSQL_DB_DATABASE_NAME
              valueFrom:
                secretKeyRef:
                  key: username
                  name: immich.postgres.credentials.postgresql.acid.zalan.do
            - name: PGSQL_DB_PASSWORD
              valueFrom:
                secretKeyRef:
                  key: password
                  name: immich.postgres.credentials.postgresql.acid.zalan.do
            - name: PGSQL_DB_USERNAME
              valueFrom:
                secretKeyRef:
                  key: username
                  name: immich.postgres.credentials.postgresql.acid.zalan.do
            - name: REDIS_HOSTNAME
              value: redis-sentinel.immich.svc.cluster.local
            - name: REDIS_PASSWORD
              valueFrom:
                secretKeyRef:
                  key: password
                  name: redis-secret
                  optional: false
            - name: REDIS_PORT
              value: '6379'
          image: ghcr.io/immich-app/immich-server:v1.101.0
          imagePullPolicy: IfNotPresent
          name: immich-microservices
          resources: {}
          terminationMessagePath: /dev/termination-log
          terminationMessagePolicy: File
          volumeMounts:
            - mountPath: /usr/src/app/upload
              name: library
      dnsPolicy: ClusterFirst
      enableServiceLinks: true
      restartPolicy: Always
      schedulerName: default-scheduler
      securityContext: {}
      serviceAccount: default
      serviceAccountName: default
      terminationGracePeriodSeconds: 30
      volumes:
        - name: library
          persistentVolumeClaim:
            claimName: immich-data

@bo0tzz
Copy link
Member

bo0tzz commented Apr 17, 2024

The DB_URL env var is ending up at the top of the list in the container spec, but for the variable interpolation to work it needs to be below the other vars that it uses. Can you try just moving it down in your values.yaml? I'm not sure if that will carry through but it's worth a try.

@safehome-jdev
Copy link
Author

@bo0tzz Great call, that worked, unfortunately, the way that the chart handles the env vars, it looks like it places the vars in alphabetical order and not based on my predefined order. I have to go into the deployments for microservices and the server itself and reorder the list.

I am using Rancher to manage my chart, and I hope this isn't what's causing the issue...

@bo0tzz
Copy link
Member

bo0tzz commented May 6, 2024

After some investigation:
We're using a map for the env vars, and golang does not give any guarantees about the ordering of maps (and seemingly ends up sorting alphabetically). As a (potentially brittle) workaround you can probably force the ordering by doing something gross like

env:
  A_PGSQL_HOST: foo
  DB_URL: postgresql://$(A_PGSQL_HOST)/etc

The real solution here would be for the Immich chart to use a list for the env vars, which does come with ordering guarantees. Unfortunately, I think that would be a breaking change, which I've been trying to avoid.

@safehome-jdev
Copy link
Author

After some investigation:

We're using a map for the env vars, and golang does not give any guarantees about the ordering of maps (and seemingly ends up sorting alphabetically). As a (potentially brittle) workaround you can probably force the ordering by doing something gross like

env:

  A_PGSQL_HOST: foo

  DB_URL: postgresql://$(A_PGSQL_HOST)/etc

The real solution here would be for the Immich chart to use a list for the env vars, which does come with ordering guarantees. Unfortunately, I think that would be a breaking change, which I've been trying to avoid.

This is likely my best option at this point; I also can't blame you for not wanting to break something 😛

@bo0tzz
Copy link
Member

bo0tzz commented May 9, 2024

Tracking this further in #92 to eventually make the breaking change to use a list.

@bo0tzz bo0tzz closed this as completed May 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants