lunny/helm-chart
1
0
Fork 1
Code Issues 36 Pull Requests 14 Actions Packages Projects Releases 93 Wiki Activity

enhancements to support postgres client-cert authentication #47

Merged
petergardfjall merged 1 commits from refs/pull/47/head into master 2021-01-20 11:28:40 +00:00
Conversation 22 Commits 1 Files Changed 3 +19 -1
petergardfjall commented 2020-10-19 08:35:05 +00:00 (Migrated from gitea.com)
Copy Link

This PR adds a few new chart features which adds to the flexibility of the chart.

  • allow extra volumes to be mounted (such as secrets): 2f862c5a48
  • pass environment variables also to the init-container: 7044049478
  • allow a preparation script to be "injected" into the init-container: 6125a69345

As a concrete example of how this can be used, I use is to configure Gitea to use client certificate authentication against an external Postgres database. That could be accomplished by having a gitea-postgres-ssl secret:

apiVersion: v1
kind: Secret
type: Opaque
metadata:
  name: gitea-postgres-ssl
data:
  postgresql.crt: <base64...>
  postgresql.key: <base64...>
  root.crt: <base64...>

and then mounting this as a volume in Gitea using:

extraVolumes:
- name: postgres-ssl-vol
  secret:
    secretName: gitea-postgres-ssl

extraVolumeMounts:
- name: postgres-ssl-vol
  readOnly: true
  mountPath: "/pg-ssl"

To get the right permissions on the credentials, we'd use the initPreScript:

initPreScript: |
  # copy postgres client and CA cert from mount and
  # give proper permissions
  mkdir -p /data/git/.postgresql
  cp /pg-ssl/* /data/git/.postgresql/
  chown -R git:git /data/git/.postgresql/
  chmod 400 /data/git/.postgresql/postgresql.key

and to make sure that Gitea uses the certificate we need to pass the proper postgres environment variables (both to the init container and the "main" container):

statefulset:
  env:
  - name:  "PGSSLCERT"
    value: "/data/git/.postgresql/postgresql.crt"
  - name:  "PGSSLKEY"
    value: "/data/git/.postgresql/postgresql.key"
  - name:  "PGSSLROOTCERT"
    value: "/data/git/.postgresql/root.crt"
This PR adds a few new chart features which adds to the flexibility of the chart. - allow extra volumes to be mounted (such as secrets): 2f862c5a48 - pass environment variables also to the init-container: 7044049478 - allow a preparation script to be "injected" into the init-container: 6125a69345 As a concrete example of how this can be used, I use is to configure Gitea to use client certificate authentication against an external Postgres database. That could be accomplished by having a `gitea-postgres-ssl` secret: ``` apiVersion: v1 kind: Secret type: Opaque metadata: name: gitea-postgres-ssl data: postgresql.crt: <base64...> postgresql.key: <base64...> root.crt: <base64...> ``` and then mounting this as a volume in Gitea using: ``` extraVolumes: - name: postgres-ssl-vol secret: secretName: gitea-postgres-ssl extraVolumeMounts: - name: postgres-ssl-vol readOnly: true mountPath: "/pg-ssl" ``` To get the right permissions on the credentials, we'd use the `initPreScript`: ``` initPreScript: | # copy postgres client and CA cert from mount and # give proper permissions mkdir -p /data/git/.postgresql cp /pg-ssl/* /data/git/.postgresql/ chown -R git:git /data/git/.postgresql/ chmod 400 /data/git/.postgresql/postgresql.key ``` and to make sure that Gitea uses the certificate we need to pass the proper postgres environment variables (both to the init container and the "main" container): ``` statefulset: env: - name: "PGSSLCERT" value: "/data/git/.postgresql/postgresql.crt" - name: "PGSSLKEY" value: "/data/git/.postgresql/postgresql.key" - name: "PGSSLROOTCERT" value: "/data/git/.postgresql/root.crt" ```
luhahn commented 2020-10-20 08:13:57 +00:00 (Migrated from gitea.com)
Copy Link

Thank you, I will test this later that day :)

Thank you, I will test this later that day :)
luhahn (Migrated from gitea.com) approved these changes 2020-10-21 13:15:23 +00:00
luhahn (Migrated from gitea.com) left a comment
Copy Link

looks good to me, tested basic functionality but didnt have time to test ssl.

looks good to me, tested basic functionality but didnt have time to test ssl.
luhahn commented 2020-10-21 13:17:08 +00:00 (Migrated from gitea.com)
Copy Link

@petergardfjall i guess you've tested, that ssl works since you've provided a detailed description?

We maybe should add the description into the README with another PR

@petergardfjall i guess you've tested, that ssl works since you've provided a detailed description? We maybe should add the description into the README with another PR
petergardfjall commented 2020-10-22 06:31:52 +00:00 (Migrated from gitea.com)
Copy Link

@petergardfjall i guess you've tested, that ssl works since you've provided a detailed description?

We maybe should add the description into the README with another PR

Yes, I could probably contribute a PR for the README if you'd like.

> @petergardfjall i guess you've tested, that ssl works since you've provided a detailed description? > > We maybe should add the description into the README with another PR Yes, I could probably contribute a PR for the README if you'd like.
luhahn (Migrated from gitea.com) reviewed 2020-10-29 17:07:35 +00:00
luhahn (Migrated from gitea.com) left a comment
Copy Link

I will have another look into this PR

I will have another look into this PR
petergardfjall commented 2020-11-03 08:08:53 +00:00 (Migrated from gitea.com)
Copy Link

I think that this PR is in need of another reviewer.

I think that this PR is in need of another reviewer.
lunny commented 2020-11-03 08:17:31 +00:00 (Migrated from gitea.com)
Copy Link

Please resolve the conflicts.

Please resolve the conflicts.
luhahn commented 2020-11-03 09:57:49 +00:00 (Migrated from gitea.com)
Copy Link

I was thinking about maybe implementing a build in variant to support certificate authorization. So that user can easily point to a cert, key and ca. This would eliminate the need of a pre init script. And would give an easier approach to support cert verification.

I was thinking about maybe implementing a build in variant to support certificate authorization. So that user can easily point to a cert, key and ca. This would eliminate the need of a pre init script. And would give an easier approach to support cert verification.
luhahn (Migrated from gitea.com) reviewed 2020-11-03 09:58:24 +00:00
luhahn (Migrated from gitea.com) left a comment
Copy Link

resolv conflicts and maybe discuss if we should implement a build in variant for certificates

resolv conflicts and maybe discuss if we should implement a build in variant for certificates
petergardfjall commented 2020-11-04 09:42:16 +00:00 (Migrated from gitea.com)
Copy Link

@luhahn I rebased from master and (force) pushed to my fork (https://gitea.com/petergardfjall/helm-chart/commits/branch/support-postgres-ssl), which I expected to update the commits of this PR as well, but didn't seem to happen (the PR commits hashes are still the same). What's the process for this?

@luhahn I rebased from `master` and (force) pushed to my fork (https://gitea.com/petergardfjall/helm-chart/commits/branch/support-postgres-ssl), which I expected to update the commits of this PR as well, but didn't seem to happen (the PR commits hashes are still the same). What's the process for this?
luhahn commented 2020-11-04 13:51:29 +00:00 (Migrated from gitea.com)
Copy Link

@petergardfjall that is indeed weird, i've had a look at your branch and it is up to date with the current master.

Normally the PR should update automatically

@petergardfjall that is indeed weird, i've had a look at your branch and it is up to date with the current master. Normally the PR should update automatically
petergardfjall commented 2020-11-13 12:26:27 +00:00 (Migrated from gitea.com)
Copy Link

@petergardfjall that is indeed weird, i've had a look at your branch and it is up to date with the current master.

Normally the PR should update automatically

I pushed an empty commit and that seems to have done the trick. The commits are now updated.

> @petergardfjall that is indeed weird, i've had a look at your branch and it is up to date with the current master. > > Normally the PR should update automatically I pushed an empty commit and that seems to have done the trick. The commits are now updated.
luhahn (Migrated from gitea.com) approved these changes 2020-11-19 10:41:56 +00:00
luhahn (Migrated from gitea.com) left a comment
Copy Link

Sorry for the delay, had a lot of todos recently.

PR looks good, I think that the extra init parameters are quite useful had a few usecases myself.

Sorry for the delay, had a lot of todos recently. PR looks good, I think that the extra init parameters are quite useful had a few usecases myself.
luhahn commented 2020-11-19 10:42:41 +00:00 (Migrated from gitea.com)
Copy Link

@petergardfjall can you once again update your branch? So we can merge it as soon as possible? Sorry again for the delay

@petergardfjall can you once again update your branch? So we can merge it as soon as possible? Sorry again for the delay
petergardfjall commented 2020-11-20 15:00:31 +00:00 (Migrated from gitea.com)
Copy Link

@petergardfjall can you once again update your branch? So we can merge it as soon as possible? Sorry again for the delay

@luhahn Ok, just updated.

> @petergardfjall can you once again update your branch? So we can merge it as soon as possible? Sorry again for the delay @luhahn Ok, just updated.
luhahn commented 2020-11-25 08:59:59 +00:00 (Migrated from gitea.com)
Copy Link

Can someone have a look at this PR? It has been open quite a while now.

The fact that this is introducing an initPreScript to inject an own script into the container is okay, since this enables users to customize the container even more.

Can someone have a look at this PR? It has been open quite a while now. The fact that this is introducing an initPreScript to inject an own script into the container is okay, since this enables users to customize the container even more.
lunny commented 2020-11-28 06:38:08 +00:00 (Migrated from gitea.com)
Copy Link

Maybe we should change the chart version. @luhahn @petergardfjall ?

Maybe we should change the chart version. @luhahn @petergardfjall ?
luhahn commented 2020-12-03 09:17:25 +00:00 (Migrated from gitea.com)
Copy Link

Yes an increment in the chart version can be done here or with another feature. Here would be better i guess.

Yes an increment in the chart version can be done here or with another feature. Here would be better i guess.
petergardfjall commented 2021-01-14 07:48:03 +00:00 (Migrated from gitea.com)
Copy Link

What exactly do you want me to do to get this PR merged? Bump the minor version in Chart.yaml to 0.0.1?

What exactly do you want me to do to get this PR merged? Bump the minor version in `Chart.yaml` to `0.0.1`?
luhahn commented 2021-01-14 09:30:40 +00:00 (Migrated from gitea.com)
Copy Link

No, we don't need the Chart version anymore. I still think that we should merge this, but we first need the conflicts to be resolved + another reviewer

No, we don't need the Chart version anymore. I still think that we should merge this, but we first need the conflicts to be resolved + another reviewer
petergardfjall commented 2021-01-15 13:02:08 +00:00 (Migrated from gitea.com)
Copy Link

No, we don't need the Chart version anymore. I still think that we should merge this, but we first need the conflicts to be resolved + another reviewer

Okay, I've rebased onto master.

> No, we don't need the Chart version anymore. I still think that we should merge this, but we first need the conflicts to be resolved + another reviewer Okay, I've rebased onto `master`.
6543 (Migrated from gitea.com) approved these changes 2021-01-20 11:14:01 +00:00
Sign in to join this conversation.
Reviewers
No Reviewers
luhahn
6543
Labels
Clear labels
has/backport
in progress
invalid
kind/breaking
kind/bug
kind/build
kind/dependency
kind/deployment
kind/docs
kind/enhancement
kind/feature
kind/lint
kind/proposal
kind/question
kind/refactor
kind/security
kind/testing
kind/translation
kind/ui
need/backport
priority/critical
priority/low
priority/maybe
priority/medium
reviewed/duplicate
reviewed/invalid
reviewed/wontfix
skip-changelog
status/blocked
status/needs-feedback
status/needs-reviews
status/wip
upstream/gitea
upstream/other
No Label
kind/feature
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
lunny
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: lunny/helm-chart#47
No description provided.
Delete Branch "refs/pull/47/head"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?