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/main/java/org/apache/cloudstack/api/response/FirewallResponse.java

@@ -51,6 +51,10 @@ public class FirewallResponse extends BaseResponse {
     @Param(description = "The Network ID of the firewall rule")
     private String networkId;
 
+    @SerializedName(ApiConstants.VPC_ID)
+    @Param(description = "The VPC ID of the firewall rule")
+    private String vpcId;
+
     @SerializedName(ApiConstants.IP_ADDRESS)
     @Param(description = "The public IP address for the firewall rule")
     private String publicIpAddress;
@@ -115,6 +119,10 @@ public void setNetworkId(String networkId) {
         this.networkId = networkId;
     }
 
+    public void setVpcId(String vpcId) {
+        this.vpcId = vpcId;
+    }
+
     public void setState(String state) {
         this.state = state;
     }

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;

engine/schema/src/main/resources/META-INF/db/schema-42210to42300.sql

@@ -202,3 +202,6 @@ CREATE TABLE IF NOT EXISTS `cloud`.`image_transfer`(
     CONSTRAINT `fk_image_transfer__host_id` FOREIGN KEY (`host_id`) REFERENCES `host`(`id`) ON DELETE CASCADE,
     INDEX `i_image_transfer__backup_id`(`backup_id`)
     ) ENGINE=InnoDB DEFAULT CHARSET=utf8;
+
+-- This is part of allowing firewall rules on public IP addresses in VPC network
+ALTER TABLE `cloud`.`firewall_rules` MODIFY COLUMN `network_id` BIGINT UNSIGNED NULL;

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

@@ -2957,8 +2957,21 @@ 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());
+            }
+        }
+
+        Long vpcId = fwRule.getVpcId();
+        if (vpcId != null) {
+            Vpc vpc = ApiDBUtils.findVpcById(vpcId);
+            if (vpc != null) {
+                response.setVpcId(vpc.getUuid());
+            }
+        }
 
         FirewallRule.State state = fwRule.getState();
         String stateToSet = state.toString();
@@ -5405,8 +5418,21 @@ public FirewallResponse createIpv6FirewallRuleResponse(FirewallRule fwRule) {
         response.setIcmpCode(fwRule.getIcmpCode());
         response.setIcmpType(fwRule.getIcmpType());
 
-        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());
+            }
+        }
+
+        Long vpcId = fwRule.getVpcId();
+        if (vpcId != null) {
+            Vpc vpc = ApiDBUtils.findVpcById(vpcId);
+            if (vpc != null) {
+                response.setVpcId(vpc.getUuid());
+            }
+        }
 
         FirewallRule.State state = fwRule.getState();
         String stateToSet = state.toString();

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

@@ -7255,10 +7255,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;
                 }
             }