Fix NPE during kubernetes cluster creation when network has rules with ports saved as null on DB (#9223)

Co-authored-by: Gabriel <gabriel.fernandes@scclouds.com.br>
Co-authored-by: Fabricio Duarte <fabricio.duarte.jr@gmail.com>
This commit is contained in:
GaOrtiga 2025-02-13 06:25:16 -03:00 committed by GitHub
parent 0dcb8da03a
commit f8563b86e7
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 41 additions and 17 deletions

View File

@ -43,7 +43,9 @@ public interface FirewallRulesDao extends GenericDao<FirewallRuleVO, Long> {
List<FirewallRuleVO> listStaticNatByVmId(long vmId);
List<FirewallRuleVO> listByIpPurposeAndProtocolAndNotRevoked(long ipAddressId, Integer startPort, Integer endPort, String protocol, FirewallRule.Purpose purpose);
List<FirewallRuleVO> listByIpPurposePortsProtocolAndNotRevoked(long ipAddressId, Integer startPort, Integer endPort, String protocol, FirewallRule.Purpose purpose);
List<FirewallRuleVO> listByIpPurposeProtocolAndNotRevoked(long ipAddressId, FirewallRule.Purpose purpose, String protocol);
FirewallRuleVO findByRelatedId(long ruleId);

View File

@ -263,8 +263,25 @@ public class FirewallRulesDaoImpl extends GenericDaoBase<FirewallRuleVO, Long> i
}
@Override
public List<FirewallRuleVO> listByIpPurposeAndProtocolAndNotRevoked(long ipAddressId, Integer startPort, Integer endPort, String protocol,
FirewallRule.Purpose purpose) {
public List<FirewallRuleVO> listByIpPurposeProtocolAndNotRevoked(long ipAddressId, Purpose purpose, String protocol) {
SearchCriteria<FirewallRuleVO> 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<FirewallRuleVO> listByIpPurposePortsProtocolAndNotRevoked(long ipAddressId, Integer startPort, Integer endPort, String protocol,
FirewallRule.Purpose purpose) {
SearchCriteria<FirewallRuleVO> sc = NotRevokedSearch.create();
sc.setParameters("ipId", ipAddressId);
sc.setParameters("state", State.Revoke);

View File

@ -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<FirewallRuleVO> rules = firewallRulesDao.listByIpAndPurposeAndNotRevoked(ipId, purpose);
List<FirewallRuleVO> 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);
}

View File

@ -498,10 +498,12 @@ public class KubernetesClusterResourceModifierActionWorker extends KubernetesClu
protected FirewallRule removeApiFirewallRule(final IpAddress publicIp) {
FirewallRule rule = null;
List<FirewallRuleVO> firewallRules = firewallRulesDao.listByIpAndPurposeAndNotRevoked(publicIp.getId(), FirewallRule.Purpose.Firewall);
List<FirewallRuleVO> 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<FirewallRuleVO> firewallRules = firewallRulesDao.listByIpAndPurposeAndNotRevoked(publicIp.getId(), FirewallRule.Purpose.Firewall);
List<FirewallRuleVO> 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());

View File

@ -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);
}

View File

@ -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<FirewallRuleVO> rules = _firewallDao.listByIpPurposeAndProtocolAndNotRevoked(ipAddrId, startPort, endPort, protocol, Purpose.Firewall);
List<FirewallRuleVO> rules = _firewallDao.listByIpPurposePortsProtocolAndNotRevoked(ipAddrId, startPort, endPort, protocol, Purpose.Firewall);
if (!rules.isEmpty()) {
return rules.get(0);
}