Skip to content

Commit f38b135

Browse files
committed
keep other clusters' acl rules when deleting a cks cluster on a vpc
1 parent d3f0700 commit f38b135

2 files changed

Lines changed: 78 additions & 1 deletion

File tree

‎plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterResourceModifierActionWorker.java‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -658,6 +658,8 @@ protected void provisionVpcTierAllowPortACLRule(final Network network, int start
658658

659659
networkACLRule.setAction(NetworkACLItem.Action.Allow.toString());
660660

661+
networkACLRule.setReason(getVpcTierAclRuleReason());
662+
661663
NetworkACLItem aclRule = networkACLService.createNetworkACLItem(networkACLRule);
662664
networkACLService.moveRuleToTheTopInACLList(aclRule);
663665
networkACLService.applyNetworkACL(aclRule.getAclId());
@@ -675,11 +677,27 @@ protected void removeVpcTierAllowPortACLRule(final Network network, int startPor
675677
networkACLItem.getAction().equals(NetworkACLItem.Action.Allow)))
676678
.collect(Collectors.toList());
677679

678-
for (NetworkACLItemVO aclItem : aclItems) {
680+
// Only remove the rules this cluster created, other clusters may use the same ACL list
681+
List<NetworkACLItemVO> clusterAclItems = aclItems.stream()
682+
.filter(networkACLItem -> getVpcTierAclRuleReason().equals(networkACLItem.getReason()))
683+
.collect(Collectors.toList());
684+
if (clusterAclItems.isEmpty()) {
685+
// Rules created before they were marked with the cluster, remove only one of them
686+
clusterAclItems = aclItems.stream()
687+
.filter(networkACLItem -> StringUtils.isEmpty(networkACLItem.getReason()))
688+
.limit(1)
689+
.collect(Collectors.toList());
690+
}
691+
692+
for (NetworkACLItemVO aclItem : clusterAclItems) {
679693
networkACLService.revokeNetworkACLItem(aclItem.getId());
680694
}
681695
}
682696

697+
protected String getVpcTierAclRuleReason() {
698+
return String.format("Kubernetes cluster %s", kubernetesCluster.getUuid());
699+
}
700+
683701
protected void provisionLoadBalancerRule(final IpAddress publicIp, final Network network,
684702
final Account account, final List<Long> clusterVMIds, final int port) throws NetworkRuleConflictException,
685703
InsufficientAddressCapacityException {

‎plugins/integrations/kubernetes-service/src/test/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterResourceModifierActionWorkerTest.java‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,11 @@
2323
import com.cloud.kubernetes.cluster.dao.KubernetesClusterDetailsDao;
2424
import com.cloud.kubernetes.cluster.dao.KubernetesClusterVmMapDao;
2525
import com.cloud.kubernetes.version.dao.KubernetesSupportedVersionDao;
26+
import com.cloud.network.Network;
27+
import com.cloud.network.vpc.NetworkACLItem;
28+
import com.cloud.network.vpc.NetworkACLItemDao;
29+
import com.cloud.network.vpc.NetworkACLItemVO;
30+
import com.cloud.network.vpc.NetworkACLService;
2631
import org.junit.Assert;
2732
import org.junit.Before;
2833
import org.junit.Test;
@@ -31,6 +36,8 @@
3136
import org.mockito.Mockito;
3237
import org.mockito.junit.MockitoJUnitRunner;
3338

39+
import java.util.List;
40+
3441
@RunWith(MockitoJUnitRunner.class)
3542
public class KubernetesClusterResourceModifierActionWorkerTest {
3643
@Mock
@@ -51,6 +58,15 @@ public class KubernetesClusterResourceModifierActionWorkerTest {
5158
@Mock
5259
private KubernetesCluster kubernetesClusterMock;
5360

61+
@Mock
62+
private NetworkACLItemDao networkACLItemDaoMock;
63+
64+
@Mock
65+
private NetworkACLService networkACLServiceMock;
66+
67+
@Mock
68+
private Network networkMock;
69+
5470
private KubernetesClusterResourceModifierActionWorker kubernetesClusterResourceModifierActionWorker;
5571

5672
@Before
@@ -61,6 +77,49 @@ public void setUp() {
6177
kubernetesClusterManagerMock.kubernetesClusterVmMapDao = kubernetesClusterVmMapDaoMock;
6278

6379
kubernetesClusterResourceModifierActionWorker = new KubernetesClusterResourceModifierActionWorker(kubernetesClusterMock, kubernetesClusterManagerMock);
80+
kubernetesClusterResourceModifierActionWorker.networkACLItemDao = networkACLItemDaoMock;
81+
kubernetesClusterResourceModifierActionWorker.networkACLService = networkACLServiceMock;
82+
}
83+
84+
private NetworkACLItemVO mockAclItem(long id, int port, String reason) {
85+
NetworkACLItemVO aclItem = Mockito.mock(NetworkACLItemVO.class);
86+
Mockito.lenient().when(aclItem.getId()).thenReturn(id);
87+
Mockito.lenient().when(aclItem.getProtocol()).thenReturn("TCP");
88+
Mockito.lenient().when(aclItem.getSourcePortStart()).thenReturn(port);
89+
Mockito.lenient().when(aclItem.getSourcePortEnd()).thenReturn(port);
90+
Mockito.lenient().when(aclItem.getAction()).thenReturn(NetworkACLItem.Action.Allow);
91+
Mockito.lenient().when(aclItem.getReason()).thenReturn(reason);
92+
return aclItem;
93+
}
94+
95+
@Test
96+
public void removeVpcTierAllowPortACLRuleTestRemovesOnlyThisClusterRules() throws Exception {
97+
Mockito.when(kubernetesClusterMock.getUuid()).thenReturn("cluster-a");
98+
Mockito.when(networkMock.getNetworkACLId()).thenReturn(10L);
99+
NetworkACLItemVO clusterARule = mockAclItem(1L, 6443, "Kubernetes cluster cluster-a");
100+
NetworkACLItemVO clusterBRule = mockAclItem(2L, 6443, "Kubernetes cluster cluster-b");
101+
NetworkACLItemVO oldRule = mockAclItem(3L, 6443, null);
102+
Mockito.when(networkACLItemDaoMock.listByACL(10L)).thenReturn(List.of(clusterARule, clusterBRule, oldRule));
103+
104+
kubernetesClusterResourceModifierActionWorker.removeVpcTierAllowPortACLRule(networkMock, 6443, 6443);
105+
106+
Mockito.verify(networkACLServiceMock).revokeNetworkACLItem(1L);
107+
Mockito.verifyNoMoreInteractions(networkACLServiceMock);
108+
}
109+
110+
@Test
111+
public void removeVpcTierAllowPortACLRuleTestRemovesOneUnmarkedRuleForOlderClusters() throws Exception {
112+
Mockito.when(kubernetesClusterMock.getUuid()).thenReturn("cluster-a");
113+
Mockito.when(networkMock.getNetworkACLId()).thenReturn(10L);
114+
NetworkACLItemVO clusterBRule = mockAclItem(2L, 22, "Kubernetes cluster cluster-b");
115+
NetworkACLItemVO oldRule1 = mockAclItem(3L, 22, null);
116+
NetworkACLItemVO oldRule2 = mockAclItem(4L, 22, null);
117+
Mockito.when(networkACLItemDaoMock.listByACL(10L)).thenReturn(List.of(clusterBRule, oldRule1, oldRule2));
118+
119+
kubernetesClusterResourceModifierActionWorker.removeVpcTierAllowPortACLRule(networkMock, 22, 22);
120+
121+
Mockito.verify(networkACLServiceMock).revokeNetworkACLItem(3L);
122+
Mockito.verifyNoMoreInteractions(networkACLServiceMock);
64123
}
65124

66125
@Test

0 commit comments

Comments
 (0)