github/kubernetes
1
0
Fork 0
mirror of https://github.com/k3s-io/kubernetes.git synced 2025-07-23 19:56:01 +00:00
Code Issues Projects Releases Wiki Activity

Merge pull request #84622 from prameshj/rename-fwrules

Browse Source 
Rename ILB FirewallRules to be consistent with other resource names.
This commit is contained in:
Kubernetes Prow Robot 2019-11-07 04:35:25 -08:00 committed by GitHub
commit 303de85dd1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 130 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

55
staging/src/k8s.io/legacy-cloud-providers/gce/gce_loadbalancer_internal.go
View File

@ -22,11 +22,11 @@ import (
"context"
"encoding/json"
"fmt"
"github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/meta"
"strconv"
"strings"
"github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud"
"github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/meta"
computebeta "google.golang.org/api/compute/v0.beta"
compute "google.golang.org/api/compute/v1"
"k8s.io/api/core/v1"
@ -248,14 +248,26 @@ func (g *Cloud) ensureInternalLoadBalancerDeleted(clusterName, clusterID string,
return err
}
klog.V(2).Infof("ensureInternalLoadBalancerDeleted(%v): deleting firewall for traffic", loadBalancerName)
if err := ignoreNotFound(g.DeleteFirewall(loadBalancerName)); err != nil {
if isForbidden(err) && g.OnXPN() {
klog.V(2).Infof("ensureInternalLoadBalancerDeleted(%v): could not delete traffic firewall on XPN cluster. Raising event.", loadBalancerName)
g.raiseFirewallChangeNeededEvent(svc, FirewallToGCloudDeleteCmd(loadBalancerName, g.NetworkProjectID()))
} else {
deleteFunc := func(fwName string) error {
if err := ignoreNotFound(g.DeleteFirewall(fwName)); err != nil {
if isForbidden(err) && g.OnXPN() {
klog.V(2).Infof("ensureInternalLoadBalancerDeleted(%v): could not delete traffic firewall on XPN cluster. Raising event.", loadBalancerName)
g.raiseFirewallChangeNeededEvent(svc, FirewallToGCloudDeleteCmd(fwName, g.NetworkProjectID()))
return nil
}
return err
}
return nil
}
fwName := MakeFirewallName(loadBalancerName)
klog.V(2).Infof("ensureInternalLoadBalancerDeleted(%v): deleting firewall %s for traffic",
loadBalancerName, fwName)
if err := deleteFunc(fwName); err != nil {
return err
}
klog.V(2).Infof("ensureInternalLoadBalancerDeleted(%v): deleting legacy name firewall for traffic", loadBalancerName)
if err := deleteFunc(loadBalancerName); err != nil {
return err
}
hcName := makeHealthCheckName(loadBalancerName, clusterID, sharedHealthCheck)
@ -317,7 +329,7 @@ func (g *Cloud) teardownInternalHealthCheckAndFirewall(svc *v1.Service, hcName s
return nil
}
func (g *Cloud) ensureInternalFirewall(svc *v1.Service, fwName, fwDesc string, sourceRanges []string, ports []string, protocol v1.Protocol, nodes []*v1.Node) error {
func (g *Cloud) ensureInternalFirewall(svc *v1.Service, fwName, fwDesc string, sourceRanges []string, ports []string, protocol v1.Protocol, nodes []*v1.Node, legacyFwName string) error {
klog.V(2).Infof("ensureInternalFirewall(%v): checking existing firewall", fwName)
targetTags, err := g.GetNodeTags(nodeNames(nodes))
if err != nil {
@ -328,6 +340,29 @@ func (g *Cloud) ensureInternalFirewall(svc *v1.Service, fwName, fwDesc string, s
if err != nil && !isNotFound(err) {
return err
}
// TODO(84821) Remove legacyFwName logic after 3 releases, so there would have been atleast 2 master upgrades that would
// have triggered service sync and deletion of the legacy rules.
if legacyFwName != "" {
// Check for firewall named with the legacy naming scheme and delete if found.
legacyFirewall, err := g.GetFirewall(legacyFwName)
if err != nil && !isNotFound(err) {
return err
}
if legacyFirewall != nil && existingFirewall != nil {
// Delete the legacyFirewall rule if the new one was already created. If not, it will be deleted in the
// next sync or when the service is deleted.
defer func() {
err = g.DeleteFirewall(legacyFwName)
if err != nil {
klog.Errorf("Failed to delete legacy firewall %s for service %s/%s, err %v",
legacyFwName, svc.Namespace, svc.Name, err)
} else {
klog.V(2).Infof("Successfully deleted legacy firewall %s for service %s/%s",
legacyFwName, svc.Namespace, svc.Name)
}
}()
}
}
expectedFirewall := &compute.Firewall{
Name: fwName,
@ -376,7 +411,7 @@ func (g *Cloud) ensureInternalFirewalls(loadBalancerName, ipAddress, clusterID s
if err != nil {
return err
}
err = g.ensureInternalFirewall(svc, loadBalancerName, fwDesc, sourceRanges.StringSlice(), ports, protocol, nodes)
err = g.ensureInternalFirewall(svc, MakeFirewallName(loadBalancerName), fwDesc, sourceRanges.StringSlice(), ports, protocol, nodes, loadBalancerName)
if err != nil {
return err
}
@ -384,7 +419,7 @@ func (g *Cloud) ensureInternalFirewalls(loadBalancerName, ipAddress, clusterID s
// Second firewall is for health checking nodes / services
fwHCName := makeHealthCheckFirewallName(loadBalancerName, clusterID, sharedHealthCheck)
hcSrcRanges := L4LoadBalancerSrcRanges()
return g.ensureInternalFirewall(svc, fwHCName, "", hcSrcRanges, []string{healthCheckPort}, v1.ProtocolTCP, nodes)
return g.ensureInternalFirewall(svc, fwHCName, "", hcSrcRanges, []string{healthCheckPort}, v1.ProtocolTCP, nodes, "")
}
func (g *Cloud) ensureInternalHealthCheck(name string, svcName types.NamespacedName, shared bool, path string, port int32) (*compute.HealthCheck, error) {

90
staging/src/k8s.io/legacy-cloud-providers/gce/gce_loadbalancer_internal_test.go
View File

@ -21,7 +21,6 @@ package gce
import (
"context"
"fmt"
"github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/meta"
"strings"
"testing"
@ -29,6 +28,7 @@ import (
"github.com/stretchr/testify/require"
"github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud"
"github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/meta"
"github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/mock"
computebeta "google.golang.org/api/compute/v0.beta"
compute "google.golang.org/api/compute/v1"
@ -305,7 +305,7 @@ func TestEnsureInternalLoadBalancerClearPreviousResources(t *testing.T) {
rule, _ := gce.GetRegionForwardingRule(lbName, gce.region)
assert.NotEqual(t, existingFwdRule, rule)
firewall, err := gce.GetFirewall(lbName)
firewall, err := gce.GetFirewall(MakeFirewallName(lbName))
require.NoError(t, err)
assert.NotEqual(t, firewall, existingFirewall)
@ -590,12 +590,87 @@ func TestClearPreviousInternalResources(t *testing.T) {
assert.Nil(t, hc1, "HealthCheck should be deleted.")
}
func TestEnsureInternalFirewallDeletesLegacyFirewall(t *testing.T) {
gce, err := fakeGCECloud(DefaultTestClusterValues())
require.NoError(t, err)
vals := DefaultTestClusterValues()
svc := fakeLoadbalancerService(string(LBTypeInternal))
lbName := gce.GetLoadBalancerName(context.TODO(), "", svc)
fwName := MakeFirewallName(lbName)
c := gce.c.(*cloud.MockGCE)
c.MockFirewalls.InsertHook = nil
c.MockFirewalls.UpdateHook = nil
nodes, err := createAndInsertNodes(gce, []string{"test-node-1"}, vals.ZoneName)
require.NoError(t, err)
sourceRange := []string{"10.0.0.0/20"}
// Manually create a firewall rule with the legacy name - lbName
gce.ensureInternalFirewall(
svc,
lbName,
"firewall with legacy name",
sourceRange,
[]string{"123"},
v1.ProtocolTCP,
nodes,
"")
if err != nil {
t.Errorf("Unexpected error %v when ensuring legacy firewall %s for svc %+v", err, lbName, svc)
}
// Now ensure the firewall again with the correct name to simulate a sync after updating to new code.
err = gce.ensureInternalFirewall(
svc,
fwName,
"firewall with new name",
sourceRange,
[]string{"123", "456"},
v1.ProtocolTCP,
nodes,
lbName)
if err != nil {
t.Errorf("Unexpected error %v when ensuring firewall %s for svc %+v", err, fwName, svc)
}
existingFirewall, err := gce.GetFirewall(fwName)
require.Nil(t, err)
require.NotNil(t, existingFirewall)
// Existing firewall will not be deleted yet since this was the first sync with the new rule created.
existingLegacyFirewall, err := gce.GetFirewall(lbName)
require.Nil(t, err)
require.NotNil(t, existingLegacyFirewall)
// Now ensure the firewall again to simulate a second sync where the old rule will be deleted.
err = gce.ensureInternalFirewall(
svc,
fwName,
"firewall with new name",
sourceRange,
[]string{"123", "456", "789"},
v1.ProtocolTCP,
nodes,
lbName)
if err != nil {
t.Errorf("Unexpected error %v when ensuring firewall %s for svc %+v", err, fwName, svc)
}
existingFirewall, err = gce.GetFirewall(fwName)
require.Nil(t, err)
require.NotNil(t, existingFirewall)
existingLegacyFirewall, err = gce.GetFirewall(lbName)
require.NotNil(t, err)
require.Nil(t, existingLegacyFirewall)
}
func TestEnsureInternalFirewallSucceedsOnXPN(t *testing.T) {
gce, err := fakeGCECloud(DefaultTestClusterValues())
require.NoError(t, err)
vals := DefaultTestClusterValues()
svc := fakeLoadbalancerService(string(LBTypeInternal))
fwName := gce.GetLoadBalancerName(context.TODO(), "", svc)
lbName := gce.GetLoadBalancerName(context.TODO(), "", svc)
fwName := MakeFirewallName(lbName)
c := gce.c.(*cloud.MockGCE)
c.MockFirewalls.InsertHook = mock.InsertFirewallsUnauthorizedErrHook
@ -616,7 +691,8 @@ func TestEnsureInternalFirewallSucceedsOnXPN(t *testing.T) {
sourceRange,
[]string{"123"},
v1.ProtocolTCP,
nodes)
nodes,
lbName)
require.Nil(t, err, "Should success when XPN is on.")
checkEvent(t, recorder, FilewallChangeMsg, true)
@ -633,7 +709,8 @@ func TestEnsureInternalFirewallSucceedsOnXPN(t *testing.T) {
sourceRange,
[]string{"123"},
v1.ProtocolTCP,
nodes)
nodes,
lbName)
require.Nil(t, err)
existingFirewall, err := gce.GetFirewall(fwName)
require.Nil(t, err)
@ -651,7 +728,8 @@ func TestEnsureInternalFirewallSucceedsOnXPN(t *testing.T) {
sourceRange,
[]string{"123"},
v1.ProtocolTCP,
nodes)
nodes,
lbName)
require.Nil(t, err, "Should success when XPN is on.")
checkEvent(t, recorder, FilewallChangeMsg, true)

2
staging/src/k8s.io/legacy-cloud-providers/gce/gce_loadbalancer_utils_test.go
View File

@ -206,7 +206,7 @@ func assertInternalLbResources(t *testing.T, gce *Cloud, apiService *v1.Service,
// Check that Firewalls are created for the LoadBalancer and the HealthCheck
fwNames := []string{
lbName, // Firewalls for internal LBs are named the same name as the loadbalancer.
MakeFirewallName(lbName),
makeHealthCheckFirewallName(lbName, vals.ClusterID, true),
}