diff --git a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterDestroyWorker.java b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterDestroyWorker.java index 62bd8b4576a4..aaa572561947 100644 --- a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterDestroyWorker.java +++ b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterDestroyWorker.java @@ -182,6 +182,7 @@ protected void deleteKubernetesClusterIsolatedNetworkRules(Network network, List if (firewallRule == null) { logMessage(Level.WARN, "Firewall rule for SSH access can't be removed", null); } + removeEtcdFirewallRules(publicIp); try { removePortForwardingRules(publicIp, network, owner, removedVmIds); } catch (ResourceUnavailableException e) { 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..fdf9a0b8f62c 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 @@ -80,6 +80,7 @@ import com.cloud.hypervisor.Hypervisor; import com.cloud.kubernetes.cluster.KubernetesCluster; import com.cloud.kubernetes.cluster.KubernetesClusterManagerImpl; +import com.cloud.kubernetes.cluster.KubernetesClusterService; import com.cloud.kubernetes.cluster.KubernetesClusterVO; import com.cloud.network.IpAddress; import com.cloud.network.Network; @@ -544,20 +545,45 @@ protected FirewallRule removeApiFirewallRule(final IpAddress publicIp) { } protected FirewallRule removeSshFirewallRule(final IpAddress publicIp, final long networkId) { - FirewallRule rule = null; + FirewallRuleVO rule = null; List firewallRules = firewallRulesDao.listByIpPurposeProtocolAndNotRevoked(publicIp.getId(), FirewallRule.Purpose.Firewall, NetUtils.TCP_PROTO); for (FirewallRuleVO firewallRule : firewallRules) { - PortForwardingRuleVO pfRule = portForwardingRulesDao.findByNetworkAndPorts(networkId, firewallRule.getSourcePortStart(), firewallRule.getSourcePortEnd()); - if (Objects.equals(firewallRule.getSourcePortStart(), CLUSTER_NODES_DEFAULT_START_SSH_PORT) || (Objects.nonNull(pfRule) && pfRule.getDestinationPortStart() == DEFAULT_SSH_PORT) ) { + if (Objects.equals(firewallRule.getSourcePortStart(), CLUSTER_NODES_DEFAULT_START_SSH_PORT)) { rule = firewallRule; - firewallService.revokeIngressFwRule(firewallRule.getId(), true); - logger.debug("The SSH firewall rule {} with the id {} was revoked", firewallRule.getName(), firewallRule.getId()); break; } + if (rule == null && firewallRule.getSourcePortStart() != null + && firewallRule.getSourcePortStart() < KubernetesClusterService.KubernetesEtcdNodeStartPort.value()) { + PortForwardingRuleVO pfRule = portForwardingRulesDao.findByNetworkAndPorts(networkId, firewallRule.getSourcePortStart(), firewallRule.getSourcePortEnd()); + if (Objects.nonNull(pfRule) && pfRule.getDestinationPortStart() == DEFAULT_SSH_PORT) { + rule = firewallRule; + } + } + } + if (rule != null) { + firewallService.revokeIngressFwRule(rule.getId(), true); + logger.debug("The SSH firewall rule {} with the id {} was revoked", rule.getName(), rule.getId()); } return rule; } + protected void removeEtcdFirewallRules(final IpAddress publicIp) { + Long etcdNodeCount = kubernetesCluster.getEtcdNodeCount(); + if (etcdNodeCount == null || etcdNodeCount <= 0) { + return; + } + int etcdStartPort = KubernetesClusterService.KubernetesEtcdNodeStartPort.value(); + int etcdEndPort = etcdStartPort + etcdNodeCount.intValue() - 1; + List firewallRules = firewallRulesDao.listByIpPurposeProtocolAndNotRevoked(publicIp.getId(), FirewallRule.Purpose.Firewall, NetUtils.TCP_PROTO); + for (FirewallRuleVO firewallRule : firewallRules) { + Integer startPort = firewallRule.getSourcePortStart(); + if (startPort != null && startPort >= etcdStartPort && startPort <= etcdEndPort) { + firewallService.revokeIngressFwRule(firewallRule.getId(), true); + logger.debug("The etcd SSH firewall rule {} with the id {} was revoked", firewallRule.getName(), firewallRule.getId()); + } + } + } + protected void removePortForwardingRules(final IpAddress publicIp, final Network network, final Account account, final List removedVMIds) throws ResourceUnavailableException { if (!CollectionUtils.isEmpty(removedVMIds)) { List pfRules = new ArrayList<>(); 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..1ab62dce0010 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 @@ -19,10 +19,20 @@ import com.cloud.kubernetes.cluster.KubernetesCluster; import com.cloud.kubernetes.cluster.KubernetesClusterManagerImpl; +import com.cloud.kubernetes.cluster.KubernetesClusterService; 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.dao.FirewallRulesDao; +import com.cloud.network.firewall.FirewallService; +import com.cloud.network.rules.FirewallRule; +import com.cloud.network.rules.FirewallRuleVO; +import com.cloud.network.rules.PortForwardingRuleVO; +import com.cloud.network.rules.dao.PortForwardingRulesDao; +import com.cloud.utils.net.NetUtils; +import java.util.Arrays; import org.junit.Assert; import org.junit.Before; import org.junit.Test; @@ -51,6 +61,18 @@ public class KubernetesClusterResourceModifierActionWorkerTest { @Mock private KubernetesCluster kubernetesClusterMock; + @Mock + private IpAddress publicIpMock; + + @Mock + private FirewallRulesDao firewallRulesDaoMock; + + @Mock + private PortForwardingRulesDao portForwardingRulesDaoMock; + + @Mock + private FirewallService firewallServiceMock; + private KubernetesClusterResourceModifierActionWorker kubernetesClusterResourceModifierActionWorker; @Before @@ -135,4 +157,99 @@ public void getKubernetesClusterNodeNamePrefixTestNormalizedPrefixShouldNotStart Mockito.when(kubernetesClusterMock.getName()).thenReturn(originalPrefix); Assert.assertEquals(expectedPrefix, kubernetesClusterResourceModifierActionWorker.getKubernetesClusterNodeNamePrefix()); } + + private static final long NETWORK_ID = 10L; + + private void mockSshFirewallRules(FirewallRuleVO... rules) { + kubernetesClusterResourceModifierActionWorker.firewallRulesDao = firewallRulesDaoMock; + kubernetesClusterResourceModifierActionWorker.portForwardingRulesDao = portForwardingRulesDaoMock; + kubernetesClusterResourceModifierActionWorker.firewallService = firewallServiceMock; + Mockito.when(publicIpMock.getId()).thenReturn(1L); + Mockito.when(firewallRulesDaoMock.listByIpPurposeProtocolAndNotRevoked(1L, FirewallRule.Purpose.Firewall, NetUtils.TCP_PROTO)).thenReturn(Arrays.asList(rules)); + } + + private FirewallRuleVO mockSshForwardedFirewallRule(int port) { + FirewallRuleVO rule = Mockito.mock(FirewallRuleVO.class); + Mockito.when(rule.getSourcePortStart()).thenReturn(port); + Mockito.when(rule.getSourcePortEnd()).thenReturn(port); + PortForwardingRuleVO pfRule = Mockito.mock(PortForwardingRuleVO.class); + Mockito.when(pfRule.getDestinationPortStart()).thenReturn(KubernetesClusterActionWorker.DEFAULT_SSH_PORT); + Mockito.when(portForwardingRulesDaoMock.findByNetworkAndPorts(NETWORK_ID, port, port)).thenReturn(pfRule); + return rule; + } + + @Test + public void removeSshFirewallRuleTestPrefersNodesRuleOverEtcdRuleListedFirst() { + FirewallRuleVO etcdRule = mockSshForwardedFirewallRule(50000); + FirewallRuleVO nodesRule = Mockito.mock(FirewallRuleVO.class); + Mockito.when(nodesRule.getSourcePortStart()).thenReturn(KubernetesClusterActionWorker.CLUSTER_NODES_DEFAULT_START_SSH_PORT); + Mockito.when(nodesRule.getId()).thenReturn(11L); + mockSshFirewallRules(etcdRule, nodesRule); + + Assert.assertSame(nodesRule, kubernetesClusterResourceModifierActionWorker.removeSshFirewallRule(publicIpMock, NETWORK_ID)); + Mockito.verify(firewallServiceMock).revokeIngressFwRule(11L, true); + Mockito.verifyNoMoreInteractions(firewallServiceMock); + } + + @Test + public void removeSshFirewallRuleTestFallsBackToSshForwardedRule() { + FirewallRuleVO externalNodeRule = mockSshForwardedFirewallRule(2225); + Mockito.when(externalNodeRule.getId()).thenReturn(12L); + mockSshFirewallRules(externalNodeRule); + + Assert.assertSame(externalNodeRule, kubernetesClusterResourceModifierActionWorker.removeSshFirewallRule(publicIpMock, NETWORK_ID)); + Mockito.verify(firewallServiceMock).revokeIngressFwRule(12L, true); + } + + @Test + public void removeSshFirewallRuleTestReturnsNullWhenNoSshRule() { + FirewallRuleVO apiRule = Mockito.mock(FirewallRuleVO.class); + Mockito.when(apiRule.getSourcePortStart()).thenReturn(KubernetesClusterActionWorker.CLUSTER_API_PORT); + Mockito.when(apiRule.getSourcePortEnd()).thenReturn(KubernetesClusterActionWorker.CLUSTER_API_PORT); + mockSshFirewallRules(apiRule); + + Assert.assertNull(kubernetesClusterResourceModifierActionWorker.removeSshFirewallRule(publicIpMock, NETWORK_ID)); + Mockito.verifyNoMoreInteractions(firewallServiceMock); + } + + @Test + public void removeSshFirewallRuleTestDoesNotPickEtcdRuleWhenNodeRuleMissing() { + FirewallRuleVO etcdRule = Mockito.mock(FirewallRuleVO.class); + Mockito.when(etcdRule.getSourcePortStart()).thenReturn(KubernetesClusterService.KubernetesEtcdNodeStartPort.value()); + mockSshFirewallRules(etcdRule); + + Assert.assertNull(kubernetesClusterResourceModifierActionWorker.removeSshFirewallRule(publicIpMock, NETWORK_ID)); + Mockito.verify(firewallServiceMock, Mockito.never()).revokeIngressFwRule(Mockito.anyLong(), Mockito.anyBoolean()); + } + + @Test + public void removeEtcdFirewallRulesTestRevokesOnlyEtcdRangeRules() { + int etcdStart = KubernetesClusterService.KubernetesEtcdNodeStartPort.value(); + Mockito.when(kubernetesClusterMock.getEtcdNodeCount()).thenReturn(2L); + + FirewallRuleVO nodeRule = Mockito.mock(FirewallRuleVO.class); + Mockito.when(nodeRule.getSourcePortStart()).thenReturn(KubernetesClusterActionWorker.CLUSTER_NODES_DEFAULT_START_SSH_PORT); + FirewallRuleVO etcdRuleOne = Mockito.mock(FirewallRuleVO.class); + Mockito.when(etcdRuleOne.getSourcePortStart()).thenReturn(etcdStart); + Mockito.when(etcdRuleOne.getId()).thenReturn(21L); + FirewallRuleVO etcdRuleTwo = Mockito.mock(FirewallRuleVO.class); + Mockito.when(etcdRuleTwo.getSourcePortStart()).thenReturn(etcdStart + 1); + Mockito.when(etcdRuleTwo.getId()).thenReturn(22L); + mockSshFirewallRules(nodeRule, etcdRuleOne, etcdRuleTwo); + + kubernetesClusterResourceModifierActionWorker.removeEtcdFirewallRules(publicIpMock); + + Mockito.verify(firewallServiceMock).revokeIngressFwRule(21L, true); + Mockito.verify(firewallServiceMock).revokeIngressFwRule(22L, true); + Mockito.verifyNoMoreInteractions(firewallServiceMock); + } + + @Test + public void removeEtcdFirewallRulesTestNoOpWhenClusterHasNoEtcdNodes() { + Mockito.when(kubernetesClusterMock.getEtcdNodeCount()).thenReturn(null); + + kubernetesClusterResourceModifierActionWorker.removeEtcdFirewallRules(publicIpMock); + + Mockito.verifyNoInteractions(firewallServiceMock, firewallRulesDaoMock); + } }