diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/vpc/CloneVPCOfferingCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/vpc/CloneVPCOfferingCmd.java index 284f60bf4e8..b4f9f4ef52f 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/vpc/CloneVPCOfferingCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/vpc/CloneVPCOfferingCmd.java @@ -16,6 +16,7 @@ // under the License. package org.apache.cloudstack.api.command.admin.vpc; +import com.cloud.exception.ResourceAllocationException; import com.cloud.network.vpc.VpcOffering; import org.apache.cloudstack.api.APICommand; import org.apache.cloudstack.api.ApiConstants; @@ -81,10 +82,23 @@ public class CloneVPCOfferingCmd extends CreateVPCOfferingCmd { /////////////// API Implementation/////////////////// ///////////////////////////////////////////////////// + @Override + public void create() throws ResourceAllocationException { + // Set a temporary entity ID (source offering ID) to prevent NullPointerException + // in ApiServer.queueCommand(). This will be updated in execute() with the actual + // cloned offering ID. + if (sourceOfferingId != null) { + setEntityId(sourceOfferingId); + } + } + @Override public void execute() { VpcOffering result = _vpcProvSvc.cloneVPCOffering(this); if (result != null) { + setEntityId(result.getId()); + setEntityUuid(result.getUuid()); + VpcOfferingResponse response = _responseGenerator.createVpcOfferingResponse(result); response.setResponseName(getCommandName()); this.setResponseObject(response); diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/vpc/CreateVPCOfferingCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/vpc/CreateVPCOfferingCmd.java index 6b425bc10d2..bf0f6aab5be 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/vpc/CreateVPCOfferingCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/vpc/CreateVPCOfferingCmd.java @@ -28,7 +28,6 @@ import java.util.Set; import java.util.stream.Collectors; import java.util.stream.Stream; -import com.cloud.exception.InvalidParameterValueException; import com.cloud.network.Network; import com.cloud.network.VirtualRouterProvider; import com.cloud.offering.NetworkOffering; @@ -179,9 +178,7 @@ public class CreateVPCOfferingCmd extends BaseAsyncCreateCmd { } public List getSupportedServices() { - if (!isExternalNetworkProvider() && CollectionUtils.isEmpty(supportedServices)) { - throw new InvalidParameterValueException("Supported services needs to be provided"); - } + // For external network providers, auto-populate services based on network mode if (isExternalNetworkProvider()) { supportedServices = new ArrayList<>(List.of( Dhcp.getName(), diff --git a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java index 38d5b702b4c..4f96fcf7f85 100644 --- a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java +++ b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java @@ -8332,7 +8332,6 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati Map> finalServiceProviderMap = resolveServiceProviderMap(cmd, sourceServiceProviderMap, finalServices); - // Reconstruct service capability list from source offering Map> sourceServiceCapabilityList = reconstructNetworkServiceCapabilityList(sourceOffering); Map sourceDetailsMap = getSourceOfferingDetails(sourceOfferingId); @@ -8366,25 +8365,38 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati List finalServices = new ArrayList<>(); for (Network.Service service : sourceServiceProviderMap.keySet()) { - // Gateway service is automatically added by createNetworkOffering if SourceNat is present - // It should not be explicitly included in supportedServices list if (service != Network.Service.Gateway) { finalServices.add(service.getName()); } } if (dropServices != null && !dropServices.isEmpty()) { - finalServices.removeAll(dropServices); - logger.debug("Dropped services from clone: {}", dropServices); + List normalizedDropServices = new ArrayList<>(); + for (String serviceName : dropServices) { + Network.Service service = Network.Service.getService(serviceName); + if (service == null) { + throw new InvalidParameterValueException("Invalid service name in dropServices: " + serviceName); + } + normalizedDropServices.add(service.getName()); + } + finalServices.removeAll(normalizedDropServices); + logger.debug("Dropped services from clone: {}", normalizedDropServices); } if (addServices != null && !addServices.isEmpty()) { - for (String service : addServices) { - if (!finalServices.contains(service)) { - finalServices.add(service); + List normalizedAddServices = new ArrayList<>(); + for (String serviceName : addServices) { + Network.Service service = Network.Service.getService(serviceName); + if (service == null) { + throw new InvalidParameterValueException("Invalid service name in addServices: " + serviceName); + } + String canonicalName = service.getName(); + if (!finalServices.contains(canonicalName)) { + finalServices.add(canonicalName); + normalizedAddServices.add(canonicalName); } } - logger.debug("Added services to clone: {}", addServices); + logger.debug("Added services to clone: {}", normalizedAddServices); } return finalServices; @@ -8423,13 +8435,10 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati setField(cmd, "supportedServices", finalServices); } if (cmd.getServiceProviders() == null || cmd.getServiceProviders().isEmpty()) { - // Convert to API parameter format: Map with HashMap values containing "service" and "provider" keys Map> apiFormatMap = convertToApiParameterFormat(finalServiceProviderMap); setField(cmd, "serviceProviderList", apiFormatMap); } - // Apply service capability list if not provided via request parameters - // Check if any servicecapabilitylist parameters were passed (e.g., servicecapabilitylist[0].service) boolean hasCapabilityParams = requestParams.keySet().stream() .anyMatch(key -> key.startsWith(ApiConstants.SERVICE_CAPABILITY_LIST)); @@ -8504,7 +8513,6 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati Map> capabilityList = new HashMap<>(); int index = 0; - // LB service capabilities if (sourceOffering.isDedicatedLB()) { Map cap = new HashMap<>(); cap.put("service", Network.Service.Lb.getName()); @@ -8544,8 +8552,6 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati capabilityList.put(String.valueOf(index++), cap); } - // SourceNat service capabilities - // Only add SupportedSourceNatTypes if explicitly set to perzone (sharedSourceNat=true) if (sourceOffering.isSharedSourceNat()) { Map cap = new HashMap<>(); cap.put("service", Network.Service.SourceNat.getName()); @@ -8554,7 +8560,6 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati capabilityList.put(String.valueOf(index++), cap); } - // Only add RedundantRouter capability when it's true if (sourceOffering.isRedundantRouter()) { Map cap1 = new HashMap<>(); cap1.put("service", Network.Service.SourceNat.getName()); @@ -8562,7 +8567,6 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati cap1.put("capabilityvalue", "true"); capabilityList.put(String.valueOf(index++), cap1); - // Also add to Gateway service only when redundantRouter is true Map cap2 = new HashMap<>(); cap2.put("service", Network.Service.Gateway.getName()); cap2.put("capabilitytype", Network.Capability.RedundantRouter.getName()); @@ -8570,7 +8574,6 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati capabilityList.put(String.valueOf(index++), cap2); } - // StaticNat service capabilities if (sourceOffering.isElasticIp()) { Map cap = new HashMap<>(); cap.put("service", Network.Service.StaticNat.getName()); @@ -8578,7 +8581,7 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati cap.put("capabilityvalue", "true"); capabilityList.put(String.valueOf(index++), cap); } - // AssociatePublicIP can only be set when ElasticIp is true + if (sourceOffering.isElasticIp() && sourceOffering.isAssociatePublicIP()) { Map cap = new HashMap<>(); cap.put("service", Network.Service.StaticNat.getName()); @@ -8592,16 +8595,14 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati public static void applyIfNotProvided(Object cmd, Map requestParams, String fieldName, String apiConstant, Object currentValue, Object sourceValue) throws Exception { - // If parameter was not provided in request and source has a value, use source value - if (!requestParams.containsKey(apiConstant) && sourceValue != null) { + if ((requestParams == null || !requestParams.containsKey(apiConstant)) && sourceValue != null) { setField(cmd, fieldName, sourceValue); } - // If parameter WAS provided in request, the framework already set it correctly } public static void applyBooleanIfNotProvided(Object cmd, Map requestParams, String fieldName, String apiConstant, Boolean sourceValue) throws Exception { - if (!requestParams.containsKey(apiConstant) && sourceValue != null) { + if ((requestParams == null || !requestParams.containsKey(apiConstant)) && sourceValue != null) { setField(cmd, fieldName, sourceValue); } } diff --git a/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java b/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java index dac643f19f5..4595ebd4c6b 100644 --- a/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java +++ b/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java @@ -632,6 +632,12 @@ public class VpcManagerImpl extends ManagerBase implements VpcManager, VpcProvis final String externalProvider, final NetworkOffering.NetworkMode networkMode, List domainIds, List zoneIds, State state, NetworkOffering.RoutingMode routingMode, boolean specifyAsNumber) { + boolean isExternalProvider = externalProvider != null && + Arrays.asList("NSX", "Netris").stream().anyMatch(s -> s.equalsIgnoreCase(externalProvider)); + if (!isExternalProvider && CollectionUtils.isEmpty(supportedServices)) { + throw new InvalidParameterValueException("Supported services needs to be provided"); + } + if (!Ipv6Service.Ipv6OfferingCreationEnabled.value() && !(internetProtocol == null || NetUtils.InternetProtocol.IPv4.equals(internetProtocol))) { throw new InvalidParameterValueException(String.format("Configuration %s needs to be enabled for creating IPv6 supported VPC offering", Ipv6Service.Ipv6OfferingCreationEnabled.key())); } @@ -920,14 +926,29 @@ public class VpcManagerImpl extends ManagerBase implements VpcManager, VpcProvis } if (dropServices != null && !dropServices.isEmpty()) { + List normalizedDropServices = new ArrayList<>(); + for (String serviceName : dropServices) { + Network.Service service = Network.Service.getService(serviceName); + if (service == null) { + throw new InvalidParameterValueException("Service " + serviceName + " is not supported in VPC"); + } + normalizedDropServices.add(service.getName()); + } finalServices.removeAll(dropServices); logger.debug("Dropped services from clone: {}", dropServices); } if (addServices != null && !addServices.isEmpty()) { - for (String service : addServices) { - if (!finalServices.contains(service)) { - finalServices.add(service); + List normalizedAddServices = new ArrayList<>(); + for (String serviceName : addServices) { + Network.Service service = Network.Service.getService(serviceName); + if (service == null) { + throw new InvalidParameterValueException("Service " + serviceName + " is not supported in VPC"); + } + String canonicalName = service.getName(); + if (!finalServices.contains(canonicalName)) { + finalServices.add(canonicalName); + normalizedAddServices.add(canonicalName); } } logger.debug("Added services to clone: {}", addServices); @@ -958,18 +979,47 @@ public class VpcManagerImpl extends ManagerBase implements VpcManager, VpcProvis return finalMap; } + /** + * Converts service provider map from Map> to the indexed format + * expected by CreateVPCOfferingCmd.serviceProviderList parameter. + * + * Input: {"Dhcp": ["VpcVirtualRouter"], "Dns": ["VpcVirtualRouter"]} + * Output: {"0": {"service": "Dhcp", "provider": "VpcVirtualRouter"}, + * "1": {"service": "Dns", "provider": "VpcVirtualRouter"}} + */ + private Map> convertToServiceProviderListFormat(Map> serviceProviderMap) { + Map> result = new HashMap<>(); + int index = 0; + + for (Map.Entry> entry : serviceProviderMap.entrySet()) { + String serviceName = entry.getKey(); + List providers = entry.getValue(); + + for (String providerName : providers) { + Map serviceProviderEntry = new HashMap<>(); + serviceProviderEntry.put("service", serviceName); + serviceProviderEntry.put("provider", providerName); + result.put(String.valueOf(index++), serviceProviderEntry); + } + } + + return result; + } + private void applyResolvedValuesToCommand(CloneVPCOfferingCmd cmd, VpcOfferingVO sourceOffering, List finalServices, Map finalServiceProviderMap, List sourceDomainIds, List sourceZoneIds, Map sourceServiceCapabilityList) { try { - Map requestParams = cmd.getFullUrlParams(); - if (cmd.getSupportedServices() == null || cmd.getSupportedServices().isEmpty()) { + logger.debug("Setting supportedServices to {} services from source offering", finalServices.size()); ConfigurationManagerImpl.setField(cmd, "supportedServices", finalServices); } + if (cmd.getServiceProviders() == null || cmd.getServiceProviders().isEmpty()) { - ConfigurationManagerImpl.setField(cmd, "serviceProviderList", finalServiceProviderMap); + Map> convertedProviderMap = convertToServiceProviderListFormat(finalServiceProviderMap); + logger.debug("Setting serviceProviderList with {} provider mappings", convertedProviderMap.size()); + ConfigurationManagerImpl.setField(cmd, "serviceProviderList", convertedProviderMap); } if ((cmd.getServiceCapabilityList() == null || cmd.getServiceCapabilityList().isEmpty()) @@ -977,34 +1027,46 @@ public class VpcManagerImpl extends ManagerBase implements VpcManager, VpcProvis ConfigurationManagerImpl.setField(cmd, "serviceCapabilityList", sourceServiceCapabilityList); } - ConfigurationManagerImpl.applyIfNotProvided(cmd, requestParams, "displayText", ApiConstants.DISPLAY_TEXT, cmd.getDisplayText(), sourceOffering.getDisplayText()); - ConfigurationManagerImpl.applyIfNotProvided(cmd, requestParams, "serviceOfferingId", ApiConstants.SERVICE_OFFERING_ID, cmd.getServiceOfferingId(), sourceOffering.getServiceOfferingId()); + if (cmd.getDisplayText() == null && sourceOffering.getDisplayText() != null) { + ConfigurationManagerImpl.setField(cmd, "displayText", sourceOffering.getDisplayText()); + } + if (cmd.getServiceOfferingId() == null && sourceOffering.getServiceOfferingId() != null) { + ConfigurationManagerImpl.setField(cmd, "serviceOfferingId", sourceOffering.getServiceOfferingId()); + } - ConfigurationManagerImpl.applyBooleanIfNotProvided(cmd, requestParams, "enable", ApiConstants.ENABLE, sourceOffering.getState() == VpcOffering.State.Enabled); - ConfigurationManagerImpl.applyBooleanIfNotProvided(cmd, requestParams, "specifyAsNumber", ApiConstants.SPECIFY_AS_NUMBER, sourceOffering.isSpecifyAsNumber()); + Boolean enableFieldValue = getRawFieldValue(cmd, "enable", Boolean.class); + if (enableFieldValue == null) { + Boolean enableState = sourceOffering.getState() == VpcOffering.State.Enabled; + ConfigurationManagerImpl.setField(cmd, "enable", enableState); + } - if (!requestParams.containsKey(ApiConstants.INTERNET_PROTOCOL)) { + Boolean specifyAsNumberFieldValue = getRawFieldValue(cmd, "specifyAsNumber", Boolean.class); + if (specifyAsNumberFieldValue == null) { + ConfigurationManagerImpl.setField(cmd, "specifyAsNumber", sourceOffering.isSpecifyAsNumber()); + } + + if (cmd.getInternetProtocol() == null) { String internetProtocol = vpcOfferingDetailsDao.getDetail(sourceOffering.getId(), ApiConstants.INTERNET_PROTOCOL); if (internetProtocol != null) { ConfigurationManagerImpl.setField(cmd, "internetProtocol", internetProtocol); } } - if (!requestParams.containsKey(ApiConstants.NETWORK_MODE) && sourceOffering.getNetworkMode() != null) { + if (cmd.getNetworkMode() == null && sourceOffering.getNetworkMode() != null) { ConfigurationManagerImpl.setField(cmd, "networkMode", sourceOffering.getNetworkMode().toString()); } - if (!requestParams.containsKey(ApiConstants.ROUTING_MODE) && sourceOffering.getRoutingMode() != null) { + if (cmd.getRoutingMode() == null && sourceOffering.getRoutingMode() != null) { ConfigurationManagerImpl.setField(cmd, "routingMode", sourceOffering.getRoutingMode().toString()); } - if (cmd.getDomainIds() == null || cmd.getDomainIds().isEmpty()) { if (sourceDomainIds != null && !sourceDomainIds.isEmpty()) { ConfigurationManagerImpl.setField(cmd, "domainIds", sourceDomainIds); } } + if (cmd.getZoneIds() == null || cmd.getZoneIds().isEmpty()) { if (sourceZoneIds != null && !sourceZoneIds.isEmpty()) { ConfigurationManagerImpl.setField(cmd, "zoneIds", sourceZoneIds); @@ -1012,10 +1074,27 @@ public class VpcManagerImpl extends ManagerBase implements VpcManager, VpcProvis } } catch (Exception e) { - logger.warn("Failed to apply some source offering parameters during clone: {}", e.getMessage()); + logger.error("Failed to apply source offering parameters during clone: {}", e.getMessage(), e); + throw new CloudRuntimeException("Failed to apply source offering parameters during VPC offering clone", e); } } + private T getRawFieldValue(Object obj, String fieldName, Class expectedType) { + try { + java.lang.reflect.Field field = ConfigurationManagerImpl.findField(obj.getClass(), fieldName); + if (field != null) { + field.setAccessible(true); + Object value = field.get(obj); + if (value == null || expectedType.isInstance(value)) { + return expectedType.cast(value); + } + } + } catch (Exception e) { + logger.debug("Could not get raw field value for {}: {}", fieldName, e.getMessage()); + } + return null; + } + private void validateConnectivtyServiceCapabilities(final Set providers, final Map serviceCapabilitystList) { if (serviceCapabilitystList != null && !serviceCapabilitystList.isEmpty()) { final Collection serviceCapabilityCollection = serviceCapabilitystList.values(); diff --git a/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java b/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java index 3bc7cd16c79..a9b620bbf9e 100644 --- a/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java @@ -1720,6 +1720,7 @@ public class BackupManagerImpl extends ManagerBase implements BackupManager { cmdList.add(ListBackupProvidersCmd.class); cmdList.add(ListBackupProviderOfferingsCmd.class); cmdList.add(ImportBackupOfferingCmd.class); + cmdList.add(CloneBackupOfferingCmd.class); cmdList.add(ListBackupOfferingsCmd.class); cmdList.add(DeleteBackupOfferingCmd.class); cmdList.add(UpdateBackupOfferingCmd.class);