[PodSecurity] Add annotations denoting the exemption reason and the enforcement policy used #105908

stlaz · 2021-10-26T13:19:35Z

What type of PR is this?

/kind feature

What this PR does / why we need it:

This PR adds a couple of annotations for the PodSecurity admission. These annotations are supposed to bring more clarity into how the pod was being evaluated. It also changes the audit annotation on audit violations to audit-violation to distinguish it from the namespace annotation used to mark the audit policy level.

Which issue(s) this PR fixes:

Fixes #103923

Special notes for your reviewer:

Does this PR introduce a user-facing change?

The pods and pod controllers that are exempted from the PodSecurity admission process are now marked with the "pod-security.kubernetes.io/exempt: user/namespace/runtimeClass" annotation, based on what caused the exemption.

The enforcement level that allowed or denied pod during PodSecurity admission is now marked by the "pod-security.kubernetes.io/enforce-policy" annotation.

The annotation that informs about audit policy violations changed from ""pod-security.kubernetes.io/audit" to ""pod-security.kubernetes.io/audit-violation".

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

[KEP]: https://github.com/kubernetes/enhancements/blob/master/keps/sig-auth/2579-psp-replacement/README.md

k8s-ci-robot · 2021-10-26T13:19:43Z

@stlaz: This issue is currently awaiting triage.

If a SIG or subproject determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

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.

k8s-ci-robot · 2021-10-26T13:19:43Z

Hi @stlaz. Thanks for your PR.

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

tallclair

Thanks! Left a few small suggestions.

tallclair · 2021-10-27T18:46:17Z

staging/src/k8s.io/pod-security-admission/admission/admission.go

@@ -427,7 +437,7 @@ func (a *Admission) EvaluatePod(ctx context.Context, nsPolicy api.Policy, nsPoli

 	// TODO: reuse previous evaluation if audit level+version is the same as enforce level+version
 	if result := policy.AggregateCheckResults(a.Evaluator.EvaluatePod(nsPolicy.Audit, podMetadata, podSpec)); !result.Allowed {
-		auditAnnotations["audit"] = fmt.Sprintf(
+		auditAnnotations["audit-violation"] = fmt.Sprintf(


KEP has this pluralized. Singular might make more sense, but we should update the KEP to match.

I think I might have just overlooked the plural, this should be fixed now

tallclair · 2021-10-27T18:50:32Z

staging/src/k8s.io/pod-security-admission/admission/admission.go

+	if a.exemptNamespace(attrs.GetNamespace()) {
+		return allowedByExemptResponse("namespace")
+	}
+
+	if a.exemptUser(attrs.GetUserName()) {
+		return allowedByExemptResponse("user")


I think we should cache a shared response for these (one for each exemption). The exemption flow isn't quite as performance critical as privileged, but I'd still like to keep it fast.

The runtimeClass exemption doesn't need to be as fast, but it probably simplifies things to just make it consistent with the other 2.

I used the same pattern as the allowed response is using, then

tallclair · 2021-10-27T18:54:53Z

staging/src/k8s.io/pod-security-admission/api/constants.go

+	ExemptionReasonLabel = labelPrefix + "exempt"
+	AuditViolationLabel  = labelPrefix + "audit-violation"
+	EnforcedPolicyLabel  = labelPrefix + "enforce-policy"


Currently unused. I like having these here though, so I recommend updating admission.go to refer to them. Also rename.

Suggested change

ExemptionReasonLabel = labelPrefix + "exempt"

AuditViolationLabel = labelPrefix + "audit-violation"

EnforcedPolicyLabel = labelPrefix + "enforce-policy"

ExemptionReasonAnnotationKey = "exempt"

AuditViolationAnnotationKey = "audit-violation"

EnforcedPolicyAnnotationKey = "enforce-policy"

Thanks for the suggestion, it should be incorporated in the PR now.

tallclair

/lgtm
/approve

k8s-ci-robot · 2021-11-01T17:28:49Z

[APPROVALNOTIFIER] This PR is APPROVED

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

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:

~~staging/src/k8s.io/pod-security-admission/OWNERS~~ [tallclair]

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

liggitt · 2021-11-01T17:29:35Z

/ok-to-test

liggitt · 2021-11-01T19:17:04Z

/retest

k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Oct 26, 2021

k8s-ci-robot requested review from deads2k and liggitt October 26, 2021 13:20

k8s-ci-robot added sig/auth Categorizes an issue or PR as relevant to SIG Auth. and removed do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Oct 26, 2021

tallclair reviewed Oct 27, 2021

View reviewed changes

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 28, 2021

stlaz force-pushed the ps_annotations branch from 65179b4 to f73f3a1 Compare November 1, 2021 12:20

podsecurity: annotations on responses allowed by exemption

4a79488

stlaz force-pushed the ps_annotations branch from f73f3a1 to 037daeb Compare November 1, 2021 12:26

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

stlaz added 2 commits November 1, 2021 13:27

podsecurity: distinguish between audit and audit violation annotations

65f88c6

podsecurity: add an annotation informing about which policy was enforced

037daeb

tallclair approved these changes Nov 1, 2021

View reviewed changes

k8s-ci-robot assigned tallclair Nov 1, 2021

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 1, 2021

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 1, 2021

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 Nov 1, 2021

k8s-ci-robot merged commit ac2d872 into kubernetes:master Nov 1, 2021

k8s-ci-robot added this to the v1.23 milestone Nov 1, 2021

stlaz deleted the ps_annotations branch November 2, 2021 08:10

liggitt mentioned this pull request Nov 3, 2021

PodSecurity admission (PodSecurityPolicy replacement) kubernetes/enhancements#2579

Closed

12 tasks

This was referenced Jan 31, 2022

Document annotation: pod-security.kubernetes.io/enforce-policy kubernetes/website#30809

Closed

Add documentation on pod-security.kubernetes.io annotations kubernetes/website#31602

Merged

stlaz mentioned this pull request Apr 1, 2022

REQUEST: New membership for stlaz kubernetes/org#3352

Closed

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[PodSecurity] Add annotations denoting the exemption reason and the enforcement policy used #105908

[PodSecurity] Add annotations denoting the exemption reason and the enforcement policy used #105908

stlaz commented Oct 26, 2021

k8s-ci-robot commented Oct 26, 2021

k8s-ci-robot commented Oct 26, 2021

tallclair left a comment

tallclair Oct 27, 2021

stlaz Nov 1, 2021

tallclair Oct 27, 2021

stlaz Nov 1, 2021

tallclair Oct 27, 2021

stlaz Nov 1, 2021

tallclair left a comment

k8s-ci-robot commented Nov 1, 2021

liggitt commented Nov 1, 2021

liggitt commented Nov 1, 2021

[PodSecurity] Add annotations denoting the exemption reason and the enforcement policy used #105908

[PodSecurity] Add annotations denoting the exemption reason and the enforcement policy used #105908

Conversation

stlaz commented Oct 26, 2021

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

k8s-ci-robot commented Oct 26, 2021

k8s-ci-robot commented Oct 26, 2021

tallclair left a comment

Choose a reason for hiding this comment

tallclair Oct 27, 2021

Choose a reason for hiding this comment

stlaz Nov 1, 2021

Choose a reason for hiding this comment

tallclair Oct 27, 2021

Choose a reason for hiding this comment

stlaz Nov 1, 2021

Choose a reason for hiding this comment

tallclair Oct 27, 2021

Choose a reason for hiding this comment

stlaz Nov 1, 2021

Choose a reason for hiding this comment

tallclair left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Nov 1, 2021

liggitt commented Nov 1, 2021

liggitt commented Nov 1, 2021