From 45ac9c4c02022752f9e7bdd53790c29510dd3a70 Mon Sep 17 00:00:00 2001 From: Daichi Sakaue Date: Wed, 20 Nov 2024 11:13:10 +0900 Subject: [PATCH 1/5] Implement manifest command Signed-off-by: Daichi Sakaue --- cmd/npv/app/const.go | 6 + cmd/npv/app/helper.go | 17 +++ cmd/npv/app/manifest.go | 13 +++ cmd/npv/app/manifest_blast.go | 106 +++++++++++++++++ cmd/npv/app/manifest_generate.go | 193 +++++++++++++++++++++++++++++++ e2e/Makefile | 5 +- e2e/manifest_test.go | 148 ++++++++++++++++++++++++ e2e/suite_test.go | 2 + e2e/summary_test.go | 1 + e2e/testdata/policy/README.md | 3 +- e2e/utils_test.go | 2 +- 11 files changed, 493 insertions(+), 3 deletions(-) create mode 100644 cmd/npv/app/manifest.go create mode 100644 cmd/npv/app/manifest_blast.go create mode 100644 cmd/npv/app/manifest_generate.go create mode 100644 e2e/manifest_test.go diff --git a/cmd/npv/app/const.go b/cmd/npv/app/const.go index b077207..fd9fbab 100644 --- a/cmd/npv/app/const.go +++ b/cmd/npv/app/const.go @@ -33,3 +33,9 @@ var gvrClusterwideNetworkPolicy schema.GroupVersionResource = schema.GroupVersio Version: "v2", Resource: "ciliumclusterwidenetworkpolicies", } + +var gvkNetworkPolicy schema.GroupVersionKind = schema.GroupVersionKind{ + Group: "cilium.io", + Version: "v2", + Kind: "CiliumNetworkPolicy", +} diff --git a/cmd/npv/app/helper.go b/cmd/npv/app/helper.go index 2627d31..9d7d370 100644 --- a/cmd/npv/app/helper.go +++ b/cmd/npv/app/helper.go @@ -103,6 +103,23 @@ func getPodEndpointID(ctx context.Context, d *dynamic.DynamicClient, namespace, return endpointID, nil } +func getPodIdentity(ctx context.Context, d *dynamic.DynamicClient, namespace, name string) (int64, error) { + ep, err := d.Resource(gvrEndpoint).Namespace(namespace).Get(ctx, name, metav1.GetOptions{}) + if err != nil { + return 0, err + } + + identity, found, err := unstructured.NestedInt64(ep.Object, "status", "identity", "id") + if err != nil { + return 0, err + } + if !found { + return 0, errors.New("pod does not have security identity") + } + + return identity, nil +} + // key: identity number // value: CiliumIdentity resource func getIdentityResourceMap(ctx context.Context, d *dynamic.DynamicClient) (map[int]*unstructured.Unstructured, error) { diff --git a/cmd/npv/app/manifest.go b/cmd/npv/app/manifest.go new file mode 100644 index 0000000..fbe3871 --- /dev/null +++ b/cmd/npv/app/manifest.go @@ -0,0 +1,13 @@ +package app + +import "github.com/spf13/cobra" + +func init() { + rootCmd.AddCommand(manifestCmd) +} + +var manifestCmd = &cobra.Command{ + Use: "manifest", + Short: "Generate CiliumNetworkPolicy", + Long: `Generate CiliumNetworkPolicy`, +} diff --git a/cmd/npv/app/manifest_blast.go b/cmd/npv/app/manifest_blast.go new file mode 100644 index 0000000..eebaaa3 --- /dev/null +++ b/cmd/npv/app/manifest_blast.go @@ -0,0 +1,106 @@ +package app + +import ( + "context" + "errors" + "io" + "sort" + "strings" + + "github.com/spf13/cobra" +) + +var manifestBlastOptions struct { + from string + to string +} + +func init() { + manifestBlastCmd.Flags().StringVar(&manifestBlastOptions.from, "from", "", "egress pod") + manifestBlastCmd.Flags().StringVar(&manifestBlastOptions.to, "to", "", "ingress pod") + manifestCmd.AddCommand(manifestBlastCmd) +} + +var manifestBlastCmd = &cobra.Command{ + Use: "blast", + Short: "Show blast radius of a generated manifest", + Long: `Show blast radius of a generated manifest`, + + Args: cobra.ExactArgs(0), + RunE: func(cmd *cobra.Command, args []string) error { + return runManifestBlast(context.Background(), cmd.OutOrStdout()) + }, +} + +type manifestBlastEntry struct { + Direction string `json:"direction"` + Namespace string `json:"namespace"` + Name string `json:"name"` +} + +func lessManifestBlastEntry(x, y *manifestBlastEntry) bool { + ret := strings.Compare(x.Direction, y.Direction) + if ret == 0 { + ret = strings.Compare(x.Namespace, y.Namespace) + } + if ret == 0 { + ret = strings.Compare(x.Name, y.Name) + } + return ret < 0 +} + +func runManifestBlast(ctx context.Context, w io.Writer) error { + if manifestBlastOptions.from == "" || manifestBlastOptions.to == "" { + return errors.New("--from and --to options are required") + } + + fromSlice := strings.Split(manifestBlastOptions.from, "/") + toSlice := strings.Split(manifestBlastOptions.to, "/") + if len(fromSlice) != 2 || len(toSlice) != 2 { + return errors.New("--from and --to should be NAMESPACE/POD") + } + + _, dynamicClient, err := createK8sClients() + if err != nil { + return err + } + + fromIdentity, err := getPodIdentity(ctx, dynamicClient, fromSlice[0], fromSlice[1]) + if err != nil { + return err + } + + toIdentity, err := getPodIdentity(ctx, dynamicClient, toSlice[0], toSlice[1]) + if err != nil { + return err + } + + idEndpoints, err := getIdentityEndpoints(ctx, dynamicClient) + if err != nil { + return err + } + + arr := make([]manifestBlastEntry, 0) + sort.Slice(arr, func(i, j int) bool { return lessManifestBlastEntry(&arr[i], &arr[j]) }) + + for _, ep := range idEndpoints[int(fromIdentity)] { + entry := manifestBlastEntry{ + Direction: directionEgress, + Namespace: ep.GetNamespace(), + Name: ep.GetName(), + } + arr = append(arr, entry) + } + for _, ep := range idEndpoints[int(toIdentity)] { + entry := manifestBlastEntry{ + Direction: directionIngress, + Namespace: ep.GetNamespace(), + Name: ep.GetName(), + } + arr = append(arr, entry) + } + return writeSimpleOrJson(w, arr, []string{"DIRECTION", "NAMESPACE", "NAME"}, len(arr), func(index int) []any { + ep := arr[index] + return []any{ep.Direction, ep.Namespace, ep.Name} + }) +} diff --git a/cmd/npv/app/manifest_generate.go b/cmd/npv/app/manifest_generate.go new file mode 100644 index 0000000..c19c1ef --- /dev/null +++ b/cmd/npv/app/manifest_generate.go @@ -0,0 +1,193 @@ +package app + +import ( + "context" + "errors" + "fmt" + "io" + "strconv" + "strings" + + "github.com/spf13/cobra" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/yaml" +) + +var manifestGenerateOptions struct { + name string + egress bool + ingress bool + allow bool + deny bool + from string + to string +} + +func init() { + manifestGenerateCmd.Flags().StringVar(&manifestGenerateOptions.name, "name", "", "resource name") + manifestGenerateCmd.Flags().BoolVar(&manifestGenerateOptions.egress, "egress", false, "generate egress rule") + manifestGenerateCmd.Flags().BoolVar(&manifestGenerateOptions.ingress, "ingress", false, "generate ingress rule") + manifestGenerateCmd.Flags().BoolVar(&manifestGenerateOptions.allow, "allow", false, "generate allow rule") + manifestGenerateCmd.Flags().BoolVar(&manifestGenerateOptions.deny, "deny", false, "generate deny rule") + manifestGenerateCmd.Flags().StringVar(&manifestGenerateOptions.from, "from", "", "egress pod") + manifestGenerateCmd.Flags().StringVar(&manifestGenerateOptions.to, "to", "", "ingress pod") + manifestCmd.AddCommand(manifestGenerateCmd) +} + +var manifestGenerateCmd = &cobra.Command{ + Use: "generate", + Short: "Generate CiliumNetworkPolicy", + Long: `Generate CiliumNetworkPolicy`, + + Args: cobra.ExactArgs(0), + RunE: func(cmd *cobra.Command, args []string) error { + return runManifestGenerate(context.Background(), cmd.OutOrStdout()) + }, +} + +func parseNamespacedName(nn string) (types.NamespacedName, error) { + li := strings.Split(nn, "/") + if len(li) != 2 { + return types.NamespacedName{}, errors.New("input is not NAMESPACE/NAME") + } + return types.NamespacedName{Namespace: li[0], Name: li[1]}, nil +} + +func runManifestGenerate(ctx context.Context, w io.Writer) error { + egress := manifestGenerateOptions.egress + ingress := manifestGenerateOptions.ingress + allow := manifestGenerateOptions.allow + deny := manifestGenerateOptions.deny + from := manifestGenerateOptions.from + to := manifestGenerateOptions.to + + if egress == ingress { + return errors.New("one of --egress or --ingress should be specified") + } + if allow == deny { + return errors.New("one of --allow or --deny should be specified") + } + + sub, err := parseNamespacedName(from) + if err != nil { + return errors.New("--from and --to should be specified as NAMESPACE/POD") + } + + obj, err := parseNamespacedName(to) + if err != nil { + return errors.New("--from and --to should be specified as NAMESPACE/POD") + } + + if ingress { + sub, obj = obj, sub + } + + // Parameters are all up, let's start querying API server + _, dynamicClient, err := createK8sClients() + if err != nil { + return err + } + + subIdentity, err := getPodIdentity(ctx, dynamicClient, sub.Namespace, sub.Name) + if err != nil { + return err + } + + subResource, err := dynamicClient.Resource(gvrIdentity).Get(ctx, strconv.Itoa(int(subIdentity)), metav1.GetOptions{}) + if err != nil { + return err + } + + subLabels, ok, err := unstructured.NestedStringMap(subResource.Object, "security-labels") + if err != nil { + return err + } + if !ok { + return errors.New("subject does not have security labels") + } + + objIdentity, err := getPodIdentity(ctx, dynamicClient, obj.Namespace, obj.Name) + if err != nil { + return err + } + + objResource, err := dynamicClient.Resource(gvrIdentity).Get(ctx, strconv.Itoa(int(objIdentity)), metav1.GetOptions{}) + if err != nil { + return err + } + + objLabels, ok, err := unstructured.NestedStringMap(objResource.Object, "security-labels") + if err != nil { + return err + } + if !ok { + return errors.New("object does not have security labels") + } + + policyName := manifestGenerateOptions.name + if policyName == "" { + direction := "egress" + policy := "allow" + if ingress { + direction = "ingress" + } + if deny { + policy = "deny" + } + policyName = fmt.Sprintf("%s-%s-%d-%d", direction, policy, subIdentity, objIdentity) + } + + var manifest unstructured.Unstructured + manifest.SetGroupVersionKind(gvkNetworkPolicy) + manifest.SetNamespace(sub.Namespace) + manifest.SetName(policyName) + err = unstructured.SetNestedStringMap(manifest.Object, subLabels, "spec", "endpointSelector", "matchLabels") + if err != nil { + return err + } + + objMap := make(map[string]any) + for k, v := range objLabels { + objMap[k] = v + } + + var section, field string + switch { + case egress && allow: + section = "egress" + field = "toEndpoints" + case egress && deny: + section = "egressDeny" + field = "toEndpoints" + case ingress && allow: + section = "ingress" + field = "fromEndpoints" + case ingress && deny: + section = "ingressDeny" + field = "fromEndpoints" + } + + err = unstructured.SetNestedField(manifest.Object, []any{ + map[string]any{ + field: []any{ + map[string]any{ + "matchLabels": objMap, + }, + }, + }, + }, "spec", section) + if err != nil { + return err + } + + data, err := yaml.Marshal(manifest.Object) + if err != nil { + return err + } + if _, err := fmt.Fprintf(w, "%s", string(data)); err != nil { + return err + } + return nil +} diff --git a/e2e/Makefile b/e2e/Makefile index e879900..845a1b6 100644 --- a/e2e/Makefile +++ b/e2e/Makefile @@ -7,6 +7,8 @@ CACHE_DIR := $(shell pwd)/../cache POLICY_VIEWER := $(BIN_DIR)/npv HELM := helm --repository-cache $(CACHE_DIR)/helm/repository --repository-config $(CACHE_DIR)/helm/repositories.yaml +DEPLOYMENT_REPLICAS ?= 1 + ##@ Basic .PHONY: help @@ -41,6 +43,7 @@ run-test-pod-%: @echo Hello | yq > /dev/null cat testdata/template/ubuntu.yaml | \ yq '.metadata.name = "$*"' | \ + yq '.spec.replicas = $(DEPLOYMENT_REPLICAS)' | \ yq '.spec.selector.matchLabels = {"test": "$*"}' | \ yq '.spec.template.metadata.labels = {"test": "$*", "group": "test"}' | \ kubectl apply -f - @@ -48,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 run-test-pod-l3-ingress-explicit-allow-all + $(MAKE) --no-print-directory DEPLOYMENT_REPLICAS=2 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 diff --git a/e2e/manifest_test.go b/e2e/manifest_test.go new file mode 100644 index 0000000..17e1c52 --- /dev/null +++ b/e2e/manifest_test.go @@ -0,0 +1,148 @@ +package e2e + +import ( + "strings" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func testManifestGenerate() { + cases := []struct { + Args []string + Expected string + }{ + { + Args: []string{"--egress", "--allow"}, + Expected: `apiVersion: cilium.io/v2 +kind: CiliumNetworkPolicy +metadata: + name: testrule + namespace: test +spec: + egress: + - toEndpoints: + - matchLabels: + k8s:group: test + k8s:io.cilium.k8s.namespace.labels.kubernetes.io/metadata.name: test + k8s:io.cilium.k8s.policy.cluster: default + k8s:io.cilium.k8s.policy.serviceaccount: default + k8s:io.kubernetes.pod.namespace: test + k8s:test: l3-ingress-explicit-allow-all + endpointSelector: + matchLabels: + k8s:group: test + k8s:io.cilium.k8s.namespace.labels.kubernetes.io/metadata.name: test + k8s:io.cilium.k8s.policy.cluster: default + k8s:io.cilium.k8s.policy.serviceaccount: default + k8s:io.kubernetes.pod.namespace: test + k8s:test: self`, + }, + { + Args: []string{"--egress", "--deny"}, + Expected: `apiVersion: cilium.io/v2 +kind: CiliumNetworkPolicy +metadata: + name: testrule + namespace: test +spec: + egressDeny: + - toEndpoints: + - matchLabels: + k8s:group: test + k8s:io.cilium.k8s.namespace.labels.kubernetes.io/metadata.name: test + k8s:io.cilium.k8s.policy.cluster: default + k8s:io.cilium.k8s.policy.serviceaccount: default + k8s:io.kubernetes.pod.namespace: test + k8s:test: l3-ingress-explicit-allow-all + endpointSelector: + matchLabels: + k8s:group: test + k8s:io.cilium.k8s.namespace.labels.kubernetes.io/metadata.name: test + k8s:io.cilium.k8s.policy.cluster: default + k8s:io.cilium.k8s.policy.serviceaccount: default + k8s:io.kubernetes.pod.namespace: test + k8s:test: self`, + }, + { + Args: []string{"--ingress", "--allow"}, + Expected: `apiVersion: cilium.io/v2 +kind: CiliumNetworkPolicy +metadata: + name: testrule + namespace: test +spec: + endpointSelector: + matchLabels: + k8s:group: test + k8s:io.cilium.k8s.namespace.labels.kubernetes.io/metadata.name: test + k8s:io.cilium.k8s.policy.cluster: default + k8s:io.cilium.k8s.policy.serviceaccount: default + k8s:io.kubernetes.pod.namespace: test + k8s:test: l3-ingress-explicit-allow-all + ingress: + - fromEndpoints: + - matchLabels: + k8s:group: test + k8s:io.cilium.k8s.namespace.labels.kubernetes.io/metadata.name: test + k8s:io.cilium.k8s.policy.cluster: default + k8s:io.cilium.k8s.policy.serviceaccount: default + k8s:io.kubernetes.pod.namespace: test + k8s:test: self`, + }, + { + Args: []string{"--ingress", "--deny"}, + Expected: `apiVersion: cilium.io/v2 +kind: CiliumNetworkPolicy +metadata: + name: testrule + namespace: test +spec: + endpointSelector: + matchLabels: + k8s:group: test + k8s:io.cilium.k8s.namespace.labels.kubernetes.io/metadata.name: test + k8s:io.cilium.k8s.policy.cluster: default + k8s:io.cilium.k8s.policy.serviceaccount: default + k8s:io.kubernetes.pod.namespace: test + k8s:test: l3-ingress-explicit-allow-all + ingressDeny: + - fromEndpoints: + - matchLabels: + k8s:group: test + k8s:io.cilium.k8s.namespace.labels.kubernetes.io/metadata.name: test + k8s:io.cilium.k8s.policy.cluster: default + k8s:io.cilium.k8s.policy.serviceaccount: default + k8s:io.kubernetes.pod.namespace: test + k8s:test: self`, + }, + } + + It("should generate manifests", func() { + from := "--from=test/" + onePodByLabelSelector(Default, "test", "test=self") + to := "--to=test/" + onePodByLabelSelector(Default, "test", "test=l3-ingress-explicit-allow-all") + for _, c := range cases { + args := append([]string{"manifest", "generate", "--name=testrule", from, to}, c.Args...) + result := strings.TrimSpace(string(runViewerSafe(Default, nil, args...))) + Expect(result).To(Equal(c.Expected), "compare failed.\nactual: %s\nexpected: %s", result, c.Expected) + } + }) +} + +func testManifestBlast() { + expected := `Egress,test,self +Ingress,test,l3-ingress-explicit-allow-all +Ingress,test,l3-ingress-explicit-allow-all` + + It("should show blast radius", func() { + from := "--from=test/" + onePodByLabelSelector(Default, "test", "test=self") + to := "--to=test/" + onePodByLabelSelector(Default, "test", "test=l3-ingress-explicit-allow-all") + result := runViewerSafe(Default, nil, "manifest", "blast", from, to, "-o=json") + // remove hash suffix from pod names + 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)]`) + result = jqSafe(Default, result, "-r", `.[] | [.direction, .namespace, .name] | @csv`) + resultString := strings.Replace(string(result), `"`, "", -1) + Expect(resultString).To(Equal(expected), "compare failed.\nactual: %s\nexpected: %s", resultString, expected) + }) +} diff --git a/e2e/suite_test.go b/e2e/suite_test.go index 89db8af..77b7415 100644 --- a/e2e/suite_test.go +++ b/e2e/suite_test.go @@ -30,4 +30,6 @@ func runTest() { Context("id-summary", testIdSummary) Context("inspect", testInspect) Context("summary", testSummary) + Context("manifest-generate", testManifestGenerate) + Context("manifest-blast", testManifestBlast) } diff --git a/e2e/summary_test.go b/e2e/summary_test.go index 0b9665e..d3faf8d 100644 --- a/e2e/summary_test.go +++ b/e2e/summary_test.go @@ -11,6 +11,7 @@ func testSummary() { expected := `l3-egress-explicit-deny-all,1,0,0,0 l3-egress-implicit-deny-all,1,0,0,0 l3-ingress-explicit-allow-all,2,0,0,0 +l3-ingress-explicit-allow-all,2,0,0,0 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 diff --git a/e2e/testdata/policy/README.md b/e2e/testdata/policy/README.md index 4f03477..a14106c 100644 --- a/e2e/testdata/policy/README.md +++ b/e2e/testdata/policy/README.md @@ -2,7 +2,8 @@ | Target | From self (Egress) | To pod (Ingress) | |-|-|-| -| l3-ingress-explicit-allow-all | allow | allow | +| l3-ingress-explicit-allow-all (1) | allow | allow | +| l3-ingress-explicit-allow-all (2) | allow | allow | | l3-ingress-implicit-deny-all | allow | - | | l3-ingress-explicit-deny-all | allow | deny | | l3-egress-implicit-deny-all | - | - | diff --git a/e2e/utils_test.go b/e2e/utils_test.go index c84bdc8..daf7fa3 100644 --- a/e2e/utils_test.go +++ b/e2e/utils_test.go @@ -64,6 +64,6 @@ func onePodByLabelSelector(g Gomega, namespace, selector string) string { data := kubectlSafe(g, nil, "get", "pod", "-n", namespace, "-l", selector, "-o=json") count, err := strconv.Atoi(string(jqSafe(g, data, "-r", ".items | length"))) g.Expect(err).NotTo(HaveOccurred()) - g.Expect(count).To(Equal(1), "namespace: %s, selector: %s", namespace, selector) + g.Expect(count).To(BeNumerically(">=", 1), "namespace: %s, selector: %s", namespace, selector) return string(jqSafe(g, data, "-r", ".items[0].metadata.name")) } From 28aaeaa48f55ba4aeb50db1bdd88b20f75a7d38b Mon Sep 17 00:00:00 2001 From: Daichi Sakaue Date: Thu, 21 Nov 2024 12:20:09 +0900 Subject: [PATCH 2/5] Reflect comments Signed-off-by: Daichi Sakaue --- cmd/npv/app/manifest_blast.go | 12 ++++++------ e2e/manifest_test.go | 8 ++++---- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/cmd/npv/app/manifest_blast.go b/cmd/npv/app/manifest_blast.go index eebaaa3..5cc18bb 100644 --- a/cmd/npv/app/manifest_blast.go +++ b/cmd/npv/app/manifest_blast.go @@ -33,13 +33,13 @@ var manifestBlastCmd = &cobra.Command{ } type manifestBlastEntry struct { - Direction string `json:"direction"` + Side string `json:"side"` Namespace string `json:"namespace"` Name string `json:"name"` } func lessManifestBlastEntry(x, y *manifestBlastEntry) bool { - ret := strings.Compare(x.Direction, y.Direction) + ret := strings.Compare(x.Side, y.Side) if ret == 0 { ret = strings.Compare(x.Namespace, y.Namespace) } @@ -85,7 +85,7 @@ func runManifestBlast(ctx context.Context, w io.Writer) error { for _, ep := range idEndpoints[int(fromIdentity)] { entry := manifestBlastEntry{ - Direction: directionEgress, + Side: "From", Namespace: ep.GetNamespace(), Name: ep.GetName(), } @@ -93,14 +93,14 @@ func runManifestBlast(ctx context.Context, w io.Writer) error { } for _, ep := range idEndpoints[int(toIdentity)] { entry := manifestBlastEntry{ - Direction: directionIngress, + Side: "To", Namespace: ep.GetNamespace(), Name: ep.GetName(), } arr = append(arr, entry) } - return writeSimpleOrJson(w, arr, []string{"DIRECTION", "NAMESPACE", "NAME"}, len(arr), func(index int) []any { + return writeSimpleOrJson(w, arr, []string{"SIDE", "NAMESPACE", "NAME"}, len(arr), func(index int) []any { ep := arr[index] - return []any{ep.Direction, ep.Namespace, ep.Name} + return []any{ep.Side, ep.Namespace, ep.Name} }) } diff --git a/e2e/manifest_test.go b/e2e/manifest_test.go index 17e1c52..d96c99d 100644 --- a/e2e/manifest_test.go +++ b/e2e/manifest_test.go @@ -130,9 +130,9 @@ spec: } func testManifestBlast() { - expected := `Egress,test,self -Ingress,test,l3-ingress-explicit-allow-all -Ingress,test,l3-ingress-explicit-allow-all` + expected := `From,test,self +To,test,l3-ingress-explicit-allow-all +To,test,l3-ingress-explicit-allow-all` It("should show blast radius", func() { from := "--from=test/" + onePodByLabelSelector(Default, "test", "test=self") @@ -141,7 +141,7 @@ Ingress,test,l3-ingress-explicit-allow-all` // remove hash suffix from pod names 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)]`) - result = jqSafe(Default, result, "-r", `.[] | [.direction, .namespace, .name] | @csv`) + result = jqSafe(Default, result, "-r", `.[] | [.side, .namespace, .name] | @csv`) resultString := strings.Replace(string(result), `"`, "", -1) Expect(resultString).To(Equal(expected), "compare failed.\nactual: %s\nexpected: %s", resultString, expected) }) From a1aa3cec42baf53f16b0e488217d55f78322a59b Mon Sep 17 00:00:00 2001 From: Daichi Sakaue Date: Fri, 22 Nov 2024 10:48:58 +0900 Subject: [PATCH 3/5] Update cmd/npv/app/helper.go Co-authored-by: Tomoki Sugiura --- cmd/npv/app/helper.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/npv/app/helper.go b/cmd/npv/app/helper.go index 9d7d370..c3fffdc 100644 --- a/cmd/npv/app/helper.go +++ b/cmd/npv/app/helper.go @@ -114,7 +114,7 @@ func getPodIdentity(ctx context.Context, d *dynamic.DynamicClient, namespace, na return 0, err } if !found { - return 0, errors.New("pod does not have security identity") + return 0, fmt.Errorf("pod %s/%s does not have security identity", namespace, name) } return identity, nil From 84a05ac2426af772016a616ed3c87f2503543936 Mon Sep 17 00:00:00 2001 From: Daichi Sakaue Date: Fri, 22 Nov 2024 13:08:48 +0900 Subject: [PATCH 4/5] Reflect comments Signed-off-by: Daichi Sakaue --- cmd/npv/app/helper.go | 11 ++++++++++- cmd/npv/app/manifest_blast.go | 28 ++++++++++++++++------------ cmd/npv/app/manifest_generate.go | 14 ++------------ e2e/id_test.go | 2 +- e2e/manifest_test.go | 2 +- 5 files changed, 30 insertions(+), 27 deletions(-) diff --git a/cmd/npv/app/helper.go b/cmd/npv/app/helper.go index c3fffdc..8dac888 100644 --- a/cmd/npv/app/helper.go +++ b/cmd/npv/app/helper.go @@ -14,6 +14,7 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/dynamic" "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" @@ -97,7 +98,7 @@ func getPodEndpointID(ctx context.Context, d *dynamic.DynamicClient, namespace, return 0, err } if !found { - return 0, errors.New("endpoint resource is broken") + return 0, fmt.Errorf("endpoint resource %s/%s is broken", namespace, name) } return endpointID, nil @@ -162,6 +163,14 @@ func getIdentityEndpoints(ctx context.Context, d *dynamic.DynamicClient) (map[in return ret, nil } +func parseNamespacedName(nn string) (types.NamespacedName, error) { + li := strings.Split(nn, "/") + if len(li) != 2 { + return types.NamespacedName{}, errors.New("input is not NAMESPACE/NAME") + } + return types.NamespacedName{Namespace: li[0], Name: li[1]}, nil +} + func writeSimpleOrJson(w io.Writer, content any, header []string, count int, values func(index int) []any) error { switch rootOptions.output { case OutputJson: diff --git a/cmd/npv/app/manifest_blast.go b/cmd/npv/app/manifest_blast.go index 5cc18bb..18d0f81 100644 --- a/cmd/npv/app/manifest_blast.go +++ b/cmd/npv/app/manifest_blast.go @@ -33,13 +33,13 @@ var manifestBlastCmd = &cobra.Command{ } type manifestBlastEntry struct { - Side string `json:"side"` + Part string `json:"part"` Namespace string `json:"namespace"` Name string `json:"name"` } func lessManifestBlastEntry(x, y *manifestBlastEntry) bool { - ret := strings.Compare(x.Side, y.Side) + ret := strings.Compare(x.Part, y.Part) if ret == 0 { ret = strings.Compare(x.Namespace, y.Namespace) } @@ -54,10 +54,14 @@ func runManifestBlast(ctx context.Context, w io.Writer) error { return errors.New("--from and --to options are required") } - fromSlice := strings.Split(manifestBlastOptions.from, "/") - toSlice := strings.Split(manifestBlastOptions.to, "/") - if len(fromSlice) != 2 || len(toSlice) != 2 { - return errors.New("--from and --to should be NAMESPACE/POD") + from, err := parseNamespacedName(manifestBlastOptions.from) + if err != nil { + return errors.New("--from and --to should be specified as NAMESPACE/POD") + } + + to, err := parseNamespacedName(manifestBlastOptions.to) + if err != nil { + return errors.New("--from and --to should be specified as NAMESPACE/POD") } _, dynamicClient, err := createK8sClients() @@ -65,12 +69,12 @@ func runManifestBlast(ctx context.Context, w io.Writer) error { return err } - fromIdentity, err := getPodIdentity(ctx, dynamicClient, fromSlice[0], fromSlice[1]) + fromIdentity, err := getPodIdentity(ctx, dynamicClient, from.Namespace, from.Name) if err != nil { return err } - toIdentity, err := getPodIdentity(ctx, dynamicClient, toSlice[0], toSlice[1]) + toIdentity, err := getPodIdentity(ctx, dynamicClient, to.Namespace, to.Name) if err != nil { return err } @@ -85,7 +89,7 @@ func runManifestBlast(ctx context.Context, w io.Writer) error { for _, ep := range idEndpoints[int(fromIdentity)] { entry := manifestBlastEntry{ - Side: "From", + Part: "From", Namespace: ep.GetNamespace(), Name: ep.GetName(), } @@ -93,14 +97,14 @@ func runManifestBlast(ctx context.Context, w io.Writer) error { } for _, ep := range idEndpoints[int(toIdentity)] { entry := manifestBlastEntry{ - Side: "To", + Part: "To", Namespace: ep.GetNamespace(), Name: ep.GetName(), } arr = append(arr, entry) } - return writeSimpleOrJson(w, arr, []string{"SIDE", "NAMESPACE", "NAME"}, len(arr), func(index int) []any { + return writeSimpleOrJson(w, arr, []string{"PART", "NAMESPACE", "NAME"}, len(arr), func(index int) []any { ep := arr[index] - return []any{ep.Side, ep.Namespace, ep.Name} + return []any{ep.Part, ep.Namespace, ep.Name} }) } diff --git a/cmd/npv/app/manifest_generate.go b/cmd/npv/app/manifest_generate.go index c19c1ef..6303105 100644 --- a/cmd/npv/app/manifest_generate.go +++ b/cmd/npv/app/manifest_generate.go @@ -6,12 +6,10 @@ import ( "fmt" "io" "strconv" - "strings" "github.com/spf13/cobra" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/yaml" ) @@ -47,14 +45,6 @@ var manifestGenerateCmd = &cobra.Command{ }, } -func parseNamespacedName(nn string) (types.NamespacedName, error) { - li := strings.Split(nn, "/") - if len(li) != 2 { - return types.NamespacedName{}, errors.New("input is not NAMESPACE/NAME") - } - return types.NamespacedName{Namespace: li[0], Name: li[1]}, nil -} - func runManifestGenerate(ctx context.Context, w io.Writer) error { egress := manifestGenerateOptions.egress ingress := manifestGenerateOptions.ingress @@ -105,7 +95,7 @@ func runManifestGenerate(ctx context.Context, w io.Writer) error { return err } if !ok { - return errors.New("subject does not have security labels") + return fmt.Errorf("pod %s/%s is not assigned security labels", sub.Namespace, sub.Name) } objIdentity, err := getPodIdentity(ctx, dynamicClient, obj.Namespace, obj.Name) @@ -123,7 +113,7 @@ func runManifestGenerate(ctx context.Context, w io.Writer) error { return err } if !ok { - return errors.New("object does not have security labels") + return fmt.Errorf("pod %s/%s is not assigned security labels", obj.Namespace, obj.Name) } policyName := manifestGenerateOptions.name diff --git a/e2e/id_test.go b/e2e/id_test.go index d66814e..340a7ea 100644 --- a/e2e/id_test.go +++ b/e2e/id_test.go @@ -40,6 +40,6 @@ func testIdSummary() { 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)) + Expect(string(result)).To(Equal(expected), "compare failed.\nactual: %s\nexpected: %s", string(result), expected) }) } diff --git a/e2e/manifest_test.go b/e2e/manifest_test.go index d96c99d..62f4ecb 100644 --- a/e2e/manifest_test.go +++ b/e2e/manifest_test.go @@ -141,7 +141,7 @@ To,test,l3-ingress-explicit-allow-all` // remove hash suffix from pod names 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)]`) - result = jqSafe(Default, result, "-r", `.[] | [.side, .namespace, .name] | @csv`) + result = jqSafe(Default, result, "-r", `.[] | [.part, .namespace, .name] | @csv`) resultString := strings.Replace(string(result), `"`, "", -1) Expect(resultString).To(Equal(expected), "compare failed.\nactual: %s\nexpected: %s", resultString, expected) }) From 768decea00d963475a7a647e938ac3b6de9cc668 Mon Sep 17 00:00:00 2001 From: Daichi Sakaue Date: Mon, 25 Nov 2024 11:30:48 +0900 Subject: [PATCH 5/5] Reflect comments Signed-off-by: Daichi Sakaue --- .../{manifest_blast.go => manifest_range.go} | 38 +++++++++---------- e2e/manifest_test.go | 6 +-- e2e/suite_test.go | 2 +- 3 files changed, 23 insertions(+), 23 deletions(-) rename cmd/npv/app/{manifest_blast.go => manifest_range.go} (65%) diff --git a/cmd/npv/app/manifest_blast.go b/cmd/npv/app/manifest_range.go similarity index 65% rename from cmd/npv/app/manifest_blast.go rename to cmd/npv/app/manifest_range.go index 18d0f81..0268750 100644 --- a/cmd/npv/app/manifest_blast.go +++ b/cmd/npv/app/manifest_range.go @@ -10,35 +10,35 @@ import ( "github.com/spf13/cobra" ) -var manifestBlastOptions struct { +var manifestRangeOptions struct { from string to string } func init() { - manifestBlastCmd.Flags().StringVar(&manifestBlastOptions.from, "from", "", "egress pod") - manifestBlastCmd.Flags().StringVar(&manifestBlastOptions.to, "to", "", "ingress pod") - manifestCmd.AddCommand(manifestBlastCmd) + manifestRangeCmd.Flags().StringVar(&manifestRangeOptions.from, "from", "", "egress pod") + manifestRangeCmd.Flags().StringVar(&manifestRangeOptions.to, "to", "", "ingress pod") + manifestCmd.AddCommand(manifestRangeCmd) } -var manifestBlastCmd = &cobra.Command{ - Use: "blast", - Short: "Show blast radius of a generated manifest", - Long: `Show blast radius of a generated manifest`, +var manifestRangeCmd = &cobra.Command{ + Use: "range", + Short: "List affected pods of a generated manifest", + Long: `List affected pods of a generated manifest`, Args: cobra.ExactArgs(0), RunE: func(cmd *cobra.Command, args []string) error { - return runManifestBlast(context.Background(), cmd.OutOrStdout()) + return runManifestRange(context.Background(), cmd.OutOrStdout()) }, } -type manifestBlastEntry struct { +type manifestRangeEntry struct { Part string `json:"part"` Namespace string `json:"namespace"` Name string `json:"name"` } -func lessManifestBlastEntry(x, y *manifestBlastEntry) bool { +func lessManifestRangeEntry(x, y *manifestRangeEntry) bool { ret := strings.Compare(x.Part, y.Part) if ret == 0 { ret = strings.Compare(x.Namespace, y.Namespace) @@ -49,17 +49,17 @@ func lessManifestBlastEntry(x, y *manifestBlastEntry) bool { return ret < 0 } -func runManifestBlast(ctx context.Context, w io.Writer) error { - if manifestBlastOptions.from == "" || manifestBlastOptions.to == "" { +func runManifestRange(ctx context.Context, w io.Writer) error { + if manifestRangeOptions.from == "" || manifestRangeOptions.to == "" { return errors.New("--from and --to options are required") } - from, err := parseNamespacedName(manifestBlastOptions.from) + from, err := parseNamespacedName(manifestRangeOptions.from) if err != nil { return errors.New("--from and --to should be specified as NAMESPACE/POD") } - to, err := parseNamespacedName(manifestBlastOptions.to) + to, err := parseNamespacedName(manifestRangeOptions.to) if err != nil { return errors.New("--from and --to should be specified as NAMESPACE/POD") } @@ -84,11 +84,11 @@ func runManifestBlast(ctx context.Context, w io.Writer) error { return err } - arr := make([]manifestBlastEntry, 0) - sort.Slice(arr, func(i, j int) bool { return lessManifestBlastEntry(&arr[i], &arr[j]) }) + arr := make([]manifestRangeEntry, 0) + sort.Slice(arr, func(i, j int) bool { return lessManifestRangeEntry(&arr[i], &arr[j]) }) for _, ep := range idEndpoints[int(fromIdentity)] { - entry := manifestBlastEntry{ + entry := manifestRangeEntry{ Part: "From", Namespace: ep.GetNamespace(), Name: ep.GetName(), @@ -96,7 +96,7 @@ func runManifestBlast(ctx context.Context, w io.Writer) error { arr = append(arr, entry) } for _, ep := range idEndpoints[int(toIdentity)] { - entry := manifestBlastEntry{ + entry := manifestRangeEntry{ Part: "To", Namespace: ep.GetNamespace(), Name: ep.GetName(), diff --git a/e2e/manifest_test.go b/e2e/manifest_test.go index 62f4ecb..c06f809 100644 --- a/e2e/manifest_test.go +++ b/e2e/manifest_test.go @@ -129,15 +129,15 @@ spec: }) } -func testManifestBlast() { +func testManifestRange() { expected := `From,test,self To,test,l3-ingress-explicit-allow-all To,test,l3-ingress-explicit-allow-all` - It("should show blast radius", func() { + It("should list affected pods", func() { from := "--from=test/" + onePodByLabelSelector(Default, "test", "test=self") to := "--to=test/" + onePodByLabelSelector(Default, "test", "test=l3-ingress-explicit-allow-all") - result := runViewerSafe(Default, nil, "manifest", "blast", from, to, "-o=json") + result := runViewerSafe(Default, nil, "manifest", "range", from, to, "-o=json") // remove hash suffix from pod names 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)]`) diff --git a/e2e/suite_test.go b/e2e/suite_test.go index 77b7415..aafaa33 100644 --- a/e2e/suite_test.go +++ b/e2e/suite_test.go @@ -31,5 +31,5 @@ func runTest() { Context("inspect", testInspect) Context("summary", testSummary) Context("manifest-generate", testManifestGenerate) - Context("manifest-blast", testManifestBlast) + Context("manifest-range", testManifestRange) }