[GLBC] Update firewall source ranges if outdated (#574)

check firewall rule source ranges
This commit is contained in:
Nick Sardo 2017-04-11 09:01:42 -07:00 committed by GitHub
parent c36d6ffbff
commit 987540f8f6
5 changed files with 182 additions and 56 deletions

View file

@ -67,7 +67,7 @@ func NewFakeClusterManager(clusterName, firewallName string) *fakeClusterManager
testDefaultBeNodePort, testDefaultBeNodePort,
namer, namer,
) )
frPool := firewalls.NewFirewallPool(firewalls.NewFakeFirewallRules(), namer) frPool := firewalls.NewFirewallPool(firewalls.NewFakeFirewallsProvider(namer), namer)
cm := &ClusterManager{ cm := &ClusterManager{
ClusterNamer: namer, ClusterNamer: namer,
instancePool: nodePool, instancePool: nodePool,

View file

@ -18,6 +18,7 @@ package firewalls
import ( import (
"fmt" "fmt"
"strconv"
compute "google.golang.org/api/compute/v1" compute "google.golang.org/api/compute/v1"
netset "k8s.io/kubernetes/pkg/util/net/sets" netset "k8s.io/kubernetes/pkg/util/net/sets"
@ -25,81 +26,78 @@ import (
"k8s.io/ingress/controllers/gce/utils" "k8s.io/ingress/controllers/gce/utils"
) )
type fakeFirewallRules struct { type fakeFirewallsProvider struct {
fw []*compute.Firewall fw map[string]*compute.Firewall
namer utils.Namer namer *utils.Namer
} }
func (f *fakeFirewallRules) GetFirewall(name string) (*compute.Firewall, error) { // NewFakeFirewallsProvider creates a fake for firewall rules.
for _, rule := range f.fw { func NewFakeFirewallsProvider(namer *utils.Namer) *fakeFirewallsProvider {
if rule.Name == name { return &fakeFirewallsProvider{
return rule, nil fw: make(map[string]*compute.Firewall),
} namer: namer,
} }
return nil, fmt.Errorf("firewall rule %v not found", name)
} }
func (f *fakeFirewallRules) CreateFirewall(name, msgTag string, srcRange netset.IPNet, ports []int64, hosts []string) error { func (f *fakeFirewallsProvider) GetFirewall(prefixedName string) (*compute.Firewall, error) {
rule, exists := f.fw[prefixedName]
if exists {
return rule, nil
}
return nil, utils.FakeGoogleAPINotFoundErr()
}
func (f *fakeFirewallsProvider) CreateFirewall(name, msgTag string, srcRange netset.IPNet, ports []int64, hosts []string) error {
prefixedName := f.namer.FrName(name)
strPorts := []string{} strPorts := []string{}
for _, p := range ports { for _, p := range ports {
strPorts = append(strPorts, fmt.Sprintf("%v", p)) strPorts = append(strPorts, strconv.FormatInt(p, 10))
} }
f.fw = append(f.fw, &compute.Firewall{ if _, exists := f.fw[prefixedName]; exists {
return fmt.Errorf("firewall rule %v already exists", prefixedName)
}
f.fw[prefixedName] = &compute.Firewall{
// To accurately mimic the cloudprovider we need to add the k8s-fw // To accurately mimic the cloudprovider we need to add the k8s-fw
// prefix to the given rule name. // prefix to the given rule name.
Name: f.namer.FrName(name), Name: prefixedName,
SourceRanges: srcRange.StringSlice(), SourceRanges: srcRange.StringSlice(),
Allowed: []*compute.FirewallAllowed{{Ports: strPorts}}, Allowed: []*compute.FirewallAllowed{{Ports: strPorts}},
}) TargetTags: hosts, // WARNING: This is actually not correct, but good enough for testing this package
}
return nil return nil
} }
func (f *fakeFirewallRules) DeleteFirewall(name string) error { func (f *fakeFirewallsProvider) DeleteFirewall(name string) error {
firewalls := []*compute.Firewall{}
exists := false
// We need the full name for the same reason as CreateFirewall. // We need the full name for the same reason as CreateFirewall.
name = f.namer.FrName(name) prefixedName := f.namer.FrName(name)
for _, rule := range f.fw { _, exists := f.fw[prefixedName]
if rule.Name == name {
exists = true
continue
}
firewalls = append(firewalls, rule)
}
if !exists { if !exists {
return fmt.Errorf("failed to find health check %v", name) return utils.FakeGoogleAPINotFoundErr()
} }
f.fw = firewalls
delete(f.fw, prefixedName)
return nil return nil
} }
func (f *fakeFirewallRules) UpdateFirewall(name, msgTag string, srcRange netset.IPNet, ports []int64, hosts []string) error { func (f *fakeFirewallsProvider) UpdateFirewall(name, msgTag string, srcRange netset.IPNet, ports []int64, hosts []string) error {
var exists bool
strPorts := []string{} strPorts := []string{}
for _, p := range ports { for _, p := range ports {
strPorts = append(strPorts, fmt.Sprintf("%v", p)) strPorts = append(strPorts, strconv.FormatInt(p, 10))
} }
// To accurately mimic the cloudprovider we need to add the k8s-fw // We need the full name for the same reason as CreateFirewall.
// prefix to the given rule name. prefixedName := f.namer.FrName(name)
name = f.namer.FrName(name) _, exists := f.fw[prefixedName]
for i := range f.fw { if !exists {
if f.fw[i].Name == name { return fmt.Errorf("update failed for rule %v, srcRange %v ports %v, rule not found", prefixedName, srcRange, ports)
exists = true
f.fw[i] = &compute.Firewall{
Name: name,
SourceRanges: srcRange.StringSlice(),
Allowed: []*compute.FirewallAllowed{{Ports: strPorts}},
}
}
} }
if exists {
return nil
}
return fmt.Errorf("update failed for rule %v, srcRange %v ports %v, rule not found", name, srcRange, ports)
}
// NewFakeFirewallRules creates a fake for firewall rules. f.fw[prefixedName] = &compute.Firewall{
func NewFakeFirewallRules() *fakeFirewallRules { Name: name,
return &fakeFirewallRules{fw: []*compute.Firewall{}, namer: utils.Namer{}} 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
} }

View file

@ -75,17 +75,30 @@ func (fr *FirewallRules) Sync(nodePorts []int64, nodeNames []string) error {
existingPorts.Insert(p) existingPorts.Insert(p)
} }
} }
if requiredPorts.Equal(existingPorts) {
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 firewall targetTags
if requiredPorts.Equal(existingPorts) && requiredCIDRs.Equal(existingCIDRs) {
glog.V(4).Info("Firewall does not need update of ports or source ranges")
return nil return nil
} }
glog.V(3).Infof("Firewall rule %v already exists, updating nodeports %v", name, nodePorts)
return fr.cloud.UpdateFirewall(suffix, "GCE L7 firewall rule", fr.srcRanges, nodePorts, nodeNames) glog.V(3).Infof("Firewall %v already exists, updating nodeports %v", name, nodePorts)
return fr.cloud.UpdateFirewall(suffix, "GCE L7 firewall", fr.srcRanges, nodePorts, nodeNames)
} }
// Shutdown shuts down this firewall rules manager. // Shutdown shuts down this firewall rules manager.
func (fr *FirewallRules) Shutdown() error { func (fr *FirewallRules) Shutdown() error {
glog.Infof("Deleting firewall rule with suffix %v", fr.namer.FrSuffix()) glog.Infof("Deleting firewall with suffix %v", fr.namer.FrSuffix())
return fr.cloud.DeleteFirewall(fr.namer.FrSuffix()) err := fr.cloud.DeleteFirewall(fr.namer.FrSuffix())
if err != nil && utils.IsHTTPErrorCode(err, 404) {
glog.Infof("Firewall with suffix %v didn't exist at Shutdown", fr.namer.FrSuffix())
return nil
}
return err
} }
// GetFirewall just returns the firewall object corresponding to the given name. // GetFirewall just returns the firewall object corresponding to the given name.

View file

@ -0,0 +1,110 @@
/*
Copyright 2015 The Kubernetes Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
package firewalls
import (
"strconv"
"testing"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/ingress/controllers/gce/utils"
netset "k8s.io/kubernetes/pkg/util/net/sets"
)
const allCIDR = "0.0.0.0/0"
func TestSyncFirewallPool(t *testing.T) {
namer := utils.NewNamer("ABC", "XYZ")
fwp := NewFakeFirewallsProvider(namer)
fp := NewFirewallPool(fwp, namer)
ruleName := namer.FrName(namer.FrSuffix())
// Test creating a firewall rule via Sync
nodePorts := []int64{80, 443, 3000}
nodes := []string{"node-a", "node-b", "node-c"}
err := fp.Sync(nodePorts, nodes)
if err != nil {
t.Errorf("unexpected err when syncing firewall, err: %v", err)
}
verifyFirewallRule(fwp, ruleName, nodePorts, nodes, l7SrcRanges, t)
// Sync to fewer ports
nodePorts = []int64{80, 443}
err = fp.Sync(nodePorts, nodes)
if err != nil {
t.Errorf("unexpected err when syncing firewall, err: %v", err)
}
verifyFirewallRule(fwp, ruleName, nodePorts, nodes, l7SrcRanges, t)
srcRanges, _ := netset.ParseIPNets(allCIDR)
err = fwp.UpdateFirewall(namer.FrSuffix(), "", srcRanges, nodePorts, nodes)
if err != nil {
t.Errorf("failed to update firewall rule, err: %v", err)
}
verifyFirewallRule(fwp, ruleName, nodePorts, nodes, []string{allCIDR}, t)
// Run Sync and expect l7 src ranges to be returned
err = fp.Sync(nodePorts, nodes)
if err != nil {
t.Errorf("unexpected err when syncing firewall, err: %v", err)
}
verifyFirewallRule(fwp, ruleName, nodePorts, nodes, l7SrcRanges, t)
// Add node and expect firewall 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 {
t.Errorf("unexpected err when syncing firewall, err: %v", err)
}
verifyFirewallRule(fwp, ruleName, nodePorts, nodes, l7SrcRanges, t)
// Remove all ports and expect firewall rule to disappear
nodePorts = []int64{}
err = fp.Sync(nodePorts, nodes)
if err != nil {
t.Errorf("unexpected err when syncing firewall, err: %v", err)
}
err = fp.Shutdown()
if err != nil {
t.Errorf("unexpected err when deleting firewall, err: %v", err)
}
}
func verifyFirewallRule(fwp *fakeFirewallsProvider, ruleName string, expectedPorts []int64, expectedNodes, expectedCIDRs []string, t *testing.T) {
var strPorts []string
for _, v := range expectedPorts {
strPorts = append(strPorts, strconv.FormatInt(v, 10))
}
// Verify firewall rule was created
f, err := fwp.GetFirewall(ruleName)
if err != nil {
t.Errorf("could not retrieve firewall via cloud api, err %v", err)
}
// Verify firewall rule has correct ports
if !sets.NewString(f.Allowed[0].Ports...).Equal(sets.NewString(strPorts...)) {
t.Errorf("allowed ports doesn't equal expected ports, Actual: %v, Expected: %v", f.Allowed[0].Ports, strPorts)
}
// Verify firewall rule has correct CIDRs
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)
}
}

View file

@ -312,6 +312,11 @@ func (g GCEURLMap) PutDefaultBackend(d *compute.BackendService) {
} }
} }
// FakeNotFoundErr creates a NotFound error with type googleapi.Error
func FakeGoogleAPINotFoundErr() *googleapi.Error {
return &googleapi.Error{Code: 404}
}
// IsHTTPErrorCode checks if the given error matches the given HTTP Error code. // IsHTTPErrorCode checks if the given error matches the given HTTP Error code.
// For this to work the error must be a googleapi Error. // For this to work the error must be a googleapi Error.
func IsHTTPErrorCode(err error, code int) bool { func IsHTTPErrorCode(err error, code int) bool {