Support Firewall for public IPs in VPC

api/src/main/java/com/cloud/network/rules/FirewallRule.java

@@ -69,7 +69,9 @@ enum TrafficType {
 
     State getState();
 
-    long getNetworkId();
+    Long getNetworkId();
+
+    Long getVpcId();
 
     Long getSourceIpAddressId();
 

api/src/main/java/org/apache/cloudstack/api/command/user/firewall/CreateEgressFirewallRuleCmd.java

@@ -212,7 +212,7 @@ public State getState() {
     }
 
     @Override
-    public long getNetworkId() {
+    public Long getNetworkId() {
         return networkId;
     }
 

api/src/main/java/org/apache/cloudstack/api/command/user/firewall/CreateFirewallRuleCmd.java

@@ -223,13 +223,9 @@ public State getState() {
     }
 
     @Override
-    public long getNetworkId() {
-        IpAddress ip = _entityMgr.findById(IpAddress.class, getIpAddressId());
-        Long ntwkId = null;
-
-        if (ip.getAssociatedWithNetworkId() != null) {
-            ntwkId = ip.getAssociatedWithNetworkId();
-        }
+    public Long getNetworkId() {
+        IpAddress ip = getIp();
+        Long ntwkId = isVpcIp(ip) ? getVpcNetworkIdForFirewallRule(ip) : getIsolatedNetworkIdForFirewallRule(ip);
 
         if (ntwkId == null) {
             throw new InvalidParameterValueException("Unable to create firewall rule for the IP address ID=" + ipAddressId +
@@ -238,6 +234,12 @@ public long getNetworkId() {
         return ntwkId;
     }
 
+    @Override
+    public Long getVpcId() {
+        IpAddress ip = getIp();
+        return isVpcIp(ip) ? ip.getVpcId() : null;
+    }
+
     @Override
     public long getEntityOwnerId() {
         Account account = CallContext.current().getCallingAccount();
@@ -300,7 +302,21 @@ public String getSyncObjType() {
 
     @Override
     public Long getSyncObjId() {
-        return getIp().getAssociatedWithNetworkId();
+        Long syncObjId = getIp().getAssociatedWithNetworkId();
+        return syncObjId != null ? syncObjId : getNetworkId();
+    }
+
+    private boolean isVpcIp(IpAddress ip) {
+        return ip.getVpcId() != null;
+    }
+
+    private Long getIsolatedNetworkIdForFirewallRule(IpAddress ip) {
+        return ip.getAssociatedWithNetworkId();
+    }
+
+    private Long getVpcNetworkIdForFirewallRule(IpAddress ip) {
+        // VPC flow is independent from tier association; manager resolves execution network.
+        return ip.getNetworkId();
     }
 
     private IpAddress getIp() {
@@ -311,6 +327,7 @@ private IpAddress getIp() {
         return ip;
     }
 
+
     @Override
     public Integer getIcmpCode() {
         if (icmpCode != null) {

api/src/main/java/org/apache/cloudstack/api/command/user/firewall/CreatePortForwardingRuleCmd.java

@@ -176,7 +176,7 @@ public Boolean getOpenFirewall() {
         }
     }
 
-    private Long getVpcId() {
+    public Long getVpcId() {
         if (ipAddressId != null) {
             IpAddress ipAddr = _networkService.getIp(ipAddressId);
             if (ipAddr == null || !ipAddr.readyToUse()) {
@@ -275,7 +275,7 @@ public State getState() {
     }
 
     @Override
-    public long getNetworkId() {
+    public Long getNetworkId() {
         IpAddress ip = _entityMgr.findById(IpAddress.class, getIpAddressId());
         Long ntwkId = _networkService.getPreferredNetworkIdForPublicIpRuleAssignment(ip, networkId);
         if (ntwkId == null) {

api/src/main/java/org/apache/cloudstack/api/command/user/nat/CreateIpForwardingRuleCmd.java

@@ -229,8 +229,13 @@ public FirewallRule.State getState() {
     }
 
     @Override
-    public long getNetworkId() {
-        return -1;
+    public Long getNetworkId() {
+        return -1L;
+    }
+
+    @Override
+    public Long getVpcId() {
+        return null;
     }
 
     @Override

api/src/test/java/org/apache/cloudstack/api/command/user/firewall/CreateFirewallRuleCmdTest.java

@@ -21,10 +21,15 @@
 import java.util.Collections;
 import java.util.List;
 
+import com.cloud.network.IpAddress;
+import com.cloud.network.NetworkService;
+import com.cloud.utils.db.EntityManager;
 import org.apache.commons.collections.CollectionUtils;
 import org.junit.Assert;
 import org.junit.Test;
 import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.Mockito;
 import org.mockito.junit.MockitoJUnitRunner;
 import org.springframework.test.util.ReflectionTestUtils;
 
@@ -33,6 +38,12 @@
 @RunWith(MockitoJUnitRunner.class)
 public class CreateFirewallRuleCmdTest {
 
+    @Mock
+    private EntityManager entityManager;
+
+    @Mock
+    private NetworkService networkService;
+
     private void validateAllIp4Cidr(final CreateFirewallRuleCmd cmd) {
         Assert.assertTrue(CollectionUtils.isNotEmpty(cmd.getSourceCidrList()));
         Assert.assertEquals(1, cmd.getSourceCidrList().size());
@@ -88,4 +99,22 @@ public void testGetSourceCidrList_EmptyFirstElementButMore() {
         Assert.assertEquals(2, cmd.getSourceCidrList().size());
         Assert.assertEquals(cidr, cmd.getSourceCidrList().get(1));
     }
+
+    @Test
+    public void testGetNetworkIdVpcWithoutAssociatedNetworkUsesVpcFallbackAndSyncObjId() {
+        final CreateFirewallRuleCmd cmd = new CreateFirewallRuleCmd();
+        final IpAddress ip = Mockito.mock(IpAddress.class);
+
+        cmd._entityMgr = entityManager;
+        cmd._networkService = networkService;
+        ReflectionTestUtils.setField(cmd, "ipAddressId", 42L);
+
+        Mockito.when(networkService.getIp(42L)).thenReturn(ip);
+        Mockito.when(ip.getAssociatedWithNetworkId()).thenReturn(null);
+        Mockito.when(ip.getVpcId()).thenReturn(100L);
+        Mockito.when(ip.getNetworkId()).thenReturn(2L);
+
+        Assert.assertEquals(Long.valueOf(2L), cmd.getNetworkId());
+        Assert.assertEquals(Long.valueOf(2L), cmd.getSyncObjId());
+    }
 }

engine/components-api/src/main/java/com/cloud/network/rules/StaticNatRuleImpl.java

@@ -80,10 +80,15 @@ public long getDomainId() {
     }
 
     @Override
-    public long getNetworkId() {
+    public Long getNetworkId() {
         return networkId;
     }
 
+    @Override
+    public Long getVpcId() {
+        return null;
+    }
+
     @Override
     public long getId() {
         return id;

engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java

@@ -561,6 +561,7 @@ public boolean configure(final String name, final Map<String, Object> params) th
         defaultVPCOffProviders.put(Service.StaticNat, defaultProviders);
         defaultVPCOffProviders.put(Service.PortForwarding, defaultProviders);
         defaultVPCOffProviders.put(Service.Vpn, defaultProviders);
+        defaultVPCOffProviders.put(Service.Firewall, defaultProviders);
 
         Transaction.execute(new TransactionCallbackNoReturn() {
             @Override

engine/schema/src/main/java/com/cloud/network/rules/FirewallRuleVO.java

@@ -85,6 +85,9 @@ public class FirewallRuleVO implements FirewallRule {
     @Column(name = "network_id")
     Long networkId;
 
+    @Column(name = "vpc_id")
+    Long vpcId;
+
     @Column(name = "icmp_code")
     Integer icmpCode;
 
@@ -190,10 +193,18 @@ public State getState() {
     }
 
     @Override
-    public long getNetworkId() {
+    public Long getNetworkId() {
         return networkId;
     }
 
+    public Long getVpcId() {
+        return vpcId;
+    }
+
+    public void setVpcId(Long vpcId) {
+        this.vpcId = vpcId;
+    }
+
     @Override
     public FirewallRuleType getType() {
         return type;
@@ -207,7 +218,7 @@ protected FirewallRuleVO() {
         uuid = UUID.randomUUID().toString();
     }
 
-    public FirewallRuleVO(String xId, Long ipAddressId, Integer portStart, Integer portEnd, String protocol, long networkId, long accountId, long domainId,
+    public FirewallRuleVO(String xId, Long ipAddressId, Integer portStart, Integer portEnd, String protocol, Long networkId, long accountId, long domainId,
             Purpose purpose, List<String> sourceCidrs, Integer icmpCode, Integer icmpType, Long related, TrafficType trafficType) {
         this.xId = xId;
         if (xId == null) {
@@ -251,7 +262,7 @@ public FirewallRuleVO(String xId, long ipAddressId, int port, String protocol, l
     }
 
 
-    public FirewallRuleVO(String xId, Long ipAddressId, Integer portStart, Integer portEnd, String protocol, long networkId, long accountId, long domainId,
+    public FirewallRuleVO(String xId, Long ipAddressId, Integer portStart, Integer portEnd, String protocol, Long networkId, long accountId, long domainId,
                           Purpose purpose, List<String> sourceCidrs, List<String> destCidrs, Integer icmpCode, Integer icmpType, Long related, TrafficType trafficType) {
         this(xId,ipAddressId, portStart, portEnd, protocol, networkId, accountId, domainId, purpose, sourceCidrs, icmpCode, icmpType, related, trafficType);
         this.destinationCidrs = destCidrs;

plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImpl.java

@@ -2986,9 +2986,8 @@ private void createNetworkOfferingForKubernetes(String offeringName, String offe
         defaultKubernetesServiceNetworkOfferingProviders.put(Service.UserData, provider);
         if (forVpc) {
             defaultKubernetesServiceNetworkOfferingProviders.put(Service.NetworkACL, forNsx ? Network.Provider.Nsx : provider);
-        } else {
-            defaultKubernetesServiceNetworkOfferingProviders.put(Service.Firewall, forNsx ? Network.Provider.Nsx : provider);
         }
+        defaultKubernetesServiceNetworkOfferingProviders.put(Service.Firewall, forNsx ? Network.Provider.Nsx : provider);
         defaultKubernetesServiceNetworkOfferingProviders.put(Service.Lb, forNsx ? Network.Provider.Nsx : provider);
         defaultKubernetesServiceNetworkOfferingProviders.put(Service.SourceNat, forNsx ? Network.Provider.Nsx : provider);
         defaultKubernetesServiceNetworkOfferingProviders.put(Service.StaticNat, forNsx ? Network.Provider.Nsx : provider);

plugins/integrations/kubernetes-service/src/test/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImplTest.java

@@ -155,7 +155,7 @@ public void validateIsolatedNetworkIpRulesNoRules() {
     }
 
     private FirewallRuleVO createRule(int startPort, int endPort) {
-        FirewallRuleVO rule = new FirewallRuleVO(null, null, startPort, endPort, "tcp", 1, 1, 1, FirewallRule.Purpose.Firewall, List.of("0.0.0.0/0"), null, null, null, FirewallRule.TrafficType.Ingress);
+        FirewallRuleVO rule = new FirewallRuleVO(null, null, startPort, endPort, "tcp", 1L, 1, 1, FirewallRule.Purpose.Firewall, List.of("0.0.0.0/0"), null, null, null, FirewallRule.TrafficType.Ingress);
         return rule;
     }
 

plugins/network-elements/palo-alto/src/test/java/com/cloud/network/resource/PaloAltoResourceTest.java

@@ -290,7 +290,7 @@ public void addEgressFirewallRule() throws ConfigurationException, Exception {
         List<FirewallRuleTO> rules = new ArrayList<FirewallRuleTO>();
         List<String> cidrList = new ArrayList<String>();
         cidrList.add("0.0.0.0/0");
-        FirewallRuleVO activeVO = new FirewallRuleVO(null, null, 80, 80, "tcp", 1, 1, 1, Purpose.Firewall, cidrList, null, null, null, FirewallRule.TrafficType.Egress);
+        FirewallRuleVO activeVO = new FirewallRuleVO(null, null, 80, 80, "tcp", 1L, 1, 1, Purpose.Firewall, cidrList, null, null, null, FirewallRule.TrafficType.Egress);
         FirewallRuleTO active = new FirewallRuleTO(activeVO, Long.toString(vlanId), null, Purpose.Firewall, FirewallRule.TrafficType.Egress);
         rules.add(active);
 
@@ -319,7 +319,7 @@ public void removeEgressFirewallRule() throws ConfigurationException, Exception
 
         long vlanId = 3954;
         List<FirewallRuleTO> rules = new ArrayList<FirewallRuleTO>();
-        FirewallRuleVO revokedVO = new FirewallRuleVO(null, null, 80, 80, "tcp", 1, 1, 1, Purpose.Firewall, null, null, null, null, FirewallRule.TrafficType.Egress);
+        FirewallRuleVO revokedVO = new FirewallRuleVO(null, null, 80, 80, "tcp", 1L, 1, 1, Purpose.Firewall, null, null, null, null, FirewallRule.TrafficType.Egress);
         revokedVO.setState(State.Revoke);
         FirewallRuleTO revoked = new FirewallRuleTO(revokedVO, Long.toString(vlanId), null, Purpose.Firewall, FirewallRule.TrafficType.Egress);
         rules.add(revoked);

server/src/main/java/com/cloud/api/ApiResponseHelper.java

@@ -2952,8 +2952,13 @@ public FirewallResponse createFirewallResponse(FirewallRule fwRule) {
             }
         }
 
-        Network network = ApiDBUtils.findNetworkById(fwRule.getNetworkId());
-        response.setNetworkId(network.getUuid());
+        Long networkId = fwRule.getNetworkId();
+        if (networkId != null) {
+            Network network = ApiDBUtils.findNetworkById(networkId);
+            if (network != null) {
+                response.setNetworkId(network.getUuid());
+            }
+        }
 
         FirewallRule.State state = fwRule.getState();
         String stateToSet = state.toString();

server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java

@@ -7223,10 +7223,12 @@ public NetworkOffering createNetworkOffering(final NetworkOfferingBaseCmd cmd) {
             }
 
             if (forVpc == null) {
-                if (service == Service.SecurityGroup || service == Service.Firewall) {
+                if (service == Service.SecurityGroup) {
                     forVpc = false;
                 } else if (service == Service.NetworkACL) {
                     forVpc = true;
+                } else if (service == Service.Firewall) {
+                    forVpc = true;
                 }
             }
 

server/src/main/java/com/cloud/network/IpAddressManagerImpl.java

@@ -656,28 +656,60 @@ public boolean applyRules(List<? extends FirewallRule> rules, FirewallRule.Purpo
         }
 
         boolean success = true;
-        Network network = _networksDao.findById(rules.get(0).getNetworkId());
-        FirewallRuleVO.TrafficType trafficType = rules.get(0).getTrafficType();
+        FirewallRule firstRule = rules.get(0);
+        Long networkId = firstRule.getNetworkId();
+        Long vpcId = firstRule.getVpcId();
+        FirewallRuleVO.TrafficType trafficType = firstRule.getTrafficType();
         List<PublicIp> publicIps = new ArrayList<PublicIp>();
 
-        if (!(rules.get(0).getPurpose() == FirewallRule.Purpose.Firewall && trafficType == FirewallRule.TrafficType.Egress)) {
-            // get the list of public ip's owned by the network
-            List<IPAddressVO> userIps = _ipAddressDao.listByAssociatedNetwork(network.getId(), null);
-            if (userIps != null && !userIps.isEmpty()) {
-                for (IPAddressVO userIp : userIps) {
-                    PublicIp publicIp = PublicIp.createFromAddrAndVlan(userIp, _vlanDao.findById(userIp.getVlanId()));
-                    publicIps.add(publicIp);
+        // For VPC firewall rules the networkId on the rule is null; resolve via VPC.
+        Network network;
+        if (networkId != null) {
+            network = _networksDao.findById(networkId);
+        } else if (vpcId != null) {
+            List<? extends Network> vpcNetworks = _vpcMgr.getVpcNetworks(vpcId);
+            network = (vpcNetworks != null && !vpcNetworks.isEmpty()) ? _networksDao.findById(vpcNetworks.get(0).getId()) : null;
+        } else {
+            network = null;
+        }
+
+        if (network == null) {
+            logger.warn("Unable to resolve network for firewall rules (networkId={}, vpcId={}); skipping IP association", networkId, vpcId);
+        } else if (!(firstRule.getPurpose() == FirewallRule.Purpose.Firewall && trafficType == FirewallRule.TrafficType.Egress)) {
+            // For VPC ingress rules, collect public IPs tied to the VPC rather than network association
+            if (vpcId != null && networkId == null) {
+                List<IPAddressVO> vpcIps = _ipAddressDao.listByAssociatedVpc(vpcId, null);
+                if (vpcIps != null) {
+                    for (IPAddressVO userIp : vpcIps) {
+                        PublicIp publicIp = PublicIp.createFromAddrAndVlan(userIp, _vlanDao.findById(userIp.getVlanId()));
+                        publicIps.add(publicIp);
+                    }
+                }
+            } else {
+                // get the list of public ip's owned by the network
+                List<IPAddressVO> userIps = _ipAddressDao.listByAssociatedNetwork(network.getId(), null);
+                if (userIps != null && !userIps.isEmpty()) {
+                    for (IPAddressVO userIp : userIps) {
+                        PublicIp publicIp = PublicIp.createFromAddrAndVlan(userIp, _vlanDao.findById(userIp.getVlanId()));
+                        publicIps.add(publicIp);
+                    }
                 }
             }
         }
-        // rules can not programmed unless IP is associated with network service provider, so run IP assoication for
+
+        // rules can not programmed unless IP is associated with network service provider, so run IP association for
         // the network so as to ensure IP is associated before applying rules (in add state)
-        if (checkIfIpAssocRequired(network, false, publicIps)) {
+        if (network != null && checkIfIpAssocRequired(network, false, publicIps)) {
             applyIpAssociations(network, false, continueOnError, publicIps);
         }
 
         try {
-            applier.applyRules(network, purpose, rules);
+            if (network != null) {
+                applier.applyRules(network, purpose, rules);
+            } else {
+                logger.warn("Skipping applyRules: no network resolved for rules (vpcId={})", vpcId);
+                success = false;
+            }
         } catch (ResourceUnavailableException e) {
             if (!continueOnError) {
                 throw e;
@@ -688,7 +720,7 @@ public boolean applyRules(List<? extends FirewallRule> rules, FirewallRule.Purpo
 
         // if there are no active rules associated with a public IP, then public IP need not be associated with a provider.
         // This IPAssoc ensures, public IP is dis-associated after last active rule is revoked.
-        if (checkIfIpAssocRequired(network, true, publicIps)) {
+        if (network != null && checkIfIpAssocRequired(network, true, publicIps)) {
             applyIpAssociations(network, true, continueOnError, publicIps);
         }