gitea/helm-chart
45
54
Fork 127
Code Issues 31 Pull Requests 8 Actions Releases 87 Activity

Rework app.ini generation #239

Merged
luhahn merged 7 commits from app-ini-rework into master 2021-11-19 21:15:46 +00:00
Conversation 1 Commits 7 Files Changed 4 +4 -10
3 changed files with 4 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 38077e63da - Show all commits

10
README.md
View File

@ -58,12 +58,11 @@ automatically in certain situations:
- New install: By default the secrets are created automatically. If you provide
secrets via `gitea.config` they will be used instead of automatic generation.
luhahn marked this conversation as resolved Outdated
justusbunsi commented 2021-11-17 19:26:56 +00:00
Outdated
Review
Copy Link

Same as above. Let's make it more outstanding.

> :rotating_light: Although the Chart provides resetting secret keys, it is 
> not advisable to do so for existing installations. Certain settings like
> _LDAP_ would not be readable anymore.
Same as above. Let's make it more outstanding. ``` > :rotating_light: Although the Chart provides resetting secret keys, it is > not advisable to do so for existing installations. Certain settings like > _LDAP_ would not be readable anymore. ```
- Existing installs: By default the secrets won't be deployed, neither via
configuration nor via auto generation.
- Existing install with `gitea.enforceAppSecretRecreation`: will allow again automatic
generation or deploy via `gitea.config`
- Existing installs: The secrets won't be deployed, neither via
configuration nor via auto generation. We explicitly prevent to set new secrets.
:rotating_light: Although the Chart provides resetting secret keys, it is
:rotating_light: It would be possible to set new secret keys manually by entering
the running container and rewriting the app.ini by hand. However, this it is
not advisable to do so for existing installations. Certain settings like
_LDAP_ would not be readable anymore.
@ -562,7 +561,6 @@ gitea:
| `initPreScript` | Bash script copied verbatim to start of init container | |
| `securityContext` | Run as a specific securityContext | `{}` |
| `schedulerName` | Use an alternate scheduler, e.g. "stork" | |
| `gitea.enforceAppSecretRecreation` | Enforce new secret key generation (SECRET_KEY, INTERNAL_TOKEN, etc.) | `false` |
### Image

2
templates/gitea/config.yaml
View File

@ -162,13 +162,11 @@ stringData:
{{- end }}
{{- end }}
{{- if not .Values.gitea.enforceAppSecretRecreation }}
# safety to prevent rewrite of secret keys if an app.ini already exists
if [ -f ${GITEA_APP_INI} ]; then
unset ENV_TO_INI__SECURITY__INTERNAL_TOKEN
unset ENV_TO_INI__SECURITY__SECRET_KEY
unset ENV_TO_INI__OAUTH2__JWT_SECRET
fi
{{- end }}
environment-to-ini -o $GITEA_APP_INI -p ENV_TO_INI

2
values.yaml
View File

@ -141,8 +141,6 @@ signing:
gpgHome: /data/git/.gnupg
gitea:
enforceAppSecretRecreation: false
admin:
luhahn marked this conversation as resolved Outdated
justusbunsi commented 2021-11-17 19:38:49 +00:00
Outdated
Review
Copy Link

This must be false to not break existing installs right away.

This must be `false` to not break existing installs right away.
justusbunsi commented 2021-11-18 18:36:06 +00:00
Outdated
Review
Copy Link

After playing around a bit, I am no longer sure this option is a good idea.

The configure-gitea init container does not run successfully on existing installations when configuring LDAP authentication sources as well. By changing the SECRET_KEY, the already configured authentication sources are no longer readable and neither creating nor editing them is possible, which leaves Gitea in a state where it can't do anything an exits.

This would break an upgrade from the chart.

In addition, I have strong concerns about how the shell script behaves with enforceAppSecretRecreation enabled during a scale-down and subsequent scale-up. Since this does not change the shell script, it still says that new values should be generated. I don't think that's the user's plan when doing e.g. a node maintenance or something, where Gitea is moved or stopped for a short time.

It might be best to just prevent overwriting these values completely if there already is an app.ini file. This way, neither a generated one nor a potentially changed value from the values.yaml will break an existing installation.

I too thought this would work better than it does.

After playing around a bit, I am no longer sure this option is a good idea. The `configure-gitea` init container does not run successfully on existing installations when configuring LDAP authentication sources as well. By changing the SECRET_KEY, the already configured authentication sources are no longer readable and neither creating nor editing them is possible, which leaves Gitea in a state where it can't do anything an exits. This would break an upgrade from the chart. In addition, I have strong concerns about how the shell script behaves with `enforceAppSecretRecreation` enabled during a scale-down and subsequent scale-up. Since this does not change the shell script, it still says that new values should be generated. I don't think that's the user's plan when doing e.g. a node maintenance or something, where Gitea is moved or stopped for a short time. It might be best to just prevent overwriting these values completely if there already is an app.ini file. This way, neither a generated one nor a potentially changed value from the values.yaml will break an existing installation. I too thought this would work better than it does.
luhahn commented 2021-11-19 09:40:22 +00:00
Outdated
Review
Copy Link

You're right, I've tested some scenarios as well and they all seem to brick gitea.
(I've previously tested with 1.14.6, which was fine :D, so I didn't noticed the problems)

-> removed the setting. User can still manually change the app.ini inside the container if they really need to.

You're right, I've tested some scenarios as well and they all seem to brick gitea. (I've previously tested with 1.14.6, which was fine :D, so I didn't noticed the problems) -> removed the setting. User can still manually change the app.ini inside the container if they really need to.
#existingSecret: gitea-admin-secret
username: gitea_admin