Address comments and move sort logic to the client side

docs/antctl.md

@@ -168,14 +168,7 @@ antctl get appliedtogroup [name] [-o yaml]
 antctl get addressgroup [name] [-o yaml]
 ```
 
-Antrea Agent additionally supports printing NetworkPolicies applied to a
-specified local Pod using this `antctl` command:
-
-```bash
-antctl get networkpolicy -p pod -n namespace
-```
-
-Antrea Agent also supports `sort-by=effectivePriority` option, which can be used to
+NetworkPolicy also supports `sort-by=effectivePriority` option, which can be used to
 view the effective order in which the NetworkPolicies are evaluated. Antrea-native
 NetworkPolicy ordering is documented [here](
 antrea-network-policy.md#antrea-native-policy-ordering-based-on-priorities).
@@ -184,6 +177,13 @@ antrea-network-policy.md#antrea-native-policy-ordering-based-on-priorities).
 antctl get networkpolicy --sort-by=effectivePriority
 ```
 
+Antrea Agent additionally supports printing NetworkPolicies applied to a
+specified local Pod using this `antctl` command:
+
+```bash
+antctl get networkpolicy -p pod -n namespace
+```
+
 #### Mapping endpoints to NetworkPolicies
 
 `antctl` supports mapping a specific Pod to the NetworkPolicies which "select"

docs/antrea-network-policy.md

@@ -503,8 +503,9 @@ policy rules match, the packet is then enforced for rules created for K8s NP.
 If the packet still does not match any rule for K8s NP, it will then be evaluated
 against policies created in the "baseline" Tier.
 
-The [antctl command](antctl.md#networkPolicy-commands) with 'sort-by' flag can be used
-to check the order of policy enforcemnet on a specific Node. An example output will look like
+The [antctl command](antctl.md#networkPolicy-commands) with 'sort-by=effectivePriority'
+flag can be used to check the order of policy enforcemnet.
+An example output will look like the following:
 
 ```text
 antctl get netpol --sort-by=effectivePriority

pkg/agent/apiserver/handlers/networkpolicy/handler.go

@@ -19,20 +19,18 @@ import (
 	"fmt"
 	"net/http"
 	"net/url"
-	"sort"
 	"strings"
 
 	agentquerier "github.com/vmware-tanzu/antrea/pkg/agent/querier"
 	cpv1beta "github.com/vmware-tanzu/antrea/pkg/apis/controlplane/v1beta2"
-	"github.com/vmware-tanzu/antrea/pkg/controller/networkpolicy"
 	"github.com/vmware-tanzu/antrea/pkg/querier"
 )
 
 // HandleFunc creates a http.HandlerFunc which uses an AgentNetworkPolicyInfoQuerier
 // to query network policy rules in current agent.
 func HandleFunc(aq agentquerier.AgentQuerier) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
-		npFilter, err := parseURLQuery(r.URL.Query())
+		npFilter, err := newFilterFromURLQuery(r.URL.Query())
 		if err != nil {
 			http.Error(w, err.Error(), http.StatusBadRequest)
 			return
@@ -50,64 +48,14 @@ func HandleFunc(aq agentquerier.AgentQuerier) http.HandlerFunc {
 		} else {
 			nps = npq.GetNetworkPolicies(npFilter)
 		}
-		npSorter := &NPSorter{
-			networkPolicies: nps,
-			sortBy:          r.URL.Query().Get("sort-by"),
-		}
-		sort.Sort(npSorter)
-		obj = cpv1beta.NetworkPolicyList{Items: npSorter.networkPolicies}
+		obj = cpv1beta.NetworkPolicyList{Items: nps}
 
 		if err := json.NewEncoder(w).Encode(obj); err != nil {
 			http.Error(w, "Failed to encode response: "+err.Error(), http.StatusInternalServerError)
 		}
 	}
 }
 
-var (
-	sortByEffectivePriority = "effectivePriority"
-	// Compute a tierPriority value in between the application tier and the baseline tier,
-	// which can be used to sort policies by tier.
-	effectiveTierPriorityK8sNP = (networkpolicy.DefaultTierPriority + networkpolicy.BaselineTierPriority) / 2
-)
-
-type NPSorter struct {
-	networkPolicies []cpv1beta.NetworkPolicy
-	sortBy          string
-}
-
-func (nps *NPSorter) Len() int { return len(nps.networkPolicies) }
-func (nps *NPSorter) Swap(i, j int) {
-	nps.networkPolicies[i], nps.networkPolicies[j] = nps.networkPolicies[j], nps.networkPolicies[i]
-}
-func (nps *NPSorter) Less(i, j int) bool {
-	switch nps.sortBy {
-	case sortByEffectivePriority:
-		var ti, tj int32
-		if nps.networkPolicies[i].TierPriority == nil {
-			ti = effectiveTierPriorityK8sNP
-		} else {
-			ti = *nps.networkPolicies[i].TierPriority
-		}
-		if nps.networkPolicies[j].TierPriority == nil {
-			tj = effectiveTierPriorityK8sNP
-		} else {
-			tj = *nps.networkPolicies[j].TierPriority
-		}
-		pi, pj := nps.networkPolicies[i].Priority, nps.networkPolicies[j].Priority
-		if ti != tj {
-			return ti < tj
-		}
-		if pi != nil && pj != nil && *pi != *pj {
-			return *pi < *pj
-		}
-		fallthrough
-	default:
-		// Do not need a tie-breaker here since NetworkPolicy names are set as UID
-		// of the source policy and will be unique.
-		return nps.networkPolicies[i].Name < nps.networkPolicies[j].Name
-	}
-}
-
 // From user shorthand input to cpv1beta1.NetworkPolicyType
 var mapToNetworkPolicyType = map[string]cpv1beta.NetworkPolicyType{
 	"NP":    cpv1beta.K8sNetworkPolicy,
@@ -117,7 +65,7 @@ var mapToNetworkPolicyType = map[string]cpv1beta.NetworkPolicyType{
 }
 
 // Create a Network Policy Filter from URL Query
-func parseURLQuery(query url.Values) (*querier.NetworkPolicyQueryFilter, error) {
+func newFilterFromURLQuery(query url.Values) (*querier.NetworkPolicyQueryFilter, error) {
 	namespace := query.Get("namespace")
 	pod := query.Get("pod")
 	if pod != "" && namespace == "" {
@@ -136,10 +84,6 @@ func parseURLQuery(query url.Values) (*querier.NetworkPolicyQueryFilter, error)
 		return nil, fmt.Errorf("with a name, none of the other fields can be set")
 	}
 
-	sortBy := query.Get("sort-by")
-	if sortBy != "" && sortBy != sortByEffectivePriority {
-		return nil, fmt.Errorf("unsupported sort-by option. Supported value is %s", sortByEffectivePriority)
-	}
 	return &querier.NetworkPolicyQueryFilter{
 		Name:       name,
 		SourceName: source,

pkg/antctl/antctl.go

@@ -109,7 +109,7 @@ var CommandList = &commandList{
   $ antctl get networkpolicy 6001549b-ba63-4752-8267-30f52b4332db
   Get the list of all control plane NetworkPolicies
   $ antctl get networkpolicy
-  Get the list of all control plane NetworkPolicies, sorted by the order in which the policies are evaluated (supported by agent only)
+  Get the list of all control plane NetworkPolicies, sorted by the order in which the policies are evaluated.
   $ antctl get networkpolicy --sort-by=effectivePriority
   Get the control plane NetworkPolicy with a specific source (supported by agent only)
   $ antctl get networkpolicy -S allow-http -n ns1
@@ -155,11 +155,6 @@ var CommandList = &commandList{
 							usage:     "Get NetworkPolicies with specific type. Type means the type of its source network policy: K8sNP, ACNP, ANP",
 							shorthand: "T",
 						},
-						{
-							name:      "sort-by",
-							usage:     "Get NetworkPolicies in specific order. Current supported value is effectivePriority. If not specified, by default results are sorted by name.",
-							shorthand: "O",
-						},
 					},
 					outputType: multiple,
 				},

pkg/antctl/command_definition.go

@@ -127,6 +127,14 @@ func (e *resourceEndpoint) flags() []flagInfo {
 			usage:        "Filter the resource by namespace",
 		})
 	}
+	if e.groupVersionResource == &v1beta2.NetworkPolicyVersionResource {
+		flags = append(flags, flagInfo{
+			name:         "sort-by",
+			shorthand:    "b",
+			defaultValue: "",
+			usage:        "Get NetworkPolicies in specific order. Current supported value is effectivePriority.",
+		})
+	}
 	return flags
 }
 
@@ -151,7 +159,7 @@ type endpoint struct {
 	// addonTransform is used to transform or update the response data received
 	// from the handler, it must returns an interface which has same type as
 	// TransformedResponse.
-	addonTransform func(reader io.Reader, single bool) (interface{}, error)
+	addonTransform func(reader io.Reader, single bool, opts map[string]string) (interface{}, error)
 }
 
 // flagInfo represents a command-line flag that can be provided when invoking an antctl command.
@@ -200,7 +208,7 @@ func (cd *commandDefinition) namespaced() bool {
 	return false
 }
 
-func (cd *commandDefinition) getAddonTransform() func(reader io.Reader, single bool) (interface{}, error) {
+func (cd *commandDefinition) getAddonTransform() func(reader io.Reader, single bool, opts map[string]string) (interface{}, error) {
 	if runtime.Mode == runtime.ModeAgent && cd.agentEndpoint != nil {
 		return cd.agentEndpoint.addonTransform
 	} else if runtime.Mode == runtime.ModeController && cd.controllerEndpoint != nil {
@@ -681,7 +689,7 @@ func (cd *commandDefinition) tableOutput(obj interface{}, writer io.Writer) erro
 // format. If the AddonTransform is set, it will use the function to transform
 // the data first. It will try to output the resp in the format ft specified after
 // doing transform.
-func (cd *commandDefinition) output(resp io.Reader, writer io.Writer, ft formatterType, single bool) (err error) {
+func (cd *commandDefinition) output(resp io.Reader, writer io.Writer, ft formatterType, single bool, args map[string]string) (err error) {
 	var obj interface{}
 	addonTransform := cd.getAddonTransform()
 
@@ -695,7 +703,7 @@ func (cd *commandDefinition) output(resp io.Reader, writer io.Writer, ft formatt
 			return fmt.Errorf("error when decoding response %v: %w", resp, err)
 		}
 	} else {
-		obj, err = addonTransform(resp, single)
+		obj, err = addonTransform(resp, single, args)
 		if err != nil {
 			return fmt.Errorf("error when doing local transform: %w", err)
 		}
@@ -780,7 +788,7 @@ func (cd *commandDefinition) newCommandRunE(c *client) func(*cobra.Command, []st
 			return err
 		}
 		isSingle := cd.getEndpoint().OutputType() != multiple && (cd.getEndpoint().OutputType() == single || argGet)
-		return cd.output(resp, os.Stdout, formatterType(outputFormat), isSingle)
+		return cd.output(resp, os.Stdout, formatterType(outputFormat), isSingle, argMap)
 	}
 }
 

pkg/antctl/command_definition_test.go

@@ -304,7 +304,7 @@ func TestFormat(t *testing.T) {
 	for _, tc := range []struct {
 		name            string
 		single          bool
-		transform       func(reader io.Reader, single bool) (interface{}, error)
+		transform       func(reader io.Reader, single bool, opts map[string]string) (interface{}, error)
 		rawResponseData interface{}
 		responseStruct  reflect.Type
 		expected        string
@@ -328,7 +328,7 @@ func TestFormat(t *testing.T) {
 		{
 			name:   "StructureData-Transform-Single-Yaml",
 			single: true,
-			transform: func(reader io.Reader, single bool) (i interface{}, err error) {
+			transform: func(reader io.Reader, single bool, opts map[string]string) (i interface{}, err error) {
 				foo := &Foobar{}
 				err = json.NewDecoder(reader).Decode(foo)
 				return &struct{ Bar string }{Bar: foo.Foo}, err
@@ -363,7 +363,7 @@ func TestFormat(t *testing.T) {
 			responseData, err := json.Marshal(tc.rawResponseData)
 			assert.Nil(t, err)
 			var outputBuf bytes.Buffer
-			err = opt.output(bytes.NewBuffer(responseData), &outputBuf, tc.formatter, tc.single)
+			err = opt.output(bytes.NewBuffer(responseData), &outputBuf, tc.formatter, tc.single, map[string]string{})
 			assert.Nil(t, err)
 			assert.Equal(t, tc.expected, outputBuf.String())
 		})

pkg/antctl/transform/addressgroup/transform.go

@@ -28,18 +28,18 @@ type Response struct {
 	Pods []common.GroupMember `json:"pods,omitempty"`
 }
 
-func listTransform(l interface{}) (interface{}, error) {
+func listTransform(l interface{}, opts map[string]string) (interface{}, error) {
 	groups := l.(*cpv1beta.AddressGroupList)
 	result := []interface{}{}
 	for i := range groups.Items {
 		item := groups.Items[i]
-		o, _ := objectTransform(&item)
+		o, _ := objectTransform(&item, opts)
 		result = append(result, o.(Response))
 	}
 	return result, nil
 }
 
-func objectTransform(o interface{}) (interface{}, error) {
+func objectTransform(o interface{}, _ map[string]string) (interface{}, error) {
 	group := o.(*cpv1beta.AddressGroup)
 	var pods []common.GroupMember
 	for _, pod := range group.GroupMembers {
@@ -48,12 +48,13 @@ func objectTransform(o interface{}) (interface{}, error) {
 	return Response{Name: group.Name, Pods: pods}, nil
 }
 
-func Transform(reader io.Reader, single bool) (interface{}, error) {
+func Transform(reader io.Reader, single bool, opts map[string]string) (interface{}, error) {
 	return transform.GenericFactory(
 		reflect.TypeOf(cpv1beta.AddressGroup{}),
 		reflect.TypeOf(cpv1beta.AddressGroupList{}),
 		objectTransform,
 		listTransform,
+		opts,
 	)(reader, single)
 }
 

pkg/antctl/transform/appliedtogroup/transform.go

@@ -28,18 +28,18 @@ type Response struct {
 	Pods []common.GroupMember `json:"pods,omitempty"`
 }
 
-func listTransform(l interface{}) (interface{}, error) {
+func listTransform(l interface{}, opts map[string]string) (interface{}, error) {
 	groups := l.(*cpv1beta.AppliedToGroupList)
 	result := []Response{}
 	for i := range groups.Items {
 		group := groups.Items[i]
-		o, _ := objectTransform(&group)
+		o, _ := objectTransform(&group, opts)
 		result = append(result, o.(Response))
 	}
 	return result, nil
 }
 
-func objectTransform(o interface{}) (interface{}, error) {
+func objectTransform(o interface{}, _ map[string]string) (interface{}, error) {
 	group := o.(*cpv1beta.AppliedToGroup)
 	var pods []common.GroupMember
 	for _, pod := range group.GroupMembers {
@@ -48,12 +48,13 @@ func objectTransform(o interface{}) (interface{}, error) {
 	return Response{Name: group.GetName(), Pods: pods}, nil
 }
 
-func Transform(reader io.Reader, single bool) (interface{}, error) {
+func Transform(reader io.Reader, single bool, opts map[string]string) (interface{}, error) {
 	return transform.GenericFactory(
 		reflect.TypeOf(cpv1beta.AppliedToGroup{}),
 		reflect.TypeOf(cpv1beta.AppliedToGroupList{}),
 		objectTransform,
 		listTransform,
+		opts,
 	)(reader, single)
 }
 

pkg/antctl/transform/controllerinfo/transform.go

@@ -38,7 +38,7 @@ type Response struct {
 	ControllerConditions        []clusterinfo.ControllerCondition       `json:"controllerConditions,omitempty"`        // Controller condition contains types like ControllerHealthy
 }
 
-func Transform(reader io.Reader, _ bool) (interface{}, error) {
+func Transform(reader io.Reader, _ bool, _ map[string]string) (interface{}, error) {
 	b, err := ioutil.ReadAll(reader)
 	if err != nil {
 		return nil, err