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

Add gpg configuration settings #343

Merged
justusbunsi merged 12 commits from refs/pull/343/head into main 2022-09-28 08:19:00 +00:00
Conversation 8 Commits 12 Files Changed 18 +429 -22
justusbunsi commented 2022-08-04 15:41:30 +00:00 (Migrated from gitea.com)
Copy Link

Description of the change

This PR adds support for gpg key setup. It allows to pass the gpg private key content inline inside values.yaml or refer to an existing secret containing the key content data.

Benefits

Administrators don't need to manually setup the gpg environment from inside a running container. It also eliminates the breaking change of Gitea 1.17 regarding [git].HOME as the GNUPGHOME environment variable is used consistently to relocate the .gnupg directory to its former location.

Applicable issues

Additional information

This PR add the first unit tests to this Helm Chart, ensuring templating integrity for signing related configuration.

Checklist

### Description of the change This PR adds support for gpg key setup. It allows to pass the gpg private key content inline inside `values.yaml` or refer to an existing secret containing the key content data. ### Benefits Administrators don't need to manually setup the gpg environment from inside a running container. It also eliminates the breaking change of Gitea 1.17 regarding `[git].HOME` as the `GNUPGHOME` environment variable is used consistently to relocate the `.gnupg` directory to its former location. ### Applicable issues - fixes #107 ### Additional information This PR add the first unit tests to this Helm Chart, ensuring templating integrity for signing related configuration. ### Checklist - [x] Parameters are documented in the `values.yaml` and added to the `README.md` using [readme-generator-for-helm](https://github.com/bitnami-labs/readme-generator-for-helm)
👍 1 ❤️ 1
lafriks (Migrated from gitea.com) reviewed 2022-08-07 19:34:14 +00:00
README.md
@ -695,3 +512,1 @@
When using the rootless image the gpg key folder is not persistent by default.
If you consider using signed commits for internal Gitea activities (e.g. initial commit), you'd need to provide a signing key.
Prior to [PR186](https://gitea.com/gitea/helm-chart/pulls/186), imported keys had to be re-imported once the container got replaced by another.
When using the rootless image the gpg key folder is not persistent by
lafriks (Migrated from gitea.com) commented 2022-08-07 19:34:14 +00:00
Copy Link
- When using the rootless image the gpg key folder was is not persistent by
+ When using the rootless image the gpg key folder is not persistent by
```diff - When using the rootless image the gpg key folder was is not persistent by + When using the rootless image the gpg key folder is not persistent by ```
justusbunsi commented 2022-08-15 10:01:49 +00:00 (Migrated from gitea.com)
Copy Link

@lafriks Suggested changes applied and conflicts resolved.

@lafriks Suggested changes applied and conflicts resolved.
takirala commented 2022-08-16 16:07:54 +00:00 (Migrated from gitea.com)
Copy Link

LGTM!

LGTM!
❤️ 1
luhahn (Migrated from gitea.com) reviewed 2022-08-19 06:32:16 +00:00
templates/gitea/init.yaml
@ -46,1 +45,3 @@
chown -v 1000:1000 "${GNUPGHOME}"
mkdir -p "${GNUPGHOME}"
chmod 700 "${GNUPGHOME}"
chown 1000:1000 "${GNUPGHOME}"
luhahn (Migrated from gitea.com) commented 2022-08-19 06:32:16 +00:00
Copy Link

Shouldn't we get this from runAsUser from ctonainerSecurityContext, if set?

Shouldn't we get this from runAsUser from ctonainerSecurityContext, if set?
luhahn (Migrated from gitea.com) commented 2022-08-21 12:15:49 +00:00
Copy Link

That's true. Unfortunately, the whole init directory script is currently locked to 1000:1000. I suggest handling it with #338 in another PR, if that's OK for you.

That's true. Unfortunately, the whole init directory script is currently locked to 1000:1000. I suggest handling it with #338 in another PR, if that's OK for you.
luhahn (Migrated from gitea.com) commented 2022-08-22 09:38:08 +00:00
Copy Link

sure :)

sure :)
luhahn (Migrated from gitea.com) approved these changes 2022-08-22 09:38:26 +00:00
luhahn (Migrated from gitea.com) left a comment
Copy Link

LGTM

LGTM
techknowlogick (Migrated from gitea.com) approved these changes 2022-08-22 20:19:27 +00:00
pat-s commented 2022-09-25 19:59:51 +00:00 (Migrated from gitea.com)
Copy Link

@justusbunsi I've briefly skimmed over this PR. It's large and I assume you've tested it locally besides the new unit tests?

Is there anything left preventing this from getting merged? I guess you got a bit under pressure with the 1.17 release and all the discussions about it around the helm chart. Maybe right now is a good moment to merge this one then?

@justusbunsi I've briefly skimmed over this PR. It's large and I assume you've tested it locally besides the new unit tests? Is there anything left preventing this from getting merged? I guess you got a bit under pressure with the 1.17 release and all the discussions about it around the helm chart. Maybe right now is a good moment to merge this one then?
justusbunsi commented 2022-09-25 20:45:49 +00:00 (Migrated from gitea.com)
Copy Link

@pat-s

Is there anything left preventing this from getting merged?

Yep, failing unit tests after the last PR merge into main. ?

Need to fix them first.

@pat-s > Is there anything left preventing this from getting merged? Yep, failing unit tests after the last PR merge into main. ? Need to fix them first.
justusbunsi commented 2022-09-27 14:15:58 +00:00 (Migrated from gitea.com)
Copy Link

@pat-s I've fixed the unit tests and this should be ready getting merged. As I am the pull request initiator I don't want to merge myself. Or is this not important to us?

I assume you've tested it locally besides the new unit tests?

Yes.

@pat-s I've fixed the unit tests and this should be ready getting merged. As I am the pull request initiator I don't want to merge myself. Or is this not important to us? > I assume you've tested it locally besides the new unit tests? Yes.
pat-s commented 2022-09-28 08:18:44 +00:00 (Migrated from gitea.com)
Copy Link

As I am the pull request initiator I don't want to merge myself. Or is this not important to us?

Good Q, I guess one could argue in both ways. Given that there were already two approvals and you didn't do any major refactoring afterwards, it should be ok to self-merge?

> As I am the pull request initiator I don't want to merge myself. Or is this not important to us? Good Q, I guess one could argue in both ways. Given that there were already two approvals and you didn't do any major refactoring afterwards, it should be ok to self-merge?
viceice commented 2022-10-18 08:26:16 +00:00 (Migrated from gitea.com)
Copy Link

This broke me while update from v6.0.1 to v6.0.2 ?

This should have been a major bump or at least a minor.

execution error at (gitea/templates/gitea/gpg-secret.yaml:3:6): Either specify `signing.privateKey` or `signing.existingKey`
This broke me while update from v6.0.1 to v6.0.2 ? This should have been a major bump or at least a minor. ``` execution error at (gitea/templates/gitea/gpg-secret.yaml:3:6): Either specify `signing.privateKey` or `signing.existingKey` ```
👀 1
pat-s commented 2022-10-18 13:32:13 +00:00 (Migrated from gitea.com)
Copy Link

This should have been a major bump or at least a minor.

Yes, you are right, we already discussed this in Discord. Sorry for the troubles!

You should be able to just roll back and only bump the gitea image tag.

> This should have been a major bump or at least a minor. Yes, you are right, we already discussed this in Discord. Sorry for the troubles! You should be able to just roll back and only bump the gitea image tag.
Sign in to join this conversation.
Reviewers
No Reviewers
luhahn
techknowlogick
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
kind/enhancement
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: lunny/helm-chart#343
No description provided.
Delete Branch "refs/pull/343/head"

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?