Skip to content

CKS: open the cluster public IP on VPC tiers when the VPC has the Firewall service #14670

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
adestis-mf wants to merge 1 commit into
base: main
Choose a base branch
from
+270 −24
Open

CKS: open the cluster public IP on VPC tiers when the VPC has the Firewall service #14670

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 51 additions & 0 deletions ...c/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterActionWorker.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
import com.cloud.kubernetes.cluster.utils.KubernetesClusterUtil;
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.RulesService;
import com.cloud.network.rules.dao.PortForwardingRulesDao;
Expand All @@ -59,6 +60,7 @@
import com.cloud.utils.component.ComponentContext;
import com.cloud.utils.db.TransactionCallbackWithException;
import com.cloud.utils.net.Ip;
import com.cloud.utils.net.NetUtils;
import com.cloud.vm.Nic;
import com.cloud.vm.NicVO;
import com.cloud.vm.VirtualMachine;
Expand Down Expand Up @@ -107,10 +109,12 @@
import com.cloud.network.Network;
import com.cloud.network.NetworkModel;
import com.cloud.network.NetworkService;
import com.cloud.network.dao.FirewallRulesDao;
import com.cloud.network.dao.IPAddressDao;
import com.cloud.network.dao.IPAddressVO;
import com.cloud.network.dao.NetworkDao;
import com.cloud.network.vpc.VpcService;
import com.cloud.network.vpc.dao.VpcServiceMapDao;
import com.cloud.projects.ProjectService;
import com.cloud.service.dao.ServiceOfferingDao;
import com.cloud.storage.Storage;
Expand Down Expand Up @@ -214,12 +218,16 @@ public class KubernetesClusterActionWorker {
@Inject
public VpcService vpcService;
@Inject
protected VpcServiceMapDao vpcServiceMapDao;
@Inject
public PortForwardingRulesDao portForwardingRulesDao;
@Inject
protected RulesService rulesService;
@Inject
protected FirewallService firewallService;
@Inject
protected FirewallRulesDao firewallRulesDao;
@Inject
private NicDao nicDao;
@Inject
protected AffinityGroupDao affinityGroupDao;
Expand Down Expand Up @@ -1113,6 +1121,49 @@ protected String updateKubeConfigWithRegistryDetails(String k8sConfig) {
return k8sConfig;
}

/**
* When the VPC virtual router provides the Firewall service of a VPC, it drops ingress traffic to
* public IPs that has no firewall rule (in addition to the network ACL of each tier). The ports
* used by CKS must then be opened on the cluster public IP, as for isolated networks. This is the
* same check that sets vpc_firewall_enabled for the VPC virtual router.
*
* @param network the network of the Kubernetes cluster
* @return true if the network is a VPC tier and the VPC virtual router provides the Firewall service
*/
protected boolean isVpcFirewallEnabled(Network network) {
if (network == null || network.getVpcId() == null) {
return false;
}
return vpcServiceMapDao.canProviderSupportServiceInVpc(network.getVpcId(), Network.Service.Firewall, Network.Provider.VPCVirtualRouter);
}

/**
* @param network the network of the Kubernetes cluster
* @return true if ingress firewall rules are needed on the cluster public IP: always for isolated
* networks, and for VPC tiers when the VPC uses the Firewall service
*/
protected boolean isPublicIpFirewallRequired(Network network) {
return network.getVpcId() == null || isVpcFirewallEnabled(network);
}

/**
* Finds the ingress firewall rule for the given public port range of the cluster. Firewall rules on a
* public IP of a VPC are stored with the VPC and without a network, so they are looked up by the IP.
*
* @return the rule, or null if none exists
*/
protected FirewallRuleVO findIngressFirewallRuleForPorts(Network network, Long publicIpId, int startPort, int endPort) {
if (network.getVpcId() == null) {
return firewallRulesDao.findByNetworkIdAndPorts(network.getId(), startPort, endPort);
}
if (publicIpId == null) {
return null;
}
List<FirewallRuleVO> rules = firewallRulesDao.listByIpPurposePortsProtocolAndNotRevoked(publicIpId, startPort, endPort,
NetUtils.TCP_PROTO, FirewallRule.Purpose.Firewall);
return CollectionUtils.isEmpty(rules) ? null : rules.get(0);
}

public Map<Long, Integer> addFirewallRulesForNodes(IpAddress publicIp, int size) throws ManagementServerException {
Map<Long, Integer> vmIdPortMap = new HashMap<>();
CallContext.register(CallContext.current(), null);
Expand Down
26 changes: 14 additions & 12 deletions .../src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterAddWorker.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
import com.cloud.kubernetes.cluster.utils.KubernetesClusterUtil;
import com.cloud.network.IpAddress;
import com.cloud.network.Network;
import com.cloud.network.dao.FirewallRulesDao;
import com.cloud.network.rules.FirewallRuleVO;
import com.cloud.network.rules.PortForwardingRuleVO;
import com.cloud.service.ServiceOfferingVO;
Expand All @@ -49,7 +48,6 @@
import org.apache.commons.codec.binary.Base64;
import org.apache.logging.log4j.Level;

import javax.inject.Inject;
import java.io.File;
import java.io.IOException;
import java.lang.reflect.Field;
Expand All @@ -60,8 +58,6 @@

public class KubernetesClusterAddWorker extends KubernetesClusterActionWorker {

@Inject
private FirewallRulesDao firewallRulesDao;
private long addNodeTimeoutTime;

List<Long> finalNodeIds = new ArrayList<>();
Expand Down Expand Up @@ -229,9 +225,13 @@ private Pair<Boolean, Integer> validateAndSetupNode(Network network, IpAddress p
provisionFirewallRules(publicIp, owner, sshStartPort, sshStartPort);
}
provisionPublicIpPortForwardingRule(publicIp, network, account, nodeId, sshStartPort, DEFAULT_SSH_PORT);
if (isVpcFirewallEnabled(network)) {
// In a VPC the firewall rule follows the port forwarding rule, see setupKubernetesClusterVpcTierRules
provisionFirewallRules(publicIp, owner, sshStartPort, sshStartPort);
}
boolean isCompatible = validateNodeCompatibility(publicIp, nodeId, sshStartPort);
if (!isCompatible) {
revertNetworkRules(network, nodeId, sshStartPort);
revertNetworkRules(network, publicIp, nodeId, sshStartPort);
return new Pair<>(false, nodeIndex);
}

Expand All @@ -249,13 +249,13 @@ private Pair<Boolean, Integer> validateAndSetupNode(Network network, IpAddress p
InsufficientCapacityException | IllegalAccessException e) {
logger.error(String.format("Failed to activate API port forwarding rules for the Kubernetes cluster : %s", kubernetesCluster.getName()));
// remove added Firewall and PF rules
revertNetworkRules(network, nodeId, sshStartPort);
revertNetworkRules(network, publicIp, nodeId, sshStartPort);
return new Pair<>( false, nodeIndex);
} catch (Exception e) {
String errMsg = String.format("Unexpected exception while trying to add the external node %s to the Kubernetes cluster %s: %s",
nodeId, kubernetesCluster.getName(), e.getMessage());
logger.error(errMsg, e);
revertNetworkRules(network, nodeId, sshStartPort);
revertNetworkRules(network, publicIp, nodeId, sshStartPort);
throw new CloudRuntimeException(e);
}
return new Pair<>(true, ++nodeIndex);
Expand Down Expand Up @@ -310,12 +310,14 @@ private void cleanupCloudInitSemFolder(UserVm userVm, IpAddress publicIp, File p
}
}

private void revertNetworkRules(Network network, long vmId, int port) {
private void revertNetworkRules(Network network, IpAddress publicIp, long vmId, int port) {
logger.debug(String.format("Reverting network rules for VM ID %s on network %s", vmId, network.getName()));
FirewallRuleVO ruleVO = firewallRulesDao.findByNetworkIdAndPorts(network.getId(), port, port);
if (Objects.isNull(network.getVpcId())) {
logger.debug(String.format("Removing firewall rule %s", ruleVO.getId()));
firewallService.revokeIngressFirewallRule(ruleVO.getId(), true);
if (isPublicIpFirewallRequired(network)) {
FirewallRuleVO ruleVO = findIngressFirewallRuleForPorts(network, publicIp.getId(), port, port);
if (ruleVO != null) {
logger.debug(String.format("Removing firewall rule %s", ruleVO.getId()));
firewallService.revokeIngressFirewallRule(ruleVO.getId(), true);
}
}
List<PortForwardingRuleVO> pfRules = portForwardingRulesDao.listByVm(vmId);
for (PortForwardingRuleVO pfRule : pfRules) {
Expand Down
15 changes: 7 additions & 8 deletions ...c/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterRemoveWorker.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
import com.cloud.kubernetes.cluster.KubernetesClusterVO;
import com.cloud.network.IpAddress;
import com.cloud.network.Network;
import com.cloud.network.dao.FirewallRulesDao;
import com.cloud.network.rules.FirewallRuleVO;
import com.cloud.network.rules.PortForwardingRuleVO;
import com.cloud.service.ServiceOfferingVO;
Expand All @@ -37,7 +36,6 @@
import org.apache.cloudstack.api.ApiCommandResourceType;
import org.apache.cloudstack.context.CallContext;

import javax.inject.Inject;
import java.io.File;
import java.util.ArrayList;
import java.util.List;
Expand All @@ -47,9 +45,6 @@

public class KubernetesClusterRemoveWorker extends KubernetesClusterActionWorker {

@Inject
private FirewallRulesDao firewallRulesDao;

private long removeNodeTimeoutTime;

public KubernetesClusterRemoveWorker(KubernetesCluster kubernetesCluster, KubernetesClusterManagerImpl clusterManager) {
Expand Down Expand Up @@ -128,9 +123,13 @@ protected boolean removeNodePortForwardingRules(Long nodeId, Network network, Us
for (PortForwardingRuleVO pfRule : pfRules) {
try {
result &= rulesService.revokePortForwardingRule(pfRule.getId(), true);
if (Objects.isNull(network.getVpcId())) {
FirewallRuleVO ruleVO = firewallRulesDao.findByNetworkIdAndPorts(network.getId(), pfRule.getSourcePortStart(), pfRule.getSourcePortEnd());
result &= firewallService.revokeIngressFirewallRule(ruleVO.getId(), true);
if (isPublicIpFirewallRequired(network)) {
FirewallRuleVO ruleVO = findIngressFirewallRuleForPorts(network, pfRule.getSourceIpAddressId(), pfRule.getSourcePortStart(), pfRule.getSourcePortEnd());
if (ruleVO != null) {
result &= firewallService.revokeIngressFirewallRule(ruleVO.getId(), true);
} else {
logger.warn("Firewall rule for port {} of node {} not found, it may have been removed manually", pfRule.getSourcePortStart(), vm.getName());
}
}
} catch (Exception e) {
String err = String.format("Failed to cleanup network rules for node %s, due to: %s", vm.getName(), e.getMessage());
Expand Down
11 changes: 8 additions & 3 deletions ...cloud/kubernetes/cluster/actionworkers/KubernetesClusterResourceModifierActionWorker.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,6 @@
import com.cloud.kubernetes.cluster.KubernetesClusterVO;
import com.cloud.network.IpAddress;
import com.cloud.network.Network;
import com.cloud.network.dao.FirewallRulesDao;
import com.cloud.network.dao.LoadBalancerDao;
import com.cloud.network.dao.LoadBalancerVO;
import com.cloud.network.lb.LoadBalancingRulesService;
Expand Down Expand Up @@ -130,8 +129,6 @@ public class KubernetesClusterResourceModifierActionWorker extends KubernetesClu
@Inject
protected HostDao hostDao;
@Inject
protected FirewallRulesDao firewallRulesDao;
@Inject
protected NetworkACLService networkACLService;
@Inject
protected NetworkACLItemDao networkACLItemDao;
Expand Down Expand Up @@ -832,6 +829,14 @@ protected void setupKubernetesClusterVpcTierRules(IpAddress publicIp, Network ne
} catch (ResourceUnavailableException | NetworkRuleConflictException e) {
throw new ManagementServerException(String.format("Failed to activate SSH port forwarding rules for the Kubernetes cluster : %s", kubernetesCluster.getName()), e);
}

// The VPC firewall drops traffic to the public IP unless a firewall rule allows it, open the
// API and SSH ports as for isolated networks. Create them after the load balancing and port
// forwarding rules: applying firewall rules on a VPC public IP does not associate the IP with
// the VPC virtual router, the first load balancing or port forwarding rule does.
if (isVpcFirewallEnabled(network)) {
createFirewallRules(publicIp, clusterVMIds, true);
}
}


Expand Down
12 changes: 12 additions & 0 deletions ...rc/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterScaleWorker.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,14 @@ private void scaleKubernetesClusterVpcTierRules(final List<Long> clusterVMIds) t
if (publicIp == null) {
throw new ManagementServerException(String.format("No public IP addresses found for VPC tier : %s, Kubernetes cluster : %s", network.getName(), kubernetesCluster.getName()));
}
boolean vpcFirewallEnabled = isVpcFirewallEnabled(network);
if (vpcFirewallEnabled) {
// Remove the existing SSH firewall rule, it is recreated below for the new node count
FirewallRule firewallRule = removeSshFirewallRule(publicIp, network.getId());
if (firewallRule == null) {
logger.warn("SSH firewall rule not found for Kubernetes cluster: {}. It may have been manually deleted or modified.", kubernetesCluster.getName());
}
}
try {
removePortForwardingRules(publicIp, network, owner, CLUSTER_NODES_DEFAULT_START_SSH_PORT, CLUSTER_NODES_DEFAULT_START_SSH_PORT + clusterVMIds.size() - 1);
} catch (ResourceUnavailableException e) {
Expand All @@ -184,6 +192,10 @@ private void scaleKubernetesClusterVpcTierRules(final List<Long> clusterVMIds) t
} catch (ResourceUnavailableException | NetworkRuleConflictException e) {
throw new ManagementServerException(String.format("Failed to activate SSH port forwarding rules for the Kubernetes cluster : %s", kubernetesCluster.getName()), e);
}
if (vpcFirewallEnabled) {
// After the port forwarding rules, as when the cluster is created
addFirewallRulesForNodes(publicIp, clusterVMIds.size());

@adestis-mf adestis-mf Nov 10, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, the scenario is valid: the imported node's single-port SSH rule overlaps the rebuilt
range, and detectRulesConflict rejects it (same protocol and source CIDR).

Reading the code, isolated networks on main behave the same way: the add-node path creates the
same per-node rule
(KubernetesClusterAddWorker.java#L229),
and the isolated scale path also revokes a single rule before recreating the range
(KubernetesClusterScaleWorker.java#L135-L149).
This PR mirrors that handling for VPC tiers, so the case is not new, it just becomes reachable on
VPC tiers with the Firewall service.

I'd prefer to keep this PR to the missing rules and fix the import-then-scale case for both
network types in #14673, since it also touches removeSshFirewallRule, which
#14165 is changing. If you'd rather have it here, I'll add it to this PR.

}
}

/**
Expand Down
8 changes: 8 additions & 0 deletions ...rc/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterStartWorker.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -691,6 +691,14 @@ protected void setupKubernetesEtcdNetworkRules(List<UserVm> etcdVms, Network net
throw new ManagementServerException(String.format("Failed to provision firewall rules for etcd nodes for the Kubernetes cluster : %s", kubernetesCluster.getName()), e);
}
provisionPublicIpPortForwardingRule(publicIp, network, owner, etcdVmIds.get(i), etcdStartPort, DEFAULT_SSH_PORT);
if (isVpcFirewallEnabled(network)) {
// In a VPC the firewall rule follows the port forwarding rule, see setupKubernetesClusterVpcTierRules
try {
provisionFirewallRules(publicIp, owner, etcdStartPort, etcdStartPort);
} catch (NoSuchFieldException | IllegalAccessException | ResourceUnavailableException | NetworkRuleConflictException e) {
throw new ManagementServerException(String.format("Failed to provision firewall rules for etcd nodes for the Kubernetes cluster : %s", kubernetesCluster.getName()), e);
}
}
}
}

Expand Down
Loading