diff --git a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterResourceModifierActionWorker.java b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterResourceModifierActionWorker.java index a1b2294bcd4d..ac290fa6f97d 100644 --- a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterResourceModifierActionWorker.java +++ b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterResourceModifierActionWorker.java @@ -658,6 +658,8 @@ protected void provisionVpcTierAllowPortACLRule(final Network network, int start networkACLRule.setAction(NetworkACLItem.Action.Allow.toString()); + networkACLRule.setReason(getVpcTierAclRuleReason()); + NetworkACLItem aclRule = networkACLService.createNetworkACLItem(networkACLRule); networkACLService.moveRuleToTheTopInACLList(aclRule); networkACLService.applyNetworkACL(aclRule.getAclId()); @@ -675,11 +677,27 @@ protected void removeVpcTierAllowPortACLRule(final Network network, int startPor networkACLItem.getAction().equals(NetworkACLItem.Action.Allow))) .collect(Collectors.toList()); - for (NetworkACLItemVO aclItem : aclItems) { + // Only remove the rules this cluster created, other clusters may use the same ACL list + List clusterAclItems = aclItems.stream() + .filter(networkACLItem -> getVpcTierAclRuleReason().equals(networkACLItem.getReason())) + .collect(Collectors.toList()); + if (clusterAclItems.isEmpty()) { + // Rules created before they were marked with the cluster, remove only one of them + clusterAclItems = aclItems.stream() + .filter(networkACLItem -> StringUtils.isEmpty(networkACLItem.getReason())) + .limit(1) + .collect(Collectors.toList()); + } + + for (NetworkACLItemVO aclItem : clusterAclItems) { networkACLService.revokeNetworkACLItem(aclItem.getId()); } } + protected String getVpcTierAclRuleReason() { + return String.format("Kubernetes cluster %s", kubernetesCluster.getUuid()); + } + protected void provisionLoadBalancerRule(final IpAddress publicIp, final Network network, final Account account, final List clusterVMIds, final int port) throws NetworkRuleConflictException, InsufficientAddressCapacityException { diff --git a/plugins/integrations/kubernetes-service/src/test/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterResourceModifierActionWorkerTest.java b/plugins/integrations/kubernetes-service/src/test/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterResourceModifierActionWorkerTest.java index c220a3468afb..119ddbebf66f 100644 --- a/plugins/integrations/kubernetes-service/src/test/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterResourceModifierActionWorkerTest.java +++ b/plugins/integrations/kubernetes-service/src/test/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterResourceModifierActionWorkerTest.java @@ -23,6 +23,11 @@ import com.cloud.kubernetes.cluster.dao.KubernetesClusterDetailsDao; import com.cloud.kubernetes.cluster.dao.KubernetesClusterVmMapDao; import com.cloud.kubernetes.version.dao.KubernetesSupportedVersionDao; +import com.cloud.network.Network; +import com.cloud.network.vpc.NetworkACLItem; +import com.cloud.network.vpc.NetworkACLItemDao; +import com.cloud.network.vpc.NetworkACLItemVO; +import com.cloud.network.vpc.NetworkACLService; import org.junit.Assert; import org.junit.Before; import org.junit.Test; @@ -31,6 +36,8 @@ import org.mockito.Mockito; import org.mockito.junit.MockitoJUnitRunner; +import java.util.List; + @RunWith(MockitoJUnitRunner.class) public class KubernetesClusterResourceModifierActionWorkerTest { @Mock @@ -51,6 +58,15 @@ public class KubernetesClusterResourceModifierActionWorkerTest { @Mock private KubernetesCluster kubernetesClusterMock; + @Mock + private NetworkACLItemDao networkACLItemDaoMock; + + @Mock + private NetworkACLService networkACLServiceMock; + + @Mock + private Network networkMock; + private KubernetesClusterResourceModifierActionWorker kubernetesClusterResourceModifierActionWorker; @Before @@ -61,6 +77,49 @@ public void setUp() { kubernetesClusterManagerMock.kubernetesClusterVmMapDao = kubernetesClusterVmMapDaoMock; kubernetesClusterResourceModifierActionWorker = new KubernetesClusterResourceModifierActionWorker(kubernetesClusterMock, kubernetesClusterManagerMock); + kubernetesClusterResourceModifierActionWorker.networkACLItemDao = networkACLItemDaoMock; + kubernetesClusterResourceModifierActionWorker.networkACLService = networkACLServiceMock; + } + + private NetworkACLItemVO mockAclItem(long id, int port, String reason) { + NetworkACLItemVO aclItem = Mockito.mock(NetworkACLItemVO.class); + Mockito.lenient().when(aclItem.getId()).thenReturn(id); + Mockito.lenient().when(aclItem.getProtocol()).thenReturn("TCP"); + Mockito.lenient().when(aclItem.getSourcePortStart()).thenReturn(port); + Mockito.lenient().when(aclItem.getSourcePortEnd()).thenReturn(port); + Mockito.lenient().when(aclItem.getAction()).thenReturn(NetworkACLItem.Action.Allow); + Mockito.lenient().when(aclItem.getReason()).thenReturn(reason); + return aclItem; + } + + @Test + public void removeVpcTierAllowPortACLRuleTestRemovesOnlyThisClusterRules() throws Exception { + Mockito.when(kubernetesClusterMock.getUuid()).thenReturn("cluster-a"); + Mockito.when(networkMock.getNetworkACLId()).thenReturn(10L); + NetworkACLItemVO clusterARule = mockAclItem(1L, 6443, "Kubernetes cluster cluster-a"); + NetworkACLItemVO clusterBRule = mockAclItem(2L, 6443, "Kubernetes cluster cluster-b"); + NetworkACLItemVO oldRule = mockAclItem(3L, 6443, null); + Mockito.when(networkACLItemDaoMock.listByACL(10L)).thenReturn(List.of(clusterARule, clusterBRule, oldRule)); + + kubernetesClusterResourceModifierActionWorker.removeVpcTierAllowPortACLRule(networkMock, 6443, 6443); + + Mockito.verify(networkACLServiceMock).revokeNetworkACLItem(1L); + Mockito.verifyNoMoreInteractions(networkACLServiceMock); + } + + @Test + public void removeVpcTierAllowPortACLRuleTestRemovesOneUnmarkedRuleForOlderClusters() throws Exception { + Mockito.when(kubernetesClusterMock.getUuid()).thenReturn("cluster-a"); + Mockito.when(networkMock.getNetworkACLId()).thenReturn(10L); + NetworkACLItemVO clusterBRule = mockAclItem(2L, 22, "Kubernetes cluster cluster-b"); + NetworkACLItemVO oldRule1 = mockAclItem(3L, 22, null); + NetworkACLItemVO oldRule2 = mockAclItem(4L, 22, null); + Mockito.when(networkACLItemDaoMock.listByACL(10L)).thenReturn(List.of(clusterBRule, oldRule1, oldRule2)); + + kubernetesClusterResourceModifierActionWorker.removeVpcTierAllowPortACLRule(networkMock, 22, 22); + + Mockito.verify(networkACLServiceMock).revokeNetworkACLItem(3L); + Mockito.verifyNoMoreInteractions(networkACLServiceMock); } @Test