[stable/minio] Add support for securityContext runAsUser and runAsGroup #17393

simonwgill · 2019-09-24T11:03:37Z

What this PR does / why we need it:

When the podSecurityContext does not allow runAsRoot, there was no method to configure minio to function. This PR allows consumers to use minio in this situation.

Is this a new chart?

No

Which issue this PR fixes

Special notes for your reviewer:

The configPath and configPathmc variables have been changed to use a default that doesn't rely on the active user being root.

Checklist

[Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.]

DCO signed
Chart Version bumped
Variables are documented in the README.md
Title of the PR starts with chart name (e.g. [stable/chart])

k8s-ci-robot · 2019-09-24T11:03:39Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: simonwgill
To complete the pull request process, please assign nitisht
You can assign the PR to them by writing /assign @nitisht in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

stable/minio/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

k8s-ci-robot · 2019-09-24T11:03:52Z

Hi @simonwgill. Thanks for your PR.

I'm waiting for a helm member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

simonwgill · 2019-09-25T10:34:05Z

/assign @nitisht for approval

k8s-ci-robot · 2019-09-25T10:34:07Z

@simonwgill: GitHub didn't allow me to assign the following users: for, approval.

Note that only helm members, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time.
For more information please see the contributor guide

In response to this:

/assign @nitisht for approval

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

simonwgill · 2019-10-07T08:04:10Z

Should I be keeping track of the head branch, or should I wait for some level of approval of the branch as it was?

nitisht · 2019-10-07T08:09:43Z

Please fix the conflicts @simonwgill

Only added User and Group Signed-off-by: Simon W. Gill <simon.gill@citizensadvice.org.uk>

Signed-off-by: Simon W. Gill <simon.gill@citizensadvice.org.uk>

zanhsieh · 2019-10-07T08:21:25Z

/ok-to-test

nitisht · 2019-10-07T10:12:27Z

can you confirm if you have deployed this locally and it works @simonwgill

simonwgill · 2019-10-07T12:23:14Z

@nitisht I had to go back and double check, but yes, it's happily running locally on my setup.

nitisht · 2019-10-07T12:31:10Z

Thank you for checking again! :)

nitisht

Few recommendations..

stable/minio/README.md

nitisht · 2019-10-07T12:36:44Z

stable/minio/README.md

@@ -108,8 +108,8 @@ The following table lists the configurable parameters of the MinIO chart and the
 | `existingSecret`                          | Name of existing secret with access and secret key.                                                                                     | `""`                                       |
 | `accessKey`                               | Default access key (5 to 20 characters)                                                                                                 | `AKIAIOSFODNN7EXAMPLE`                     |
 | `secretKey`                               | Default secret key (8 to 40 characters)                                                                                                 | `wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY` |
-| `configPath`                              | Default config file location                                                                                                            | `~/.minio`                                 |
-| `configPathmc`                            | Default config file location for MinIO client - mc                                                                                      | `~/.mc`                                    |
+| `configPath`                              | Default config file location                                                                                                            | `/etc/minio`                                 |


--config-dir has been deprecated from MinIO server as of minio/minio#7033, can you please remove this option completely?

I've removed the option. Haven't implemented the certs-dir functionality at this point though.

Are you expecting --certs-dir to be implemented under this ticket? That seems out of scope for what I was updating and I'm not sure how to set up any tests to handle it. Doesn't look like anyone raised the deprecation as an issue either.

nitisht · 2019-10-07T12:37:22Z

stable/minio/values.yaml

@@ -48,11 +48,18 @@ priorityClassName: ""
 existingSecret: ""
 accessKey: "AKIAIOSFODNN7EXAMPLE"
 secretKey: "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY"
-configPath: "/root/.minio/"
-configPathmc: "/root/.mc/"
+configPath: "/etc/minio/"


--config-dir has been deprecated from MinIO server as of minio/minio#7033, can you please remove this option completely?

Signed-off-by: Simon W. Gill <simon.gill@citizensadvice.org.uk>

…on by minio. Signed-off-by: Simon W. Gill <simon.gill@citizensadvice.org.uk> fix(stable/minio): Removed configDir references

simonwgill · 2019-10-14T07:50:26Z

Does the --cert-dir option need support adding in this PR? That seems out of scope for the original issue.

dmccaffery · 2019-11-04T00:45:42Z

Just ran into this myself -- what do we need to do to get the ball rolling @nitisht / @simonwgill ? -- looks like there are a couple of conflicts -- is anything else required?

simonwgill · 2019-11-04T15:37:41Z

I wasn't sure if adding --cert-dir was in scope for this PR, and I've been waiting on an answer to that question from someone.

Haven't looked at what's been integrated since I last rebased though.

stale · 2019-12-04T16:28:16Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Any further update will cause the issue/pull request to no longer be considered stale. Thank you for your contributions.

stale · 2019-12-18T17:04:00Z

This issue is being automatically closed due to inactivity.

helm-bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Sep 24, 2019

k8s-ci-robot requested review from krisis and wlan0 September 24, 2019 11:03

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Sep 24, 2019

simonwgill force-pushed the simonwgill-patch-1 branch from ddbc804 to ebef169 Compare September 24, 2019 11:06

helm-bot added the Contribution Allowed If the contributor has signed the DCO or the CNCF CLA (prior to the move to a DCO). label Sep 24, 2019

k8s-ci-robot assigned nitisht Sep 25, 2019

simonwgill and others added 3 commits October 7, 2019 09:14

feat(mino): Added securityContext to Deployment

88fb870

Only added User and Group Signed-off-by: Simon W. Gill <simon.gill@citizensadvice.org.uk>

fix(minio): Altered to deploy correctly without root as sub chart

eb17e3c

Signed-off-by: Simon W. Gill <simon.gill@citizensadvice.org.uk>

doc(minio): Bumped chart version and updated variables in README.md

ab0c6d2

Signed-off-by: Simon W. Gill <simon.gill@citizensadvice.org.uk>

simonwgill force-pushed the simonwgill-patch-1 branch from ebef169 to ab0c6d2 Compare October 7, 2019 08:15

k8s-ci-robot added ok-to-test and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Oct 7, 2019

nitisht suggested changes Oct 7, 2019

View reviewed changes

helm-bot removed the Contribution Allowed If the contributor has signed the DCO or the CNCF CLA (prior to the move to a DCO). label Oct 7, 2019

simonwgill added 2 commits October 7, 2019 14:49

typo(stable/minio): Fixed duplicate variable name after copy-paste

5ef08bc

Signed-off-by: Simon W. Gill <simon.gill@citizensadvice.org.uk>

chore(stable/minio): Removed support for --config-dir after deprecati…

3b212dd

…on by minio. Signed-off-by: Simon W. Gill <simon.gill@citizensadvice.org.uk> fix(stable/minio): Removed configDir references

simonwgill force-pushed the simonwgill-patch-1 branch from 5448855 to 3b212dd Compare October 7, 2019 13:50

helm-bot added the Contribution Allowed If the contributor has signed the DCO or the CNCF CLA (prior to the move to a DCO). label Oct 7, 2019

stale bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Dec 4, 2019

stale bot closed this Dec 18, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[stable/minio] Add support for securityContext runAsUser and runAsGroup #17393

[stable/minio] Add support for securityContext runAsUser and runAsGroup #17393

simonwgill commented Sep 24, 2019

k8s-ci-robot commented Sep 24, 2019

k8s-ci-robot commented Sep 24, 2019

simonwgill commented Sep 25, 2019

k8s-ci-robot commented Sep 25, 2019

simonwgill commented Oct 7, 2019

nitisht commented Oct 7, 2019

zanhsieh commented Oct 7, 2019

nitisht commented Oct 7, 2019

simonwgill commented Oct 7, 2019

nitisht commented Oct 7, 2019

nitisht left a comment

nitisht Oct 7, 2019

simonwgill Oct 7, 2019

nitisht Oct 7, 2019

simonwgill commented Oct 14, 2019

dmccaffery commented Nov 4, 2019

simonwgill commented Nov 4, 2019

stale bot commented Dec 4, 2019

stale bot commented Dec 18, 2019

[stable/minio] Add support for securityContext runAsUser and runAsGroup #17393

[stable/minio] Add support for securityContext runAsUser and runAsGroup #17393

Conversation

simonwgill commented Sep 24, 2019

What this PR does / why we need it:

Is this a new chart?

Which issue this PR fixes

Special notes for your reviewer:

Checklist

k8s-ci-robot commented Sep 24, 2019

k8s-ci-robot commented Sep 24, 2019

simonwgill commented Sep 25, 2019

k8s-ci-robot commented Sep 25, 2019

simonwgill commented Oct 7, 2019

nitisht commented Oct 7, 2019

zanhsieh commented Oct 7, 2019

nitisht commented Oct 7, 2019

simonwgill commented Oct 7, 2019

nitisht commented Oct 7, 2019

nitisht left a comment

Choose a reason for hiding this comment

nitisht Oct 7, 2019

Choose a reason for hiding this comment

simonwgill Oct 7, 2019

Choose a reason for hiding this comment

nitisht Oct 7, 2019

Choose a reason for hiding this comment

simonwgill commented Oct 14, 2019

dmccaffery commented Nov 4, 2019

simonwgill commented Nov 4, 2019

stale bot commented Dec 4, 2019

stale bot commented Dec 18, 2019