diff --git a/PendingReleaseNotes b/PendingReleaseNotes index c9afb25f7ed7..70bde7a3f859 100644 --- a/PendingReleaseNotes +++ b/PendingReleaseNotes @@ -55,6 +55,11 @@ example.ver.1 > example.ver.2: continue to snapshot live. 4.23.0.0 > 24.0.0: + * NSX network offerings can reference existing IP discovery, MAC discovery, + and segment security profiles. New NSX segments bind selected profiles in + the same hierarchical request as segment creation. Existing networks are + not silently reconfigured when profile bindings change. + * VMware-to-KVM import using VDDK can import powered-off VMware VMs directly into Ceph RBD primary storage when forceconverttopool is enabled and the selected KVM conversion host supports qemu RBD access and in-place virt-v2v diff --git a/api/src/main/java/com/cloud/offering/NetworkOffering.java b/api/src/main/java/com/cloud/offering/NetworkOffering.java index 159c1e2fe319..9e042886e757 100644 --- a/api/src/main/java/com/cloud/offering/NetworkOffering.java +++ b/api/src/main/java/com/cloud/offering/NetworkOffering.java @@ -40,7 +40,22 @@ public enum State { } public enum Detail { - InternalLbProvider, PublicLbProvider, servicepackageuuid, servicepackagedescription, PromiscuousMode, MacAddressChanges, ForgedTransmits, MacLearning, RelatedNetworkOffering, domainid, zoneid, pvlanType, internetProtocol + InternalLbProvider, + PublicLbProvider, + servicepackageuuid, + servicepackagedescription, + PromiscuousMode, + MacAddressChanges, + ForgedTransmits, + MacLearning, + NsxIpDiscoveryProfileId, + NsxMacDiscoveryProfileId, + NsxSegmentSecurityProfileId, + RelatedNetworkOffering, + domainid, + zoneid, + pvlanType, + internetProtocol } public enum NetworkMode { diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/network/CloneNetworkOfferingCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/network/CloneNetworkOfferingCmd.java index 19760ffaaa10..7dc5be95bd31 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/network/CloneNetworkOfferingCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/network/CloneNetworkOfferingCmd.java @@ -16,7 +16,9 @@ // under the License. package org.apache.cloudstack.api.command.admin.network; +import java.util.Collections; import java.util.List; +import java.util.Map; import org.apache.cloudstack.api.APICommand; import org.apache.cloudstack.api.ApiConstants; @@ -62,6 +64,13 @@ public class CloneNetworkOfferingCmd extends NetworkOfferingBaseCmd { "If specified along with 'supportedservices', this parameter is ignored.") private List dropServices; + @Parameter(name = ApiConstants.CLEAN_UP_DETAILS, + type = CommandType.BOOLEAN, + description = "Whether to omit network offering details from the clone. If true, source details are not copied " + + "and the details parameter is ignored. If false or omitted, details are inherited unless explicitly supplied.", + since = "24.0") + private Boolean cleanupDetails; + @Parameter(name = ApiConstants.TRAFFIC_TYPE, type = CommandType.STRING, description = "The traffic type for the network offering. Supported type in current release is GUEST only") @@ -87,6 +96,15 @@ public List getDropServices() { return dropServices; } + public boolean isCleanupDetails() { + return Boolean.TRUE.equals(cleanupDetails); + } + + @Override + public Map getDetails() { + return isCleanupDetails() ? Collections.emptyMap() : super.getDetails(); + } + public String getGuestIpType() { return guestIptype; } diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/network/NetworkOfferingBaseCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/network/NetworkOfferingBaseCmd.java index 4b4d0dc94825..27c8a1662feb 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/network/NetworkOfferingBaseCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/network/NetworkOfferingBaseCmd.java @@ -40,7 +40,6 @@ import java.util.List; import java.util.Map; import java.util.Set; -import java.util.stream.Collectors; import static com.cloud.network.Network.Service.Dhcp; import static com.cloud.network.Network.Service.Dns; @@ -167,7 +166,9 @@ public abstract class NetworkOfferingBaseCmd extends BaseCmd { @Parameter(name = ApiConstants.DETAILS, type = CommandType.MAP, since = "4.2.0", description = "Network offering details in key/value pairs." + " Supported keys are internallbprovider/publiclbprovider with service provider as a value, and" - + " promiscuousmode/macaddresschanges/forgedtransmits with true/false as value to accept/reject the security settings if available for a nic/portgroup") + + " promiscuousmode/macaddresschanges/forgedtransmits with true/false as value to accept/reject the security settings if available for a nic/portgroup." + + " NSX offerings also support nsxipdiscoveryprofileid, nsxmacdiscoveryprofileid, and nsxsegmentsecurityprofileid." + + " Values are IDs of existing operator-managed NSX profiles to bind to segments created from the offering.") protected Map details; @Parameter(name = ApiConstants.EGRESS_DEFAULT_POLICY, @@ -382,18 +383,18 @@ private void getServiceProviderMapForExternalProvider(Map> VirtualRouterProvider.Type.VirtualRouter.name(); List unsupportedServices = new ArrayList<>(List.of("Vpn", "Gateway", "SecurityGroup", "Connectivity", "BaremetalPxeService")); List routerSupported = List.of("Dhcp", "Dns", "UserData"); - List allServices = Network.Service.listAllServices().stream().map(Network.Service::getName).collect(Collectors.toList()); if (routerProvider.equals(VirtualRouterProvider.Type.VPCVirtualRouter.name())) { unsupportedServices.add("Firewall"); } else { unsupportedServices.add("NetworkACL"); } - for (String service : allServices) { + for (String service : getSupportedServices()) { if (unsupportedServices.contains(service)) continue; if (routerSupported.contains(service)) serviceProviderMap.put(service, List.of(routerProvider)); - else if (NetworkOffering.NetworkMode.NATTED.name().equalsIgnoreCase(getNetworkMode()) || NetworkACL.getName().equalsIgnoreCase(service)) { + else if (NetworkOffering.NetworkMode.NATTED.name().equalsIgnoreCase(getNetworkMode()) || NetworkACL.getName().equalsIgnoreCase(service) + || (isForNsx() && Firewall.getName().equalsIgnoreCase(service))) { serviceProviderMap.put(service, List.of(provider)); } if (isNsxWithoutLb(getProvider(), getNsxSupportsLbService()) || isNetrisRouted(getProvider(), getNetworkMode())) { diff --git a/api/src/test/java/org/apache/cloudstack/api/command/admin/network/CloneNetworkOfferingCmdTest.java b/api/src/test/java/org/apache/cloudstack/api/command/admin/network/CloneNetworkOfferingCmdTest.java index 096395b1359e..27e6bad675a4 100644 --- a/api/src/test/java/org/apache/cloudstack/api/command/admin/network/CloneNetworkOfferingCmdTest.java +++ b/api/src/test/java/org/apache/cloudstack/api/command/admin/network/CloneNetworkOfferingCmdTest.java @@ -35,8 +35,10 @@ import java.util.Map; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.when; @@ -86,6 +88,43 @@ public void testGetDropServices() { assertEquals(dropServices, cloneNetworkOfferingCmd.getDropServices()); } + @Test + public void testDetailsAreInheritedWhenCleanupIsOmittedOrFalse() { + Map sourceDetails = Map.of("NsxIpDiscoveryProfileId", "source-ip-profile"); + ReflectionTestUtils.setField(cloneNetworkOfferingCmd, "sourceDetailsMap", sourceDetails); + + assertFalse(cloneNetworkOfferingCmd.isCleanupDetails()); + assertEquals(sourceDetails, cloneNetworkOfferingCmd.getDetails()); + + ReflectionTestUtils.setField(cloneNetworkOfferingCmd, "cleanupDetails", false); + assertEquals(sourceDetails, cloneNetworkOfferingCmd.getDetails()); + } + + @Test + public void testSuppliedDetailsReplaceSourceDetails() { + Map sourceDetails = Map.of("NsxIpDiscoveryProfileId", "source-ip-profile", + "NsxMacDiscoveryProfileId", "source-mac-profile"); + Map requestedDetails = Map.of("nsxipdiscoveryprofileid", "replacement-ip-profile"); + ReflectionTestUtils.setField(cloneNetworkOfferingCmd, "sourceDetailsMap", sourceDetails); + ReflectionTestUtils.setField(cloneNetworkOfferingCmd, "details", Map.of("0", requestedDetails)); + + assertEquals(requestedDetails, cloneNetworkOfferingCmd.getDetails()); + } + + @Test + public void testCleanupDetailsOmitsInheritedAndSuppliedDetails() { + Map sourceDetails = Map.of("NsxIpDiscoveryProfileId", "source-ip-profile"); + ReflectionTestUtils.setField(cloneNetworkOfferingCmd, "sourceDetailsMap", sourceDetails); + ReflectionTestUtils.setField(cloneNetworkOfferingCmd, "cleanupDetails", true); + + assertTrue(cloneNetworkOfferingCmd.getDetails().isEmpty()); + + ReflectionTestUtils.setField(cloneNetworkOfferingCmd, "details", + Map.of("0", Map.of("nsxipdiscoveryprofileid", "replacement-ip-profile"))); + assertTrue(cloneNetworkOfferingCmd.getDetails().isEmpty()); + assertEquals("source-ip-profile", sourceDetails.get("NsxIpDiscoveryProfileId")); + } + @Test public void testGetGuestIpType() { String guestIpType = "Isolated"; diff --git a/api/src/test/java/org/apache/cloudstack/api/command/admin/offering/CreateNetworkOfferingCmdTest.java b/api/src/test/java/org/apache/cloudstack/api/command/admin/offering/CreateNetworkOfferingCmdTest.java index ef10ebff4678..b86331a15512 100644 --- a/api/src/test/java/org/apache/cloudstack/api/command/admin/offering/CreateNetworkOfferingCmdTest.java +++ b/api/src/test/java/org/apache/cloudstack/api/command/admin/offering/CreateNetworkOfferingCmdTest.java @@ -17,6 +17,9 @@ package org.apache.cloudstack.api.command.admin.offering; +import java.util.List; +import java.util.Map; + import org.apache.cloudstack.api.command.admin.network.CreateNetworkOfferingCmd; import org.junit.Assert; import org.junit.Test; @@ -36,4 +39,57 @@ public void createVpcNtwkOffWithEmptyDisplayText() { ReflectionTestUtils.setField(createNetworkOfferingCmd, "networkOfferingName", netName); Assert.assertEquals(createNetworkOfferingCmd.getDisplayText(), netName); } + + @Test + public void nsxNattedNetworkUsesNsxFirewallAndNatServices() { + assertServiceProviders("NSX", "NATTED", false, Map.of( + "Dhcp", List.of("VirtualRouter"), "Dns", List.of("VirtualRouter"), "UserData", List.of("VirtualRouter"), + "Firewall", List.of("Nsx"), "SourceNat", List.of("Nsx"), "StaticNat", List.of("Nsx"), "PortForwarding", List.of("Nsx"))); + } + + @Test + public void nsxNattedVpcTierUsesNsxAclAndNatServices() { + assertServiceProviders("NSX", "NATTED", true, Map.of( + "Dhcp", List.of("VPCVirtualRouter"), "Dns", List.of("VPCVirtualRouter"), "UserData", List.of("VPCVirtualRouter"), + "NetworkACL", List.of("Nsx"), "SourceNat", List.of("Nsx"), "StaticNat", List.of("Nsx"), "PortForwarding", List.of("Nsx"))); + } + + @Test + public void nsxRoutedNetworkUsesNsxFirewallWithoutNatServices() { + assertServiceProviders("NSX", "ROUTED", false, Map.of( + "Dhcp", List.of("VirtualRouter"), "Dns", List.of("VirtualRouter"), "UserData", List.of("VirtualRouter"), + "Firewall", List.of("Nsx"))); + } + + @Test + public void nsxRoutedVpcTierUsesNsxAclWithoutNatServices() { + assertServiceProviders("NSX", "ROUTED", true, Map.of( + "Dhcp", List.of("VPCVirtualRouter"), "Dns", List.of("VPCVirtualRouter"), "UserData", List.of("VPCVirtualRouter"), + "NetworkACL", List.of("Nsx"))); + } + + @Test + public void netrisNattedNetworkKeepsNetrisNatAndLoadBalancerProviders() { + assertServiceProviders("Netris", "NATTED", false, Map.of( + "Dhcp", List.of("VirtualRouter"), "Dns", List.of("VirtualRouter"), "UserData", List.of("VirtualRouter"), + "Firewall", List.of("Netris"), "SourceNat", List.of("Netris"), "StaticNat", List.of("Netris"), + "PortForwarding", List.of("Netris"), "Lb", List.of("Netris"), "Vpn", List.of("VirtualRouter"))); + } + + @Test + public void netrisRoutedNetworkKeepsOnlyRouterProviders() { + assertServiceProviders("Netris", "ROUTED", false, Map.of( + "Dhcp", List.of("VirtualRouter"), "Dns", List.of("VirtualRouter"), "UserData", List.of("VirtualRouter"))); + } + + private void assertServiceProviders(String provider, String networkMode, boolean forVpc, Map> expected) { + ReflectionTestUtils.setField(createNetworkOfferingCmd, "provider", provider); + ReflectionTestUtils.setField(createNetworkOfferingCmd, "networkMode", networkMode); + ReflectionTestUtils.setField(createNetworkOfferingCmd, "forVpc", forVpc); + + Map> providers = createNetworkOfferingCmd.getServiceProviders(); + + Assert.assertEquals(expected, providers); + Assert.assertTrue(createNetworkOfferingCmd.getSupportedServices().containsAll(providers.keySet())); + } } diff --git a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/CreateNsxSegmentCommand.java b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/CreateNsxSegmentCommand.java index b4b86bd640a6..e03fb1751190 100644 --- a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/CreateNsxSegmentCommand.java +++ b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/CreateNsxSegmentCommand.java @@ -26,10 +26,21 @@ public class CreateNsxSegmentCommand extends NsxCommand { private String networkName; private String networkGateway; private String networkCidr; + private String ipDiscoveryProfileId; + private String macDiscoveryProfileId; + private String segmentSecurityProfileId; public CreateNsxSegmentCommand(long domainId, long accountId, long zoneId, Long vpcId, String vpcName, long networkId, String networkName, String networkGateway, String networkCidr) { + this(domainId, accountId, zoneId, vpcId, vpcName, networkId, networkName, networkGateway, + networkCidr, null, null, null); + } + + public CreateNsxSegmentCommand(long domainId, long accountId, long zoneId, + Long vpcId, String vpcName, long networkId, String networkName, + String networkGateway, String networkCidr, String ipDiscoveryProfileId, + String macDiscoveryProfileId, String segmentSecurityProfileId) { super(domainId, accountId, zoneId); this.vpcId = vpcId; this.vpcName = vpcName; @@ -37,6 +48,9 @@ public CreateNsxSegmentCommand(long domainId, long accountId, long zoneId, this.networkName = networkName; this.networkGateway = networkGateway; this.networkCidr = networkCidr; + this.ipDiscoveryProfileId = ipDiscoveryProfileId; + this.macDiscoveryProfileId = macDiscoveryProfileId; + this.segmentSecurityProfileId = segmentSecurityProfileId; } public Long getVpcId() { @@ -63,6 +77,18 @@ public String getNetworkCidr() { return networkCidr; } + public String getIpDiscoveryProfileId() { + return ipDiscoveryProfileId; + } + + public String getMacDiscoveryProfileId() { + return macDiscoveryProfileId; + } + + public String getSegmentSecurityProfileId() { + return segmentSecurityProfileId; + } + @Override public boolean equals(Object o) { if (this == o) return true; diff --git a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/resource/NsxResource.java b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/resource/NsxResource.java index 78a9363a5e49..c14e9483c801 100644 --- a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/resource/NsxResource.java +++ b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/resource/NsxResource.java @@ -351,7 +351,8 @@ private Answer executeRequest(CreateNsxSegmentCommand cmd) { boolean isResourceVpc = !Objects.isNull(cmd.getVpcId()); String tier1GatewayName = NsxControllerUtils.getTier1GatewayName(cmd.getDomainId(), cmd.getAccountId(), cmd.getZoneId(), networkResourceId, isResourceVpc); - nsxApiClient.createSegment(segmentName, tier1GatewayName, gatewayAddress, enforcementPointPath, transportZones); + nsxApiClient.createSegment(segmentName, tier1GatewayName, gatewayAddress, enforcementPointPath, transportZones, + cmd.getIpDiscoveryProfileId(), cmd.getMacDiscoveryProfileId(), cmd.getSegmentSecurityProfileId()); nsxApiClient.createGroupForSegment(segmentName); } catch (Exception e) { logger.error(String.format("Failed to create network: %s", cmd.getNetworkName())); diff --git a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxApiClient.java b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxApiClient.java index 4d78f2a0ab26..0197ba88f4ee 100644 --- a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxApiClient.java +++ b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxApiClient.java @@ -25,30 +25,38 @@ import com.vmware.nsx.model.ControllerClusterStatus; import com.vmware.nsx.model.TransportZone; import com.vmware.nsx.model.TransportZoneListResult; +import com.vmware.nsx_policy.Infra; import com.vmware.nsx_policy.infra.DhcpRelayConfigs; +import com.vmware.nsx_policy.infra.IpDiscoveryProfiles; import com.vmware.nsx_policy.infra.LbAppProfiles; import com.vmware.nsx_policy.infra.LbMonitorProfiles; import com.vmware.nsx_policy.infra.LbPools; import com.vmware.nsx_policy.infra.LbServices; import com.vmware.nsx_policy.infra.LbVirtualServers; +import com.vmware.nsx_policy.infra.MacDiscoveryProfiles; +import com.vmware.nsx_policy.infra.SegmentSecurityProfiles; import com.vmware.nsx_policy.infra.Segments; import com.vmware.nsx_policy.infra.Services; import com.vmware.nsx_policy.infra.Sites; import com.vmware.nsx_policy.infra.Tier1s; import com.vmware.nsx_policy.infra.domains.Groups; import com.vmware.nsx_policy.infra.domains.SecurityPolicies; -import com.vmware.nsx_policy.infra.domains.groups.members.SegmentPorts; import com.vmware.nsx_policy.infra.domains.security_policies.Rules; +import com.vmware.nsx_policy.infra.segments.Ports; import com.vmware.nsx_policy.infra.sites.EnforcementPoints; import com.vmware.nsx_policy.infra.tier_0s.LocaleServices; import com.vmware.nsx_policy.infra.tier_1s.nat.NatRules; import com.vmware.nsx_policy.model.ApiError; +import com.vmware.nsx_policy.model.ChildSegment; +import com.vmware.nsx_policy.model.ChildSegmentDiscoveryProfileBindingMap; +import com.vmware.nsx_policy.model.ChildSegmentSecurityProfileBindingMap; import com.vmware.nsx_policy.model.DhcpRelayConfig; import com.vmware.nsx_policy.model.EnforcementPoint; import com.vmware.nsx_policy.model.EnforcementPointListResult; import com.vmware.nsx_policy.model.Group; import com.vmware.nsx_policy.model.GroupListResult; import com.vmware.nsx_policy.model.ICMPTypeServiceEntry; +import com.vmware.nsx_policy.model.IPDiscoveryProfile; import com.vmware.nsx_policy.model.L4PortSetServiceEntry; import com.vmware.nsx_policy.model.LBAppProfileListResult; import com.vmware.nsx_policy.model.LBIcmpMonitorProfile; @@ -61,14 +69,18 @@ import com.vmware.nsx_policy.model.LBVirtualServer; import com.vmware.nsx_policy.model.LBVirtualServerListResult; import com.vmware.nsx_policy.model.LocaleServicesListResult; +import com.vmware.nsx_policy.model.MacDiscoveryProfile; import com.vmware.nsx_policy.model.PathExpression; -import com.vmware.nsx_policy.model.PolicyGroupMembersListResult; import com.vmware.nsx_policy.model.PolicyNatRule; import com.vmware.nsx_policy.model.PolicyNatRuleListResult; -import com.vmware.nsx_policy.model.PolicyGroupMemberDetails; import com.vmware.nsx_policy.model.Rule; import com.vmware.nsx_policy.model.SecurityPolicy; import com.vmware.nsx_policy.model.Segment; +import com.vmware.nsx_policy.model.SegmentDiscoveryProfileBindingMap; +import com.vmware.nsx_policy.model.SegmentPort; +import com.vmware.nsx_policy.model.SegmentPortListResult; +import com.vmware.nsx_policy.model.SegmentSecurityProfile; +import com.vmware.nsx_policy.model.SegmentSecurityProfileBindingMap; import com.vmware.nsx_policy.model.SegmentSubnet; import com.vmware.nsx_policy.model.ServiceListResult; import com.vmware.nsx_policy.model.Site; @@ -90,9 +102,10 @@ import org.apache.cloudstack.resource.NsxNetworkRule; import org.apache.cloudstack.utils.NsxControllerUtils; import org.apache.commons.collections.CollectionUtils; +import org.apache.commons.lang3.BooleanUtils; +import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -import org.apache.commons.lang3.BooleanUtils; import java.util.ArrayList; import java.util.List; @@ -128,6 +141,11 @@ public class NsxApiClient { private static final String TIER_1_RESOURCE_TYPE = "Tier1"; private static final String TIER_1_LOCALE_SERVICE_ID = "default"; private static final String SEGMENT_RESOURCE_TYPE = "Segment"; + private static final String INFRA_RESOURCE_TYPE = "Infra"; + private static final String SEGMENT_DISCOVERY_PROFILE_BINDING_RESOURCE_TYPE = "SegmentDiscoveryProfileBindingMap"; + private static final String SEGMENT_SECURITY_PROFILE_BINDING_RESOURCE_TYPE = "SegmentSecurityProfileBindingMap"; + private static final String SEGMENT_DISCOVERY_PROFILE_BINDING_ID = "cloudstack-discovery-profile-binding"; + private static final String SEGMENT_SECURITY_PROFILE_BINDING_ID = "cloudstack-security-profile-binding"; private static final String TIER_0_GATEWAY_PATH_PREFIX = "/infra/tier-0s/"; private static final String TIER_1_GATEWAY_PATH_PREFIX = "/infra/tier-1s/"; protected static final String SEGMENTS_PATH = "/infra/segments"; @@ -466,8 +484,16 @@ public TransportZoneListResult getTransportZones() { public void createSegment(String segmentName, String tier1GatewayName, String gatewayAddress, String enforcementPointPath, List transportZones) { + createSegment(segmentName, tier1GatewayName, gatewayAddress, enforcementPointPath, transportZones, null, null, null); + } + + public void createSegment(String segmentName, String tier1GatewayName, String gatewayAddress, String enforcementPointPath, + List transportZones, String ipDiscoveryProfileId, String macDiscoveryProfileId, + String segmentSecurityProfileId) { try { - Segments segmentService = (Segments) nsxService.apply(Segments.class); + String ipDiscoveryProfilePath = getIpDiscoveryProfilePath(ipDiscoveryProfileId); + String macDiscoveryProfilePath = getMacDiscoveryProfilePath(macDiscoveryProfileId); + String segmentSecurityProfilePath = getSegmentSecurityProfilePath(segmentSecurityProfileId); SegmentSubnet subnet = new SegmentSubnet.Builder() .setGatewayAddress(gatewayAddress) .build(); @@ -480,7 +506,24 @@ public void createSegment(String segmentName, String tier1GatewayName, String ga .setSubnets(List.of(subnet)) .setTransportZonePath(enforcementPointPath + "/transport-zones/" + transportZones.get(0).getId()) .build(); - segmentService.patch(segmentName, segment); + List profileBindings = getSegmentProfileBindings(ipDiscoveryProfilePath, macDiscoveryProfilePath, + segmentSecurityProfilePath); + if (profileBindings.isEmpty()) { + Segments segmentService = (Segments) nsxService.apply(Segments.class); + segmentService.patch(segmentName, segment); + } else { + segment.setChildren(profileBindings); + ChildSegment childSegment = new ChildSegment.Builder() + .setId(segmentName) + .setSegment(segment) + .build(); + com.vmware.nsx_policy.model.Infra infra = new com.vmware.nsx_policy.model.Infra.Builder() + .setResourceType(INFRA_RESOURCE_TYPE) + .setChildren(List.of(childSegment)) + .build(); + Infra infraService = (Infra) nsxService.apply(Infra.class); + infraService.patch(infra, false); + } } catch (Error error) { ApiError ae = error.getData()._convertTo(ApiError.class); String msg = String.format("Error creating segment %s: %s", segmentName, ae.getErrorMessage()); @@ -489,18 +532,96 @@ public void createSegment(String segmentName, String tier1GatewayName, String ga } } + protected String getIpDiscoveryProfilePath(String profileId) { + if (StringUtils.isBlank(profileId)) { + return null; + } + IpDiscoveryProfiles profiles = (IpDiscoveryProfiles) nsxService.apply(IpDiscoveryProfiles.class); + IPDiscoveryProfile profile = profiles.get(profileId); + return validateProfile(profileId, profile.getId(), profile.getPath(), "/infra/ip-discovery-profiles/", profile.getMarkedForDelete()); + } + + protected String getMacDiscoveryProfilePath(String profileId) { + if (StringUtils.isBlank(profileId)) { + return null; + } + MacDiscoveryProfiles profiles = (MacDiscoveryProfiles) nsxService.apply(MacDiscoveryProfiles.class); + MacDiscoveryProfile profile = profiles.get(profileId); + return validateProfile(profileId, profile.getId(), profile.getPath(), "/infra/mac-discovery-profiles/", profile.getMarkedForDelete()); + } + + protected String getSegmentSecurityProfilePath(String profileId) { + if (StringUtils.isBlank(profileId)) { + return null; + } + SegmentSecurityProfiles profiles = (SegmentSecurityProfiles) nsxService.apply(SegmentSecurityProfiles.class); + SegmentSecurityProfile profile = profiles.get(profileId); + return validateProfile(profileId, profile.getId(), profile.getPath(), "/infra/segment-security-profiles/", profile.getMarkedForDelete()); + } + + protected String validateProfile(String requestedId, String resolvedId, String profilePath, + String expectedPathPrefix, Boolean markedForDelete) { + if (!Objects.equals(requestedId, resolvedId)) { + throw new CloudRuntimeException(String.format("NSX returned profile %s while resolving requested profile %s", resolvedId, requestedId)); + } + if (!Objects.equals(expectedPathPrefix + requestedId, profilePath)) { + throw new CloudRuntimeException(String.format("NSX profile %s did not return a canonical resource path of the expected type", requestedId)); + } + if (Boolean.TRUE.equals(markedForDelete)) { + throw new CloudRuntimeException(String.format("NSX profile %s is marked for deletion", requestedId)); + } + return profilePath; + } + + protected List getSegmentProfileBindings(String ipDiscoveryProfilePath, String macDiscoveryProfilePath, + String segmentSecurityProfilePath) { + List bindings = new ArrayList<>(); + if (StringUtils.isNotBlank(ipDiscoveryProfilePath) || StringUtils.isNotBlank(macDiscoveryProfilePath)) { + SegmentDiscoveryProfileBindingMap binding = new SegmentDiscoveryProfileBindingMap.Builder() + .setResourceType(SEGMENT_DISCOVERY_PROFILE_BINDING_RESOURCE_TYPE) + .setId(SEGMENT_DISCOVERY_PROFILE_BINDING_ID) + .setIpDiscoveryProfilePath(ipDiscoveryProfilePath) + .setMacDiscoveryProfilePath(macDiscoveryProfilePath) + .build(); + bindings.add(new ChildSegmentDiscoveryProfileBindingMap.Builder() + .setId(SEGMENT_DISCOVERY_PROFILE_BINDING_ID) + .setSegmentDiscoveryProfileBindingMap(binding) + .build()); + } + if (StringUtils.isNotBlank(segmentSecurityProfilePath)) { + SegmentSecurityProfileBindingMap binding = new SegmentSecurityProfileBindingMap.Builder() + .setResourceType(SEGMENT_SECURITY_PROFILE_BINDING_RESOURCE_TYPE) + .setId(SEGMENT_SECURITY_PROFILE_BINDING_ID) + .setSegmentSecurityProfilePath(segmentSecurityProfilePath) + .build(); + bindings.add(new ChildSegmentSecurityProfileBindingMap.Builder() + .setId(SEGMENT_SECURITY_PROFILE_BINDING_ID) + .setSegmentSecurityProfileBindingMap(binding) + .build()); + } + return bindings; + } + public void deleteSegment(long zoneId, long domainId, long accountId, Long vpcId, long networkId, String segmentName) { try { removeSegmentDistributedFirewallRules(segmentName); if (Objects.isNull(vpcId)) { String t1GatewayName = getTier1GatewayName(domainId, accountId, zoneId, networkId, false); - deleteLoadBalancer(getLoadBalancerName(t1GatewayName)); + try { + deleteLoadBalancer(getLoadBalancerName(t1GatewayName)); + } catch (NotFound e) { + logger.debug("Load balancer for segment {} is already absent", segmentName); + } } removeSegment(segmentName, zoneId); DhcpRelayConfigs dhcpRelayConfig = (DhcpRelayConfigs) nsxService.apply(DhcpRelayConfigs.class); String dhcpRelayConfigId = NsxControllerUtils.getNsxDhcpRelayConfigId(zoneId, domainId, accountId, vpcId, networkId); logger.debug("Removing the DHCP relay config with ID {}", dhcpRelayConfigId); - dhcpRelayConfig.delete(dhcpRelayConfigId); + try { + dhcpRelayConfig.delete(dhcpRelayConfigId); + } catch (NotFound e) { + logger.debug("DHCP relay config {} is already absent", dhcpRelayConfigId); + } } catch (Error error) { ApiError ae = error.getData()._convertTo(ApiError.class); String msg = String.format("Error deleting segment %s: %s", segmentName, ae.getErrorMessage()); @@ -519,22 +640,38 @@ protected void removeSegment(String segmentName, long zoneId) { logger.warn(errMsg); return; } - } catch (Exception e) { + } catch (NotFound e) { logger.warn(errMsg); return; } - String siteId = getDefaultSiteId(); - String enforcementPointPath = getDefaultEnforcementPointPath(siteId); - SegmentPorts segmentPortsService = (SegmentPorts) nsxService.apply(SegmentPorts.class); - PolicyGroupMembersListResult segmentPortsList = getSegmentPortList(segmentPortsService, segmentName, enforcementPointPath); + Ports segmentPortsService = (Ports) nsxService.apply(Ports.class); + SegmentPortListResult segmentPortsList = getSegmentPortList(segmentPortsService, segmentName); Long portCount = segmentPortsList.getResultCount(); if (portCount > 0L) { - portCount = retrySegmentDeletion(segmentPortsService, segmentName, enforcementPointPath, zoneId); + portCount = retrySegmentDeletion(segmentPortsService, segmentName, zoneId); } if (portCount == 0L) { logger.debug("Removing the segment with ID {}", segmentName); removeGroupForSegment(segmentName); - segmentService.delete(segmentName); + Segment segment = new Segment.Builder() + .setResourceType(SEGMENT_RESOURCE_TYPE) + .setId(segmentName) + .build(); + ChildSegment childSegment = new ChildSegment.Builder() + .setId(segmentName) + .setSegment(segment) + .setMarkedForDelete(true) + .build(); + com.vmware.nsx_policy.model.Infra infra = new com.vmware.nsx_policy.model.Infra.Builder() + .setResourceType(INFRA_RESOURCE_TYPE) + .setChildren(List.of(childSegment)) + .build(); + Infra infraService = (Infra) nsxService.apply(Infra.class); + try { + infraService.patch(infra, false); + } catch (NotFound e) { + logger.debug("Segment {} is already absent", segmentName); + } } else { String msg = String.format("Cannot remove the NSX segment %s because there are still %s port group(s) attached to it", segmentName, portCount); logger.error(msg); @@ -542,14 +679,13 @@ protected void removeSegment(String segmentName, long zoneId) { } } - private PolicyGroupMembersListResult getSegmentPortList(SegmentPorts segmentPortsService, String segmentName, String enforcementPointPath) { + private SegmentPortListResult getSegmentPortList(Ports segmentPortsService, String segmentName) { return PagedFetcher. - withPageFetcher( - cursor -> segmentPortsService.list(DEFAULT_DOMAIN, segmentName, cursor, enforcementPointPath, - false, null, 50L, false, null) + withPageFetcher( + cursor -> segmentPortsService.list(segmentName, cursor, false, null, 50L, false, null) ) - .cursorExtractor(PolicyGroupMembersListResult::getCursor) - .itemsExtractor(PolicyGroupMembersListResult::getResults) + .cursorExtractor(SegmentPortListResult::getCursor) + .itemsExtractor(SegmentPortListResult::getResults) .itemsSetter((page, allItems) -> { page.setResults(allItems); page.setResultCount((long) allItems.size()); @@ -557,7 +693,7 @@ private PolicyGroupMembersListResult getSegmentPortList(SegmentPorts segmentPort .fetchAll(); } - private Long retrySegmentDeletion(SegmentPorts segmentPortsService, String segmentName, String enforcementPointPath, long zoneId) { + private Long retrySegmentDeletion(Ports segmentPortsService, String segmentName, long zoneId) { int retries = NsxService.NSX_API_FAILURE_RETRIES.valueIn(zoneId); int waitingSecs = NsxService.NSX_API_FAILURE_INTERVAL.valueIn(zoneId); int count = 1; @@ -567,7 +703,7 @@ private Long retrySegmentDeletion(SegmentPorts segmentPortsService, String segme logger.info("Waiting for all port groups to be unlinked from the segment {} - " + "Attempt: {}. Waiting for {} secs", segmentName, count++, waitingSecs); Thread.sleep(waitingSecs * 1000L); - portCount = getSegmentPortList(segmentPortsService, segmentName, enforcementPointPath).getResultCount(); + portCount = getSegmentPortList(segmentPortsService, segmentName).getResultCount(); retries--; } catch (InterruptedException e) { throw new CloudRuntimeException(String.format("Unable to delete segment %s due to: %s", segmentName, e.getLocalizedMessage())); @@ -1101,13 +1237,19 @@ public void createGroupForSegment(String segmentName) { private void removeGroupForSegment(String segmentName) { logger.info(String.format("Removing Group for Segment %s", segmentName)); Groups service = (Groups) nsxService.apply(Groups.class); - service.delete(DEFAULT_DOMAIN, segmentName, true, false); + try { + service.delete(DEFAULT_DOMAIN, segmentName, true, false); + } catch (NotFound e) { + logger.debug("Group for segment {} is already absent", segmentName); + } } private void removeSegmentDistributedFirewallRules(String segmentName) { try { SecurityPolicies services = (SecurityPolicies) nsxService.apply(SecurityPolicies.class); services.delete(DEFAULT_DOMAIN, segmentName); + } catch (NotFound e) { + logger.debug("Distributed firewall policy for segment {} is already absent", segmentName); } catch (Error error) { ApiError ae = error.getData()._convertTo(ApiError.class); String msg = String.format("Failed to remove NSX distributed firewall policy for segment %s, due to: %s", segmentName, ae.getErrorMessage()); diff --git a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxGuestNetworkGuru.java b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxGuestNetworkGuru.java index 0f7865d7d73f..6d1c717e90c4 100644 --- a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxGuestNetworkGuru.java +++ b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxGuestNetworkGuru.java @@ -59,6 +59,7 @@ import javax.inject.Inject; import java.util.List; +import java.util.Map; import java.util.Objects; public class NsxGuestNetworkGuru extends GuestNetworkGuru implements NetworkMigrationResponder { @@ -324,10 +325,19 @@ public void createNsxSegment(NetworkVO networkVO, DataCenter zone) { throw new CloudRuntimeException(msg); } } - CreateNsxSegmentCommand command = NsxHelper.createNsxSegmentCommand(domain, account, zone, vpcName, networkVO); + Map offeringDetails = _networkModel.getNtwkOffDetails(networkVO.getNetworkOfferingId()); + String ipDiscoveryProfile = getOfferingDetail(offeringDetails, NetworkOffering.Detail.NsxIpDiscoveryProfileId); + String macDiscoveryProfile = getOfferingDetail(offeringDetails, NetworkOffering.Detail.NsxMacDiscoveryProfileId); + String segmentSecurityProfile = getOfferingDetail(offeringDetails, NetworkOffering.Detail.NsxSegmentSecurityProfileId); + CreateNsxSegmentCommand command = NsxHelper.createNsxSegmentCommand(domain, account, zone, vpcName, networkVO, + ipDiscoveryProfile, macDiscoveryProfile, segmentSecurityProfile); NsxAnswer answer = nsxControllerUtils.sendNsxCommand(command, zone.getId()); if (!answer.getResult()) { throw new CloudRuntimeException("can not create NSX network"); } } + + protected String getOfferingDetail(Map offeringDetails, NetworkOffering.Detail detail) { + return offeringDetails == null ? null : offeringDetails.get(detail); + } } diff --git a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/utils/NsxHelper.java b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/utils/NsxHelper.java index b0668a0704f9..a4986ab44c69 100644 --- a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/utils/NsxHelper.java +++ b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/utils/NsxHelper.java @@ -41,8 +41,14 @@ public static CreateNsxDhcpRelayConfigCommand createNsxDhcpRelayConfigCommand(Do } public static CreateNsxSegmentCommand createNsxSegmentCommand(DomainVO domain, Account account, DataCenter zone, String vpcName, NetworkVO networkVO) { + return createNsxSegmentCommand(domain, account, zone, vpcName, networkVO, null, null, null); + } + + public static CreateNsxSegmentCommand createNsxSegmentCommand(DomainVO domain, Account account, DataCenter zone, String vpcName, NetworkVO networkVO, + String ipDiscoveryProfileId, String macDiscoveryProfileId, String segmentSecurityProfileId) { return new CreateNsxSegmentCommand(domain.getId(), account.getId(), zone.getId(), - networkVO.getVpcId(), vpcName, networkVO.getId(), networkVO.getName(), networkVO.getGateway(), networkVO.getCidr()); + networkVO.getVpcId(), vpcName, networkVO.getId(), networkVO.getName(), networkVO.getGateway(), networkVO.getCidr(), + ipDiscoveryProfileId, macDiscoveryProfileId, segmentSecurityProfileId); } public static CreateOrUpdateNsxTier1NatRuleCommand createOrUpdateNsxNatRuleCommand(long domainId, long accountId, long zoneId, diff --git a/plugins/network-elements/nsx/src/test/java/org/apache/cloudstack/resource/NsxResourceTest.java b/plugins/network-elements/nsx/src/test/java/org/apache/cloudstack/resource/NsxResourceTest.java index 0d74bb8a3b3d..21cb60fe9c29 100644 --- a/plugins/network-elements/nsx/src/test/java/org/apache/cloudstack/resource/NsxResourceTest.java +++ b/plugins/network-elements/nsx/src/test/java/org/apache/cloudstack/resource/NsxResourceTest.java @@ -60,6 +60,7 @@ import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; @@ -184,6 +185,24 @@ public void testCreateNsxSegment() { assertTrue(answer.getResult()); } + @Test + public void testCreateNsxSegmentPassesProfileIdsToApiClient() { + List transportZoneList = List.of(new TransportZone.Builder().setDisplayName(transportZone).build()); + CreateNsxSegmentCommand command = new CreateNsxSegmentCommand(domainId, accountId, zoneId, + 2L, "VPC01", 3L, "Web", "10.10.10.1", "10.10.10.0/24", + "ip-profile", "mac-profile", "security-profile"); + when(nsxApi.getDefaultSiteId()).thenReturn("site1"); + when(nsxApi.getDefaultEnforcementPointPath("site1")).thenReturn("enforcementPointPath"); + when(nsxApi.getTransportZones()).thenReturn(transportZoneListResult); + when(transportZoneListResult.getResults()).thenReturn(transportZoneList); + + NsxAnswer answer = (NsxAnswer) nsxResource.executeRequest(command); + + assertTrue(answer.getResult()); + verify(nsxApi).createSegment(anyString(), anyString(), anyString(), eq("enforcementPointPath"), + eq(transportZoneList), eq("ip-profile"), eq("mac-profile"), eq("security-profile")); + } + @Test public void testCreateNsxSegmentEmptySites() { when(nsxApi.getDefaultSiteId()).thenReturn(null); diff --git a/plugins/network-elements/nsx/src/test/java/org/apache/cloudstack/service/NsxApiClientTest.java b/plugins/network-elements/nsx/src/test/java/org/apache/cloudstack/service/NsxApiClientTest.java index 5f8d771f75df..6099419338f7 100644 --- a/plugins/network-elements/nsx/src/test/java/org/apache/cloudstack/service/NsxApiClientTest.java +++ b/plugins/network-elements/nsx/src/test/java/org/apache/cloudstack/service/NsxApiClientTest.java @@ -22,14 +22,27 @@ import com.vmware.nsx.cluster.Status; import com.vmware.nsx.model.ClusterStatus; import com.vmware.nsx.model.ControllerClusterStatus; +import com.vmware.nsx_policy.Infra; +import com.vmware.nsx_policy.InfraStub; +import com.vmware.nsx_policy.infra.DhcpRelayConfigs; +import com.vmware.nsx_policy.infra.IpDiscoveryProfiles; import com.vmware.nsx_policy.infra.LbAppProfiles; import com.vmware.nsx_policy.infra.LbMonitorProfiles; import com.vmware.nsx_policy.infra.LbPools; import com.vmware.nsx_policy.infra.LbServices; import com.vmware.nsx_policy.infra.LbVirtualServers; +import com.vmware.nsx_policy.infra.MacDiscoveryProfiles; +import com.vmware.nsx_policy.infra.SegmentSecurityProfiles; +import com.vmware.nsx_policy.infra.Segments; import com.vmware.nsx_policy.infra.domains.Groups; +import com.vmware.nsx_policy.infra.domains.SecurityPolicies; +import com.vmware.nsx_policy.infra.segments.Ports; import com.vmware.nsx_policy.model.ApiError; +import com.vmware.nsx_policy.model.ChildSegment; +import com.vmware.nsx_policy.model.ChildSegmentDiscoveryProfileBindingMap; +import com.vmware.nsx_policy.model.ChildSegmentSecurityProfileBindingMap; import com.vmware.nsx_policy.model.Group; +import com.vmware.nsx_policy.model.IPDiscoveryProfile; import com.vmware.nsx_policy.model.LBAppProfileListResult; import com.vmware.nsx_policy.model.LBIcmpMonitorProfile; import com.vmware.nsx_policy.model.LBService; @@ -37,20 +50,35 @@ import com.vmware.nsx_policy.model.LBPool; import com.vmware.nsx_policy.model.LBPoolMember; import com.vmware.nsx_policy.model.LBVirtualServer; +import com.vmware.nsx_policy.model.MacDiscoveryProfile; import com.vmware.nsx_policy.model.PathExpression; +import com.vmware.nsx_policy.model.Segment; +import com.vmware.nsx_policy.model.SegmentDiscoveryProfileBindingMap; +import com.vmware.nsx_policy.model.SegmentPort; +import com.vmware.nsx_policy.model.SegmentPortListResult; +import com.vmware.nsx_policy.model.SegmentSecurityProfile; import com.vmware.vapi.bindings.Service; +import com.vmware.vapi.bindings.StubConfiguration; import com.vmware.vapi.bindings.Structure; +import com.vmware.vapi.core.ApiProvider; +import com.vmware.vapi.core.AsyncHandle; +import com.vmware.vapi.core.MethodResult; +import com.vmware.vapi.data.DataValue; import com.vmware.vapi.std.errors.Error; import com.vmware.vapi.std.errors.NotFound; +import org.apache.cloudstack.framework.config.ConfigKey; +import org.apache.cloudstack.framework.config.impl.ConfigDepotImpl; import org.apache.cloudstack.resource.NsxLoadBalancerMember; import org.apache.cloudstack.utils.NsxControllerUtils; import org.junit.Assert; import org.junit.Before; import org.junit.Test; +import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.MockedConstruction; import org.mockito.Mockito; import org.mockito.MockitoAnnotations; +import org.springframework.test.util.ReflectionTestUtils; import java.util.List; import java.util.function.Function; @@ -68,11 +96,23 @@ public class NsxApiClientTest { private static final String TIER_1_GATEWAY_NAME = "t1"; + private static final String SEGMENT_NAME = "segment"; + private static final String ENFORCEMENT_POINT_PATH = "/infra/sites/default/enforcement-points/default"; @Mock private Function, Service> nsxService; @Mock private Groups groupService; + @Mock + private Segments segmentService; + @Mock + private Infra infraService; + @Mock + private SecurityPolicies securityPolicies; + @Mock + private DhcpRelayConfigs dhcpRelayConfigs; + @Mock + private Ports segmentPorts; private NsxApiClient client = new NsxApiClient(); @@ -102,6 +142,507 @@ public void testCreateGroupForSegment() { } } + @Test + public void testCreateSegmentBindsConfiguredProfiles() { + ApiProvider apiProvider = Mockito.mock(ApiProvider.class); + Infra infraService = Mockito.spy(new InfraStub(apiProvider, new StubConfiguration())); + IpDiscoveryProfiles ipProfiles = Mockito.mock(IpDiscoveryProfiles.class); + MacDiscoveryProfiles macProfiles = Mockito.mock(MacDiscoveryProfiles.class); + SegmentSecurityProfiles securityProfiles = Mockito.mock(SegmentSecurityProfiles.class); + IPDiscoveryProfile ipProfile = Mockito.mock(IPDiscoveryProfile.class); + MacDiscoveryProfile macProfile = Mockito.mock(MacDiscoveryProfile.class); + SegmentSecurityProfile securityProfile = Mockito.mock(SegmentSecurityProfile.class); + when(nsxService.apply(Infra.class)).thenReturn(infraService); + when(nsxService.apply(IpDiscoveryProfiles.class)).thenReturn(ipProfiles); + when(nsxService.apply(MacDiscoveryProfiles.class)).thenReturn(macProfiles); + when(nsxService.apply(SegmentSecurityProfiles.class)).thenReturn(securityProfiles); + when(ipProfiles.get("ip-profile")).thenReturn(ipProfile); + when(macProfiles.get("mac-profile")).thenReturn(macProfile); + when(securityProfiles.get("security-profile")).thenReturn(securityProfile); + when(ipProfile.getId()).thenReturn("ip-profile"); + when(macProfile.getId()).thenReturn("mac-profile"); + when(securityProfile.getId()).thenReturn("security-profile"); + when(ipProfile.getPath()).thenReturn("/infra/ip-discovery-profiles/ip-profile"); + when(macProfile.getPath()).thenReturn("/infra/mac-discovery-profiles/mac-profile"); + when(securityProfile.getPath()).thenReturn("/infra/segment-security-profiles/security-profile"); + Mockito.doAnswer(invocation -> { + AsyncHandle asyncHandle = invocation.getArgument(4); + asyncHandle.setResult(MethodResult.EMPTY); + return null; + }).when(apiProvider).invoke(anyString(), eq("patch"), any(DataValue.class), any(), any()); + ArgumentCaptor infraCaptor = + ArgumentCaptor.forClass(com.vmware.nsx_policy.model.Infra.class); + + client.createSegment("segment", "tier1", "10.10.10.1/24", "/infra/sites/default/enforcement-points/default", + List.of(new com.vmware.nsx.model.TransportZone.Builder().setId("tz").build()), + "ip-profile", "mac-profile", "security-profile"); + + verify(infraService).patch(infraCaptor.capture(), eq(false)); + verify(apiProvider).invoke(anyString(), eq("patch"), any(DataValue.class), any(), any()); + verify(nsxService, never()).apply(Segments.class); + Assert.assertEquals("Infra", infraCaptor.getValue().getResourceType()); + Assert.assertEquals(1, infraCaptor.getValue().getChildren().size()); + ChildSegment childSegment = (ChildSegment) infraCaptor.getValue().getChildren().get(0); + Assert.assertEquals("ChildSegment", childSegment.getResourceType()); + Assert.assertEquals("segment", childSegment.getId()); + Assert.assertEquals("Segment", childSegment.getSegment().getResourceType()); + Assert.assertEquals("segment", childSegment.getSegment().getId()); + Assert.assertEquals(2, childSegment.getSegment().getChildren().size()); + ChildSegmentDiscoveryProfileBindingMap discoveryChild = (ChildSegmentDiscoveryProfileBindingMap) + childSegment.getSegment().getChildren().get(0); + Assert.assertEquals("ChildSegmentDiscoveryProfileBindingMap", discoveryChild.getResourceType()); + SegmentDiscoveryProfileBindingMap discoveryBinding = discoveryChild.getSegmentDiscoveryProfileBindingMap(); + Assert.assertEquals("cloudstack-discovery-profile-binding", discoveryChild.getId()); + Assert.assertEquals("cloudstack-discovery-profile-binding", discoveryBinding.getId()); + Assert.assertEquals("SegmentDiscoveryProfileBindingMap", discoveryBinding.getResourceType()); + Assert.assertEquals("/infra/ip-discovery-profiles/ip-profile", discoveryBinding.getIpDiscoveryProfilePath()); + Assert.assertEquals("/infra/mac-discovery-profiles/mac-profile", discoveryBinding.getMacDiscoveryProfilePath()); + ChildSegmentSecurityProfileBindingMap securityChild = (ChildSegmentSecurityProfileBindingMap) + childSegment.getSegment().getChildren().get(1); + Assert.assertEquals("cloudstack-security-profile-binding", securityChild.getId()); + Assert.assertEquals("cloudstack-security-profile-binding", + securityChild.getSegmentSecurityProfileBindingMap().getId()); + Assert.assertEquals("ChildSegmentSecurityProfileBindingMap", securityChild.getResourceType()); + Assert.assertEquals("SegmentSecurityProfileBindingMap", + securityChild.getSegmentSecurityProfileBindingMap().getResourceType()); + Assert.assertEquals("/infra/segment-security-profiles/security-profile", + securityChild.getSegmentSecurityProfileBindingMap().getSegmentSecurityProfilePath()); + } + + @Test + public void testSegmentProfileBindingsWithOnlyIpDiscoveryProfile() { + List bindings = client.getSegmentProfileBindings("/infra/ip-discovery-profiles/ip-profile", null, null); + + Assert.assertEquals(1, bindings.size()); + ChildSegmentDiscoveryProfileBindingMap child = (ChildSegmentDiscoveryProfileBindingMap) bindings.get(0); + Assert.assertEquals("ChildSegmentDiscoveryProfileBindingMap", child.getResourceType()); + Assert.assertEquals("SegmentDiscoveryProfileBindingMap", + child.getSegmentDiscoveryProfileBindingMap().getResourceType()); + Assert.assertEquals("/infra/ip-discovery-profiles/ip-profile", + child.getSegmentDiscoveryProfileBindingMap().getIpDiscoveryProfilePath()); + Assert.assertNull(child.getSegmentDiscoveryProfileBindingMap().getMacDiscoveryProfilePath()); + } + + @Test + public void testSegmentProfileBindingsWithOnlyMacDiscoveryProfile() { + List bindings = client.getSegmentProfileBindings(null, "/infra/mac-discovery-profiles/mac-profile", null); + + Assert.assertEquals(1, bindings.size()); + ChildSegmentDiscoveryProfileBindingMap child = (ChildSegmentDiscoveryProfileBindingMap) bindings.get(0); + Assert.assertEquals("ChildSegmentDiscoveryProfileBindingMap", child.getResourceType()); + Assert.assertEquals("SegmentDiscoveryProfileBindingMap", + child.getSegmentDiscoveryProfileBindingMap().getResourceType()); + Assert.assertNull(child.getSegmentDiscoveryProfileBindingMap().getIpDiscoveryProfilePath()); + Assert.assertEquals("/infra/mac-discovery-profiles/mac-profile", + child.getSegmentDiscoveryProfileBindingMap().getMacDiscoveryProfilePath()); + } + + @Test + public void testSegmentProfileBindingsWithOnlySecurityProfile() { + List bindings = client.getSegmentProfileBindings(null, null, + "/infra/segment-security-profiles/security-profile"); + + Assert.assertEquals(1, bindings.size()); + ChildSegmentSecurityProfileBindingMap child = (ChildSegmentSecurityProfileBindingMap) bindings.get(0); + Assert.assertEquals("ChildSegmentSecurityProfileBindingMap", child.getResourceType()); + Assert.assertEquals("SegmentSecurityProfileBindingMap", + child.getSegmentSecurityProfileBindingMap().getResourceType()); + Assert.assertEquals("/infra/segment-security-profiles/security-profile", + child.getSegmentSecurityProfileBindingMap().getSegmentSecurityProfilePath()); + } + + @Test + public void testCreateSegmentWithoutProfilesPreservesExistingBehavior() { + Segments segmentService = Mockito.mock(Segments.class); + when(nsxService.apply(Segments.class)).thenReturn(segmentService); + + client.createSegment("segment", "tier1", "10.10.10.1/24", "/infra/sites/default/enforcement-points/default", + List.of(new com.vmware.nsx.model.TransportZone.Builder().setId("tz").build())); + + verify(segmentService).patch(eq("segment"), any(Segment.class)); + verify(nsxService, never()).apply(IpDiscoveryProfiles.class); + verify(nsxService, never()).apply(MacDiscoveryProfiles.class); + verify(nsxService, never()).apply(SegmentSecurityProfiles.class); + verify(nsxService, never()).apply(Infra.class); + } + + @Test + public void testCreateSegmentRejectsMissingProfileBeforeCreatingSegment() { + Segments segmentService = Mockito.mock(Segments.class); + Infra infraService = Mockito.mock(Infra.class); + IpDiscoveryProfiles ipProfiles = Mockito.mock(IpDiscoveryProfiles.class); + Structure errorData = Mockito.mock(Structure.class); + ApiError apiError = new ApiError(); + apiError.setErrorMessage("profile not found"); + when(nsxService.apply(Segments.class)).thenReturn(segmentService); + when(nsxService.apply(Infra.class)).thenReturn(infraService); + when(nsxService.apply(IpDiscoveryProfiles.class)).thenReturn(ipProfiles); + when(errorData._convertTo(ApiError.class)).thenReturn(apiError); + when(ipProfiles.get("missing-profile")).thenThrow(new NotFound(List.of(), errorData)); + + CloudRuntimeException exception = Assert.assertThrows(CloudRuntimeException.class, + () -> client.createSegment("segment", "tier1", "10.10.10.1/24", + "/infra/sites/default/enforcement-points/default", + List.of(new com.vmware.nsx.model.TransportZone.Builder().setId("tz").build()), + "missing-profile", null, null)); + + Assert.assertEquals("Error creating segment segment: profile not found", exception.getMessage()); + verify(segmentService, never()).patch(anyString(), any(Segment.class)); + verify(infraService, never()).patch(any(com.vmware.nsx_policy.model.Infra.class), eq(false)); + } + + @Test + public void testCreateSegmentRejectsProfileWithoutCanonicalPathBeforeCreatingSegment() { + Segments segmentService = Mockito.mock(Segments.class); + IpDiscoveryProfiles ipProfiles = Mockito.mock(IpDiscoveryProfiles.class); + IPDiscoveryProfile ipProfile = Mockito.mock(IPDiscoveryProfile.class); + when(nsxService.apply(Segments.class)).thenReturn(segmentService); + when(nsxService.apply(IpDiscoveryProfiles.class)).thenReturn(ipProfiles); + when(ipProfiles.get("ip-profile")).thenReturn(ipProfile); + when(ipProfile.getId()).thenReturn("ip-profile"); + when(ipProfile.getPath()).thenReturn(" "); + + Assert.assertThrows(CloudRuntimeException.class, + () -> client.createSegment("segment", "tier1", "10.10.10.1/24", + "/infra/sites/default/enforcement-points/default", + List.of(new com.vmware.nsx.model.TransportZone.Builder().setId("tz").build()), + "ip-profile", null, null)); + + verify(segmentService, never()).patch(anyString(), any(Segment.class)); + verify(nsxService, never()).apply(Infra.class); + } + + @Test + public void testCreateSegmentRejectsProfileMarkedForDeletionBeforeCreatingSegment() { + Segments segmentService = Mockito.mock(Segments.class); + IpDiscoveryProfiles ipProfiles = Mockito.mock(IpDiscoveryProfiles.class); + IPDiscoveryProfile ipProfile = Mockito.mock(IPDiscoveryProfile.class); + when(nsxService.apply(Segments.class)).thenReturn(segmentService); + when(nsxService.apply(IpDiscoveryProfiles.class)).thenReturn(ipProfiles); + when(ipProfiles.get("ip-profile")).thenReturn(ipProfile); + when(ipProfile.getId()).thenReturn("ip-profile"); + when(ipProfile.getPath()).thenReturn("/infra/ip-discovery-profiles/ip-profile"); + when(ipProfile.getMarkedForDelete()).thenReturn(true); + + Assert.assertThrows(CloudRuntimeException.class, + () -> client.createSegment("segment", "tier1", "10.10.10.1/24", + "/infra/sites/default/enforcement-points/default", + List.of(new com.vmware.nsx.model.TransportZone.Builder().setId("tz").build()), + "ip-profile", null, null)); + + verify(segmentService, never()).patch(anyString(), any(Segment.class)); + verify(nsxService, never()).apply(Infra.class); + } + + @Test + public void testCreateSegmentRejectsProfileResolvedWithDifferentIdBeforeCreatingSegment() { + Segments segmentService = Mockito.mock(Segments.class); + IpDiscoveryProfiles ipProfiles = Mockito.mock(IpDiscoveryProfiles.class); + IPDiscoveryProfile ipProfile = Mockito.mock(IPDiscoveryProfile.class); + when(nsxService.apply(Segments.class)).thenReturn(segmentService); + when(nsxService.apply(IpDiscoveryProfiles.class)).thenReturn(ipProfiles); + when(ipProfiles.get("ip-profile")).thenReturn(ipProfile); + when(ipProfile.getId()).thenReturn("different-profile"); + when(ipProfile.getPath()).thenReturn("/infra/ip-discovery-profiles/different-profile"); + + Assert.assertThrows(CloudRuntimeException.class, + () -> client.createSegment("segment", "tier1", "10.10.10.1/24", + "/infra/sites/default/enforcement-points/default", + List.of(new com.vmware.nsx.model.TransportZone.Builder().setId("tz").build()), + "ip-profile", null, null)); + + verify(segmentService, never()).patch(anyString(), any(Segment.class)); + verify(nsxService, never()).apply(Infra.class); + } + + @Test + public void testDeleteSegmentCascadesDiscoveryProfileBindings() { + assertDeleteSegmentCascadesBindings("ip-profile", "mac-profile", null); + } + + @Test + public void testDeleteSegmentCascadesSecurityProfileBindings() { + assertDeleteSegmentCascadesBindings(null, null, "security-profile"); + } + + @Test + public void testDeleteSegmentCascadesAllConfiguredProfileBindings() { + assertDeleteSegmentCascadesBindings("ip-profile", "mac-profile", "security-profile"); + } + + @Test + public void testDeleteSegmentWithoutProfilesUsesScopedCascade() { + assertDeleteSegmentCascadesBindings(null, null, null); + } + + @Test + public void testDeleteSegmentSkipsAlreadyAbsentSegment() { + prepareSegmentDeletion(); + when(segmentService.get(SEGMENT_NAME)).thenThrow(new NotFound(List.of(), null)); + + client.deleteSegment(1L, 2L, 3L, 4L, 5L, SEGMENT_NAME); + + verify(infraService, never()).patch(any(com.vmware.nsx_policy.model.Infra.class), eq(false)); + verify(groupService, never()).delete(anyString(), anyString(), eq(true), eq(false)); + verify(dhcpRelayConfigs).delete(anyString()); + } + + @Test + public void testDeleteSegmentToleratesConcurrentSegmentDeletion() { + prepareSegmentDeletion(); + doThrow(new NotFound(List.of(), null)).when(infraService) + .patch(any(com.vmware.nsx_policy.model.Infra.class), eq(false)); + + client.deleteSegment(1L, 2L, 3L, 4L, 5L, SEGMENT_NAME); + + verify(dhcpRelayConfigs).delete(anyString()); + } + + @Test + public void testDeleteStandaloneSegmentToleratesAlreadyRemovedDependencies() { + prepareSegmentDeletion(); + LbServices lbServices = Mockito.mock(LbServices.class); + when(nsxService.apply(LbServices.class)).thenReturn(lbServices); + doThrow(new NotFound(List.of(), null)).when(securityPolicies).delete(NsxApiClient.DEFAULT_DOMAIN, SEGMENT_NAME); + doThrow(new NotFound(List.of(), null)).when(lbServices).delete(anyString(), eq(true)); + doThrow(new NotFound(List.of(), null)).when(groupService).delete(NsxApiClient.DEFAULT_DOMAIN, SEGMENT_NAME, true, false); + doThrow(new NotFound(List.of(), null)).when(dhcpRelayConfigs).delete(anyString()); + + client.deleteSegment(1L, 2L, 3L, null, 5L, SEGMENT_NAME); + + verify(infraService).patch(any(com.vmware.nsx_policy.model.Infra.class), eq(false)); + verify(dhcpRelayConfigs).delete(anyString()); + } + + @Test + public void testDeleteSegmentRetriesAfterPartialCleanupAndCascadeFailure() { + prepareSegmentDeletion(); + Mockito.doNothing().doThrow(new NotFound(List.of(), null)).when(securityPolicies) + .delete(NsxApiClient.DEFAULT_DOMAIN, SEGMENT_NAME); + Mockito.doNothing().doThrow(new NotFound(List.of(), null)).when(groupService) + .delete(NsxApiClient.DEFAULT_DOMAIN, SEGMENT_NAME, true, false); + doThrow(nsxError("temporary cascade failure")).doNothing().when(infraService) + .patch(any(com.vmware.nsx_policy.model.Infra.class), eq(false)); + Mockito.doNothing().doThrow(new NotFound(List.of(), null)).when(dhcpRelayConfigs).delete(anyString()); + + CloudRuntimeException failure = assertThrows(CloudRuntimeException.class, + () -> client.deleteSegment(1L, 2L, 3L, 4L, 5L, SEGMENT_NAME)); + Assert.assertTrue(failure.getMessage().contains("temporary cascade failure")); + verify(dhcpRelayConfigs, never()).delete(anyString()); + + client.deleteSegment(1L, 2L, 3L, 4L, 5L, SEGMENT_NAME); + when(segmentService.get(SEGMENT_NAME)).thenThrow(new NotFound(List.of(), null)); + client.deleteSegment(1L, 2L, 3L, 4L, 5L, SEGMENT_NAME); + + verify(infraService, Mockito.times(2)).patch(any(com.vmware.nsx_policy.model.Infra.class), eq(false)); + verify(dhcpRelayConfigs, Mockito.times(2)).delete(anyString()); + verify(segmentService, never()).delete(anyString()); + } + + @Test + public void testDeleteSegmentPropagatesLookupFailureWithoutDeletingSegmentOrRelay() { + prepareSegmentDeletion(); + Error lookupError = nsxError("segment lookup denied"); + when(segmentService.get(SEGMENT_NAME)).thenThrow(lookupError); + + CloudRuntimeException failure = assertThrows(CloudRuntimeException.class, + () -> client.deleteSegment(1L, 2L, 3L, 4L, 5L, SEGMENT_NAME)); + + Assert.assertTrue(failure.getMessage().contains("segment lookup denied")); + verify(groupService, never()).delete(anyString(), anyString(), eq(true), eq(false)); + verify(infraService, never()).patch(any(com.vmware.nsx_policy.model.Infra.class), eq(false)); + verify(dhcpRelayConfigs, never()).delete(anyString()); + } + + @Test + public void testDeleteSegmentPropagatesPortLookupFailureBeforeCascade() { + prepareSegmentDeletion(); + Error lookupError = nsxError("port lookup denied"); + when(segmentPorts.list(SEGMENT_NAME, null, false, null, 50L, false, null)).thenThrow(lookupError); + + CloudRuntimeException failure = assertThrows(CloudRuntimeException.class, + () -> client.deleteSegment(1L, 2L, 3L, 4L, 5L, SEGMENT_NAME)); + + Assert.assertTrue(failure.getMessage().contains("port lookup denied")); + verify(groupService, never()).delete(anyString(), anyString(), eq(true), eq(false)); + verify(infraService, never()).patch(any(com.vmware.nsx_policy.model.Infra.class), eq(false)); + verify(dhcpRelayConfigs, never()).delete(anyString()); + } + + @Test + public void testDeleteSegmentPropagatesFirewallFailure() { + prepareSegmentDeletion(); + doThrow(nsxError("firewall deletion denied")).when(securityPolicies) + .delete(NsxApiClient.DEFAULT_DOMAIN, SEGMENT_NAME); + + CloudRuntimeException failure = assertThrows(CloudRuntimeException.class, + () -> client.deleteSegment(1L, 2L, 3L, 4L, 5L, SEGMENT_NAME)); + + Assert.assertTrue(failure.getMessage().contains("firewall deletion denied")); + verify(segmentService, never()).get(anyString()); + verify(infraService, never()).patch(any(com.vmware.nsx_policy.model.Infra.class), eq(false)); + verify(dhcpRelayConfigs, never()).delete(anyString()); + } + + @Test + public void testDeleteStandaloneSegmentPropagatesLoadBalancerFailure() { + prepareSegmentDeletion(); + LbServices lbServices = Mockito.mock(LbServices.class); + when(nsxService.apply(LbServices.class)).thenReturn(lbServices); + doThrow(nsxError("load balancer deletion denied")).when(lbServices).delete(anyString(), eq(true)); + + CloudRuntimeException failure = assertThrows(CloudRuntimeException.class, + () -> client.deleteSegment(1L, 2L, 3L, null, 5L, SEGMENT_NAME)); + + Assert.assertTrue(failure.getMessage().contains("load balancer deletion denied")); + verify(segmentService, never()).get(anyString()); + verify(infraService, never()).patch(any(com.vmware.nsx_policy.model.Infra.class), eq(false)); + verify(dhcpRelayConfigs, never()).delete(anyString()); + } + + @Test + public void testDeleteSegmentPropagatesGroupFailureBeforeCascade() { + prepareSegmentDeletion(); + doThrow(nsxError("group deletion denied")).when(groupService) + .delete(NsxApiClient.DEFAULT_DOMAIN, SEGMENT_NAME, true, false); + + CloudRuntimeException failure = assertThrows(CloudRuntimeException.class, + () -> client.deleteSegment(1L, 2L, 3L, 4L, 5L, SEGMENT_NAME)); + + Assert.assertTrue(failure.getMessage().contains("group deletion denied")); + verify(infraService, never()).patch(any(com.vmware.nsx_policy.model.Infra.class), eq(false)); + verify(dhcpRelayConfigs, never()).delete(anyString()); + } + + @Test + public void testDeleteSegmentPropagatesDhcpRelayFailure() { + prepareSegmentDeletion(); + doThrow(nsxError("DHCP relay deletion denied")).when(dhcpRelayConfigs).delete(anyString()); + + CloudRuntimeException failure = assertThrows(CloudRuntimeException.class, + () -> client.deleteSegment(1L, 2L, 3L, 4L, 5L, SEGMENT_NAME)); + + Assert.assertTrue(failure.getMessage().contains("DHCP relay deletion denied")); + verify(infraService).patch(any(com.vmware.nsx_policy.model.Infra.class), eq(false)); + } + + @Test + public void testDeleteSegmentDoesNotCascadeWhilePortsRemain() { + prepareSegmentDeletion(); + SegmentPortListResult ports = new SegmentPortListResult(); + ports.setResults(List.of(new SegmentPort.Builder().setId("attached-port").build())); + ports.setResultCount(1L); + when(segmentPorts.list(SEGMENT_NAME, null, false, null, 50L, false, null)).thenReturn(ports); + ConfigDepotImpl previousDepot = (ConfigDepotImpl) ReflectionTestUtils.getField(ConfigKey.class, "s_depot"); + ConfigDepotImpl depot = Mockito.mock(ConfigDepotImpl.class); + when(depot.getConfigStringValue("nsx.api.failure.retries", ConfigKey.Scope.Zone, 1L)).thenReturn("1"); + when(depot.getConfigStringValue("nsx.api.failure.interval", ConfigKey.Scope.Zone, 1L)).thenReturn("0"); + try { + ConfigKey.init(depot); + CloudRuntimeException failure = assertThrows(CloudRuntimeException.class, + () -> client.deleteSegment(1L, 2L, 3L, 4L, 5L, SEGMENT_NAME)); + Assert.assertTrue(failure.getMessage().contains("there are still 1 port group(s) attached")); + } finally { + ConfigKey.init(previousDepot); + } + + verify(groupService, never()).delete(anyString(), anyString(), eq(true), eq(false)); + verify(infraService, never()).patch(any(com.vmware.nsx_policy.model.Infra.class), eq(false)); + verify(dhcpRelayConfigs, never()).delete(anyString()); + } + + private void assertDeleteSegmentCascadesBindings(String ipProfileId, String macProfileId, String securityProfileId) { + prepareSegmentDeletion(); + ApiProvider apiProvider = Mockito.mock(ApiProvider.class); + Infra serializingInfraService = Mockito.spy(new InfraStub(apiProvider, new StubConfiguration())); + when(nsxService.apply(Infra.class)).thenReturn(serializingInfraService); + Mockito.doAnswer(invocation -> { + AsyncHandle asyncHandle = invocation.getArgument(4); + asyncHandle.setResult(MethodResult.EMPTY); + return null; + }).when(apiProvider).invoke(anyString(), eq("patch"), any(DataValue.class), any(), any()); + IpDiscoveryProfiles ipProfiles = Mockito.mock(IpDiscoveryProfiles.class); + MacDiscoveryProfiles macProfiles = Mockito.mock(MacDiscoveryProfiles.class); + SegmentSecurityProfiles securityProfiles = Mockito.mock(SegmentSecurityProfiles.class); + when(nsxService.apply(IpDiscoveryProfiles.class)).thenReturn(ipProfiles); + when(nsxService.apply(MacDiscoveryProfiles.class)).thenReturn(macProfiles); + when(nsxService.apply(SegmentSecurityProfiles.class)).thenReturn(securityProfiles); + if (ipProfileId != null) { + IPDiscoveryProfile profile = new IPDiscoveryProfile.Builder() + .setId(ipProfileId).setPath("/infra/ip-discovery-profiles/" + ipProfileId).build(); + when(ipProfiles.get(ipProfileId)).thenReturn(profile); + } + if (macProfileId != null) { + MacDiscoveryProfile profile = new MacDiscoveryProfile.Builder() + .setId(macProfileId).setPath("/infra/mac-discovery-profiles/" + macProfileId).build(); + when(macProfiles.get(macProfileId)).thenReturn(profile); + } + if (securityProfileId != null) { + SegmentSecurityProfile profile = new SegmentSecurityProfile.Builder() + .setId(securityProfileId).setPath("/infra/segment-security-profiles/" + securityProfileId).build(); + when(securityProfiles.get(securityProfileId)).thenReturn(profile); + } + + client.createSegment(SEGMENT_NAME, TIER_1_GATEWAY_NAME, "10.10.10.1/24", ENFORCEMENT_POINT_PATH, + List.of(new com.vmware.nsx.model.TransportZone.Builder().setId("tz").build()), + ipProfileId, macProfileId, securityProfileId); + client.deleteSegment(1L, 2L, 3L, 4L, 5L, SEGMENT_NAME); + + int bindingCount = (ipProfileId != null || macProfileId != null ? 1 : 0) + (securityProfileId != null ? 1 : 0); + int infraCalls = bindingCount == 0 ? 1 : 2; + ArgumentCaptor requests = ArgumentCaptor.forClass(com.vmware.nsx_policy.model.Infra.class); + verify(serializingInfraService, Mockito.times(infraCalls)).patch(requests.capture(), eq(false)); + verify(apiProvider, Mockito.times(infraCalls)).invoke(anyString(), eq("patch"), any(DataValue.class), any(), any()); + if (bindingCount > 0) { + ChildSegment created = (ChildSegment) requests.getAllValues().get(0).getChildren().get(0); + Assert.assertEquals(bindingCount, created.getSegment().getChildren().size()); + } else { + verify(segmentService).patch(eq(SEGMENT_NAME), any(Segment.class)); + } + com.vmware.nsx_policy.model.Infra deletion = requests.getAllValues().get(infraCalls - 1); + Assert.assertEquals("Infra", deletion.getResourceType()); + Assert.assertEquals(1, deletion.getChildren().size()); + ChildSegment child = (ChildSegment) deletion.getChildren().get(0); + Assert.assertEquals("ChildSegment", child.getResourceType()); + Assert.assertEquals(SEGMENT_NAME, child.getId()); + Assert.assertEquals(Boolean.TRUE, child.getMarkedForDelete()); + Assert.assertEquals("Segment", child.getSegment().getResourceType()); + Assert.assertEquals(SEGMENT_NAME, child.getSegment().getId()); + Assert.assertTrue(child.getSegment().getChildren() == null || child.getSegment().getChildren().isEmpty()); + verify(segmentService, never()).delete(anyString()); + if (ipProfileId != null) { + verify(ipProfiles).get(ipProfileId); + } + if (macProfileId != null) { + verify(macProfiles).get(macProfileId); + } + if (securityProfileId != null) { + verify(securityProfiles).get(securityProfileId); + } + Mockito.verifyNoMoreInteractions(ipProfiles, macProfiles, securityProfiles); + } + + private void prepareSegmentDeletion() { + when(nsxService.apply(Segments.class)).thenReturn(segmentService); + when(nsxService.apply(Infra.class)).thenReturn(infraService); + when(nsxService.apply(SecurityPolicies.class)).thenReturn(securityPolicies); + when(nsxService.apply(DhcpRelayConfigs.class)).thenReturn(dhcpRelayConfigs); + when(nsxService.apply(Ports.class)).thenReturn(segmentPorts); + when(segmentService.get(SEGMENT_NAME)).thenReturn(new Segment.Builder().setId(SEGMENT_NAME).build()); + SegmentPortListResult ports = new SegmentPortListResult(); + ports.setResults(List.of()); + ports.setResultCount(0L); + when(segmentPorts.list(SEGMENT_NAME, null, false, null, 50L, false, null)).thenReturn(ports); + } + + private Error nsxError(String message) { + Structure data = Mockito.mock(Structure.class); + ApiError error = new ApiError(); + error.setErrorMessage(message); + when(data._convertTo(ApiError.class)).thenReturn(error); + return new Error(List.of(), data); + } + @Test public void testGetGroupsForTrafficIngress() { SDNProviderNetworkRule rule = Mockito.mock(SDNProviderNetworkRule.class); diff --git a/plugins/network-elements/nsx/src/test/java/org/apache/cloudstack/service/NsxGuestNetworkGuruTest.java b/plugins/network-elements/nsx/src/test/java/org/apache/cloudstack/service/NsxGuestNetworkGuruTest.java index cb79873f364d..251e5214e43f 100644 --- a/plugins/network-elements/nsx/src/test/java/org/apache/cloudstack/service/NsxGuestNetworkGuruTest.java +++ b/plugins/network-elements/nsx/src/test/java/org/apache/cloudstack/service/NsxGuestNetworkGuruTest.java @@ -58,6 +58,7 @@ import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; import org.mockito.ArgumentMatchers; import org.mockito.Mock; import org.mockito.Mockito; @@ -66,6 +67,7 @@ import org.springframework.test.util.ReflectionTestUtils; import java.util.List; +import java.util.Map; import static org.junit.Assert.assertTrue; import static org.junit.Assert.assertEquals; @@ -304,6 +306,29 @@ public void testCreateNsxSegmentForVpc() { anyLong()); } + @Test + public void testCreateNsxSegmentPassesNetworkOfferingProfiles() { + NetworkVO networkVO = Mockito.mock(NetworkVO.class); + DataCenter dataCenter = Mockito.mock(DataCenter.class); + when(networkVO.getAccountId()).thenReturn(1L); + when(networkVO.getNetworkOfferingId()).thenReturn(42L); + when(networkModel.getNtwkOffDetails(42L)).thenReturn(Map.of( + NetworkOffering.Detail.NsxIpDiscoveryProfileId, "ip-profile", + NetworkOffering.Detail.NsxMacDiscoveryProfileId, "mac-profile", + NetworkOffering.Detail.NsxSegmentSecurityProfileId, "security-profile")); + when(nsxControllerUtils.sendNsxCommand(any(CreateNsxSegmentCommand.class), anyLong())) + .thenReturn(new NsxAnswer(new NsxCommand(), true, "")); + ArgumentCaptor commandCaptor = ArgumentCaptor.forClass(CreateNsxSegmentCommand.class); + + guru.createNsxSegment(networkVO, dataCenter); + + verify(nsxControllerUtils).sendNsxCommand(commandCaptor.capture(), anyLong()); + CreateNsxSegmentCommand command = commandCaptor.getValue(); + assertEquals("ip-profile", command.getIpDiscoveryProfileId()); + assertEquals("mac-profile", command.getMacDiscoveryProfileId()); + assertEquals("security-profile", command.getSegmentSecurityProfileId()); + } + @Test public void testCreateNsxSegmentForIsolatedNetwork() { diff --git a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java index 70ec6b3d8b0a..f34e638e6b3f 100644 --- a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java +++ b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java @@ -575,6 +575,9 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati private static final String DefaultVlanForPodIpRange = Vlan.UNTAGGED; private static final Set VPC_ONLY_PROVIDERS = Sets.newHashSet(Provider.VPCVirtualRouter, Provider.JuniperContrailVpcRouter, Provider.InternalLbVm); + private static final Set NSX_SEGMENT_PROFILE_DETAILS = Set.of(Detail.NsxIpDiscoveryProfileId, + Detail.NsxMacDiscoveryProfileId, Detail.NsxSegmentSecurityProfileId); + private static final int MAX_NSX_PROFILE_ID_LENGTH = 255; private static final List SUPPORTED_ROUTING_MODE_STRS = Arrays.asList(Static.toString().toLowerCase(), Dynamic.toString().toLowerCase()); private static final long GiB_TO_BYTES = 1024 * 1024 * 1024; @@ -7777,7 +7780,7 @@ public NetworkOfferingVO createNetworkOffering(final String name, final String d serviceProviderMap.put(Service.Gateway, Sets.newHashSet(Provider.VirtualRouter)); } } else { - Set providers = serviceProviderMap.get(Service.NetworkACL); + Set providers = serviceProviderMap.get(forVpc ? Service.NetworkACL : Service.Firewall); serviceProviderMap.put(Service.Gateway, Sets.newHashSet(providers.iterator().next())); } } @@ -8041,8 +8044,28 @@ boolean isSharedSourceNat(Map> serviceProviderMap, Map details, final Map> serviceProviderMap) { + boolean nsxProviderConfigured = serviceProviderMap != null && serviceProviderMap.values().stream() + .filter(Objects::nonNull) + .anyMatch(providers -> providers.contains(Provider.Nsx)); for (final Detail detail : details.keySet()) { + if (NSX_SEGMENT_PROFILE_DETAILS.contains(detail)) { + String profileId = details.get(detail); + if (!nsxProviderConfigured) { + throw new InvalidParameterValueException(String.format("Detail %s is supported only by NSX network offerings", detail)); + } + if (StringUtils.isBlank(profileId)) { + throw new InvalidParameterValueException(String.format("A non-empty NSX profile ID is required for detail %s", detail)); + } + if (!profileId.equals(profileId.trim())) { + throw new InvalidParameterValueException(String.format("NSX profile ID for detail %s must not contain surrounding whitespace", detail)); + } + if (profileId.length() > MAX_NSX_PROFILE_ID_LENGTH) { + throw new InvalidParameterValueException(String.format("NSX profile ID for detail %s cannot exceed %d characters", + detail, MAX_NSX_PROFILE_ID_LENGTH)); + } + } + Provider lbProvider = null; if (detail == NetworkOffering.Detail.InternalLbProvider || detail == NetworkOffering.Detail.PublicLbProvider) { // 1) Vaidate the detail values - have to match the lb provider @@ -8690,7 +8713,7 @@ private void applyResolvedValuesToCommand(CloneNetworkOfferingCmd cmd, NetworkOf setField(cmd, "routingMode", sourceOffering.getRoutingMode().toString()); } - if (cmd.getDetails() == null || cmd.getDetails().isEmpty()) { + if (!cmd.isCleanupDetails() && (cmd.getDetails() == null || cmd.getDetails().isEmpty())) { if (!sourceDetailsMap.isEmpty()) { setField(cmd, "sourceDetailsMap", sourceDetailsMap); } diff --git a/server/src/main/java/com/cloud/network/NetworkServiceImpl.java b/server/src/main/java/com/cloud/network/NetworkServiceImpl.java index 9ac33d47a34d..2fd72a44cf99 100644 --- a/server/src/main/java/com/cloud/network/NetworkServiceImpl.java +++ b/server/src/main/java/com/cloud/network/NetworkServiceImpl.java @@ -5012,6 +5012,12 @@ private boolean canMoveToPhysicalNetwork(Network network, long oldNetworkOfferin NetworkOffering oldNetworkOffering = _networkOfferingDao.findByIdIncludingRemoved(oldNetworkOfferingId); NetworkOffering newNetworkOffering = _networkOfferingDao.findById(newNetworkOfferingId); + if (!haveMatchingNsxSegmentProfiles(oldNetworkOfferingId, newNetworkOfferingId)) { + logger.debug("Network offerings {} and {} have different NSX segment profile bindings, can't upgrade", + oldNetworkOffering, newNetworkOffering); + return false; + } + // can move only Isolated networks for now if (oldNetworkOffering.getGuestType() != GuestType.Isolated) { throw new InvalidParameterValueException("NetworkOfferingId can be upgraded only for the network of type " + GuestType.Isolated); @@ -5100,6 +5106,20 @@ protected boolean canUpgrade(Network network, long oldNetworkOfferingId, long ne return canMoveToPhysicalNetwork(network, oldNetworkOfferingId, newNetworkOfferingId); } + protected boolean haveMatchingNsxSegmentProfiles(long oldNetworkOfferingId, long newNetworkOfferingId) { + Map oldDetails = _networkModel.getNtwkOffDetails(oldNetworkOfferingId); + Map newDetails = _networkModel.getNtwkOffDetails(newNetworkOfferingId); + for (NetworkOffering.Detail detail : List.of(NetworkOffering.Detail.NsxIpDiscoveryProfileId, + NetworkOffering.Detail.NsxMacDiscoveryProfileId, NetworkOffering.Detail.NsxSegmentSecurityProfileId)) { + String oldProfileId = oldDetails == null ? null : oldDetails.get(detail); + String newProfileId = newDetails == null ? null : newDetails.get(detail); + if (!Objects.equals(oldProfileId, newProfileId)) { + return false; + } + } + return true; + } + @Override @DB @ActionEvent(eventType = EventTypes.EVENT_PHYSICAL_NETWORK_CREATE, eventDescription = "Creating Physical Network", create = true) diff --git a/server/src/test/java/com/cloud/configuration/ConfigurationManagerCloneIntegrationTest.java b/server/src/test/java/com/cloud/configuration/ConfigurationManagerCloneIntegrationTest.java index 33093a60d0cd..46981912fea3 100644 --- a/server/src/test/java/com/cloud/configuration/ConfigurationManagerCloneIntegrationTest.java +++ b/server/src/test/java/com/cloud/configuration/ConfigurationManagerCloneIntegrationTest.java @@ -26,6 +26,7 @@ import com.cloud.offering.DiskOffering; import com.cloud.offering.NetworkOffering; import com.cloud.offering.ServiceOffering; +import com.cloud.offerings.NetworkOfferingDetailsVO; import com.cloud.offerings.NetworkOfferingVO; import com.cloud.offerings.dao.NetworkOfferingDao; import com.cloud.offerings.dao.NetworkOfferingServiceMapDao; @@ -63,6 +64,7 @@ import org.mockito.Mockito; import org.mockito.Spy; import org.mockito.junit.MockitoJUnitRunner; +import org.springframework.test.util.ReflectionTestUtils; import java.util.ArrayList; import java.util.Collections; @@ -730,6 +732,96 @@ public void testCloneNetworkOfferingFailsWhenSourceNotFound() { configurationManager.cloneNetworkOffering(cmd); } + @Test + public void testCloneNetworkOfferingInheritsNsxProfilesWhenDetailsAreOmitted() { + Map sourceDetails = Map.of( + NetworkOffering.Detail.NsxIpDiscoveryProfileId, "source-ip-profile", + NetworkOffering.Detail.NsxMacDiscoveryProfileId, "source-mac-profile", + NetworkOffering.Detail.NsxSegmentSecurityProfileId, "source-security-profile"); + + Map details = cloneNetworkOfferingAndCaptureDetails(sourceDetails, null, null); + + Assert.assertEquals(Map.of("NsxIpDiscoveryProfileId", "source-ip-profile", + "NsxMacDiscoveryProfileId", "source-mac-profile", + "NsxSegmentSecurityProfileId", "source-security-profile"), details); + } + + @Test + public void testCloneNetworkOfferingClearsNsxProfilesWithoutInheritingSourceDetails() { + Map sourceDetails = Map.of( + NetworkOffering.Detail.NsxIpDiscoveryProfileId, "source-ip-profile", + NetworkOffering.Detail.NsxMacDiscoveryProfileId, "source-mac-profile", + NetworkOffering.Detail.NsxSegmentSecurityProfileId, "source-security-profile"); + + Map details = cloneNetworkOfferingAndCaptureDetails(sourceDetails, null, true); + + Assert.assertTrue(details.isEmpty()); + } + + @Test + public void testCloneNetworkOfferingExplicitDetailsRemoveOtherSourceProfiles() { + Map sourceDetails = Map.of( + NetworkOffering.Detail.NsxIpDiscoveryProfileId, "source-ip-profile", + NetworkOffering.Detail.NsxMacDiscoveryProfileId, "source-mac-profile", + NetworkOffering.Detail.RelatedNetworkOffering, "related-offering"); + Map requestedDetails = Map.of("nsxmacdiscoveryprofileid", "source-mac-profile", + "RelatedNetworkOffering", "related-offering"); + + Map details = cloneNetworkOfferingAndCaptureDetails(sourceDetails, requestedDetails, false); + + Assert.assertEquals(requestedDetails, details); + } + + private Map cloneNetworkOfferingAndCaptureDetails(Map sourceDetails, + Map requestedDetails, Boolean cleanupDetails) { + long sourceId = 1L; + NetworkOfferingVO sourceOffering = mock(NetworkOfferingVO.class); + when(sourceOffering.getId()).thenReturn(sourceId); + when(sourceOffering.getName()).thenReturn("source-network-offering"); + when(sourceOffering.getGuestType()).thenReturn(Network.GuestType.Isolated); + when(sourceOffering.getTrafficType()).thenReturn(Networks.TrafficType.Guest); + when(sourceOffering.getNetworkMode()).thenReturn(NetworkOffering.NetworkMode.NATTED); + when(networkOfferingDao.findById(sourceId)).thenReturn(sourceOffering); + when(_networkModel.getNetworkOfferingServiceProvidersMap(sourceId)).thenReturn( + Map.of(Network.Service.Firewall, Collections.singleton(Network.Provider.Nsx))); + + List sourceDetailRows = new ArrayList<>(); + sourceDetails.forEach((key, value) -> sourceDetailRows.add(new NetworkOfferingDetailsVO(sourceId, key, value, true))); + sourceDetailRows.add(new NetworkOfferingDetailsVO(sourceId, NetworkOffering.Detail.internetProtocol, "ipv4", true)); + sourceDetailRows.add(new NetworkOfferingDetailsVO(sourceId, NetworkOffering.Detail.domainid, "11", true)); + sourceDetailRows.add(new NetworkOfferingDetailsVO(sourceId, NetworkOffering.Detail.zoneid, "22", true)); + when(networkOfferingDetailsDao.listDetails(sourceId)).thenReturn(sourceDetailRows); + when(networkOfferingDetailsDao.getDetail(sourceId, NetworkOffering.Detail.internetProtocol)).thenReturn("ipv4"); + when(networkOfferingDetailsDao.findDomainIds(sourceId)).thenReturn(new ArrayList<>(List.of(11L))); + when(networkOfferingDetailsDao.findZoneIds(sourceId)).thenReturn(new ArrayList<>(List.of(22L))); + + CloneNetworkOfferingCmd cmd = new CloneNetworkOfferingCmd(); + ReflectionTestUtils.setField(cmd, "sourceOfferingId", sourceId); + ReflectionTestUtils.setField(cmd, "networkOfferingName", "cloned-network-offering"); + ReflectionTestUtils.setField(cmd, "cleanupDetails", cleanupDetails); + if (requestedDetails != null) { + ReflectionTestUtils.setField(cmd, "details", Map.of("0", requestedDetails)); + } + cmd.setFullUrlParams(new HashMap<>()); + + Map capturedDetails = new HashMap<>(); + NetworkOfferingVO clonedOffering = mock(NetworkOfferingVO.class); + Mockito.doAnswer(invocation -> { + CloneNetworkOfferingCmd resolvedCmd = invocation.getArgument(0); + if (resolvedCmd.getDetails() != null) { + capturedDetails.putAll(resolvedCmd.getDetails()); + } + Assert.assertEquals("ipv4", resolvedCmd.getInternetProtocol()); + Assert.assertEquals(List.of(11L), resolvedCmd.getDomainIds()); + Assert.assertEquals(List.of(22L), resolvedCmd.getZoneIds()); + return clonedOffering; + }).when(configurationManager).createNetworkOffering(cmd); + + Assert.assertSame(clonedOffering, configurationManager.cloneNetworkOffering(cmd)); + verify(configurationManager).createNetworkOffering(cmd); + return capturedDetails; + } + @Test(expected = InvalidParameterValueException.class) public void testCloneNetworkOfferingFailsWhenNameIsNull() { CloneNetworkOfferingCmd cmd = mock(CloneNetworkOfferingCmd.class); diff --git a/server/src/test/java/com/cloud/configuration/ConfigurationManagerImplTest.java b/server/src/test/java/com/cloud/configuration/ConfigurationManagerImplTest.java index 9a0b150780e4..7dcbd85f8a23 100644 --- a/server/src/test/java/com/cloud/configuration/ConfigurationManagerImplTest.java +++ b/server/src/test/java/com/cloud/configuration/ConfigurationManagerImplTest.java @@ -41,7 +41,9 @@ import com.cloud.offering.DiskOffering; import com.cloud.offering.NetworkOffering; import com.cloud.offerings.NetworkOfferingVO; +import com.cloud.offerings.NetworkOfferingServiceMapVO; import com.cloud.offerings.dao.NetworkOfferingDao; +import com.cloud.offerings.dao.NetworkOfferingServiceMapDao; import com.cloud.service.ServiceOfferingVO; import com.cloud.storage.DiskOfferingVO; import com.cloud.storage.StorageManager; @@ -71,6 +73,7 @@ import org.apache.cloudstack.framework.config.ConfigKey; import org.apache.cloudstack.framework.config.dao.ConfigurationDao; import org.apache.cloudstack.framework.config.impl.ConfigurationVO; +import org.apache.cloudstack.network.RoutedIpv4Manager; import org.apache.cloudstack.storage.datastore.db.ImageStoreDao; import org.apache.cloudstack.resourcedetail.DiskOfferingDetailVO; import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; @@ -181,6 +184,8 @@ public class ConfigurationManagerImplTest { @Mock NetworkOfferingDao networkOfferingDao; @Mock + NetworkOfferingServiceMapDao networkOfferingServiceMapDao; + @Mock NetworkService networkService; @Mock NetworkModel networkModel; @@ -1410,4 +1415,112 @@ public void testGetExternalNetworkProviderReturnsNullWhenNoExternalProviders() { mapWithEmptySet.put(Network.Service.Firewall, Collections.emptySet()); Assert.assertNull(ConfigurationManagerImpl.getExternalNetworkProvider(null, mapWithEmptySet)); } + + @Test + public void testValidateNetworkOfferingDetailsAcceptsNsxSegmentProfilesForNsxOffering() { + Map details = Map.of( + NetworkOffering.Detail.NsxIpDiscoveryProfileId, "cloudstack-ip-discovery", + NetworkOffering.Detail.NsxMacDiscoveryProfileId, "cloudstack-mac-discovery", + NetworkOffering.Detail.NsxSegmentSecurityProfileId, "cloudstack-segment-security"); + Map> serviceProviderMap = Map.of( + Network.Service.Connectivity, Set.of(Network.Provider.Nsx)); + + configurationManagerImplSpy.validateNtwkOffDetails(details, serviceProviderMap); + } + + @Test + public void testCreateNsxSegmentProfilesWithActualOfferingCommands() { + Map details = Map.of( + NetworkOffering.Detail.NsxIpDiscoveryProfileId, "ip-discovery", + NetworkOffering.Detail.NsxMacDiscoveryProfileId, "mac-discovery", + NetworkOffering.Detail.NsxSegmentSecurityProfileId, "segment-security"); + List savedServices = new ArrayList<>(); + when(networkModel.resolveProvider(anyString())).thenAnswer(invocation -> Network.Provider.getProvider(invocation.getArgument(0))); + when(networkOfferingDao.persist(any(NetworkOfferingVO.class), anyMap())).thenAnswer(invocation -> invocation.getArgument(0)); + when(networkOfferingServiceMapDao.persist(any(NetworkOfferingServiceMapVO.class))).thenAnswer(invocation -> { + NetworkOfferingServiceMapVO service = invocation.getArgument(0); + savedServices.add(service); + return service; + }); + + String originalRoutedDefault = RoutedIpv4Manager.RoutedNetworkVpcEnabled.defaultValue(); + ReflectionTestUtils.setField(RoutedIpv4Manager.RoutedNetworkVpcEnabled, "_defaultValue", "true"); + CallContext context = mock(CallContext.class); + try (MockedStatic mockedCallContext = Mockito.mockStatic(CallContext.class)) { + mockedCallContext.when(CallContext::current).thenReturn(context); + for (NetworkOffering.NetworkMode mode : NetworkOffering.NetworkMode.values()) { + for (boolean forVpc : List.of(false, true)) { + CreateNetworkOfferingCmd command = new CreateNetworkOfferingCmd(); + ReflectionTestUtils.setField(command, "networkOfferingName", "nsx-profiles"); + ReflectionTestUtils.setField(command, "traffictype", Networks.TrafficType.Guest.name()); + ReflectionTestUtils.setField(command, "guestIptype", Network.GuestType.Isolated.name()); + ReflectionTestUtils.setField(command, "provider", "NSX"); + ReflectionTestUtils.setField(command, "networkMode", mode.name()); + ReflectionTestUtils.setField(command, "forVpc", forVpc); + ReflectionTestUtils.setField(command, "details", Map.of("0", Map.of( + "nsxipdiscoveryprofileid", "ip-discovery", + "nsxmacdiscoveryprofileid", "mac-discovery", + "nsxsegmentsecurityprofileid", "segment-security"))); + savedServices.clear(); + Mockito.clearInvocations(networkOfferingDao); + + NetworkOfferingVO offering = (NetworkOfferingVO) configurationManagerImplSpy.createNetworkOffering(command); + + Assert.assertEquals(mode, offering.getNetworkMode()); + Assert.assertEquals(forVpc, offering.isForVpc()); + verify(networkOfferingDao).persist(Mockito.same(offering), Mockito.eq(details)); + if (mode == NetworkOffering.NetworkMode.ROUTED) { + Assert.assertTrue(savedServices.stream().anyMatch(service -> + Network.Service.Gateway.getName().equals(service.getService()) + && Network.Provider.Nsx.getName().equals(service.getProvider()))); + } + } + } + } finally { + ReflectionTestUtils.setField(RoutedIpv4Manager.RoutedNetworkVpcEnabled, "_defaultValue", originalRoutedDefault); + } + } + + @Test + public void testValidateNetworkOfferingDetailsRejectsWhitespaceInNsxProfileId() { + Map details = Map.of( + NetworkOffering.Detail.NsxMacDiscoveryProfileId, " mac-profile "); + Map> serviceProviderMap = Map.of( + Network.Service.Connectivity, Set.of(Network.Provider.Nsx)); + + Assert.assertThrows(InvalidParameterValueException.class, + () -> configurationManagerImplSpy.validateNtwkOffDetails(details, serviceProviderMap)); + } + + @Test + public void testValidateNetworkOfferingDetailsRejectsNsxSegmentProfileForNonNsxOffering() { + Map details = Map.of( + NetworkOffering.Detail.NsxIpDiscoveryProfileId, "cloudstack-ip-discovery"); + Map> serviceProviderMap = Map.of( + Network.Service.Connectivity, Set.of(Network.Provider.VPCVirtualRouter)); + + Assert.assertThrows(InvalidParameterValueException.class, + () -> configurationManagerImplSpy.validateNtwkOffDetails(details, serviceProviderMap)); + } + + @Test + public void testValidateNetworkOfferingDetailsRejectsBlankNsxSegmentProfile() { + Map details = Map.of(NetworkOffering.Detail.NsxMacDiscoveryProfileId, " "); + Map> serviceProviderMap = Map.of( + Network.Service.Connectivity, Set.of(Network.Provider.Nsx)); + + Assert.assertThrows(InvalidParameterValueException.class, + () -> configurationManagerImplSpy.validateNtwkOffDetails(details, serviceProviderMap)); + } + + @Test + public void testValidateNetworkOfferingDetailsRejectsOversizedNsxSegmentProfile() { + Map details = Map.of( + NetworkOffering.Detail.NsxSegmentSecurityProfileId, "x".repeat(256)); + Map> serviceProviderMap = Map.of( + Network.Service.Connectivity, Set.of(Network.Provider.Nsx)); + + Assert.assertThrows(InvalidParameterValueException.class, + () -> configurationManagerImplSpy.validateNtwkOffDetails(details, serviceProviderMap)); + } } diff --git a/server/src/test/java/com/cloud/network/NetworkServiceImplTest.java b/server/src/test/java/com/cloud/network/NetworkServiceImplTest.java index 22317e3e118d..a8daa996cc9d 100644 --- a/server/src/test/java/com/cloud/network/NetworkServiceImplTest.java +++ b/server/src/test/java/com/cloud/network/NetworkServiceImplTest.java @@ -125,6 +125,7 @@ import com.cloud.network.vpc.VpcManager; import com.cloud.network.vpc.VpcVO; import com.cloud.network.vpc.dao.VpcDao; +import com.cloud.network.vpc.dao.VpcOfferingDao; import com.cloud.offering.NetworkOffering; import com.cloud.offering.ServiceOffering; import com.cloud.offerings.NetworkOfferingVO; @@ -135,6 +136,7 @@ import com.cloud.service.dao.ServiceOfferingDao; import com.cloud.storage.VMTemplateVO; import com.cloud.storage.dao.VMTemplateDao; +import com.cloud.tags.dao.ResourceTagDao; import com.cloud.user.Account; import com.cloud.user.AccountManager; import com.cloud.user.AccountService; @@ -156,6 +158,7 @@ import com.cloud.vm.VirtualMachine; import com.cloud.vm.dao.DomainRouterDao; import com.cloud.vm.dao.NicDao; +import com.cloud.vm.dao.VMInstanceDao; @RunWith(MockitoJUnitRunner.class) public class NetworkServiceImplTest { @@ -187,6 +190,14 @@ public class NetworkServiceImplTest { @Mock NetworkOfferingServiceMapDao networkOfferingServiceMapDao; @Mock + NetworkMigrationManager networkMigrationManager; + @Mock + VMInstanceDao vmInstanceDao; + @Mock + ResourceTagDao resourceTagDao; + @Mock + VpcOfferingDao vpcOfferingDao; + @Mock EntityManager entityMgr; @Mock NetworkService networkService; @@ -751,6 +762,155 @@ public void testCheckAndUpdateNetworkNoUpdate() { Assert.assertFalse(service.checkAndUpdateNetworkDns(network1, Mockito.mock(NetworkOffering.class), ip4Dns[0], null, ip6Dns[0], null)); } + @Test + public void testMatchingNsxSegmentProfilesAllowOfferingUpgrade() { + long oldOfferingId = 1L; + long newOfferingId = 2L; + Map profiles = Map.of( + NetworkOffering.Detail.NsxIpDiscoveryProfileId, "ip-profile", + NetworkOffering.Detail.NsxMacDiscoveryProfileId, "mac-profile", + NetworkOffering.Detail.NsxSegmentSecurityProfileId, "security-profile"); + Mockito.when(networkModel.getNtwkOffDetails(oldOfferingId)).thenReturn(profiles); + Mockito.when(networkModel.getNtwkOffDetails(newOfferingId)).thenReturn(profiles); + + Assert.assertTrue(service.haveMatchingNsxSegmentProfiles(oldOfferingId, newOfferingId)); + } + + @Test + public void testDifferentNsxSegmentProfilesRejectOfferingUpgrade() { + long oldOfferingId = 1L; + long newOfferingId = 2L; + long networkId = 3L; + Network network = Mockito.mock(Network.class); + NetworkOfferingVO oldOffering = Mockito.mock(NetworkOfferingVO.class); + NetworkOfferingVO newOffering = Mockito.mock(NetworkOfferingVO.class); + Mockito.when(network.getId()).thenReturn(networkId); + Mockito.when(networkOfferingDao.findByIdIncludingRemoved(oldOfferingId)).thenReturn(oldOffering); + Mockito.when(networkOfferingDao.findById(newOfferingId)).thenReturn(newOffering); + Mockito.when(oldOffering.getGuestType()).thenReturn(Network.GuestType.Isolated); + Mockito.when(newOffering.getGuestType()).thenReturn(Network.GuestType.Isolated); + Mockito.when(oldOffering.getTrafficType()).thenReturn(Networks.TrafficType.Guest); + Mockito.when(newOffering.getTrafficType()).thenReturn(Networks.TrafficType.Guest); + Mockito.when(ipAddressDao.listByAssociatedNetwork(networkId, null)).thenReturn(List.of()); + Mockito.when(networkModel.getNetworkOfferingServiceProvidersMap(newOfferingId)).thenReturn(new HashMap<>()); + Mockito.when(networkModel.getNtwkOffDetails(oldOfferingId)).thenReturn(Map.of( + NetworkOffering.Detail.NsxMacDiscoveryProfileId, "old-mac-profile")); + Mockito.when(networkModel.getNtwkOffDetails(newOfferingId)).thenReturn(Map.of( + NetworkOffering.Detail.NsxMacDiscoveryProfileId, "old-mac-profile")); + + Assert.assertTrue(service.canUpgrade(network, oldOfferingId, newOfferingId)); + + Mockito.when(networkModel.getNtwkOffDetails(newOfferingId)).thenReturn(Map.of( + NetworkOffering.Detail.NsxMacDiscoveryProfileId, "new-mac-profile")); + + Assert.assertFalse(service.canUpgrade(network, oldOfferingId, newOfferingId)); + } + + @Test + public void testAbsentNsxSegmentProfilesAllowOfferingUpgrade() { + long oldOfferingId = 1L; + long newOfferingId = 2L; + Mockito.when(networkModel.getNtwkOffDetails(oldOfferingId)).thenReturn(null); + Mockito.when(networkModel.getNtwkOffDetails(newOfferingId)).thenReturn(Map.of()); + + Assert.assertTrue(service.haveMatchingNsxSegmentProfiles(oldOfferingId, newOfferingId)); + } + + @Test + public void testDifferentNsxSegmentProfilesRejectNetworkMigrationBeforeMutation() { + NetworkVO network = prepareNsxProfileMigration(false); + + InvalidParameterValueException exception = Assert.assertThrows(InvalidParameterValueException.class, + () -> service.migrateGuestNetwork(network.getId(), 102L, accountMock, mock(User.class), false)); + + Assert.assertTrue(exception.getMessage().contains("Can't upgrade from network offering")); + Mockito.verify(networkModel).getNtwkOffDetails(101L); + Mockito.verify(networkModel).getNtwkOffDetails(102L); + Mockito.verifyNoInteractions(networkMigrationManager); + Mockito.verify(networkDao, Mockito.never()).update(anyLong(), any(NetworkVO.class)); + } + + @Test + public void testDifferentNsxSegmentProfilesRejectVpcMigrationBeforeMutation() { + NetworkVO network = prepareNsxProfileMigration(true); + VpcVO migratingVpc = mock(VpcVO.class); + when(migratingVpc.getVpcOfferingId()).thenReturn(401L); + when(vpcDao.findById(301L)).thenReturn(migratingVpc); + when(networkDao.listByVpc(301L)).thenReturn(List.of(network)); + NetworkOfferingVO newOffering = networkOfferingDao.findById(102L); + when(networkOfferingDao.findByUuid("new-nsx-offering")).thenReturn(newOffering); + when(configMgr.isOfferingForVpc(newOffering)).thenReturn(true); + + InvalidParameterValueException exception = Assert.assertThrows(InvalidParameterValueException.class, + () -> service.migrateVpcNetwork(301L, 402L, Map.of(network.getUuid(), "new-nsx-offering"), + accountMock, mock(User.class), false)); + + Assert.assertTrue(exception.getMessage().contains("Can't upgrade from network offering")); + Mockito.verify(networkModel).getNtwkOffDetails(101L); + Mockito.verify(networkModel).getNtwkOffDetails(102L); + Mockito.verifyNoInteractions(networkMigrationManager); + Mockito.verify(networkDao, Mockito.never()).update(anyLong(), any(NetworkVO.class)); + } + + @Test + public void testMatchingNsxSegmentProfilesAllowNetworkMigrationValidation() { + NetworkVO network = prepareNsxProfileMigration(false); + when(networkModel.getNtwkOffDetails(102L)).thenReturn(Map.of( + NetworkOffering.Detail.NsxSegmentSecurityProfileId, "old-security-profile")); + + Boolean needsMigration = ReflectionTestUtils.invokeMethod(service, "networkNeedsMigration", network, 501L, + networkOfferingDao.findByIdIncludingRemoved(101L), networkOfferingDao.findById(102L)); + + Assert.assertEquals(Boolean.TRUE, needsMigration); + } + + private NetworkVO prepareNsxProfileMigration(boolean forVpc) { + NetworkVO migratingNetwork = mock(NetworkVO.class); + NetworkOfferingVO oldOffering = mock(NetworkOfferingVO.class); + NetworkOfferingVO newOffering = mock(NetworkOfferingVO.class); + when(migratingNetwork.getId()).thenReturn(201L); + when(migratingNetwork.getRelated()).thenReturn(201L); + when(migratingNetwork.getUuid()).thenReturn("nsx-network"); + when(migratingNetwork.getVpcId()).thenReturn(forVpc ? 301L : null); + when(migratingNetwork.getNetworkOfferingId()).thenReturn(101L); + when(migratingNetwork.getDataCenterId()).thenReturn(10L); + when(migratingNetwork.getPhysicalNetworkId()).thenReturn(501L); + when(migratingNetwork.getTrafficType()).thenReturn(Networks.TrafficType.Guest); + when(migratingNetwork.getState()).thenReturn(Network.State.Implemented); + when(networkDao.findById(201L)).thenReturn(migratingNetwork); + when(oldOffering.getId()).thenReturn(101L); + when(newOffering.getId()).thenReturn(102L); + when(oldOffering.getUuid()).thenReturn("old-nsx-offering"); + when(newOffering.getUuid()).thenReturn("new-nsx-offering"); + when(oldOffering.getGuestType()).thenReturn(Network.GuestType.Isolated); + when(newOffering.getGuestType()).thenReturn(Network.GuestType.Isolated); + when(oldOffering.getTrafficType()).thenReturn(Networks.TrafficType.Guest); + when(newOffering.getTrafficType()).thenReturn(Networks.TrafficType.Guest); + when(newOffering.getState()).thenReturn(NetworkOffering.State.Enabled); + when(newOffering.getTags()).thenReturn("nsx"); + when(networkOfferingDao.findByIdIncludingRemoved(101L)).thenReturn(oldOffering); + when(networkOfferingDao.findById(102L)).thenReturn(newOffering); + when(networkModel.findPhysicalNetworkId(10L, "nsx", Networks.TrafficType.Guest)).thenReturn(501L); + when(networkModel.resolveProvider(Network.Provider.Nsx.getName())).thenReturn(Network.Provider.Nsx); + when(_networkMgr.finalizeServicesAndProvidersForNetwork(oldOffering, 501L)) + .thenReturn(Map.of(Network.Service.Firewall.getName(), Network.Provider.Nsx.getName())); + when(_networkMgr.finalizeServicesAndProvidersForNetwork(newOffering, 501L)) + .thenReturn(Map.of(Network.Service.Firewall.getName(), Network.Provider.Nsx.getName())); + when(ipAddressDao.listByAssociatedNetwork(201L, null)).thenReturn(List.of()); + when(vmInstanceDao.listNonRemovedVmsByTypeAndNetwork(201L, null)).thenReturn(List.of()); + when(networkModel.getNetworkOfferingServiceProvidersMap(102L)).thenReturn(new HashMap<>()); + when(networkModel.getNtwkOffDetails(101L)).thenReturn(Map.of( + NetworkOffering.Detail.NsxSegmentSecurityProfileId, "old-security-profile")); + when(networkModel.getNtwkOffDetails(102L)).thenReturn(Map.of( + NetworkOffering.Detail.NsxSegmentSecurityProfileId, "new-security-profile")); + service._networkMgr = _networkMgr; + service._networkMigrationManager = networkMigrationManager; + service._vmDao = vmInstanceDao; + service._resourceTagDao = resourceTagDao; + service._vpcOfferingDao = vpcOfferingDao; + return migratingNetwork; + } + @Test public void testCheckAndUpdateNetworkOfferingChangeReset() { NetworkVO networkVO = new NetworkVO(); diff --git a/ui/public/locales/en.json b/ui/public/locales/en.json index 23e7f41de927..8b60b2063e89 100644 --- a/ui/public/locales/en.json +++ b/ui/public/locales/en.json @@ -1932,6 +1932,9 @@ "label.nsx.provider.edgecluster": "NSX provider edge Cluster", "label.nsx.provider.tier0gateway": "NSX provider tier-0 gateway", "label.nsx.provider.transportzone": "NSX provider transport Zone", +"label.nsx.ip.discovery.profile.id": "NSX IP discovery profile ID", +"label.nsx.mac.discovery.profile.id": "NSX MAC discovery profile ID", +"label.nsx.segment.security.profile.id": "NSX segment security profile ID", "label.nsx.supports.internal.lb": "Enable NSX internal LB service", "label.nsx.supports.lb": "Enable NSX LB service", "label.num.cpu.cores": "# of CPU cores", @@ -4013,6 +4016,9 @@ "message.network.offering.mac.address.changes": "Applicable for guest Networks on VMware hypervisor only.\nReject - If the guest OS changes the effective MAC address of the Instance to a value that is different from the MAC address of the Instance Network adapter (set in the .vmx configuration file), the switch drops all inbound frames to the adapter.\nIf the guest OS changes the effective MAC address of the Instance back to the MAC address of the Instance Network adapter, the virtual machine receives frames again.\nAccept - If the guest OS changes the effective MAC address of the virtual machine to a value that is different from the MAC address of the Instance Network adapter, the switch allows frames to the new address to pass.\nNone - Default to value from global setting.", "message.network.offering.mac.learning": "Applicable for guest Networks on VMware hypervisor only with VMware Distributed Virtual Switches version 6.6.0 & above and vSphere version 6.7 & above.\nMAC learning enables Network connectivity for multiple MAC addresses behind a single vNIC.\nNone - Default to value from global setting.", "message.network.offering.mac.learning.warning": "WARNING: In order to use MAC Learning you must ensure your hypervisor hosts are running ESXi 6.7+ and the Network uses distributed vSwitch 6.6.0+.", +"message.network.offering.nsx.ip.discovery.profile.id": "ID of an existing NSX IP discovery profile to bind to segments created from this offering.", +"message.network.offering.nsx.mac.discovery.profile.id": "ID of an existing NSX MAC discovery profile to bind to segments created from this offering.", +"message.network.offering.nsx.segment.security.profile.id": "ID of an existing NSX segment security profile to bind to segments created from this offering.", "message.network.offering.promiscuous.mode": "Applicable for guest Networks on VMware hypervisor only.\nReject - The switch drops any outbound frame from a virtual machine adapter with a source MAC address that is different from the one in the .vmx configuration file.\nAccept - The switch does not perform filtering, and permits all outbound frames.\nNone - Default to value from global setting.", "message.network.removenic": "Please confirm that want to remove this NIC, which will also remove the associated Network from the Instance.", "message.network.restart.required": "Restart is required for network(s). Click here to view network(s) which require restart.", diff --git a/ui/src/views/offering/AddNetworkOffering.vue b/ui/src/views/offering/AddNetworkOffering.vue index 95d170412b89..ded5341401b6 100644 --- a/ui/src/views/offering/AddNetworkOffering.vue +++ b/ui/src/views/offering/AddNetworkOffering.vue @@ -160,6 +160,38 @@ + + + + + + + + + + + + + + + + + + + +