lunny/helm-chart
1
0
Fork 1
Code Issues 36 Pull Requests 14 Actions Packages Projects Releases 93 Wiki Activity

Add support of sensitive data for external Database #279

Closed
fkocik wants to merge 3 commits from feature/external-database into main
pull from: feature/external-database
merge into: lunny:main
Conversation 10 Commits 3 Files Changed 2 +970 -6933
fkocik commented 2022-01-18 08:52:17 +00:00 (Migrated from gitea.com)
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.
lafriks commented 2022-01-18 11:49:33 +00:00 (Migrated from gitea.com)
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 (Migrated from gitea.com)
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 (Migrated from gitea.com)
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 (Migrated from gitea.com)
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 (Migrated from gitea.com)
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.
fkocik (Migrated from gitea.com) reviewed 2022-01-20 09:44:31 +00:00
templates/_helpers.tpl
@ -127,0 +84,4 @@
{{- else if .Values.mariadb.enabled -}}
{{ .Values.mariadb.primary.service.port }}
{{- else if .Values.externalDatabase.enabled -}}
{{ .Values.externalDatabase.port }}
fkocik (Migrated from gitea.com) commented 2022-01-20 09:44:31 +00:00
Copy Link

Re reading my code today : I see that I had to modify this because I encountered the issue #274.

Re reading my code today : I see that I had to modify this because I encountered the issue #274.
justusbunsi commented 2022-02-10 14:17:35 +00:00 (Migrated from gitea.com)
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 (Migrated from gitea.com)
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 (Migrated from gitea.com)
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 (Migrated from gitea.com)
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.

Pull request closed

This pull request cannot be reopened because the branch was deleted.
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
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
lunny
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: lunny/helm-chart#279
No description provided.
Delete Branch "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?