Generic way for configuring Gitea app.ini #240

2021-11-12T18:32:29Z

justusbunsi commented

2021-11-12 18:32:29 +00:00

(Migrated from gitea.com)

With the result of PR #239 it is much easier to provide additional values to the app.ini configuration from different sources.
These changes adds an additionalConfigSources field where the users can define such sources. This enables the users to choose
on their own whether to store values in values.yaml or load them from Kuberetes Secrets or ConfigMaps.

Fixes #243
Fixes #174
Fixes #260

With the result of PR #239 it is much easier to provide additional values to the _app.ini_ configuration from different sources. These changes adds an _additionalConfigSources_ field where the users can define such sources. This enables the users to choose on their own whether to store values in _values.yaml_ or load them from Kuberetes Secrets or ConfigMaps. - Fixes #243 - Fixes #174 - Fixes #260

justusbunsi commented

2021-11-12 18:40:14 +00:00

(Migrated from gitea.com)

Currently, I just wrote down my thoughts without implementing it, because I need a second opinion on what the users should actually define and how. Both ways require some structure for the references Kubernetes resources:

Use the current approach, mounted them as files and then converted into environment variables, or
Inject them directly as environment variables and let the user take care of things like GITEA__LOG_0x2E_CONSOLE__STDERR=false

The first approach would keep prefixing with GITEA_ to the Helm Chart as well as converting . into 0x2E. The second one is easier for the Helm Chart but more complex for the users.

@luhahn & @lunny, shall I proceed with the first approach? Details here.

EDIT: After chatting with Luhahn, I'll proceed with the current approach (the first one).

Currently, I just wrote down my thoughts without implementing it, because I need a second opinion on what the users should actually define and how. Both ways require some structure for the references Kubernetes resources: - Use the current approach, mounted them as files and then converted into environment variables, or - Inject them directly as environment variables and let the user take care of things like `GITEA__LOG_0x2E_CONSOLE__STDERR=false` The first approach would keep prefixing with `GITEA_` to the Helm Chart as well as converting `.` into `0x2E`. The second one is easier for the Helm Chart but more complex for the users. @luhahn & @lunny, shall I proceed with the first approach? Details [here](https://gitea.com/justusbunsi/helm-chart/src/branch/feature/rework-sensitive-ini-settings/README.md#additional-_app-ini_-settings). EDIT: After chatting with Luhahn, I'll proceed with the current approach (the first one).

justusbunsi commented

2021-11-13 11:27:06 +00:00

(Migrated from gitea.com)

~~Just realized that this fixes neither #174 nor the mentioned oauth topic since these values are not configured inside app.ini. ?‍♂️~~

~~But I think it is still a neat addition to the chart giving more flexibility about how to configure Gitea.~~

See PR comment.

~~Just realized that this fixes neither #174 nor the mentioned oauth topic since these values are not configured inside app.ini. ?‍♂️~~ ~~But I think it is still a neat addition to the chart giving more flexibility about how to configure Gitea.~~ See PR comment.

justusbunsi (Migrated from gitea.com) reviewed 2021-11-20 15:56:52 +00:00

justusbunsi commented

2021-11-20 15:58:13 +00:00

(Migrated from gitea.com)

Hi @lord-kyron and @volker.raschek. You've opened several issues in the past asking for options to securely pass sensitive data to the Gitea app.ini. This PR provides a really generic way to craft the app.ini from various sources.

Feel free to check it out and experiment with it. I'd be happy for any user feedback. Detailed description on how to use it can be found here.

⚠️ This PR is based on different open PRs and contains some breaking changes, so it is considered bloody edge. Please have a look at #244 and #248.

Hi @lord-kyron and @volker.raschek. You've opened several issues in the past asking for options to securely pass sensitive data to the Gitea _app.ini_. This PR provides a really generic way to craft the _app.ini_ from various sources. Feel free to check it out and experiment with it. I'd be happy for any user feedback. Detailed description on how to use it can be found [here](https://gitea.com/justusbunsi/helm-chart/src/branch/feature/rework-sensitive-ini-settings/README.md#additional-_app-ini_-settings). ⚠️ This PR is based on different open PRs and contains some breaking changes, so it is **considered bloody edge**. Please have a look at #244 and #248.

justusbunsi (Migrated from gitea.com) reviewed 2021-12-17 19:13:37 +00:00

justusbunsi (Migrated from gitea.com) reviewed 2021-12-18 17:52:47 +00:00

justusbunsi (Migrated from gitea.com) left a comment

Some thoughts for reviewers

justusbunsi (Migrated from gitea.com) reviewed 2021-12-19 17:40:22 +00:00

justusbunsi commented

2021-12-19 17:54:53 +00:00

(Migrated from gitea.com)

Glossary for this comment:
  - Inline sources:     `gitea.config` from values.yaml files
  - Additional sources: Kubernetes resources defined and provided by the users

The latest changes align the processing for inline sources with additional sources. Therefore I've added another Kubernetes secret where the inline sources are stored the same way as additional sources requires it.

Logging helps to understand what setting is used for the final app.ini and from which source it came. In case of duplicate settings, there is a (natural) override mechanism:

User values.yaml over default Helm Chart values.yaml settings
Settings from additional sources over inline sources

The already ensured persistence for "secret" settings remains untouched.

```plain Glossary for this comment: - Inline sources: `gitea.config` from values.yaml files - Additional sources: Kubernetes resources defined and provided by the users ``` The latest changes align the processing for inline sources with additional sources. Therefore I've added another Kubernetes secret where the inline sources are stored the same way as additional sources requires it. Logging helps to understand what setting is used for the final `app.ini` and from which source it came. In case of duplicate settings, there is a (natural) override mechanism: - User `values.yaml` over default Helm Chart `values.yaml` settings - Settings from additional sources over inline sources The already ensured persistence for "secret" settings remains untouched.

luhahn (Migrated from gitea.com) approved these changes 2021-12-20 11:51:35 +00:00

luhahn (Migrated from gitea.com) left a comment

LGTM, already tested a bunch of values

justusbunsi commented

2021-12-21 11:03:44 +00:00

(Migrated from gitea.com)

Ok. Kept it in sync with head branch the last 24 hours. ?

nmasse-itix commented

2021-12-21 15:41:07 +00:00

(Migrated from gitea.com)

Hello !

I will try this PR in the afternoon.

Having reviewed the changes, I find the following benefits :

it solves a real pain (secrets not stored in values.yaml when using a gitops approach)
no more Bash code generation from templates (maintenability improved)

From a very first sight, it seems there is a typo in commit 30e4f75b (line 302-304).

        {{- if .Values.extraVolumeMounts }}
        {{- toYaml .Values.extraVolumeMounts | nindent 12 }}
        {{- end }}

Those lines are volume mounts in a volume section with the wrong indentation. It breaks templating when defining extra volume mounts in values.yaml.

Hello ! I will try this PR in the afternoon. Having reviewed the changes, I find the following benefits : - it solves a real pain (secrets not stored in values.yaml when using a gitops approach) - no more Bash code generation from templates (maintenability improved) From a very first sight, it seems there is a typo in commit 30e4f75b (line 302-304). ```yaml {{- if .Values.extraVolumeMounts }} {{- toYaml .Values.extraVolumeMounts | nindent 12 }} {{- end }} ``` Those lines are volume mounts in a volume section with the wrong indentation. It breaks templating when defining extra volume mounts in values.yaml.

justusbunsi commented

2021-12-21 15:55:38 +00:00

(Migrated from gitea.com)

Hello !

I will try this PR in the afternoon.

Awesome.

From a very first sight, it seems there is a typo in commit 30e4f75b (line 302-304).

Good catch. Fixed.

> Hello ! > > I will try this PR in the afternoon. Awesome. > From a very first sight, it seems there is a typo in commit 30e4f75b (line 302-304). Good catch. Fixed.

lotherk (Migrated from gitea.com) approved these changes 2021-12-22 10:23:19 +00:00

lotherk (Migrated from gitea.com) left a comment

looks good to me

wxiaoguang (Migrated from gitea.com) approved these changes 2021-12-22 10:39:51 +00:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: lunny/helm-chart#240