api,server,ui: improve listing public ip for associate (#11591)

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>
This commit is contained in:
Abhishek Kumar 2025-11-26 09:24:12 +01:00 committed by GitHub
parent dba889ea3e
commit 9ec8cc4186
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 146 additions and 82 deletions

View File

@ -53,7 +53,7 @@ public class ListPublicIpAddressesCmd extends BaseListRetrieveOnlyResourceCountC
@Parameter(name = ApiConstants.ALLOCATED_ONLY, type = CommandType.BOOLEAN, description = "limits search results to allocated public IP addresses")
private Boolean allocatedOnly;
@Parameter(name = ApiConstants.STATE, type = CommandType.STRING, description = "lists all public IP addresses by state")
@Parameter(name = ApiConstants.STATE, type = CommandType.STRING, description = "lists all public IP addresses by state. A comma-separated list of states can be passed")
private String state;
@Parameter(name = ApiConstants.FOR_VIRTUAL_NETWORK, type = CommandType.BOOLEAN, description = "the virtual network for the IP address")

View File

@ -823,6 +823,7 @@ import com.cloud.user.dao.AccountDao;
import com.cloud.user.dao.SSHKeyPairDao;
import com.cloud.user.dao.UserDao;
import com.cloud.user.dao.UserDataDao;
import com.cloud.utils.EnumUtils;
import com.cloud.utils.NumbersUtil;
import com.cloud.utils.Pair;
import com.cloud.utils.PasswordGenerator;
@ -2419,6 +2420,22 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
return new Pair<>(result.first(), result.second());
}
protected List<IpAddress.State> getStatesForIpAddressSearch(final ListPublicIpAddressesCmd cmd) {
final String statesStr = cmd.getState();
final List<IpAddress.State> states = new ArrayList<>();
if (StringUtils.isBlank(statesStr)) {
return states;
}
for (String s : StringUtils.split(statesStr, ",")) {
IpAddress.State state = EnumUtils.getEnumIgnoreCase(IpAddress.State.class, s.trim());
if (state == null) {
throw new InvalidParameterValueException("Invalid state: " + s);
}
states.add(state);
}
return states;
}
@Override
public Pair<List<? extends IpAddress>, Integer> searchForIPAddresses(final ListPublicIpAddressesCmd cmd) {
final Long associatedNetworkId = cmd.getAssociatedNetworkId();
@ -2429,20 +2446,20 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
final Long networkId = cmd.getNetworkId();
final Long vpcId = cmd.getVpcId();
final String state = cmd.getState();
final List<IpAddress.State> states = getStatesForIpAddressSearch(cmd);
Boolean isAllocated = cmd.isAllocatedOnly();
if (isAllocated == null) {
if (state != null && (state.equalsIgnoreCase(IpAddress.State.Free.name()) || state.equalsIgnoreCase(IpAddress.State.Reserved.name()))) {
if (states.contains(IpAddress.State.Free) || states.contains(IpAddress.State.Reserved)) {
isAllocated = Boolean.FALSE;
} else {
isAllocated = Boolean.TRUE; // default
}
} else {
if (state != null && (state.equalsIgnoreCase(IpAddress.State.Free.name()) || state.equalsIgnoreCase(IpAddress.State.Reserved.name()))) {
if (states.contains(IpAddress.State.Free) || states.contains(IpAddress.State.Reserved)) {
if (isAllocated) {
throw new InvalidParameterValueException("Conflict: allocatedonly is true but state is Free");
}
} else if (state != null && state.equalsIgnoreCase(IpAddress.State.Allocated.name())) {
} else if (states.contains(IpAddress.State.Allocated)) {
isAllocated = Boolean.TRUE;
}
}
@ -2521,10 +2538,8 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
Boolean isRecursive = cmd.isRecursive();
final List<Long> permittedAccounts = new ArrayList<>();
ListProjectResourcesCriteria listProjectResourcesCriteria = null;
boolean isAllocatedOrReserved = false;
if (isAllocated || IpAddress.State.Reserved.name().equalsIgnoreCase(state)) {
isAllocatedOrReserved = true;
}
boolean isAllocatedOrReserved = isAllocated ||
(states.size() == 1 && IpAddress.State.Reserved.equals(states.get(0)));
if (isAllocatedOrReserved || (vlanType == VlanType.VirtualNetwork && (caller.getType() != Account.Type.ADMIN || cmd.getDomainId() != null))) {
final Ternary<Long, Boolean, ListProjectResourcesCriteria> domainIdRecursiveListProject = new Ternary<>(cmd.getDomainId(), cmd.isRecursive(),
null);
@ -2538,7 +2553,7 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
buildParameters(sb, cmd, vlanType == VlanType.VirtualNetwork ? true : isAllocated);
SearchCriteria<IPAddressVO> sc = sb.create();
setParameters(sc, cmd, vlanType, isAllocated);
setParameters(sc, cmd, vlanType, isAllocated, states);
if (isAllocatedOrReserved || (vlanType == VlanType.VirtualNetwork && (caller.getType() != Account.Type.ADMIN || cmd.getDomainId() != null))) {
_accountMgr.buildACLSearchCriteria(sc, domainId, isRecursive, permittedAccounts, listProjectResourcesCriteria);
@ -2606,7 +2621,7 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
buildParameters(searchBuilder, cmd, false);
SearchCriteria<IPAddressVO> searchCriteria = searchBuilder.create();
setParameters(searchCriteria, cmd, vlanType, false);
setParameters(searchCriteria, cmd, vlanType, false, states);
searchCriteria.setParameters("state", IpAddress.State.Free.name());
addrs.addAll(_publicIpAddressDao.search(searchCriteria, searchFilter)); // Free IPs on shared network
}
@ -2619,7 +2634,7 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
sb2.and("quarantinedPublicIpsIdsNIN", sb2.entity().getId(), SearchCriteria.Op.NIN);
SearchCriteria<IPAddressVO> sc2 = sb2.create();
setParameters(sc2, cmd, vlanType, isAllocated);
setParameters(sc2, cmd, vlanType, isAllocated, states);
sc2.setParameters("ids", freeAddrIds.toArray());
_publicIpAddressDao.buildQuarantineSearchCriteria(sc2);
addrs.addAll(_publicIpAddressDao.search(sc2, searchFilter)); // Allocated + Free
@ -2649,7 +2664,7 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
sb.and("isSourceNat", sb.entity().isSourceNat(), SearchCriteria.Op.EQ);
sb.and("isStaticNat", sb.entity().isOneToOneNat(), SearchCriteria.Op.EQ);
sb.and("vpcId", sb.entity().getVpcId(), SearchCriteria.Op.EQ);
sb.and("state", sb.entity().getState(), SearchCriteria.Op.EQ);
sb.and("state", sb.entity().getState(), SearchCriteria.Op.IN);
sb.and("display", sb.entity().isDisplay(), SearchCriteria.Op.EQ);
sb.and(FOR_SYSTEMVMS, sb.entity().isForSystemVms(), SearchCriteria.Op.EQ);
@ -2692,7 +2707,8 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
}
}
protected void setParameters(SearchCriteria<IPAddressVO> sc, final ListPublicIpAddressesCmd cmd, VlanType vlanType, Boolean isAllocated) {
protected void setParameters(SearchCriteria<IPAddressVO> sc, final ListPublicIpAddressesCmd cmd, VlanType vlanType,
Boolean isAllocated, List<IpAddress.State> states) {
final Object keyword = cmd.getKeyword();
final Long physicalNetworkId = cmd.getPhysicalNetworkId();
final Long sourceNetworkId = cmd.getNetworkId();
@ -2703,7 +2719,6 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
final Boolean sourceNat = cmd.isSourceNat();
final Boolean staticNat = cmd.isStaticNat();
final Boolean forDisplay = cmd.getDisplay();
final String state = cmd.getState();
final Boolean forSystemVms = cmd.getForSystemVMs();
final boolean forProvider = cmd.isForProvider();
final Map<String, String> tags = cmd.getTags();
@ -2760,13 +2775,14 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
sc.setParameters("display", forDisplay);
}
if (state != null) {
sc.setParameters("state", state);
if (CollectionUtils.isNotEmpty(states)) {
sc.setParameters("state", states.toArray());
} else if (isAllocated != null && isAllocated) {
sc.setParameters("state", IpAddress.State.Allocated);
}
if (IpAddressManagerImpl.getSystemvmpublicipreservationmodestrictness().value() && IpAddress.State.Free.name().equalsIgnoreCase(state)) {
if (IpAddressManagerImpl.getSystemvmpublicipreservationmodestrictness().value() &&
states.contains(IpAddress.State.Free)) {
sc.setParameters(FOR_SYSTEMVMS, false);
} else {
sc.setParameters(FOR_SYSTEMVMS, forSystemVms);

View File

@ -26,6 +26,7 @@ import static org.mockito.Mockito.when;
import java.lang.reflect.Field;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import org.apache.cloudstack.annotation.dao.AnnotationDao;
@ -258,14 +259,14 @@ public class ManagementServerImplTest {
Mockito.when(cmd.getId()).thenReturn(null);
Mockito.when(cmd.isSourceNat()).thenReturn(null);
Mockito.when(cmd.isStaticNat()).thenReturn(null);
Mockito.when(cmd.getState()).thenReturn(IpAddress.State.Free.name());
Mockito.when(cmd.getTags()).thenReturn(null);
spy.setParameters(sc, cmd, VlanType.VirtualNetwork, Boolean.FALSE);
List<IpAddress.State> states = Collections.singletonList(IpAddress.State.Free);
spy.setParameters(sc, cmd, VlanType.VirtualNetwork, Boolean.FALSE, states);
Mockito.verify(sc, Mockito.times(1)).setJoinParameters("vlanSearch", "vlanType", VlanType.VirtualNetwork);
Mockito.verify(sc, Mockito.times(1)).setParameters("display", false);
Mockito.verify(sc, Mockito.times(1)).setParameters("sourceNetworkId", 10L);
Mockito.verify(sc, Mockito.times(1)).setParameters("state", "Free");
Mockito.verify(sc, Mockito.times(1)).setParameters("state", states.toArray());
Mockito.verify(sc, Mockito.times(1)).setParameters("forsystemvms", false);
}
@ -281,14 +282,14 @@ public class ManagementServerImplTest {
Mockito.when(cmd.getId()).thenReturn(null);
Mockito.when(cmd.isSourceNat()).thenReturn(null);
Mockito.when(cmd.isStaticNat()).thenReturn(null);
Mockito.when(cmd.getState()).thenReturn(IpAddress.State.Free.name());
Mockito.when(cmd.getTags()).thenReturn(null);
spy.setParameters(sc, cmd, VlanType.VirtualNetwork, Boolean.FALSE);
List<IpAddress.State> states = Collections.singletonList(IpAddress.State.Free);
spy.setParameters(sc, cmd, VlanType.VirtualNetwork, Boolean.FALSE, states);
Mockito.verify(sc, Mockito.times(1)).setJoinParameters("vlanSearch", "vlanType", VlanType.VirtualNetwork);
Mockito.verify(sc, Mockito.times(1)).setParameters("display", false);
Mockito.verify(sc, Mockito.times(1)).setParameters("sourceNetworkId", 10L);
Mockito.verify(sc, Mockito.times(1)).setParameters("state", "Free");
Mockito.verify(sc, Mockito.times(1)).setParameters("state", states.toArray());
Mockito.verify(sc, Mockito.times(1)).setParameters("forsystemvms", false);
}
@ -304,13 +305,13 @@ public class ManagementServerImplTest {
Mockito.when(cmd.getId()).thenReturn(null);
Mockito.when(cmd.isSourceNat()).thenReturn(null);
Mockito.when(cmd.isStaticNat()).thenReturn(null);
Mockito.when(cmd.getState()).thenReturn(null);
Mockito.when(cmd.getTags()).thenReturn(null);
spy.setParameters(sc, cmd, VlanType.VirtualNetwork, Boolean.TRUE);
spy.setParameters(sc, cmd, VlanType.VirtualNetwork, Boolean.TRUE, Collections.emptyList());
Mockito.verify(sc, Mockito.times(1)).setJoinParameters("vlanSearch", "vlanType", VlanType.VirtualNetwork);
Mockito.verify(sc, Mockito.times(1)).setParameters("display", false);
Mockito.verify(sc, Mockito.times(1)).setParameters("sourceNetworkId", 10L);
Mockito.verify(sc, Mockito.times(1)).setParameters("state", IpAddress.State.Allocated);
Mockito.verify(sc, Mockito.times(1)).setParameters("forsystemvms", false);
}
@ -326,13 +327,13 @@ public class ManagementServerImplTest {
Mockito.when(cmd.getId()).thenReturn(null);
Mockito.when(cmd.isSourceNat()).thenReturn(null);
Mockito.when(cmd.isStaticNat()).thenReturn(null);
Mockito.when(cmd.getState()).thenReturn(null);
Mockito.when(cmd.getTags()).thenReturn(null);
spy.setParameters(sc, cmd, VlanType.VirtualNetwork, Boolean.TRUE);
spy.setParameters(sc, cmd, VlanType.VirtualNetwork, Boolean.TRUE, Collections.emptyList());
Mockito.verify(sc, Mockito.times(1)).setJoinParameters("vlanSearch", "vlanType", VlanType.VirtualNetwork);
Mockito.verify(sc, Mockito.times(1)).setParameters("display", false);
Mockito.verify(sc, Mockito.times(1)).setParameters("sourceNetworkId", 10L);
Mockito.verify(sc, Mockito.times(1)).setParameters("state", IpAddress.State.Allocated);
Mockito.verify(sc, Mockito.times(1)).setParameters("forsystemvms", false);
}
@ -1033,4 +1034,49 @@ public class ManagementServerImplTest {
Assert.assertNotNull(spy.getExternalVmConsole(virtualMachine, host));
Mockito.verify(extensionManager).getInstanceConsole(virtualMachine, host);
}
@Test
public void getStatesForIpAddressSearchReturnsValidStates() {
ListPublicIpAddressesCmd cmd = Mockito.mock(ListPublicIpAddressesCmd.class);
Mockito.when(cmd.getState()).thenReturn("Allocated ,free");
List<IpAddress.State> result = spy.getStatesForIpAddressSearch(cmd);
Assert.assertEquals(2, result.size());
Assert.assertTrue(result.contains(IpAddress.State.Allocated));
Assert.assertTrue(result.contains(IpAddress.State.Free));
}
@Test
public void getStatesForIpAddressSearchReturnsEmptyListForNullState() {
ListPublicIpAddressesCmd cmd = Mockito.mock(ListPublicIpAddressesCmd.class);
Mockito.when(cmd.getState()).thenReturn(null);
List<IpAddress.State> result = spy.getStatesForIpAddressSearch(cmd);
Assert.assertTrue(result.isEmpty());
}
@Test
public void getStatesForIpAddressSearchReturnsEmptyListForBlankState() {
ListPublicIpAddressesCmd cmd = Mockito.mock(ListPublicIpAddressesCmd.class);
Mockito.when(cmd.getState()).thenReturn(" ");
List<IpAddress.State> result = spy.getStatesForIpAddressSearch(cmd);
Assert.assertTrue(result.isEmpty());
}
@Test(expected = InvalidParameterValueException.class)
public void getStatesForIpAddressSearchThrowsExceptionForInvalidState() {
ListPublicIpAddressesCmd cmd = Mockito.mock(ListPublicIpAddressesCmd.class);
Mockito.when(cmd.getState()).thenReturn("InvalidState");
spy.getStatesForIpAddressSearch(cmd);
}
@Test
public void getStatesForIpAddressSearchHandlesMixedValidAndInvalidStates() {
ListPublicIpAddressesCmd cmd = Mockito.mock(ListPublicIpAddressesCmd.class);
Mockito.when(cmd.getState()).thenReturn("Allocated,InvalidState");
try {
spy.getStatesForIpAddressSearch(cmd);
Assert.fail("Expected InvalidParameterValueException to be thrown");
} catch (InvalidParameterValueException e) {
Assert.assertEquals("Invalid state: InvalidState", e.getMessage());
}
}
}

View File

@ -43,6 +43,7 @@
- defaultOption (Object, optional): Preselected object to include initially
- showIcon (Boolean, optional): Whether to show icon for the options. Default is true
- defaultIcon (String, optional): Icon to be shown when there is no resource icon for the option. Default is 'cloud-outlined'
- autoSelectFirstOption (Boolean, optional): Whether to automatically select the first option when options are loaded. Default is false
Events:
- @change-option-value (Function): Emits the selected option value(s) when value(s) changes. Do not use @change as it will give warnings and may not work
@ -81,7 +82,7 @@
<resource-icon v-if="option.icon && option.icon.base64image" :image="option.icon.base64image" size="1x" style="margin-right: 5px"/>
<render-icon v-else :icon="defaultIcon" style="margin-right: 5px" />
</span>
<span>{{ option[optionLabelKey] }}</span>
<span>{{ optionLabelFn ? optionLabelFn(option) : option[optionLabelKey] }}</span>
</span>
</a-select-option>
</a-select>
@ -120,6 +121,10 @@ export default {
type: String,
default: 'name'
},
optionLabelFn: {
type: Function,
default: null
},
defaultOption: {
type: Object,
default: null
@ -135,6 +140,10 @@ export default {
pageSize: {
type: Number,
default: null
},
autoSelectFirstOption: {
type: Boolean,
default: false
}
},
data () {
@ -147,11 +156,12 @@ export default {
searchTimer: null,
scrollHandlerAttached: false,
preselectedOptionValue: null,
successiveFetches: 0
successiveFetches: 0,
canSelectFirstOption: false
}
},
created () {
this.addDefaultOptionIfNeeded(true)
this.addDefaultOptionIfNeeded()
},
mounted () {
this.preselectedOptionValue = this.$attrs.value
@ -208,6 +218,7 @@ export default {
}).catch(error => {
this.$notifyError(error)
}).finally(() => {
this.canSelectFirstOption = true
if (this.successiveFetches === 0) {
this.loading = false
}
@ -218,6 +229,12 @@ export default {
(Array.isArray(this.preselectedOptionValue) && this.preselectedOptionValue.length === 0) ||
this.successiveFetches >= this.maxSuccessiveFetches) {
this.resetPreselectedOptionValue()
if (!this.canSelectFirstOption && this.autoSelectFirstOption && this.options.length > 0) {
this.$nextTick(() => {
this.preselectedOptionValue = this.options[0][this.optionValueKey]
this.onChange(this.preselectedOptionValue)
})
}
return
}
const matchValue = Array.isArray(this.preselectedOptionValue) ? this.preselectedOptionValue[0] : this.preselectedOptionValue
@ -239,6 +256,7 @@ export default {
},
addDefaultOptionIfNeeded () {
if (this.defaultOption) {
this.canSelectFirstOption = true
this.options.push(this.defaultOption)
}
},

View File

@ -148,20 +148,17 @@
<a-alert :message="$t('message.action.acquire.ip')" type="warning" />
<a-form layout="vertical" style="margin-top: 10px">
<a-form-item :label="$t('label.ipaddress')">
<a-select
<infinite-scroll-select
v-focus="true"
style="width: 100%;"
v-model:value="acquireIp"
showSearch
optionFilterProp="label"
:filterOption="(input, option) => {
return option.label.toLowerCase().indexOf(input.toLowerCase()) >= 0
}" >
<a-select-option
v-for="ip in listPublicIpAddress"
:key="ip.ipaddress"
:label="ip.ipaddress + '(' + ip.state + ')'">{{ ip.ipaddress }} ({{ ip.state }})</a-select-option>
</a-select>
api="listPublicIpAddresses"
:apiParams="listApiParamsForAssociate"
resourceType="publicipaddress"
optionValueKey="ipaddress"
:optionLabelFn="ip => ip.ipaddress + ' (' + ip.state + ')'"
defaultIcon="environment-outlined"
:autoSelectFirstOption="true"
@change-option-value="(ip) => acquireIp = ip" />
</a-form-item>
<div :span="24" class="action-button">
<a-button @click="onCloseModal">{{ $t('label.cancel') }}</a-button>
@ -212,13 +209,15 @@ import Status from '@/components/widgets/Status'
import TooltipButton from '@/components/widgets/TooltipButton'
import BulkActionView from '@/components/view/BulkActionView'
import eventBus from '@/config/eventBus'
import InfiniteScrollSelect from '@/components/widgets/InfiniteScrollSelect'
export default {
name: 'IpAddressesTab',
components: {
Status,
TooltipButton,
BulkActionView
BulkActionView,
InfiniteScrollSelect
},
props: {
resource: {
@ -281,7 +280,6 @@ export default {
showAcquireIp: false,
acquireLoading: false,
acquireIp: null,
listPublicIpAddress: [],
changeSourceNat: false,
zoneExtNetProvider: ''
}
@ -302,6 +300,26 @@ export default {
}
},
inject: ['parentFetchData'],
computed: {
listApiParams () {
const params = {
zoneid: this.resource.zoneid,
domainid: this.resource.domainid,
account: this.resource.account,
forvirtualnetwork: true,
allocatedonly: false
}
if (['nsx', 'netris'].includes(this.zoneExtNetProvider?.toLowerCase())) {
params.forprovider = true
}
return params
},
listApiParamsForAssociate () {
const params = this.listApiParams
params.state = 'Free,Reserved'
return params
}
},
methods: {
fetchData () {
const params = {
@ -344,19 +362,9 @@ export default {
}).catch(reject)
})
},
fetchListPublicIpAddress () {
fetchListPublicIpAddress (state) {
return new Promise((resolve, reject) => {
const params = {
zoneid: this.resource.zoneid,
domainid: this.resource.domainid,
account: this.resource.account,
forvirtualnetwork: true,
allocatedonly: false
}
if (['nsx', 'netris'].includes(this.zoneExtNetProvider?.toLowerCase())) {
params.forprovider = true
}
getAPI('listPublicIpAddresses', params).then(json => {
getAPI('listPublicIpAddresses', this.listApiParams).then(json => {
const listPublicIps = json.listpublicipaddressesresponse.publicipaddress || []
resolve(listPublicIps)
}).catch(reject)
@ -554,30 +562,6 @@ export default {
},
async onShowAcquireIp () {
this.showAcquireIp = true
this.acquireLoading = true
this.listPublicIpAddress = []
try {
const listPublicIpAddress = await this.fetchListPublicIpAddress()
listPublicIpAddress.forEach(item => {
if (item.state === 'Free' || item.state === 'Reserved') {
this.listPublicIpAddress.push({
ipaddress: item.ipaddress,
state: item.state
})
}
})
this.listPublicIpAddress.sort(function (a, b) {
if (a.ipaddress < b.ipaddress) { return -1 }
if (a.ipaddress > b.ipaddress) { return 1 }
return 0
})
this.acquireIp = this.listPublicIpAddress && this.listPublicIpAddress.length > 0 ? this.listPublicIpAddress[0].ipaddress : null
this.acquireLoading = false
} catch (e) {
this.acquireLoading = false
this.$notifyError(e)
}
},
onCloseModal () {
this.showAcquireIp = false

View File

@ -19,7 +19,7 @@
package com.cloud.utils;
public class EnumUtils {
public class EnumUtils extends org.apache.commons.lang3.EnumUtils {
public static String listValues(Enum<?>[] enums) {
StringBuilder b = new StringBuilder("[");