From 5679831ace855eb6f34d1060bada25803abd3098 Mon Sep 17 00:00:00 2001 From: Nick Sardo Date: Fri, 7 Apr 2017 14:41:48 -0700 Subject: [PATCH] change notes on firewall targets --- controllers/gce/firewalls/fakes.go | 2 ++ controllers/gce/firewalls/firewalls.go | 2 ++ controllers/gce/firewalls/firewalls_test.go | 6 ++---- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/controllers/gce/firewalls/fakes.go b/controllers/gce/firewalls/fakes.go index dc0723e0c..852e22451 100644 --- a/controllers/gce/firewalls/fakes.go +++ b/controllers/gce/firewalls/fakes.go @@ -63,6 +63,7 @@ func (f *fakeFirewallsProvider) CreateFirewall(name, msgTag string, srcRange net Name: prefixedName, SourceRanges: srcRange.StringSlice(), Allowed: []*compute.FirewallAllowed{{Ports: strPorts}}, + TargetTags: hosts, // WARNING: This is actually not correct, but good enough for testing this package } return nil } @@ -96,6 +97,7 @@ func (f *fakeFirewallsProvider) UpdateFirewall(name, msgTag string, srcRange net Name: name, SourceRanges: srcRange.StringSlice(), Allowed: []*compute.FirewallAllowed{{Ports: strPorts}}, + TargetTags: hosts, // WARNING: This is actually not correct, but good enough for testing this package } return nil } diff --git a/controllers/gce/firewalls/firewalls.go b/controllers/gce/firewalls/firewalls.go index 0e01529dc..3aa07d80d 100644 --- a/controllers/gce/firewalls/firewalls.go +++ b/controllers/gce/firewalls/firewalls.go @@ -79,6 +79,8 @@ func (fr *FirewallRules) Sync(nodePorts []int64, nodeNames []string) error { requiredCIDRs := sets.NewString(l7SrcRanges...) existingCIDRs := sets.NewString(rule.SourceRanges...) + // Do not update if ports and source cidrs are not outdated. + // NOTE: We are not checking if nodeNames matches the firwall targetTags if requiredPorts.Equal(existingPorts) && requiredCIDRs.Equal(existingCIDRs) { return nil } diff --git a/controllers/gce/firewalls/firewalls_test.go b/controllers/gce/firewalls/firewalls_test.go index 5abd9b36e..fb3708f39 100644 --- a/controllers/gce/firewalls/firewalls_test.go +++ b/controllers/gce/firewalls/firewalls_test.go @@ -47,7 +47,8 @@ func TestSyncFirewallPool(t *testing.T) { } verifyFirewallRule(fwp, ruleName, nodePorts, nodes, l7SrcRanges, t) - // Add node and expect firwall to change nodes list + // Add node and expect firwall to remain the same + // NOTE: See computeHostTag(..) in gce cloudprovider nodes = []string{"node-a", "node-b", "node-c", "node-d"} err = fp.Sync(nodePorts, nodes) if err != nil { @@ -89,7 +90,4 @@ func verifyFirewallRule(fwp *fakeFirewallsProvider, ruleName string, expectedPor if !sets.NewString(f.SourceRanges...).Equal(sets.NewString(expectedCIDRs...)) { t.Errorf("source CIDRs doesn't equal expected CIDRs. Actual: %v, Expected: %v", f.SourceRanges, expectedCIDRs) } - - // Verify firwall rule has correct nodes - // TODO: Check host tags are updated }