Skip to content

Commit 8d34ded

Browse files
rahulaitrahulait
authored
[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
1 parent c044e45 commit 8d34ded

File tree

4 files changed

+264
-0
lines changed

4 files changed

+264
-0
lines changed

‎cloud/linode/firewall/firewalls.go‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,44 @@ func (l *LinodeClient) CreateFirewall(ctx context.Context, opts linodego.Firewal
4444
}
4545

4646
func (l *LinodeClient) DeleteFirewall(ctx context.Context, firewall *linodego.Firewall) error {
47+
fwDevices, err := l.Client.ListFirewallDevices(ctx, firewall.ID, &linodego.ListOptions{})
48+
if err != nil {
49+
klog.Errorf("Error in listing firewall devices: %v", err)
50+
return err
51+
}
52+
if len(fwDevices) > 1 {
53+
klog.Errorf("Found more than one device attached to firewall ID: %d, devices: %+v. Skipping delete of firewall", firewall.ID, fwDevices)
54+
return nil
55+
}
4756
return l.Client.DeleteFirewall(ctx, firewall.ID)
4857
}
4958

59+
func (l *LinodeClient) DeleteNodeBalancerFirewall(
60+
ctx context.Context,
61+
service *v1.Service,
62+
nb *linodego.NodeBalancer,
63+
) error {
64+
_, fwACLExists := service.GetAnnotations()[annotations.AnnLinodeCloudFirewallACL]
65+
if fwACLExists { // if an ACL exists, check if firewall exists and delete it.
66+
firewalls, err := l.Client.ListNodeBalancerFirewalls(ctx, nb.ID, &linodego.ListOptions{})
67+
if err != nil {
68+
return err
69+
}
70+
71+
switch len(firewalls) {
72+
case 0:
73+
klog.Info("No firewall attached to nodebalancer, nothing to clean")
74+
case 1:
75+
return l.DeleteFirewall(ctx, &firewalls[0])
76+
default:
77+
klog.Errorf("Found more than one firewall attached to nodebalancer: %d, firewall IDs: %v", nb.ID, firewalls)
78+
return ErrTooManyNBFirewalls
79+
}
80+
}
81+
82+
return nil
83+
}
84+
5085
func ipsChanged(ips *linodego.NetworkAddresses, rules []linodego.FirewallRule) bool {
5186
var ruleIPv4s []string
5287
var ruleIPv6s []string

‎cloud/linode/loadbalancers.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -558,6 +558,11 @@ func (l *loadbalancers) EnsureLoadBalancerDeleted(ctx context.Context, clusterNa
558558
return nil
559559
}
560560

561+
fwClient := firewall.LinodeClient{Client: l.client}
562+
if err = fwClient.DeleteNodeBalancerFirewall(ctx, service, nb); err != nil {
563+
return err
564+
}
565+
561566
if err = l.client.DeleteNodeBalancer(ctx, nb.ID); err != nil {
562567
klog.Errorf("failed to delete NodeBalancer (%d) for service (%s): %s", nb.ID, serviceNn, err)
563568
sentry.CaptureError(ctx, err)

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

Lines changed: 156 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,156 @@
1+
# yaml-language-server: $schema=https://raw.githubusercontent.com/kyverno/chainsaw/main/.schemas/json/test-chainsaw-v1alpha1.json
2+
apiVersion: chainsaw.kyverno.io/v1alpha1
3+
kind: Test
4+
metadata:
5+
name: lb-fw-delete-acl
6+
spec:
7+
namespace: "lb-fw-delete-acl"
8+
steps:
9+
- name: Check if CCM is deployed
10+
try:
11+
- assert:
12+
file: ../assert-ccm-resources.yaml
13+
- name: Create pods and services
14+
try:
15+
- apply:
16+
file: create-pods-services.yaml
17+
catch:
18+
- describe:
19+
apiVersion: v1
20+
kind: Pod
21+
- describe:
22+
apiVersion: v1
23+
kind: Service
24+
- name: Check that loadbalancer ip is assigned
25+
try:
26+
- assert:
27+
resource:
28+
apiVersion: v1
29+
kind: Service
30+
metadata:
31+
name: svc-test
32+
status:
33+
(loadBalancer.ingress[0].ip != null): true
34+
- name: Fetch Nodebalancer ID, make sure it has firewall attached
35+
try:
36+
- script:
37+
content: |
38+
set -e
39+
40+
for i in {1..10}; do
41+
nbid=$(KUBECONFIG=$KUBECONFIG NAMESPACE=$NAMESPACE LINODE_TOKEN=$LINODE_TOKEN ../scripts/get-nb-id.sh)
42+
43+
fw=$(curl -s --request GET \
44+
-H "Authorization: Bearer $LINODE_TOKEN" \
45+
-H "Content-Type: application/json" \
46+
-H "accept: application/json" \
47+
"https://api.linode.com/v4/nodebalancers/${nbid}/firewalls" || true)
48+
49+
fwCount=$(echo $fw | jq '.data | length')
50+
ips=$(echo $fw | jq '.data[].rules.inbound[].addresses.ipv4[]')
51+
if [[ $fwCount -eq 1 && -n $ips && $ips == *"7.7.7.7/32"* ]]; then
52+
echo "firewall attached and rule has specified ip"
53+
break
54+
fi
55+
sleep 10
56+
done
57+
check:
58+
($error == null): true
59+
(contains($stdout, 'firewall attached and rule has specified ip')): true
60+
- name: Delete ACL and check that firewall no longer exists
61+
try:
62+
- script:
63+
content: |
64+
set -e
65+
66+
for i in {1..10}; do
67+
nbid=$(KUBECONFIG=$KUBECONFIG NAMESPACE=$NAMESPACE LINODE_TOKEN=$LINODE_TOKEN ../scripts/get-nb-id.sh)
68+
69+
fw=$(curl -s --request GET \
70+
-H "Authorization: Bearer $LINODE_TOKEN" \
71+
-H "Content-Type: application/json" \
72+
-H "accept: application/json" \
73+
"https://api.linode.com/v4/nodebalancers/${nbid}/firewalls" || true)
74+
75+
fwid=$(echo $fw | jq -r '.data[].id')
76+
77+
# Patch service to remove ACL annotation
78+
kubectl patch service svc-test -n $NAMESPACE --type=json -p='[{"op": "remove", "path": "/metadata/annotations/service.beta.kubernetes.io~1linode-loadbalancer-firewall-acl"}]'
79+
sleep 5
80+
81+
# Check that firewall is no longer attached to nb
82+
fw=$(curl -s --request GET \
83+
-H "Authorization: Bearer $LINODE_TOKEN" \
84+
-H "Content-Type: application/json" \
85+
-H "accept: application/json" \
86+
"https://api.linode.com/v4/nodebalancers/${nbid}/firewalls" || true)
87+
88+
fwCount=$(echo $fw | jq -r '.data | length')
89+
90+
# Check if firewall is deleted
91+
fwRespCode=$(curl -s -o /dev/null -w "%{http_code}" \
92+
--request GET \
93+
-H "Authorization: Bearer $LINODE_TOKEN" \
94+
-H "accept: application/json" \
95+
"https://api.linode.com/v4/networking/firewalls/${fwid}" || true)
96+
97+
if [[ $fwCount -eq 0 && $fwRespCode -eq "404" ]]; then
98+
echo "firewall detatched and deleted"
99+
break
100+
fi
101+
sleep 10
102+
done
103+
check:
104+
($error == null): true
105+
(contains($stdout, 'firewall detatched and deleted')): true
106+
- name: Refresh service by adding the ACL again
107+
try:
108+
- apply:
109+
file: create-pods-services.yaml
110+
catch:
111+
- describe:
112+
apiVersion: v1
113+
kind: Service
114+
- name: Delete service and make sure nb and fw are deleted automatically
115+
try:
116+
- script:
117+
content: |
118+
set -e
119+
120+
for i in {1..10}; do
121+
nbid=$(KUBECONFIG=$KUBECONFIG NAMESPACE=$NAMESPACE LINODE_TOKEN=$LINODE_TOKEN ../scripts/get-nb-id.sh)
122+
123+
fw=$(curl -s --request GET \
124+
-H "Authorization: Bearer $LINODE_TOKEN" \
125+
-H "Content-Type: application/json" \
126+
-H "accept: application/json" \
127+
"https://api.linode.com/v4/nodebalancers/${nbid}/firewalls" || true)
128+
129+
fwid=$(echo $fw | jq -r '.data[].id')
130+
131+
# Remove service
132+
kubectl delete service svc-test -n $NAMESPACE --ignore-not-found
133+
sleep 5
134+
135+
# Check if nodebalancer is deleted
136+
nbRespCode=$(curl -s -o /dev/null -w "%{http_code}" \
137+
--request GET \
138+
-H "Authorization: Bearer $LINODE_TOKEN" \
139+
-H "accept: application/json" \
140+
"https://api.linode.com/v4/nodebalancers/${nbid}" || true)
141+
142+
# Check if firewall is deleted
143+
fwRespCode=$(curl -s -o /dev/null -w "%{http_code}" \
144+
--request GET \
145+
-H "Authorization: Bearer $LINODE_TOKEN" \
146+
-H "accept: application/json" \
147+
"https://api.linode.com/v4/networking/firewalls/${fwid}" || true)
148+
if [[ $nbRespCode == "404" && $fwRespCode == "404" ]]; then
149+
echo "nb and fw deleted"
150+
break
151+
fi
152+
sleep 10
153+
done
154+
check:
155+
($error == null): true
156+
(contains($stdout, 'nb and fw deleted')): true

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

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
---
2+
apiVersion: apps/v1
3+
kind: Deployment
4+
metadata:
5+
labels:
6+
app: lb-fw-delete-acl
7+
name: test
8+
spec:
9+
replicas: 2
10+
selector:
11+
matchLabels:
12+
app: lb-fw-delete-acl
13+
template:
14+
metadata:
15+
labels:
16+
app: lb-fw-delete-acl
17+
spec:
18+
affinity:
19+
podAntiAffinity:
20+
preferredDuringSchedulingIgnoredDuringExecution:
21+
- podAffinityTerm:
22+
labelSelector:
23+
matchExpressions:
24+
- key: app
25+
operator: In
26+
values:
27+
- simple-lb
28+
topologyKey: kubernetes.io/hostname
29+
weight: 100
30+
containers:
31+
- image: appscode/test-server:2.3
32+
name: test
33+
ports:
34+
- name: http-1
35+
containerPort: 8080
36+
protocol: TCP
37+
env:
38+
- name: POD_NAME
39+
valueFrom:
40+
fieldRef:
41+
apiVersion: v1
42+
fieldPath: metadata.name
43+
---
44+
apiVersion: v1
45+
kind: Service
46+
metadata:
47+
name: svc-test
48+
annotations:
49+
service.beta.kubernetes.io/linode-loadbalancer-firewall-acl: |
50+
{
51+
"denyList": {
52+
"ipv4": ["8.8.8.8/32",
53+
"9.9.9.9/32",
54+
"7.7.7.7/32"]
55+
}
56+
}
57+
labels:
58+
app: lb-fw-delete-acl
59+
spec:
60+
type: LoadBalancer
61+
selector:
62+
app: lb-fw-delete-acl
63+
ports:
64+
- name: http-1
65+
protocol: TCP
66+
port: 80
67+
targetPort: 8080
68+
sessionAffinity: None

0 commit comments

Comments
 (0)