Merge pull request gardener-attic#169 from plkokanov/fix-gcp-firewall…

…-deletion

Fixes deletion of firewall rules on gcp

controllers/provider-gcp/pkg/controller/infrastructure/actuator_delete.go

@@ -34,6 +34,7 @@ func (a *actuator) cleanupKubernetesFirewallRules(
 	client gcpclient.Interface,
 	tf *terraformer.Terraformer,
 	account *internal.ServiceAccount,
+	shootSeedNamespace string,
 ) error {
 	state, err := infrastructure.ExtractTerraformState(tf, config)
 	if err != nil {
@@ -43,7 +44,7 @@ func (a *actuator) cleanupKubernetesFirewallRules(
 		return err
 	}
 
-	return infrastructure.CleanupKubernetesFirewalls(ctx, client, account.ProjectID, state.VPCName)
+	return infrastructure.CleanupKubernetesFirewalls(ctx, client, account.ProjectID, state.VPCName, shootSeedNamespace)
 }
 
 func (a *actuator) cleanupKubernetesRoutes(
@@ -96,7 +97,7 @@ func (a *actuator) Delete(ctx context.Context, infra *extensionsv1alpha1.Infrast
 		destroyKubernetesFirewallRules = g.Add(flow.Task{
 			Name: "Destroying Kubernetes firewall rules",
 			Fn: flow.TaskFn(func(ctx context.Context) error {
-				return a.cleanupKubernetesFirewallRules(ctx, config, gcpClient, tf, serviceAccount)
+				return a.cleanupKubernetesFirewallRules(ctx, config, gcpClient, tf, serviceAccount, infra.Namespace)
 			}).
 				RetryUntilTimeout(10*time.Second, 5*time.Minute).
 				DoIf(configExists),

controllers/provider-gcp/pkg/internal/infrastructure/infrastructure.go

@@ -31,13 +31,22 @@ const (
 	shootPrefix                  string = "shoot--"
 )
 
-// ListKubernetesFirewalls lists all firewalls that are in the given network and have the KubernetesFirewallNamePrefix.
-func ListKubernetesFirewalls(ctx context.Context, client gcpclient.Interface, projectID, network string) ([]string, error) {
+// ListKubernetesFirewalls lists all firewalls that are in the given network and for the given shoot and have the KubernetesFirewallNamePrefix.
+func ListKubernetesFirewalls(ctx context.Context, client gcpclient.Interface, projectID, network, shootSeedNamespace string) ([]string, error) {
 	var names []string
 	err := client.Firewalls().List(projectID).Pages(ctx, func(list *compute.FirewallList) error {
 		for _, firewall := range list.Items {
-			if strings.HasSuffix(firewall.Network, network) && (strings.HasPrefix(firewall.Name, KubernetesFirewallNamePrefix) || strings.HasPrefix(firewall.Name, shootPrefix)) {
-				names = append(names, firewall.Name)
+			if strings.HasSuffix(firewall.Network, network) {
+				if strings.HasPrefix(firewall.Name, KubernetesFirewallNamePrefix) {
+					for _, targetTag := range firewall.TargetTags {
+						if targetTag == shootSeedNamespace {
+							names = append(names, firewall.Name)
+							break
+						}
+					}
+				} else if strings.HasPrefix(firewall.Name, shootSeedNamespace) {
+					names = append(names, firewall.Name)
+				}
 			}
 		}
 		return nil
@@ -91,8 +100,8 @@ func DeleteRoutes(ctx context.Context, client gcpclient.Interface, projectID str
 // CleanupKubernetesFirewalls lists all Kubernetes firewall rules and then deletes them one after another.
 //
 // If a deletion fails, this method returns immediately with the encountered error.
-func CleanupKubernetesFirewalls(ctx context.Context, client gcpclient.Interface, projectID, network string) error {
-	firewallNames, err := ListKubernetesFirewalls(ctx, client, projectID, network)
+func CleanupKubernetesFirewalls(ctx context.Context, client gcpclient.Interface, projectID, network, shootSeedNamespace string) error {
+	firewallNames, err := ListKubernetesFirewalls(ctx, client, projectID, network, shootSeedNamespace)
 	if err != nil {
 		return err
 	}

controllers/provider-gcp/pkg/internal/infrastructure/infrastructure_test.go

@@ -39,12 +39,19 @@ var _ = Describe("Infrastructure", func() {
 	Describe("#ListKubernetesFirewalls", func() {
 		It("should list all kubernetes related firewall names", func() {
 			var (
-				ctx       = context.TODO()
-				projectID = "foo"
-				network   = "bar"
+				ctx                     = context.TODO()
+				projectID               = "foo"
+				network                 = "bar"
+				shootSeedNamespace      = "shoot--foo--bar"
+				otherShootSeedNamespace = "shoot--foo--other"
 
-				firewallName  = fmt.Sprintf("%sfw", KubernetesFirewallNamePrefix)
-				firewallNames = []string{firewallName}
+				k8sFirewallName   = fmt.Sprintf("%sbar-fw", KubernetesFirewallNamePrefix)
+				shootFirewallName = fmt.Sprintf("%sbar-fw", shootSeedNamespace)
+
+				оtherK8SFirewallName   = fmt.Sprintf("%sother-fw", KubernetesFirewallNamePrefix)
+				otherShootFirewallName = fmt.Sprintf("%sother-fw", otherShootSeedNamespace)
+
+				firewallNames = []string{k8sFirewallName, shootFirewallName}
 
 				client            = mockgcpclient.NewMockInterface(ctrl)
 				firewalls         = mockgcpclient.NewMockFirewallsService(ctrl)
@@ -58,13 +65,16 @@ var _ = Describe("Infrastructure", func() {
 					DoAndReturn(func(_ context.Context, f func(*compute.FirewallList) error) error {
 						return f(&compute.FirewallList{
 							Items: []*compute.Firewall{
-								{Name: firewallName, Network: network},
+								{Name: k8sFirewallName, Network: network, TargetTags: []string{shootSeedNamespace}},
+								{Name: shootFirewallName, Network: network},
+								{Name: оtherK8SFirewallName, Network: network, TargetTags: []string{otherShootSeedNamespace}},
+								{Name: otherShootFirewallName, Network: network},
 							},
 						})
 					}),
 			)
 
-			actual, err := ListKubernetesFirewalls(ctx, client, projectID, network)
+			actual, err := ListKubernetesFirewalls(ctx, client, projectID, network, shootSeedNamespace)
 
 			Expect(err).NotTo(HaveOccurred())
 			Expect(actual).To(Equal(firewallNames))