Skip to content
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

v1.23 blog post: Pod Security Admission Graduates to Beta #30502

Merged
merged 1 commit into from
Dec 8, 2021

Conversation

jimangel
Copy link
Member

@jimangel jimangel commented Nov 16, 2021

Draft blog post on behalf of SIG Auth for the v1.23 release around Pod Security Admission (supporting KEP).

I also had an issue with my demo until I changed runtimeClassNames to runtimeClasses. Potentially someone can weigh in on that during tech review.

Next Steps:

  • Update blog & official docs URLs (in content and at bottom)
  • Update kindest/node:v1.23.0 to the correct sha256 tag (using latest for now)
  • Identify the "official" blog release date (update filename / front matter)
  • SIG Docs review
  • SIG Auth tech review

Preview quick link: https://deploy-preview-30502--kubernetes-io-main-staging.netlify.app/blog/2021/12/09/pod-security-admission-beta/

/hold (until all steps are complete)
/cc @lachie83 @reylejano @liggitt

@k8s-ci-robot k8s-ci-robot added this to the 1.23 milestone Nov 16, 2021
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Nov 16, 2021
@netlify
Copy link

netlify bot commented Nov 16, 2021

👷 Deploy Preview for kubernetes-io-vnext-staging processing.

🔨 Explore the source changes: b4b25c6

🔍 Inspect the deploy log: https://app.netlify.com/sites/kubernetes-io-vnext-staging/deploys/61943df67b69390007d34f2c

@k8s-ci-robot k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. area/blog Issues or PRs related to the Kubernetes Blog subproject language/en Issues or PRs related to English language sig/docs Categorizes an issue or PR as relevant to SIG Docs. labels Nov 16, 2021
@liggitt
Copy link
Member

liggitt commented Nov 16, 2021

runtimeClassNames typo fixed in https://github.com/kubernetes/website/pull/30274/files

@jimangel
Copy link
Member Author

jimangel commented Nov 16, 2021

runtimeClassNames typo fixed in https://github.com/kubernetes/website/pull/30274/files

Perfect! As long as it's technically correct, it will have no impact to the release process (and fixes the inflight v1.23 docs code section too).

Edit: Removed files w/ typo corrections in PR to make for easy, single file, review. We might need to still retroactively patch the docs in dev-1.23 or main depending on when we see this.

@jlbutler
Copy link
Contributor

Should this be filed against main rather than the dev branch?

/cc @karenhchu

Copy link
Contributor

@sftim sftim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some early feedback. I realise it's a WIP PR so please see everything here as a suggestion only.

@sftim
Copy link
Contributor

sftim commented Nov 16, 2021

We should get a tech review on this from SIG Auth (which I hope will be easy to get because of the eyes on this already)

@lachie83
Copy link
Member

We should get a tech review on this from SIG Auth (which I hope will be easy to get because of the eyes on this already)

I will reach out to @ritazh from SIG Auth to see who can help with tech review.

@jimangel
Copy link
Member Author

jimangel commented Nov 16, 2021

Should this be filed against main rather than the dev branch?

If we know the blog release date before the Kubernetes version is released, it shouldn't matter. As it could merge and not go live until the date.

I think you're right though, historically we've opened blogs against main since they would get merged / go live post-release.

I'm good with either option, let me know a decision and I can act. I would love someone closer to the blog subproject to weigh in with a suggestion.

Edit: Moved to main based on Tim's recommendation!

Copy link
Member

@tallclair tallclair left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, thanks for writing this!


It's important to note that Pod Security Admission doesn't have complete feature parity with the deprecated Pod Security Policy. Specifically, it doesn't have the ability to `mutate` or change Kubernetes resources to auto-remediate a policy violation on behalf of the user. Additionally, it doesn't provide fine-grain control over each allowed field and value within a pod specification or any other Kubernetes resource that you may wish to evaluate. If you need more fine-grained policy control then take a look at the [Gatekeeper](https://github.com/open-policy-agent/gatekeeper) project which supports such use cases.

Pod Security Admission also adheres to Kubernetes best practices of declarative object management by denying resources that violate the policy. This requires resources to be updated in source repositories, and tooling to be updated prior to being deployed to Kubernetes.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I understand this sentence. Isn't this also true of PSP (and most validating admission controllers, for that matter)?

Copy link
Member Author

@jimangel jimangel Nov 16, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you're referring to "Pod Security Admission also adheres to Kubernetes best practices of declarative object management by denying resources that violate the policy. This requires resources to be updated in source repositories, and tooling to be updated prior to being deployed to Kubernetes."

The intention is to call out the explicit declarative nature of Pod Security (and any admission controller), whereas PSP had the ability to introspect CRI information to make decisions. For example, if you built a container as non-root, and didn't include a runAsUser, it would pass PSP but fail Pod Security (circumstances depending).

Open to alternative ways to draw attention to this paradigm shift from PSP to "normal" admission behavior.

@sftim
Copy link
Contributor

sftim commented Nov 16, 2021

If we know the blog release date before the Kubernetes version is released, it shouldn't matter. As it could merge and not go live until the date.
I think you're right though, historically we've opened blogs against main since they would get merged / go live post-release.

I would have this ready-to-merge but not merge it until the release is confirmed. And I'd target main.
In essence, we shouldn't merge any announcement for publication until we are certain of the publication date.

@jimangel jimangel force-pushed the blog-psa-beta branch 2 times, most recently from c039c05 to 2966dc7 Compare November 16, 2021 22:57
@jimangel jimangel changed the base branch from dev-1.23 to main November 16, 2021 22:57
@jimangel jimangel changed the base branch from main to dev-1.23 November 16, 2021 22:58
@sftim
Copy link
Contributor

sftim commented Nov 19, 2021

It would also be ideal for both to be merged / published around the same dates so that they are available for consumption and most importantly the cross-links work

The easy answer is for us to have the tutorial page and the blog article both ready to go on v1.23 release day, and then manually merge both of these on a different day post release.

Slightly more faff, but possible: publish the tutorial at release time without a reference to the blog article, have the blog scheduled, have a follow-up PR ready that hyperlinks from the tutorial to the blog article as further reading.

Copy link
Contributor

@sftim sftim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this could go in as-is, and the changes I'm suggesting here (are what I think would) add polish to an already good article.


### When to use `warn`?

The typical uses for `warn` are:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
The typical uses for `warn` are:
The typical uses for `warn` are to get ready for a future change where you want to enforce
a different policy. The most two common cases would be:

How about case 3, you're not enforcing any policy yet, and you want to preview the impact? I think that's a good use for warn (maybe also: delete all existing Pods once you do).

I should add that I haven't actually tried Pod Security admission in any real-world cluster with a real world workload on it, so please take that in mind when considering my suggestions.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about case 3, you're not enforcing any policy yet, and you want to preview the impact? I think that's a good use for warn

That's sort of the same as "warn at a stricter level", where enforce is privileged and warn is baseline or restricted. If you think it's clearer to explicitly call that out as a third case, that's fine

(maybe also: delete all existing Pods once you do).

If you're looking to check a new policy non-disruptively, deleting existing pods is not the way to do it :-)

@sftim
Copy link
Contributor

sftim commented Dec 5, 2021

@jimangel I think this is ready for review, right?

/approve
The intent of this PR is a change I am very happy to see made.

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 5, 2021
@PushkarJ
Copy link
Member

PushkarJ commented Dec 6, 2021

@sftim as per your feedback: #30502 (comment) (I think) the tutorial #30422 is now ready to go and currently on /hold for this blog PR to get merged / published.

Namespace level tutorial preview
Cluster level tutorial preview

@jimangel
Copy link
Member Author

jimangel commented Dec 7, 2021

This is ready to merge / final review. I just pushed the following changes:

  • Actionable feedback addressed
  • TODO's removed
  • v1.23 links added
  • Incorporated the future tutorial reference / links (thanks @PushkarJ!)
  • Modified the file / date to match intended publish date
  • General clean up / checks

At this point I don't anticipate making any further changes unless requested! Once the build finishes I'll update the preview links.

/cc @lachie83

@sftim
Copy link
Contributor

sftim commented Dec 8, 2021

/hold cancel

v1.23 is released, I don't see anything blocking publication.

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 8, 2021
Co-authored-by: Lachlan Evenson <lachlan.evenson@microsoft.com>
@ritazh
Copy link
Member

ritazh commented Dec 8, 2021

/approve
/lgtm
Thank you everyone for working on this PR! 🎉

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

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

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

@sftim
Copy link
Contributor

sftim commented Dec 8, 2021

Not sure Prow saw the edited /lgtm from #30502 (comment) so:
/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 8, 2021
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: 07950e1abcb37c0ef675c295684026195c6852cc

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/blog Issues or PRs related to the Kubernetes Blog subproject cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. language/en Issues or PRs related to English language lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/docs Categorizes an issue or PR as relevant to SIG Docs. sig/security Categorizes an issue or PR as relevant to SIG Security. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.