fix: NetworkPolicies of Control-Planes #1701
Conversation
✅ Deploy Preview for vcluster-docs ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
chart/templates/networkpolicy.yaml
Outdated
| - namespaceSelector: | ||
| matchLabels: | ||
| kubernetes.io/metadata.name: 'kube-system' | ||
| podSelector: | ||
| - podSelector: |
There was a problem hiding this comment.
Is this necessary? Since vCluster should only communicate with pods in the namespace kube-system and the labels k8s-app=kube-dns, this would change the logic to an OR and then basically allow communication with all pods in the kube-system namespace.
There was a problem hiding this comment.
OK. I misunderstand the grammar of NetworkPolicyEgressRule. Just revert back
| - port: 53 | ||
| protocol: UDP | ||
| - port: 53 | ||
| protocol: TCP |
There was a problem hiding this comment.
Since we already have the rule below that selects DNS in the kube-system namespace, why is this still required?
There was a problem hiding this comment.
The previous NetworkPolicy vc-work-{{ .Release.Name }} will work on workload node based on its podSelector:
podSelector:
matchLabels:
vcluster.loft.sh/managed-by: {{ .Release.Name }}
And, the control-plane of a vcluster still need to communicate host's kube-dns. That is why we want add this rule. And it did crash without this rule.
Meanwhile, the original version of first podSelector: {}, is too loose. We only need vcluster's control-plane to communicate necessary nodes, like:
- podSelector:
matchLabels:
release: {{ .Release.Name }}
chart/templates/networkpolicy.yaml
Outdated
| @@ -30,7 +30,7 @@ spec: | |||
| - ports: | |||
| - port: 443 | |||
| - port: 8443 | |||
| to: | |||
| - to: | |||
There was a problem hiding this comment.
This changes logic from an and to an or, why is this required? This basically allows communication with all pods on port 443 and 8443 as well as all ports on the control-plane
There was a problem hiding this comment.
Same as previous one. I misunderstand the grammar of NetworkPolicyEgressRule. Just revert back
|
@leoncamel thanks for your PR! I posted a couple of comments, would be great if you could take a look. |
|
@FabianKramm I just revert two typos, and explain the main idea about adding udp/53 rule for control-plane. |
|
@FabianKramm Any comment on this? |
What issue type does this pull request address? (keep at least one, remove the others)
/kind bugfix
What does this pull request do? Which issues does it resolve? (use
resolves #<issue_number>if possible)resolves #
This pull request will fix networkpolicies of control-planes. Since from v0.19.x, the control-plane will pack into single
pod. The issue is not obvious.
Please provide a short message that should be published in the vcluster release notes
We encounter a issue with v0.18.x + k8s. After some investigation from our team, we found
the rule to system's host-dns is missing.
And we look into several version, we think the networkpolicies should be fixed.
What else do we need to know?