Skip to content

Commit

Permalink
Fix dumping OVS flows of a NetworkPolicy
Browse files Browse the repository at this point in the history
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 <jainpu@vmware.com>
  • Loading branch information
Pulkit Jain committed Feb 24, 2022
1 parent 0ffcfc9 commit a6c1efa
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 2 deletions.
8 changes: 7 additions & 1 deletion pkg/agent/openflow/network_policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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).
Expand Down
8 changes: 7 additions & 1 deletion pkg/agent/openflow/network_policy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,12 @@ 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)
err := c.NewDNSpacketInConjunction(dnsID)
require.Nil(t, err)

ruleID1 := uint32(101)
rule1 := &types.PolicyRule{
Direction: v1beta2.DirectionOut,
Expand Down Expand Up @@ -189,7 +195,7 @@ func TestInstallPolicyRuleFlows(t *testing.T) {
assert.Equal(t, len(rule1.From), getChangedFlowCount(dropFlows))
assert.Equal(t, 0, getChangedFlowCount(matchFlows))
assert.Equal(t, 2, getDenyAllRuleOPCount(matchFlows, insertion))
err := c.applyConjunctiveMatchFlows(ctxChanges)
err = c.applyConjunctiveMatchFlows(ctxChanges)
require.Nil(t, err)

ruleID2 := uint32(102)
Expand Down

0 comments on commit a6c1efa

Please sign in to comment.