aws / aws-network-policy-agent

Apache License 2.0
42 stars 25 forks source link

Support for network policies applied to pods with '.' in their pod name #225

Closed zachdorame closed 3 months ago

zachdorame commented 5 months ago

Issue #, if available: https://github.com/aws/aws-network-policy-agent/issues/118

Description of changes: Pods with . in their pod name will now have . changed to _.

Testing: I applied Sample Deployment and Sample Policy from this comment: https://github.com/aws/aws-network-policy-agent/issues/118#issue-1967965139 to a cluster. With this change, I observed that the network policy reconciler was no longer crashing and that Sample Policy was being successfully applied:

network-policy-agent.log:

{"level":"info","ts":"2024-03-04T21:15:37.487Z","logger":"controllers.policyEndpoints","caller":"controller/controller.go:316","msg":"Received a new reconcile request","req":{"name":"deny-all-ingress-gtr69","namespace":"default"}}
{"level":"info","ts":"2024-03-04T21:15:37.487Z","logger":"controllers.policyEndpoints","caller":"controllers/policyendpoints_controller.go:127","msg":"Processing Policy Endpoint  ","Name: ":"deny-all-ingress-gtr69","Namespace ":"default"}
{"level":"info","ts":"2024-03-04T21:15:37.487Z","logger":"controllers.policyEndpoints","caller":"controllers/policyendpoints_controller.go:146","msg":"Parent NP resource:","Name: ":"deny-all-ingress"}
{"level":"info","ts":"2024-03-04T21:15:37.487Z","logger":"controllers.policyEndpoints","caller":"controllers/policyendpoints_controller.go:211","msg":"Found another PE resource for the parent NP","name":"deny-all-ingress-gtr69"}
{"level":"info","ts":"2024-03-04T21:15:37.487Z","logger":"controllers.policyEndpoints","caller":"controllers/policyendpoints_controller.go:146","msg":"Total PEs for Parent NP:","Count: ":1}
{"level":"info","ts":"2024-03-04T21:15:37.487Z","logger":"controllers.policyEndpoints","caller":"controllers/policyendpoints_controller.go:146","msg":"Derive PE Object ","Name ":"deny-all-ingress-gtr69"}
{"level":"info","ts":"2024-03-04T21:15:37.487Z","logger":"controllers.policyEndpoints","caller":"controllers/policyendpoints_controller.go:146","msg":"Processing PE ","Name ":"deny-all-ingress-gtr69"}
{"level":"info","ts":"2024-03-04T21:15:37.487Z","logger":"controllers.policyEndpoints","caller":"controllers/policyendpoints_controller.go:211","msg":"Found a matching Pod: ","name: ":"sample-app.app-7c547489fb-h72pk","namespace: ":"default"}
{"level":"info","ts":"2024-03-04T21:15:37.487Z","logger":"controllers.policyEndpoints","caller":"controllers/policyendpoints_controller.go:211","msg":"Derived ","Pod identifier: ":"sample-appdotapp-7c547489fb-default"}
{"level":"info","ts":"2024-03-04T21:15:37.487Z","logger":"controllers.policyEndpoints","caller":"controllers/policyendpoints_controller.go:466","msg":"Current PE Count for Parent NP:","Count: ":1}
{"level":"info","ts":"2024-03-04T21:15:37.487Z","logger":"controllers.policyEndpoints","caller":"controllers/policyendpoints_controller.go:146","msg":"Total number of PolicyEndpoint resources for","podIdentifier ":"sample-appdotapp-7c547489fb-default"," are ":1}
{"level":"info","ts":"2024-03-04T21:15:37.487Z","logger":"controllers.policyEndpoints","caller":"controllers/policyendpoints_controller.go:146","msg":"Deriving Firewall rules for PolicyEndpoint:","Name: ":"deny-all-ingress-gtr69"}
{"level":"info","ts":"2024-03-04T21:15:37.487Z","logger":"controllers.policyEndpoints","caller":"controllers/policyendpoints_controller.go:146","msg":"Total no.of - ","ingressRules":0,"egressRules":0}
{"level":"info","ts":"2024-03-04T21:15:37.487Z","logger":"controllers.policyEndpoints","caller":"controllers/policyendpoints_controller.go:223","msg":"Default Deny enabled on Ingress"}
{"level":"info","ts":"2024-03-04T21:15:37.487Z","logger":"controllers.policyEndpoints","caller":"controllers/policyendpoints_controller.go:127","msg":"No Egress rules and no egress isolation - Appending catch allentry"}
{"level":"info","ts":"2024-03-04T21:15:37.487Z","logger":"controllers.policyEndpoints","caller":"controllers/policyendpoints_controller.go:146","msg":"Processing Pod: ","name:":"sample-app.app-7c547489fb-h72pk","namespace:":"default","podIdentifier: ":"sample-appdotapp-7c547489fb-default"}
{"level":"info","ts":"2024-03-04T21:15:37.487Z","logger":"ebpf-client","caller":"controllers/policyendpoints_controller.go:243","msg":"AttacheBPFProbes for","pod":"sample-app.app-7c547489fb-h72pk"," in namespace":"default"," with hostVethName":"enif41a4bc0fa2"}
{"level":"info","ts":"2024-03-04T21:15:37.487Z","logger":"ebpf-client","caller":"controllers/policyendpoints_controller.go:270","msg":"Found an existing instance, let's derive the ingress context.."}
{"level":"info","ts":"2024-03-04T21:15:37.487Z","logger":"ebpf-client","caller":"controllers/policyendpoints_controller.go:270","msg":"Attempting to do an Ingress Attach ","with progFD: ":14}
{"level":"info","ts":"2024-03-04T21:15:37.509Z","logger":"ebpf-client","caller":"controllers/policyendpoints_controller.go:243","msg":"Successfully attached Ingress TC probe for","pod: ":"sample-app.app-7c547489fb-h72pk"," in namespace":"default"}
{"level":"info","ts":"2024-03-04T21:15:37.509Z","logger":"ebpf-client","caller":"controllers/policyendpoints_controller.go:270","msg":"Found an existing instance, let's derive the egress context.."}
{"level":"info","ts":"2024-03-04T21:15:37.509Z","logger":"ebpf-client","caller":"controllers/policyendpoints_controller.go:270","msg":"Attempting to do an Egress Attach ","with progFD: ":13}
{"level":"info","ts":"2024-03-04T21:15:37.525Z","logger":"ebpf-client","caller":"controllers/policyendpoints_controller.go:243","msg":"Successfully attached Egress TC probe for","pod: ":"sample-app.app-7c547489fb-h72pk"," in namespace":"default"}
{"level":"info","ts":"2024-03-04T21:15:37.525Z","logger":"controllers.policyEndpoints","caller":"controllers/policyendpoints_controller.go:146","msg":"Successfully attached required eBPF probes for","pod:":"sample-app.app-7c547489fb-h72pk","in namespace":"default"}
{"level":"info","ts":"2024-03-04T21:15:37.525Z","logger":"ebpf-client","caller":"controllers/policyendpoints_controller.go:278","msg":"Pod has an Ingress hook attached. Update the corresponding map","progFD: ":14,"mapName: ":"ingress_map"}
{"level":"info","ts":"2024-03-04T21:15:37.525Z","logger":"ebpf-client","caller":"ebpf/bpf_client.go:707","msg":"L4 values: ","protocol: ":254,"startPort: ":0,"endPort: ":0}
{"level":"info","ts":"2024-03-04T21:15:37.525Z","logger":"ebpf-client","caller":"ebpf/bpf_client.go:707","msg":"Total L4 entry count for catch all entry: ","count: ":0}
{"level":"info","ts":"2024-03-04T21:15:37.525Z","logger":"ebpf-client","caller":"controllers/policyendpoints_controller.go:436","msg":"ID of map to update: ","ID: ":98}
{"level":"info","ts":"2024-03-04T21:15:37.525Z","logger":"ebpf-client","caller":"controllers/policyendpoints_controller.go:278","msg":"Pod has an Egress hook attached. Update the corresponding map","progFD: ":13,"mapName: ":"egress_map"}
{"level":"info","ts":"2024-03-04T21:15:37.525Z","logger":"ebpf-client","caller":"ebpf/bpf_client.go:707","msg":"L4 values: ","protocol: ":254,"startPort: ":0,"endPort: ":0}
{"level":"info","ts":"2024-03-04T21:15:37.525Z","logger":"ebpf-client","caller":"ebpf/bpf_client.go:707","msg":"Current L4 entry count for catch all entry: ","count: ":0}
{"level":"info","ts":"2024-03-04T21:15:37.525Z","logger":"ebpf-client","caller":"ebpf/bpf_client.go:707","msg":"Total L4 entry count for catch all entry: ","count: ":0}
{"level":"info","ts":"2024-03-04T21:15:37.525Z","logger":"ebpf-client","caller":"ebpf/bpf_client.go:707","msg":"L4 values: ","protocol: ":254,"startPort: ":0,"endPort: ":0}
{"level":"info","ts":"2024-03-04T21:15:37.525Z","logger":"ebpf-client","caller":"controllers/policyendpoints_controller.go:436","msg":"ID of map to update: ","ID: ":99}

Cyclonus tests also passed

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

zachdorame commented 5 months ago

Regardless of how we replace the "." character, we want to make sure the controller has no issues when the name of the pod is changed. Did you try creating and deleting on the pod on a real cluster? And applying and removing network policies to the pod?

I verified manually on a real cluster that I can create a pod my.pod.with.dots and that a network policy can be successfully applied and revoked. But, as part of my next revision, I've also added an integration test that performs this validation as well

mjnovice commented 3 months ago

Hye @zachdorame anything blocking this PR ? Would be happy to help if needed 🙂