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

mtardy · 2022-02-02T19:29:18Z

Addresses issue #30809 and more 😄 !

This PR adds documentation for the following audit log annotations in the Well-Known Labels, Annotations and Taints page:

pod-security.kubernetes.io/exempt
pod-security.kubernetes.io/audit-violations
pod-security.kubernetes.io/enforce-policy

These annotations were added by PR [PodSecurity] Add annotations denoting the exemption reason and the enforcement policy used #105908 in the Kubernetes repository.

It's maybe a bit special because they are the first notes on audit logs Event kind on the page. I also took the liberty to use an abbreviated example for audit-violations.

netlify · 2022-02-02T19:32:51Z

✔️ Deploy Preview for kubernetes-io-main-staging ready!

🔨 Explore the source changes: cc64e7c

🔍 Inspect the deploy log: https://app.netlify.com/sites/kubernetes-io-main-staging/deploys/621fc2116376d00008505901

😎 Browse the preview: https://deploy-preview-31602--kubernetes-io-main-staging.netlify.app

sftim · 2022-02-03T13:42:19Z

/sig auth

sftim

As written, I think this would mislead readers into thinking that audit annotations apply to the Event objects - they don't.

I suggested a clarification.

PS. https://kubernetes.io/docs/reference/glossary/?fundamental=true&core-object=true#term-event might be useful too?

sftim · 2022-02-03T13:46:14Z

content/en/docs/reference/labels-annotations-taints.md

+
+Example: `pod-security.kubernetes.io/exempt: namespace`
+
+Used on: Event


This isn't used on resources of kind Event; in fact, this annotation is not used on resources.

Suggested change

Used on: Event

This annotation is not used within the Kubernetes API. When you

[enable auditing](/docs/tasks/debug-application-cluster/audit/) in your cluster,

audit event data is written using `Event` from API group `audit.k8s.io`.

The annotation applies to audit events. Audit events are different from objects in the

[Event API](/docs/reference/kubernetes-api/cluster-resources/event-v1/) (API group

`events.k8s.io`).

But audit logs aren't composed of kind Event? Like this entry of the logs for example?

{ "kind": "Event", "apiVersion": "audit.k8s.io/v1", "level": "Metadata", "auditID": "c981b4a5-e51a-4c14-b9c1-dabe06fd605a", "stage": "ResponseComplete", "requestURI": "/api/v1/namespaces/verify-pod-security/pods?fieldManager=kubectl-client-side-apply", "verb": "create", "user": { "username": "minikube-user", "groups": [ "system:masters", "system:authenticated" ] }, "sourceIPs": [ "192.168.49.1" ], "userAgent": "kubectl/v1.23.3 (darwin/amd64) kubernetes/816c97a", "objectRef": { "resource": "pods", "namespace": "verify-pod-security", "name": "busybox-privileged", "apiVersion": "v1" }, "responseStatus": { "metadata": {}, "status": "Failure", "reason": "Forbidden", "code": 403 }, "requestReceivedTimestamp": "2022-02-02T18:32:40.833098Z", "stageTimestamp": "2022-02-02T18:32:40.841531Z", "annotations": { "authorization.k8s.io/decision": "allow", "authorization.k8s.io/reason": "", "pod-security.kubernetes.io/audit-violations": "would violate PodSecurity \"restricted:latest\": allowPrivilegeEscalation != false (container \"busybox\" must set securityContext.allowPrivilegeEscalation=false), unrestricted capabilities (container \"busybox\" must set securityContext.capabilities.drop=[\"ALL\"]), runAsNonRoot != true (pod or container \"busybox\" must set securityContext.runAsNonRoot=true), seccompProfile (pod or container \"busybox\" must set securityContext.seccompProfile.type to \"RuntimeDefault\" or \"Localhost\")", "pod-security.kubernetes.io/enforce-policy": "restricted:latest" } }

Event from audit.k8s.io/v1 != Event from events.k8s.io/v1.

Unless we're clear about it, some readers will assume that Event means the kind used in the API.

There's no HTTP API that lets you create, read, update or delete the audit.k8s.io/v1 sort of Event.

oh 😲 ! All right I see, thanks a lot! It's confusing I admit, I was not very familiar with audits logs and didn't notice they were completely different objects. So what do you think, we should put a disclaimer in front of the three annotations "This annotation is not used within the Kubernetes API" or put them in a separate category then?

We discussed the subject a little bit with sig security docs folks and thought that maybe a good idea would be to publish the missing content first (with the disclaimer you proposed) and open an issue to reorganize this reference section.

👍

I made the choice to repeat the disclaimer to avoid dependencies between references like on this one that adds:

The taints listed below are always used on Nodes

Which can be skipped if you are only looking for a specific label, annotation or taint without reading the whole document.

I also noticed that the API objects should be in inline code format according to the style guide.

Would it be preferable to create multiple issues for the possible independent contributions or create one asking for the whole reorganization/reformatting?

As written, I think this would mislead readers into thinking that audit annotations apply to the Event objects - they don't.

I had the same reaction before seeing this comment. This info is good to document, but I don't think it belongs on this page. Putting it on an audit-specific or PodSecurity-specific page seems less misleading

We could create a separate page for audit annotations and have the main Well-Known Labels, Annotations and Taints refer to the other page. Maybe that, then?

It'd be nice to put the audit annotation keys somewhere in the main Well-Known Labels, Annotations and Taints page, I think, so that if people search for any of those, they do find a match.

Yes! Originally, the idea would have been to add it "as is", but I admit that it might be confusing... And then to reorganize the whole reference page to have categories.

Tell me if you think we could organize this differently so I change that PR! :)

sftim · 2022-02-03T13:47:33Z

content/en/docs/reference/labels-annotations-taints.md

+See [Pod Security Standards](/docs/concepts/security/pod-security-standards/)
+for more information.


Maybe link directly to https://k8s.io/docs/concepts/security/pod-security-standards/#policy-instantiation ?

Yes why not, I admit I prefered the beginning of the page because of the table with the privilege, baseline, restricted descriptions and these specific part on instantiation was not that helpful!

…roups

sftim

A bit more feedback

content/en/docs/reference/labels-annotations-taints.md

mtardy · 2022-02-08T16:50:10Z

Are the new changes ok @sftim? 😸

Review was stale

sftim · 2022-02-08T18:17:05Z

I'd like to have SIG Auth provide a technical review here. The Markdown changes LGTM.

tengqm · 2022-02-21T11:34:34Z

/approve

kbhawkey · 2022-03-01T01:59:43Z

Page preview:
https://deploy-preview-31602--kubernetes-io-main-staging.netlify.app/docs/reference/labels-annotations-taints/#pod-security-kubernetes-io-exempt

kbhawkey · 2022-03-01T02:04:44Z

Ping
/sig auth

liggitt · 2022-03-01T02:58:21Z

The blurbs on each annotation look correct, but my comment at #31602 (comment) stands... I don't think this is the right page for audit annotations that are not set on REST API objects.

This This document serves both as a reference to the values and as a coordination point for assigning values. statement this page starts with isn't accurate for audit annotations, and needing to put a note beside each audit annotation calling out that it is not actually an annotation you can set seems like confirmation this is a confusing place to put these.

mtardy · 2022-03-01T09:16:29Z

The blurbs on each annotation look correct, but my comment at #31602 (comment) stands... I don't think this is the right page for audit annotations that are not set on REST API objects.

This This document serves both as a reference to the values and as a coordination point for assigning values. statement this page starts with isn't accurate for audit annotations, and needing to put a note beside each audit annotation calling out that it is not actually an annotation you can set seems like confirmation this is a confusing place to put these.

ok @liggitt, so I'm not sure what I must do then. We also agreed that it was a problem and created this issue Reorganize the "Well-Known Labels, Annotations and Taints" reference page #31648. And Tim Bannister already noted that some others audit annotations were missing from the doc in Document built-in audit annotations #31576.

So you think we should not merge this PR before it has been reorganized? Or that it is even better we split the reference page about annotations in two, having two subpages for "Well-Known Labels, Annotations and Taints"?

And can I really work on this (reorganizing pages and creating a new one) or is it too big a change that needs to be discussed and approved first?

liggitt · 2022-03-01T13:28:36Z

I think a separate page for audit annotations would be clearer. Including them on the existing page seems more confusing than helpful to me.

sftim · 2022-03-01T13:41:47Z

I'm fine with having a separate page for audit annotations, provided that a “search in page” for those annotation keys, in the main annotations/labels/taints page, yields something (perhaps a hyperlink).

If we skip that, people looking at the official list may well think we've missed them out, and might think that even if the page does include text that mentions checking other lists.

sftim · 2022-03-01T13:42:43Z

@mtardy based on the discussion so far, what course of action would you recommend? (if you're flummoxed enough that you don't feel you can recommend anything, I'd understand!)

mtardy · 2022-03-02T09:42:29Z

I'm fine with having a separate page for audit annotations, provided that a “search in page” for those annotation keys, in the main annotations/labels/taints page, yields something (perhaps a hyperlink).

If we skip that, people looking at the official list may well think we've missed them out, and might think that even if the page does include text that mentions checking other lists.

Ahah I'm a bit flummoxed (btw thanks I didn't know this word 😄 ), I admit!

But nevertheless, what would you think that we do something like this:

.
├── [...]
├── API Access Control
│   └── ...
├── Well-Known Labels, Annotations and Taints  <-- Reference two categories page with simple lists and links
│   ├── API*                                   <-- Page with all the current details from the existing page
│   └── Audits*                                <-- Page with the new documentation for audits stuff
├── Kubernetes API
│   └── ...
└── [...]

* not the final name

The idea is to have a complete reference page, one in which you can CTRL+F, a kind of table of content for the two sub-pages. And the details in the two sub-pages, separated into the existing stuff, which is about the Kubernetes API, and the new documentation provided by this PR and the future PR for the issue #31576 for example.

What do you think? If you agree, what do you think is the next step? Do I need to explicit all that in a separate issue or can I directly create a PR from this discussion to try to fix that?

sftim · 2022-03-02T13:12:38Z

That suggestion might be more complicated than it needs to be @mtardy

I was thinking of including a simple unordered list near the end of https://kubernetes.io/docs/reference/labels-annotations-taints/, headed “Annotations used for audit”, where each entry is a hyperlink to the relevant heading in that separate Audit annotations page.

No need to create a new page that lists API annotations / labels / taints; those can stay in https://kubernetes.io/docs/reference/labels-annotations-taints/

Would that work? I think it'd be less effort.

mtardy · 2022-03-02T17:12:17Z

.
├── [...]
├── API Access Control
│   └── ...
├── Well-Known Labels, Annotations and Taints  <-- Current page "as-is" + list of audit annotations in new category
│   └── Audits*                                <-- Page with the new documentation for audit annotations
├── Kubernetes API
│   └── ...
└── [...]

* not the final name

Ok so something like this? You are right, it will be much simpler!

sftim · 2022-03-02T17:14:14Z

…on the main page

mtardy · 2022-03-02T19:19:03Z

I'm not really sure about the level 2 headers:

Labels, annotations and taints used on API objects
Annotations used for audit

We can remake it flat with only level 2 headers, but the "Annotations used for audit" looks a bit weird with all the others headers named from a label, an annotation, or a taint.

It feels a bit messy in the end but this page was not already super neat! What do you think?

mtardy · 2022-03-07T09:13:16Z

Here are the previsualizations for the Well-Known Labels, Annotations and Taints page and the Audit Annotations new page.

Do you think it's better this way @liggitt and @sftim?

liggitt · 2022-03-07T13:47:36Z

Do you think it's better this way @liggitt and @sftim?

I think so.

I'm not really sure why we need to double-maintain the list of annotations as links from the API labels/annotations page, if search results would take people directly to the audit annotations page anyway (and including them in the current page seems likely to actually confuse search results and maybe make people do two hops), but this is definitely clearer.

mtardy · 2022-03-07T13:57:51Z

I'm not really sure why we need to double-maintain the list of annotations as links from the API labels/annotations page, if search results would take people directly to the audit annotations page anyway (and including them in the current page seems likely to actually confuse search results and maybe make people do two hops), but this is definitely clearer.

I agree, Tim suggested a searchable reference page, that's why. But it results in a "double-maintain" list, as you said, which is not very handy, maybe we should keep the current "Well know annotations" page as-is and create the new one without creating a single searchable page (and use the search engines as the searchable page).

liggitt · 2022-03-07T14:02:03Z

I don't feel that strongly, so I'll defer the searchability / double-maintenance burden question to docs folks

content lgtm

sftim · 2022-03-07T14:02:45Z

Let's do it with the extra toil for now. If we find we're documenting lots of audit annotations, we can look at adding automation to make that easier.

I also actually like the side-effect that non-audit annotations now use a 3rd-level heading, which makes the page body more compact.
/lgtm
/approve

k8s-ci-robot · 2022-03-07T14:03:07Z

LGTM label has been added.

Git tree hash: 4b21127e8dca72ae881782ed206d8bb243571347

k8s-ci-robot · 2022-03-07T14:03:13Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sftim, tengqm

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:

~~content/en/OWNERS~~ [sftim,tengqm]

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

sftim · 2022-03-07T14:04:23Z

Aside: we might in future allow filtering the annotations / labels / taints, the way you can filter glossary entries.

mtardy · 2022-03-07T14:08:10Z

Aside: we might in future allow filtering the annotations / labels / taints, the way you can filter glossary entries.

It would be cool, maybe I can mention that in the reorganization #31648 issue!

Thanks for the feedback and the help! 🤗

Add documentation on pod-security.kubernetes.io annotations

167ddd3

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Feb 2, 2022

k8s-ci-robot requested review from divya-mohan0209 and onlydole February 2, 2022 19:30

k8s-ci-robot added language/en Issues or PRs related to English language sig/docs Categorizes an issue or PR as relevant to SIG Docs. labels Feb 2, 2022

mtardy mentioned this pull request Feb 3, 2022

Create a security checklist for deploying a cluster kubernetes/sig-security#28

Closed

9 tasks

k8s-ci-robot added the sig/auth Categorizes an issue or PR as relevant to SIG Auth. label Feb 3, 2022

sftim reviewed Feb 3, 2022

View reviewed changes

Add disclaimers before reorganizing this reference page between API g…

3cc1b1b

…roups

sftim previously requested changes Feb 4, 2022

View reviewed changes

content/en/docs/reference/labels-annotations-taints.md Outdated Show resolved Hide resolved

content/en/docs/reference/labels-annotations-taints.md Outdated Show resolved Hide resolved

Remove the Used on information and replace caution tag with note

e00fe1d

mtardy mentioned this pull request Feb 7, 2022

Reorganize the "Well-Known Labels, Annotations and Taints" reference page #31648

Open

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 21, 2022

mtardy added 2 commits March 2, 2022 19:23

Merge branch 'kubernetes:main' into main

876f452

Add a separate subpage for audit annotations and add reference to it …

cc64e7c

…on the main page

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Mar 2, 2022

k8s-ci-robot assigned sftim Mar 7, 2022

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 7, 2022

k8s-ci-robot merged commit 28ffd81 into kubernetes:main Mar 7, 2022

sftim mentioned this pull request Mar 7, 2022

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

Closed

mtardy mentioned this pull request Apr 26, 2022

REQUEST: New membership for mtardy kubernetes/org#3393

Closed

9 tasks


		Example: `pod-security.kubernetes.io/exempt: namespace`

		Used on: Event

-Used on: Event
+This annotation is not used within the Kubernetes API. When you
+[enable auditing](/docs/tasks/debug-application-cluster/audit/) in your cluster,
+audit event data is written using `Event` from API group `audit.k8s.io`.
+The annotation applies to audit events. Audit events are different from objects in the
+[Event API](/docs/reference/kubernetes-api/cluster-resources/event-v1/) (API group
+`events.k8s.io`).

		See [Pod Security Standards](/docs/concepts/security/pod-security-standards/)
		for more information.

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

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

Conversation

mtardy commented Feb 2, 2022

netlify bot commented Feb 2, 2022 • edited Loading

sftim commented Feb 3, 2022

sftim left a comment

Choose a reason for hiding this comment

sftim Feb 3, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mtardy Feb 4, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sftim left a comment

Choose a reason for hiding this comment

mtardy commented Feb 8, 2022

sftim commented Feb 8, 2022

tengqm commented Feb 21, 2022

kbhawkey commented Mar 1, 2022

kbhawkey commented Mar 1, 2022

liggitt commented Mar 1, 2022 • edited Loading

mtardy commented Mar 1, 2022 • edited Loading

liggitt commented Mar 1, 2022

sftim commented Mar 1, 2022

sftim commented Mar 1, 2022

mtardy commented Mar 2, 2022

sftim commented Mar 2, 2022

mtardy commented Mar 2, 2022 • edited Loading

sftim commented Mar 2, 2022

mtardy commented Mar 2, 2022 • edited Loading

mtardy commented Mar 7, 2022 • edited Loading

liggitt commented Mar 7, 2022 • edited Loading

mtardy commented Mar 7, 2022 • edited Loading

liggitt commented Mar 7, 2022

sftim commented Mar 7, 2022

k8s-ci-robot commented Mar 7, 2022

k8s-ci-robot commented Mar 7, 2022

sftim commented Mar 7, 2022

mtardy commented Mar 7, 2022

netlify bot commented Feb 2, 2022 •

edited

Loading

sftim Feb 3, 2022 •

edited

Loading

mtardy Feb 4, 2022 •

edited

Loading

liggitt commented Mar 1, 2022 •

edited

Loading

mtardy commented Mar 1, 2022 •

edited

Loading

mtardy commented Mar 2, 2022 •

edited

Loading

mtardy commented Mar 2, 2022 •

edited

Loading

mtardy commented Mar 7, 2022 •

edited

Loading

liggitt commented Mar 7, 2022 •

edited

Loading

mtardy commented Mar 7, 2022 •

edited

Loading