[fix] : delete fw if provisioned by CCM with ACL (#307)

* delete fw if provisioned by CCM with ACL

* address review comments

* add e2e test to check firewall is deleted on ACL and service deletion

cloud/linode/firewall/firewalls.go

@@ -44,9 +44,44 @@ func (l *LinodeClient) CreateFirewall(ctx context.Context, opts linodego.Firewal
 }
 
 func (l *LinodeClient) DeleteFirewall(ctx context.Context, firewall *linodego.Firewall) error {
+	fwDevices, err := l.Client.ListFirewallDevices(ctx, firewall.ID, &linodego.ListOptions{})
+	if err != nil {
+		klog.Errorf("Error in listing firewall devices: %v", err)
+		return err
+	}
+	if len(fwDevices) > 1 {
+		klog.Errorf("Found more than one device attached to firewall ID: %d, devices: %+v. Skipping delete of firewall", firewall.ID, fwDevices)
+		return nil
+	}
 	return l.Client.DeleteFirewall(ctx, firewall.ID)
 }
 
+func (l *LinodeClient) DeleteNodeBalancerFirewall(
+	ctx context.Context,
+	service *v1.Service,
+	nb *linodego.NodeBalancer,
+) error {
+	_, fwACLExists := service.GetAnnotations()[annotations.AnnLinodeCloudFirewallACL]
+	if fwACLExists { // if an ACL exists, check if firewall exists and delete it.
+		firewalls, err := l.Client.ListNodeBalancerFirewalls(ctx, nb.ID, &linodego.ListOptions{})
+		if err != nil {
+			return err
+		}
+
+		switch len(firewalls) {
+		case 0:
+			klog.Info("No firewall attached to nodebalancer, nothing to clean")
+		case 1:
+			return l.DeleteFirewall(ctx, &firewalls[0])
+		default:
+			klog.Errorf("Found more than one firewall attached to nodebalancer: %d, firewall IDs: %v", nb.ID, firewalls)
+			return ErrTooManyNBFirewalls
+		}
+	}
+
+	return nil
+}
+
 func ipsChanged(ips *linodego.NetworkAddresses, rules []linodego.FirewallRule) bool {
 	var ruleIPv4s []string
 	var ruleIPv6s []string

cloud/linode/loadbalancers.go

@@ -558,6 +558,11 @@ func (l *loadbalancers) EnsureLoadBalancerDeleted(ctx context.Context, clusterNa
 		return nil
 	}
 
+	fwClient := firewall.LinodeClient{Client: l.client}
+	if err = fwClient.DeleteNodeBalancerFirewall(ctx, service, nb); err != nil {
+		return err
+	}
+
 	if err = l.client.DeleteNodeBalancer(ctx, nb.ID); err != nil {
 		klog.Errorf("failed to delete NodeBalancer (%d) for service (%s): %s", nb.ID, serviceNn, err)
 		sentry.CaptureError(ctx, err)

e2e/test/lb-fw-delete-acl/chainsaw-test.yaml

@@ -0,0 +1,156 @@
+# yaml-language-server: $schema=https://raw.githubusercontent.com/kyverno/chainsaw/main/.schemas/json/test-chainsaw-v1alpha1.json
+apiVersion: chainsaw.kyverno.io/v1alpha1
+kind: Test
+metadata:
+  name: lb-fw-delete-acl
+spec:
+  namespace: "lb-fw-delete-acl"
+  steps:
+    - name: Check if CCM is deployed
+      try:
+        - assert:
+            file: ../assert-ccm-resources.yaml
+    - name: Create pods and services
+      try:
+        - apply:
+            file: create-pods-services.yaml
+      catch:
+        - describe:
+            apiVersion: v1
+            kind: Pod
+        - describe:
+            apiVersion: v1
+            kind: Service
+    - name: Check that loadbalancer ip is assigned
+      try:
+      - assert:
+          resource:
+            apiVersion: v1
+            kind: Service
+            metadata:
+              name: svc-test
+            status:
+              (loadBalancer.ingress[0].ip != null): true
+    - name: Fetch Nodebalancer ID, make sure it has firewall attached
+      try:
+        - script:
+            content: |
+              set -e
+
+              for i in {1..10}; do
+                  nbid=$(KUBECONFIG=$KUBECONFIG NAMESPACE=$NAMESPACE LINODE_TOKEN=$LINODE_TOKEN ../scripts/get-nb-id.sh)
+
+                  fw=$(curl -s --request GET \
+                    -H "Authorization: Bearer $LINODE_TOKEN" \
+                    -H "Content-Type: application/json" \
+                    -H "accept: application/json" \
+                    "https://api.linode.com/v4/nodebalancers/${nbid}/firewalls" || true)
+                  
+                  fwCount=$(echo $fw | jq '.data | length')
+                  ips=$(echo $fw | jq '.data[].rules.inbound[].addresses.ipv4[]')
+                  if [[ $fwCount -eq 1 && -n $ips && $ips == *"7.7.7.7/32"* ]]; then
+                      echo "firewall attached and rule has specified ip"
+                      break
+                  fi
+                  sleep 10
+              done
+            check:
+                ($error == null): true
+                (contains($stdout, 'firewall attached and rule has specified ip')): true
+    - name: Delete ACL and check that firewall no longer exists
+      try:
+        - script:
+            content: |
+              set -e
+
+              for i in {1..10}; do
+                  nbid=$(KUBECONFIG=$KUBECONFIG NAMESPACE=$NAMESPACE LINODE_TOKEN=$LINODE_TOKEN ../scripts/get-nb-id.sh)
+
+                  fw=$(curl -s --request GET \
+                    -H "Authorization: Bearer $LINODE_TOKEN" \
+                    -H "Content-Type: application/json" \
+                    -H "accept: application/json" \
+                    "https://api.linode.com/v4/nodebalancers/${nbid}/firewalls" || true)
+                  
+                  fwid=$(echo $fw | jq -r '.data[].id')
+
+                  # Patch service to remove ACL annotation
+                  kubectl patch service svc-test -n $NAMESPACE --type=json -p='[{"op": "remove", "path": "/metadata/annotations/service.beta.kubernetes.io~1linode-loadbalancer-firewall-acl"}]'
+                  sleep 5
+
+                  # Check that firewall is no longer attached to nb
+                  fw=$(curl -s --request GET \
+                    -H "Authorization: Bearer $LINODE_TOKEN" \
+                    -H "Content-Type: application/json" \
+                    -H "accept: application/json" \
+                    "https://api.linode.com/v4/nodebalancers/${nbid}/firewalls" || true)
+                  
+                  fwCount=$(echo $fw | jq -r '.data | length')
+
+                  # Check if firewall is deleted
+                  fwRespCode=$(curl -s -o /dev/null -w "%{http_code}" \
+                    --request GET \
+                    -H "Authorization: Bearer $LINODE_TOKEN" \
+                    -H "accept: application/json" \
+                    "https://api.linode.com/v4/networking/firewalls/${fwid}" || true)
+
+                  if [[ $fwCount -eq 0 && $fwRespCode -eq "404" ]]; then
+                      echo "firewall detatched and deleted"
+                      break
+                  fi
+                  sleep 10
+              done
+            check:
+                ($error == null): true
+                (contains($stdout, 'firewall detatched and deleted')): true
+    - name: Refresh service by adding the ACL again
+      try:
+        - apply:
+            file: create-pods-services.yaml
+      catch:
+        - describe:
+            apiVersion: v1
+            kind: Service
+    - name: Delete service and make sure nb and fw are deleted automatically
+      try:
+        - script:
+            content: |
+              set -e
+
+              for i in {1..10}; do
+                  nbid=$(KUBECONFIG=$KUBECONFIG NAMESPACE=$NAMESPACE LINODE_TOKEN=$LINODE_TOKEN ../scripts/get-nb-id.sh)
+
+                  fw=$(curl -s --request GET \
+                    -H "Authorization: Bearer $LINODE_TOKEN" \
+                    -H "Content-Type: application/json" \
+                    -H "accept: application/json" \
+                    "https://api.linode.com/v4/nodebalancers/${nbid}/firewalls" || true)
+                  
+                  fwid=$(echo $fw | jq -r '.data[].id')
+
+                  # Remove service
+                  kubectl delete service svc-test -n $NAMESPACE --ignore-not-found
+                  sleep 5
+
+                  # Check if nodebalancer is deleted
+                  nbRespCode=$(curl -s -o /dev/null -w "%{http_code}" \
+                    --request GET \
+                    -H "Authorization: Bearer $LINODE_TOKEN" \
+                    -H "accept: application/json" \
+                    "https://api.linode.com/v4/nodebalancers/${nbid}" || true)
+
+                  # Check if firewall is deleted
+                  fwRespCode=$(curl -s -o /dev/null -w "%{http_code}" \
+                    --request GET \
+                    -H "Authorization: Bearer $LINODE_TOKEN" \
+                    -H "accept: application/json" \
+                    "https://api.linode.com/v4/networking/firewalls/${fwid}" || true)
+                  if [[ $nbRespCode == "404" && $fwRespCode == "404" ]]; then
+                      echo "nb and fw deleted"
+                      break
+                  fi
+                  sleep 10
+              done
+            check:
+                ($error == null): true
+                (contains($stdout, 'nb and fw deleted')): true

e2e/test/lb-fw-delete-acl/create-pods-services.yaml

@@ -0,0 +1,68 @@
+---
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  labels:
+    app: lb-fw-delete-acl
+  name: test
+spec:
+  replicas: 2
+  selector:
+    matchLabels:
+      app: lb-fw-delete-acl
+  template:
+    metadata:
+      labels:
+        app: lb-fw-delete-acl
+    spec:
+      affinity:
+        podAntiAffinity:
+          preferredDuringSchedulingIgnoredDuringExecution:
+          - podAffinityTerm:
+              labelSelector:
+                matchExpressions:
+                - key: app
+                  operator: In
+                  values:
+                  - simple-lb
+              topologyKey: kubernetes.io/hostname
+            weight: 100
+      containers:
+      - image: appscode/test-server:2.3
+        name: test
+        ports:
+        - name: http-1
+          containerPort: 8080
+          protocol: TCP
+        env:
+        - name: POD_NAME
+          valueFrom:
+            fieldRef:
+              apiVersion: v1
+              fieldPath: metadata.name
+---
+apiVersion: v1
+kind: Service
+metadata:
+  name: svc-test
+  annotations:
+    service.beta.kubernetes.io/linode-loadbalancer-firewall-acl: |
+      {
+        "denyList": {
+            "ipv4": ["8.8.8.8/32",
+                    "9.9.9.9/32",
+                    "7.7.7.7/32"]
+        }
+      }
+  labels:
+    app: lb-fw-delete-acl
+spec:
+  type: LoadBalancer
+  selector:
+    app: lb-fw-delete-acl
+  ports:
+    - name: http-1
+      protocol: TCP
+      port: 80
+      targetPort: 8080
+  sessionAffinity: None