Address comments and use json response

Signed-off-by: Qiyue Yao <[email protected]>

pkg/antctl/transform/networkpolicy/response.go

@@ -17,7 +17,6 @@ package networkpolicy
 import (
 	"encoding/json"
 	"io"
-	"math"
 	"reflect"
 	"sort"
 	"strconv"
@@ -187,26 +186,13 @@ func (r EvaluationResponse) GetTableHeader() []string {
 
 func (r EvaluationResponse) GetTableRow(_ int) []string {
 	if r.NetworkPolicyEvaluation != nil && r.Response != nil {
-		action := ""
-		if r.Response.Rule.Action != nil {
-			action = string(*r.Response.Rule.Action)
-		} else if r.Response.RuleIndex == math.MaxInt32 {
-			// Responses from endpoint query with original rules will always have
-			// valid action fields, except for the synthetic isolation rules,
-			// identified by a MaxInt32 rule index. "Isolate" corresponds to
-			// a drop action because of the default isolation model of K8s NPs.
-			action = "Isolate"
-		} else {
-			// Should not be possible.
-			action = "Unknown"
-		}
 		return []string{
 			r.Response.NetworkPolicy.Name,
 			r.Response.NetworkPolicy.Namespace,
 			string(r.Response.NetworkPolicy.Type),
 			strconv.Itoa(int(r.Response.RuleIndex)),
 			string(r.Response.Rule.Direction),
-			action,
+			string(*r.Response.Rule.Action),
 		}
 	}
 	return make([]string, len(r.GetTableHeader()))

pkg/antctl/transform/networkpolicy/response_test.go

@@ -182,7 +182,7 @@ func TestEvaluationResponseTransform(t *testing.T) {
 	assert.Equal(t, []string{"NAME", "NAMESPACE", "POLICY-TYPE", "RULE-INDEX", "DIRECTION", "ACTION"}, test.GetTableHeader())
 	assert.False(t, test.SortRows())
 	assert.Equal(t, []string{"", "", "", "", "", ""}, test.GetTableRow(32))
-	testDropAction, testAllowAction := crdv1beta1.RuleActionDrop, crdv1beta1.RuleActionAllow
+	testDropAction, testAllowAction, testIsolateAction := crdv1beta1.RuleActionDrop, crdv1beta1.RuleActionAllow, "Isolate"
 
 	tests := []struct {
 		name           string
@@ -224,23 +224,10 @@ func TestEvaluationResponseTransform(t *testing.T) {
 					Name:      "testK8s",
 				},
 				RuleIndex: math.MaxInt32,
-				Rule:      cpv1beta.RuleRef{Direction: cpv1beta.DirectionIn},
+				Rule:      cpv1beta.RuleRef{Direction: cpv1beta.DirectionIn, Action: (*crdv1beta1.RuleAction)(&testIsolateAction)},
 			},
 			expectedOutput: []string{"testK8s", "ns", "K8sNetworkPolicy", fmt.Sprint(math.MaxInt32), "In", "Isolate"},
 		},
-		{
-			name: "Unknown action in response",
-			testResponse: &cpv1beta.NetworkPolicyEvaluationResponse{
-				NetworkPolicy: cpv1beta.NetworkPolicyReference{
-					Type:      cpv1beta.AntreaNetworkPolicy,
-					Namespace: "ns",
-					Name:      "testError",
-				},
-				RuleIndex: 10,
-				Rule:      cpv1beta.RuleRef{Direction: cpv1beta.DirectionIn},
-			},
-			expectedOutput: []string{"testError", "ns", "AntreaNetworkPolicy", "10", "In", "Unknown"},
-		},
 	}
 
 	for _, tt := range tests {

pkg/apiserver/registry/networkpolicy/networkpolicyevaluation/rest.go

@@ -17,13 +17,15 @@ package networkpolicyevaluation
 import (
 	"context"
 	"fmt"
+	"math"
 
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apiserver/pkg/registry/rest"
 
 	"antrea.io/antrea/pkg/apis/controlplane"
+	"antrea.io/antrea/pkg/apis/crd/v1beta1"
 	"antrea.io/antrea/pkg/controller/networkpolicy"
 )
 
@@ -59,6 +61,20 @@ func (r *REST) Create(ctx context.Context, obj runtime.Object, createValidation
 	if err != nil {
 		return nil, errors.NewInternalError(err)
 	}
+	if response != nil && response.Rule.Action == nil {
+		action := ""
+		if response.RuleIndex == math.MaxInt32 {
+			// Responses from endpoint query with original rules will always have
+			// valid action fields, except for the synthetic isolation rules,
+			// identified by a MaxInt32 rule index. "Isolate" corresponds to
+			// a drop action because of the default isolation model of K8s NPs.
+			action = "Isolate"
+		} else {
+			// Should not be possible.
+			action = "Unknown"
+		}
+		response.Rule.Action = (*v1beta1.RuleAction)(&action)
+	}
 	eval.Response = response
 	return eval, nil
 }

pkg/apiserver/registry/networkpolicy/networkpolicyevaluation/rest_test.go

@@ -17,6 +17,7 @@ package networkpolicyevaluation
 import (
 	"context"
 	"fmt"
+	"math"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -26,6 +27,7 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 
 	"antrea.io/antrea/pkg/apis/controlplane"
+	"antrea.io/antrea/pkg/apis/crd/v1beta1"
 	queriermock "antrea.io/antrea/pkg/controller/networkpolicy/testing"
 )
 
@@ -37,6 +39,7 @@ func TestREST(t *testing.T) {
 
 func TestRESTCreate(t *testing.T) {
 	request := controlplane.NetworkPolicyEvaluationRequest{Source: controlplane.Entity{Pod: &controlplane.PodReference{Namespace: "ns", Name: "pod1"}}, Destination: controlplane.Entity{Pod: &controlplane.PodReference{Namespace: "ns", Name: "pod2"}}}
+	action := "Isolate"
 	tests := []struct {
 		name                string
 		obj                 runtime.Object
@@ -53,12 +56,14 @@ func TestRESTCreate(t *testing.T) {
 			expectedReturnedObj: &controlplane.NetworkPolicyEvaluation{
 				Request: &request,
 				Response: &controlplane.NetworkPolicyEvaluationResponse{
-					NetworkPolicy: controlplane.NetworkPolicyReference{Name: "test"},
-					Rule:          controlplane.RuleRef{Direction: controlplane.DirectionIn},
+					NetworkPolicy: controlplane.NetworkPolicyReference{Type: controlplane.K8sNetworkPolicy, Namespace: "ns", Name: "test"},
+					RuleIndex:     math.MaxInt32,
+					Rule:          controlplane.RuleRef{Direction: controlplane.DirectionIn, Action: (*v1beta1.RuleAction)(&action)},
 				},
 			},
 			mockResponse: &controlplane.NetworkPolicyEvaluationResponse{
-				NetworkPolicy: controlplane.NetworkPolicyReference{Name: "test"},
+				NetworkPolicy: controlplane.NetworkPolicyReference{Type: controlplane.K8sNetworkPolicy, Namespace: "ns", Name: "test"},
+				RuleIndex:     math.MaxInt32,
 				Rule:          controlplane.RuleRef{Direction: controlplane.DirectionIn},
 			},
 		},

test/e2e/antctl_test.go

@@ -24,7 +24,6 @@ import (
 	"testing"
 	"time"
 
-	log "github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	corev1 "k8s.io/api/core/v1"
@@ -380,7 +379,6 @@ func createAntctlServiceAccount(t *testing.T, data *TestData, name string) {
 			Name:      name,
 		},
 	}
-	log.Infof("Creating ServiceAccount '%s/%s' for antctl test Pods", data.testNamespace, name)
 	_, err := data.clientset.CoreV1().ServiceAccounts(data.testNamespace).Create(context.TODO(), serviceAccount, metav1.CreateOptions{})
 	require.NoErrorf(t, err, "failed to create ServiceAccount '%s/%s' for antctl test Pods", data.testNamespace, name)