Add user stories for ANP Egress Cluster control #86

tssurya · 2023-03-26T14:50:36Z

Relates-to #28

netlify · 2023-03-26T14:50:41Z

✅ Deploy Preview for kubernetes-sigs-network-policy-api ready!

Name	Link
🔨 Latest commit	`05a85d3`
🔍 Latest deploy log	https://app.netlify.com/sites/kubernetes-sigs-network-policy-api/deploys/645941c613cc1f0008cf4cd9
😎 Deploy Preview	https://deploy-preview-86--kubernetes-sigs-network-policy-api.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

k8s-ci-robot · 2023-03-26T14:50:43Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: tssurya
Once this PR has been reviewed and has the lgtm label, please assign dyanngg for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

danwinship · 2023-03-27T14:04:50Z

docs/userstories/v1/egress-control-in-anp/egress-control-cluster-nodes.md

+
+1. Since ANP is a new API, define if our current namespace and pod selectors
+consider host-networked pods or not? Or do we want to allow this discretion to the
+CNI plugins?


IMO we should avoid implementation-defined behavior as much as possible. A policy object should have a single well-defined meaning, which is the same regardless of network plugin.

The host-networked pod case is tricky because: (a) there are good use cases for being able to select host-network pods, but (b) most existing NetworkPolicy implementations are not able to distinguish traffic from different host-network pods so we can't just require it.

One possibility would be to add explicit hostNetworkPodSelector fields in addition to the regular podSelector, and then have clear semantics for rejecting the policy if the plugin can't implement it.

IMO we should avoid implementation-defined behavior as much as possible. A policy object should have a single well-defined meaning, which is the same regardless of network plugin.

+1

The host-networked pod case is tricky because: (a) there are good use cases for being able to select host-network pods, but (b) most existing NetworkPolicy implementations are not able to distinguish traffic from different host-network pods so we can't just require it.
true.

One possibility would be to add explicit hostNetworkPodSelector fields in addition to the regular podSelector, and then have clear semantics for rejecting the policy if the plugin can't implement it.

yeah that's what I had in mind, since we still want to give users the ability to select host-net pods but also want clarity on what the API fields stand for (whether it includes host-net or just regular pods), we could just go with what you said, our regular PodSelector will select only CNI pods, and we introduce a new hostNetworkPodSelector that can be used for specifying host-net pods. NamespacedPeer and NamespacedPodPeer and NamespacedPodSubject can all be modified to include this field.

fixed this up, let me know what you think @danwinship .
/cc @astoycos @Dyanngg

docs/userstories/v1/egress-control-in-anp/egress-control-cluster-nodes.md

docs/userstories/v1/egress-control-in-anp/egress-control-outside-cluster.md

danwinship · 2023-03-27T14:10:23Z

docs/userstories/v1/egress-control-in-anp/egress-control-outside-cluster.md

+2. **As a** cluster administrator **I want** to explicitly allow traffic
+from cluster workloads to handpicked destinations outside the cluster and
+deny the rest **so that** I can guarantee cluster wide egress traffic
+guardrails irrespective of whatever network policies are defined by the


"guardrails" implies "deny" doesn't it? I forget what we call cluster-wide "allow" rules

(Hm... I guess it depends on what the rails are guarding...)

:D maybe I can refactor this bit in a more simpler way if its confusing? What I really meant was the first user story is for deny northbound and second user story is for allow northbound towards explicit destination on top of the deny.

guardrails are deny I guess, we should be using a word that implies opposite of that....

docs/userstories/v1/egress-control-in-anp/egress-control-outside-cluster.md

danwinship · 2023-03-27T14:13:07Z

docs/userstories/v1/egress-control-in-anp/egress-control-outside-cluster.md

+those workloads towards well defined destinations then those workloads should
+not be able to connect to any other external destinations than the explicitly
+allowed ones.
+


I think you need another criterion for the "guardrails" case?

I thought point 2 was the criterion for the "guardrails" case.. :D I changed the word guardrail, ptal the second iteration to see if it makes sense.

docs/userstories/v1/egress-control-in-anp/egress-control-outside-cluster.md

docs/userstories/v1/egress-control-in-anp/egress-control-cluster-nodes.md

tssurya · 2023-04-05T07:41:17Z

@Dyanngg @astoycos @danwinship and anyone interested: PTAL at the new revision and add your thoughts!

docs/userstories/v1/egress-control-in-anp/egress-control-cluster-nodes.md

Dyanngg · 2023-04-26T21:41:55Z

docs/userstories/v1/egress-control-in-anp/egress-control-cluster-nodes.md

+
+1. Clarify that the existing `podSelector` in the ANP API are for regular
+non-hostNetworked pods only. These don't apply to hostNetworked pods.
+2. Add a new well defined optional field called `hostNetworkPodSelector` to both


policy rules will be very tricky to enforce though, since hostnet pods share the same node ip...

hmm I agree with you but on L3/L4 level I am hoping because of how we don't have implicit deny's in ANP users will use this along with the port field to say what they want to allow like 6443 for k8s-server? -> but that's on a peer level... on a subject level you are right.. I need to think if we have strong use cases for host-networked pods being allowed in subject...but it is certainly something to look out for! if accidentally people block everything on this IP, then that does impact the to/from traffic of nodes...

Just to get this down in writing somewhere: it is theoretically possible to distinguish "traffic from hostNetwork pod A", "traffic from hostNetwork pod B", and "traffic from a node process" because each hostNetwork pod has its own cgroup and there are APIs (eg iptables -m cgroup) that can match based on the cgroup of the socket associated with a connection.

AFAIK no one has tried to do this in practice, and the iptables-extensions man page suggests that it only works on outbound packets, not inbound ones (though the nft man page does not have the same warning and actually gives an example using input, so maybe the iptables warning is out of date?)

Just to get this down in writing somewhere: it is theoretically possible to distinguish "traffic from hostNetwork pod A", "traffic from hostNetwork pod B", and "traffic from a node process" because each hostNetwork pod has its own cgroup and there are APIs (eg iptables -m cgroup) that can match based on the cgroup of the socket associated with a connection.

AFAIK no one has tried to do this in practice, and the iptables-extensions man page suggests that it only works on outbound packets, not inbound ones (though the nft man page does not have the same warning and actually gives an example using input, so maybe the iptables warning is out of date?)

thanks @danwinship! I am certainly learning a lot here! So seems like this is "tricky" based on implementations but not impossible to implement in the end...

From API perspective - these are egress stories so normal pods (to)-> host-networked pods OR normal pods (to)-> external entities outside the cluster
However I mentioned adding support for host-networked pods (to)-> normal-pods which makes this ingress use case depending on how we look at it but in the end host-networked pods are in the cluster.. Just wondering if there are asks or stories to add support for host-networked pods in the subject field.

Signed-off-by: Surya Seetharaman <[email protected]>

docs/userstories/egress-control-in-anp/egress-control-cluster-nodes.md

docs/userstories/egress-control-in-anp/egress-control-outside-cluster.md

astoycos · 2023-06-20T14:14:41Z

/close

in favor of #117

k8s-ci-robot · 2023-06-20T14:14:45Z

@astoycos: Closed this PR.

In response to this:

/close

in favor of #117

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.

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Mar 26, 2023

k8s-ci-robot requested review from Dyanngg and rikatz March 26, 2023 14:50

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Mar 26, 2023

This was referenced Mar 26, 2023

reopen the egress policy debate :) #28

Closed

API Graduation Criteria (Alpha to Beta) and Open Questions #87

Open

danwinship reviewed Mar 27, 2023

View reviewed changes

tssurya commented Mar 28, 2023

View reviewed changes

docs/userstories/v1/egress-control-in-anp/egress-control-cluster-nodes.md Outdated Show resolved Hide resolved

tssurya force-pushed the ANP-cluster-egress-control branch from 95ce207 to 544d23b Compare March 28, 2023 14:28

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 28, 2023

k8s-ci-robot requested a review from astoycos March 28, 2023 14:55

Dyanngg reviewed Apr 26, 2023

View reviewed changes

tssurya force-pushed the ANP-cluster-egress-control branch from 544d23b to 5bbdeb4 Compare May 8, 2023 11:20

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels May 8, 2023

Add user stories for ANP Egress Cluster control

05a85d3

Signed-off-by: Surya Seetharaman <[email protected]>

tssurya force-pushed the ANP-cluster-egress-control branch from 5bbdeb4 to 05a85d3 Compare May 8, 2023 18:39

danwinship reviewed May 9, 2023

View reviewed changes

docs/userstories/egress-control-in-anp/egress-control-cluster-nodes.md Show resolved Hide resolved

docs/userstories/egress-control-in-anp/egress-control-cluster-nodes.md Show resolved Hide resolved

docs/userstories/egress-control-in-anp/egress-control-outside-cluster.md Show resolved Hide resolved

tssurya mentioned this pull request Jun 11, 2023

NPEP: Add support for egress traffic control #117

Merged

k8s-ci-robot closed this Jun 20, 2023

tssurya mentioned this pull request Jul 18, 2023

[ENHANCEMENT] Add Northbound traffic support for ANP API #126

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add user stories for ANP Egress Cluster control #86

Add user stories for ANP Egress Cluster control #86

tssurya commented Mar 26, 2023

netlify bot commented Mar 26, 2023 •

edited

Loading

k8s-ci-robot commented Mar 26, 2023

danwinship Mar 27, 2023

tssurya Mar 28, 2023

tssurya Mar 28, 2023 •

edited

Loading

danwinship Mar 27, 2023

tssurya Mar 28, 2023 •

edited

Loading

danwinship Mar 27, 2023

tssurya Mar 28, 2023

tssurya commented Apr 5, 2023 •

edited

Loading

Dyanngg Apr 26, 2023

tssurya Apr 27, 2023 •

edited

Loading

danwinship Apr 28, 2023

tssurya Apr 28, 2023

astoycos commented Jun 20, 2023

k8s-ci-robot commented Jun 20, 2023

Add user stories for ANP Egress Cluster control #86

Add user stories for ANP Egress Cluster control #86

Conversation

tssurya commented Mar 26, 2023

netlify bot commented Mar 26, 2023 • edited Loading

✅ Deploy Preview for kubernetes-sigs-network-policy-api ready!

k8s-ci-robot commented Mar 26, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tssurya Mar 28, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tssurya Mar 28, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tssurya commented Apr 5, 2023 • edited Loading

Choose a reason for hiding this comment

tssurya Apr 27, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

astoycos commented Jun 20, 2023

k8s-ci-robot commented Jun 20, 2023

netlify bot commented Mar 26, 2023 •

edited

Loading

tssurya Mar 28, 2023 •

edited

Loading

tssurya Mar 28, 2023 •

edited

Loading

tssurya commented Apr 5, 2023 •

edited

Loading

tssurya Apr 27, 2023 •

edited

Loading