Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add sort-by effectivePriority for antctl get networkpolicy #1530

Merged
merged 3 commits into from
Dec 4, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions docs/antctl.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:

Expand Down
27 changes: 23 additions & 4 deletions docs/antrea-network-policy.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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
Expand Down
13 changes: 9 additions & 4 deletions pkg/agent/apiserver/handlers/networkpolicy/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,13 @@ import (
"github.com/vmware-tanzu/antrea/pkg/querier"
)

const sortByEffectivePriority = "effectivePriority"

// 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 := newFilterFromURLQuery(r.URL.Query())
npFilter, err := parseURLQuery(r.URL.Query())
if err != nil {
http.Error(w, err.Error(), http.StatusBadRequest)
return
Expand All @@ -48,7 +50,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 {
Expand All @@ -66,7 +67,7 @@ var mapToNetworkPolicyType = map[string]cpv1beta.NetworkPolicyType{
}

// Create a Network Policy Filter from URL Query
func newFilterFromURLQuery(query url.Values) (*querier.NetworkPolicyQueryFilter, error) {
func parseURLQuery(query url.Values) (*querier.NetworkPolicyQueryFilter, error) {
namespace := query.Get("namespace")
pod := query.Get("pod")
if pod != "" && namespace == "" {
Expand All @@ -80,12 +81,16 @@ 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")
}

sortBy := query.Get("sort-by")
if sortBy != "" && sortBy != sortByEffectivePriority {
return nil, fmt.Errorf("unsupported value for sort-by option. Current supported value is %s", sortByEffectivePriority)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Dyanngg I am confused as to why the check is done on the server-side, and only for the agent?

Copy link
Contributor Author

@Dyanngg Dyanngg Dec 2, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are totally right. Reverted that and added the check on the client-side


return &querier.NetworkPolicyQueryFilter{
Name: name,
SourceName: source,
Expand Down
2 changes: 2 additions & 0 deletions pkg/antctl/antctl.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
17 changes: 12 additions & 5 deletions pkg/antctl/command_definition.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,13 @@ func (e *resourceEndpoint) flags() []flagInfo {
usage: "Filter the resource by namespace",
})
}
if e.groupVersionResource == &v1beta2.NetworkPolicyVersionResource {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in a future PR, we can consider extending this flag to other resources & other fields. Would you mind opening a Github issue for that, I think it would make a "good first issue" as well

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opened #1605

flags = append(flags, flagInfo{
name: "sort-by",
defaultValue: "",
usage: "Get NetworkPolicies in specific order. Current supported value is effectivePriority.",
})
}
return flags
}

Expand All @@ -151,7 +158,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.
Expand Down Expand Up @@ -200,7 +207,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 {
Expand Down Expand Up @@ -681,7 +688,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()

Expand All @@ -695,7 +702,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)
}
Expand Down Expand Up @@ -780,7 +787,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)
}
}

Expand Down
19 changes: 13 additions & 6 deletions pkg/antctl/command_definition_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,11 @@ type Foobar struct {
Foo string `json:"foo"`
}

var (
AntreaPolicyTierPriority = int32(250)
AntreaPolicyPriority = float64(1.0)
)
antoninbas marked this conversation as resolved.
Show resolved Hide resolved

func TestCommandList_tableOutputForGetCommands(t *testing.T) {
for _, tc := range []struct {
name string
Expand Down Expand Up @@ -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{
{
Expand All @@ -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
`,
},
{
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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())
})
Expand Down
9 changes: 5 additions & 4 deletions pkg/antctl/transform/addressgroup/transform.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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)
}

Expand Down
9 changes: 5 additions & 4 deletions pkg/antctl/transform/appliedtogroup/transform.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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)
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/antctl/transform/controllerinfo/transform.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading