NSX: remove unsupported VPN capability from NAT VPC offering

engine/schema/src/main/java/com/cloud/upgrade/DatabaseUpgradeChecker.java

@@ -94,6 +94,7 @@
 import com.cloud.upgrade.dao.Upgrade42040to42100;
 import com.cloud.upgrade.dao.Upgrade42100to42200;
 import com.cloud.upgrade.dao.Upgrade42200to42210;
+import com.cloud.upgrade.dao.Upgrade42210to42220;
 import com.cloud.upgrade.dao.Upgrade420to421;
 import com.cloud.upgrade.dao.Upgrade421to430;
 import com.cloud.upgrade.dao.Upgrade430to440;
@@ -246,6 +247,7 @@ public DatabaseUpgradeChecker() {
                 .next("4.20.4.0", new Upgrade42040to42100())
                 .next("4.21.0.0", new Upgrade42100to42200())
                 .next("4.22.0.0", new Upgrade42200to42210())
+                .next("4.22.1.0", new Upgrade42210to42220())
                 .build();
     }
 

engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade42210to42220.java

@@ -0,0 +1,85 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package com.cloud.upgrade.dao;
+
+import java.io.InputStream;
+import java.sql.Connection;
+import java.sql.PreparedStatement;
+import java.sql.SQLException;
+
+import com.cloud.network.Network;
+import com.cloud.network.vpc.VpcOffering;
+import com.cloud.utils.exception.CloudRuntimeException;
+
+public class Upgrade42210to42220 extends DbUpgradeAbstractImpl implements DbUpgrade, DbUpgradeSystemVmTemplate {
+
+    static final String DELETE_VPC_SERVICE_MAPPING = "DELETE FROM cloud.vpc_service_map "
+            + "WHERE vpc_id IN (SELECT v.id FROM cloud.vpc v "
+            + "INNER JOIN cloud.vpc_offerings vo ON vo.id = v.vpc_offering_id WHERE vo.unique_name = ?) "
+            + "AND service = ? AND provider = ?";
+
+    static final String DELETE_VPC_OFFERING_SERVICE_MAPPING = "DELETE FROM cloud.vpc_offering_service_map "
+            + "WHERE vpc_offering_id IN (SELECT vo.id FROM cloud.vpc_offerings vo WHERE vo.unique_name = ?) "
+            + "AND service = ? AND provider = ?";
+
+    @Override
+    public String[] getUpgradableVersionRange() {
+        return new String[] {"4.22.1.0", "4.22.2.0"};
+    }
+
+    @Override
+    public String getUpgradedVersion() {
+        return "4.22.2.0";
+    }
+
+    @Override
+    public InputStream[] getPrepareScripts() {
+        return new InputStream[0];
+    }
+
+    @Override
+    public void performDataMigration(Connection conn) {
+        removeUnsupportedNsxVpnServiceMappings(conn);
+    }
+
+    @Override
+    public InputStream[] getCleanupScripts() {
+        return new InputStream[0];
+    }
+
+    protected void removeUnsupportedNsxVpnServiceMappings(Connection conn) {
+        try (PreparedStatement deleteVpcMapping = conn.prepareStatement(DELETE_VPC_SERVICE_MAPPING);
+             PreparedStatement deleteOfferingMapping = conn.prepareStatement(DELETE_VPC_OFFERING_SERVICE_MAPPING)) {
+            setNsxVpnMappingParameters(deleteVpcMapping);
+            int vpcMappingsRemoved = deleteVpcMapping.executeUpdate();
+
+            setNsxVpnMappingParameters(deleteOfferingMapping);
+            int offeringMappingsRemoved = deleteOfferingMapping.executeUpdate();
+
+            logger.info("Removed {} VPC and {} VPC offering unsupported Vpn/Nsx service mappings",
+                    vpcMappingsRemoved, offeringMappingsRemoved);
+        } catch (SQLException e) {
+            throw new CloudRuntimeException("Failed to remove unsupported Vpn/Nsx service mappings from the default NSX NAT VPC offering", e);
+        }
+    }
+
+    private void setNsxVpnMappingParameters(PreparedStatement statement) throws SQLException {
+        statement.setString(1, VpcOffering.DEFAULT_VPC_NAT_NSX_OFFERING_NAME);
+        statement.setString(2, Network.Service.Vpn.getName());
+        statement.setString(3, Network.Provider.Nsx.getName());
+    }
+}

engine/schema/src/test/java/com/cloud/upgrade/DatabaseUpgradeCheckerTest.java

@@ -47,6 +47,7 @@
 import com.cloud.upgrade.dao.Upgrade42020to42030;
 import com.cloud.upgrade.dao.Upgrade42030to42040;
 import com.cloud.upgrade.dao.Upgrade42040to42100;
+import com.cloud.upgrade.dao.Upgrade42210to42220;
 import com.cloud.upgrade.dao.Upgrade452to453;
 import com.cloud.upgrade.dao.Upgrade453to460;
 import com.cloud.upgrade.dao.Upgrade460to461;
@@ -422,4 +423,19 @@ public void testCalculateUpgradePath42010to42100() {
         assertTrue(upgrades[2] instanceof Upgrade42040to42100);
         assertEquals(currentVersion.toString(), upgrades[2].getUpgradedVersion());
     }
+
+    @Test
+    public void testCalculateUpgradePath42210to42220() {
+        final CloudStackVersion dbVersion = CloudStackVersion.parse("4.22.1.0");
+        final CloudStackVersion currentVersion = CloudStackVersion.parse("4.22.2.0");
+
+        final DatabaseUpgradeChecker checker = new DatabaseUpgradeChecker();
+        final DbUpgrade[] upgrades = checker.calculateUpgradePath(dbVersion, currentVersion);
+
+        assertNotNull(upgrades);
+        assertEquals(1, upgrades.length);
+        assertTrue(upgrades[0] instanceof Upgrade42210to42220);
+        assertArrayEquals(new String[] {"4.22.1.0", "4.22.2.0"}, upgrades[0].getUpgradableVersionRange());
+        assertEquals(currentVersion.toString(), upgrades[0].getUpgradedVersion());
+    }
 }

engine/schema/src/test/java/com/cloud/upgrade/dao/Upgrade42210to42220Test.java

@@ -0,0 +1,108 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package com.cloud.upgrade.dao;
+
+import java.sql.Connection;
+import java.sql.PreparedStatement;
+import java.sql.SQLException;
+
+import com.cloud.network.Network;
+import com.cloud.network.vpc.VpcOffering;
+import com.cloud.utils.exception.CloudRuntimeException;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.InOrder;
+import org.mockito.Mock;
+import org.mockito.junit.MockitoJUnitRunner;
+
+import static org.junit.Assert.assertArrayEquals;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThrows;
+import static org.mockito.Mockito.inOrder;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+@RunWith(MockitoJUnitRunner.class)
+public class Upgrade42210to42220Test {
+
+    @Mock
+    private Connection connection;
+    @Mock
+    private PreparedStatement deleteVpcMapping;
+    @Mock
+    private PreparedStatement deleteOfferingMapping;
+
+    private Upgrade42210to42220 upgrade;
+
+    @Before
+    public void setUp() throws SQLException {
+        upgrade = new Upgrade42210to42220();
+        when(connection.prepareStatement(Upgrade42210to42220.DELETE_VPC_SERVICE_MAPPING)).thenReturn(deleteVpcMapping);
+        when(connection.prepareStatement(Upgrade42210to42220.DELETE_VPC_OFFERING_SERVICE_MAPPING)).thenReturn(deleteOfferingMapping);
+    }
+
+    @Test
+    public void testVersionRange() {
+        assertArrayEquals(new String[] {"4.22.1.0", "4.22.2.0"}, upgrade.getUpgradableVersionRange());
+        assertEquals("4.22.2.0", upgrade.getUpgradedVersion());
+        assertEquals(0, upgrade.getPrepareScripts().length);
+        assertEquals(0, upgrade.getCleanupScripts().length);
+    }
+
+    @Test
+    public void testPerformDataMigrationRemovesOnlySeededNsxVpnMappings() throws SQLException {
+        when(deleteVpcMapping.executeUpdate()).thenReturn(2);
+        when(deleteOfferingMapping.executeUpdate()).thenReturn(1);
+
+        upgrade.performDataMigration(connection);
+
+        verifyParameters(deleteVpcMapping);
+        verifyParameters(deleteOfferingMapping);
+        InOrder executionOrder = inOrder(deleteVpcMapping, deleteOfferingMapping);
+        executionOrder.verify(deleteVpcMapping).executeUpdate();
+        executionOrder.verify(deleteOfferingMapping).executeUpdate();
+    }
+
+    @Test
+    public void testPerformDataMigrationIsSafeWhenMappingsAreAlreadyAbsent() throws SQLException {
+        when(deleteVpcMapping.executeUpdate()).thenReturn(0);
+        when(deleteOfferingMapping.executeUpdate()).thenReturn(0);
+
+        upgrade.performDataMigration(connection);
+
+        verify(deleteVpcMapping).executeUpdate();
+        verify(deleteOfferingMapping).executeUpdate();
+    }
+
+    @Test
+    public void testPerformDataMigrationFailsUpgradeOnDatabaseError() throws SQLException {
+        SQLException cause = new SQLException("database failure");
+        when(deleteVpcMapping.executeUpdate()).thenThrow(cause);
+
+        CloudRuntimeException exception = assertThrows(CloudRuntimeException.class,
+                () -> upgrade.performDataMigration(connection));
+
+        assertEquals(cause, exception.getCause());
+    }
+
+    private void verifyParameters(PreparedStatement statement) throws SQLException {
+        verify(statement).setString(1, VpcOffering.DEFAULT_VPC_NAT_NSX_OFFERING_NAME);
+        verify(statement).setString(2, Network.Service.Vpn.getName());
+        verify(statement).setString(3, Network.Provider.Nsx.getName());
+    }
+}

server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java

@@ -434,16 +434,7 @@ public void doInTransactionWithoutResult(final TransactionStatus status) {
                 // configure default vpc offering with NSX as network service provider in NAT mode
                 if (_vpcOffDao.findByUniqueName(VpcOffering.DEFAULT_VPC_NAT_NSX_OFFERING_NAME) == null) {
                     logger.debug("Creating default VPC offering with NSX as network service provider" + VpcOffering.DEFAULT_VPC_NAT_NSX_OFFERING_NAME);
-                    final Map<Service, Set<Provider>> svcProviderMap = new HashMap<Service, Set<Provider>>();
-                    final Set<Provider> defaultProviders = Set.of(Provider.Nsx);
-                    for (final Service svc : getSupportedServices()) {
-                        if (List.of(Service.UserData, Service.Dhcp, Service.Dns).contains(svc)) {
-                            final Set<Provider> userDataProvider = Set.of(Provider.VPCVirtualRouter);
-                            svcProviderMap.put(svc, userDataProvider);
-                        } else {
-                            svcProviderMap.put(svc, defaultProviders);
-                        }
-                    }
+                    final Map<Service, Set<Provider>> svcProviderMap = getDefaultVpcNatNsxServiceProviderMap();
                     createVpcOffering(VpcOffering.DEFAULT_VPC_NAT_NSX_OFFERING_NAME, VpcOffering.DEFAULT_VPC_NAT_NSX_OFFERING_NAME, svcProviderMap, false,
                             State.Enabled, null, false, false, false, NetworkOffering.NetworkMode.NATTED, null, false);
 
@@ -1974,6 +1965,20 @@ protected List<Service> getSupportedServices() {
         return services;
     }
 
+    Map<Service, Set<Provider>> getDefaultVpcNatNsxServiceProviderMap() {
+        final Map<Service, Set<Provider>> serviceProviderMap = new HashMap<>();
+        final Set<Provider> nsxProvider = Set.of(Provider.Nsx);
+        final Set<Provider> virtualRouterProvider = Set.of(Provider.VPCVirtualRouter);
+        for (final Service service : getSupportedServices()) {
+            if (List.of(Service.UserData, Service.Dhcp, Service.Dns).contains(service)) {
+                serviceProviderMap.put(service, virtualRouterProvider);
+            } else if (service != Service.Vpn) {
+                serviceProviderMap.put(service, nsxProvider);
+            }
+        }
+        return serviceProviderMap;
+    }
+
     @Override
     public boolean startVpc(final long vpcId, final boolean destroyOnFailure) throws ConcurrentOperationException, ResourceUnavailableException, InsufficientCapacityException {
         final CallContext ctx = CallContext.current();

server/src/main/java/com/cloud/network/vpn/Site2SiteVpnManagerImpl.java

@@ -149,6 +149,10 @@ public Site2SiteVpnGateway createVpnGateway(CreateVpnGatewayCmd cmd) {
             throw new InvalidParameterValueException(String.format("The VPN gateway of VPC %s already existed!", vpc));
         }
 
+        if (!vpcManager.isProviderSupportServiceInVpc(vpcId, Network.Service.Vpn, Network.Provider.VPCVirtualRouter)) {
+            throw new InvalidParameterValueException(String.format("VPC %s does not support Site-to-Site VPN through the VPC virtual router", vpc));
+        }
+
         IPAddressVO requestedIp = _ipAddressDao.findById(cmd.getIpAddressId());
         IPAddressVO ipAddress = getIpAddressIdForVpn(vpcId, vpc.getVpcOfferingId(), requestedIp);
         Site2SiteVpnGatewayVO gw = new Site2SiteVpnGatewayVO(owner.getAccountId(), owner.getDomainId(), ipAddress.getId(), vpcId);

server/src/test/java/com/cloud/network/vpc/VpcManagerImplTest.java

@@ -98,6 +98,7 @@
 import java.util.UUID;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyBoolean;
@@ -257,6 +258,23 @@ public void getVpcOffSvcProvidersMapForEmptyServiceTest() {
         assertEquals(map.size(), 1);
     }
 
+    @Test
+    public void testDefaultVpcNatNsxServiceProviderMapDoesNotAdvertiseUnsupportedVpn() {
+        Map<Service, Set<Provider>> serviceProviderMap = manager.getDefaultVpcNatNsxServiceProviderMap();
+
+        assertEquals(manager.getSupportedServices().size() - 1, serviceProviderMap.size());
+        assertFalse(serviceProviderMap.containsKey(Service.Vpn));
+        for (Service service : manager.getSupportedServices()) {
+            if (service == Service.Vpn) {
+                continue;
+            }
+            Set<Provider> expectedProviders = List.of(Service.UserData, Service.Dhcp, Service.Dns).contains(service)
+                    ? Set.of(Provider.VPCVirtualRouter)
+                    : Set.of(Provider.Nsx);
+            assertEquals(expectedProviders, serviceProviderMap.get(service));
+        }
+    }
+
     protected Map<String, String> createFakeCapabilityInputMap() {
         Map<String, String> map = new HashMap<String, String>();
         map.put(VpcManagerImpl.CAPABILITYVALUE, VpcManagerImpl.TRUE_VALUE);