Skip to content

Conversation

@KentaTada
Copy link

seccomp has already been GA since Kubernetes 1.19.
I think it is essential for the current Kubernetes, but could you give me your opinion if you have any concern about adding the seccomp check?

Signed-off-by: Kenta Tada Kenta.Tada@sony.com

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. labels Apr 12, 2021
@k8s-ci-robot k8s-ci-robot added sig/node Categorizes an issue or PR as relevant to SIG Node. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Apr 12, 2021
{Name: "NETFILTER_XT_MATCH_COMMENT"},
{Name: "FAIR_GROUP_SCHED"},
{Name: "SECCOMP"},
{Name: "SECCOMP_FILTER"},
Copy link
Member

@neolit123 neolit123 Apr 12, 2021

Choose a reason for hiding this comment

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

hi, given we are adding this to "required", do we know if there are any popular distributions used for k8s that have this config explicitly disabled (i.e. the options are missing in the config)?

Copy link
Author

Choose a reason for hiding this comment

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

I don't have the list of kernel configs of popular distributions although I have never heard to change the kernel config of seccomp from y to n.
I think some embedded systems may change the default to disable seccomp to reduce the impact of performance.

At first, I thought seccomp was one of requirements in the current Kubernetes.
But SeccompProfile is the optional currently and Kubernetes can launch the container without seccomp at least.
As of now, I think it is ok to change this check from Required to Optional.

Copy link
Author

Choose a reason for hiding this comment

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

In addition, the seccomp e2e test is not included in the conformance check.
I'm sorry I should check it at first.

Copy link
Author

Choose a reason for hiding this comment

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

I updated this check from Required to Optional.

Signed-off-by: Kenta Tada <Kenta.Tada@sony.com>
@KentaTada KentaTada force-pushed the add-seccomp-kernel-configs branch from e91fdf8 to c9ffc95 Compare April 13, 2021 04:13
@neolit123
Copy link
Member

neolit123 commented Apr 13, 2021 via email

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

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: KentaTada, neolit123

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 Apr 13, 2021
@k8s-ci-robot k8s-ci-robot merged commit 43713be into kubernetes:master Apr 13, 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. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. sig/node Categorizes an issue or PR as relevant to SIG Node. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants