✨ specify nonroot uid for manager (v3+ only) #1635

harpratap · 2020-08-11T12:54:54Z

PR #983 changes the manager's user to nonroot but it doesn't work out of the box.

This change modifies the default behaviour of manager pod to use non-root uid for both kube-rbac-proxy container and the manager.

There are two issues when deploying the manager in non-root cluster if we try to use the config/default as-is

kube-rbac-proxy uses root by default instead of a nonroot image - https://github.com/brancz/kube-rbac-proxy/blob/master/Dockerfile#L1 which causes pod to request for root and fail.
Kubernetes cannot verify the nonroot user in manager because it is a string (https://github.com/kubernetes-sigs/kubebuilder/blob/master/pkg/plugin/v2/scaffolds/internal/templates/dockerfile.go#L65) resulting in container has runAsNonRoot and image has non-numeric user (nonroot), cannot verify user is non-root

k8s-ci-robot · 2020-08-11T12:55:02Z

Welcome @harpratap!

It looks like this is your first PR to kubernetes-sigs/kubebuilder 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/kubebuilder has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

k8s-ci-robot · 2020-08-11T12:55:02Z

Hi @harpratap. Thanks for your PR.

I'm waiting for a kubernetes-sigs 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.

estroz · 2020-08-11T16:54:10Z

@harpratap can you make this change to pkg/plugin/v3, and run ./generate_testdata.sh from the project root

camilamacedo86 · 2020-08-12T11:00:11Z

/hold

HI @harpratap,

+1 for @estroz commands and then:

Could you please raise an issue with the scenarios faced and ALL step performed to check them?
I am unable so far to check these issues, also if I understood your scenario, the first one is indeed tested in the e2e so I am not sure if it would be the case.

Also, after we are able to reproduce the problems faced we need to discuss if it would be the best approach as well.

harpratap · 2020-08-12T15:40:33Z

Sorry about that, I have created an issue - #1637
Once this is confirmed as an issue and we have a consensus on the correct approach to solve it I will update this PR

camilamacedo86 · 2020-09-01T10:41:33Z

pkg/plugin/v2/scaffolds/internal/templates/manager/config.go

@@ -67,6 +67,8 @@ spec:
      labels:
        control-plane: controller-manager
    spec:
+      securityContext:
+        runAsUser: 1337


Could you please do it for v3 only and as it was done for the proxy in brancz/kube-rbac-proxy#86? We need to update the image as well. So the change will be:

in /pkg/plugin/v3/scaffolds/internal/templates/manager/config.go

in v3/scaffolds/internal/templates/dockerfile.go

And then, see that you need to run make generate to update the samples.

Okay, updated the PR

camilamacedo86

It is great. could you please check the comment

camilamacedo86 · 2020-09-02T14:40:03Z

testdata/project-v3-addon/config/manager/manager.yaml

@@ -22,6 +22,8 @@ spec:
      labels:
        control-plane: controller-manager
    spec:
+      securityContext:
+        runAsUser: 65532
      containers:
      - command:


Could we add in the manifests allowPrivilegeEscalation: false?

estroz · 2020-09-11T17:54:41Z

/ok-to-test

camilamacedo86 · 2020-09-11T18:01:41Z

hi @harpratap,

It shows great. 🥇 You just need to run make generate for all samples in the testdata/ be updated properly. Also, it is good to ensure that it is rebased with the master. It needs pass in the CI for we are able to merge.

Also, in the title of the issue could you please add the mojo? See: https://github.com/kubernetes-sigs/kubebuilder/blob/master/CONTRIBUTING.md#pr-process. also, could you please add (v3+ only) in the title for we know that it is done only for v3 by checking the git history?

camilamacedo86 · 2020-09-12T00:08:13Z

/hold cancel

harpratap · 2020-09-14T15:21:39Z

/retest

camilamacedo86 · 2020-09-14T15:41:55Z

/test pull-kubebuilder-e2e-k8s-1-15-3
/test pull-kubebuilder-e2e-k8s-1-16-2

camilamacedo86

/approve

Great work 👍

k8s-ci-robot · 2020-09-14T15:43:36Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: camilamacedo86, harpratap

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [camilamacedo86]

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

estroz

/lgtm

camilamacedo86 · 2020-09-14T16:12:47Z

/test pull-kubebuilder-e2e-k8s-1-16-2

See kubernetes-sigs/kubebuilder#1635 for more info Signed-off-by: He Weiwei <hweiwei@vmware.com>

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Aug 11, 2020

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 Aug 11, 2020

k8s-ci-robot requested review from estroz and pwittrock August 11, 2020 12:55

k8s-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Aug 11, 2020

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 12, 2020

harpratap mentioned this pull request Aug 12, 2020

Deploying manager pod as non-root #1637

Closed

camilamacedo86 reviewed Sep 1, 2020

View reviewed changes

camilamacedo86 requested changes Sep 1, 2020

View reviewed changes

harpratap force-pushed the bugfix/nonroot_user_id branch from ac14fa8 to ab10f57 Compare September 2, 2020 12:28

k8s-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Sep 2, 2020

harpratap force-pushed the bugfix/nonroot_user_id branch from ab10f57 to 567993c Compare September 2, 2020 12:37

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 2, 2020

camilamacedo86 reviewed Sep 2, 2020

View reviewed changes

harpratap force-pushed the bugfix/nonroot_user_id branch from 567993c to 39544fa Compare September 11, 2020 15:23

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Sep 11, 2020

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 12, 2020

specify nonroot uid for manager

a9b4fcb

harpratap force-pushed the bugfix/nonroot_user_id branch from 39544fa to a9b4fcb Compare September 14, 2020 15:17

harpratap changed the title ~~specify nonroot uid for manager~~ ✨ specify nonroot uid for manager (v3+ only) Sep 14, 2020

camilamacedo86 approved these changes Sep 14, 2020

View reviewed changes

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 14, 2020

estroz approved these changes Sep 14, 2020

View reviewed changes

k8s-ci-robot assigned estroz Sep 14, 2020

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 14, 2020

k8s-ci-robot merged commit 9a8866f into kubernetes-sigs:master Sep 14, 2020

camilamacedo86 mentioned this pull request Jan 6, 2021

docs: Quickstart for Go-based Operators, kind=Memcached fails operator-framework/operator-sdk#4364

Closed

heww added a commit to heww/harbor-operator that referenced this pull request Jan 27, 2021

fix: specify nonroot uid for manager

f3ffc12

See kubernetes-sigs/kubebuilder#1635 for more info Signed-off-by: He Weiwei <hweiwei@vmware.com>

heww mentioned this pull request Jan 27, 2021

fix: specify nonroot uid for manager goharbor/harbor-operator#420

Merged

heww added a commit to heww/harbor-operator that referenced this pull request Jan 27, 2021

fix: specify nonroot uid for manager

be3beb7

See kubernetes-sigs/kubebuilder#1635 for more info Signed-off-by: He Weiwei <hweiwei@vmware.com>

heww added a commit to heww/harbor-operator that referenced this pull request Feb 3, 2021

fix: specify nonroot uid for manager

37a23bd

See kubernetes-sigs/kubebuilder#1635 for more info Signed-off-by: He Weiwei <hweiwei@vmware.com>

camilamacedo86 mentioned this pull request Feb 18, 2021

New Release - proposing 3.0.0 stable #2029

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

✨ specify nonroot uid for manager (v3+ only) #1635

✨ specify nonroot uid for manager (v3+ only) #1635

harpratap commented Aug 11, 2020

k8s-ci-robot commented Aug 11, 2020

k8s-ci-robot commented Aug 11, 2020

estroz commented Aug 11, 2020

camilamacedo86 commented Aug 12, 2020

harpratap commented Aug 12, 2020

camilamacedo86 Sep 1, 2020 •

edited

Loading

harpratap Sep 2, 2020

camilamacedo86 left a comment

camilamacedo86 Sep 2, 2020

harpratap Sep 11, 2020

estroz commented Sep 11, 2020

camilamacedo86 commented Sep 11, 2020 •

edited

Loading

camilamacedo86 commented Sep 12, 2020

harpratap commented Sep 14, 2020

camilamacedo86 commented Sep 14, 2020

camilamacedo86 left a comment

k8s-ci-robot commented Sep 14, 2020

estroz left a comment

camilamacedo86 commented Sep 14, 2020

✨ specify nonroot uid for manager (v3+ only) #1635

✨ specify nonroot uid for manager (v3+ only) #1635

Conversation

harpratap commented Aug 11, 2020

k8s-ci-robot commented Aug 11, 2020

k8s-ci-robot commented Aug 11, 2020

estroz commented Aug 11, 2020

camilamacedo86 commented Aug 12, 2020

harpratap commented Aug 12, 2020

camilamacedo86 Sep 1, 2020 • edited Loading

Choose a reason for hiding this comment

harpratap Sep 2, 2020

Choose a reason for hiding this comment

camilamacedo86 left a comment

Choose a reason for hiding this comment

camilamacedo86 Sep 2, 2020

Choose a reason for hiding this comment

harpratap Sep 11, 2020

Choose a reason for hiding this comment

estroz commented Sep 11, 2020

camilamacedo86 commented Sep 11, 2020 • edited Loading

camilamacedo86 commented Sep 12, 2020

harpratap commented Sep 14, 2020

camilamacedo86 commented Sep 14, 2020

camilamacedo86 left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Sep 14, 2020

estroz left a comment

Choose a reason for hiding this comment

camilamacedo86 commented Sep 14, 2020

camilamacedo86 Sep 1, 2020 •

edited

Loading

camilamacedo86 commented Sep 11, 2020 •

edited

Loading