diff --git a/staging/src/k8s.io/legacy-cloud-providers/gce/gce_loadbalancer_external.go b/staging/src/k8s.io/legacy-cloud-providers/gce/gce_loadbalancer_external.go index f82b748e904..b95ad4166c0 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/gce/gce_loadbalancer_external.go +++ b/staging/src/k8s.io/legacy-cloud-providers/gce/gce_loadbalancer_external.go @@ -23,6 +23,7 @@ import ( "context" "fmt" "net/http" + "reflect" "strconv" "strings" @@ -902,6 +903,13 @@ func (g *Cloud) firewallNeedsUpdate(name, serviceName, ipAddress string, ports [ if !sourceRanges.Equal(actualSourceRanges) { return true, true, nil } + + destinationRanges := []string{ipAddress} + + if !reflect.DeepEqual(destinationRanges, fw.DestinationRanges) { + return true, true, nil + } + return true, false, nil } diff --git a/staging/src/k8s.io/legacy-cloud-providers/gce/gce_loadbalancer_external_test.go b/staging/src/k8s.io/legacy-cloud-providers/gce/gce_loadbalancer_external_test.go index efa0cd2c0bf..7ab6513e869 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/gce/gce_loadbalancer_external_test.go +++ b/staging/src/k8s.io/legacy-cloud-providers/gce/gce_loadbalancer_external_test.go @@ -1044,6 +1044,18 @@ func TestFirewallNeedsUpdate(t *testing.T) { needsUpdate: true, hasErr: false, }, + "When the destination ranges are not equal.": { + lbName: lbName, + ipAddr: "8.8.8.8", + ports: svc.Spec.Ports, + ipnet: ipnet, + fwIPProtocol: "tcp", + getHook: nil, + sourceRange: fw.SourceRanges[0], + exists: true, + needsUpdate: true, + hasErr: false, + }, "When basic flow without exceptions.": { lbName: lbName, ipAddr: ipAddr, @@ -1139,6 +1151,7 @@ func TestFirewallNeedsUpdate(t *testing.T) { fw.SourceRanges[0] = tc.sourceRange fw, err = gce.GetFirewall(MakeFirewallName(lbName)) require.Equal(t, fw.SourceRanges[0], tc.sourceRange) + require.Equal(t, fw.DestinationRanges[0], status.Ingress[0].IP) c := gce.c.(*cloud.MockGCE) c.MockFirewalls.GetHook = tc.getHook diff --git a/staging/src/k8s.io/legacy-cloud-providers/gce/gce_loadbalancer_internal.go b/staging/src/k8s.io/legacy-cloud-providers/gce/gce_loadbalancer_internal.go index ae8ef31ed1f..179c2567e0e 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/gce/gce_loadbalancer_internal.go +++ b/staging/src/k8s.io/legacy-cloud-providers/gce/gce_loadbalancer_internal.go @@ -776,6 +776,7 @@ func firewallRuleEqual(a, b *compute.Firewall) bool { a.Allowed[0].IPProtocol == b.Allowed[0].IPProtocol && equalStringSets(a.Allowed[0].Ports, b.Allowed[0].Ports) && equalStringSets(a.SourceRanges, b.SourceRanges) && + equalStringSets(a.DestinationRanges, b.DestinationRanges) && equalStringSets(a.TargetTags, b.TargetTags) } diff --git a/staging/src/k8s.io/legacy-cloud-providers/gce/gce_loadbalancer_internal_test.go b/staging/src/k8s.io/legacy-cloud-providers/gce/gce_loadbalancer_internal_test.go index 6e2a739ed00..ca398d98e34 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/gce/gce_loadbalancer_internal_test.go +++ b/staging/src/k8s.io/legacy-cloud-providers/gce/gce_loadbalancer_internal_test.go @@ -1690,7 +1690,7 @@ func TestEnsureInternalFirewallPortRanges(t *testing.T) { destinationIP := "10.1.2.3" sourceRange := []string{"10.0.0.0/20"} // Manually create a firewall rule with the legacy name - lbName - gce.ensureInternalFirewall( + err = gce.ensureInternalFirewall( svc, fwName, "firewall with legacy name", @@ -1713,6 +1713,65 @@ func TestEnsureInternalFirewallPortRanges(t *testing.T) { } } +func TestEnsureInternalFirewallDestinations(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) + + nodes, err := createAndInsertNodes(gce, []string{"test-node-1"}, vals.ZoneName) + require.NoError(t, err) + + destinationIP := "10.1.2.3" + sourceRange := []string{"10.0.0.0/20"} + + err = gce.ensureInternalFirewall( + svc, + fwName, + "firewall with legacy name", + destinationIP, + sourceRange, + []string{"8080"}, + v1.ProtocolTCP, + nodes, + "") + if err != nil { + t.Errorf("Unexpected error %v when ensuring firewall %s for svc %+v", err, fwName, svc) + } + existingFirewall, err := gce.GetFirewall(fwName) + if err != nil || existingFirewall == nil || len(existingFirewall.Allowed) == 0 { + t.Errorf("Unexpected error %v when looking up firewall %s, Got firewall %+v", err, fwName, existingFirewall) + } + + newDestinationIP := "20.1.2.3" + + err = gce.ensureInternalFirewall( + svc, + fwName, + "firewall with legacy name", + newDestinationIP, + sourceRange, + []string{"8080"}, + v1.ProtocolTCP, + nodes, + "") + if err != nil { + t.Errorf("Unexpected error %v when ensuring firewall %s for svc %+v", err, fwName, svc) + } + + updatedFirewall, err := gce.GetFirewall(fwName) + if err != nil || updatedFirewall == nil || len(updatedFirewall.Allowed) == 0 { + t.Errorf("Unexpected error %v when looking up firewall %s, Got firewall %+v", err, fwName, existingFirewall) + } + + if reflect.DeepEqual(existingFirewall.DestinationRanges, updatedFirewall.DestinationRanges) { + t.Errorf("DestinationRanges is not updated. existingFirewall.DestinationRanges: %v, updatedFirewall.DestinationRanges: %v", existingFirewall.DestinationRanges, updatedFirewall.DestinationRanges) + } + +} + func TestEnsureInternalLoadBalancerFinalizer(t *testing.T) { t.Parallel() diff --git a/staging/src/k8s.io/legacy-cloud-providers/gce/gce_util.go b/staging/src/k8s.io/legacy-cloud-providers/gce/gce_util.go index 0a2729dd88c..09feed922a5 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/gce/gce_util.go +++ b/staging/src/k8s.io/legacy-cloud-providers/gce/gce_util.go @@ -78,6 +78,7 @@ func fakeGCECloud(vals TestClusterValues) (*Cloud, error) { mockGCE.MockRegionBackendServices.UpdateHook = mock.UpdateRegionBackendServiceHook mockGCE.MockHealthChecks.UpdateHook = mock.UpdateHealthCheckHook mockGCE.MockFirewalls.UpdateHook = mock.UpdateFirewallHook + mockGCE.MockFirewalls.PatchHook = mock.UpdateFirewallHook keyGA := meta.GlobalKey("key-ga") mockGCE.MockZones.Objects[*keyGA] = &cloud.MockZonesObj{