Reflect comments

Signed-off-by: Daichi Sakaue <[email protected]>

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:

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,23 +54,27 @@ 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()
 	if err != nil {
 		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,22 +89,22 @@ 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(),
 		}
 		arr = append(arr, entry)
 	}
 	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}
 	})
 }

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

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)
 	})
 }

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)
 	})