gitea/helm-gitea
Fork 169
Code Issues 53 Pull Requests 11 Actions Releases 103 Activity

enhancements to support postgres client-cert authentication #47

Merged
luhahn merged 3 commits from petergardfjall/helm-chart:support-postgres-ssl into master 2021-01-20 11:28:40 +00:00
Conversation 22 Commits 3 Files Changed 4 +58 -8
petergardfjall commented 2020-10-19 08:35:05 +00:00
Contributor
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
Member
Copy Link

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

Thank you, I will test this later that day :)
lunny added the
kind
feature
 label 2020-10-20 08:17:30 +00:00
luhahn approved these changes 2020-10-21 13:15:23 +00:00
Dismissed
luhahn left a comment
Member
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
Member
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
Author
Contributor
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 reviewed 2020-10-29 17:07:35 +00:00
Dismissed
luhahn left a comment
Member
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
Author
Contributor
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
Owner
Copy Link

Please resolve the conflicts.

Please resolve the conflicts.
luhahn commented 2020-11-03 09:57:49 +00:00
Member
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 requested changes 2020-11-03 09:58:24 +00:00
Dismissed
luhahn left a comment
Member
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
Author
Contributor
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
Member
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
Author
Contributor
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 approved these changes 2020-11-19 10:41:56 +00:00
Dismissed
luhahn left a comment
Member
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
Member
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 force-pushed support-postgres-ssl from 4f9816f423 to 4d6f34faf1 2020-11-20 14:59:36 +00:00 Compare
petergardfjall commented 2020-11-20 15:00:31 +00:00
Author
Contributor
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
Member
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
Owner
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
Member
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
Author
Contributor
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
Member
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 force-pushed support-postgres-ssl from 4d6f34faf1 to 43d812d873 2021-01-15 13:01:13 +00:00 Compare
petergardfjall commented 2021-01-15 13:02:08 +00:00
Author
Contributor
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 approved these changes 2021-01-20 11:14:01 +00:00
Dismissed
luhahn merged commit 57479bdf37 into master 2021-01-20 11:28:40 +00:00
lafriks deleted branch support-postgres-ssl 2021-01-20 12:07:45 +00:00
Sign in to join this conversation.
Reviewers
No Reviewers
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
workaround
No Label
kind
feature
Milestone
No items
No Milestone
Assignees
Clear assignees
42wim 6543 a1012112796 Akkuman appleboy axifive betacat0 blumia bwko caicandong ChristopherHX DaanSelen davidsvantesson delvh denyskon fuxiaohei gary-kim garymoon giteabot guillep2k HesterG hiifong jaqra jolheiser jonasfranz jpraet kdumontnu kemzeb kerwin612 khmarbaise KN4CK3R kolaente lafriks lng2020 lotherk luhahn lunny mrsdizzie noerw pat-s puni9869 renovate-bot richmahn rossigee sapk screenshots sillyguodong silverwind smartclip_tim strk tboerger techknowlogick TheFox0x7 tobiasbp typeless volker.raschek wolfogre wxiaoguang yardenshoham yp05327 zeripath Zettat123
No Assignees
4 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: gitea/helm-gitea#47
No description provided.
Delete Branch "petergardfjall/helm-chart:support-postgres-ssl"

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?