Labelling this PR as size/XS

LGTM

GCE e2e test build/test passed for commit d602088db8f757a741075c4c9e2e14acdd961df8.

• Build Log
• Test Artifacts
• Internal Jenkins Results

@k8s-bot unit test this issue: #18708

ping @ArtfulCoder

@k8s-bot test this

Tests are more than 48 hours old. Re-running tests.

GCE e2e test build/test passed for commit d602088db8f757a741075c4c9e2e14acdd961df8.

• Build Log
• Test Artifacts
• Internal Jenkins Results

@k8s-bot test this

Tests are more than 48 hours old. Re-running tests.

GCE e2e test build/test passed for commit d602088db8f757a741075c4c9e2e14acdd961df8.

• Build Log
• Test Artifacts
• Internal Jenkins Results

@roberthbailey

https://github.com/kubernetes/kubernetes/blob/master/plugin/pkg/admission/securitycontext/scdeny/admission.go#L51

The admission controller does not check if the pod spec has set pod.Spec.SecurityContext.Privileged == True.

As a result, my test which explicity tests whether Privileged pod can be created, succeeds, even though the SecurityContextDeny was set on ApiServer.
https://github.com/kubernetes/kubernetes/blob/master/test/e2e/privileged.go#L129

I believe it might be a bug in the SecurityContext Admission control that we don't verify whether Privileged is set or not

@bgrant0607

Actually, there is a separate flag for allow-privileged, probably for historical reasons and hence it is not part of the SecurityContextDeny admission control.

So that should solve the mystery of why privileged pods are allowed even with SecurityContextDeny.
We have turned on --allow-privileged=true

Please also do a search for SecurityContextDeny. It appears in lots of places.

cc @pweil-

@ArtfulCoder What SecurityPolicyDeny does is described here:
https://github.com/kubernetes/kubernetes/blob/master/docs/admin/admission-controllers.md#securitycontextdeny

The comment says:

// Admit will deny any pod that defines SELinuxOptions or RunAsUser.

And we want to allow privileged pods by default, in case that wasn't clear.

The feature should be controlled by PodSecurityPolicy instead, in most cases:

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

GCE e2e build/test failed for commit d602088db8f757a741075c4c9e2e14acdd961df8.

• Build Log
• Test Artifacts
• Internal Jenkins Results

@bgrant0607
The doc you mentioned, https://github.com/kubernetes/kubernetes/blob/master/docs/admin/admission-controllers.md#securitycontextdeny has a link to
https://github.com/kubernetes/kubernetes/blob/master/docs/user-guide/security-context.md#security-contexts which says it is for (uid, gid, capabilities, SELinux role, etc..)

Note the use of "etc" in the document :))

It is super confusing to have Admission Control (securitycontextdeny), PodSecurityPolicy and on top, another flag --allow-privileged, with seemingly overlapping responsibilities.

@ArtfulCoder I agree. I don't think we need SecurityContextDeny at this point. I was suggesting removing it more broadly from the defaults: from hyperkube, vagrant, kubemark, centos, ubuntu, HA config, etc.

Openshift uses SecurityContextConstraint instead:
https://github.com/openshift/origin/blob/master/pkg/cmd/server/kubernetes/master_config.go#L48
https://github.com/openshift/origin/blob/master/pkg/security/admission/admission.go#L97

cc @derekwaynecarr @pweil- @erictune

+1 to removing this. It was initially implemented to prevent usage of the features but now that they've had plenty of burn in time I don't think it's serving a purpose.

In the future a PSP admission controller will serve this purpose.

Also it would close: #16456

SGTM.

On Tue, Feb 23, 2016 at 11:45 AM, Paul Weil notifications@github.com
wrote:

+1 to removing this. It was initially implemented to prevent usage of the
features but now that they've had plenty of burn in time I don't think it's
serving a purpose.

In the future a PSP admission controller will serve this purpose.

Also it would close: #16456
#16456

—
Reply to this email directly or view it on GitHub
#21004 (comment)
.

@k8s-bot test this: issue #21463

+1

GCE e2e build/test failed for commit d602088db8f757a741075c4c9e2e14acdd961df8.

• Build Log
• Test Artifacts
• Internal Jenkins Results

@k8s-bot ok to test
@k8s-bot test this

pr builder appears to be missing, activating due to 'lgtm' label.

Rebased to try and avoid flakes.

@k8s-bot test this issue #21475

PR changed after LGTM, removing LGTM.

GCE e2e test build/test passed for commit a238d90.

• Build Log
• Test Artifacts
• Internal Jenkins Results