PSP admission #24600

pweil- · 2016-04-21T14:59:06Z

Update PodSecurityPolicy types and add admission controller that could enforce them

Still working on removing the non-relevant parts of the tests but I wanted to get this open to start soliciting feedback.

bring PSP up to date with any new features we've added to SCC for discussion
create admission controller that is a pared down version of SCC (no ns based strategies, no user/groups/service account permissioning)
fix tests

@liggitt @pmorie - this is the simple implementation requested that assumes all PSPs should be checked for each requests. It is a slimmed down version of our SCC admission controller

@erictune @smarterclayton

pmorie · 2016-04-21T15:00:48Z

Nice even number. Auspicious!

erictune · 2016-04-21T21:47:32Z

fyi @ericchiang

smarterclayton · 2016-04-21T22:12:02Z

I'd like to have it be possible to inject new strategies via interface so
we can slim down to one implementation and not have to keep refactoring
code. I.e. the admission controller accepts a map of strategies or similar.

On Thu, Apr 21, 2016 at 5:47 PM, Eric Tune [email protected] wrote:

fyi @ericchiang https://github.com/ericchiang

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#24600 (comment)

pweil- · 2016-04-22T12:33:25Z

I'd like to have it be possible to inject new strategies via interface so
we can slim down to one implementation and not have to keep refactoring
code. I.e. the admission controller accepts a map of strategies or similar.

I'm not following this one yet. Why would we inject the strategies that a PSP uses when it's configured on the PSP object? Or is that not what you're referring to?

smarterclayton · 2016-04-22T12:43:48Z

Inject into the admission controller custom strategies for the constants on each field - ie RunAsRange should be implemented as an interface so Openshift can have the namespace handler without core Kube code changing. Also, I want to experiment with future constants / expressions without having to refactor all of PSP. The PSP admission controller should be a skeleton that enforcement policies are injected into.

pweil- · 2016-04-22T12:49:36Z

ie RunAsRange should be implemented as an
interface

Ok, getting clearer. The field strategies are implementing a field based interface because the return values are field dependent. I think we can abstract it a bit more though. I'll give it a go.

smarterclayton · 2016-04-22T12:51:46Z

I feel like the interface selection should be opaque - ask a list of
strategies if they accept the constant value. Or just invoke a single
interface with the string, which allows an implementation of a slice of
strategies all matching.

On Apr 22, 2016, at 8:49 AM, Paul Weil [email protected] wrote:

ie RunAsRange should be implemented as an
interface

Ok, getting clearer. The field strategies are implementing a field based
interface because the return values are field dependent. I think we can
abstract it a bit more though. I'll give it a go.

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#24600 (comment)

pweil- · 2016-04-25T18:01:03Z

@smarterclayton - last commit abstracts out the creation of strategies that the provider uses by passing in a strategy factory. This can be used downstream to create strategies based on namespace settings. PTAL and let me know what you think.

erictune · 2016-04-27T05:37:15Z

API changes in 6f320 look good, but do you know why there are all the generated code changes around versioned.Watch? Doesn't seem like your non-generated code changes would cause that?

erictune · 2016-04-27T05:38:24Z

aa42e7 looks good.

erictune · 2016-04-27T05:51:20Z

pkg/security/podsecuritypolicy/provider.go

+	supplementalGroupsField = "supplementalGroups"
+)
+
+// simpleProvider is the default implementation of SecurityContextConstraintsProvider


I think this is just called Provider now?

pweil- · 2016-04-27T12:46:50Z

Doesn't seem like your non-generated code changes would cause that?

looks suspicious. I'll investigate

therc · 2016-04-27T14:28:31Z

I have had maddening issues with verify-codecgen complaining right after running update-codecgen, with spurious diffs. Maybe related to #23749 or #24473

erictune · 2016-04-27T15:03:35Z

pkg/security/podsecuritypolicy/provider.go

+// ensure we implement the interface correctly.
+var _ Provider = &simpleProvider{}
+
+// NewSimpleProvider creates a new SecurityContextConstraintsProvider instance.


This is just called "Provider" now not "SecurityContextConstraintsProvider".

erictune · 2016-05-02T15:01:44Z

plugin/pkg/admission/security/podsecuritypolicy/admission.go

+// 2.  Create the providers, includes setting pre-allocated values if necessary.
+// 3.  Try to generate and validate a PSP with providers.  If we find one then admit the pod
+//     with the validated PSP.  If we don't find any reject the pod and give all errors from the
+//     failed attempts.


How do we bootstrap a cluster if there are no PSPs initially? Does that mean all pods are rejected? Or does an admin need a hardcoded override? Or should lack of any PSP be interpreted as no PSP required. How does this work on OS and how do you see it working here?

In OpenShift we bootstrap infrastructure policies on cluster start and also provide reconciliation commands that can be used during the upgrade process. This is how our default policies and grants are created.

Here, we can either choose to add a bootstrapping process or make the assumption that if no policies exist that none will be enforced which is backwards compatible. @smarterclayton WDYT?

I went ahead and made this a configuration option for the plugin since there was no decision.

That seems fine.

pweil- · 2016-05-10T14:10:02Z

rebased

pweil- · 2016-05-11T22:07:50Z

rebased

k8s-bot · 2016-05-12T04:11:15Z

GCE e2e build/test passed for commit 65c8a1f.

k8s-github-robot · 2016-05-12T04:26:44Z

Automatic merge from submit-queue

googlebot added the cla: yes label Apr 21, 2016

k8s-github-robot assigned thockin Apr 21, 2016

thockin assigned erictune and unassigned thockin Apr 21, 2016

erictune reviewed Apr 27, 2016
View reviewed changes

pweil- mentioned this pull request May 2, 2016

why securityContext is not in ServiceAccount? #25025

Closed

erictune reviewed May 2, 2016
View reviewed changes

eparis added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 10, 2016

pweil- force-pushed the psp branch from 604e97b to e1464e8 Compare May 10, 2016 14:09

k8s-github-robot removed lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels May 10, 2016

pmorie added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 10, 2016

k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 10, 2016

pweil- force-pushed the psp branch from e1464e8 to 3cb843d Compare May 10, 2016 16:36

k8s-github-robot removed lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels May 10, 2016

pmorie added lgtm "Looks good to me", indicates that a PR is ready to be merged. cla: yes and removed cla: yes labels May 10, 2016

k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 11, 2016

pweil- added 6 commits May 11, 2016 17:27

retain read only root file system in determineEffectiveSecurityContext

04dc71f

PSP types

56193b7

PSP admission

4970f0c

PSP manual generator changes

53b1a9d

generated code

f11a4ab

default policy

65c8a1f

pweil- force-pushed the psp branch from 3cb843d to 65c8a1f Compare May 11, 2016 22:07

k8s-github-robot removed lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels May 11, 2016

pmorie added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 11, 2016

erictune added the priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. label May 11, 2016

k8s-github-robot merged commit 4e57c80 into kubernetes:master May 12, 2016

sttts mentioned this pull request May 31, 2016

Bring PodSecurityPolicy to usable state #23217

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PSP admission #24600

PSP admission #24600

pweil- commented Apr 21, 2016 •

edited by erictune

Loading

pmorie commented Apr 21, 2016

erictune commented Apr 21, 2016

smarterclayton commented Apr 21, 2016

pweil- commented Apr 22, 2016

smarterclayton commented Apr 22, 2016 via email

pweil- commented Apr 22, 2016

smarterclayton commented Apr 22, 2016

pweil- commented Apr 25, 2016

erictune commented Apr 27, 2016

erictune commented Apr 27, 2016

erictune Apr 27, 2016

pweil- commented Apr 27, 2016

therc commented Apr 27, 2016

erictune Apr 27, 2016

erictune May 2, 2016

pweil- May 2, 2016

pweil- May 6, 2016

erictune May 6, 2016

pweil- commented May 10, 2016

pweil- commented May 11, 2016

k8s-bot commented May 12, 2016

k8s-github-robot commented May 12, 2016

PSP admission #24600

PSP admission #24600

Conversation

pweil- commented Apr 21, 2016 • edited by erictune Loading

pmorie commented Apr 21, 2016

erictune commented Apr 21, 2016

smarterclayton commented Apr 21, 2016

pweil- commented Apr 22, 2016

smarterclayton commented Apr 22, 2016 via email

pweil- commented Apr 22, 2016

smarterclayton commented Apr 22, 2016

pweil- commented Apr 25, 2016

erictune commented Apr 27, 2016

erictune commented Apr 27, 2016

erictune Apr 27, 2016

Choose a reason for hiding this comment

pweil- commented Apr 27, 2016

therc commented Apr 27, 2016

erictune Apr 27, 2016

Choose a reason for hiding this comment

erictune May 2, 2016

Choose a reason for hiding this comment

pweil- May 2, 2016

Choose a reason for hiding this comment

pweil- May 6, 2016

Choose a reason for hiding this comment

erictune May 6, 2016

Choose a reason for hiding this comment

pweil- commented May 10, 2016

pweil- commented May 11, 2016

k8s-bot commented May 12, 2016

k8s-github-robot commented May 12, 2016

pweil- commented Apr 21, 2016 •

edited by erictune

Loading