From 66b268eb135bc46fa2f748a996eb89a8653532d8 Mon Sep 17 00:00:00 2001 From: Daichi Sakaue Date: Mon, 9 Dec 2024 15:34:43 +0900 Subject: [PATCH 1/3] Implement reach command Signed-off-by: Daichi Sakaue --- cmd/npv/app/reach.go | 159 ++++++++++++++++++++++++++++++++++ e2e/Makefile | 1 + e2e/id_test.go | 1 + e2e/inspect_test.go | 6 ++ e2e/list_test.go | 6 ++ e2e/reach_test.go | 96 ++++++++++++++++++++ e2e/suite_test.go | 1 + e2e/summary_test.go | 1 + e2e/testdata/policy/README.md | 1 + e2e/testdata/policy/l4.yaml | 15 ++++ 10 files changed, 287 insertions(+) create mode 100644 cmd/npv/app/reach.go create mode 100644 e2e/reach_test.go diff --git a/cmd/npv/app/reach.go b/cmd/npv/app/reach.go new file mode 100644 index 0000000..39a648d --- /dev/null +++ b/cmd/npv/app/reach.go @@ -0,0 +1,159 @@ +package app + +import ( + "context" + "errors" + "io" + "strconv" + + "github.com/cilium/cilium/pkg/u8proto" + "github.com/spf13/cobra" +) + +var reachOptions struct { + from string + to string +} + +func init() { + reachCmd.Flags().StringVar(&reachOptions.from, "from", "", "egress pod") + reachCmd.Flags().StringVar(&reachOptions.to, "to", "", "ingress pod") + reachCmd.RegisterFlagCompletionFunc("from", completeNamespacePods) + reachCmd.RegisterFlagCompletionFunc("to", completeNamespacePods) + rootCmd.AddCommand(reachCmd) +} + +var reachCmd = &cobra.Command{ + Use: "reach", + Short: "List traffic policies between pod pair", + Long: `List traffic policies between pod pair`, + + Args: cobra.ExactArgs(0), + RunE: func(cmd *cobra.Command, args []string) error { + return runReach(context.Background(), cmd.OutOrStdout()) + }, +} + +type reachEntry struct { + Namespace string `json:"namespace"` + Name string `json:"name"` + Direction string `json:"direction"` + Policy string `json:"policy"` + Identity int `json:"identity"` + WildcardProtocol bool `json:"wildcard_protocol"` + WildcardPort bool `json:"wildcard_port"` + Protocol int `json:"protocol"` + Port int `json:"port"` + Bytes int `json:"bytes"` + Packets int `json:"packets"` +} + +func runReach(ctx context.Context, w io.Writer) error { + if reachOptions.from == "" || reachOptions.to == "" { + return errors.New("--from and --to options are required") + } + + from, err := parseNamespacedName(reachOptions.from) + if err != nil { + return errors.New("--from and --to should be specified as NAMESPACE/POD") + } + + to, err := parseNamespacedName(reachOptions.to) + if err != nil { + return errors.New("--from and --to should be specified as NAMESPACE/POD") + } + + clientset, dynamicClient, err := createK8sClients() + if err != nil { + return err + } + + fromIdentity, err := getPodIdentity(ctx, dynamicClient, from.Namespace, from.Name) + if err != nil { + return err + } + + toIdentity, err := getPodIdentity(ctx, dynamicClient, to.Namespace, to.Name) + if err != nil { + return err + } + + fromPolicies, err := queryPolicyMap(ctx, clientset, dynamicClient, from.Namespace, from.Name) + if err != nil { + return err + } + + toPolicies, err := queryPolicyMap(ctx, clientset, dynamicClient, to.Namespace, to.Name) + if err != nil { + return err + } + + arr := make([]reachEntry, 0) + for _, p := range fromPolicies { + if (p.Key.Identity != 0) && (p.Key.Identity != int(toIdentity)) { + continue + } + if !p.IsEgressRule() { + continue + } + var entry reachEntry + entry.Namespace = from.Namespace + entry.Name = from.Name + entry.Direction = directionEgress + if p.IsDenyRule() { + entry.Policy = policyDeny + } else { + entry.Policy = policyAllow + } + entry.Identity = p.Key.Identity + entry.WildcardProtocol = p.IsWildcardProtocol() + entry.WildcardPort = p.IsWildcardPort() + entry.Protocol = p.Key.Protocol + entry.Port = p.Key.Port() + entry.Bytes = p.Bytes + entry.Packets = p.Packets + arr = append(arr, entry) + } + for _, p := range toPolicies { + if (p.Key.Identity != 0) && (p.Key.Identity != int(fromIdentity)) { + continue + } + if p.IsEgressRule() { + continue + } + var entry reachEntry + entry.Namespace = to.Namespace + entry.Name = to.Name + entry.Direction = directionIngress + if p.IsDenyRule() { + entry.Policy = policyDeny + } else { + entry.Policy = policyAllow + } + entry.Identity = p.Key.Identity + entry.WildcardProtocol = p.IsWildcardProtocol() + entry.WildcardPort = p.IsWildcardPort() + entry.Protocol = p.Key.Protocol + entry.Port = p.Key.Port() + entry.Bytes = p.Bytes + entry.Packets = p.Packets + arr = append(arr, entry) + } + + header := []string{"NAMESPACE", "NAME", "DIRECTION", "POLICY", "IDENTITY", "PROTOCOL", "PORT", "BYTES", "PACKETS"} + return writeSimpleOrJson(w, arr, header, len(arr), func(index int) []any { + p := arr[index] + var protocol, port string + if p.WildcardProtocol { + protocol = "ANY" + } else { + protocol = u8proto.U8proto(p.Protocol).String() + } + if p.WildcardPort { + port = "ANY" + } else { + port = strconv.Itoa(p.Port) + } + return []any{p.Namespace, p.Name, p.Direction, p.Policy, p.Identity, protocol, port, p.Bytes, p.Packets} + }) +} diff --git a/e2e/Makefile b/e2e/Makefile index 8d92e56..3dffb79 100644 --- a/e2e/Makefile +++ b/e2e/Makefile @@ -63,6 +63,7 @@ install-test-pod: $(MAKE) --no-print-directory run-test-pod-l4-ingress-explicit-deny-udp $(MAKE) --no-print-directory run-test-pod-l4-egress-explicit-deny-any $(MAKE) --no-print-directory run-test-pod-l4-egress-explicit-deny-tcp + $(MAKE) --no-print-directory run-test-pod-l4-ingress-all-allow-tcp $(MAKE) --no-print-directory wait-for-workloads kubectl apply -f testdata/policy/l3.yaml diff --git a/e2e/id_test.go b/e2e/id_test.go index 340a7ea..5ccb805 100644 --- a/e2e/id_test.go +++ b/e2e/id_test.go @@ -21,6 +21,7 @@ func testIdLabel() { "l3-ingress-implicit-deny-all", "l4-egress-explicit-deny-any", "l4-egress-explicit-deny-tcp", + "l4-ingress-all-allow-tcp", "l4-ingress-explicit-allow-any", "l4-ingress-explicit-allow-tcp", "l4-ingress-explicit-deny-any", diff --git a/e2e/inspect_test.go b/e2e/inspect_test.go index 61c314f..da2b70b 100644 --- a/e2e/inspect_test.go +++ b/e2e/inspect_test.go @@ -92,6 +92,12 @@ Deny,Ingress,self,false,false,17,161`, Selector: "test=l4-egress-explicit-deny-tcp", Expected: `Allow,Ingress,reserved:host,true,true,0,0`, }, + { + Selector: "test=l4-ingress-all-allow-tcp", + Expected: `Allow,Ingress,reserved:host,false,false,6,8080 +Allow,Ingress,reserved:host,true,true,0,0 +Allow,Ingress,reserved:unknown,false,false,6,8080`, + }, } It("should inspect policy configuration", func() { diff --git a/e2e/list_test.go b/e2e/list_test.go index 591c558..ec15535 100644 --- a/e2e/list_test.go +++ b/e2e/list_test.go @@ -80,6 +80,12 @@ Ingress,CiliumClusterwideNetworkPolicy,-,l3-baseline`, Expected: `Egress,CiliumClusterwideNetworkPolicy,-,l3-baseline Ingress,CiliumClusterwideNetworkPolicy,-,l3-baseline`, }, + { + Selector: "test=l4-ingress-all-allow-tcp", + Expected: `Egress,CiliumClusterwideNetworkPolicy,-,l3-baseline +Ingress,CiliumClusterwideNetworkPolicy,-,l3-baseline +Ingress,CiliumNetworkPolicy,test,l4-ingress-all-allow-tcp`, + }, } It("should list applied policies", func() { diff --git a/e2e/reach_test.go b/e2e/reach_test.go new file mode 100644 index 0000000..3f326ac --- /dev/null +++ b/e2e/reach_test.go @@ -0,0 +1,96 @@ +package e2e + +import ( + "strings" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func testReach() { + cases := []struct { + Selector string + Expected string + }{ + { + Selector: "test=l3-ingress-explicit-allow-all", + Expected: `test,l3-ingress-explicit-allow-all,Ingress,Allow,true,true,0,0 +test,self,Egress,Allow,true,true,0,0`, + }, + { + Selector: "test=l3-ingress-implicit-deny-all", + Expected: `test,self,Egress,Allow,true,true,0,0`, + }, + { + Selector: "test=l3-ingress-explicit-deny-all", + Expected: `test,l3-ingress-explicit-deny-all,Ingress,Deny,true,true,0,0 +test,self,Egress,Allow,true,true,0,0`, + }, + { + Selector: "test=l3-egress-implicit-deny-all", + Expected: ``, + }, + { + Selector: "test=l3-egress-explicit-deny-all", + Expected: `test,self,Egress,Deny,true,true,0,0`, + }, + { + Selector: "test=l4-ingress-explicit-allow-any", + Expected: `test,l4-ingress-explicit-allow-any,Ingress,Allow,false,false,6,53 +test,l4-ingress-explicit-allow-any,Ingress,Allow,false,false,17,53 +test,l4-ingress-explicit-allow-any,Ingress,Allow,false,false,132,53 +test,self,Egress,Allow,false,false,6,53 +test,self,Egress,Allow,false,false,17,53 +test,self,Egress,Allow,false,false,132,53`, + }, + { + Selector: "test=l4-ingress-explicit-allow-tcp", + Expected: `test,l4-ingress-explicit-allow-tcp,Ingress,Allow,false,false,6,8080 +test,self,Egress,Allow,false,false,6,8080`, + }, + { + Selector: "test=l4-ingress-explicit-deny-any", + Expected: `test,l4-ingress-explicit-deny-any,Ingress,Deny,false,false,6,53 +test,l4-ingress-explicit-deny-any,Ingress,Deny,false,false,17,53 +test,l4-ingress-explicit-deny-any,Ingress,Deny,false,false,132,53 +test,self,Egress,Allow,false,false,6,53 +test,self,Egress,Allow,false,false,17,53 +test,self,Egress,Allow,false,false,132,53`, + }, + { + Selector: "test=l4-ingress-explicit-deny-udp", + Expected: `test,l4-ingress-explicit-deny-udp,Ingress,Deny,false,false,17,161 +test,self,Egress,Allow,false,false,17,161`, + }, + { + Selector: "test=l4-egress-explicit-deny-any", + Expected: `test,self,Egress,Deny,false,false,6,53 +test,self,Egress,Deny,false,false,17,53 +test,self,Egress,Deny,false,false,132,53`, + }, + { + Selector: "test=l4-egress-explicit-deny-tcp", + Expected: `test,self,Egress,Deny,false,false,6,8080`, + }, + { + Selector: "test=l4-ingress-all-allow-tcp", + Expected: `test,l4-ingress-all-allow-tcp,Ingress,Allow,false,false,6,8080`, + }, + } + + It("should list traffic policy", func() { + for _, c := range cases { + fromOption := "--from=test/" + onePodByLabelSelector(Default, "test", "test=self") + toOption := "--to=test/" + onePodByLabelSelector(Default, "test", c.Selector) + + result := runViewerSafe(Default, nil, "reach", "-o=json", fromOption, toOption) + result = jqSafe(Default, result, "-r", `[.[] | .name = (.name | split("-") | .[0:5] | join("-"))]`) + result = jqSafe(Default, result, "-r", `[.[] | .name = (.name | if startswith("self") then "self" else . end)]`) + // "npv reach" returns a unstable result, so we need to sort it in test + result = jqSafe(Default, result, "-r", `sort_by(.namespace, .name, .direction, .policy, .wildcard_protocol, .wildcard_port, .protocol, .port)`) + result = jqSafe(Default, result, "-r", `.[] | [.namespace, .name, .direction, .policy, .wildcard_protocol, .wildcard_port, .protocol, .port] | @csv`) + resultString := strings.Replace(string(result), `"`, "", -1) + Expect(resultString).To(Equal(c.Expected), "compare failed. selector: %s\nactual: %s\nexpected: %s", c.Selector, resultString, c.Expected) + } + }) +} diff --git a/e2e/suite_test.go b/e2e/suite_test.go index aafaa33..bbb009d 100644 --- a/e2e/suite_test.go +++ b/e2e/suite_test.go @@ -32,4 +32,5 @@ func runTest() { Context("summary", testSummary) Context("manifest-generate", testManifestGenerate) Context("manifest-range", testManifestRange) + Context("reach", testReach) } diff --git a/e2e/summary_test.go b/e2e/summary_test.go index d3faf8d..a56027e 100644 --- a/e2e/summary_test.go +++ b/e2e/summary_test.go @@ -16,6 +16,7 @@ l3-ingress-explicit-deny-all,1,1,0,0 l3-ingress-implicit-deny-all,1,0,0,0 l4-egress-explicit-deny-any,1,0,0,0 l4-egress-explicit-deny-tcp,1,0,0,0 +l4-ingress-all-allow-tcp,3,0,0,0 l4-ingress-explicit-allow-any,4,0,0,0 l4-ingress-explicit-allow-tcp,2,0,0,0 l4-ingress-explicit-deny-any,1,3,0,0 diff --git a/e2e/testdata/policy/README.md b/e2e/testdata/policy/README.md index a14106c..58c065c 100644 --- a/e2e/testdata/policy/README.md +++ b/e2e/testdata/policy/README.md @@ -14,5 +14,6 @@ | l4-ingress-explicit-deny-udp | allow (L4) | deny (L4) | | l4-egress-explicit-deny-any | deny (L4) | - | | l4-egress-explicit-deny-tcp | deny (L4) | - | +| l4-ingress-all-allow-tcp | - | allow (L4-only) | | 8.8.8.8 (Google Public DNS) | allow (L4) | - | | 8.8.4.4 (Google Public DNS) | deny (L4) | - | diff --git a/e2e/testdata/policy/l4.yaml b/e2e/testdata/policy/l4.yaml index f1d3051..9fcd6af 100644 --- a/e2e/testdata/policy/l4.yaml +++ b/e2e/testdata/policy/l4.yaml @@ -128,3 +128,18 @@ spec: - ports: - port: "161" # SNMP (UDP) protocol: UDP +--- +apiVersion: cilium.io/v2 +kind: CiliumNetworkPolicy +metadata: + namespace: test + name: l4-ingress-all-allow-tcp +spec: + endpointSelector: + matchLabels: + k8s:test: l4-ingress-all-allow-tcp + ingress: + - toPorts: + - ports: + - port: "8080" + protocol: TCP From c8101ce5db2310b7c7b126a615da37193f58a79c Mon Sep 17 00:00:00 2001 From: Daichi Sakaue Date: Wed, 11 Dec 2024 10:42:29 +0900 Subject: [PATCH 2/3] Fix flaky test Signed-off-by: Daichi Sakaue --- e2e/Makefile | 7 ++++++- e2e/id_test.go | 39 +++++++++++++++++++++++++++++++++++---- 2 files changed, 41 insertions(+), 5 deletions(-) diff --git a/e2e/Makefile b/e2e/Makefile index 3dffb79..e1579fa 100644 --- a/e2e/Makefile +++ b/e2e/Makefile @@ -51,7 +51,7 @@ run-test-pod-%: .PHONY: install-test-pod install-test-pod: $(MAKE) --no-print-directory run-test-pod-self - $(MAKE) --no-print-directory DEPLOYMENT_REPLICAS=2 run-test-pod-l3-ingress-explicit-allow-all + $(MAKE) --no-print-directory run-test-pod-l3-ingress-explicit-allow-all $(MAKE) --no-print-directory run-test-pod-l3-ingress-implicit-deny-all $(MAKE) --no-print-directory run-test-pod-l3-ingress-explicit-deny-all $(MAKE) --no-print-directory run-test-pod-l3-egress-implicit-deny-all @@ -66,6 +66,11 @@ install-test-pod: $(MAKE) --no-print-directory run-test-pod-l4-ingress-all-allow-tcp $(MAKE) --no-print-directory wait-for-workloads + # Cilium-agents on different nodes may simultaneously create multiple CiliumIdentities for a same set of labels. + # To enforce the following test deployment to use a same CiliumIdentity, we first create it with replicas=1 and then upscale. + $(MAKE) --no-print-directory DEPLOYMENT_REPLICAS=2 run-test-pod-l3-ingress-explicit-allow-all + $(MAKE) --no-print-directory wait-for-workloads + kubectl apply -f testdata/policy/l3.yaml kubectl apply -f testdata/policy/l4.yaml diff --git a/e2e/id_test.go b/e2e/id_test.go index 5ccb805..6b0e1b0 100644 --- a/e2e/id_test.go +++ b/e2e/id_test.go @@ -1,6 +1,9 @@ package e2e import ( + "fmt" + "strconv" + . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" ) @@ -37,10 +40,38 @@ func testIdLabel() { } func testIdSummary() { - expected := `{"default":1,"kube-system":2,"local-path-storage":1,"test":12}` + cases := []struct { + Namespace string + Count int + }{ + { + Namespace: "default", + Count: 1, + }, + { + Namespace: "kube-system", + Count: 2, + }, + { + Namespace: "local-path-storage", + Count: 1, + }, + { + Namespace: "test", + Count: 13, + }, + } It("should show ID summary", func() { - result := runViewerSafe(Default, nil, "id", "summary", "-o=json") - result = jqSafe(Default, result, "-c") - Expect(string(result)).To(Equal(expected), "compare failed.\nactual: %s\nexpected: %s", string(result), expected) + for _, c := range cases { + resultData := runViewerSafe(Default, nil, "id", "summary", "-o=json") + resultData = jqSafe(Default, resultData, "-r", fmt.Sprintf(`."%s"`, c.Namespace)) + result, err := strconv.Atoi(string(resultData)) + Expect(err).NotTo(HaveOccurred()) + + expected := c.Count + + // Multiple CiliumIdentities may be generated for a same set of security-relevant labels + Expect(result).To(BeNumerically(">=", expected), "compare failed. namespace: %s\nactual: %d\nexpected: %d", result, expected) + } }) } From b041c09cd4d069f338c56820897cb27376d0c3fc Mon Sep 17 00:00:00 2001 From: Daichi Sakaue Date: Wed, 11 Dec 2024 13:54:12 +0900 Subject: [PATCH 3/3] Fix SEGV Signed-off-by: Daichi Sakaue --- cmd/npv/app/list.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/cmd/npv/app/list.go b/cmd/npv/app/list.go index adb8b78..75b77eb 100644 --- a/cmd/npv/app/list.go +++ b/cmd/npv/app/list.go @@ -2,6 +2,7 @@ package app import ( "context" + "errors" "fmt" "io" "sort" @@ -102,6 +103,15 @@ func runList(ctx context.Context, w io.Writer, name string) error { if err != nil { return fmt.Errorf("failed to get endpoint information: %w", err) } + if response.Payload == nil || + response.Payload.Status == nil || + response.Payload.Status.Policy == nil || + response.Payload.Status.Policy.Realized == nil || + response.Payload.Status.Policy.Realized.L4 == nil || + response.Payload.Status.Policy.Realized.L4.Ingress == nil || + response.Payload.Status.Policy.Realized.L4.Egress == nil { + return errors.New("api response is insufficient") + } // The same rule appears multiple times in the response, so we need to dedup it policySet := make(map[derivedFromEntry]struct{})