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

Add seccomp default feature blog post #28951

Merged
merged 1 commit into from
Aug 24, 2021

Conversation

saschagrunert
Copy link
Member

@saschagrunert saschagrunert commented Jul 15, 2021

This adds the blog post about the new Kubernetes SeccompDefault alpha feature.

/hold
cc @PI-Victor @pmmalinov01 @ashnehete @carlisia @chrisnegus @ritpanjw

Todo

  • Adjust publishing date

@k8s-ci-robot k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. area/blog Issues or PRs related to the Kubernetes Blog subproject labels Jul 15, 2021
@k8s-ci-robot 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 Jul 15, 2021
@netlify
Copy link

netlify bot commented Jul 15, 2021

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

🔨 Explore the source changes: 84e472e

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

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

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.

This looks like a thorough explanation.

Here's some feedback on the article.

content/en/blog/2021-08-15-seccomp-default.md Outdated Show resolved Hide resolved
content/en/blog/2021-08-15-seccomp-default.md Outdated Show resolved Hide resolved
content/en/blog/2021-08-15-seccomp-default.md Outdated Show resolved Hide resolved
content/en/blog/2021-08-15-seccomp-default.md Outdated Show resolved Hide resolved
content/en/blog/2021-08-15-seccomp-default.md Outdated Show resolved Hide resolved
content/en/blog/2021-08-15-seccomp-default.md Outdated Show resolved Hide resolved
content/en/blog/2021-08-15-seccomp-default.md Outdated Show resolved Hide resolved
content/en/blog/2021-08-15-seccomp-default.md Outdated Show resolved Hide resolved
@sftim sftim added this to Ready for review in Blog articles via automation Jul 20, 2021
@sftim sftim moved this from Ready for review to Planned in Blog articles Jul 20, 2021
@sftim sftim moved this from Planned to Ready for review in Blog articles Jul 21, 2021
@saschagrunert saschagrunert force-pushed the seccomp-default-blog branch 2 times, most recently from 0c9aa66 to 3919f2d Compare July 23, 2021 07:11
@sftim
Copy link
Contributor

sftim commented Jul 28, 2021

Looks good to review and I don't see a need for a hold (the article is future dated, to a date after the scheduled release).

/hold cancel

@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 Jul 28, 2021
@sftim
Copy link
Contributor

sftim commented Jul 28, 2021

Any more feedback or is this OK for LGTM?

@PI-Victor
Copy link
Member

i gave this a read way back, LGTM from my side

@saschagrunert saschagrunert force-pushed the seccomp-default-blog branch 2 times, most recently from 1480b9c to 0106220 Compare July 29, 2021 11:43
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.

Hi. Here's some feedback. It's fine to ship this without addressing the style nits I'm mentioning in this review.

content/en/blog/_posts/2021-08-20-seccomp-default.md Outdated Show resolved Hide resolved
content/en/blog/_posts/2021-08-20-seccomp-default.md Outdated Show resolved Hide resolved
content/en/blog/_posts/2021-08-20-seccomp-default.md Outdated Show resolved Hide resolved
content/en/blog/_posts/2021-08-20-seccomp-default.md Outdated Show resolved Hide resolved
content/en/blog/_posts/2021-08-20-seccomp-default.md Outdated Show resolved Hide resolved
content/en/blog/_posts/2021-08-20-seccomp-default.md Outdated Show resolved Hide resolved
content/en/blog/_posts/2021-08-20-seccomp-default.md Outdated Show resolved Hide resolved
content/en/blog/_posts/2021-08-20-seccomp-default.md Outdated Show resolved Hide resolved
content/en/blog/_posts/2021-08-20-seccomp-default.md Outdated Show resolved Hide resolved
@saschagrunert saschagrunert force-pushed the seccomp-default-blog branch 2 times, most recently from 36dca2e to d64aa11 Compare August 2, 2021 08:40
@saschagrunert
Copy link
Member Author

Hi. Here's some feedback. It's fine to ship this without addressing the style nits I'm mentioning in this review.

Thank you! I addressed all of the review comments.

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 19, 2021
@saschagrunert
Copy link
Member Author

/hold

Publication is due tomorrow and reviews are not yet in place.

If this article goes past the publication date, we should pick a new date, update the article to match that, check all is good, and then remove this hold.

From my point of view the open discussion point has been resolved. Are we good to merge?

@sftim
Copy link
Contributor

sftim commented Aug 20, 2021

@saschagrunert can you point to a signoff that confirms what this PR is saying is technically correct? If that were available, I'd be happy to approve it right away.

@sftim
Copy link
Contributor

sftim commented Aug 20, 2021

@PushkarJ are you willing to add /lgtm here?

@saschagrunert
Copy link
Member Author

@saschagrunert can you point to a signoff that confirms what this PR is saying is technically correct? If that were available, I'd be happy to approve it right away.

I only have source code, container runtimes basically do nothing on privileged containers:

https://github.com/containerd/containerd/blob/cf600abecc27200a3c0e1415cd1f6c325eb05edf/pkg/cri/server/container_create_linux.go#L424

CRI-O behaves in the same way.

@sftim
Copy link
Contributor

sftim commented Aug 20, 2021

From the blog team's perspective, we're looking for a subject matter expert (or someone who is expert enough) to check technical accuracy. SIG Docs can't promise to have the skills for this.

Copy link
Member

@PushkarJ PushkarJ left a comment

Choose a reason for hiding this comment

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

These are mostly quick fixes and minor nits. Happy to lgtm after that

content/en/blog/_posts/2021-08-20-seccomp-default.md Outdated Show resolved Hide resolved
content/en/blog/_posts/2021-08-20-seccomp-default.md Outdated Show resolved Hide resolved
- name: test-container-nginx
image: nginx:1.21
securityContext:
seccompProfile:
Copy link
Member

Choose a reason for hiding this comment

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

Thank you for making this change

content/en/blog/_posts/2021-08-20-seccomp-default.md Outdated Show resolved Hide resolved
[strace][strace]) for any executed syscalls which may be blocked by the
default profiles. If that's the case, then you can create a custom seccomp
profile based on the default by adding the additional syscalls to the
`"action": "SCMP_ACT_ALLOW"` section.
Copy link
Member

Choose a reason for hiding this comment

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

+1 on blog post

content/en/blog/_posts/2021-08-20-seccomp-default.md Outdated Show resolved Hide resolved
@saschagrunert saschagrunert force-pushed the seccomp-default-blog branch 2 times, most recently from bf7aede to 3f8b8fc Compare August 23, 2021 07:56
@saschagrunert
Copy link
Member Author

saschagrunert commented Aug 23, 2021

@PushkarJ added the changes. Thank you for the review, PTAL again.
@sftim since we passed the deadline, do you have any suggestion on a new date?

@PushkarJ
Copy link
Member

Thank you @saschagrunert for patiently resolving all the comments. Looking forward to this blog post and more people using the feature as a result of that :)

/lgtm

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

LGTM label has been added.

Git tree hash: edb48bb7299357c2ce0983dc6863e10171ff0205

This adds the blog post about the new Kubernetes `SeccompDefault` alpha
feature.

Signed-off-by: Sascha Grunert <sgrunert@redhat.com>
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 24, 2021
@sftim
Copy link
Contributor

sftim commented Aug 24, 2021

/hold cancel
/approve

borrowing LGTM from #28951 (comment)
/lgtm

@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 Aug 24, 2021
@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 24, 2021
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: ad9c7b7f6f8cd1357b08fc08e1e5aeea81ad8519

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 24, 2021
@sftim sftim moved this from Ready for review to Scheduled in Blog articles Aug 24, 2021
@k8s-ci-robot k8s-ci-robot merged commit 157f1d7 into kubernetes:main Aug 24, 2021
@saschagrunert saschagrunert deleted the seccomp-default-blog branch August 24, 2021 10:15
@sftim sftim moved this from Scheduled to Published in Blog articles Sep 7, 2021
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/node Categorizes an issue or PR as relevant to SIG Node. sig/security Categorizes an issue or PR as relevant to SIG Security. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
No open projects
Blog articles
Published
Development

Successfully merging this pull request may close these issues.

None yet