From 7692426a4c1e817cb2f67f81e7e1e0fa9b57d67e Mon Sep 17 00:00:00 2001 From: pulkit_jain <42600268+jainpulkit22@users.noreply.github.com> Date: Fri, 25 Feb 2022 20:30:05 +0530 Subject: [PATCH] Fix dumping OVS flows of a NetworkPolicy (#3335) Fixes #3306. This commit fixes the nil pointer dereference error while dumping ovsflows of NetworkPolicy, and adds a test to validate the changes. Signed-off-by: Pulkit Jain --- pkg/agent/openflow/network_policy.go | 8 +++++++- pkg/agent/openflow/network_policy_test.go | 5 +++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/pkg/agent/openflow/network_policy.go b/pkg/agent/openflow/network_policy.go index badd3020c45..fea261c7e6f 100644 --- a/pkg/agent/openflow/network_policy.go +++ b/pkg/agent/openflow/network_policy.go @@ -525,7 +525,8 @@ type policyRuleConjunction struct { serviceClause *clause actionFlows []binding.Flow metricFlows []binding.Flow - // NetworkPolicy reference information for debugging usage. + // NetworkPolicy reference information for debugging usage, its value can be nil + // for conjunctions that are not built for a specific NetworkPolicy, e.g. DNS packetin Conjunction. npRef *v1beta2.NetworkPolicyReference ruleTableID uint8 } @@ -1455,6 +1456,11 @@ func (c *client) GetNetworkPolicyFlowKeys(npName, npNamespace string) []string { for _, conjObj := range c.policyCache.List() { conj := conjObj.(*policyRuleConjunction) + // If the NetworkPolicyReference in the policyRuleConjunction is nil then that entry in client's + // policyCache should be ignored because here we need to dump flows of NetworkPolicy. + if conj.npRef == nil { + continue + } if conj.npRef.Name == npName && conj.npRef.Namespace == npNamespace { // There can be duplicated flows added due to conjunctive matches // shared by multiple policy rules (clauses). diff --git a/pkg/agent/openflow/network_policy_test.go b/pkg/agent/openflow/network_policy_test.go index 4b547fa9f97..1e9a5774bb9 100644 --- a/pkg/agent/openflow/network_policy_test.go +++ b/pkg/agent/openflow/network_policy_test.go @@ -159,6 +159,11 @@ func TestInstallPolicyRuleFlows(t *testing.T) { c.networkConfig = &config.NetworkConfig{IPv4Enabled: true} c.ipProtocols = []binding.Protocol{binding.ProtocolIP} defaultAction := crdv1alpha1.RuleActionAllow + // Create a policyRuleConjunction for the dns response interception flows + // to ensure nil NetworkPolicyReference is handled correctly by GetNetworkPolicyFlowKeys. + dnsID := uint32(1) + require.NoError(t, c.NewDNSpacketInConjunction(dnsID)) + ruleID1 := uint32(101) rule1 := &types.PolicyRule{ Direction: v1beta2.DirectionOut,