Rework app.ini generation #239
No reviewers
Labels
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
No Milestone
No Assignees
3 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: gitea/helm-chart#239
Loading…
Reference in New Issue
Block a user
No description provided.
Delete Branch "app-ini-rework"
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?
App ini is now generated by environment-to-ini
This should prevent some of the problems we had earlier with persisting the app.ini
Fixes #106
08d5a7bfd2
to58dfa3de86
We should document that the Chart now requires a Gitea image with environment-to-ini inside. Older images won't work anymore. Since this tool exists since 2019, but I think it's no breaking change in 2021.
9376bd7886
toccd459517d
df2797a765
to04169e9440
WIP: Rework app.ini generationto Rework app.ini generation@ -40,0 +46,4 @@
The app.ini generation has changed and now utilizes the environment-to-ini
script provided by newer Gitea versions.
:warning: Since environment to ini is used it is not possible to use Gitea
Let's add a line break right before the warning to have it in separate line. Or something like the following to have it more outstanding from the rest of this chapter.
@ -40,0 +53,4 @@
#### Secret Key generation
Gitea secret keys are now generated automatically if not provided. The values
The current text leaves room for interpretation regarding "automatically if not provided". I guess we should structure it a bit more on how the Chart in v1.5.0 will behave in various situations.
gitea.config
gitea.config
To me, such structure makes it sound less breaking.
@ -40,0 +57,4 @@
are not set, if an app.ini already is existing.
You can enforce new secret keys, if `gitea.forceSecrets` is set to true.
:warning: When secret keys are changed if already provided, it is possible,
Same as above. Let's make it more outstanding.
@ -535,0 +555,4 @@
| 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.forceSecrets` | Enforce new secret key generation (SECRET_KEY, INTERNAL_TOKEN, etc.) | `force` |
I think the default is either
true
orfalse
?. Why highlight the parameter instead of plain text as the rest in this table?And maybe use the same hint underneath this table is it is used in the changelog notes to tell people about the dangers of resetting such values for existing installs.
This is needed because of the linter. It would complain that gitea needs to be Gitea.
I now highlighted simply all the values in the values description.
@ -136,0 +142,4 @@
{{- if not .Values.gitea.forceSecrets }}
if [ ! -f ${GITEA_APP_INI} ]; then
{{- end }}
{{- if not (hasKey .Values.gitea.config.security "INTERNAL_TOKEN") }}
? I really like that the recreate script is not rendered if a value is already provided via configuration. It's a nice safety guard for us.
There's nothing to do here. ?
reworked this slighty
I removed the file check guard on the export/key generation and only added it to the unset section.
@ -149,0 +173,4 @@
unset ENV_TO_INI__SECURITY__INTERNAL_TOKEN
unset ENV_TO_INI__SECURITY__SECRET_KEY
unset ENV_TO_INI__OAUTH2__JWT_SECRET
{{- end }}
If I read this correctly, this section shall ensure these environment variables got removed if no forceful updated is requested.
Unluckily, this breaks initial installs when such values are provided via
gitea.config
. They get unset by this code block before being applied.Maybe it is enough to set the
forceSecrets
to false by default and explicitly mention that it would be a bad idea for existing installs to change those values afterwards. Instead of unset these values here. If the users choose to change the values anyway, we can't do something about it. ?♂️I added an if to check if the app.ini exists. This way we can unset the values if they are set either via config or are autogenerated and forceSecrets is false
@ -240,6 +263,7 @@ spec:
- name: config
secret:
secretName: {{ include "gitea.fullname" . }}
defaultMode: 0777
Would
execute
permissions not be enough?@ -141,6 +141,8 @@ signing:
gpgHome: /data/git/.gnupg
gitea:
forceSecrets: true
This must be
false
to not break existing installs right away.04169e9440
to9efbafdfa0
@ -135,1 +142,3 @@
{{- /* autogenerate app.ini */ -}}
{{- if not (hasKey .Values.gitea.config.security "INTERNAL_TOKEN") }}
export ENV_TO_INI__SECURITY__INTERNAL_TOKEN=$(gitea generate secret INTERNAL_TOKEN)
{{- end }}
Unluckily, these checks interfere with
gitea.enforceAppSecretRecreation
. If you have one or more of those settings defined, the script won't generate a new value for them.I think the rendering behaviour should be:
enforceAppSecretRecreation: false
enforceAppSecretRecreation: true
Did I miss anything?
@ -141,6 +141,8 @@ signing:
gpgHome: /data/git/.gnupg
gitea:
enforceAppSecretRecreation: false
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.
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.
6b98d40083
to38077e63da
?