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.

Sign up for GitHub

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

Jump to bottom

Graduate SeccompDefault feature to stable / GA #115719

Merged
merged 1 commit into from
Feb 20, 2023
Merged

Graduate SeccompDefault feature to stable / GA #115719

merged 1 commit into from
Feb 20, 2023
+4 −14

Conversation

saschagrunert
Copy link
Member

@saschagrunert saschagrunert commented Feb 13, 2023 •
edited
Loading

What type of PR is this?

/kind feature

What this PR does / why we need it:

Updating the required code and docs for SeccompDefault to go GA, which now means that we enable the feature per default.

Which issue(s) this PR fixes:

Refers to kubernetes/enhancements#2413

Special notes for your reviewer:

cc @kubernetes/sig-node-api-reviews

Does this PR introduce a user-facing change?

Graduated seccomp profile defaulting to GA.

Set the kubelet `--seccomp-default` flag or `seccompDefault` kubelet configuration field to `true` to make pods on that node default to using the `RuntimeDefault` seccomp profile.

Enabling seccomp for your workload can have a negative performance impact depending on the kernel and container runtime version in use.

Guidance for identifying and mitigating those issues is outlined in the Kubernetes [seccomp tutorial](https://k8s.io/docs/tutorials/security/seccomp).

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

KEP: https://github.com/kubernetes/enhancements/tree/05f81b8/keps/sig-node/2413-seccomp-by-default

@k8s-ci-robot k8s-ci-robot added release-note-action-required Denotes a PR that introduces potentially breaking changes that require user action. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. kind/feature Categorizes issue or PR as related to a new feature. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Feb 13, 2023
@saschagrunert saschagrunert mentioned this pull request Feb 13, 2023
Closed
12 tasks
@k8s-ci-robot k8s-ci-robot added area/code-generation area/kubelet kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. sig/node Categorizes an issue or PR as relevant to SIG Node. and removed do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Feb 13, 2023
@saschagrunert
Copy link
Member Author

/priority important-soon

@k8s-ci-robot k8s-ci-robot added priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. and removed needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Feb 13, 2023
@saschagrunert saschagrunert force-pushed the seccomp-default-ga branch 2 times, most recently from 62e79a0 to 0f7a5cf Compare February 13, 2023 10:09
@k8s-ci-robot k8s-ci-robot added area/test sig/testing Categorizes an issue or PR as relevant to SIG Testing. labels Feb 13, 2023
@k8s-triage-robot
Copy link

This PR may require API review.

If so, when the changes are ready, complete the pre-review checklist and request an API review.

Status of requested reviews is tracked in the API Review project.

@saschagrunert
Copy link
Member Author

/label api-review

@k8s-ci-robot k8s-ci-robot added the api-review Categorizes an issue or PR as actively needing an API review. label Feb 13, 2023
@saschagrunert
Copy link
Member Author

/test pull-kubernetes-e2e-gce

@bart0sh
Copy link
Contributor

bart0sh commented Feb 13, 2023

/triage accepted

@k8s-ci-robot k8s-ci-robot added the triage/accepted Indicates an issue or PR is ready to be actively worked on. label Feb 13, 2023
@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-action-required Denotes a PR that introduces potentially breaking changes that require user action. labels Feb 15, 2023
@saschagrunert
Graduate SeccompDefault feature to stable / GA
37caed0 
Updating the required code and docs for SeccompDefault to go GA, which
now means that we enable the feature per default.

Signed-off-by: Sascha Grunert <sgrunert@redhat.com>
@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Feb 15, 2023
@liggitt
Copy link
Member

liggitt commented Feb 15, 2023

/approve
for kubelet config API changes

will leave lgtm to node review

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: liggitt, saschagrunert, SergeyKanzhelev

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 Feb 15, 2023
@saschagrunert
Copy link
Member Author

/test pull-kubernetes-unit

@dims
Copy link
Member

dims commented Feb 16, 2023

this PR by itself LGTM. folks have pointed out that the KEP text needs to match this PR. I am ok either to land this first and then update the KEP or the other way around.

@SergeyKanzhelev
Copy link
Member

@saschagrunert would you please also address the todo comment that is relatively related:

kubernetes/pkg/kubelet/kuberuntime/kuberuntime_sandbox.go

Lines 171 to 174 in e55f2a9

// TODO: Deprecated, remove after we switch to Seccomp field
// Forcing sandbox to run as `runtime/default` allow users to
// use least privileged seccomp profiles at pod level. Issue #84623
SeccompProfilePath: v1.SeccompProfileRuntimeDefault,

this PR is lgtm

/lgtm
/hold

(hold if you will decide to address TODO as part of this PR, feel free to unhold any moment)

@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. lgtm "Looks good to me", indicates that a PR is ready to be merged. labels Feb 17, 2023
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: ba30bddf1e5c89c8d9af6ae22b4bba778159069f

@saschagrunert
Copy link
Member Author

saschagrunert commented Feb 20, 2023 •
edited
Loading

@SergeyKanzhelev I'm going to follow-up on the TODO, thank you for the hint!

/unhold

See #115898

@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 Feb 20, 2023
@k8s-ci-robot k8s-ci-robot merged commit 9e356a4 into kubernetes:master Feb 20, 2023
@k8s-ci-robot k8s-ci-robot added this to the v1.27 milestone Feb 20, 2023
@saschagrunert saschagrunert deleted the seccomp-default-ga branch February 20, 2023 09:17
@saschagrunert saschagrunert mentioned this pull request Feb 20, 2023
Merged
@sftim
Copy link
Contributor

sftim commented Feb 21, 2023

My suggestion for a changelog entry (we can revise these after merge):

Graduated seccomp profile defaulting to GA.

Set the kubelet `--seccomp-default` flag or `seccompDefault` kubelet configuration field to `true` to make
pods on that node default to using the `RuntimeDefault` seccomp profile.

Enabling seccomp for your workload can have a negative performance impact depending on the kernel and
container runtime version in use.
Guidance for identifying and mitigating those issues is outlined in the Kubernetes
[seccomp tutorial](https://k8s.io/docs/tutorials/security/seccomp).

Should we also track work to update related docs, such as https://kubernetes.io/docs/concepts/security/security-checklist/?

@saschagrunert
Copy link
Member Author

saschagrunert commented Feb 22, 2023 •
edited
Loading

My suggestion for a changelog entry (we can revise these after merge):
…
Should we also track work to update related docs, such as https://kubernetes.io/docs/concepts/security/security-checklist/?

Thanks, changed the release notes as suggested 👍

Yes, docs updates will be part of the usual enhancement process and I'm following-up on them in the next weeks.

@cbandy cbandy mentioned this pull request Apr 26, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bart0sh bart0sh bart0sh left review comments

@liggitt liggitt liggitt left review comments

@dchen1107 dchen1107 Awaiting requested review from dchen1107

@roycaihw roycaihw Awaiting requested review from roycaihw

@derekwaynecarr derekwaynecarr Awaiting requested review from derekwaynecarr

@dims dims Awaiting requested review from dims

@mrunalp mrunalp Awaiting requested review from mrunalp

@SergeyKanzhelev SergeyKanzhelev Awaiting requested review from SergeyKanzhelev

Assignees

@dims dims

@mrunalp mrunalp

@derekwaynecarr derekwaynecarr

@dchen1107 dchen1107

@SergeyKanzhelev SergeyKanzhelev

Labels
api-review Categorizes an issue or PR as actively needing an API review. approved Indicates a PR has been approved by an approver from all required OWNERS files. area/code-generation area/e2e-test-framework Issues or PRs related to refactoring the kubernetes e2e test framework area/kubelet area/test cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API kind/feature Categorizes issue or PR as related to a new feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. sig/node Categorizes an issue or PR as relevant to SIG Node. sig/storage Categorizes an issue or PR as relevant to SIG Storage. sig/testing Categorizes an issue or PR as relevant to SIG Testing. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
API Reviews
Status: API review completed, 1.27
SIG Node CI/Test Board
Archived in project
SIG Node: code and documentation PRs
Archived in project
Milestone
v1.27
Development

Successfully merging this pull request may close these issues.