mattermost / mattermost-helm

Mattermost Helm charts for Kubernetes
Apache License 2.0
165 stars 147 forks source link

add extraEnv to jobserver values for libpq PG* variables #316

Closed ruckc closed 2 years ago

ruckc commented 2 years ago

Summary

This PR adds extraEnv array to the jobserver configuration in order to set various environment variables for mattermost.

In short, when using an external postgres, with driverName: postgres, you can leave datasource as an empty string, and use extraEnv to pass libpq environment variables to the mattermost process. Some of these can be pulled from secrets.

Ticket Link

This PR partially addresses #140, when using an external postgresql database.

Values example

When using mattermost paired with Postgres Operator

extraEnv:
  - name: PGHOST
    value: mattermost-database.mattermost.svc.cluster.local
  - name: PGPORT
    value: "5432"
  - name: PGDATABASE
    value: mattermost
  - name: MM_LDAPSETTINGS_BINDUSERNAME
    valueFrom:
      secretKeyRef:
        key: dn
        name: ldap-credentials
  - name: MM_LDAPSETTINGS_BINDPASSWORD
    valueFrom:
      secretKeyRef:
        key: password
        name: ldap-credentials
  - name: PGUSER
    valueFrom:
      secretKeyRef:
        key: username
        name: mattermost.mattermost-database.credentials.postgresql.acid.zalan.do
  - name: PGPASSWORD
    valueFrom:
      secretKeyRef:
        key: password
        name: mattermost.mattermost-database.credentials.postgresql.acid.zalan.do
mattermod commented 2 years ago

Hello @ruckc,

Thanks for your pull request! A Core Committer will review your pull request soon. For code contributions, you can learn more about the review process here.

ruckc commented 2 years ago

@stylianosrigas - can you look at this PR please?

stylianosrigas commented 2 years ago

@stylianosrigas - can you look at this PR please?

Thanks @ruckc for the PR. Can you fix the test failures? For every change you also need to bump the chart version. Adding @spirosoik and @pfltdv for review ;)

ruckc commented 2 years ago

@pfltdv - should i bump the version also? that is why one of the lint-charts is failing

ghost commented 2 years ago

@pfltdv - should i bump the version also? that is why one of the lint-charts is failing

@ruckc It should not be a requirement for you since we are not generating a release. Looks like the versions at master branch need to be fixed. I will check, fix and notify you as soon as it is ready.

ruckc commented 2 years ago

i should only need to bump the ee version since i didn't change team edition

ghost commented 2 years ago

Sorry about it. Yes it looks ok now.

ruckc commented 2 years ago

@pfltdv / @stylianosrigas - when will this be merged?