Add "MayRunAs" value among other GroupStrategies by stlaz · Pull Request #65135 · kubernetes/kubernetes

stlaz · 2018-06-15T11:31:53Z

What this PR does / why we need it:
Adds "MayRunAs" value among other group strategies. This strategy
allows to define a certain range of GIDs for FSGroupStrategy and
SupplementalGroupStrategy in a PSP.

This new strategy works similarly to the "MustRunAs" one, except that
when no GID is specified in a pod/container security context then no
GID is generated for the respective containers.

Which issue(s) this PR fixes
Resolves #56173

Release note:

PodSecurityPolicy objects now support a `MayRunAs` rule for `fsGroup` and `supplementalGroups` options. This allows specifying ranges of allowed GIDs for pods/containers without forcing a default GID the way `MustRunAs` does. This means that a container to which such a policy applies to won't use any fsGroup/supplementalGroup GID if not explicitly specified, yet a specified GID must still fall in the GID range according to the policy.

php-coder · 2018-06-15T12:30:26Z

/sig auth
/ok-to-test

PTAL @kubernetes/sig-auth-api-reviews

CC @simo5

php-coder · 2018-06-15T12:37:03Z

pkg/security/podsecuritypolicy/group/mayrunas.go

Please, update the comment.

php-coder · 2018-06-15T12:37:26Z

pkg/security/podsecuritypolicy/group/mayrunas.go

s/MustRunAs/MayRunAs/

php-coder · 2018-06-15T12:41:12Z

pkg/apis/policy/types.go

s/to run as a particular gid/to run with particular gids/ ?

It was but a mere copy-paste but "with" sounds definitely better

php-coder · 2018-06-15T12:41:57Z

pkg/apis/policy/types.go

s/when it is applied, it has to/when they are specified, they have to/ ?

php-coder · 2018-06-15T12:43:41Z

@stlaz One more place to update:

kubernetes/pkg/apis/policy/fuzzer/fuzzer.go

Lines 49 to 58 in 3abba25

    
           supplementalGroupsRules := []policy.SupplementalGroupsStrategyType{ 
        
           	policy.SupplementalGroupsStrategyRunAsAny, 
        
           	policy.SupplementalGroupsStrategyMustRunAs, 
        
           } 
        
           psp.SupplementalGroups.Rule = supplementalGroupsRules[c.Rand.Intn(len(supplementalGroupsRules))] 
        
           fsGroupRules := []policy.FSGroupStrategyType{ 
        
           	policy.FSGroupStrategyMustRunAs, 
        
           	policy.FSGroupStrategyRunAsAny, 
        
           }

php-coder · 2018-06-15T12:44:44Z

@kubernetes/sig-auth-api-reviews Do we need to update PSP in extensions API group?

php-coder · 2018-06-15T12:51:05Z

/assign @tallclair

stlaz · 2018-06-15T14:17:58Z

Updated the patch to fix verify-basel failure and to address the PR comments.

stlaz · 2018-06-18T12:40:34Z

This change introduces backward incompatibility since the "MayRunAs" value to the groups is unknown to previous API versions. If we convert this to "MustRunAs" internally, we wouldn't be able to later get the information that it's actually "MayRunAs".
What would be the preferred way to handle such an issue? Is the rollback desirable here? Does @tallclair have an input on this?

krmayankk · 2018-06-20T06:53:17Z

@tallclair @php-coder i believe the MayRunAs would also be needed by RunAsGroup field. I will include that in the PSP changes for RunAsGroup in my next PR.

php-coder · 2018-06-26T15:47:04Z

PTAL @tallclair

tallclair

This change introduces backward incompatibility since the "MayRunAs" value to the groups is unknown to previous API versions.

Backwards incompatibility means that a resource that was valid in a previous version is no longer valid (or behaves differently), which isn't the case here. What you're refering to is forwards compatibility, and that's not something Kubernetes provides, so this is a non-issue.

tallclair · 2018-07-04T00:23:03Z

pkg/security/podsecuritypolicy/group/mayrunas.go

dedupe this code with mustRunAs.Validate (extract a shared helper)

tallclair · 2018-07-04T00:27:11Z

pkg/security/podsecuritypolicy/group/mayrunas_test.go

This method is constant, don't bother testing it here. Instead, add a couple cases under https://github.com/kubernetes/kubernetes/blob/master/pkg/security/podsecuritypolicy/provider_test.go to ensure that the nil return works as expected.

stlaz · 2018-09-06T12:56:21Z

/retest

stlaz · 2018-09-06T12:57:33Z

The "fall in all groups" issue should now be resolved, a positive multi-ranges test was added on top of the mayrunas tests.

liggitt · 2018-09-06T20:03:25Z

thanks. both tallclair and I are out of the office at the moment, but will pick this up when master reopens for 1.13.

stlaz · 2018-09-07T06:49:53Z

Thank you. I'm not sure exactly why the verify test does not pass since make verify seems to be running fine on my system. The only guess I have was my upgrade of golang from 1.10 to 1.11 🤔

stlaz · 2018-09-14T07:04:30Z

/retest

pkg/security/podsecuritypolicy/group/mayrunas_test.go

stlaz · 2018-09-25T08:21:40Z

@tallclair Test cases for multiple groups added.

tallclair

A couple nits, but LGTM. Please squash.

tallclair · 2018-09-26T21:15:39Z

pkg/security/podsecuritypolicy/group/helpers.go

nit: inline

tallclair · 2018-09-26T21:15:46Z

pkg/security/podsecuritypolicy/group/helpers.go

Adds "MayRunAs" value among other group strategies. This strategy allows to define a certain range of GIDs for FSGroupStrategy and SupplementalGroupStrategy in a PSP. This new strategy works similarly to the "MustRunAs" one, except that when no GID is specified in a pod/container security context then no GID is generated for the respective containers. Resolves kubernetes#56173

stlaz · 2018-09-27T10:49:19Z

Comments addressed + squashed.

tallclair · 2018-09-27T21:13:32Z

/lgtm
/hold cancel

liggitt · 2018-09-30T01:21:06Z

/approve

k8s-ci-robot · 2018-09-30T01:21:14Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: liggitt, stlaz, tallclair

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~pkg/apis/policy/OWNERS~~ [liggitt]
~~pkg/security/podsecuritypolicy/OWNERS~~ [liggitt,tallclair]

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

yue9944882 · 2018-10-11T22:00:47Z

hi folks, why not syncing these new internal definitions to the external repo? i'm afraid currently we can't specify the new strategy w/ client-go. also the "MayRunAs" will be an undefined value when deserialized into external types.

ref: #69704

k8s-ci-robot requested review from enisoc and ericchiang June 15, 2018 11:31

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 Jun 15, 2018

k8s-github-robot added the kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API label Jun 15, 2018

k8s-ci-robot added sig/auth Categorizes an issue or PR as relevant to SIG Auth. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jun 15, 2018

php-coder reviewed Jun 15, 2018

View reviewed changes

pkg/security/podsecuritypolicy/group/mayrunas.go Outdated

Copy link
Copy Markdown

Contributor

php-coder Jun 15, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, update the comment.

php-coder reviewed Jun 15, 2018

View reviewed changes

pkg/security/podsecuritypolicy/group/mayrunas.go Outdated

Copy link
Copy Markdown

Contributor

php-coder Jun 15, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/MustRunAs/MayRunAs/

php-coder reviewed Jun 15, 2018

View reviewed changes

k8s-ci-robot assigned tallclair Jun 15, 2018

php-coder mentioned this pull request Jun 15, 2018

[PodSecurityPolicy] "MayRunAs" strategies #56173

Closed

stlaz force-pushed the psp_group_mayrunas branch 2 times, most recently from 3d44643 to 5e430d8 Compare June 15, 2018 14:16

stlaz changed the title ~~[WIP] Add "MayRunAs" value among other GroupStrategies~~ Add "MayRunAs" value among other GroupStrategies Jun 18, 2018

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 18, 2018

tallclair reviewed Jul 4, 2018

View reviewed changes

stlaz force-pushed the psp_group_mayrunas branch from 5e430d8 to 698d161 Compare July 17, 2018 13:12

k8s-ci-robot added sig/architecture Categorizes an issue or PR as relevant to SIG Architecture. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. labels Sep 6, 2018

stlaz force-pushed the psp_group_mayrunas branch 2 times, most recently from fb98885 to 5cb67ba Compare September 6, 2018 11:42

stlaz force-pushed the psp_group_mayrunas branch from 5cb67ba to c946886 Compare September 6, 2018 14:51

k8s-ci-robot added the sig/network Categorizes an issue or PR as relevant to SIG Network. label Sep 6, 2018

stlaz force-pushed the psp_group_mayrunas branch from c946886 to b738d40 Compare September 13, 2018 10:48

tallclair reviewed Sep 20, 2018

View reviewed changes

pkg/security/podsecuritypolicy/group/mayrunas_test.go Outdated Show resolved Hide resolved

k8s-ci-robot removed the status/approved-for-milestone label Sep 21, 2018

stlaz force-pushed the psp_group_mayrunas branch from b738d40 to 4ae930c Compare September 25, 2018 08:21

tallclair reviewed Sep 26, 2018

View reviewed changes

tallclair mentioned this pull request Sep 26, 2018

Implement RunAsGroup Strategy in PSP #67802

Merged

stlaz force-pushed the psp_group_mayrunas branch from 4ae930c to a577b50 Compare September 27, 2018 10:48

k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. and removed do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Sep 27, 2018

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

k8s-ci-robot merged commit 0b3a5cd into kubernetes:master Sep 30, 2018

stlaz mentioned this pull request Apr 1, 2022

REQUEST: New membership for stlaz kubernetes/org#3352

Closed

9 tasks

Conversation

stlaz commented Jun 15, 2018 • edited by liggitt Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

php-coder commented Jun 15, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

php-coder commented Jun 15, 2018

Uh oh!

php-coder commented Jun 15, 2018

Uh oh!

php-coder commented Jun 15, 2018

Uh oh!

stlaz commented Jun 15, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stlaz commented Jun 18, 2018

Uh oh!

krmayankk commented Jun 20, 2018

Uh oh!

php-coder commented Jun 26, 2018

Uh oh!

tallclair left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stlaz commented Sep 6, 2018

Uh oh!

stlaz commented Sep 6, 2018

Uh oh!

liggitt commented Sep 6, 2018

Uh oh!

stlaz commented Sep 7, 2018

Uh oh!

stlaz commented Sep 14, 2018

Uh oh!

Uh oh!

stlaz commented Sep 25, 2018

Uh oh!

tallclair left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stlaz commented Sep 27, 2018

Uh oh!

tallclair commented Sep 27, 2018

Uh oh!

liggitt commented Sep 30, 2018

Uh oh!

k8s-ci-robot commented Sep 30, 2018

Uh oh!

yue9944882 commented Oct 11, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

stlaz commented Jun 15, 2018 •

edited by liggitt

Loading

stlaz commented Jun 15, 2018 •

edited

Loading

yue9944882 commented Oct 11, 2018 •

edited

Loading