New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Externalize PSP admission controller #69685

Merged
merged 1 commit into from Oct 23, 2018

Conversation

@yue9944882
Contributor

yue9944882 commented Oct 11, 2018

continue with previous work by @xmudrii #67846.
sadly he's recently busy to keep track of this thread.

/kind cleanup
/sig api-machinery

Release note:

NONE
@jennybuckley

This comment has been minimized.

Show comment
Hide comment
@jennybuckley
Contributor

jennybuckley commented Oct 11, 2018

/assign @caesarxuchao

@yue9944882

This comment has been minimized.

Show comment
Hide comment
@yue9944882
Contributor

yue9944882 commented Oct 16, 2018

@k8s-ci-robot k8s-ci-robot requested review from liggitt, php-coder and tallclair Oct 16, 2018

@liggitt

This comment has been minimized.

Show comment
Hide comment
@liggitt

liggitt Oct 16, 2018

Member

please leave the security/podsecuritypolicy/OWNERS file at the parent location

Member

liggitt commented Oct 16, 2018

please leave the security/podsecuritypolicy/OWNERS file at the parent location

@@ -339,19 +339,15 @@ func (s *simpleProvider) ValidateContainer(pod *api.Pod, container *api.Containe
}
allowEscalation := sc.AllowPrivilegeEscalation()
if !s.psp.Spec.AllowPrivilegeEscalation && allowEscalation == nil {

This comment has been minimized.

@liggitt

liggitt Oct 16, 2018

Member

let's not mix changes like this into this PR. defaulting ensures AllowPrivilegeEscalation is non-nil. if you want to pursue this anyway, let's open a separate PR to discuss it.

@liggitt

liggitt Oct 16, 2018

Member

let's not mix changes like this into this PR. defaulting ensures AllowPrivilegeEscalation is non-nil. if you want to pursue this anyway, let's open a separate PR to discuss it.

This comment has been minimized.

@yue9944882

yue9944882 Oct 17, 2018

Contributor

defaulting ensures AllowPrivilegeEscalation is non-nil.

i added this to prevent nil panic. but yeah, ci still looks happy w/o this.

@yue9944882

yue9944882 Oct 17, 2018

Contributor

defaulting ensures AllowPrivilegeEscalation is non-nil.

i added this to prevent nil panic. but yeah, ci still looks happy w/o this.

allErrs = append(allErrs, field.Invalid(scPath.Child("allowPrivilegeEscalation"), allowEscalation, "Allowing privilege escalation for containers is not allowed"))
}
if !s.psp.Spec.AllowPrivilegeEscalation && allowEscalation != nil && *allowEscalation {

This comment has been minimized.

@liggitt

liggitt Oct 16, 2018

Member

same here, pull this change out to a separate PR

@liggitt

liggitt Oct 16, 2018

Member

same here, pull this change out to a separate PR

@k8s-ci-robot k8s-ci-robot added size/XL and removed size/XXL labels Oct 17, 2018

@k8s-ci-robot k8s-ci-robot added size/L and removed size/XL labels Oct 17, 2018

@yue9944882

This comment has been minimized.

Show comment
Hide comment
@yue9944882

yue9944882 Oct 18, 2018

Contributor

/test pull-kubernetes-integration

Contributor

yue9944882 commented Oct 18, 2018

/test pull-kubernetes-integration

@liggitt

This comment has been minimized.

Show comment
Hide comment
@liggitt

liggitt Oct 23, 2018

Member

one comment on initializing slice length during copy, squash commits, then LGTM

Member

liggitt commented Oct 23, 2018

one comment on initializing slice length during copy, squash commits, then LGTM

@liggitt

This comment has been minimized.

Show comment
Hide comment
@liggitt

liggitt Oct 23, 2018

Member

/lgtm
/approve

Member

liggitt commented Oct 23, 2018

/lgtm
/approve

@k8s-ci-robot

This comment has been minimized.

Show comment
Hide comment
@k8s-ci-robot

k8s-ci-robot Oct 23, 2018

Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: liggitt, yue9944882

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:

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

Contributor

k8s-ci-robot commented Oct 23, 2018

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: liggitt, yue9944882

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:

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 k8s-ci-robot merged commit 753dfbe into kubernetes:master Oct 23, 2018

18 checks passed

cla/linuxfoundation yue9944882 authorized
Details
pull-kubernetes-bazel-build Job succeeded.
Details
pull-kubernetes-bazel-test Job succeeded.
Details
pull-kubernetes-cross Skipped
pull-kubernetes-e2e-gce Job succeeded.
Details
pull-kubernetes-e2e-gce-100-performance Job succeeded.
Details
pull-kubernetes-e2e-gce-device-plugin-gpu Job succeeded.
Details
pull-kubernetes-e2e-gke Skipped
pull-kubernetes-e2e-kops-aws Job succeeded.
Details
pull-kubernetes-e2e-kubeadm-gce Skipped
pull-kubernetes-integration Job succeeded.
Details
pull-kubernetes-kubemark-e2e-gce-big Job succeeded.
Details
pull-kubernetes-local-e2e Skipped
pull-kubernetes-local-e2e-containerized Skipped
pull-kubernetes-node-e2e Job succeeded.
Details
pull-kubernetes-typecheck Job succeeded.
Details
pull-kubernetes-verify Job succeeded.
Details
tide In merge pool.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment