diff --git a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImpl.java b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImpl.java index d19470f8bab2..344fb9b72dec 100644 --- a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImpl.java +++ b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImpl.java @@ -602,7 +602,7 @@ protected void validateVpcTier(Network network) { if (Network.State.Allocated.equals(network.getState())) { // Allocated networks won't have IP and rules return; } - if (network.getNetworkACLId() == NetworkACL.DEFAULT_DENY) { + if (Objects.equals(network.getNetworkACLId(), NetworkACL.DEFAULT_DENY)) { throw new InvalidParameterValueException(String.format("Network ID: %s can not be used for Kubernetes cluster as it uses default deny ACL", network.getUuid())); } } 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 55924cb32e8a..5ed1f23dcaed 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 @@ -717,7 +717,7 @@ protected void setupKubernetesClusterIsolatedNetworkRules(IpAddress publicIp, Ne } protected void createVpcTierAclRules(Network network) throws ManagementServerException { - if (network.getNetworkACLId() == NetworkACL.DEFAULT_ALLOW) { + if (Objects.equals(network.getNetworkACLId(), NetworkACL.DEFAULT_ALLOW)) { return; } // ACL rule for API access for control node VMs @@ -746,7 +746,8 @@ protected void createVpcTierAclRules(Network network) throws ManagementServerExc } protected void removeVpcTierAclRules(Network network) throws ManagementServerException { - if (network.getNetworkACLId() == NetworkACL.DEFAULT_ALLOW) { + Long networkAclId = network.getNetworkACLId(); + if (networkAclId == null || Objects.equals(networkAclId, NetworkACL.DEFAULT_ALLOW)) { return; } // ACL rule for API access for control node VMs diff --git a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterStartWorker.java b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterStartWorker.java index aa9317e619b0..bde482acf65a 100644 --- a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterStartWorker.java +++ b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterStartWorker.java @@ -646,7 +646,7 @@ protected void setupKubernetesEtcdNetworkRules(List etcdVms, Network net try { if (Objects.isNull(network.getVpcId())) { provisionFirewallRules(publicIp, owner, etcdStartPort, etcdStartPort); - } else if (network.getNetworkACLId() != NetworkACL.DEFAULT_ALLOW) { + } else if (!Objects.equals(network.getNetworkACLId(), NetworkACL.DEFAULT_ALLOW)) { try { provisionVpcTierAllowPortACLRule(network, ETCD_NODE_CLIENT_REQUEST_PORT, ETCD_NODE_CLIENT_REQUEST_PORT); if (logger.isInfoEnabled()) { diff --git a/plugins/integrations/kubernetes-service/src/test/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImplTest.java b/plugins/integrations/kubernetes-service/src/test/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImplTest.java index 2a381f282de2..2e90eea22f03 100644 --- a/plugins/integrations/kubernetes-service/src/test/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImplTest.java +++ b/plugins/integrations/kubernetes-service/src/test/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImplTest.java @@ -130,6 +130,14 @@ public void testValidateVpcTierValid() { kubernetesClusterManager.validateVpcTier(network); } + @Test + public void testValidateVpcTierWithoutAcl() { + Network network = Mockito.mock(Network.class); + Mockito.when(network.getState()).thenReturn(Network.State.Implemented); + Mockito.when(network.getNetworkACLId()).thenReturn(null); + kubernetesClusterManager.validateVpcTier(network); + } + @Test public void validateIsolatedNetworkIpRulesNoRules() { long ipId = 1L; 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..5d37ae43af4b 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 @@ -17,17 +17,28 @@ package com.cloud.kubernetes.cluster.actionworkers; +import java.util.List; + import com.cloud.kubernetes.cluster.KubernetesCluster; import com.cloud.kubernetes.cluster.KubernetesClusterManagerImpl; import com.cloud.kubernetes.cluster.dao.KubernetesClusterDao; 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.IpAddress; +import com.cloud.network.Network; +import com.cloud.network.dao.IPAddressDao; +import com.cloud.network.dao.IPAddressVO; +import com.cloud.network.vpc.NetworkACL; +import com.cloud.user.Account; +import com.cloud.uservm.UserVm; +import org.apache.cloudstack.context.CallContext; import org.junit.Assert; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; +import org.mockito.MockedStatic; import org.mockito.Mockito; import org.mockito.junit.MockitoJUnitRunner; @@ -51,7 +62,52 @@ public class KubernetesClusterResourceModifierActionWorkerTest { @Mock private KubernetesCluster kubernetesClusterMock; - private KubernetesClusterResourceModifierActionWorker kubernetesClusterResourceModifierActionWorker; + @Mock + private IPAddressDao ipAddressDaoMock; + + @Mock + private Account accountMock; + + private TestKubernetesClusterResourceModifierActionWorker kubernetesClusterResourceModifierActionWorker; + + private static class TestKubernetesClusterResourceModifierActionWorker extends KubernetesClusterResourceModifierActionWorker { + private int provisionAclRuleCalls; + private int removeAclRuleCalls; + + TestKubernetesClusterResourceModifierActionWorker(KubernetesCluster kubernetesCluster, KubernetesClusterManagerImpl clusterManager) { + super(kubernetesCluster, clusterManager); + } + + @Override + protected void provisionVpcTierAllowPortACLRule(Network network, int startPort, int endPort) { + provisionAclRuleCalls++; + } + + @Override + protected void removeVpcTierAllowPortACLRule(Network network, int startPort, int endPort) { + removeAclRuleCalls++; + } + } + + private static class TestKubernetesClusterStartWorker extends KubernetesClusterStartWorker { + private int provisionAclRuleCalls; + private int provisionPortForwardingRuleCalls; + + TestKubernetesClusterStartWorker(KubernetesCluster kubernetesCluster, KubernetesClusterManagerImpl clusterManager) { + super(kubernetesCluster, clusterManager); + } + + @Override + protected void provisionVpcTierAllowPortACLRule(Network network, int startPort, int endPort) { + provisionAclRuleCalls++; + } + + @Override + protected void provisionPublicIpPortForwardingRule(IpAddress publicIp, Network network, Account account, + long vmId, int sourcePort, int destPort) { + provisionPortForwardingRuleCalls++; + } + } @Before public void setUp() { @@ -60,7 +116,7 @@ public void setUp() { kubernetesClusterManagerMock.kubernetesClusterDetailsDao = kubernetesClusterDetailsDaoMock; kubernetesClusterManagerMock.kubernetesClusterVmMapDao = kubernetesClusterVmMapDaoMock; - kubernetesClusterResourceModifierActionWorker = new KubernetesClusterResourceModifierActionWorker(kubernetesClusterMock, kubernetesClusterManagerMock); + kubernetesClusterResourceModifierActionWorker = new TestKubernetesClusterResourceModifierActionWorker(kubernetesClusterMock, kubernetesClusterManagerMock); } @Test @@ -135,4 +191,67 @@ public void getKubernetesClusterNodeNamePrefixTestNormalizedPrefixShouldNotStart Mockito.when(kubernetesClusterMock.getName()).thenReturn(originalPrefix); Assert.assertEquals(expectedPrefix, kubernetesClusterResourceModifierActionWorker.getKubernetesClusterNodeNamePrefix()); } + + @Test + public void createVpcTierAclRulesWithoutAclProvisionsRules() throws Exception { + Network network = Mockito.mock(Network.class); + Mockito.when(network.getNetworkACLId()).thenReturn(null); + + try (MockedStatic ignored = Mockito.mockStatic(CallContext.class)) { + kubernetesClusterResourceModifierActionWorker.createVpcTierAclRules(network); + } + + Assert.assertEquals(2, kubernetesClusterResourceModifierActionWorker.provisionAclRuleCalls); + } + + @Test + public void createVpcTierAclRulesWithDefaultAllowDoesNotProvisionRules() throws Exception { + Network network = Mockito.mock(Network.class); + Mockito.when(network.getNetworkACLId()).thenReturn(NetworkACL.DEFAULT_ALLOW); + + kubernetesClusterResourceModifierActionWorker.createVpcTierAclRules(network); + + Assert.assertEquals(0, kubernetesClusterResourceModifierActionWorker.provisionAclRuleCalls); + } + + @Test + public void removeVpcTierAclRulesWithoutAclDoesNotRemoveRules() throws Exception { + Network network = Mockito.mock(Network.class); + Mockito.when(network.getNetworkACLId()).thenReturn(null); + + kubernetesClusterResourceModifierActionWorker.removeVpcTierAclRules(network); + + Assert.assertEquals(0, kubernetesClusterResourceModifierActionWorker.removeAclRuleCalls); + } + + @Test + public void removeVpcTierAclRulesWithCustomAclRemovesRules() throws Exception { + Network network = Mockito.mock(Network.class); + Mockito.when(network.getNetworkACLId()).thenReturn(3L); + + kubernetesClusterResourceModifierActionWorker.removeVpcTierAclRules(network); + + Assert.assertEquals(2, kubernetesClusterResourceModifierActionWorker.removeAclRuleCalls); + } + + @Test + public void setupKubernetesEtcdNetworkRulesWithoutAclProvisionsAclRule() throws Exception { + Network network = Mockito.mock(Network.class); + Mockito.when(network.getVpcId()).thenReturn(1L); + Mockito.when(network.getNetworkACLId()).thenReturn(null); + UserVm etcdVm = Mockito.mock(UserVm.class); + Mockito.when(etcdVm.getId()).thenReturn(1L); + IPAddressVO publicIp = Mockito.mock(IPAddressVO.class); + Mockito.when(ipAddressDaoMock.findByIpAndDcId(Mockito.anyLong(), Mockito.anyString())).thenReturn(publicIp); + + TestKubernetesClusterStartWorker worker = new TestKubernetesClusterStartWorker(kubernetesClusterMock, kubernetesClusterManagerMock); + worker.ipAddressDao = ipAddressDaoMock; + worker.owner = accountMock; + worker.publicIpAddress = "192.0.2.1"; + + worker.setupKubernetesEtcdNetworkRules(List.of(etcdVm), network); + + Assert.assertEquals(1, worker.provisionAclRuleCalls); + Assert.assertEquals(1, worker.provisionPortForwardingRuleCalls); + } }