Merge remote-tracking branch 'origin/4.18'

engine/components-api/src/main/java/com/cloud/network/IpAddressManager.java

@@ -56,6 +56,10 @@ public interface IpAddressManager {
             "Set placement of vrouter ips in redundant mode in vpc tiers, this can be 3 value: `first` to use first ips in tiers, `last` to use last ips in tiers and `random` to take random ips in tiers.",
             true, ConfigKey.Scope.Account, null, null, null, null, null, ConfigKey.Kind.Select, "first,last,random");
 
+    ConfigKey<Boolean> AllowUserListAvailableIpsOnSharedNetwork = new ConfigKey<Boolean>("Advanced", Boolean.class, "allow.user.list.available.ips.on.shared.network", "false",
+            "Determines whether users can list available IPs on shared networks",
+            true, ConfigKey.Scope.Global);
+
     /**
      * Assigns a new public ip address.
      *

server/src/main/java/com/cloud/network/IpAddressManagerImpl.java

@@ -2341,7 +2341,7 @@ public class IpAddressManagerImpl extends ManagerBase implements IpAddressManage
 
     @Override
     public ConfigKey<?>[] getConfigKeys() {
-        return new ConfigKey<?>[] {UseSystemPublicIps, RulesContinueOnError, SystemVmPublicIpReservationModeStrictness, VrouterRedundantTiersPlacement};
+        return new ConfigKey<?>[] {UseSystemPublicIps, RulesContinueOnError, SystemVmPublicIpReservationModeStrictness, VrouterRedundantTiersPlacement, AllowUserListAvailableIpsOnSharedNetwork};
     }
 
     /**

server/src/main/java/com/cloud/server/ManagementServerImpl.java

@@ -2322,6 +2322,7 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
                 isAllocated = Boolean.TRUE;
             }
         }
+        boolean isAllocatedTemp = isAllocated;
 
         VlanType vlanType = null;
         if (forVirtualNetwork != null) {
@@ -2332,6 +2333,7 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
 
         final Account caller = getCaller();
         List<IPAddressVO> addrs = new ArrayList<>();
+        NetworkVO network = null;   // shared network
 
         if (vlanType == VlanType.DirectAttached && networkId == null && ipId == null) { // only root admin can list public ips in all shared networks
             if (caller.getType() != Account.Type.ADMIN) {
@@ -2340,7 +2342,6 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
         } else if (vlanType == VlanType.DirectAttached) {
             // list public ip address on shared network
             // access control. admin: all Ips, domain admin/user: all Ips in shared network in the domain/sub-domain/user
-            NetworkVO network = null;
             if (networkId == null) {
                 IPAddressVO ip = _publicIpAddressDao.findById(ipId);
                 if (ip == null) {
@@ -2474,7 +2475,20 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
             for (IPAddressVO addr: freeAddrs) {
                 freeAddrIds.add(addr.getId());
             }
+        } else if (vlanType == VlanType.DirectAttached && network != null && !isAllocatedTemp && isAllocated) {
+            if (caller.getType() != Account.Type.ADMIN && !IpAddressManager.AllowUserListAvailableIpsOnSharedNetwork.value()) {
+                s_logger.debug("Non-admin users are not allowed to list available IPs on shared networks");
+            } else {
+                final SearchBuilder<IPAddressVO> searchBuilder = _publicIpAddressDao.createSearchBuilder();
+                buildParameters(searchBuilder, cmd, false);
+
+                SearchCriteria<IPAddressVO> searchCriteria = searchBuilder.create();
+                setParameters(searchCriteria, cmd, vlanType, false);
+                searchCriteria.setParameters("state", IpAddress.State.Free.name());
+                addrs.addAll(_publicIpAddressDao.search(searchCriteria, searchFilter)); // Free IPs on shared network
+            }
         }
+
         if (freeAddrIds.size() > 0) {
             final SearchBuilder<IPAddressVO> sb2 = _publicIpAddressDao.createSearchBuilder();
             buildParameters(sb2, cmd, false);

server/src/main/java/com/cloud/vm/UserVmManagerImpl.java

@@ -2772,13 +2772,18 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir
         final List<String> userReadOnlySettings = Stream.of(QueryService.UserVMReadOnlyDetails.value().split(","))
                 .map(item -> (item).trim())
                 .collect(Collectors.toList());
+        List<UserVmDetailVO> existingDetails = userVmDetailsDao.listDetails(id);
         if (cleanupDetails){
             if (caller != null && caller.getType() == Account.Type.ADMIN) {
-                userVmDetailsDao.removeDetails(id);
+                for (final UserVmDetailVO detail : existingDetails) {
+                    if (detail != null && detail.isDisplay()) {
+                        userVmDetailsDao.removeDetail(id, detail.getName());
+                    }
+                }
             } else {
-                for (final UserVmDetailVO detail : userVmDetailsDao.listDetails(id)) {
+                for (final UserVmDetailVO detail : existingDetails) {
                     if (detail != null && !userDenyListedSettings.contains(detail.getName())
-                            && !userReadOnlySettings.contains(detail.getName())) {
+                            && !userReadOnlySettings.contains(detail.getName()) && detail.isDisplay()) {
                         userVmDetailsDao.removeDetail(id, detail.getName());
                     }
                 }
@@ -2798,15 +2803,25 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir
                         if (userReadOnlySettings.contains(detailName)) {
                             throw new InvalidParameterValueException("You're not allowed to add or edit the read-only setting: " + detailName);
                         }
+                        if (existingDetails.stream().anyMatch(d -> Objects.equals(d.getName(), detailName) && !d.isDisplay())){
+                            throw new InvalidParameterValueException("You're not allowed to add or edit the non-displayable setting: " + detailName);
+                        }
                     }
-                    // Add any hidden/denied or read-only detail
-                    for (final UserVmDetailVO detail : userVmDetailsDao.listDetails(id)) {
+                    // Add any existing user denied or read-only details. We do it here because admins would already provide these (or can delete them).
+                    for (final UserVmDetailVO detail : existingDetails) {
                         if (userDenyListedSettings.contains(detail.getName()) || userReadOnlySettings.contains(detail.getName())) {
                             details.put(detail.getName(), detail.getValue());
                         }
                     }
                 }
 
+                // ensure details marked as non-displayable are maintained, regardless of admin or not
+                for (final UserVmDetailVO existingDetail : existingDetails) {
+                    if (!existingDetail.isDisplay()) {
+                        details.put(existingDetail.getName(), existingDetail.getValue());
+                    }
+                }
+
                 verifyVmLimits(vmInstance, details);
                 vmInstance.setDetails(details);
                 _vmDao.saveDetails(vmInstance);

server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java

@@ -162,7 +162,7 @@ public class UserVmManagerImplTest {
     private EntityManager entityManager;
 
     @Mock
-    private UserVmDetailsDao userVmDetailVO;
+    private UserVmDetailsDao userVmDetailsDao;
 
     @Mock
     private UserVmVO userVmVoMock;
@@ -333,7 +333,7 @@ public class UserVmManagerImplTest {
         verifyMethodsThatAreAlwaysExecuted();
 
         Mockito.verify(userVmManagerImpl).updateDisplayVmFlag(false, vmId, userVmVoMock);
-        Mockito.verify(userVmDetailVO, Mockito.times(0)).removeDetails(vmId);
+        Mockito.verify(userVmDetailsDao, Mockito.times(0)).removeDetail(anyLong(), anyString());
     }
 
     @Test
@@ -343,12 +343,15 @@ public class UserVmManagerImplTest {
         Mockito.when(_serviceOfferingDao.findById(Mockito.anyLong(), Mockito.anyLong())).thenReturn((ServiceOfferingVO) offering);
         Mockito.when(updateVmCommand.isCleanupDetails()).thenReturn(true);
         Mockito.lenient().doNothing().when(userVmManagerImpl).updateDisplayVmFlag(false, vmId, userVmVoMock);
-        Mockito.doNothing().when(userVmDetailVO).removeDetails(vmId);
+
         Mockito.when(updateVmCommand.getUserdataId()).thenReturn(null);
 
+        prepareExistingDetails(vmId, "userdetail");
+
         userVmManagerImpl.updateVirtualMachine(updateVmCommand);
         verifyMethodsThatAreAlwaysExecuted();
-        Mockito.verify(userVmDetailVO).removeDetails(vmId);
+        Mockito.verify(userVmDetailsDao).removeDetail(vmId, "userdetail");
+        Mockito.verify(userVmDetailsDao, Mockito.times(0)).removeDetail(vmId, "systemdetail");
         Mockito.verify(userVmManagerImpl, Mockito.times(0)).updateDisplayVmFlag(false, vmId, userVmVoMock);
     }
 
@@ -373,6 +376,16 @@ public class UserVmManagerImplTest {
         prepareAndExecuteMethodDealingWithDetails(false, false);
     }
 
+    private List<UserVmDetailVO> prepareExistingDetails(Long vmId, String... existingDetailKeys) {
+        List<UserVmDetailVO> existingDetails = new ArrayList<>();
+        for (String detail : existingDetailKeys) {
+            existingDetails.add(new UserVmDetailVO(vmId, detail, "foo", true));
+        }
+        existingDetails.add(new UserVmDetailVO(vmId, "systemdetail", "bar", false));
+        Mockito.when(userVmDetailsDao.listDetails(vmId)).thenReturn(existingDetails);
+        return existingDetails;
+    }
+
     private void prepareAndExecuteMethodDealingWithDetails(boolean cleanUpDetails, boolean isDetailsEmpty) throws ResourceUnavailableException, InsufficientCapacityException {
         configureDoNothingForMethodsThatWeDoNotWantToTest();
 
@@ -393,8 +406,9 @@ public class UserVmManagerImplTest {
         lenient().doNothing().when(_networkMgr).saveExtraDhcpOptions(anyString(), anyLong(), anyMap());
         HashMap<String, String> details = new HashMap<>();
         if(!isDetailsEmpty) {
-            details.put("", "");
+            details.put("newdetail", "foo");
         }
+        prepareExistingDetails(vmId, "existingdetail");
         Mockito.when(updateVmCommand.getUserdataId()).thenReturn(null);
         Mockito.when(updateVmCommand.getDetails()).thenReturn(details);
         Mockito.when(updateVmCommand.isCleanupDetails()).thenReturn(cleanUpDetails);
@@ -404,14 +418,15 @@ public class UserVmManagerImplTest {
         verifyMethodsThatAreAlwaysExecuted();
 
         Mockito.verify(userVmVoMock, Mockito.times(cleanUpDetails || isDetailsEmpty ? 0 : 1)).setDetails(details);
-        Mockito.verify(userVmDetailVO, Mockito.times(cleanUpDetails ? 1: 0)).removeDetails(vmId);
+        Mockito.verify(userVmDetailsDao, Mockito.times(cleanUpDetails ? 1 : 0)).removeDetail(vmId, "existingdetail");
+        Mockito.verify(userVmDetailsDao, Mockito.times(0)).removeDetail(vmId, "systemdetail");
         Mockito.verify(userVmDao, Mockito.times(cleanUpDetails || isDetailsEmpty ? 0 : 1)).saveDetails(userVmVoMock);
         Mockito.verify(userVmManagerImpl, Mockito.times(0)).updateDisplayVmFlag(false, vmId, userVmVoMock);
     }
 
     private void configureDoNothingForDetailsMethod() {
         Mockito.lenient().doNothing().when(userVmManagerImpl).updateDisplayVmFlag(false, vmId, userVmVoMock);
-        Mockito.doNothing().when(userVmDetailVO).removeDetails(vmId);
+        Mockito.doNothing().when(userVmDetailsDao).removeDetail(anyLong(), anyString());
         Mockito.doNothing().when(userVmDao).saveDetails(userVmVoMock);
     }
 

test/integration/component/test_update_vm.py

@@ -15,11 +15,11 @@
 # specific language governing permissions and limitations
 # under the License.
 
-
 from marvin.cloudstackTestCase import cloudstackTestCase
 from marvin.lib.base import Account, VirtualMachine, ServiceOffering
-from marvin.lib.utils import cleanup_resources
+from marvin.lib.utils import (validateList, cleanup_resources)
 from marvin.lib.common import get_zone, get_domain, get_template
+from marvin.codes import PASS
 from nose.plugins.attrib import attr
 
 class TestData(object):
@@ -59,6 +59,7 @@ class TestUpdateVirtualMachine(cloudstackTestCase):
     def setUp(self):
         self.testdata = TestData().testdata
         self.apiclient = self.testClient.getApiClient()
+        self.dbclient = self.testClient.getDbConnection()
 
         # Get Zone, Domain and Default Built-in template
         self.domain = get_domain(self.apiclient)
@@ -106,18 +107,7 @@ class TestUpdateVirtualMachine(cloudstackTestCase):
             templateid=self.template.id
         )
 
-        list_vms = VirtualMachine.list(self.apiclient, id=self.virtual_machine.id)
-        self.assertEqual(
-            isinstance(list_vms, list),
-            True,
-            "List VM response was not a valid list"
-        )
-        self.assertNotEqual(
-            len(list_vms),
-            0,
-            "List VM response was empty"
-        )
-        vm = list_vms[0]
+        vm = self.listVmById(self.virtual_machine.id)
 
         self.debug(
             "VirtualMachine launched with id, name, displayname: %s %s %s"\
@@ -152,6 +142,112 @@ class TestUpdateVirtualMachine(cloudstackTestCase):
         self.assertEqual(vmnew.displayname, vmnewstarted.displayname,
             msg="display name changed on start, displayname is %s" % vmnewstarted.displayname)
 
+    @attr(tags=['advanced', 'simulator', 'basic', 'sg', 'details'], required_hardware="false")
+    def test_update_vm_details_admin(self):
+        """Test Update VirtualMachine Details
+
+        # Set up a VM
+        # Set up hidden detail in DB for VM
+
+        # Validate the following:
+        # 1. Can add two details (detail1, detail2)
+        # 2. Can fetch new details on VM
+        # 3. Can delete detail1
+        # 4. Hidden detail not removed
+        # 6. The detail2 remains
+        # 7. Ensure cleanup parameter doesn't remove hidden details
+        """
+        hidden_detail_name = "configDriveLocation"
+        detail1 = "detail1"
+        detail2 = "detail2"
+
+        # set up a VM
+        self.virtual_machine = VirtualMachine.create(
+            self.apiclient,
+            self.testdata["virtual_machine"],
+            accountid=self.account.name,
+            zoneid=self.zone.id,
+            domainid=self.account.domainid,
+            serviceofferingid=self.service_offering.id,
+            templateid=self.template.id
+        )
+        self.cleanup.append(self.virtual_machine)
+
+        vm = self.listVmById(self.virtual_machine.id)
+
+        self.debug(
+            "VirtualMachine launched with id, name, displayname: %s %s %s" \
+            % (self.virtual_machine.id, vm.name, vm.displayname)
+        )
+
+        # set up a hidden detail
+        dbresult = self.dbclient.execute("select id from vm_instance where uuid='%s'" % vm.id)
+        self.assertEqual(validateList(dbresult)[0], PASS, "sql query returned invalid response")
+        vm_db_id = dbresult[0][0]
+        self.debug("VM has database id %d" % vm_db_id)
+
+        self.dbclient.execute("insert into user_vm_details (vm_id, name, value, display) values (%d,'%s','HOST', 0)" % (vm_db_id, hidden_detail_name))
+
+        vm = self.listVmById(self.virtual_machine.id)
+        self.debug("VirtualMachine fetched with details: %s of type %s" % (vm.details, type(vm.details)))
+
+        self.assertIsNone(vm.details[hidden_detail_name], "hidden detail should be hidden")
+
+        # add two details by appending to what was returned via API
+        updating_vm_details = vm.details.__dict__
+        updating_vm_details[detail1] = "foo"
+        updating_vm_details[detail2] = "bar"
+
+        self.debug("Updating VM to new details: %s" % updating_vm_details)
+        vm = self.virtual_machine.update(self.apiclient, details=[updating_vm_details])
+
+        self.assertIsNotNone(vm.details[detail1], "Expect " + detail1)
+        self.assertIsNotNone(vm.details[detail2], "Expect " + detail2)
+        self.assertIsNone(vm.details[hidden_detail_name], "hidden detail should be hidden")
+        self.assertTrue(self.detailInDatabase(vm_db_id, hidden_detail_name), "hidden detail should still exist in db")
+
+        # delete one detail
+        updating_vm_details = vm.details.__dict__
+        del updating_vm_details["detail1"]
+
+        self.debug("Deleting one detail by updating details: %s" % updating_vm_details)
+        vm = self.virtual_machine.update(self.apiclient, details=[updating_vm_details])
+
+        self.assertIsNone(vm.details[detail1], "Do not expect " + detail1)
+        self.assertIsNotNone(vm.details[detail2], "Expect " + detail2)
+        self.assertIsNone(vm.details[hidden_detail_name], "hidden detail should be hidden")
+        self.assertTrue(self.detailInDatabase(vm_db_id, hidden_detail_name), "hidden detail should still exist in db")
+
+        # cleanup, ensure hidden detail is not deleted
+        vm = self.virtual_machine.update(self.apiclient, cleanupdetails="true")
+        self.assertIsNone(vm.details[detail1], "Do not expect " + detail1)
+        self.assertIsNone(vm.details[detail2], "Do not expect " + detail2)
+        self.assertIsNone(vm.details[hidden_detail_name], "hidden detail should be hidden")
+        self.assertTrue(self.detailInDatabase(vm_db_id, hidden_detail_name), "hidden detail should still exist in db")
+
+
+    def detailInDatabase(self, vm_id, detail_name):
+        dbresult = self.dbclient.execute("select id from user_vm_details where vm_id=%s and name='%s'" % (vm_id, detail_name))
+        self.debug("Detail %s for VM %s: %s" % (detail_name, vm_id, dbresult))
+        if validateList(dbresult)[0] == PASS:
+            return True
+        return False
+
+
+    def listVmById(self, id):
+        list_vms = VirtualMachine.list(self.apiclient, id=id)
+        self.assertEqual(
+            isinstance(list_vms, list),
+            True,
+            "List VM response was not a valid list"
+        )
+        self.assertNotEqual(
+            len(list_vms),
+            0,
+            "List VM response was empty"
+        )
+        return list_vms[0]
+
     def tearDown(self):
         try:
            cleanup_resources(self.apiclient, self.cleanup)

ui/src/components/header/HeaderNotice.vue

@@ -109,7 +109,7 @@ export default {
       (state, getters) => getters.headerNotices,
       (newValue, oldValue) => {
         if (oldValue !== newValue && newValue !== undefined) {
-          this.notices = newValue.reverse()
+          this.notices = newValue
         }
       }
     )

ui/src/components/header/ProjectMenu.vue

@@ -90,9 +90,8 @@ export default {
             getNextPage()
           }
         }).finally(() => {
-          this.projects = _.orderBy(projects, ['displaytext'], ['asc'])
-          this.projects.unshift({ name: this.$t('label.default.view') })
           this.loading = false
+          this.$store.commit('RELOAD_ALL_PROJECTS', projects)
         })
       }
       getNextPage()
@@ -113,6 +112,17 @@ export default {
     filterProject (input, option) {
       return option.label.toLowerCase().indexOf(input.toLowerCase()) >= 0
     }
+  },
+  mounted () {
+    this.$store.watch(
+      (state, getters) => getters.allProjects,
+      (newValue, oldValue) => {
+        if (oldValue !== newValue && newValue !== undefined) {
+          this.projects = _.orderBy(newValue, ['displaytext'], ['asc'])
+          this.projects.unshift({ name: this.$t('label.default.view') })
+        }
+      }
+    )
   }
 }
 </script>

ui/src/store/getters.js

@@ -49,6 +49,7 @@ const getters = {
   twoFaProvider: state => state.user.twoFaProvider,
   twoFaIssuer: state => state.user.twoFaIssuer,
   loginFlag: state => state.user.loginFlag,
+  allProjects: (state) => state.app.allProjects,
   customHypervisorName: state => state.user.customHypervisorName
 }
 

ui/src/store/modules/app.js

@@ -30,7 +30,8 @@ import {
   USE_BROWSER_TIMEZONE,
   SERVER_MANAGER,
   VUE_VERSION,
-  CUSTOM_COLUMNS
+  CUSTOM_COLUMNS,
+  RELOAD_ALL_PROJECTS
 } from '@/store/mutation-types'
 
 const app = {
@@ -50,7 +51,8 @@ const app = {
     metrics: false,
     listAllProjects: false,
     server: '',
-    vueVersion: ''
+    vueVersion: '',
+    allProjects: []
   },
   mutations: {
     SET_SIDEBAR_TYPE: (state, type) => {
@@ -122,6 +124,10 @@ const app = {
       vueProps.$localStorage.set(CUSTOM_COLUMNS, customColumns)
       state.customColumns = customColumns
     },
+    RELOAD_ALL_PROJECTS: (state, allProjects = []) => {
+      vueProps.$localStorage.set(RELOAD_ALL_PROJECTS, allProjects)
+      state.allProjects = allProjects
+    },
     SET_SHUTDOWN_TRIGGERED: (state, shutdownTriggered) => {
       state.shutdownTriggered = shutdownTriggered
     }
@@ -181,6 +187,9 @@ const app = {
     SetCustomColumns ({ commit }, bool) {
       commit('SET_CUSTOM_COLUMNS', bool)
     },
+    ReloadAllProjects ({ commit, allProjects }) {
+      commit('RELOAD_ALL_PROJECTS', allProjects)
+    },
     SetShutdownTriggered ({ commit }, bool) {
       commit('SET_SHUTDOWN_TRIGGERED', bool)
     }

ui/src/store/modules/user.js

@@ -365,9 +365,13 @@ const user = {
       if (noticeIdx === -1) {
         noticeArray.push(noticeJson)
       } else {
+        const existingNotice = noticeArray[noticeIdx]
+        noticeJson.timestamp = existingNotice.timestamp
         noticeArray[noticeIdx] = noticeJson
       }
-
+      noticeArray.sort(function (a, b) {
+        return new Date(b.timestamp) - new Date(a.timestamp)
+      })
       commit('SET_HEADER_NOTICES', noticeArray)
     },
     ProjectView ({ commit }, projectid) {

ui/src/utils/plugins.js

@@ -65,7 +65,8 @@ export const pollJobPlugin = {
         key: jobId,
         title,
         description,
-        status: 'progress'
+        status: 'progress',
+        timestamp: new Date()
       })
 
       eventBus.on('update-job-details', (args) => {
@@ -107,7 +108,8 @@ export const pollJobPlugin = {
             title,
             description,
             status: 'done',
-            duration: 2
+            duration: 2,
+            timestamp: new Date()
           })
           eventBus.emit('update-job-details', { jobId, resourceId })
           // Ensure we refresh on the same / parent page
@@ -157,7 +159,8 @@ export const pollJobPlugin = {
             title,
             description: desc,
             status: 'failed',
-            duration: 2
+            duration: 2,
+            timestamp: new Date()
           })
           eventBus.emit('update-job-details', { jobId, resourceId })
           // Ensure we refresh on the same / parent page

ui/src/views/AutogenView.vue

@@ -718,8 +718,10 @@ export default {
       }
       api('listProjects', { id: projectId, listall: true, details: 'min' }).then(json => {
         if (!json || !json.listprojectsresponse || !json.listprojectsresponse.project) return
+        const projects = json.listprojectsresponse.project
         const project = json.listprojectsresponse.project[0]
         this.$store.dispatch('SetProject', project)
+        this.$store.commit('RELOAD_ALL_PROJECTS', projects)
         this.$store.dispatch('ToggleTheme', project.id === undefined ? 'light' : 'dark')
         this.$message.success(`${this.$t('message.switch.to')} "${project.name}"`)
         const query = Object.assign({}, this.$route.query)
@@ -963,6 +965,7 @@ export default {
         }
 
         if (this.apiName === 'listProjects' && this.items.length > 0) {
+          this.$store.commit('RELOAD_ALL_PROJECTS', this.items)
           this.columns.map(col => {
             if (col.title === 'Account') {
               col.title = this.$t('label.project.owner')