From 60ca507b304e8fe5ce9f03d6311a190ed5051245 Mon Sep 17 00:00:00 2001 From: Ramgopal Nagaboina Date: Mon, 14 Sep 2026 12:56:17 -0400 Subject: [PATCH 1/2] kubernetes: prefer the node SSH rule when scaling a cluster with etcd nodes removeSshFirewallRule took the first TCP firewall rule on the source NAT IP that either starts at the node SSH start port or forwards to port 22. Clusters with separate etcd nodes also have single port rules that forward to port 22, and the rules are listed without an order. After a scale up the node SSH rule is recreated, so an etcd rule can come first and be revoked instead. The scale worker then used its end port as the bound for removing SSH port forwards, removing the etcd forwards too and failing to recreate the node rule. Prefer the rule that starts at the node SSH start port and only fall back to the port forwarding match when no such rule exists. Fixes #14159 --- ...esClusterResourceModifierActionWorker.java | 17 +++-- ...usterResourceModifierActionWorkerTest.java | 75 +++++++++++++++++++ 2 files changed, 87 insertions(+), 5 deletions(-) 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..f2fbdbc6f53f 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 @@ -544,16 +544,23 @@ 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) { + 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; } 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..597fa6974050 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,15 @@ 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 +60,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 +156,58 @@ 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); + } } From 75edb363b14715d73c7141d5552aaa8e5881ef5a Mon Sep 17 00:00:00 2001 From: rootnomad Date: Thu, 15 Oct 2026 11:06:20 -0400 Subject: [PATCH 2/2] kubernetes: keep etcd firewall rules out of the node SSH cleanup The SSH-rule lookup fell back to any firewall rule forwarding to port 22 when the node range rule (2222) was absent. etcd nodes also forward 50000+ to 22, so with the node rule deleted by hand the fallback could pick an etcd rule and the scale path would then wipe 2222..50000. Bound the fallback below KubernetesEtcdNodeStartPort; the caller already falls back to a safe range when no rule is returned. Also revoke the etcd ingress firewall rules on cluster destroy. They were left open in a network the user keeps, so the next cluster with etcd nodes clashed on those ports. --- .../KubernetesClusterDestroyWorker.java | 1 + ...esClusterResourceModifierActionWorker.java | 21 +++++++++- ...usterResourceModifierActionWorkerTest.java | 42 +++++++++++++++++++ 3 files changed, 63 insertions(+), 1 deletion(-) 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 f2fbdbc6f53f..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; @@ -551,7 +552,8 @@ protected FirewallRule removeSshFirewallRule(final IpAddress publicIp, final lon rule = firewallRule; break; } - if (rule == null) { + 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; @@ -565,6 +567,23 @@ protected FirewallRule removeSshFirewallRule(final IpAddress publicIp, final lon 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 597fa6974050..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,6 +19,7 @@ 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; @@ -210,4 +211,45 @@ public void removeSshFirewallRuleTestReturnsNullWhenNoSshRule() { 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); + } }