From a610e2dd1380f3278d7e19ff5eb11c442ffe7c91 Mon Sep 17 00:00:00 2001 From: Qiyue Yao Date: Tue, 19 Jul 2022 15:29:03 -0700 Subject: [PATCH 01/12] Controller K8s Enablelogging Signed-off-by: Qiyue Yao --- .../networkpolicy/networkpolicy_controller.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/pkg/controller/networkpolicy/networkpolicy_controller.go b/pkg/controller/networkpolicy/networkpolicy_controller.go index 02d7856f809..4a77a5e25bc 100644 --- a/pkg/controller/networkpolicy/networkpolicy_controller.go +++ b/pkg/controller/networkpolicy/networkpolicy_controller.go @@ -81,6 +81,8 @@ const ( PriorityIndex = "priority" // ClusterGroupIndex is used to index ClusterNetworkPolicies by ClusterGroup names. ClusterGroupIndex = "clustergroup" + // EnableNPLoggingAnnotationKey can be added to Namespace to enable logging K8s NP. + EnableNPLoggingAnnotationKey = "policy.antrea.io/enable-np-logging" appliedToGroupType grouping.GroupType = "appliedToGroup" addressGroupType grouping.GroupType = "addressGroup" @@ -566,6 +568,12 @@ func (n *NetworkPolicyController) processNetworkPolicy(np *networkingv1.NetworkP appliedToGroupKey := n.createAppliedToGroup(np.Namespace, &np.Spec.PodSelector, nil, nil) appliedToGroupNames := []string{appliedToGroupKey} rules := make([]controlplane.NetworkPolicyRule, 0, len(np.Spec.Ingress)+len(np.Spec.Egress)) + // Retrieve Namespace logging annotation. + enableLogging := false + namespace, err := n.namespaceLister.Get(np.Namespace) + if err == nil { + enableLogging, _ = strconv.ParseBool(namespace.Annotations[EnableNPLoggingAnnotationKey]) + } var ingressRuleExists, egressRuleExists bool // Compute NetworkPolicyRule for Ingress Rule. for _, ingressRule := range np.Spec.Ingress { @@ -577,7 +585,7 @@ func (n *NetworkPolicyController) processNetworkPolicy(np *networkingv1.NetworkP Services: services, Priority: defaultRulePriority, Action: &defaultAction, - EnableLogging: false, + EnableLogging: enableLogging, }) } // Compute NetworkPolicyRule for Egress Rule. @@ -590,7 +598,7 @@ func (n *NetworkPolicyController) processNetworkPolicy(np *networkingv1.NetworkP Services: services, Priority: defaultRulePriority, Action: &defaultAction, - EnableLogging: false, + EnableLogging: enableLogging, }) } From 852aae1e492904effbbb3e34117fb986b8685ecd Mon Sep 17 00:00:00 2001 From: Qiyue Yao Date: Thu, 21 Jul 2022 10:53:24 -0700 Subject: [PATCH 02/12] log k8s drop Signed-off-by: Qiyue Yao --- .../controller/networkpolicy/audit_logging.go | 17 +++++++---- pkg/agent/openflow/network_policy.go | 26 ++++++++--------- pkg/agent/openflow/network_policy_test.go | 10 +++---- pkg/agent/openflow/pipeline.go | 28 +++++++++++++------ .../networkpolicy_controller_test.go | 2 ++ 5 files changed, 50 insertions(+), 33 deletions(-) diff --git a/pkg/agent/controller/networkpolicy/audit_logging.go b/pkg/agent/controller/networkpolicy/audit_logging.go index 97265d12b07..f6cfe7b8b1d 100644 --- a/pkg/agent/controller/networkpolicy/audit_logging.go +++ b/pkg/agent/controller/networkpolicy/audit_logging.go @@ -28,6 +28,7 @@ import ( "k8s.io/klog/v2" "antrea.io/antrea/pkg/agent/openflow" + "antrea.io/antrea/pkg/apis/controlplane/v1beta2" binding "antrea.io/antrea/pkg/ovs/openflow" "antrea.io/antrea/pkg/util/ip" "antrea.io/antrea/pkg/util/logdir" @@ -196,13 +197,17 @@ func getNetworkPolicyInfo(pktIn *ofctrl.PacketIn, c *Controller, ob *logInfo) er // Set match to corresponding ingress/egress reg according to disposition. match = getMatch(matchers, tableID, info) - // Get Network Policy full name and OF priority of the conjunction. - info, err = getInfoInReg(match, nil) - if err != nil { - return fmt.Errorf("received error while unloading conjunction id from reg: %v", err) + if match != nil { + // Get Network Policy full name and OF priority of the conjunction. + info, err = getInfoInReg(match, nil) + if err != nil { + return fmt.Errorf("received error while unloading conjunction id from reg: %v", err) + } + ob.npRef, ob.ofPriority = c.ofClient.GetPolicyInfoFromConjunction(info) + } else { + // For K8s NetworkPolicy implicit drop action, we cannot get name/namespace. + ob.npRef, ob.ofPriority = string(v1beta2.K8sNetworkPolicy), "-1" } - ob.npRef, ob.ofPriority = c.ofClient.GetPolicyInfoFromConjunction(info) - return nil } diff --git a/pkg/agent/openflow/network_policy.go b/pkg/agent/openflow/network_policy.go index 20624b95e2f..1471a620a58 100644 --- a/pkg/agent/openflow/network_policy.go +++ b/pkg/agent/openflow/network_policy.go @@ -703,7 +703,7 @@ func (c *client) DeleteAddressFromDNSConjunction(id uint32, addrs []types.Addres return c.DeletePolicyRuleAddress(id, types.DstAddress, addrs, &dnsPriority) } -func (c *clause) addConjunctiveMatchFlow(featureNetworkPolicy *featureNetworkPolicy, match *conjunctiveMatch) *conjMatchFlowContextChange { +func (c *clause) addConjunctiveMatchFlow(featureNetworkPolicy *featureNetworkPolicy, match *conjunctiveMatch, enableLogging bool) *conjMatchFlowContextChange { matcherKey := match.generateGlobalMapKey() _, found := c.matches[matcherKey] if found { @@ -727,7 +727,7 @@ func (c *clause) addConjunctiveMatchFlow(featureNetworkPolicy *featureNetworkPol // Generate the default drop flow if dropTable is not nil and the default drop flow is not set yet. if c.dropTable != nil && context.dropFlow == nil { dropFlow = &flowChange{ - flow: context.featureNetworkPolicy.defaultDropFlow(c.dropTable, match.matchPairs), + flow: context.featureNetworkPolicy.defaultDropFlow(c.dropTable, match.matchPairs, enableLogging), changeType: insertion, } } @@ -923,12 +923,12 @@ func serviceToBitRanges(service v1beta2.Service) []types.BitRange { // addAddrFlows translates the specified addresses to conjunctiveMatchFlows, and returns the corresponding changes on the // conjunctiveMatchFlows. -func (c *clause) addAddrFlows(featureNetworkPolicy *featureNetworkPolicy, addrType types.AddressType, addresses []types.Address, priority *uint16) []*conjMatchFlowContextChange { +func (c *clause) addAddrFlows(featureNetworkPolicy *featureNetworkPolicy, addrType types.AddressType, addresses []types.Address, priority *uint16, enableLogging bool) []*conjMatchFlowContextChange { var conjMatchFlowContextChanges []*conjMatchFlowContextChange // Calculate Openflow changes for the added addresses. for _, addr := range addresses { match := generateAddressConjMatch(c.ruleTable.GetID(), addr, addrType, priority) - ctxChange := c.addConjunctiveMatchFlow(featureNetworkPolicy, match) + ctxChange := c.addConjunctiveMatchFlow(featureNetworkPolicy, match, enableLogging) if ctxChange != nil { conjMatchFlowContextChanges = append(conjMatchFlowContextChanges, ctxChange) } @@ -943,7 +943,7 @@ func (c *clause) addServiceFlows(featureNetworkPolicy *featureNetworkPolicy, ser for _, service := range services { matches := generateServiceConjMatches(c.ruleTable.GetID(), service, priority, featureNetworkPolicy.ipProtocols, matchSrc) for _, match := range matches { - ctxChange := c.addConjunctiveMatchFlow(featureNetworkPolicy, match) + ctxChange := c.addConjunctiveMatchFlow(featureNetworkPolicy, match, false) conjMatchFlowContextChanges = append(conjMatchFlowContextChanges, ctxChange) } } @@ -1143,20 +1143,20 @@ func (f *featureNetworkPolicy) addRuleToConjunctiveMatch(conj *policyRuleConjunc if conj.fromClause != nil { for _, addr := range rule.From { match := generateAddressConjMatch(conj.fromClause.ruleTable.GetID(), addr, types.SrcAddress, rule.Priority) - f.addActionToConjunctiveMatch(conj.fromClause, match) + f.addActionToConjunctiveMatch(conj.fromClause, match, rule.EnableLogging) } } if conj.toClause != nil { for _, addr := range rule.To { match := generateAddressConjMatch(conj.toClause.ruleTable.GetID(), addr, types.DstAddress, rule.Priority) - f.addActionToConjunctiveMatch(conj.toClause, match) + f.addActionToConjunctiveMatch(conj.toClause, match, rule.EnableLogging) } } if conj.serviceClause != nil { for _, eachService := range rule.Service { matches := generateServiceConjMatches(conj.serviceClause.ruleTable.GetID(), eachService, rule.Priority, f.ipProtocols, false) for _, match := range matches { - f.addActionToConjunctiveMatch(conj.serviceClause, match) + f.addActionToConjunctiveMatch(conj.serviceClause, match, rule.EnableLogging) } } } @@ -1165,7 +1165,7 @@ func (f *featureNetworkPolicy) addRuleToConjunctiveMatch(conj *policyRuleConjunc // addActionToConjunctiveMatch adds a clause to corresponding conjunctive match context. // It updates the context status directly and doesn't calculate the match flow, which is supposed to be calculated after // all actions are added. It's used in initial batch install only. -func (f *featureNetworkPolicy) addActionToConjunctiveMatch(clause *clause, match *conjunctiveMatch) { +func (f *featureNetworkPolicy) addActionToConjunctiveMatch(clause *clause, match *conjunctiveMatch, enableLogging bool) { matcherKey := match.generateGlobalMapKey() _, found := clause.matches[matcherKey] if found { @@ -1184,7 +1184,7 @@ func (f *featureNetworkPolicy) addActionToConjunctiveMatch(clause *clause, match } // Generate the default drop flow if dropTable is not nil. if clause.dropTable != nil { - context.dropFlow = context.featureNetworkPolicy.defaultDropFlow(clause.dropTable, match.matchPairs) + context.dropFlow = context.featureNetworkPolicy.defaultDropFlow(clause.dropTable, match.matchPairs, enableLogging) } f.globalConjMatchFlowCache[matcherKey] = context } @@ -1371,10 +1371,10 @@ func (c *policyRuleConjunction) calculateClauses(rule *types.PolicyRule) (uint8, func (c *policyRuleConjunction) calculateChangesForRuleCreation(featureNetworkPolicy *featureNetworkPolicy, rule *types.PolicyRule) []*conjMatchFlowContextChange { var ctxChanges []*conjMatchFlowContextChange if c.fromClause != nil { - ctxChanges = append(ctxChanges, c.fromClause.addAddrFlows(featureNetworkPolicy, types.SrcAddress, rule.From, rule.Priority)...) + ctxChanges = append(ctxChanges, c.fromClause.addAddrFlows(featureNetworkPolicy, types.SrcAddress, rule.From, rule.Priority, rule.EnableLogging)...) } if c.toClause != nil { - ctxChanges = append(ctxChanges, c.toClause.addAddrFlows(featureNetworkPolicy, types.DstAddress, rule.To, rule.Priority)...) + ctxChanges = append(ctxChanges, c.toClause.addAddrFlows(featureNetworkPolicy, types.DstAddress, rule.To, rule.Priority, rule.EnableLogging)...) } if c.serviceClause != nil { ctxChanges = append(ctxChanges, c.serviceClause.addServiceFlows(featureNetworkPolicy, rule.Service, rule.Priority, false)...) @@ -1569,7 +1569,7 @@ func (c *client) AddPolicyRuleAddress(ruleID uint32, addrType types.AddressType, c.featureNetworkPolicy.conjMatchFlowLock.Lock() defer c.featureNetworkPolicy.conjMatchFlowLock.Unlock() - flowChanges := clause.addAddrFlows(c.featureNetworkPolicy, addrType, addresses, priority) + flowChanges := clause.addAddrFlows(c.featureNetworkPolicy, addrType, addresses, priority, false) return c.featureNetworkPolicy.applyConjunctiveMatchFlows(flowChanges) } diff --git a/pkg/agent/openflow/network_policy_test.go b/pkg/agent/openflow/network_policy_test.go index 89eb37fea23..614941811e2 100644 --- a/pkg/agent/openflow/network_policy_test.go +++ b/pkg/agent/openflow/network_policy_test.go @@ -105,7 +105,7 @@ func TestPolicyRuleConjunction(t *testing.T) { var addedAddrs = parseAddresses([]string{"192.168.1.3", "192.168.1.30", "192.168.2.0/24", "103", "104"}) expectConjunctionsCount([]*expectConjunctionTimes{{5, ruleID1, clauseID, nClause}}) - flowChanges1 := clause1.addAddrFlows(c.featureNetworkPolicy, types.SrcAddress, addedAddrs, nil) + flowChanges1 := clause1.addAddrFlows(c.featureNetworkPolicy, types.SrcAddress, addedAddrs, nil, false) err := c.featureNetworkPolicy.applyConjunctiveMatchFlows(flowChanges1) require.Nil(t, err, "Failed to invoke addAddrFlows") checkFlowCount(t, len(addedAddrs)) @@ -130,7 +130,7 @@ func TestPolicyRuleConjunction(t *testing.T) { var addedAddrs2 = parseAddresses([]string{"192.168.1.30", "192.168.1.50"}) expectConjunctionsCount([]*expectConjunctionTimes{{2, ruleID2, clauseID2, nClause}}) expectConjunctionsCount([]*expectConjunctionTimes{{1, ruleID1, clauseID, nClause}}) - flowChanges3 := clause2.addAddrFlows(c.featureNetworkPolicy, types.SrcAddress, addedAddrs2, nil) + flowChanges3 := clause2.addAddrFlows(c.featureNetworkPolicy, types.SrcAddress, addedAddrs2, nil, false) err = c.featureNetworkPolicy.applyConjunctiveMatchFlows(flowChanges3) require.Nil(t, err, "Failed to invoke addAddrFlows") testAddr := NewIPAddress(net.ParseIP("192.168.1.30")) @@ -146,7 +146,7 @@ func TestPolicyRuleConjunction(t *testing.T) { nClause3 := uint8(1) clause3 := conj3.newClause(clauseID3, nClause3, mockEgressRuleTable, mockEgressDefaultTable) var addedAddrs3 = parseAddresses([]string{"192.168.1.30"}) - flowChanges4 := clause3.addAddrFlows(c.featureNetworkPolicy, types.SrcAddress, addedAddrs3, nil) + flowChanges4 := clause3.addAddrFlows(c.featureNetworkPolicy, types.SrcAddress, addedAddrs3, nil, false) err = c.featureNetworkPolicy.applyConjunctiveMatchFlows(flowChanges4) require.Nil(t, err, "Failed to invoke addAddrFlows") checkConjMatchFlowActions(t, c, clause3, testAddr, types.SrcAddress, 2, 1) @@ -643,7 +643,7 @@ func TestConjMatchFlowContextKeyConflict(t *testing.T) { id: ruleID1, } clause1 := conj1.newClause(1, 3, mockEgressRuleTable, mockEgressDefaultTable) - flowChange1 := clause1.addAddrFlows(c.featureNetworkPolicy, types.DstAddress, parseAddresses([]string{ip.String()}), nil) + flowChange1 := clause1.addAddrFlows(c.featureNetworkPolicy, types.DstAddress, parseAddresses([]string{ip.String()}), nil, false) err := c.featureNetworkPolicy.applyConjunctiveMatchFlows(flowChange1) require.Nil(t, err, "no error expect in applyConjunctiveMatchFlows") @@ -652,7 +652,7 @@ func TestConjMatchFlowContextKeyConflict(t *testing.T) { id: ruleID2, } clause2 := conj2.newClause(1, 3, mockEgressRuleTable, mockEgressDefaultTable) - flowChange2 := clause2.addAddrFlows(c.featureNetworkPolicy, types.DstAddress, parseAddresses([]string{ipNet.String()}), nil) + flowChange2 := clause2.addAddrFlows(c.featureNetworkPolicy, types.DstAddress, parseAddresses([]string{ipNet.String()}), nil, false) err = c.featureNetworkPolicy.applyConjunctiveMatchFlows(flowChange2) require.Nil(t, err, "no error expect in applyConjunctiveMatchFlows") expectedMatchKey := fmt.Sprintf("table:%d,priority:%s,matchPair:%s", EgressRuleTable.GetID(), strconv.Itoa(int(priorityNormal)), singleMatchPair.KeyString()) diff --git a/pkg/agent/openflow/pipeline.go b/pkg/agent/openflow/pipeline.go index 0abfae8c152..cb198d42eb4 100644 --- a/pkg/agent/openflow/pipeline.go +++ b/pkg/agent/openflow/pipeline.go @@ -2118,22 +2118,32 @@ func (f *featureNetworkPolicy) conjunctiveMatchFlow(tableID uint8, matchPairs [] } // defaultDropFlow generates the flow to drop packets if the match condition is matched. -func (f *featureNetworkPolicy) defaultDropFlow(table binding.Table, matchPairs []matchPair) binding.Flow { +func (f *featureNetworkPolicy) defaultDropFlow(table binding.Table, matchPairs []matchPair, enableLogging bool) binding.Flow { cookieID := f.cookieAllocator.Request(f.category).Raw() fb := table.BuildFlow(priorityNormal) for _, eachMatchPair := range matchPairs { fb = f.addFlowMatch(fb, eachMatchPair.matchKey, eachMatchPair.matchValue) } + fb = fb.Action().Drop() + + var customReason int if f.enableDenyTracking { - return fb.Action().Drop(). - Action().LoadRegMark(DispositionDropRegMark, CustomReasonDenyRegMark). - Action().SendToController(uint8(PacketInReasonNP)). - Cookie(cookieID). - Done() + customReason += CustomReasonDeny + fb = fb. + Action().LoadRegMark(DispositionDropRegMark, CustomReasonDenyRegMark) } - return fb.Action().Drop(). - Cookie(cookieID). - Done() + if enableLogging { + customReason += CustomReasonLogging + fb = fb. + Action().LoadRegMark(DispositionDropRegMark, CustomReasonDenyRegMark) + } + + if enableLogging || f.enableDenyTracking { + fb = fb. + Action().LoadToRegField(CustomReasonField, uint32(customReason)). + Action().SendToController(uint8(PacketInReasonNP)) + } + return fb.Cookie(cookieID).Done() } // dnsPacketInFlow generates the flow to send dns response packets of fqdn policy selected Pods to the fqdnController for diff --git a/pkg/controller/networkpolicy/networkpolicy_controller_test.go b/pkg/controller/networkpolicy/networkpolicy_controller_test.go index b7b4f1ca142..288cc43ada1 100644 --- a/pkg/controller/networkpolicy/networkpolicy_controller_test.go +++ b/pkg/controller/networkpolicy/networkpolicy_controller_test.go @@ -158,6 +158,7 @@ func newControllerWithoutEventHandler(k8sObjects, crdObjects []runtime.Object) ( addressGroupStore := store.NewAddressGroupStore() internalNetworkPolicyStore := store.NewNetworkPolicyStore() internalGroupStore := store.NewGroupStore() + namespaceInformer := informerFactory.Core().V1().Namespaces() networkPolicyInformer := informerFactory.Networking().V1().NetworkPolicies() tierInformer := crdInformerFactory.Crd().V1alpha1().Tiers() cnpInformer := crdInformerFactory.Crd().V1alpha1().ClusterNetworkPolicies() @@ -167,6 +168,7 @@ func newControllerWithoutEventHandler(k8sObjects, crdObjects []runtime.Object) ( npController := &NetworkPolicyController{ kubeClient: client, crdClient: crdClient, + namespaceLister: namespaceInformer.Lister(), networkPolicyInformer: networkPolicyInformer, networkPolicyLister: networkPolicyInformer.Lister(), networkPolicyListerSynced: networkPolicyInformer.Informer().HasSynced, From f15717de128edb95bc6d4d4552abec67855af973 Mon Sep 17 00:00:00 2001 From: Qiyue Yao Date: Thu, 21 Jul 2022 18:55:02 -0700 Subject: [PATCH 03/12] unit test Signed-off-by: Qiyue Yao --- .../networkpolicy_controller_test.go | 113 ++++++++++++++++-- 1 file changed, 100 insertions(+), 13 deletions(-) diff --git a/pkg/controller/networkpolicy/networkpolicy_controller_test.go b/pkg/controller/networkpolicy/networkpolicy_controller_test.go index 288cc43ada1..db5c1443df9 100644 --- a/pkg/controller/networkpolicy/networkpolicy_controller_test.go +++ b/pkg/controller/networkpolicy/networkpolicy_controller_test.go @@ -231,6 +231,30 @@ func newClientset(objects ...runtime.Object) *fake.Clientset { return client } +type mockNamespaceAnnotationLog struct{} + +func (s *mockNamespaceAnnotationLog) List(selector labels.Selector) (ret []*corev1.Namespace, err error) { + testNamespace := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: corev1.NamespaceDefault, + Name: "test-ns", + Annotations: map[string]string{"policy.antrea.io/enable-np-logging": "true"}, + }, + } + return []*corev1.Namespace{testNamespace}, nil +} + +func (s *mockNamespaceAnnotationLog) Get(name string) (*corev1.Namespace, error) { + testNamespace := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: corev1.NamespaceDefault, + Name: name, + Annotations: map[string]string{"policy.antrea.io/enable-np-logging": "true"}, + }, + } + return testNamespace, nil +} + func TestAddNetworkPolicy(t *testing.T) { selectorA := metav1.LabelSelector{MatchLabels: map[string]string{"foo1": "bar1"}} selectorB := metav1.LabelSelector{MatchLabels: map[string]string{"foo2": "bar2"}} @@ -2534,11 +2558,12 @@ func TestProcessNetworkPolicy(t *testing.T) { UID: "uidA", }, Rules: []controlplane.NetworkPolicyRule{{ - Direction: controlplane.DirectionIn, - From: matchAllPeer, - Services: nil, - Priority: defaultRulePriority, - Action: &defaultAction, + Direction: controlplane.DirectionIn, + From: matchAllPeer, + Services: nil, + Priority: defaultRulePriority, + Action: &defaultAction, + EnableLogging: false, }}, AppliedToGroups: []string{getNormalizedUID(antreatypes.NewGroupSelector("nsA", &metav1.LabelSelector{}, nil, nil, nil).NormalizedName)}, }, @@ -2628,8 +2653,9 @@ func TestProcessNetworkPolicy(t *testing.T) { Port: &int80, }, }, - Priority: defaultRulePriority, - Action: &defaultAction, + Priority: defaultRulePriority, + Action: &defaultAction, + EnableLogging: false, }, { Direction: controlplane.DirectionOut, @@ -2642,8 +2668,9 @@ func TestProcessNetworkPolicy(t *testing.T) { Port: &int81, }, }, - Priority: defaultRulePriority, - Action: &defaultAction, + Priority: defaultRulePriority, + Action: &defaultAction, + EnableLogging: false, }, }, AppliedToGroups: []string{getNormalizedUID(antreatypes.NewGroupSelector("nsA", &selectorA, nil, nil, nil).NormalizedName)}, @@ -2706,8 +2733,9 @@ func TestProcessNetworkPolicy(t *testing.T) { Port: &int80, }, }, - Priority: defaultRulePriority, - Action: &defaultAction, + Priority: defaultRulePriority, + Action: &defaultAction, + EnableLogging: false, }, { Direction: controlplane.DirectionIn, @@ -2720,8 +2748,9 @@ func TestProcessNetworkPolicy(t *testing.T) { Port: &int81, }, }, - Priority: defaultRulePriority, - Action: &defaultAction, + Priority: defaultRulePriority, + Action: &defaultAction, + EnableLogging: false, }, }, AppliedToGroups: []string{getNormalizedUID(antreatypes.NewGroupSelector("nsA", &selectorA, nil, nil, nil).NormalizedName)}, @@ -2749,6 +2778,64 @@ func TestProcessNetworkPolicy(t *testing.T) { } } +func TestProcessNetworkPolicyLogging(t *testing.T) { + tests := []struct { + name string + inputPolicy *networkingv1.NetworkPolicy + expectedPolicy *antreatypes.NetworkPolicy + expectedAppliedToGroups int + expectedAddressGroups int + }{ + { + name: "default-allow-ingress", + inputPolicy: &networkingv1.NetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{Namespace: "nsA", Name: "npA", UID: "uidA"}, + Spec: networkingv1.NetworkPolicySpec{ + PodSelector: metav1.LabelSelector{}, + PolicyTypes: []networkingv1.PolicyType{networkingv1.PolicyTypeIngress}, + Ingress: []networkingv1.NetworkPolicyIngressRule{{}}, + }, + }, + expectedPolicy: &antreatypes.NetworkPolicy{ + UID: "uidA", + Name: "uidA", + SourceRef: &controlplane.NetworkPolicyReference{ + Type: controlplane.K8sNetworkPolicy, + Namespace: "nsA", + Name: "npA", + UID: "uidA", + }, + Rules: []controlplane.NetworkPolicyRule{{ + Direction: controlplane.DirectionIn, + From: matchAllPeer, + Services: nil, + Priority: defaultRulePriority, + Action: &defaultAction, + EnableLogging: true, + }}, + AppliedToGroups: []string{getNormalizedUID(antreatypes.NewGroupSelector("nsA", &metav1.LabelSelector{}, nil, nil, nil).NormalizedName)}, + }, + expectedAppliedToGroups: 1, + expectedAddressGroups: 0, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, c := newController() + // Replace with custom lister that returns Namespace with logging Annotation. + c.namespaceLister = &mockNamespaceAnnotationLog{} + + if actualPolicy := c.processNetworkPolicy(tt.inputPolicy); !reflect.DeepEqual(actualPolicy, tt.expectedPolicy) { + t.Errorf("processNetworkPolicy() got %v, want %v", actualPolicy, tt.expectedPolicy) + } + + if actualAppliedToGroups := len(c.appliedToGroupStore.List()); actualAppliedToGroups != tt.expectedAppliedToGroups { + t.Errorf("len(appliedToGroupStore.List()) got %v, want %v", actualAppliedToGroups, tt.expectedAppliedToGroups) + } + }) + } +} + func TestPodToGroupMember(t *testing.T) { namedPod := getPod("", "", "", "", true) unNamedPod := getPod("", "", "", "", false) From bef0b2403110c33779db6f0c58dee3ba44e2b175 Mon Sep 17 00:00:00 2001 From: Qiyue Yao Date: Tue, 26 Jul 2022 00:02:42 -0700 Subject: [PATCH 04/12] e2e tests Signed-off-by: Qiyue Yao --- test/e2e/antreapolicy_test.go | 95 ++++++++++++++++++++++++++++++++++- test/e2e/framework.go | 33 +++++++++++- 2 files changed, 126 insertions(+), 2 deletions(-) diff --git a/test/e2e/antreapolicy_test.go b/test/e2e/antreapolicy_test.go index 9667d10af5f..2653718703e 100644 --- a/test/e2e/antreapolicy_test.go +++ b/test/e2e/antreapolicy_test.go @@ -15,6 +15,7 @@ package e2e import ( + "antrea.io/antrea/pkg/controller/networkpolicy" "context" "encoding/json" "fmt" @@ -1946,7 +1947,7 @@ func testANPMultipleAppliedTo(t *testing.T, data *TestData, singleRule bool) { failOnError(k8sUtils.DeleteANP(builder.Namespace, builder.Name), t) } -// testAuditLoggingBasic tests that a audit log is generated when egress drop applied +// testAuditLoggingBasic tests that audit logs are generated when egress drop applied func testAuditLoggingBasic(t *testing.T, data *TestData) { builder := &ClusterNetworkPolicySpecBuilder{} builder = builder.SetName("test-log-acnp-deny"). @@ -2034,6 +2035,96 @@ func testAuditLoggingBasic(t *testing.T, data *TestData) { failOnError(k8sUtils.CleanACNPs(), t) } +// testAuditLoggingEnableNP tests that audit logs are generated when K8s NP is applied +// tests both Allow traffic by K8s NP and Drop traffic by implicit K8s drop +func testAuditLoggingEnableNP(t *testing.T, data *TestData) { + data.updateNamespaceWithAnnotations(namespaces["x"], map[string]string{networkpolicy.EnableNPLoggingAnnotationKey: "true"}) + // Add a K8s namespaced NetworkPolicy in ns x that allow ingress traffic from + // Pod x/b to x/a which default denies other ingress including from Pod x/c to x/a + k8sNPBuilder := &NetworkPolicySpecBuilder{} + k8sNPBuilder = k8sNPBuilder.SetName(namespaces["x"], "allow-x-b-to-x-a"). + SetPodSelector(map[string]string{"pod": "a"}). + SetTypeIngress(). + AddIngress(v1.ProtocolTCP, &p80, nil, nil, nil, + map[string]string{"pod": "b"}, nil, nil, nil) + + knp, err := k8sUtils.CreateOrUpdateNetworkPolicy(k8sNPBuilder.Get()) + failOnError(err, t) + failOnError(waitForResourceReady(t, timeout, knp), t) + + // generate some traffic that will be dropped by implicit K8s policy drop + var wg sync.WaitGroup + oneProbe := func(ns1, pod1, ns2, pod2 string) { + wg.Add(1) + go func() { + defer wg.Done() + k8sUtils.Probe(ns1, pod1, ns2, pod2, p80, ProtocolTCP) + }() + } + oneProbe(namespaces["x"], "b", namespaces["x"], "a") + oneProbe(namespaces["x"], "c", namespaces["x"], "a") + wg.Wait() + + podXA, err := k8sUtils.GetPodByLabel(namespaces["x"], "a") + if err != nil { + t.Errorf("Failed to get Pod in Namespace x with label 'pod=a': %v", err) + } + // nodeName is guaranteed to be set at this stage, since the framework waits for all Pods to be in Running phase + nodeName := podXA.Spec.NodeName + antreaPodName, err := data.getAntreaPodOnNode(nodeName) + if err != nil { + t.Errorf("Error occurred when trying to get the Antrea Agent Pod running on Node %s: %v", nodeName, err) + } + cmd := []string{"cat", logDir + logfileName} + + if err := wait.Poll(1*time.Second, 10*time.Second, func() (bool, error) { + stdout, stderr, err := data.RunCommandFromPod(antreaNamespace, antreaPodName, "antrea-agent", cmd) + if err != nil || stderr != "" { + // file may not exist yet + t.Logf("Error when printing the audit log file, err: %v, stderr: %v", err, stderr) + return false, nil + } + if !strings.Contains(stdout, "K8sNetworkPolicy") { + t.Logf("Audit log file does not contain entries for 'test-log-acnp-deny' yet") + return false, nil + } + + var expectedNumEntries, actualNumEntries int + srcPods := []string{namespaces["x"] + "/b", namespaces["x"] + "/c"} + expectedLogPrefix := []string{"allow-x-b-to-x-a Allow [0-9]+ ", "K8sNetworkPolicy Drop -1 "} + destIPs, _ := podIPs[namespaces["x"]+"/a"] + for i := 0; i < len(srcPods); i++ { + srcIPs, _ := podIPs[srcPods[i]] + for _, srcIP := range srcIPs { + for _, destIP := range destIPs { + // only look for an entry in the audit log file if srcIP and + // dstIP are of the same family + if strings.Contains(srcIP, ".") != strings.Contains(destIP, ".") { + continue + } + expectedNumEntries += 1 + // The audit log should contain log entry `... Drop ...` + re := regexp.MustCompile(expectedLogPrefix[i] + srcIP + ` [0-9]+ ` + destIP + ` ` + strconv.Itoa(int(p80))) + if re.MatchString(stdout) { + actualNumEntries += 1 + } else { + t.Logf("Audit log does not contain expected entry from %s (%s) to x/a (%s)", srcPods[i], srcIP, destIP) + } + break + } + } + } + if actualNumEntries != expectedNumEntries { + t.Logf("Missing entries in audit log with K8s NP: expected %d but found %d", expectedNumEntries, actualNumEntries) + return false, nil + } + return true, nil + }); err != nil { + t.Errorf("Error when polling audit log files for required entries: %v", err) + } + failOnError(k8sUtils.DeleteNetworkPolicy(namespaces["x"], "allow-x-b-to-x-a"), t) +} + func testAppliedToPerRule(t *testing.T) { builder := &AntreaNetworkPolicySpecBuilder{} builder = builder.SetName(namespaces["y"], "np1").SetPriority(1.0) @@ -3556,7 +3647,9 @@ func TestAntreaPolicy(t *testing.T) { t.Run("TestGroupAuditLogging", func(t *testing.T) { t.Run("Case=AuditLoggingBasic", func(t *testing.T) { testAuditLoggingBasic(t, data) }) + t.Run("Case=AuditLoggingEnableNP", func(t *testing.T) { testAuditLoggingEnableNP(t, data) }) }) + printResults() t.Run("TestMulticastNP", func(t *testing.T) { skipIfMulticastDisabled(t) diff --git a/test/e2e/framework.go b/test/e2e/framework.go index 40d8ee35c42..504cf7af16b 100644 --- a/test/e2e/framework.go +++ b/test/e2e/framework.go @@ -612,8 +612,39 @@ func (data *TestData) CreateNamespace(namespace string, mutateFunc func(*corev1. return nil } +func (data *TestData) UpdateNamespace(namespace string, mutateFunc func(*corev1.Namespace)) error { + ns := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: namespace, + }, + } + if mutateFunc != nil { + mutateFunc(ns) + } + if ns, err := data.clientset.CoreV1().Namespaces().Update(context.TODO(), ns, metav1.UpdateOptions{}); err != nil { + // Check namespace phase + if ns.Status.Phase == corev1.NamespaceTerminating { + return fmt.Errorf("error when updating '%s' Namespace: namespace is in 'Terminating' phase", namespace) + } + return fmt.Errorf("error when updating '%s' Namespace: %v", namespace, err) + } + return nil +} + // createNamespaceWithAnnotations creates the namespace with Annotations. func (data *TestData) createNamespaceWithAnnotations(namespace string, annotations map[string]string) error { + mutateFunc := data.generateNamespaceAnnotationsMutateFunc(annotations) + return data.CreateNamespace(namespace, mutateFunc) +} + +// updateNamespaceWithAnnotations updates the given namespace with Annotations. +func (data *TestData) updateNamespaceWithAnnotations(namespace string, annotations map[string]string) error { + mutateFunc := data.generateNamespaceAnnotationsMutateFunc(annotations) + return data.UpdateNamespace(namespace, mutateFunc) +} + +// generateAnnotationsMutateFunc generates a mutate function to add given annotations to a namespace. +func (data *TestData) generateNamespaceAnnotationsMutateFunc(annotations map[string]string) func(*corev1.Namespace) { var mutateFunc func(*corev1.Namespace) if annotations != nil { mutateFunc = func(namespace *corev1.Namespace) { @@ -625,7 +656,7 @@ func (data *TestData) createNamespaceWithAnnotations(namespace string, annotatio } } } - return data.CreateNamespace(namespace, mutateFunc) + return mutateFunc } // DeleteNamespace deletes the provided Namespace, and waits for deletion to actually complete if timeout>=0 From b28c370e8c3dcdd838e0af64cf743f3d4da4185e Mon Sep 17 00:00:00 2001 From: Qiyue Yao Date: Tue, 26 Jul 2022 12:58:32 -0700 Subject: [PATCH 05/12] documentation Signed-off-by: Qiyue Yao --- docs/antrea-network-policy.md | 19 +++++++++++++++++++ pkg/agent/openflow/network_policy.go | 2 +- .../networkpolicy_controller_test.go | 8 ++++---- test/e2e/antreapolicy_test.go | 4 ++-- 4 files changed, 26 insertions(+), 7 deletions(-) diff --git a/docs/antrea-network-policy.md b/docs/antrea-network-policy.md index ff8186eb2cb..d6c51fcb28c 100644 --- a/docs/antrea-network-policy.md +++ b/docs/antrea-network-policy.md @@ -652,6 +652,25 @@ The rules are logged in the following format: 2021/06/24 23:56:41.346165 AntreaPolicyEgressRule AntreaNetworkPolicy:default/test-anp Drop 44900 10.10.1.65 35402 10.0.0.5 80 TCP 60 [3 packets in 1.011379442s] ``` +Kubernetes Network Policies can also be audited using Antrea logging to the same file +(`/var/log/antrea/networkpolicy/np.log`). Set the Namespace Annotations to +`policy.antrea.io/enable-np-logging: "true"`, then all the rules of Kubernetes +Network Policies in this Namespace will be processed similar to setting their +`enableLogging` field to true. Packet of any connection that matches the rules +will be logged with Kubernetes Network Policy reference, but packets dropped by +implicit default drop will only be logged with consistent name `K8sNetworkPolicy` +for reference. The rules are logged in the following format: + +```text +