From 2555e491e1884db56250b97f3dca78c456c6afb8 Mon Sep 17 00:00:00 2001 From: Mike Bryant Date: Wed, 23 Aug 2017 01:21:15 +0100 Subject: [PATCH 1/2] fix: Missing nil checks Fixes #3095 --- npc/rule.go | 18 ++++++++++++------ npc/rule_test.go | 16 ++++++++++++++++ 2 files changed, 28 insertions(+), 6 deletions(-) create mode 100644 npc/rule_test.go diff --git a/npc/rule.go b/npc/rule.go index 5ed49129..d2fac379 100644 --- a/npc/rule.go +++ b/npc/rule.go @@ -30,13 +30,19 @@ func newRuleSpec(proto *string, srcHost *selectorSpec, dstHost *selectorSpec, ds args = append(args, "--dport", *dstPort) } args = append(args, "-j", "ACCEPT") - srcComment := "" - if srcHost.nsName != "" { - srcComment = fmt.Sprintf("pods: namespace: %s, selector: %s", srcHost.nsName, srcHost.key) - } else { - srcComment = fmt.Sprintf("namespaces: selector: %s", srcHost.key) + srcComment := "anywhere" + if srcHost != nil { + if srcHost.nsName != "" { + srcComment = fmt.Sprintf("pods: namespace: %s, selector: %s", srcHost.nsName, srcHost.key) + } else { + srcComment = fmt.Sprintf("namespaces: selector: %s", srcHost.key) + } } - args = append(args, "-m", "comment", "--comment", fmt.Sprintf("%s -> namespace: %s, key: %s", srcComment, dstHost.nsName, dstHost.key)) + dstComment := "anywhere" + if dstHost != nil { + dstComment = fmt.Sprintf("pods: namespace: %s, selector: %s", dstHost.nsName, dstHost.key) + } + args = append(args, "-m", "comment", "--comment", fmt.Sprintf("%s -> %s", srcComment, dstComment)) key := strings.Join(args, " ") return &ruleSpec{key, args} diff --git a/npc/rule_test.go b/npc/rule_test.go new file mode 100644 index 00000000..dee1802f --- /dev/null +++ b/npc/rule_test.go @@ -0,0 +1,16 @@ +package npc + +import ( + "github.com/stretchr/testify/require" + "testing" +) + +func TestRegressionHandleNil3095(t *testing.T) { + // Test for handling nil values + // https://github.com/weaveworks/weave/issues/3095 + + rs := newRuleSpec(nil, nil, nil, nil) + + require.NotEqual(t, rs.key, "") + +} From de1d409c6ac44c9d68c358a5de51ab79fbb06211 Mon Sep 17 00:00:00 2001 From: Mike Bryant Date: Wed, 23 Aug 2017 09:40:44 +0100 Subject: [PATCH 2/2] Fix review comments --- npc/rule.go | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/npc/rule.go b/npc/rule.go index d2fac379..fd695f98 100644 --- a/npc/rule.go +++ b/npc/rule.go @@ -20,18 +20,9 @@ func newRuleSpec(proto *string, srcHost *selectorSpec, dstHost *selectorSpec, ds if proto != nil { args = append(args, "-p", *proto) } - if srcHost != nil { - args = append(args, "-m", "set", "--match-set", string(srcHost.ipsetName), "src") - } - if dstHost != nil { - args = append(args, "-m", "set", "--match-set", string(dstHost.ipsetName), "dst") - } - if dstPort != nil { - args = append(args, "--dport", *dstPort) - } - args = append(args, "-j", "ACCEPT") srcComment := "anywhere" if srcHost != nil { + args = append(args, "-m", "set", "--match-set", string(srcHost.ipsetName), "src") if srcHost.nsName != "" { srcComment = fmt.Sprintf("pods: namespace: %s, selector: %s", srcHost.nsName, srcHost.key) } else { @@ -40,8 +31,13 @@ func newRuleSpec(proto *string, srcHost *selectorSpec, dstHost *selectorSpec, ds } dstComment := "anywhere" if dstHost != nil { + args = append(args, "-m", "set", "--match-set", string(dstHost.ipsetName), "dst") dstComment = fmt.Sprintf("pods: namespace: %s, selector: %s", dstHost.nsName, dstHost.key) } + if dstPort != nil { + args = append(args, "--dport", *dstPort) + } + args = append(args, "-j", "ACCEPT") args = append(args, "-m", "comment", "--comment", fmt.Sprintf("%s -> %s", srcComment, dstComment)) key := strings.Join(args, " ")