feat(injector): add list of ignored network interfaces #4700

nojnhuh · 2022-04-27T17:10:15Z

Description:
This change adds a new configurable list of ignored network interface
names. Traffic received from and sent to those interfaces is not
forwarded to the sidecar container by iptables. When this list is empty
(the default), osm-injector produces the exact same iptables commands as
it did before.

The list is configured in the chart at osm.networkInterfaceExclusionList
and in the MeshConfig at spec.traffic.networkInterfaceExclusionList.

Fixes #4546

Testing done:
Updated unit tests

Tested manually e2e

Installed Multus CNI on an AKS cluster: kubectl apply -f https://raw.githubusercontent.com/k8snetworkplumbingwg/multus-cni/master/deployments/multus-daemonset-thick-plugin.yml

Created a new network interface net1:

apiVersion: "k8s.cni.cncf.io/v1"
kind: NetworkAttachmentDefinition
metadata:
  name: macvlan-conf
spec:
  config: '{
      "cniVersion": "0.3.0",
      "type": "macvlan",
      "master": "eth0",
      "mode": "bridge",
      "ipam": {
        "type": "host-local",
        "subnet": "192.168.1.0/24",
        "rangeStart": "192.168.1.200",
        "rangeEnd": "192.168.1.216",
        "routes": [
          { "dst": "0.0.0.0/0" }
        ],
        "gateway": "192.168.1.1"
      }
    }'

Created 2 ubuntu pods from deployments named client and server each running containers defined by:

FROM ubuntu
RUN apt update && apt install -y iproute2 curl python3

apiVersion: apps/v1
kind: Deployment
metadata:
  name: client
spec:
  replicas: 1
  selector:
    matchLabels:
      app: client
  template:
    metadata:
      labels:
        app: client
      annotations:
        k8s.v1.cni.cncf.io/networks: macvlan-conf
    spec:
      nodeName: aks-nodepool2-29838616-vmss000000
      containers:
      - name: client
        command: ["/bin/sh", "-c", "trap : TERM INT; sleep infinity & wait"]
        image: jondev.azurecr.io/nettest

Both pods must be running on the same node

kubectl exec -it deploy/server -c server -- bash -c 'ip a && python3 -m http.server'
- Note the IP address of the net1 interface, e.g. 192.168.1.200, which changes each time the pod is recreated

kubectl exec -it deploy/client -c client -- curl -I 192.168.1.200:8000

  HTTP/1.0 200 OK
  Server: SimpleHTTP/0.6 Python/3.8.10
  Date: Wed, 27 Apr 2022 16:13:02 GMT
  Content-type: text/html; charset=utf-8
  Content-Length: 1018

Installed OSM
osm namespace add default
kubectl rollout restart deploy client server
kubectl exec -it deploy/server -c server -- bash -c 'ip a && python3 -m http.server'

kubectl exec -it deploy/client -c client -- curl -I 192.168.1.200:8000

  curl: (7) Failed to connect to 192.168.1.213 port 8000: Connection refused
  command terminated with exit code 7

kubectl patch meshconfig -n osm-system osm-mesh-config -p '{"spec": {"traffic": {"networkInterfaceExclusionList": ["net1"]}}}' --type=merge
kubectl rollout restart deploy client server
kubectl exec -it deploy/server -c server -- bash -c 'ip a && python3 -m http.server'
- Note the IP address of the net1 interface, e.g. 192.168.1.200

kubectl exec -it deploy/client -c client -- curl -I 192.168.1.200:8000

  HTTP/1.0 200 OK
  Server: SimpleHTTP/0.6 Python/3.8.10
  Date: Wed, 27 Apr 2022 16:13:02 GMT
  Content-type: text/html; charset=utf-8
  Content-Length: 1018

/stats Envoy endpoint on either pod didn't show any of that traffic going through Envoy.

Affected area:

Functional Area
New Functionality	[X]
CI System	[ ]
CLI Tool	[ ]
Certificate Management	[ ]
Control Plane	[ ]
Demo	[ ]
Documentation	[ ]
Egress	[ ]
Ingress	[ ]
Install	[ ]
Networking	[X]
Observability	[ ]
Performance	[ ]
SMI Policy	[ ]
Security	[ ]
Sidecar Injection	[X]
Tests	[ ]
Upgrade	[ ]
Other	[ ]

Please answer the following questions with yes/no.

Does this change contain code from or inspired by another project? No
- Did you notify the maintainers and provide attribution?
Is this a breaking change? No
Has documentation corresponding to this change been updated in the osm-docs repo (if applicable)? Yes, add network interface exclusions osm-docs#361

This change adds a new configurable list of ignored network interface names. Traffic received from and sent to those interfaces is not forwarded to the sidecar container by iptables. When this list is empty (the default), osm-injector produces the exact same iptables commands as it did before. The list is configured in the chart at `osm.networkInterfaceExclusionList` and in the MeshConfig at `spec.traffic.networkInterfaceExclusionList`. Fixes #4546 Signed-off-by: Jon Huhn <johuhn@microsoft.com>

codecov-commenter · 2022-04-27T17:25:09Z

Codecov Report

Merging #4700 (ad9e3e0) into main (d485366) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #4700   +/-   ##
=======================================
  Coverage   69.48%   69.48%           
=======================================
  Files         217      217           
  Lines       15437    15495   +58     
=======================================
+ Hits        10726    10767   +41     
- Misses       4661     4678   +17     
  Partials       50       50

Flag	Coverage Δ
unittests	`69.48% <100.00%> (+<0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
pkg/injector/init_container.go	`100.00% <100.00%> (ø)`
pkg/injector/iptables.go	`100.00% <100.00%> (ø)`
pkg/injector/patch.go	`76.21% <100.00%> (+0.25%)`	⬆️
pkg/certificate/manager.go	`82.30% <0.00%> (-1.54%)`	⬇️
pkg/cli/verifier/envoy_config.go	`66.75% <0.00%> (+0.77%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d485366...ad9e3e0. Read the comment docs.

Signed-off-by: Jon Huhn <johuhn@microsoft.com>

shashankram

Looks good, just 1 blocking comment regarding rule ordering.

shashankram · 2022-04-27T18:21:56Z

pkg/injector/iptables.go

@@ -74,6 +74,11 @@ func generateIptablesCommands(outboundIPRangeExclusionList []string, outboundIPR
 	// 1. Create inbound rules
 	cmds = append(cmds, iptablesInboundStaticRules...)

+	// Ignore inbound traffic on specified interfaces
+	for _, iface := range networkInterfaceExclusionList {
+		cmds = append(cmds, fmt.Sprintf("-I OSM_PROXY_INBOUND -i %s -j RETURN", iface))


What's the reason the inbound rule uses -I (insert before) and outbound rule uses -A (append)?

If ordering is important, kindly document this in the code.

These inbound rules only work when they're added before the static rules, either with -I or by adding them to the slice before with -A. I would need help figuring out an exact reason why that's the case.

Iptable rules are executed in order. So it makes sense to use -I here so that the exclusions get applied first before other rules that can match the traffic. Could you add a comment here and also for -I OSM_PROXY_INBOUND -p tcp --match multiport --dports %s -j RETURN which is missing a comment.

I am looking into the code to see if the ordering of outbound rules is correct.

I narrowed it down to needing these commands at least before the last static rule:

// Redirect remaining inbound traffic to Envoy "-A OSM_PROXY_INBOUND -p tcp -j OSM_PROXY_IN_REDIRECT",

So this is how the rules should be ordered.

INBOUND:
exclusions
inclusions
static rules that don't redirect to proxy
redirect to proxy

OUTBOUND
exclusions
inclusions
status rules that don't redirect to proxy
redirect to proxy

The reason -I is used with inbound is because the redirect to proxy rule: ...-A OSM_PROXY_INBOUND -p tcp -j OSM_PROXY_IN_REDIRECT is a part of the static rule set. While for outbound, redirect to proxy rule: ... -A OSM_PROXY_OUTBOUND -j OSM_PROXY_OUT_REDIRECT is added only after the exclusions/inclusions rules because the redirection rule OSM_PROXY_OUTBOUND -> OSM_PROXY_OUT_REDIRECT is different when inclusions are applied.

Your change looks good, could you add the following comment next to the inbound rule added:

// *Note: it is important to use the insert option '-I' instead of the append option '-A' to ensure the // exclusion of traffic to the network interface happens before the rule that redirects traffic to the proxy

shashankram

Non blocking comment #4700 (comment)

Signed-off-by: Jon Huhn <johuhn@microsoft.com>

…sh#4700) * feat(injector): add list of ignored network interfaces This change adds a new configurable list of ignored network interface names. Traffic received from and sent to those interfaces is not forwarded to the sidecar container by iptables. When this list is empty (the default), osm-injector produces the exact same iptables commands as it did before. The list is configured in the chart at `osm.networkInterfaceExclusionList` and in the MeshConfig at `spec.traffic.networkInterfaceExclusionList`. Fixes openservicemesh#4546 Signed-off-by: Jon Huhn <johuhn@microsoft.com> * codegen Signed-off-by: Jon Huhn <johuhn@microsoft.com> * add comment Signed-off-by: Jon Huhn <johuhn@microsoft.com> Co-authored-by: Shashank Ram <21697719+shashankram@users.noreply.github.com>

nojnhuh requested review from shashankram, snehachhabria, draychev, jaellio and trstringer as code owners April 27, 2022 17:10

codegen

1358077

Signed-off-by: Jon Huhn <johuhn@microsoft.com>

trstringer approved these changes Apr 27, 2022

View reviewed changes

shashankram suggested changes Apr 27, 2022

View reviewed changes

shashankram approved these changes Apr 27, 2022

View reviewed changes

add comment

ad9e3e0

Signed-off-by: Jon Huhn <johuhn@microsoft.com>

nojnhuh merged commit f922b5c into openservicemesh:main Apr 27, 2022

nojnhuh deleted the multi-iface branch April 27, 2022 19:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(injector): add list of ignored network interfaces #4700

feat(injector): add list of ignored network interfaces #4700

Uh oh!

nojnhuh commented Apr 27, 2022

Uh oh!

codecov-commenter commented Apr 27, 2022 •

edited

Loading

Uh oh!

shashankram left a comment

Uh oh!

shashankram Apr 27, 2022

Uh oh!

nojnhuh Apr 27, 2022

Uh oh!

shashankram Apr 27, 2022

Uh oh!

nojnhuh Apr 27, 2022

Uh oh!

shashankram Apr 27, 2022

Uh oh!

shashankram left a comment

Uh oh!

Uh oh!

feat(injector): add list of ignored network interfaces #4700

feat(injector): add list of ignored network interfaces #4700

Uh oh!

Conversation

nojnhuh commented Apr 27, 2022

Uh oh!

codecov-commenter commented Apr 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

shashankram left a comment

Choose a reason for hiding this comment

Uh oh!

shashankram Apr 27, 2022

Choose a reason for hiding this comment

Uh oh!

nojnhuh Apr 27, 2022

Choose a reason for hiding this comment

Uh oh!

shashankram Apr 27, 2022

Choose a reason for hiding this comment

Uh oh!

nojnhuh Apr 27, 2022

Choose a reason for hiding this comment

Uh oh!

shashankram Apr 27, 2022

Choose a reason for hiding this comment

Uh oh!

shashankram left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codecov-commenter commented Apr 27, 2022 •

edited

Loading