Skip to content

Commit 3b05797

Browse files
committed
Merge remote-tracking branch 'origin/4.18'
2 parents c683de4 + 3c38ed7 commit 3b05797

13 files changed

Lines changed: 211 additions & 37 deletions

File tree

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,10 @@ public interface IpAddressManager {
5656
"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.",
5757
true, ConfigKey.Scope.Account, null, null, null, null, null, ConfigKey.Kind.Select, "first,last,random");
5858

59+
ConfigKey<Boolean> AllowUserListAvailableIpsOnSharedNetwork = new ConfigKey<Boolean>("Advanced", Boolean.class, "allow.user.list.available.ips.on.shared.network", "false",
60+
"Determines whether users can list available IPs on shared networks",
61+
true, ConfigKey.Scope.Global);
62+
5963
/**
6064
* Assigns a new public ip address.
6165
*

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2341,7 +2341,7 @@ public String getConfigComponentName() {
23412341

23422342
@Override
23432343
public ConfigKey<?>[] getConfigKeys() {
2344-
return new ConfigKey<?>[] {UseSystemPublicIps, RulesContinueOnError, SystemVmPublicIpReservationModeStrictness, VrouterRedundantTiersPlacement};
2344+
return new ConfigKey<?>[] {UseSystemPublicIps, RulesContinueOnError, SystemVmPublicIpReservationModeStrictness, VrouterRedundantTiersPlacement, AllowUserListAvailableIpsOnSharedNetwork};
23452345
}
23462346

23472347
/**

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

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2322,6 +2322,7 @@ public Pair<List<? extends IpAddress>, Integer> searchForIPAddresses(final ListP
23222322
isAllocated = Boolean.TRUE;
23232323
}
23242324
}
2325+
boolean isAllocatedTemp = isAllocated;
23252326

23262327
VlanType vlanType = null;
23272328
if (forVirtualNetwork != null) {
@@ -2332,6 +2333,7 @@ public Pair<List<? extends IpAddress>, Integer> searchForIPAddresses(final ListP
23322333

23332334
final Account caller = getCaller();
23342335
List<IPAddressVO> addrs = new ArrayList<>();
2336+
NetworkVO network = null; // shared network
23352337

23362338
if (vlanType == VlanType.DirectAttached && networkId == null && ipId == null) { // only root admin can list public ips in all shared networks
23372339
if (caller.getType() != Account.Type.ADMIN) {
@@ -2340,7 +2342,6 @@ public Pair<List<? extends IpAddress>, Integer> searchForIPAddresses(final ListP
23402342
} else if (vlanType == VlanType.DirectAttached) {
23412343
// list public ip address on shared network
23422344
// access control. admin: all Ips, domain admin/user: all Ips in shared network in the domain/sub-domain/user
2343-
NetworkVO network = null;
23442345
if (networkId == null) {
23452346
IPAddressVO ip = _publicIpAddressDao.findById(ipId);
23462347
if (ip == null) {
@@ -2474,7 +2475,20 @@ public Pair<List<? extends IpAddress>, Integer> searchForIPAddresses(final ListP
24742475
for (IPAddressVO addr: freeAddrs) {
24752476
freeAddrIds.add(addr.getId());
24762477
}
2478+
} else if (vlanType == VlanType.DirectAttached && network != null && !isAllocatedTemp && isAllocated) {
2479+
if (caller.getType() != Account.Type.ADMIN && !IpAddressManager.AllowUserListAvailableIpsOnSharedNetwork.value()) {
2480+
s_logger.debug("Non-admin users are not allowed to list available IPs on shared networks");
2481+
} else {
2482+
final SearchBuilder<IPAddressVO> searchBuilder = _publicIpAddressDao.createSearchBuilder();
2483+
buildParameters(searchBuilder, cmd, false);
2484+
2485+
SearchCriteria<IPAddressVO> searchCriteria = searchBuilder.create();
2486+
setParameters(searchCriteria, cmd, vlanType, false);
2487+
searchCriteria.setParameters("state", IpAddress.State.Free.name());
2488+
addrs.addAll(_publicIpAddressDao.search(searchCriteria, searchFilter)); // Free IPs on shared network
2489+
}
24772490
}
2491+
24782492
if (freeAddrIds.size() > 0) {
24792493
final SearchBuilder<IPAddressVO> sb2 = _publicIpAddressDao.createSearchBuilder();
24802494
buildParameters(sb2, cmd, false);

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

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2772,13 +2772,18 @@ public UserVm updateVirtualMachine(UpdateVMCmd cmd) throws ResourceUnavailableEx
27722772
final List<String> userReadOnlySettings = Stream.of(QueryService.UserVMReadOnlyDetails.value().split(","))
27732773
.map(item -> (item).trim())
27742774
.collect(Collectors.toList());
2775+
List<UserVmDetailVO> existingDetails = userVmDetailsDao.listDetails(id);
27752776
if (cleanupDetails){
27762777
if (caller != null && caller.getType() == Account.Type.ADMIN) {
2777-
userVmDetailsDao.removeDetails(id);
2778+
for (final UserVmDetailVO detail : existingDetails) {
2779+
if (detail != null && detail.isDisplay()) {
2780+
userVmDetailsDao.removeDetail(id, detail.getName());
2781+
}
2782+
}
27782783
} else {
2779-
for (final UserVmDetailVO detail : userVmDetailsDao.listDetails(id)) {
2784+
for (final UserVmDetailVO detail : existingDetails) {
27802785
if (detail != null && !userDenyListedSettings.contains(detail.getName())
2781-
&& !userReadOnlySettings.contains(detail.getName())) {
2786+
&& !userReadOnlySettings.contains(detail.getName()) && detail.isDisplay()) {
27822787
userVmDetailsDao.removeDetail(id, detail.getName());
27832788
}
27842789
}
@@ -2798,15 +2803,25 @@ public UserVm updateVirtualMachine(UpdateVMCmd cmd) throws ResourceUnavailableEx
27982803
if (userReadOnlySettings.contains(detailName)) {
27992804
throw new InvalidParameterValueException("You're not allowed to add or edit the read-only setting: " + detailName);
28002805
}
2806+
if (existingDetails.stream().anyMatch(d -> Objects.equals(d.getName(), detailName) && !d.isDisplay())){
2807+
throw new InvalidParameterValueException("You're not allowed to add or edit the non-displayable setting: " + detailName);
2808+
}
28012809
}
2802-
// Add any hidden/denied or read-only detail
2803-
for (final UserVmDetailVO detail : userVmDetailsDao.listDetails(id)) {
2810+
// Add any existing user denied or read-only details. We do it here because admins would already provide these (or can delete them).
2811+
for (final UserVmDetailVO detail : existingDetails) {
28042812
if (userDenyListedSettings.contains(detail.getName()) || userReadOnlySettings.contains(detail.getName())) {
28052813
details.put(detail.getName(), detail.getValue());
28062814
}
28072815
}
28082816
}
28092817

2818+
// ensure details marked as non-displayable are maintained, regardless of admin or not
2819+
for (final UserVmDetailVO existingDetail : existingDetails) {
2820+
if (!existingDetail.isDisplay()) {
2821+
details.put(existingDetail.getName(), existingDetail.getValue());
2822+
}
2823+
}
2824+
28102825
verifyVmLimits(vmInstance, details);
28112826
vmInstance.setDetails(details);
28122827
_vmDao.saveDetails(vmInstance);

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

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ public class UserVmManagerImplTest {
162162
private EntityManager entityManager;
163163

164164
@Mock
165-
private UserVmDetailsDao userVmDetailVO;
165+
private UserVmDetailsDao userVmDetailsDao;
166166

167167
@Mock
168168
private UserVmVO userVmVoMock;
@@ -333,7 +333,7 @@ public void updateVirtualMachineTestDisplayChanged() throws ResourceUnavailableE
333333
verifyMethodsThatAreAlwaysExecuted();
334334

335335
Mockito.verify(userVmManagerImpl).updateDisplayVmFlag(false, vmId, userVmVoMock);
336-
Mockito.verify(userVmDetailVO, Mockito.times(0)).removeDetails(vmId);
336+
Mockito.verify(userVmDetailsDao, Mockito.times(0)).removeDetail(anyLong(), anyString());
337337
}
338338

339339
@Test
@@ -343,12 +343,15 @@ public void updateVirtualMachineTestCleanUpTrue() throws ResourceUnavailableExce
343343
Mockito.when(_serviceOfferingDao.findById(Mockito.anyLong(), Mockito.anyLong())).thenReturn((ServiceOfferingVO) offering);
344344
Mockito.when(updateVmCommand.isCleanupDetails()).thenReturn(true);
345345
Mockito.lenient().doNothing().when(userVmManagerImpl).updateDisplayVmFlag(false, vmId, userVmVoMock);
346-
Mockito.doNothing().when(userVmDetailVO).removeDetails(vmId);
346+
347347
Mockito.when(updateVmCommand.getUserdataId()).thenReturn(null);
348348

349+
prepareExistingDetails(vmId, "userdetail");
350+
349351
userVmManagerImpl.updateVirtualMachine(updateVmCommand);
350352
verifyMethodsThatAreAlwaysExecuted();
351-
Mockito.verify(userVmDetailVO).removeDetails(vmId);
353+
Mockito.verify(userVmDetailsDao).removeDetail(vmId, "userdetail");
354+
Mockito.verify(userVmDetailsDao, Mockito.times(0)).removeDetail(vmId, "systemdetail");
352355
Mockito.verify(userVmManagerImpl, Mockito.times(0)).updateDisplayVmFlag(false, vmId, userVmVoMock);
353356
}
354357

@@ -373,6 +376,16 @@ public void updateVirtualMachineTestCleanUpFalseAndDetailsEmpty() throws Resourc
373376
prepareAndExecuteMethodDealingWithDetails(false, false);
374377
}
375378

379+
private List<UserVmDetailVO> prepareExistingDetails(Long vmId, String... existingDetailKeys) {
380+
List<UserVmDetailVO> existingDetails = new ArrayList<>();
381+
for (String detail : existingDetailKeys) {
382+
existingDetails.add(new UserVmDetailVO(vmId, detail, "foo", true));
383+
}
384+
existingDetails.add(new UserVmDetailVO(vmId, "systemdetail", "bar", false));
385+
Mockito.when(userVmDetailsDao.listDetails(vmId)).thenReturn(existingDetails);
386+
return existingDetails;
387+
}
388+
376389
private void prepareAndExecuteMethodDealingWithDetails(boolean cleanUpDetails, boolean isDetailsEmpty) throws ResourceUnavailableException, InsufficientCapacityException {
377390
configureDoNothingForMethodsThatWeDoNotWantToTest();
378391

@@ -393,8 +406,9 @@ private void prepareAndExecuteMethodDealingWithDetails(boolean cleanUpDetails, b
393406
lenient().doNothing().when(_networkMgr).saveExtraDhcpOptions(anyString(), anyLong(), anyMap());
394407
HashMap<String, String> details = new HashMap<>();
395408
if(!isDetailsEmpty) {
396-
details.put("", "");
409+
details.put("newdetail", "foo");
397410
}
411+
prepareExistingDetails(vmId, "existingdetail");
398412
Mockito.when(updateVmCommand.getUserdataId()).thenReturn(null);
399413
Mockito.when(updateVmCommand.getDetails()).thenReturn(details);
400414
Mockito.when(updateVmCommand.isCleanupDetails()).thenReturn(cleanUpDetails);
@@ -404,14 +418,15 @@ private void prepareAndExecuteMethodDealingWithDetails(boolean cleanUpDetails, b
404418
verifyMethodsThatAreAlwaysExecuted();
405419

406420
Mockito.verify(userVmVoMock, Mockito.times(cleanUpDetails || isDetailsEmpty ? 0 : 1)).setDetails(details);
407-
Mockito.verify(userVmDetailVO, Mockito.times(cleanUpDetails ? 1: 0)).removeDetails(vmId);
421+
Mockito.verify(userVmDetailsDao, Mockito.times(cleanUpDetails ? 1 : 0)).removeDetail(vmId, "existingdetail");
422+
Mockito.verify(userVmDetailsDao, Mockito.times(0)).removeDetail(vmId, "systemdetail");
408423
Mockito.verify(userVmDao, Mockito.times(cleanUpDetails || isDetailsEmpty ? 0 : 1)).saveDetails(userVmVoMock);
409424
Mockito.verify(userVmManagerImpl, Mockito.times(0)).updateDisplayVmFlag(false, vmId, userVmVoMock);
410425
}
411426

412427
private void configureDoNothingForDetailsMethod() {
413428
Mockito.lenient().doNothing().when(userVmManagerImpl).updateDisplayVmFlag(false, vmId, userVmVoMock);
414-
Mockito.doNothing().when(userVmDetailVO).removeDetails(vmId);
429+
Mockito.doNothing().when(userVmDetailsDao).removeDetail(anyLong(), anyString());
415430
Mockito.doNothing().when(userVmDao).saveDetails(userVmVoMock);
416431
}
417432

test/integration/component/test_update_vm.py

Lines changed: 110 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,11 @@
1515
# specific language governing permissions and limitations
1616
# under the License.
1717

18-
1918
from marvin.cloudstackTestCase import cloudstackTestCase
2019
from marvin.lib.base import Account, VirtualMachine, ServiceOffering
21-
from marvin.lib.utils import cleanup_resources
20+
from marvin.lib.utils import (validateList, cleanup_resources)
2221
from marvin.lib.common import get_zone, get_domain, get_template
22+
from marvin.codes import PASS
2323
from nose.plugins.attrib import attr
2424

2525
class TestData(object):
@@ -59,6 +59,7 @@ class TestUpdateVirtualMachine(cloudstackTestCase):
5959
def setUp(self):
6060
self.testdata = TestData().testdata
6161
self.apiclient = self.testClient.getApiClient()
62+
self.dbclient = self.testClient.getDbConnection()
6263

6364
# Get Zone, Domain and Default Built-in template
6465
self.domain = get_domain(self.apiclient)
@@ -106,18 +107,7 @@ def test_update_vm_name(self):
106107
templateid=self.template.id
107108
)
108109

109-
list_vms = VirtualMachine.list(self.apiclient, id=self.virtual_machine.id)
110-
self.assertEqual(
111-
isinstance(list_vms, list),
112-
True,
113-
"List VM response was not a valid list"
114-
)
115-
self.assertNotEqual(
116-
len(list_vms),
117-
0,
118-
"List VM response was empty"
119-
)
120-
vm = list_vms[0]
110+
vm = self.listVmById(self.virtual_machine.id)
121111

122112
self.debug(
123113
"VirtualMachine launched with id, name, displayname: %s %s %s"\
@@ -152,6 +142,112 @@ def test_update_vm_name(self):
152142
self.assertEqual(vmnew.displayname, vmnewstarted.displayname,
153143
msg="display name changed on start, displayname is %s" % vmnewstarted.displayname)
154144

145+
@attr(tags=['advanced', 'simulator', 'basic', 'sg', 'details'], required_hardware="false")
146+
def test_update_vm_details_admin(self):
147+
"""Test Update VirtualMachine Details
148+
149+
# Set up a VM
150+
# Set up hidden detail in DB for VM
151+
152+
# Validate the following:
153+
# 1. Can add two details (detail1, detail2)
154+
# 2. Can fetch new details on VM
155+
# 3. Can delete detail1
156+
# 4. Hidden detail not removed
157+
# 6. The detail2 remains
158+
# 7. Ensure cleanup parameter doesn't remove hidden details
159+
"""
160+
hidden_detail_name = "configDriveLocation"
161+
detail1 = "detail1"
162+
detail2 = "detail2"
163+
164+
# set up a VM
165+
self.virtual_machine = VirtualMachine.create(
166+
self.apiclient,
167+
self.testdata["virtual_machine"],
168+
accountid=self.account.name,
169+
zoneid=self.zone.id,
170+
domainid=self.account.domainid,
171+
serviceofferingid=self.service_offering.id,
172+
templateid=self.template.id
173+
)
174+
self.cleanup.append(self.virtual_machine)
175+
176+
vm = self.listVmById(self.virtual_machine.id)
177+
178+
self.debug(
179+
"VirtualMachine launched with id, name, displayname: %s %s %s" \
180+
% (self.virtual_machine.id, vm.name, vm.displayname)
181+
)
182+
183+
# set up a hidden detail
184+
dbresult = self.dbclient.execute("select id from vm_instance where uuid='%s'" % vm.id)
185+
self.assertEqual(validateList(dbresult)[0], PASS, "sql query returned invalid response")
186+
vm_db_id = dbresult[0][0]
187+
self.debug("VM has database id %d" % vm_db_id)
188+
189+
self.dbclient.execute("insert into user_vm_details (vm_id, name, value, display) values (%d,'%s','HOST', 0)" % (vm_db_id, hidden_detail_name))
190+
191+
vm = self.listVmById(self.virtual_machine.id)
192+
self.debug("VirtualMachine fetched with details: %s of type %s" % (vm.details, type(vm.details)))
193+
194+
self.assertIsNone(vm.details[hidden_detail_name], "hidden detail should be hidden")
195+
196+
# add two details by appending to what was returned via API
197+
updating_vm_details = vm.details.__dict__
198+
updating_vm_details[detail1] = "foo"
199+
updating_vm_details[detail2] = "bar"
200+
201+
self.debug("Updating VM to new details: %s" % updating_vm_details)
202+
vm = self.virtual_machine.update(self.apiclient, details=[updating_vm_details])
203+
204+
self.assertIsNotNone(vm.details[detail1], "Expect " + detail1)
205+
self.assertIsNotNone(vm.details[detail2], "Expect " + detail2)
206+
self.assertIsNone(vm.details[hidden_detail_name], "hidden detail should be hidden")
207+
self.assertTrue(self.detailInDatabase(vm_db_id, hidden_detail_name), "hidden detail should still exist in db")
208+
209+
# delete one detail
210+
updating_vm_details = vm.details.__dict__
211+
del updating_vm_details["detail1"]
212+
213+
self.debug("Deleting one detail by updating details: %s" % updating_vm_details)
214+
vm = self.virtual_machine.update(self.apiclient, details=[updating_vm_details])
215+
216+
self.assertIsNone(vm.details[detail1], "Do not expect " + detail1)
217+
self.assertIsNotNone(vm.details[detail2], "Expect " + detail2)
218+
self.assertIsNone(vm.details[hidden_detail_name], "hidden detail should be hidden")
219+
self.assertTrue(self.detailInDatabase(vm_db_id, hidden_detail_name), "hidden detail should still exist in db")
220+
221+
# cleanup, ensure hidden detail is not deleted
222+
vm = self.virtual_machine.update(self.apiclient, cleanupdetails="true")
223+
self.assertIsNone(vm.details[detail1], "Do not expect " + detail1)
224+
self.assertIsNone(vm.details[detail2], "Do not expect " + detail2)
225+
self.assertIsNone(vm.details[hidden_detail_name], "hidden detail should be hidden")
226+
self.assertTrue(self.detailInDatabase(vm_db_id, hidden_detail_name), "hidden detail should still exist in db")
227+
228+
229+
def detailInDatabase(self, vm_id, detail_name):
230+
dbresult = self.dbclient.execute("select id from user_vm_details where vm_id=%s and name='%s'" % (vm_id, detail_name))
231+
self.debug("Detail %s for VM %s: %s" % (detail_name, vm_id, dbresult))
232+
if validateList(dbresult)[0] == PASS:
233+
return True
234+
return False
235+
236+
237+
def listVmById(self, id):
238+
list_vms = VirtualMachine.list(self.apiclient, id=id)
239+
self.assertEqual(
240+
isinstance(list_vms, list),
241+
True,
242+
"List VM response was not a valid list"
243+
)
244+
self.assertNotEqual(
245+
len(list_vms),
246+
0,
247+
"List VM response was empty"
248+
)
249+
return list_vms[0]
250+
155251
def tearDown(self):
156252
try:
157253
cleanup_resources(self.apiclient, self.cleanup)

ui/src/components/header/HeaderNotice.vue

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ export default {
109109
(state, getters) => getters.headerNotices,
110110
(newValue, oldValue) => {
111111
if (oldValue !== newValue && newValue !== undefined) {
112-
this.notices = newValue.reverse()
112+
this.notices = newValue
113113
}
114114
}
115115
)

ui/src/components/header/ProjectMenu.vue

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,9 +90,8 @@ export default {
9090
getNextPage()
9191
}
9292
}).finally(() => {
93-
this.projects = _.orderBy(projects, ['displaytext'], ['asc'])
94-
this.projects.unshift({ name: this.$t('label.default.view') })
9593
this.loading = false
94+
this.$store.commit('RELOAD_ALL_PROJECTS', projects)
9695
})
9796
}
9897
getNextPage()
@@ -113,6 +112,17 @@ export default {
113112
filterProject (input, option) {
114113
return option.label.toLowerCase().indexOf(input.toLowerCase()) >= 0
115114
}
115+
},
116+
mounted () {
117+
this.$store.watch(
118+
(state, getters) => getters.allProjects,
119+
(newValue, oldValue) => {
120+
if (oldValue !== newValue && newValue !== undefined) {
121+
this.projects = _.orderBy(newValue, ['displaytext'], ['asc'])
122+
this.projects.unshift({ name: this.$t('label.default.view') })
123+
}
124+
}
125+
)
116126
}
117127
}
118128
</script>

ui/src/store/getters.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ const getters = {
4949
twoFaProvider: state => state.user.twoFaProvider,
5050
twoFaIssuer: state => state.user.twoFaIssuer,
5151
loginFlag: state => state.user.loginFlag,
52+
allProjects: (state) => state.app.allProjects,
5253
customHypervisorName: state => state.user.customHypervisorName
5354
}
5455

0 commit comments

Comments
 (0)