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

Allow setting serviceAccountName #357

Closed
eugene-davis wants to merge 5 commits from main into main
pull from: main
merge into: lunny:main
Conversation 7 Commits 5 Files Changed 3 +6
eugene-davis commented 2022-09-08 21:59:55 +00:00 (Migrated from gitea.com)
Copy Link

Description of the change

Allows specifying the Service Account used with Gitea pods

Benefits

Allows tighter control over the service accounts used.

Possible drawbacks

N/A, is an optional setting.

Checklist

  • Parameters are documented in the values.yaml and added to the README.md using readme-generator-for-helm
  • Breaking changes are documented in the README.md
### Description of the change Allows specifying the Service Account used with Gitea pods ### Benefits Allows tighter control over the service accounts used. ### Possible drawbacks N/A, is an optional setting. ### Checklist <!-- [Place an '[X]' (no spaces) in all applicable fields. Please remove unrelated fields.] --> - [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) - [X] Breaking changes are documented in the `README.md`
pat-s (Migrated from gitea.com) reviewed 2022-09-14 08:51:41 +00:00
pat-s (Migrated from gitea.com) left a comment
Copy Link

Thanks! Did you testdrive this on an installation of yours?

Thanks! Did you testdrive this on an installation of yours?
values.yaml
@ -221,3 +189,2 @@
## @param dnsConfig dnsConfig for the deployment
## @param dnsConfig dnsConfig for the statefulset
dnsConfig: {}
pat-s (Migrated from gitea.com) commented 2022-09-14 08:51:41 +00:00
Copy Link

Suggestion: Name of service account to use for the deployments

Suggestion: Name of service account to use for the deployments
pat-s (Migrated from gitea.com) commented 2022-09-14 12:25:19 +00:00
Copy Link

Thanks, updated it

Thanks, updated it
eugene-davis commented 2022-09-14 12:24:27 +00:00 (Migrated from gitea.com)
Copy Link

Thanks! Did you testdrive this on an installation of yours?

Yes, this has been running for around a week and a half, with the specified SA being used to authenticate against HCP Vault.

> Thanks! Did you testdrive this on an installation of yours? Yes, this has been running for around a week and a half, with the specified SA being used to authenticate against HCP Vault.
justusbunsi (Migrated from gitea.com) reviewed 2022-09-25 12:26:38 +00:00
values.yaml
@ -233,1 +195,4 @@
## @param statefulset.annotations Annotations for the Gitea StatefulSet to be created
## @param statefulset.serviceAccountName Name of service account to use for the deployments
statefulset:
env:
justusbunsi (Migrated from gitea.com) commented 2022-09-25 12:26:38 +00:00
Copy Link

@eugene-davis Thank you for contributing to this helm chart. While this is a simple solution for using another service account name than "default", defining this setting there would result in a breaking change when adding the chart feature of creating its own service account. I suggest introducing a new object in the security section of values.yaml (https://gitea.com/gitea/helm-chart#security):

serviceAccount:
  name: ""

This object would be extendable by the create flag and other related settings.

@eugene-davis Thank you for contributing to this helm chart. While this is a simple solution for using another service account name than "default", defining this setting there would result in a breaking change when adding the chart feature of creating its own service account. I suggest introducing a new object in the security section of values.yaml (https://gitea.com/gitea/helm-chart#security): ```yaml serviceAccount: name: "" ``` This object would be extendable by the `create` flag and other related settings.
pat-s (Migrated from gitea.com) reviewed 2023-03-23 22:02:38 +00:00
values.yaml
@ -233,1 +195,4 @@
## @param statefulset.annotations Annotations for the Gitea StatefulSet to be created
## @param statefulset.serviceAccountName Name of service account to use for the deployments
statefulset:
env:
pat-s (Migrated from gitea.com) commented 2023-03-23 22:02:38 +00:00
Copy Link

I also like this approach. It leaves the "default" one untouched and doesn't result in a breaking change - and one can be more granular with the permissions as they don't need to cover the capabilities of the "default" one but only the ones for it's desired scope.

OTOH one could also argue that users should provision additional SA outside of Gitea (and it's helm chart) if additional ones are needed.

@eugene-davis @justusbunsi Is anyone of you motivated to continue here or shall we close?

I also like this approach. It leaves the "default" one untouched and doesn't result in a breaking change - and one can be more granular with the permissions as they don't need to cover the capabilities of the "default" one but only the ones for it's desired scope. OTOH one could also argue that users should provision additional SA outside of Gitea (and it's helm chart) if additional ones are needed. @eugene-davis @justusbunsi Is anyone of you motivated to continue here or shall we close?
justusbunsi commented 2023-04-01 11:28:04 +00:00 (Migrated from gitea.com)
Copy Link

Is anyone of you motivated to continue here or shall we close?

I can proceed if @eugene-davis does not respond within a week.

> Is anyone of you motivated to continue here or shall we close? I can proceed if @eugene-davis does not respond within a week.
👍 1
pat-s commented 2023-04-18 20:54:57 +00:00 (Migrated from gitea.com)
Copy Link

@justusbunsi If you find some time, you could go ahead here :)

@justusbunsi If you find some time, you could go ahead here :)
👍 1
tobiasbp commented 2023-05-24 12:45:48 +00:00 (Migrated from gitea.com)
Copy Link

I'd really like to see this feature.

I'd really like to see this feature.
pat-s commented 2023-05-24 14:48:49 +00:00 (Migrated from gitea.com)
Copy Link

@tobiasbp You can also go ahead and create a new PR to move forward. This one seems a bit stale atm but I am always happy to review any new PR that comes in!

@tobiasbp You can also go ahead and create a new PR to move forward. This one seems a bit stale atm but I am always happy to review any new PR that comes in!
justusbunsi commented 2023-05-24 15:13:38 +00:00 (Migrated from gitea.com)
Copy Link

@tobiasbp You can also go ahead and create a new PR to move forward. This one seems a bit stale atm but I am always happy to review any new PR that comes in!

Absolutely. I hadn't had the time to adapt the changes. If you like, go ahead. 👍

>@tobiasbp You can also go ahead and create a new PR to move forward. This one seems a bit stale atm but I am always happy to review any new PR that comes in! Absolutely. I hadn't had the time to adapt the changes. If you like, go ahead. 👍
justusbunsi commented 2023-05-30 20:36:03 +00:00 (Migrated from gitea.com)
Copy Link

Superseded by #451.

Superseded by #451.

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
kind/feature
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#357
No description provided.
Delete Branch "main"

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?