Add sort-by effectivePriority for antctl get networkpolicy (#1530)

* Add sort-by effectivePriority for antctl get networkpolicy

* Address comments and move sort logic to the client side

* Add value check for sort-by flag

docs/antctl.md

@@ -172,6 +172,15 @@ antctl get appliedtogroup [name] [-o yaml]
 antctl get addressgroup [name] [-o yaml]
 ```
 
+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).
+
+```bash
+antctl get networkpolicy --sort-by=effectivePriority
+```
+
 Antrea Agent additionally supports printing NetworkPolicies applied to a
 specified local Pod using this `antctl` command:
 

docs/antrea-network-policy.md

@@ -481,10 +481,10 @@ policies will be enforced last.
 Within a tier, Antrea-native Policy CRDs are ordered by the `priority` at the policy
 level. Thus, the policy with the highest precedence (lowest priority number
 value) is enforced first. This ordering is performed solely based on the
-`priority` assigned as opposed to the "Kind" of the resource, i.e. the relative
-ordering between a [ClusterNetworkPolicy resource](#antrea-clusternetworkpolicy) and an [Antrea NetworkPolicy
-resource](#antrea-networkpolicy) within a Tier depends only on the `priority`
-set in each of the two resources.
+`priority` assigned, as opposed to the "Kind" of the resource, i.e. the relative
+ordering between a [ClusterNetworkPolicy resource](#antrea-clusternetworkpolicy) and
+an [Antrea NetworkPolicy resource](#antrea-networkpolicy) within a Tier depends only
+on the `priority` set in each of the two resources.
 
 ### Rule enforcement based on priorities
 
@@ -505,6 +505,25 @@ 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=effectivePriority'
+flag can be used to check the order of policy enforcement.
+An example output will look like the following:
+
+```text
+antctl get netpol --sort-by=effectivePriority
+NAME                                 APPLIED-TO                           RULES SOURCE                                 TIER-PRIORITY PRIORITY
+4c504456-9158-4838-bfab-f81665dfae12 85b88ddb-b474-5b44-93d3-c9192c09085e 1     AntreaClusterNetworkPolicy:acnp-1      250           1
+41e510e0-e430-4606-b4d9-261424184fba e36f8beb-9b0b-5b49-b1b7-5c5307cddd83 1     AntreaClusterNetworkPolicy:acnp-2      250           2
+819b8482-ede5-4423-910c-014b731fdba6 bb6711a1-87c7-5a15-9a4a-71bf49a78056 2     AntreaNetworkPolicy:anp-10             250           10
+4d18e031-f05a-48f6-bd91-0197b556ccca e216c104-770c-5731-bfd3-ff4ccbc38c39 2     K8sNetworkPolicy:default/test-1        <NONE>        <NONE>
+c547002a-d8c7-40f1-bdd1-8eb6d0217a67 e216c104-770c-5731-bfd3-ff4ccbc38c39 1     K8sNetworkPolicy:default/test-2        <NONE>        <NONE>
+aac8b8bc-f3bf-4c41-b6e0-2af1863204eb bb6711a1-87c7-5a15-9a4a-71bf49a78056 3     AntreaClusterNetworkPolicy:baseline    253           10
+```
+
+The [ovs-pipeline doc](design/ovs-pipeline.md) contains more information on how
+policy rules are realized by OpenFlow, and how the priority of flows reflects the
+order in which they are enforced.
+
 ## RBAC
 
 Antrea-native Policy CRDs are meant for admins to manage the security of their

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

@@ -48,7 +48,6 @@ func HandleFunc(aq agentquerier.AgentQuerier) http.HandlerFunc {
 		} else {
 			nps = npq.GetNetworkPolicies(npFilter)
 		}
-
 		obj = cpv1beta.NetworkPolicyList{Items: nps}
 
 		if err := json.NewEncoder(w).Encode(obj); err != nil {
@@ -80,7 +79,6 @@ func newFilterFromURLQuery(query url.Values) (*querier.NetworkPolicyQueryFilter,
 	}
 
 	source := query.Get("source")
-
 	name := query.Get("name")
 	if name != "" && (source != "" || namespace != "" || pod != "" || strSourceType != "") {
 		return nil, fmt.Errorf("with a name, none of the other fields can be set")

pkg/antctl/antctl.go

@@ -109,6 +109,8 @@ 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.
+  $ 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
   Get the list of control plane NetworkPolicies whose source NetworkPolicies are in a Namespace (supported by agent only)

pkg/antctl/command_definition.go

@@ -47,7 +47,8 @@ const (
 )
 
 const (
-	maxTableOutputColumnLength int = 50
+	maxTableOutputColumnLength int    = 50
+	sortByEffectivePriority    string = "effectivePriority"
 )
 
 // commandGroup is used to group commands, it could be specified in commandDefinition.
@@ -127,6 +128,14 @@ func (e *resourceEndpoint) flags() []flagInfo {
 			usage:        "Filter the resource by namespace",
 		})
 	}
+	if e.groupVersionResource == &v1beta2.NetworkPolicyVersionResource {
+		flags = append(flags, flagInfo{
+			name:            "sort-by",
+			defaultValue:    "",
+			supportedValues: []string{sortByEffectivePriority},
+			usage:           "Get NetworkPolicies in specific order. Current supported value is effectivePriority.",
+		})
+	}
 	return flags
 }
 
@@ -151,16 +160,17 @@ 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.
 type flagInfo struct {
-	name         string
-	shorthand    string
-	defaultValue string
-	arg          bool
-	usage        string
+	name            string
+	shorthand       string
+	defaultValue    string
+	supportedValues []string
+	arg             bool
+	usage           string
 }
 
 // rawCommand defines a full function cobra.Command which lets developers
@@ -200,7 +210,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 +691,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 +705,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)
 		}
@@ -718,7 +728,7 @@ func (cd *commandDefinition) output(resp io.Reader, writer io.Writer, ft formatt
 			return cd.tableOutput(obj, writer)
 		}
 	default:
-		return fmt.Errorf("unsupport format type: %v", ft)
+		return fmt.Errorf("unsupported format type: %v", ft)
 	}
 	return nil
 }
@@ -734,6 +744,9 @@ func (cd *commandDefinition) collectFlags(cmd *cobra.Command, args []string) (ma
 			} else {
 				vs, err := cmd.Flags().GetString(f.name)
 				if err == nil && len(vs) != 0 {
+					if f.supportedValues != nil && !cd.validateFlagValue(vs, f.supportedValues) {
+						return nil, fmt.Errorf("unsupported value %s for flag %s", vs, f.name)
+					}
 					argMap[f.name] = vs
 					continue
 				}
@@ -746,6 +759,15 @@ func (cd *commandDefinition) collectFlags(cmd *cobra.Command, args []string) (ma
 	return argMap, nil
 }
 
+func (cd *commandDefinition) validateFlagValue(val string, supportedValues []string) bool {
+	for _, s := range supportedValues {
+		if s == val {
+			return true
+		}
+	}
+	return false
+}
+
 // newCommandRunE creates the RunE function for the command. The RunE function
 // checks the args according to argOption and flags.
 func (cd *commandDefinition) newCommandRunE(c *client) func(*cobra.Command, []string) error {
@@ -780,7 +802,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

@@ -45,6 +45,11 @@ type Foobar struct {
 	Foo string `json:"foo"`
 }
 
+var (
+	AntreaPolicyTierPriority = int32(250)
+	AntreaPolicyPriority     = float64(1.0)
+)
+
 func TestCommandList_tableOutputForGetCommands(t *testing.T) {
 	for _, tc := range []struct {
 		name            string
@@ -172,6 +177,8 @@ foo2
 						ObjectMeta: metav1.ObjectMeta{
 							Name: "880db7e8-fc2a-4030-aefe-09afc5f341ad",
 						},
+						TierPriority:    &AntreaPolicyTierPriority,
+						Priority:        &AntreaPolicyPriority,
 						AppliedToGroups: []string{"32ef631b-6817-5a18-86eb-93f4abf0467c"},
 						Rules: []cpv1beta.NetworkPolicyRule{
 							{
@@ -192,9 +199,9 @@ foo2
 					},
 				},
 			},
-			expected: `NAME                                 APPLIED-TO                                       RULES SOURCE                               
-6001549b-ba63-4752-8267-30f52b4332db 32ef631b-6817-5a18-86eb-93f4abf0467c + 1 more... 1     K8sNetworkPolicy:default/allow-all   
-880db7e8-fc2a-4030-aefe-09afc5f341ad 32ef631b-6817-5a18-86eb-93f4abf0467c             2     AntreaNetworkPolicy:default/allow-all
+			expected: `NAME                                 APPLIED-TO                                       RULES SOURCE                                TIER-PRIORITY PRIORITY
+6001549b-ba63-4752-8267-30f52b4332db 32ef631b-6817-5a18-86eb-93f4abf0467c + 1 more... 1     K8sNetworkPolicy:default/allow-all    <NONE>        <NONE>  
+880db7e8-fc2a-4030-aefe-09afc5f341ad 32ef631b-6817-5a18-86eb-93f4abf0467c             2     AntreaNetworkPolicy:default/allow-all 250           1       
 `,
 		},
 		{
@@ -297,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
@@ -321,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
@@ -356,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