DaanHoogland commented on PR #7597: URL: https://github.com/apache/cloudstack/pull/7597#issuecomment-1585044930
> > > > Do we need to make java changes as well ? cc @DaanHoogland @rohityadavcloud > > > > > > > > > this is just a UI change @weizhouapache . Do we need/want to block this on the API as well? I see no need for that but if we think so I will look into it. > > > > > > I checked the code and the `clusterid` and `hostid` are not guarded against use by users in the API, so I think it makes no sense to guard against the use of `considerlasthost`. it is true by default whic is the old bahaviour. Having it as false will only make the process resemble a start of a fresh installed VM. Can you agree @weizhouapache ? > > @DaanHoogland if normal user passes `clusterid` or `hostid`, it will might get an exception > > https://github.com/apache/cloudstack/blob/4.18.0.0/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java#L5421-L5451 > > ``` > private Cluster getDestinationCluster(Long clusterId, boolean isRootAdmin) { > Cluster destinationCluster = null; > if (clusterId != null) { > if (!isRootAdmin) { > throw new PermissionDeniedException( > "Parameter " + ApiConstants.CLUSTER_ID + " can only be specified by a Root Admin, permission denied"); > } > destinationCluster = _clusterDao.findById(clusterId); > if (destinationCluster == null) { > throw new InvalidParameterValueException("Unable to find the cluster to deploy the VM, cluster id=" + clusterId); > } > } > return destinationCluster; > } > > private Host getDestinationHost(Long hostId, boolean isRootAdmin, boolean isExplicitHost) { > Host destinationHost = null; > if (hostId != null) { > if (isExplicitHost && !isRootAdmin) { > throw new PermissionDeniedException( > "Parameter " + ApiConstants.HOST_ID + " can only be specified by a Root Admin, permission denied"); > } > destinationHost = _hostDao.findById(hostId); > if (destinationHost == null) { > throw new InvalidParameterValueException("Unable to find the host to deploy the VM, host id=" + hostId); > } else if (destinationHost.getResourceState() != ResourceState.Enabled || destinationHost.getStatus() != Status.Up ) { > throw new InvalidParameterValueException("Unable to deploy the VM as the host: " + destinationHost.getName() + " is not in the right state"); > } > } > return destinationHost; > } > ``` ok I didn´t dig that deep, but still unsetting considerlasthost is not doing much. I think not bothering normal users with it is enough, we don't need to forbid the use. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
