diff --git a/engine/schema/src/main/java/com/cloud/network/dao/FirewallRulesDao.java b/engine/schema/src/main/java/com/cloud/network/dao/FirewallRulesDao.java index b80ccd9cd1b9..056445225d0b 100644 --- a/engine/schema/src/main/java/com/cloud/network/dao/FirewallRulesDao.java +++ b/engine/schema/src/main/java/com/cloud/network/dao/FirewallRulesDao.java @@ -43,7 +43,9 @@ public interface FirewallRulesDao extends GenericDao { List listStaticNatByVmId(long vmId); - List listByIpPurposeAndProtocolAndNotRevoked(long ipAddressId, Integer startPort, Integer endPort, String protocol, FirewallRule.Purpose purpose); + List listByIpPurposePortsProtocolAndNotRevoked(long ipAddressId, Integer startPort, Integer endPort, String protocol, FirewallRule.Purpose purpose); + + List listByIpPurposeProtocolAndNotRevoked(long ipAddressId, FirewallRule.Purpose purpose, String protocol); FirewallRuleVO findByRelatedId(long ruleId); diff --git a/engine/schema/src/main/java/com/cloud/network/dao/FirewallRulesDaoImpl.java b/engine/schema/src/main/java/com/cloud/network/dao/FirewallRulesDaoImpl.java index 1698e0ed2da9..fb53efda24b8 100644 --- a/engine/schema/src/main/java/com/cloud/network/dao/FirewallRulesDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/network/dao/FirewallRulesDaoImpl.java @@ -266,8 +266,25 @@ public void saveDestinationCidrs(FirewallRuleVO firewallRule, List cidrL } @Override - public List listByIpPurposeAndProtocolAndNotRevoked(long ipAddressId, Integer startPort, Integer endPort, String protocol, - FirewallRule.Purpose purpose) { + public List listByIpPurposeProtocolAndNotRevoked(long ipAddressId, Purpose purpose, String protocol) { + SearchCriteria sc = NotRevokedSearch.create(); + sc.setParameters("ipId", ipAddressId); + sc.setParameters("state", State.Revoke); + + if (purpose != null) { + sc.setParameters("purpose", purpose); + } + + if (protocol != null) { + sc.setParameters("protocol", protocol); + } + + return listBy(sc); + } + + @Override + public List listByIpPurposePortsProtocolAndNotRevoked(long ipAddressId, Integer startPort, Integer endPort, String protocol, + FirewallRule.Purpose purpose) { SearchCriteria sc = NotRevokedSearch.create(); sc.setParameters("ipId", ipAddressId); sc.setParameters("state", State.Revoke); 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 39e4bbf935fc..f31e161b33b5 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 @@ -74,6 +74,7 @@ import org.apache.cloudstack.network.RoutedIpv4Manager; import org.apache.commons.codec.binary.Base64; import org.apache.commons.collections.CollectionUtils; +import org.apache.commons.lang3.ObjectUtils; import org.apache.commons.lang3.StringUtils; import com.cloud.api.ApiDBUtils; @@ -382,10 +383,10 @@ public VMTemplateVO getKubernetesServiceTemplate(DataCenter dataCenter, Hypervis } protected void validateIsolatedNetworkIpRules(long ipId, FirewallRule.Purpose purpose, Network network, int clusterTotalNodeCount) { - List rules = firewallRulesDao.listByIpAndPurposeAndNotRevoked(ipId, purpose); + List rules = firewallRulesDao.listByIpPurposeProtocolAndNotRevoked(ipId, purpose, NetUtils.TCP_PROTO); for (FirewallRuleVO rule : rules) { - Integer startPort = rule.getSourcePortStart(); - Integer endPort = rule.getSourcePortEnd(); + int startPort = ObjectUtils.defaultIfNull(rule.getSourcePortStart(), 1); + int endPort = ObjectUtils.defaultIfNull(rule.getSourcePortEnd(), KubernetesClusterActionWorker.MAX_PORT); if (logger.isDebugEnabled()) { logger.debug(String.format("Validating rule with purpose: %s for network: %s with ports: %d-%d", purpose.toString(), network.getUuid(), startPort, endPort)); } diff --git a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterActionWorker.java b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterActionWorker.java index 743962a1f00b..4df47e12b025 100644 --- a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterActionWorker.java +++ b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterActionWorker.java @@ -106,6 +106,7 @@ public class KubernetesClusterActionWorker { public static final int DEFAULT_SSH_PORT = 22; public static final int CLUSTER_NODES_DEFAULT_START_SSH_PORT = 2222; public static final int CLUSTER_NODES_DEFAULT_SSH_PORT_SG = DEFAULT_SSH_PORT; + public static final int MAX_PORT = 65535; public static final String CKS_CLUSTER_SECURITY_GROUP_NAME = "CKSSecurityGroup"; public static final String CKS_SECURITY_GROUP_DESCRIPTION = "Security group for CKS nodes"; 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 ec461610e328..e5f916632e9f 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 @@ -498,10 +498,12 @@ protected void provisionSshPortForwardingRules(IpAddress publicIp, Network netwo protected FirewallRule removeApiFirewallRule(final IpAddress publicIp) { FirewallRule rule = null; - List firewallRules = firewallRulesDao.listByIpAndPurposeAndNotRevoked(publicIp.getId(), FirewallRule.Purpose.Firewall); + List firewallRules = firewallRulesDao.listByIpPurposeProtocolAndNotRevoked(publicIp.getId(), FirewallRule.Purpose.Firewall, NetUtils.TCP_PROTO); for (FirewallRuleVO firewallRule : firewallRules) { - if (firewallRule.getSourcePortStart() == CLUSTER_API_PORT && - firewallRule.getSourcePortEnd() == CLUSTER_API_PORT) { + Integer startPort = firewallRule.getSourcePortStart(); + Integer endPort = firewallRule.getSourcePortEnd(); + if (startPort != null && startPort == CLUSTER_API_PORT && + endPort != null && endPort == CLUSTER_API_PORT) { rule = firewallRule; firewallService.revokeIngressFwRule(firewallRule.getId(), true); logger.debug("The API firewall rule [%s] with the id [%s] was revoked",firewallRule.getName(),firewallRule.getId()); @@ -513,9 +515,10 @@ protected FirewallRule removeApiFirewallRule(final IpAddress publicIp) { protected FirewallRule removeSshFirewallRule(final IpAddress publicIp) { FirewallRule rule = null; - List firewallRules = firewallRulesDao.listByIpAndPurposeAndNotRevoked(publicIp.getId(), FirewallRule.Purpose.Firewall); + List firewallRules = firewallRulesDao.listByIpPurposeProtocolAndNotRevoked(publicIp.getId(), FirewallRule.Purpose.Firewall, NetUtils.TCP_PROTO); for (FirewallRuleVO firewallRule : firewallRules) { - if (firewallRule.getSourcePortStart() == CLUSTER_NODES_DEFAULT_START_SSH_PORT) { + Integer startPort = firewallRule.getSourcePortStart(); + if (startPort != null && startPort == CLUSTER_NODES_DEFAULT_START_SSH_PORT) { rule = firewallRule; firewallService.revokeIngressFwRule(firewallRule.getId(), true); logger.debug("The SSH firewall rule [%s] with the id [%s] was revoked",firewallRule.getName(),firewallRule.getId()); 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 a6d46ffc9aa1..f64c467b71b0 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 @@ -37,6 +37,7 @@ import com.cloud.user.Account; import com.cloud.user.AccountManager; import com.cloud.user.User; +import com.cloud.utils.net.NetUtils; import com.cloud.vm.VMInstanceVO; import com.cloud.vm.dao.VMInstanceDao; import org.apache.cloudstack.api.BaseCmd; @@ -117,7 +118,7 @@ public void validateIsolatedNetworkIpRulesNoRules() { long ipId = 1L; FirewallRule.Purpose purpose = FirewallRule.Purpose.Firewall; Network network = Mockito.mock(Network.class); - Mockito.when(firewallRulesDao.listByIpAndPurposeAndNotRevoked(ipId, purpose)).thenReturn(new ArrayList<>()); + Mockito.when(firewallRulesDao.listByIpPurposeProtocolAndNotRevoked(ipId, purpose, NetUtils.TCP_PROTO)).thenReturn(new ArrayList<>()); kubernetesClusterManager.validateIsolatedNetworkIpRules(ipId, FirewallRule.Purpose.Firewall, network, 3); } @@ -131,7 +132,7 @@ public void validateIsolatedNetworkIpRulesNoConflictingRules() { long ipId = 1L; FirewallRule.Purpose purpose = FirewallRule.Purpose.Firewall; Network network = Mockito.mock(Network.class); - Mockito.when(firewallRulesDao.listByIpAndPurposeAndNotRevoked(ipId, purpose)).thenReturn(List.of(createRule(80, 80), createRule(443, 443))); + Mockito.when(firewallRulesDao.listByIpPurposeProtocolAndNotRevoked(ipId, purpose, NetUtils.TCP_PROTO)).thenReturn(List.of(createRule(80, 80), createRule(443, 443))); kubernetesClusterManager.validateIsolatedNetworkIpRules(ipId, FirewallRule.Purpose.Firewall, network, 3); } @@ -140,7 +141,7 @@ public void validateIsolatedNetworkIpRulesApiConflictingRules() { long ipId = 1L; FirewallRule.Purpose purpose = FirewallRule.Purpose.Firewall; Network network = Mockito.mock(Network.class); - Mockito.when(firewallRulesDao.listByIpAndPurposeAndNotRevoked(ipId, purpose)).thenReturn(List.of(createRule(6440, 6445), createRule(443, 443))); + Mockito.when(firewallRulesDao.listByIpPurposeProtocolAndNotRevoked(ipId, purpose, NetUtils.TCP_PROTO)).thenReturn(List.of(createRule(6440, 6445), createRule(443, 443))); kubernetesClusterManager.validateIsolatedNetworkIpRules(ipId, FirewallRule.Purpose.Firewall, network, 3); } @@ -149,7 +150,7 @@ public void validateIsolatedNetworkIpRulesSshConflictingRules() { long ipId = 1L; FirewallRule.Purpose purpose = FirewallRule.Purpose.Firewall; Network network = Mockito.mock(Network.class); - Mockito.when(firewallRulesDao.listByIpAndPurposeAndNotRevoked(ipId, purpose)).thenReturn(List.of(createRule(2200, KubernetesClusterActionWorker.CLUSTER_NODES_DEFAULT_START_SSH_PORT), createRule(443, 443))); + Mockito.when(firewallRulesDao.listByIpPurposeProtocolAndNotRevoked(ipId, purpose, NetUtils.TCP_PROTO)).thenReturn(List.of(createRule(2200, KubernetesClusterActionWorker.CLUSTER_NODES_DEFAULT_START_SSH_PORT), createRule(443, 443))); kubernetesClusterManager.validateIsolatedNetworkIpRules(ipId, FirewallRule.Purpose.Firewall, network, 3); } @@ -158,7 +159,7 @@ public void validateIsolatedNetworkIpRulesNearConflictingRules() { long ipId = 1L; FirewallRule.Purpose purpose = FirewallRule.Purpose.Firewall; Network network = Mockito.mock(Network.class); - Mockito.when(firewallRulesDao.listByIpAndPurposeAndNotRevoked(ipId, purpose)).thenReturn(List.of(createRule(2220, 2221), createRule(2225, 2227), createRule(6440, 6442), createRule(6444, 6446))); + Mockito.when(firewallRulesDao.listByIpPurposeProtocolAndNotRevoked(ipId, purpose, NetUtils.TCP_PROTO)).thenReturn(List.of(createRule(2220, 2221), createRule(2225, 2227), createRule(6440, 6442), createRule(6444, 6446))); kubernetesClusterManager.validateIsolatedNetworkIpRules(ipId, FirewallRule.Purpose.Firewall, network, 3); } diff --git a/server/src/main/java/com/cloud/network/firewall/FirewallManagerImpl.java b/server/src/main/java/com/cloud/network/firewall/FirewallManagerImpl.java index 7194b86e3e1d..c92b46771556 100644 --- a/server/src/main/java/com/cloud/network/firewall/FirewallManagerImpl.java +++ b/server/src/main/java/com/cloud/network/firewall/FirewallManagerImpl.java @@ -968,7 +968,7 @@ public FirewallRule createRuleForAllCidrs(long ipAddrId, Account caller, Integer Long relatedRuleId, long networkId) throws NetworkRuleConflictException { // If firwallRule for this port range already exists, return it - List rules = _firewallDao.listByIpPurposeAndProtocolAndNotRevoked(ipAddrId, startPort, endPort, protocol, Purpose.Firewall); + List rules = _firewallDao.listByIpPurposePortsProtocolAndNotRevoked(ipAddrId, startPort, endPort, protocol, Purpose.Firewall); if (!rules.isEmpty()) { return rules.get(0); }