This change request includes two different things to improve OAuth source handling:
- Allow multiple OAuth source configuration (Fixes: #191)
- Support reading sensitive OAuth configuration data from Kubernetes secrets (Closes: #242)
⚠️ BREAKING ⚠️
---
Users need to migrate their `gitea.oauth` configuration.
Reviewed-on: https://gitea.com/gitea/helm-chart/pulls/244
Reviewed-by: luhahn <[email protected]>
Reviewed-by: Andrew Thornton <[email protected]>
Co-authored-by: justusbunsi <[email protected]>
Co-committed-by: justusbunsi <[email protected]>
fixes#251
The rendering is a bit more programatic but the result is the same if you don't have an override. This makes the code a little easier at the end of the template, and slightly less easier to read at the beginning, which I think is a valid tradeoff.
Co-authored-by: Martyn Ranyard <[email protected]>
Reviewed-on: https://gitea.com/gitea/helm-chart/pulls/252
Reviewed-by: justusbunsi <[email protected]>
Reviewed-by: luhahn <[email protected]>
Co-authored-by: iMartyn <[email protected]>
Co-committed-by: iMartyn <[email protected]>
Hello !
I'm using the new Helm chart (5.x) and I really like the new configuration mechanism. 👍
I would like to contribute the following enhancement.
## The problem I want to solve
I'm trying to deploy Gitea in a Kubernetes shared platform and I need to make sure each instance is running as a different user so that in case of container escape, the risk of data leak is minimized.
Additionally, on my platform (OpenShift), arbitrary users (such as uid 1000 for Gitea) are not allowed.
The current helm chart does not allow me to achieve this because:
- the container security context is configurable only for the main container. The security context of init containers cannot be specified.
- a fixed uid is hard coded
- a fixed fs group is hard coded
Also, the securityContext of a pod and the securityContext of a container do not accept the same options.
- https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.19/#podsecuritycontext-v1-core
- https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.19/#securitycontext-v1-core
## How I'm solving the problem
I split the `securityContext` (values.yaml) in two: `containerSecurityContext` and `podSecurityContext`. The containerSecurityContext applies to all containers (init and main) in order to be consistent with file permissions.
The behavior for existing deployments is unchanged:
- fsGroup 1000 is the default value for the podSecurityContext variable
- the "configure-gitea" init container uses the uid 1000 unless otherwise stated in the containerSecurityContext
- the main container is using the existing securityContext variable when defined in order not to break existing deployments and uses the new containerSecurityContext variable if not.
This approach is well tested: it is used consistently on bitnami's Helm charts.
## How I tested
I tested both root and rootless variants on a Kubernetes 1.22, as well as rootless variant on OpenShift 4.7.
**rootless variant on Kubernetes**:
```yaml
podSecurityContext:
fsGroup: 10001
containerSecurityContext:
allowPrivilegeEscalation: false
capabilities:
drop:
- ALL
add:
- SYS_CHROOT
privileged: false
runAsGroup: 10001
runAsNonRoot: true
runAsUser: 10001
extraVolumes:
- name: var-lib-gitea
emptyDir: {}
extraVolumeMounts:
- name: var-lib-gitea
readOnly: false
mountPath: "/var/lib/gitea"
```
**rootless variant on OpenShift**:
```yaml
podSecurityContext:
fsGroup: null
containerSecurityContext:
allowPrivilegeEscalation: false
privileged: false
runAsNonRoot: true
runAsUser: 1000790000
extraVolumes:
- name: var-lib-gitea
emptyDir: {}
extraVolumeMounts:
- name: var-lib-gitea
readOnly: false
mountPath: "/var/lib/gitea"
```
Let me know if something is unclear.
Co-authored-by: Nicolas MASSE <[email protected]>
Reviewed-on: https://gitea.com/gitea/helm-chart/pulls/259
Reviewed-by: luhahn <[email protected]>
Reviewed-by: justusbunsi <[email protected]>
Co-authored-by: nmasse-itix <[email protected]>
Co-committed-by: nmasse-itix <[email protected]>
Hi,
I just add some minor changes to support specifying ingressClassName to support the newer specification in `networking.k8s.io/v1`. The annotation `kubernetes.io/ingress.class: nginx` only works with older API `networking.k8s.io/v1beta1`.
This is part of our move to support kubernetes 1.22.
Co-authored-by: Leong Wai Kit <[email protected]>
Reviewed-on: https://gitea.com/gitea/helm-chart/pulls/217
Reviewed-by: luhahn <[email protected]>
Reviewed-by: techknowlogick <[email protected]>
Co-authored-by: wkit23 <[email protected]>
Co-committed-by: wkit23 <[email protected]>
Currently there are two different styles for defining both ldap and oauth configuration in _values.yaml_ file: `camelCase` and `kebab-case`.
Supporting both styles created multiple regressions in the past.
⚠️ BREAKING ⚠️
---------------
These changes completely remove any support for `kebab-case` notation in _values.yaml_ in favor of `camelCase`. Configuration keys must use `camelCase`.
Only exception are Kubernetes resource keys for annotations or labels.
Fixes: #188
Reviewed-on: https://gitea.com/gitea/helm-chart/pulls/196
Reviewed-by: luhahn <[email protected]>
Reviewed-by: Lunny Xiao <[email protected]>
Co-authored-by: justusbunsi <[email protected]>
Co-committed-by: justusbunsi <[email protected]>
These changes rewrite the init script to be error aware, informative and have a bit more security awareness.
During rewrite several hidden bugs could be identified and fixed, such as:
- LDAP configuration options interpreted by the shell before passed to command
- Finding multiple ldap ids instead of one during lookup when their names are almost identical
e.g. `_my-ldap-auth` and `my-ldap-auth`
- Properly filter auth sources by their types to prevent unintended type converting attempts that fail
In addition to that the script is a bit cleaner. Some commands do not exist anymore and would cause false-positive errors during script execution.
Helps for: #149
Reviewed-on: https://gitea.com/gitea/helm-chart/pulls/178
Reviewed-by: luhahn <[email protected]>
Reviewed-by: techknowlogick <[email protected]>
Co-authored-by: justusbunsi <[email protected]>
Co-committed-by: justusbunsi <[email protected]>
The `HOME` path is not persistent when using the rootless image, so the
`.gnupg` folder isn't either. Since the chart always used `/data/...` as
mount point for storage of all kinds, it is a minimal impact to just
relocate the dynamic `$HOME/.gnupg` folder location to the persistent
`/data/git/.gnupg`. This is where the signing keys are stored when
running root based environments. Doing so will
- allow migrations between both image variants
- persist signing keys for rootless environments
Fixes: #155
Co-authored-by: techknowlogick <[email protected]>
Reviewed-on: https://gitea.com/gitea/helm-chart/pulls/186
Reviewed-by: luhahn <[email protected]>
Reviewed-by: techknowlogick <[email protected]>
Co-authored-by: justusbunsi <[email protected]>
Co-committed-by: justusbunsi <[email protected]>
This fixes several flaws introduced by commits for #169 (see c49dc047a4).
- Respect kebab-case ldap bind inline definition
- Prevent camelCase ldap bind inline definition from being overridden by empty string
- Create admin account when `existingSecret` is used
Reviewed-on: https://gitea.com/gitea/helm-chart/pulls/183
Reviewed-by: Andrew Thornton <[email protected]>
Reviewed-by: luhahn <[email protected]>
Co-authored-by: justusbunsi <[email protected]>
Co-committed-by: justusbunsi <[email protected]>
Due to #160 it was no longer possible to create repositories in root-based containers. This was caused by the missing `/tmp/gitea` directory in that image. It was dynamically created by Gitea internal functionality with less privileges than necessary.
Explicitly creating the directory and set proper permissions fix this.
Fixes: #171
Co-authored-by: JustusBunsi <[email protected]>
Reviewed-on: https://gitea.com/gitea/helm-chart/pulls/172
Reviewed-by: luhahn <[email protected]>
Reviewed-by: 6543 <[email protected]>
Co-authored-by: justusbunsi <[email protected]>
Co-committed-by: justusbunsi <[email protected]>
At least the latest versions of prometheus watch for a label called "release" and not "prometheus" so with that change prometheus started collecting logs.
If that old label is still needed, we / I could change it to add entries from "additionalLabels", so that we are quite flexible and don't have breaking changes.
Co-authored-by: Justin Lamp <[email protected]>
Co-authored-by: techknowlogick <[email protected]>
Reviewed-on: https://gitea.com/gitea/helm-chart/pulls/157
Reviewed-by: luhahn <[email protected]>
Reviewed-by: techknowlogick <[email protected]>
Co-authored-by: modzilla <[email protected]>
Co-committed-by: modzilla <[email protected]>
I've noticed that the commented `securityContext` is not really useable with the rootless image due to different directory structure compared to the default image.
Important for the `readOnlyRootFilesystem` is to declare the `TMPDIR` environment variable, so that the tmp directory (which is readonly in this case) won't be used. Instead, another writeable directory can be used.
Another thing is the explicit hint that all these security options cannot be used with the default (root-based) image, because of its design.
~~Although this PR would fix the referenced issue, I am not totally happy with the current implementation. It would be more straight forward to use the same mount points for both image variants. Unfortunately, this is not possible right now due to hard coded paths in the default (root) image startup scripts.~~
~~Anyone have suggestions on how this could be more simple?~~
-------
**Sum-up:**
As mentioned in Discord, this PR tried to make too many changes. The necessary changes made in 1f331a7e6577fc798196a84a957330aca0d663cd will fix an error that occurs due to restricted access to the `/tmp` directory in a rootless image with all the `securityContext` options enabled.
I also updated the default image to 1.14.2.
Fixes: #158
Co-authored-by: JustusBunsi <[email protected]>
Reviewed-on: https://gitea.com/gitea/helm-chart/pulls/160
Reviewed-by: luhahn <[email protected]>
Reviewed-by: 6543 <[email protected]>
Co-authored-by: justusbunsi <[email protected]>
Co-committed-by: justusbunsi <[email protected]>
This change adds a new value *statefulset.labels* to allow the user to add custom labels to the StatefulSet.
An example of where this could be useful is if gitea's pvc is stored on OpenEBS. With this new option, the user can add the extra *openebs.io/sts-target-affinity* label to specify that the volume target pod should run on the same node as gitea's StatefulSet.
Co-authored-by: Baptiste Covolato <[email protected]>
Reviewed-on: https://gitea.com/gitea/helm-chart/pulls/130
Reviewed-by: luhahn <[email protected]>
Reviewed-by: lafriks <[email protected]>
Co-authored-by: Nakrez <[email protected]>
Co-committed-by: Nakrez <[email protected]>
There are currently 2 issues that prevent using this chart to deploy gitea with a SQLite3 database.
1) The value from *gitea.config.database.HOST* is used to set *db.servicename* when all the databases under *gitea.database.buildIn* are not enabled. This causes a type error during the template processing:
`Error: UPGRADE FAILED: template: gitea/templates/gitea/init.yaml:24:20: executing "gitea/templates/gitea/init.yaml" at <include "db.servicename" .>: error calling include: template: gitea/templates/_helpers.tpl:64:31: executing "db.servicename" at <.Values.gitea.config.database.HOST>: wrong type for value; expected string; got interface {}`
2) In *init_gitea.sh*, we use the value *db.servicename* and *db.port* to ping the database. If this database responds to ping, we proceed with the init. The problem here is that *db.port* is not set when all the databases under *gitea.database.buildIn* are disabled. In turn, this raises an error from busybox's *nc*, because no parameter is passed for *PORT*. This causes the init container to go in *CrashLoopBackOff* forever.
The simple fix that is proposed in this PR is to check wether or not *.Values.gitea.config.database.DB_TYPE* is set to determine the value *db.servicename*. If *DB_TYPE* is *'sqlite3'*, leave *db.servicename* empty and use that to bypass the database ping.
Co-authored-by: Baptiste Covolato <[email protected]>
Reviewed-on: https://gitea.com/gitea/helm-chart/pulls/124
Reviewed-by: Andrew Thornton <[email protected]>
Reviewed-by: lafriks <[email protected]>
Reviewed-by: luhahn <[email protected]>
Co-authored-by: Nakrez <[email protected]>
Co-committed-by: Nakrez <[email protected]>
This pull request adds the `app` and `version` labels that are used by Istio.
> Pods with app and version labels: We recommend adding an explicit app label and version label to the specification of the pods deployed using a Kubernetes Deployment. The app and version labels add contextual information to the metrics and telemetry that Istio collects.
>
> * The app label: Each deployment should have a distinct app label with a meaningful value. The app label is used to add contextual information in distributed tracing.
>
> * The version label: This label indicates the version of the application corresponding to the particular deployment.
From https://istio.io/latest/docs/ops/deployment/requirements/#pod-requirements
Reviewed-on: https://gitea.com/gitea/helm-chart/pulls/121
Reviewed-by: luhahn <[email protected]>
Reviewed-by: lafriks <[email protected]>
Co-authored-by: Starefossen <[email protected]>
Co-committed-by: Starefossen <[email protected]>
justusbunsi
aleksey.sergeyandAleksey Sergey
iMartynandMartyn Ranyard
nmasse-itixandNicolas MASSE
luhahnandLucas Hahn
Markus PeschandMarkus Pesch
techknowlogick
pat-sandpat-s
wkit23andLeong Wai Kit
Michael KrieseandLunny Xiao
skrieschandtechknowlogick
mattkaarandMatt Kaar
mrdimaandDimitri Ars
kaipmdhandlafriks
NakrezandBaptiste Covolato
Dunky13andMarc Went
Starefossen