api,server,ui: allow cleaning up external details for host and serviceoffering (#11548)

api/src/main/java/org/apache/cloudstack/api/ApiConstants.java

@@ -1161,6 +1161,7 @@ public class ApiConstants {
     public static final String OVM3_CLUSTER = "ovm3cluster";
     public static final String OVM3_VIP = "ovm3vip";
     public static final String CLEAN_UP_DETAILS = "cleanupdetails";
+    public static final String CLEAN_UP_EXTERNAL_DETAILS = "cleanupexternaldetails";
     public static final String CLEAN_UP_PARAMETERS = "cleanupparameters";
     public static final String VIRTUAL_SIZE = "virtualsize";
     public static final String NETSCALER_CONTROLCENTER_ID = "netscalercontrolcenterid";

api/src/main/java/org/apache/cloudstack/api/command/admin/host/UpdateHostCmd.java

@@ -72,6 +72,14 @@ public class UpdateHostCmd extends BaseCmd {
     @Parameter(name = ApiConstants.EXTERNAL_DETAILS, type = CommandType.MAP, description = "Details in key/value pairs using format externaldetails[i].keyname=keyvalue. Example: externaldetails[0].endpoint.url=urlvalue", since = "4.21.0")
     protected Map externalDetails;
 
+    @Parameter(name = ApiConstants.CLEAN_UP_EXTERNAL_DETAILS,
+            type = CommandType.BOOLEAN,
+            description = "Optional boolean field, which indicates if external details should be cleaned up or not " +
+                    "(If set to true, external details removed for this host, externaldetails field ignored; " +
+                    "if false or not set, no action)",
+            since = "4.22.0")
+    protected Boolean cleanupExternalDetails;
+
     /////////////////////////////////////////////////////
     /////////////////// Accessors ///////////////////////
     /////////////////////////////////////////////////////
@@ -112,6 +120,10 @@ public class UpdateHostCmd extends BaseCmd {
         return convertExternalDetailsToMap(externalDetails);
     }
 
+    public boolean isCleanupExternalDetails() {
+        return Boolean.TRUE.equals(cleanupExternalDetails);
+    }
+
     /////////////////////////////////////////////////////
     /////////////// API Implementation///////////////////
     /////////////////////////////////////////////////////

api/src/main/java/org/apache/cloudstack/api/command/admin/offering/UpdateServiceOfferingCmd.java

@@ -101,6 +101,14 @@ public class UpdateServiceOfferingCmd extends BaseCmd {
             since = "4.21.0")
     private Map externalDetails;
 
+    @Parameter(name = ApiConstants.CLEAN_UP_EXTERNAL_DETAILS,
+            type = CommandType.BOOLEAN,
+            description = "Optional boolean field, which indicates if external details should be cleaned up or not " +
+                    "(If set to true, external details removed for this offering, externaldetails field ignored; " +
+                    "if false or not set, no action)",
+            since = "4.22.0")
+    protected Boolean cleanupExternalDetails;
+
     /////////////////////////////////////////////////////
     /////////////////// Accessors ///////////////////////
     /////////////////////////////////////////////////////
@@ -205,6 +213,10 @@ public class UpdateServiceOfferingCmd extends BaseCmd {
         return convertExternalDetailsToMap(externalDetails);
     }
 
+    public boolean isCleanupExternalDetails() {
+        return Boolean.TRUE.equals(cleanupExternalDetails);
+    }
+
     /////////////////////////////////////////////////////
     /////////////// API Implementation///////////////////
     /////////////////////////////////////////////////////

engine/schema/src/main/java/com/cloud/host/dao/HostDetailsDao.java

@@ -33,6 +33,8 @@ public interface HostDetailsDao extends GenericDao<DetailVO, Long> {
 
     List<DetailVO> findByName(String name);
 
+    void removeExternalDetails(long hostId);
+
     void replaceExternalDetails(long hostId, Map<String, String> details);
 
 }

engine/schema/src/main/java/com/cloud/host/dao/HostDetailsDaoImpl.java

@@ -39,6 +39,7 @@ public class HostDetailsDaoImpl extends GenericDaoBase<DetailVO, Long> implement
     protected final SearchBuilder<DetailVO> HostSearch;
     protected final SearchBuilder<DetailVO> DetailSearch;
     protected final SearchBuilder<DetailVO> DetailNameSearch;
+    protected final SearchBuilder<DetailVO> ExternalDetailSearch;
 
     public HostDetailsDaoImpl() {
         HostSearch = createSearchBuilder();
@@ -53,6 +54,11 @@ public class HostDetailsDaoImpl extends GenericDaoBase<DetailVO, Long> implement
         DetailNameSearch = createSearchBuilder();
         DetailNameSearch.and("name", DetailNameSearch.entity().getName(), SearchCriteria.Op.EQ);
         DetailNameSearch.done();
+
+        ExternalDetailSearch = createSearchBuilder();
+        ExternalDetailSearch.and("hostId", ExternalDetailSearch.entity().getHostId(), SearchCriteria.Op.EQ);
+        ExternalDetailSearch.and("name", ExternalDetailSearch.entity().getName(), SearchCriteria.Op.LIKE);
+        ExternalDetailSearch.done();
     }
 
     @Override
@@ -133,6 +139,17 @@ public class HostDetailsDaoImpl extends GenericDaoBase<DetailVO, Long> implement
         return listBy(sc);
     }
 
+    @Override
+    public void removeExternalDetails(long hostId) {
+        TransactionLegacy txn = TransactionLegacy.currentTxn();
+        txn.start();
+        SearchCriteria<DetailVO> sc = ExternalDetailSearch.create();
+        sc.setParameters("hostId", hostId);
+        sc.setParameters("name", VmDetailConstants.EXTERNAL_DETAIL_PREFIX + "%");
+        remove(sc);
+        txn.commit();
+    }
+
     @Override
     public void replaceExternalDetails(long hostId, Map<String, String> details) {
         if (details.isEmpty()) {
@@ -149,11 +166,7 @@ public class HostDetailsDaoImpl extends GenericDaoBase<DetailVO, Long> implement
             }
             detailVOs.add(new DetailVO(hostId, name, value));
         }
-        SearchBuilder<DetailVO> sb = createSearchBuilder();
+        SearchCriteria<DetailVO> sc = ExternalDetailSearch.create();
-        sb.and("hostId", sb.entity().getHostId(), SearchCriteria.Op.EQ);
-        sb.and("name", sb.entity().getName(), SearchCriteria.Op.LIKE);
-        sb.done();
-        SearchCriteria<DetailVO> sc = sb.create();
         sc.setParameters("hostId", hostId);
         sc.setParameters("name", VmDetailConstants.EXTERNAL_DETAIL_PREFIX + "%");
         remove(sc);

framework/extensions/src/main/java/org/apache/cloudstack/framework/extensions/api/UpdateExtensionCmd.java

@@ -74,7 +74,7 @@ public class UpdateExtensionCmd extends BaseCmd {
     @Parameter(name = ApiConstants.CLEAN_UP_DETAILS,
             type = CommandType.BOOLEAN,
             description = "Optional boolean field, which indicates if details should be cleaned up or not " +
-                    "(If set to true, details removed for this action, details field ignored; " +
+                    "(If set to true, details removed for this extension, details field ignored; " +
                     "if false or not set, no action)")
     private Boolean cleanupDetails;
 

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

@@ -3807,8 +3807,8 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
     }
 
     protected boolean serviceOfferingExternalDetailsNeedUpdate(final Map<String, String> offeringDetails,
-               final Map<String, String> externalDetails) {
+               final Map<String, String> externalDetails, final boolean cleanupExternalDetails) {
-        if (MapUtils.isEmpty(externalDetails)) {
+        if (MapUtils.isEmpty(externalDetails) && !cleanupExternalDetails) {
             return false;
         }
 
@@ -3816,6 +3816,10 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
                 .filter(detail -> detail.getKey().startsWith(VmDetailConstants.EXTERNAL_DETAIL_PREFIX))
                 .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
 
+       if (cleanupExternalDetails) {
+           return !MapUtils.isEmpty(existingExternalDetails);
+       }
+
         if (MapUtils.isEmpty(existingExternalDetails) || existingExternalDetails.size() != externalDetails.size()) {
             return true;
         }
@@ -3845,6 +3849,7 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
         ServiceOffering.State state = cmd.getState();
         boolean purgeResources = cmd.isPurgeResources();
         final Map<String, String> externalDetails = cmd.getExternalDetails();
+        final boolean cleanupExternalDetails = cmd.isCleanupExternalDetails();
 
         if (userId == null) {
             userId = Long.valueOf(User.UID_SYSTEM);
@@ -3938,7 +3943,7 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
 
         final boolean updateNeeded = name != null || displayText != null || sortKey != null || storageTags != null || hostTags != null || state != null;
         final boolean serviceOfferingExternalDetailsNeedUpdate =
-                serviceOfferingExternalDetailsNeedUpdate(offeringDetails, externalDetails);
+                serviceOfferingExternalDetailsNeedUpdate(offeringDetails, externalDetails, cleanupExternalDetails);
         final boolean detailsUpdateNeeded = !filteredDomainIds.equals(existingDomainIds) ||
                 !filteredZoneIds.equals(existingZoneIds) || purgeResources != existingPurgeResources ||
                 serviceOfferingExternalDetailsNeedUpdate;
@@ -4013,8 +4018,10 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
                 SearchCriteria<ServiceOfferingDetailsVO> externalDetailsRemoveSC = sb.create();
                 externalDetailsRemoveSC.setParameters("detailNameLike", VmDetailConstants.EXTERNAL_DETAIL_PREFIX + "%");
                 _serviceOfferingDetailsDao.remove(externalDetailsRemoveSC);
-                for (Map.Entry<String, String> entry : externalDetails.entrySet()) {
+                if (!cleanupExternalDetails) {
-                    detailsVO.add(new ServiceOfferingDetailsVO(id, entry.getKey(), entry.getValue(), true));
+                    for (Map.Entry<String, String> entry : externalDetails.entrySet()) {
+                        detailsVO.add(new ServiceOfferingDetailsVO(id, entry.getKey(), entry.getValue(), true));
+                    }
                 }
             }
         }

server/src/main/java/com/cloud/resource/ResourceManagerImpl.java

@@ -2799,12 +2799,15 @@ public class ResourceManagerImpl extends ManagerBase implements ResourceManager,
 
     @Override
     public Host updateHost(final UpdateHostCmd cmd) throws NoTransitionException {
-        return updateHost(cmd.getId(), cmd.getName(), cmd.getOsCategoryId(),
+        return updateHost(cmd.getId(), cmd.getName(), cmd.getOsCategoryId(), cmd.getAllocationState(), cmd.getUrl(),
-                cmd.getAllocationState(), cmd.getUrl(), cmd.getHostTags(), cmd.getIsTagARule(), cmd.getAnnotation(), false, cmd.getExternalDetails());
+                cmd.getHostTags(), cmd.getIsTagARule(), cmd.getAnnotation(), false,
+                cmd.getExternalDetails(), cmd.isCleanupExternalDetails());
     }
 
     private Host updateHost(Long hostId, String name, Long guestOSCategoryId, String allocationState,
-                            String url, List<String> hostTags, Boolean isTagARule, String annotation, boolean isUpdateFromHostHealthCheck, Map<String, String> externalDetails) throws NoTransitionException {
+            String url, List<String> hostTags, Boolean isTagARule, String annotation,
+            boolean isUpdateFromHostHealthCheck, Map<String, String> externalDetails,
+            boolean cleanupExternalDetails) throws NoTransitionException {
         // Verify that the host exists
         final HostVO host = _hostDao.findById(hostId);
         if (host == null) {
@@ -2828,8 +2831,12 @@ public class ResourceManagerImpl extends ManagerBase implements ResourceManager,
             updateHostTags(host, hostId, hostTags, isTagARule);
         }
 
-        if (MapUtils.isNotEmpty(externalDetails)) {
+        if (cleanupExternalDetails) {
-            _hostDetailsDao.replaceExternalDetails(hostId, externalDetails);
+            _hostDetailsDao.removeExternalDetails(hostId);
+        } else {
+            if (MapUtils.isNotEmpty(externalDetails)) {
+                _hostDetailsDao.replaceExternalDetails(hostId, externalDetails);
+            }
         }
 
         if (url != null) {
@@ -2888,7 +2895,7 @@ public class ResourceManagerImpl extends ManagerBase implements ResourceManager,
 
     @Override
     public Host autoUpdateHostAllocationState(Long hostId, ResourceState.Event resourceEvent) throws NoTransitionException {
-        return updateHost(hostId, null, null, resourceEvent.toString(), null, null, null, null, true, null);
+        return updateHost(hostId, null, null, resourceEvent.toString(), null, null, null, null, true, null, false);
     }
 
     @Override

server/src/test/java/com/cloud/configuration/ConfigurationManagerImplTest.java

@@ -1028,17 +1028,47 @@ public class ConfigurationManagerImplTest {
         Map<String, String> offeringDetails = Map.of("key1", "value1");
         Map<String, String> externalDetails = Collections.emptyMap();
 
-        boolean result = configurationManagerImplSpy.serviceOfferingExternalDetailsNeedUpdate(offeringDetails, externalDetails);
+        boolean result = configurationManagerImplSpy.serviceOfferingExternalDetailsNeedUpdate(offeringDetails, externalDetails, false);
 
         Assert.assertFalse(result);
     }
 
+    @Test
+    public void serviceOfferingExternalDetailsNeedUpdateReturnsFalseWhenExternalDetailsIsEmptyAndCleanupTrue() {
+        Map<String, String> offeringDetails = Map.of("key1", "value1");
+        Map<String, String> externalDetails = Collections.emptyMap();
+
+        boolean result = configurationManagerImplSpy.serviceOfferingExternalDetailsNeedUpdate(offeringDetails, externalDetails, true);
+
+        Assert.assertFalse(result);
+    }
+
+    @Test
+    public void serviceOfferingExternalDetailsNeedUpdateReturnsTrueWhenExistingDetailsExistExternalDetailsIsEmptyAndCleanupTrue() {
+        Map<String, String> offeringDetails = Map.of("External:key1", "value1");
+        Map<String, String> externalDetails = Collections.emptyMap();
+
+        boolean result = configurationManagerImplSpy.serviceOfferingExternalDetailsNeedUpdate(offeringDetails, externalDetails, true);
+
+        Assert.assertTrue(result);
+    }
+
+    @Test
+    public void serviceOfferingExternalDetailsNeedUpdateReturnsTrueWhenExistingExternalDetailsExistValidExternalDetailsAndCleanupTrue() {
+        Map<String, String> offeringDetails = Map.of("External:key1", "value1");
+        Map<String, String> externalDetails = Collections.emptyMap();
+
+        boolean result = configurationManagerImplSpy.serviceOfferingExternalDetailsNeedUpdate(offeringDetails, externalDetails, true);
+
+        Assert.assertTrue(result);
+    }
+
     @Test
     public void serviceOfferingExternalDetailsNeedUpdateReturnsTrueWhenExistingExternalDetailsIsEmpty() {
         Map<String, String> offeringDetails = Map.of("key1", "value1");
         Map<String, String> externalDetails = Map.of("External:key1", "value1");
 
-        boolean result = configurationManagerImplSpy.serviceOfferingExternalDetailsNeedUpdate(offeringDetails, externalDetails);
+        boolean result = configurationManagerImplSpy.serviceOfferingExternalDetailsNeedUpdate(offeringDetails, externalDetails, false);
 
         Assert.assertTrue(result);
     }
@@ -1048,7 +1078,7 @@ public class ConfigurationManagerImplTest {
         Map<String, String> offeringDetails = Map.of("External:key1", "value1");
         Map<String, String> externalDetails = Map.of("External:key1", "value1", "External:key2", "value2");
 
-        boolean result = configurationManagerImplSpy.serviceOfferingExternalDetailsNeedUpdate(offeringDetails, externalDetails);
+        boolean result = configurationManagerImplSpy.serviceOfferingExternalDetailsNeedUpdate(offeringDetails, externalDetails, false);
 
         Assert.assertTrue(result);
     }
@@ -1058,7 +1088,7 @@ public class ConfigurationManagerImplTest {
         Map<String, String> offeringDetails = Map.of("External:key1", "value1");
         Map<String, String> externalDetails = Map.of("External:key1", "differentValue");
 
-        boolean result = configurationManagerImplSpy.serviceOfferingExternalDetailsNeedUpdate(offeringDetails, externalDetails);
+        boolean result = configurationManagerImplSpy.serviceOfferingExternalDetailsNeedUpdate(offeringDetails, externalDetails, false);
 
         Assert.assertTrue(result);
     }
@@ -1068,7 +1098,7 @@ public class ConfigurationManagerImplTest {
         Map<String, String> offeringDetails = Map.of("External:key1", "value1", "External:key2", "value2");
         Map<String, String> externalDetails = Map.of("External:key1", "value1", "External:key2", "value2");
 
-        boolean result = configurationManagerImplSpy.serviceOfferingExternalDetailsNeedUpdate(offeringDetails, externalDetails);
+        boolean result = configurationManagerImplSpy.serviceOfferingExternalDetailsNeedUpdate(offeringDetails, externalDetails, false);
 
         Assert.assertFalse(result);
     }

ui/src/views/AutogenView.vue

@@ -1770,9 +1770,22 @@ export default {
                 params[key] = param.opts[input].name
               }
             } else if (param.type === 'map' && typeof input === 'object') {
-              Object.entries(values.externaldetails).forEach(([key, value]) => {
+              const details = values[key]
-                params[param.name + '[0].' + key] = value
+              if (details && Object.keys(details).length > 0) {
-              })
+                Object.entries(details).forEach(([k, v]) => {
+                  params[key + '[0].' + k] = v
+                })
+              } else {
+                if (['details', 'externaldetails'].includes(key)) {
+                  const updateApiParams = this.$getApiParams(action.api)
+                  const cleanupKey = 'cleanup' + key
+                  if (cleanupKey in updateApiParams) {
+                    params[cleanupKey] = true
+                    break
+                  }
+                }
+                params[key] = {}
+              }
             } else {
               params[key] = input
             }

ui/src/views/infra/HostUpdate.vue

@@ -198,10 +198,12 @@ export default {
         if (values.istagarule !== undefined) {
           params.istagarule = values.istagarule
         }
-        if (values.externaldetails) {
+        if (values.externaldetails && Object.keys(values.externaldetails).length > 0) {
           Object.entries(values.externaldetails).forEach(([key, value]) => {
             params['externaldetails[0].' + key] = value
           })
+        } else {
+          params.cleanupexternaldetails = true
         }
         this.loading = true