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 b80ccd9cd1b..056445225d0 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 c8bd7e2147e..a793a9172d4 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 @@ -263,8 +263,25 @@ public class FirewallRulesDaoImpl extends GenericDaoBase i } @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 bf7a2a5344a..22c8f0f0abc 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.managed.context.ManagedContextRunnable; 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 class KubernetesClusterManagerImpl extends ManagerBase implements Kuberne } 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(), NetUtils.PORT_RANGE_MAX); if (logger.isDebugEnabled()) { logger.debug("Validating rule with purpose: {} for network: {} with ports: {}-{}", purpose.toString(), network, startPort, endPort); } 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 8c983149d02..a8c4b307065 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 @@ public class KubernetesClusterResourceModifierActionWorker extends KubernetesClu 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 @@ public class KubernetesClusterResourceModifierActionWorker extends KubernetesClu 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 a6d46ffc9aa..f64c467b71b 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.storage.dao.VMTemplateDao; 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 class KubernetesClusterManagerImplTest { 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 class KubernetesClusterManagerImplTest { 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 class KubernetesClusterManagerImplTest { 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 class KubernetesClusterManagerImplTest { 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 class KubernetesClusterManagerImplTest { 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 6b8133534d2..4aee5fef48a 100644 --- a/server/src/main/java/com/cloud/network/firewall/FirewallManagerImpl.java +++ b/server/src/main/java/com/cloud/network/firewall/FirewallManagerImpl.java @@ -1009,7 +1009,7 @@ public class FirewallManagerImpl extends ManagerBase implements FirewallService, 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); }