Refactor implementation

cmd/list.go

@@ -5,12 +5,14 @@ import (
 	"encoding/json"
 	"fmt"
 	"io"
+	"sort"
 	"strconv"
 	"strings"
 	"text/tabwriter"
 
 	"github.com/cilium/cilium/api/v1/client/endpoint"
 	"github.com/spf13/cobra"
+	"golang.org/x/exp/maps"
 )
 
 func init() {
@@ -40,6 +42,20 @@ type derivedFromEntry struct {
 	Name      string `json:"name"`
 }
 
+func lessDerivedFromEntry(x, y *derivedFromEntry) bool {
+	ret := strings.Compare(x.Direction, y.Direction)
+	if ret == 0 {
+		ret = strings.Compare(x.Kind, y.Kind)
+	}
+	if ret == 0 {
+		ret = strings.Compare(x.Namespace, y.Namespace)
+	}
+	if ret == 0 {
+		ret = strings.Compare(x.Name, y.Name)
+	}
+	return ret < 0
+}
+
 func parseDerivedFromEntry(input []string, direction string) derivedFromEntry {
 	val := derivedFromEntry{
 		Direction: direction,
@@ -77,22 +93,28 @@ func runList(ctx context.Context, w io.Writer, name string) error {
 		return err
 	}
 
-	policyList := make([]derivedFromEntry, 0)
+	// The same rule appears multiple times in the response, so we need to dedup it
+	policySet := make(map[derivedFromEntry]struct{})
 
 	ingressRules := response.Payload.Status.Policy.Realized.L4.Ingress
 	for _, rule := range ingressRules {
 		for _, r := range rule.DerivedFromRules {
-			policyList = append(policyList, parseDerivedFromEntry(r, directionIngress))
+			entry := parseDerivedFromEntry(r, directionIngress)
+			policySet[entry] = struct{}{}
 		}
 	}
 
 	egressRules := response.Payload.Status.Policy.Realized.L4.Egress
 	for _, rule := range egressRules {
 		for _, r := range rule.DerivedFromRules {
-			policyList = append(policyList, parseDerivedFromEntry(r, directionEgress))
+			entry := parseDerivedFromEntry(r, directionEgress)
+			policySet[entry] = struct{}{}
 		}
 	}
 
+	policyList := maps.Keys(policySet)
+	sort.Slice(policyList, func(i, j int) bool { return lessDerivedFromEntry(&policyList[i], &policyList[j]) })
+
 	switch rootOptions.output {
 	case OutputJson:
 		text, err := json.MarshalIndent(policyList, "", "  ")
@@ -103,13 +125,13 @@ func runList(ctx context.Context, w io.Writer, name string) error {
 		return err
 	case OutputSimple:
 		tw := tabwriter.NewWriter(w, 0, 1, 1, ' ', 0)
-		_, err := tw.Write([]byte("DIRECTION\tKIND\tNAMESPACE\tNAME\n"))
-		if err != nil {
-			return err
+		if !rootOptions.noHeaders {
+			if _, err := tw.Write([]byte("DIRECTION\tKIND\tNAMESPACE\tNAME\n")); err != nil {
+				return err
+			}
 		}
 		for _, p := range policyList {
-			_, err := tw.Write([]byte(fmt.Sprintf("%v\t%v\t%v\t%v\n", p.Direction, p.Kind, p.Namespace, p.Name)))
-			if err != nil {
+			if _, err := tw.Write([]byte(fmt.Sprintf("%v\t%v\t%v\t%v\n", p.Direction, p.Kind, p.Namespace, p.Name))); err != nil {
 				return err
 			}
 		}

cmd/root.go

@@ -17,13 +17,15 @@ var rootOptions struct {
 	proxySelector string
 	proxyPort     uint16
 	output        string
+	noHeaders     bool
 }
 
 func init() {
 	rootCmd.PersistentFlags().StringVarP(&rootOptions.namespace, "namespace", "n", "default", "namespace of a pod")
 	rootCmd.PersistentFlags().StringVar(&rootOptions.proxySelector, "proxy-selector", "app.kubernetes.io/name=cilium-agent-proxy", "label selector to find the proxy pods")
 	rootCmd.PersistentFlags().Uint16Var(&rootOptions.proxyPort, "proxy-port", 8080, "port number of the proxy endpoints")
 	rootCmd.PersistentFlags().StringVarP(&rootOptions.output, "output", "o", OutputSimple, "output format")
+	rootCmd.PersistentFlags().BoolVar(&rootOptions.noHeaders, "no-headers", false, "stop printing header")
 }
 
 var rootCmd = &cobra.Command{}

e2e/Makefile

@@ -33,22 +33,33 @@ start:
 	$(MAKE) --no-print-directory wait-for-workloads
 
 run-test-pod-%:
+	@# https://github.com/orgs/aquaproj/discussions/2964
+	@echo Hello | yq > /dev/null
 	cat testdata/template/ubuntu.yaml | \
 		yq '.metadata.name = "$*"' | \
 		yq '.spec.selector.matchLabels = {"test": "$*"}' | \
-		yq '.spec.template.metadata.labels = {"test": "$*"}' | \
+		yq '.spec.template.metadata.labels = {"test": "$*", "group": "test"}' | \
 		kubectl apply -f -
 
 .PHONY: install-test-pod
 install-test-pod:
 	$(MAKE) --no-print-directory run-test-pod-self
 	$(MAKE) --no-print-directory run-test-pod-l3-ingress-explicit-allow
-	$(MAKE) --no-print-directory run-test-pod-l3-ingress-no-rule
+	$(MAKE) --no-print-directory run-test-pod-l3-ingress-implicit-deny
 	$(MAKE) --no-print-directory run-test-pod-l3-ingress-explicit-deny
 	$(MAKE) --no-print-directory run-test-pod-l3-egress-implicit-deny
 	$(MAKE) --no-print-directory run-test-pod-l3-egress-explicit-deny
+
+	$(MAKE) --no-print-directory run-test-pod-l4-ingress-explicit-allow-any
+	$(MAKE) --no-print-directory run-test-pod-l4-ingress-explicit-allow-tcp
+	$(MAKE) --no-print-directory run-test-pod-l4-ingress-explicit-deny-any
+	$(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 wait-for-workloads
+
 	kubectl apply -f testdata/policy/l3.yaml
+	kubectl apply -f testdata/policy/l4.yaml
 
 .PHONY: install-policy-viewer
 install-policy-viewer:

e2e/list_test.go

@@ -1,71 +1,94 @@
 package e2e
 
 import (
-	"encoding/json"
-	"fmt"
-	"reflect"
+	"strings"
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 )
 
-func testJson(g Gomega, target []byte, expected string) {
-	var t, e interface{}
-	err := json.Unmarshal(target, &t)
-	g.Expect(err).NotTo(HaveOccurred(), "actual: %s", target)
-
-	err = json.Unmarshal([]byte(expected), &e)
-	g.Expect(err).NotTo(HaveOccurred(), "expected: %s", expected)
-
-	if !reflect.DeepEqual(t, e) {
-		err := fmt.Errorf("compare failed. actual: %s, expected: %s", target, expected)
-		g.Expect(err).NotTo(HaveOccurred())
-	}
-}
-
 func testList() {
 	cases := []struct {
 		Selector string
 		Expected string
 	}{
 		{
 			Selector: "test=self",
-			Expected: `[{
-				"direction": "EGRESS",
-				"kind": "CiliumNetworkPolicy",
-				"namespace": "default",
-				"name": "l3-egress"
-			}]`,
+			Expected: `EGRESS,CiliumNetworkPolicy,default,l3-baseline
+EGRESS,CiliumNetworkPolicy,default,l3-egress
+EGRESS,CiliumNetworkPolicy,default,l4-egress
+INGRESS,CiliumNetworkPolicy,default,l3-baseline`,
 		},
 		{
 			Selector: "test=l3-ingress-explicit-allow",
-			Expected: `[{
-				"direction": "INGRESS",
-				"kind": "CiliumNetworkPolicy",
-				"namespace": "default",
-				"name": "l3-ingress-explicit-allow"
-			}]`,
+			Expected: `EGRESS,CiliumNetworkPolicy,default,l3-baseline
+INGRESS,CiliumNetworkPolicy,default,l3-baseline
+INGRESS,CiliumNetworkPolicy,default,l3-ingress-explicit-allow`,
 		},
 		{
-			Selector: "test=l3-ingress-no-rule",
-			Expected: `[]`,
+			Selector: "test=l3-ingress-implicit-deny",
+			Expected: `EGRESS,CiliumNetworkPolicy,default,l3-baseline
+INGRESS,CiliumNetworkPolicy,default,l3-baseline`,
 		},
 		{
 			Selector: "test=l3-ingress-explicit-deny",
-			Expected: `[{
-				"direction": "INGRESS",
-				"kind": "CiliumNetworkPolicy",
-				"namespace": "default",
-				"name": "l3-ingress-explicit-deny"
-			}]`,
+			Expected: `EGRESS,CiliumNetworkPolicy,default,l3-baseline
+INGRESS,CiliumNetworkPolicy,default,l3-baseline
+INGRESS,CiliumNetworkPolicy,default,l3-ingress-explicit-deny`,
+		},
+		{
+			Selector: "test=l3-egress-implicit-deny",
+			Expected: `EGRESS,CiliumNetworkPolicy,default,l3-baseline
+INGRESS,CiliumNetworkPolicy,default,l3-baseline`,
+		},
+		{
+			Selector: "test=l3-egress-explicit-deny",
+			Expected: `EGRESS,CiliumNetworkPolicy,default,l3-baseline
+INGRESS,CiliumNetworkPolicy,default,l3-baseline`,
+		},
+		{
+			Selector: "test=l4-ingress-explicit-allow-any",
+			Expected: `EGRESS,CiliumNetworkPolicy,default,l3-baseline
+INGRESS,CiliumNetworkPolicy,default,l3-baseline
+INGRESS,CiliumNetworkPolicy,default,l4-ingress-explicit-allow-any`,
+		},
+		{
+			Selector: "test=l4-ingress-explicit-allow-tcp",
+			Expected: `EGRESS,CiliumNetworkPolicy,default,l3-baseline
+INGRESS,CiliumNetworkPolicy,default,l3-baseline
+INGRESS,CiliumNetworkPolicy,default,l4-ingress-explicit-allow-tcp`,
+		},
+		{
+			Selector: "test=l4-ingress-explicit-deny-any",
+			Expected: `EGRESS,CiliumNetworkPolicy,default,l3-baseline
+INGRESS,CiliumNetworkPolicy,default,l3-baseline
+INGRESS,CiliumNetworkPolicy,default,l4-ingress-explicit-deny-any`,
+		},
+		{
+			Selector: "test=l4-ingress-explicit-deny-udp",
+			Expected: `EGRESS,CiliumNetworkPolicy,default,l3-baseline
+INGRESS,CiliumNetworkPolicy,default,l3-baseline
+INGRESS,CiliumNetworkPolicy,default,l4-ingress-explicit-deny-udp`,
+		},
+		{
+			Selector: "test=l4-egress-explicit-deny-any",
+			Expected: `EGRESS,CiliumNetworkPolicy,default,l3-baseline
+INGRESS,CiliumNetworkPolicy,default,l3-baseline`,
+		},
+		{
+			Selector: "test=l4-egress-explicit-deny-tcp",
+			Expected: `EGRESS,CiliumNetworkPolicy,default,l3-baseline
+INGRESS,CiliumNetworkPolicy,default,l3-baseline`,
 		},
 	}
 
 	It("should list applied policies", func() {
 		for _, c := range cases {
 			podName := onePodByLabelSelector(Default, "default", c.Selector)
-			result := runViewerSafe(Default, nil, "list", "-o=json", podName)
-			testJson(Default, result, c.Expected)
+			result := runViewerSafe(Default, nil, "list", "-o=json", "--no-headers", podName)
+			result = jqSafe(Default, result, "-r", ".[] | [.direction, .kind, .namespace, .name] | @csv")
+			resultString := strings.Replace(string(result), `"`, "", -1)
+			Expect(resultString).To(Equal(c.Expected), "compare failed. selector: %s, actual: %s, expected: %s", c.Selector, resultString, c.Expected)
 		}
 	})
 }

e2e/testdata/policy/README.md

@@ -0,0 +1,15 @@
+# NetworkPolicy Configuration for Test Pods
+
+| Target | From self (Egress) | To pod (Ingress) |
+|-|-|-|
+| l3-ingress-explicit-allow | allow | allow |
+| l3-ingress-implicit-deny | allow | - |
+| l3-ingress-explicit-deny | allow | deny |
+| l3-egress-implicit-deny | - | - |
+| l3-egress-explicit-deny | deny | - |
+| l4-ingress-explicit-allow-any | allow (L4) | allow (L4) |
+| l4-ingress-explicit-allow-tcp | allow (L4) | allow (L4) |
+| l4-ingress-explicit-deny-any | allow (L4) | deny (L4) |
+| l4-ingress-explicit-deny-udp | allow (L4) | deny (L4) |
+| l4-egress-explicit-deny-any | deny (L4) | - |
+| l4-egress-explicit-deny-tcp | deny (L4) | - |

e2e/testdata/policy/l3.yaml

@@ -1,5 +1,22 @@
 apiVersion: cilium.io/v2
 kind: CiliumNetworkPolicy
+metadata:
+  name: l3-baseline
+spec:
+  endpointSelector:
+    matchLabels:
+      k8s:group: test
+  ingressDeny:
+    - fromEndpoints:
+        - matchLabels:
+            k8s:test: scapegoat
+  egressDeny:
+    - toEndpoints:
+        - matchLabels:
+            k8s:test: scapegoat
+---
+apiVersion: cilium.io/v2
+kind: CiliumNetworkPolicy
 metadata:
   name: l3-egress
 spec:
@@ -10,6 +27,9 @@ spec:
     - toEndpoints:
         - matchLabels:
             k8s:test: l3-ingress-explicit-allow
+    - toEndpoints:
+        - matchLabels:
+            k8s:test: l3-ingress-no-rule
     - toEndpoints:
         - matchLabels:
             k8s:test: l3-ingress-implicit-deny