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

Add support of sensitive data for external Database #279

Closed
fkocik wants to merge 3 commits from fkocik/gitea-helm-chart:feature/external-database into main
pull from: fkocik/gitea-helm-chart:feature/external-database
merge into: gitea:main
Conversation 9 Commits 3 Files Changed 2 +49 -9
fkocik commented 2022-01-18 08:52:17 +00:00
First-time contributor
Copy Link

Hi,

When using Gitea with an external database instance, it may be necessary to manipulate sensitive data that should not appear in the values.yaml file.

The purpose of this PR is to allow externalDatabase configuration injection in a secure way.

For each token, it will be possible to use a literal value (directly readable from the values.yaml file) or a secret reference (using name and key) in order to reuse existing data from Kubernetes engine.

Regards.

Hi, When using Gitea with an external database instance, it may be necessary to manipulate sensitive data that should not appear in the `values.yaml` file. The purpose of this PR is to allow `externalDatabase` configuration injection in a secure way. For each token, it will be possible to use a *literal* value (directly readable from the `values.yaml` file) or a *secret* reference (using `name` and `key`) in order to reuse existing data from **Kubernetes** engine. Regards.
fkocik added 1 commit 2022-01-18 08:52:18 +00:00
Add support of sensitive data for external Database
All checks were successful
continuous-integration/drone/pr Build is passing
Details
8ed963bdcb
fkocik added 1 commit 2022-01-18 08:55:19 +00:00
fix readme about literals/secret usage
Some checks failed
continuous-integration/drone/pr Build is failing
Details
a5e255aa55
fkocik changed title from WIP: Add support of sensitive data for external Database to Add support of sensitive data for external Database 2022-01-18 08:55:33 +00:00
fkocik added 1 commit 2022-01-18 08:59:05 +00:00
fix markdown lint failed
All checks were successful
continuous-integration/drone/pr Build is passing
Details
78428e7bf6
lafriks commented 2022-01-18 11:49:33 +00:00
Member
Copy Link

imho it would be better to allow using secrets for values without changing existing structure

imho it would be better to allow using secrets for values without changing existing structure
fkocik commented 2022-01-18 12:50:44 +00:00
Author
First-time contributor
Copy Link

It still remains possible through additionalConfigSources.

Yet, when using Kubernetes native approach (such as database operators) : it requires to synchronize data between database secrets and Gitea secrets.

The idea here is to reuse the same secrets using SecretKeySelector yaml style notation in values.yaml.

It still remains possible through `additionalConfigSources`. Yet, when using **Kubernetes** native approach (such as database operators) : it requires to synchronize data between database secrets and **Gitea** secrets. The idea here is to reuse the same secrets using [SecretKeySelector](https://pkg.go.dev/k8s.io/client-go/pkg/api#SecretKeySelector) yaml style notation in `values.yaml`.
lafriks commented 2022-01-18 19:55:51 +00:00
Member
Copy Link

I understand but why we could not extend it to support such syntax:

gitea:
  config:
    database:
      DB_TYPE: mysql
      HOST: 127.0.0.1:3306
      NAME: gitea
      USER: root
      PASSWD:
        secret:
          name: mysql-admin
          key: admin
      SCHEMA: gitea

while still supporting also

      PASSWD: gitea
I understand but why we could not extend it to support such syntax: ```yaml gitea: config: database: DB_TYPE: mysql HOST: 127.0.0.1:3306 NAME: gitea USER: root PASSWD: secret: name: mysql-admin key: admin SCHEMA: gitea ``` while still supporting also ```yaml PASSWD: gitea ```
👍 1
justusbunsi commented 2022-01-18 20:29:02 +00:00
Member
Copy Link

As far as I see this whole litetal vs. Kubernetes resource distinction is already possible through additionalConfigSources. I understand your idea to reuse such secrets and appreciate your invested time to contribute this. But implementing it that way seems like stepping back to previous configurations which were eliminated by additionalConfigSources. Is there a need for a more fine grained mapping of such additional sources to fit your need?

As far as I see this whole litetal vs. Kubernetes resource distinction is already possible through `additionalConfigSources`. I understand your idea to reuse such secrets and appreciate your invested time to contribute this. But implementing it that way seems like stepping back to previous configurations which were eliminated by `additionalConfigSources`. Is there a need for a more fine grained mapping of such additional sources to fit your need?
👍 1
fkocik commented 2022-01-19 08:58:28 +00:00
Author
First-time contributor
Copy Link

Thanks all for your feedback.

I did not succeed to perform such configuration using additionalConfigSources. Perhaps caused by my misunderstanding of the existing mechanisms.

My need is to avoid intermediate process that generate a secret from another existing secret and only describe the desired system state. So, I agree with @justusbunsi that more fine grained mapping of additionalConfigSources could be the solution.

I can try to review my PR in order to see if it is possible to use thoses resources.

Thanks all for your feedback. I did not succeed to perform such configuration using `additionalConfigSources`. Perhaps caused by my misunderstanding of the existing mechanisms. My need is to avoid intermediate process that generate a secret from another existing secret and only describe the desired system state. So, I agree with @justusbunsi that more fine grained mapping of `additionalConfigSources` could be the solution. I can try to review my PR in order to see if it is possible to use thoses resources.
justusbunsi commented 2022-02-10 14:17:35 +00:00
Member
Copy Link

It might not that easy to reuse an existing Kubernetes secret storing data for another application, e.g. database. The data structure necessary for additionalConfigSources is more or less an app.ini that got cut into pieces allowing to store settings from one section as Kubernetes Secret or ConfigMap or both. The keys for each entry in such Kubernetes resource must be the section name. Otherwise the data collecting won't work.

But I could be wrong and you find a neat way to achieve it. ?

It might not that easy to reuse an existing Kubernetes secret storing data for another application, e.g. database. The data structure necessary for `additionalConfigSources` is more or less an app.ini that got cut into pieces allowing to store settings from one section as Kubernetes Secret or ConfigMap or both. The keys for each entry in such Kubernetes resource must be the section name. Otherwise the data collecting won't work. But I could be wrong and you find a neat way to achieve it. ?
mmalyska commented 2022-02-24 08:28:48 +00:00
Contributor
Copy Link

Hi, this solution would really help. I'm using postgres operator that creates DB connect info as secret. To use them I have used

statefulset:
    env:
      - name: GITEA_database_HOST
        valueFrom:
          secretKeyRef:
            name: giteadb-pguser-giteadb
            key: host
      - name: GITEA_database_NAME
        valueFrom:
          secretKeyRef:
            name: giteadb-pguser-giteadb
            key: dbname
      - name: GITEA_database_USER
        valueFrom:
          secretKeyRef:
            name: giteadb-pguser-giteadb
            key: user
      - name: GITEA_database_PASSWD
        valueFrom:
          secretKeyRef:
            name: giteadb-pguser-giteadb
            key: password

But still everything is in crashloop because scipts and other things don't get this envs into consideration. This PR would really help with that.

Hi, this solution would really help. I'm using postgres operator that creates DB connect info as secret. To use them I have used ``` statefulset: env: - name: GITEA_database_HOST valueFrom: secretKeyRef: name: giteadb-pguser-giteadb key: host - name: GITEA_database_NAME valueFrom: secretKeyRef: name: giteadb-pguser-giteadb key: dbname - name: GITEA_database_USER valueFrom: secretKeyRef: name: giteadb-pguser-giteadb key: user - name: GITEA_database_PASSWD valueFrom: secretKeyRef: name: giteadb-pguser-giteadb key: password ``` But still everything is in crashloop because scipts and other things don't get this envs into consideration. This PR would really help with that.
justusbunsi commented 2022-03-08 16:03:50 +00:00
Member
Copy Link

Hey 👋. This PR got stuck in our heads and with the next release the Helm Chart respects specific environment variables during app.ini creation (see #298). This allows to reuse existing secrets from other applications and much more. Feel free to check it out and give us feedback.

Considering that, this PR is not necessary anymore. Right?

Hey :wave:. This PR got stuck in our heads and with the next release the Helm Chart respects specific environment variables during _app.ini_ creation (see #298). This allows to reuse existing secrets from other applications and much more. Feel free to check it out and give us feedback. Considering that, this PR is not necessary anymore. Right?
justusbunsi commented 2022-04-21 10:35:20 +00:00
Member
Copy Link

Closing with regards to this comment.

Feel free to reopen this PR if necessary.

Closing with regards to [this comment](https://gitea.com/gitea/helm-chart/pulls/279#issuecomment-693733). Feel free to reopen this PR if necessary.
justusbunsi closed this pull request 2022-04-21 10:35:20 +00:00
Some checks are pending
continuous-integration/drone/pr Build is passing
Details
check-and-test / check-and-test (pull_request)
Required

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No reviewers
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
Assignees
Clear assignees
No Assignees
4 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: gitea/helm-chart#279
No description provided.
Delete Branch "fkocik/gitea-helm-chart:feature/external-database"

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?