gitea/helm-chart
45
55
Fork 125
Code Issues 32 Pull Requests 8 Actions Releases 87 Activity

Generic way for configuring Gitea app.ini #240

Merged
luhahn merged 8 commits from justusbunsi/helm-chart:feature/rework-sensitive-ini-settings into master 2021-12-22 10:44:05 +00:00
Conversation 7 Commits 8 Files Changed 6 +314 -138
justusbunsi commented 2021-11-12 18:32:29 +00:00
Member
Copy Link

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.

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 added this to the 5.0.0 milestone 2021-11-12 18:32:29 +00:00
justusbunsi added the
kind
feature
status
needs-feedback
status
wip
 labels 2021-11-12 18:32:29 +00:00
justusbunsi added 1 commit 2021-11-12 18:32:30 +00:00
Rework app.ini generation 
- Now generated by environment-to-ini to persist and not overwrite app.ini
All checks were successful
continuous-integration/drone/push Build is passing
Details
continuous-integration/drone/pr Build is passing
Details
58dfa3de86
justusbunsi commented 2021-11-12 18:40:14 +00:00
Author
Member
Copy Link

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 force-pushed feature/rework-sensitive-ini-settings from 119d7352b7 to 5def46b650 2021-11-12 18:42:34 +00:00 Compare
justusbunsi commented 2021-11-13 11:27:06 +00:00
Author
Member
Copy Link

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 added the
kind
security
 label 2021-11-13 11:35:54 +00:00
justusbunsi changed title from WIP: Rework sensitive ini settings to WIP: Generic way for configuring Gitea 2021-11-13 11:48:17 +00:00
justusbunsi removed this from the 5.0.0 milestone 2021-11-13 12:21:27 +00:00
justusbunsi added this to the 5.0.0 milestone 2021-11-16 16:53:05 +00:00
justusbunsi added the
status
blocked
 label 2021-11-19 18:50:28 +00:00
justusbunsi added a new dependency 2021-11-19 18:50:53 +00:00
#244 Rework OAuth sources
justusbunsi force-pushed feature/rework-sensitive-ini-settings from 5def46b650 to 6ead599695 2021-11-19 23:08:42 +00:00 Compare
justusbunsi reviewed 2021-11-20 15:56:52 +00:00
templates/gitea/config.yaml Outdated
@ -162,8 +206,15 @@ stringData:
{{- end }}
{{- end }}
env2ini::load_additional_config_sources
justusbunsi commented 2021-11-20 15:56:52 +00:00
Author
Member
Copy Link

The functions I wrote above gives us the possibility to completely rewrite the config.yaml file content. We could split scripts from data and use a ConfigMap for the script instead of Secret. The values that are currently rendered by the Helm template engine could be written to a separate Kubernetes Secret which then gets processed along with the additional config sources.

I think it's out of scope for this PR but I really like the idea of separating user input from Chart script logic.

The functions I wrote above gives us the possibility to completely rewrite the `config.yaml` file content. We could split scripts from data and use a ConfigMap for the script instead of Secret. The values that are currently rendered by the Helm template engine could be written to a separate Kubernetes Secret which then gets processed along with the additional config sources. I think it's out of scope for this PR but I really like the idea of separating user input from Chart script logic.
justusbunsi commented 2021-12-19 17:55:39 +00:00
Author
Member
Copy Link

? I had time to implement this as well.

? I had time to implement this as well.
justusbunsi marked this conversation as resolved
justusbunsi commented 2021-11-20 15:58:13 +00:00
Author
Member
Copy Link

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 changed title from WIP: Generic way for configuring Gitea to Generic way for configuring Gitea 2021-11-20 15:58:20 +00:00
justusbunsi removed the
status
wip
 label 2021-11-20 15:58:30 +00:00
justusbunsi reviewed 2021-12-17 19:13:37 +00:00
templates/gitea/config.yaml Outdated
@ -13,0 +20,4 @@
# xargs echo -n trims all whitespaces and a trailing new line
local setting=$(awk -F '=' '{print $1}' <<< "${line}" | xargs echo -n)
local value=$(awk -F '=' '{print $NF}' <<< "${line}" | xargs echo -n)
justusbunsi commented 2021-12-17 19:13:37 +00:00
Author
Member
Copy Link

TODO: Proper handling for values with = and " inside.

TODO: Proper handling for values with `=` and `"` inside.
justusbunsi marked this conversation as resolved
justusbunsi force-pushed feature/rework-sensitive-ini-settings from df3539a056 to b206414764 2021-12-18 11:34:18 +00:00 Compare
justusbunsi reviewed 2021-12-18 17:56:24 +00:00
justusbunsi left a comment
Author
Member
Copy Link

Some thoughts for reviewers

Some thoughts for reviewers
templates/gitea/config.yaml Outdated
@ -13,0 +32,4 @@
fi
local value=''
local regex="^${setting}(\s*)=(\s*)(.*)"
justusbunsi commented 2021-12-18 17:52:47 +00:00
Author
Member
Copy Link

Users can have spaces around the = character in their config sources. Just like it is possible within app.ini. This regex takes this into account when stripping the setting name from the line to parse.

Users can have spaces around the `=` character in their config sources. Just like it is possible within `app.ini`. This regex takes this into account when stripping the setting name from the line to parse.
justusbunsi marked this conversation as resolved
templates/gitea/config.yaml Outdated
@ -13,0 +37,4 @@
value="${BASH_REMATCH[3]}"
else
env2ini::log ' ! invalid setting'
exit 1
justusbunsi commented 2021-12-18 17:55:39 +00:00
Author
Member
Copy Link

I've decided to let the script fail in case any line is not processable. That way the user is required to fix their configuration before the currently applied breaks somehow. Open for discussion here.

I've decided to let the script fail in case any line is not processable. That way the user is required to fix their configuration before the currently applied breaks somehow. Open for discussion here.
justusbunsi marked this conversation as resolved
justusbunsi removed the
status
needs-feedback
 label 2021-12-18 18:10:56 +00:00
justusbunsi changed title from Generic way for configuring Gitea to Generic way for configuring Gitea app.ini 2021-12-19 17:37:18 +00:00
justusbunsi reviewed 2021-12-19 17:40:22 +00:00
templates/gitea/config.yaml Outdated
@ -164,0 +53,4 @@
env2ini::log " + '${setting}'"
if [[ -z "${section}" ]]; then
export "ENV_TO_INI____${setting^^}=${value}" # '^^' makes the variable content uppercase
justusbunsi commented 2021-12-19 17:40:22 +00:00
Author
Member
Copy Link

____ ensures that global settings inside app.ini are actually taken into account and stored at the correct ini level.

This broke with the switch to environment-to-ini tool.

`____` ensures that global settings inside app.ini are actually taken into account and stored at the correct ini level. This broke with the switch to `environment-to-ini` tool.
justusbunsi marked this conversation as resolved
justusbunsi commented 2021-12-19 17:54:53 +00:00
Author
Member
Copy Link
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 approved these changes 2021-12-20 11:51:35 +00:00
luhahn left a comment
Member
Copy Link

LGTM, already tested a bunch of values

LGTM, already tested a bunch of values
justusbunsi force-pushed feature/rework-sensitive-ini-settings from 7bcfd26322 to b486175eca 2021-12-20 12:04:29 +00:00 Compare
justusbunsi force-pushed feature/rework-sensitive-ini-settings from b486175eca to e3d01dd8f6 2021-12-20 15:33:17 +00:00 Compare
justusbunsi removed the
status
blocked
 label 2021-12-20 15:34:11 +00:00
justusbunsi force-pushed feature/rework-sensitive-ini-settings from e3d01dd8f6 to c33e34e956 2021-12-21 11:02:59 +00:00 Compare
justusbunsi commented 2021-12-21 11:03:44 +00:00
Author
Member
Copy Link

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

Ok. Kept it in sync with head branch the last 24 hours. ?
nmasse-itix commented 2021-12-21 15:41:07 +00:00
Contributor
Copy Link

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 added 1 commit 2021-12-21 15:55:11 +00:00
Fix indention level for extraVolumeMounts
All checks were successful
continuous-integration/drone/pr Build is passing
Details
52541836ea
justusbunsi commented 2021-12-21 15:55:38 +00:00
Author
Member
Copy Link

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.
justusbunsi added 1 commit 2021-12-21 16:09:18 +00:00
Stop trying to map mounts as volumes
All checks were successful
continuous-integration/drone/pr Build is passing
Details
073a56ff86
lotherk approved these changes 2021-12-22 10:23:19 +00:00
lotherk left a comment
Member
Copy Link

looks good to me

looks good to me
wxiaoguang approved these changes 2021-12-22 10:39:51 +00:00
luhahn merged commit 7b0a1c7ae6 into master 2021-12-22 10:44:05 +00:00
justusbunsi deleted branch feature/rework-sensitive-ini-settings 2021-12-23 13:48:24 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
luhahn
lotherk
wxiaoguang
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
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
Milestone
Clear milestone
No items
No Milestone
5.0.0
Assignees
Clear assignees
No Assignees
5 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Depends on
#244 Rework OAuth sources
gitea/helm-chart
Reference: gitea/helm-chart#240
No description provided.
Delete Branch "justusbunsi/helm-chart:feature/rework-sensitive-ini-settings"

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?