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

Fix network policy in local setup in conjunction with HA VPN. #8370

Conversation

ScheererJ
Copy link
Member

How to categorize this PR?

/area networking
/area dev-productivity
/kind bug

What this PR does / why we need it:
Fix network policy in local setup in conjunction with HA VPN.

Previously, the network policy specifying the allowed traffic to the machine pods in the local setup only listed other machine pods and vpn-seed-server as allowed ingress sources. However, in the HA VPN case kube-apiserver connects to machines as well. The connection to kubelet was allowed, but everything else was blocked by policy. This meant that kubectl proxy would not allow proxying traffic to pods in the host network due to network policy. This change adapts the network policy to work in both VPN cases and use the more general to-shoot-networks labels, which were already used correctly.

Which issue(s) this PR fixes:
None.

Special notes for your reviewer:

Release note:

`kubectl proxy` now works as expected in the local development setup in conjunction with highly available vpn

/cc @istvanballok @MartinWeindel

Previously, the network policy specifying the allowed traffic to the machine
pods in the local setup only listed other machine pods and `vpn-seed-server`
as allowed ingress sources. However, in the HA VPN case `kube-apiserver`
connects to machines as well. The connection to kubelet was allowed, but
everything else was blocked by policy. This meant that `kubectl proxy` would
not allow proxying traffic to pods in the host network due to network policy.
This change adapts the network policy to work in both VPN cases and use the
more general `to-shoot-networks` labels, which were already used correctly.
@gardener-prow gardener-prow bot added area/networking Networking related area/dev-productivity Developer productivity related (how to improve development) kind/bug Bug size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. cla: no Indicates the PR's author has not signed the cla-assistant.io CLA. cla: yes Indicates the PR's author has signed the cla-assistant.io CLA. and removed cla: no Indicates the PR's author has not signed the cla-assistant.io CLA. labels Aug 18, 2023
@ScheererJ
Copy link
Member Author

Issues reaching github.com in tests:
/retest

Copy link
Member

@istvanballok istvanballok left a comment

Choose a reason for hiding this comment

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

Thank you! 🎉 I could confirm that it works in my local setup!

/lgtm

@gardener-prow gardener-prow bot added the lgtm Indicates that a PR is ready to be merged. label Aug 18, 2023
@gardener-prow
Copy link
Contributor

gardener-prow bot commented Aug 18, 2023

LGTM label has been added.

Git tree hash: 87ca83273624d56dc2c0f22863f72126fea1a92a

@gardener-prow
Copy link
Contributor

gardener-prow bot commented Aug 18, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: acumino

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

@gardener-prow gardener-prow bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 18, 2023
@acumino
Copy link
Member

acumino commented Aug 18, 2023

/retest

1 similar comment
@ScheererJ
Copy link
Member Author

/retest

@ScheererJ
Copy link
Member Author

Test cluster should be fixed now.
/retest

@gardener-prow gardener-prow bot merged commit 33d1e0d into gardener:master Aug 18, 2023
1 check passed
briantopping pushed a commit to briantopping/gardener that referenced this pull request Aug 22, 2023
…er#8370)

Previously, the network policy specifying the allowed traffic to the machine
pods in the local setup only listed other machine pods and `vpn-seed-server`
as allowed ingress sources. However, in the HA VPN case `kube-apiserver`
connects to machines as well. The connection to kubelet was allowed, but
everything else was blocked by policy. This meant that `kubectl proxy` would
not allow proxying traffic to pods in the host network due to network policy.
This change adapts the network policy to work in both VPN cases and use the
more general `to-shoot-networks` labels, which were already used correctly.
nickytd pushed a commit to nickytd/gardener that referenced this pull request Sep 11, 2023
…er#8370)

Previously, the network policy specifying the allowed traffic to the machine
pods in the local setup only listed other machine pods and `vpn-seed-server`
as allowed ingress sources. However, in the HA VPN case `kube-apiserver`
connects to machines as well. The connection to kubelet was allowed, but
everything else was blocked by policy. This meant that `kubectl proxy` would
not allow proxying traffic to pods in the host network due to network policy.
This change adapts the network policy to work in both VPN cases and use the
more general `to-shoot-networks` labels, which were already used correctly.
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/dev-productivity Developer productivity related (how to improve development) area/networking Networking related cla: yes Indicates the PR's author has signed the cla-assistant.io CLA. kind/bug Bug lgtm Indicates that a PR is ready to be merged. 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