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

Skip injection for host network pods #48

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

wkgcass
Copy link

@wkgcass wkgcass commented Aug 22, 2020

What and why?

Provide the ability to skip the injection for host network pods.

Some sidecars will run ip rule, ip route and iptables to manipulate traffic to/from the sidecar. This might be dangerous if the pod is running hostNetwork. It would be great if the injector configuration can be configured to skip the injection for host network pods, just like Istio.

Testing Steps

  • Added unit tests for this feature (make test)

Reviewers

Required reviewers: @byxorna
Request reviews from other people you want to review this PR in the "Reviewers" section on the right.

⚠️ this PR must have at least 2 thumbs from the MAINTAINERS.md of the project before merging!

@komapa komapa requested a review from a team August 23, 2020 02:53
Copy link

@vrrs vrrs left a comment

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

@byxorna byxorna left a comment

Choose a reason for hiding this comment

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

Thanks, this looks great! Also, props on unit tests :)

@@ -531,6 +531,14 @@ func (whsvr *WebhookServer) mutate(req *v1beta1.AdmissionRequest) *v1beta1.Admis
}
}

if injectionConfig.SkipHostNetwork && pod.Spec.HostNetwork {
glog.Infof("Injection skipped by SkipHostNetwork")
Copy link
Contributor

Choose a reason for hiding this comment

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

[nit] maybe a more expressive message like "injection skipped due to pod using HostNetwork, and SkipHostNetwork is enabled for this sidecar"?

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for your review! I've pushed a new commit which prints readable skipping reason, as well as the ns/name of the pod and the name of the sidecar config.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants