From 2e1c2821a8234530bebfc288177753d010c4bcb0 Mon Sep 17 00:00:00 2001 From: Charles Queiroz Date: Thu, 24 Aug 2023 09:13:47 +0100 Subject: [PATCH 1/4] UI: Add central project store and watch functionality (#7900) Added a new getter 'allProjects' and mutation 'RELOAD_ALL_PROJECTS' to centralize the tracking of available projects in the state. This eliminates direct manipulation of the Project list in separate components and improves data consistency across the application. A watcher in ProjectMenu.vue has been implemented to handle changes to the 'allProjects' getter. The 'RELOAD_ALL_PROJECTS' mutation was also added where necessary to ensure projects list is updated in the state whenever changes occur. --- ui/src/components/header/ProjectMenu.vue | 14 ++++++++++++-- ui/src/store/getters.js | 3 ++- ui/src/store/modules/app.js | 13 +++++++++++-- ui/src/views/AutogenView.vue | 4 +++- 4 files changed, 28 insertions(+), 6 deletions(-) diff --git a/ui/src/components/header/ProjectMenu.vue b/ui/src/components/header/ProjectMenu.vue index 95a12ffeb1d..9182303c765 100644 --- a/ui/src/components/header/ProjectMenu.vue +++ b/ui/src/components/header/ProjectMenu.vue @@ -102,9 +102,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() @@ -125,6 +124,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') }) + } + } + ) } } diff --git a/ui/src/store/getters.js b/ui/src/store/getters.js index f6a6dab8ad2..0273fd02b19 100644 --- a/ui/src/store/getters.js +++ b/ui/src/store/getters.js @@ -47,7 +47,8 @@ const getters = { twoFaEnabled: state => state.user.twoFaEnabled, twoFaProvider: state => state.user.twoFaProvider, twoFaIssuer: state => state.user.twoFaIssuer, - loginFlag: state => state.user.loginFlag + loginFlag: state => state.user.loginFlag, + allProjects: (state) => state.app.allProjects } export default getters diff --git a/ui/src/store/modules/app.js b/ui/src/store/modules/app.js index 8ed27f88b4c..806e11b2adb 100644 --- a/ui/src/store/modules/app.js +++ b/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) => { @@ -121,6 +123,10 @@ const app = { SET_CUSTOM_COLUMNS: (state, customColumns) => { vueProps.$localStorage.set(CUSTOM_COLUMNS, customColumns) state.customColumns = customColumns + }, + RELOAD_ALL_PROJECTS: (state, allProjects = []) => { + vueProps.$localStorage.set(RELOAD_ALL_PROJECTS, allProjects) + state.allProjects = allProjects } }, actions: { @@ -177,6 +183,9 @@ const app = { }, SetCustomColumns ({ commit }, bool) { commit('SET_CUSTOM_COLUMNS', bool) + }, + ReloadAllProjects ({ commit, allProjects }) { + commit('RELOAD_ALL_PROJECTS', allProjects) } } } diff --git a/ui/src/views/AutogenView.vue b/ui/src/views/AutogenView.vue index c8e931b6be0..4bc0c43616f 100644 --- a/ui/src/views/AutogenView.vue +++ b/ui/src/views/AutogenView.vue @@ -693,8 +693,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) @@ -937,6 +939,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') @@ -1808,7 +1811,6 @@ export default { this.rules[field.name].push(rule) break default: - console.log('hererere') rule.required = field.required rule.message = this.$t('message.error.required.input') this.rules[field.name].push(rule) From a0702279aae011316a5a5af7e206ad0309f06b41 Mon Sep 17 00:00:00 2001 From: Marcus Sorensen Date: Thu, 24 Aug 2023 02:45:39 -0600 Subject: [PATCH 2/4] server Don't allow inadvertent deletion of hidden details via API (#7880) * Don't allow inadvertent deletion of hidden details via API * Update VM details unit test ensuring system/hidden details not removed * Update test/integration/component/test_update_vm.py --------- Co-authored-by: Marcus Sorensen Co-authored-by: dahn --- .../java/com/cloud/vm/UserVmManagerImpl.java | 25 +++- .../com/cloud/vm/UserVmManagerImplTest.java | 29 +++- test/integration/component/test_update_vm.py | 124 ++++++++++++++++-- 3 files changed, 152 insertions(+), 26 deletions(-) diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index 94ceb0de363..c1840065603 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -2756,13 +2756,18 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir final List userReadOnlySettings = Stream.of(QueryService.UserVMReadOnlyDetails.value().split(",")) .map(item -> (item).trim()) .collect(Collectors.toList()); + List 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()); } } @@ -2782,15 +2787,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); diff --git a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java index 18b4a891917..22a9bed7764 100644 --- a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java +++ b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java @@ -149,7 +149,7 @@ public class UserVmManagerImplTest { private EntityManager entityManager; @Mock - private UserVmDetailsDao userVmDetailVO; + private UserVmDetailsDao userVmDetailsDao; @Mock private UserVmVO userVmVoMock; @@ -300,7 +300,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 @@ -310,12 +310,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); } @@ -340,6 +343,16 @@ public class UserVmManagerImplTest { prepareAndExecuteMethodDealingWithDetails(false, false); } + private List prepareExistingDetails(Long vmId, String... existingDetailKeys) { + List 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(); @@ -360,8 +373,9 @@ public class UserVmManagerImplTest { lenient().doNothing().when(_networkMgr).saveExtraDhcpOptions(anyString(), anyLong(), anyMap()); HashMap 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); @@ -371,14 +385,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); } diff --git a/test/integration/component/test_update_vm.py b/test/integration/component/test_update_vm.py index ca20a5ad3f8..4d167c63330 100644 --- a/test/integration/component/test_update_vm.py +++ b/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) From 8ad1009ad2d59571f48a89b94d9c1424a46030f9 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Thu, 24 Aug 2023 14:59:25 +0530 Subject: [PATCH 3/4] ui: fix notification list reordering intermittently (#7609) Signed-off-by: Abhishek Kumar --- ui/src/components/header/HeaderNotice.vue | 2 +- ui/src/store/modules/user.js | 6 +++++- ui/src/utils/plugins.js | 9 ++++++--- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/ui/src/components/header/HeaderNotice.vue b/ui/src/components/header/HeaderNotice.vue index fe9c579581e..82dc4d45edc 100644 --- a/ui/src/components/header/HeaderNotice.vue +++ b/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 } } ) diff --git a/ui/src/store/modules/user.js b/ui/src/store/modules/user.js index 3e9aef7af08..3994a3fc29d 100644 --- a/ui/src/store/modules/user.js +++ b/ui/src/store/modules/user.js @@ -348,9 +348,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) { diff --git a/ui/src/utils/plugins.js b/ui/src/utils/plugins.js index 5cc4bc6269a..7223f050368 100644 --- a/ui/src/utils/plugins.js +++ b/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 From 3c38ed7a6567b95f3c18c094094e0447231c0aa2 Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Thu, 24 Aug 2023 18:12:01 +0200 Subject: [PATCH 4/4] server: allow user to list available IPs on shared networks (#7898) This fixes #7817 --- .../java/com/cloud/network/IpAddressManager.java | 4 ++++ .../com/cloud/network/IpAddressManagerImpl.java | 2 +- .../com/cloud/server/ManagementServerImpl.java | 16 +++++++++++++++- 3 files changed, 20 insertions(+), 2 deletions(-) diff --git a/engine/components-api/src/main/java/com/cloud/network/IpAddressManager.java b/engine/components-api/src/main/java/com/cloud/network/IpAddressManager.java index 2fa66d7166b..73d3de0ef0c 100644 --- a/engine/components-api/src/main/java/com/cloud/network/IpAddressManager.java +++ b/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 AllowUserListAvailableIpsOnSharedNetwork = new ConfigKey("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. * diff --git a/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java b/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java index 5436dd6acb1..60e7c5d12bf 100644 --- a/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java +++ b/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java @@ -2342,7 +2342,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}; } /** diff --git a/server/src/main/java/com/cloud/server/ManagementServerImpl.java b/server/src/main/java/com/cloud/server/ManagementServerImpl.java index c6ace852bd3..31a78744153 100644 --- a/server/src/main/java/com/cloud/server/ManagementServerImpl.java +++ b/server/src/main/java/com/cloud/server/ManagementServerImpl.java @@ -2323,6 +2323,7 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe isAllocated = Boolean.TRUE; } } + boolean isAllocatedTemp = isAllocated; VlanType vlanType = null; if (forVirtualNetwork != null) { @@ -2333,6 +2334,7 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe final Account caller = getCaller(); List 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) { @@ -2341,7 +2343,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) { @@ -2475,7 +2476,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 searchBuilder = _publicIpAddressDao.createSearchBuilder(); + buildParameters(searchBuilder, cmd, false); + + SearchCriteria 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 sb2 = _publicIpAddressDao.createSearchBuilder(); buildParameters(sb2, cmd, false);