Skip to content

Conversation

@zhuchenwang
Copy link
Contributor

Signed-off-by: Zhuchen Wang [email protected]

Fix #7442

@k8s-ci-robot
Copy link

Hi @zhuchenwang. Thanks for your PR.

I'm waiting for a containerd member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@samuelkarp samuelkarp added status/needs-discussion Needs discussion and decision from maintainers impact/breaking labels Oct 11, 2022
@AkihiroSuda AkihiroSuda requested a review from thaJeztah October 12, 2022 02:02
Copy link
Member

@kzys kzys left a comment

Choose a reason for hiding this comment

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

I'm fine blocking the socket call with AF_VSOCK. Most people wouldn't need that.

I'm not too sure about the value of backporting since this is technically a breaking change.

@samuelkarp
Copy link
Member

I'm not too sure about the value of backporting since this is technically a breaking change.

I'm hesitant about this too, especially with 1.6 moving to a long term stable status.

@samuelkarp
Copy link
Member

/ok-to-test

@zhuchenwang
Copy link
Contributor Author

I think it's okay to not backport this change. The KubeVirt feature will be behind a feature gate. For vendors who want to use the feature, they can backport this change to their own containerd build.

@dmcgowan dmcgowan added this to the 1.7 milestone Oct 13, 2022
@dmcgowan dmcgowan removed the status/needs-discussion Needs discussion and decision from maintainers label Oct 13, 2022
@dmcgowan
Copy link
Member

Results of discussion in community meeting: this one is OK for 1.7 but too risky for 1.6. If needed for 1.6 it can be manually backported by users or we can add a hardcoded workaround for Kubevirt via runtime handlers.

@gaby
Copy link

gaby commented Dec 17, 2022

I think it's okay to not backport this change. The KubeVirt feature will be behind a feature gate. For vendors who want to use the feature, they can backport this change to their own containerd build.

I wish Docker had done the same, instead they backported this into a patch release breaking customera systems without notice.

Even worst, the only way we found to enable these was to do an emergency deployment of a seccomp profile.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Blocking the socket syscall for AF_VSOCK in the default seccomp profile

7 participants