diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/address/ListPublicIpAddressesCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/address/ListPublicIpAddressesCmd.java index 357f0c83ed74..57fd733d51d2 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/address/ListPublicIpAddressesCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/address/ListPublicIpAddressesCmd.java @@ -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") diff --git a/server/src/main/java/com/cloud/server/ManagementServerImpl.java b/server/src/main/java/com/cloud/server/ManagementServerImpl.java index 9e8fdb60694e..e6032662e926 100644 --- a/server/src/main/java/com/cloud/server/ManagementServerImpl.java +++ b/server/src/main/java/com/cloud/server/ManagementServerImpl.java @@ -823,6 +823,7 @@ 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 Pair, Integer> listConfigurationGroups return new Pair<>(result.first(), result.second()); } + protected List getStatesForIpAddressSearch(final ListPublicIpAddressesCmd cmd) { + final String statesStr = cmd.getState(); + final List 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, Integer> searchForIPAddresses(final ListPublicIpAddressesCmd cmd) { final Long associatedNetworkId = cmd.getAssociatedNetworkId(); @@ -2429,20 +2446,20 @@ public Pair, Integer> searchForIPAddresses(final ListP final Long networkId = cmd.getNetworkId(); final Long vpcId = cmd.getVpcId(); - final String state = cmd.getState(); + final List 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 Pair, Integer> searchForIPAddresses(final ListP Boolean isRecursive = cmd.isRecursive(); final List 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 domainIdRecursiveListProject = new Ternary<>(cmd.getDomainId(), cmd.isRecursive(), null); @@ -2538,7 +2553,7 @@ public Pair, Integer> searchForIPAddresses(final ListP buildParameters(sb, cmd, vlanType == VlanType.VirtualNetwork ? true : isAllocated); SearchCriteria 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 Pair, Integer> searchForIPAddresses(final ListP buildParameters(searchBuilder, cmd, false); SearchCriteria 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 Pair, Integer> searchForIPAddresses(final ListP sb2.and("quarantinedPublicIpsIdsNIN", sb2.entity().getId(), SearchCriteria.Op.NIN); SearchCriteria 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 @@ private void buildParameters(final SearchBuilder sb, final ListPubl 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 @@ private void buildParameters(final SearchBuilder sb, final ListPubl } } - protected void setParameters(SearchCriteria sc, final ListPublicIpAddressesCmd cmd, VlanType vlanType, Boolean isAllocated) { + protected void setParameters(SearchCriteria sc, final ListPublicIpAddressesCmd cmd, VlanType vlanType, + Boolean isAllocated, List states) { final Object keyword = cmd.getKeyword(); final Long physicalNetworkId = cmd.getPhysicalNetworkId(); final Long sourceNetworkId = cmd.getNetworkId(); @@ -2703,7 +2719,6 @@ protected void setParameters(SearchCriteria sc, final ListPublicIpA 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 tags = cmd.getTags(); @@ -2760,13 +2775,14 @@ protected void setParameters(SearchCriteria sc, final ListPublicIpA 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); diff --git a/server/src/test/java/com/cloud/server/ManagementServerImplTest.java b/server/src/test/java/com/cloud/server/ManagementServerImplTest.java index ebced92f8fe9..2f52df982b7a 100644 --- a/server/src/test/java/com/cloud/server/ManagementServerImplTest.java +++ b/server/src/test/java/com/cloud/server/ManagementServerImplTest.java @@ -26,6 +26,7 @@ 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 void setParametersTestWhenStateIsFreeAndSystemVmPublicIsTrue() throws Ill 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 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 void setParametersTestWhenStateIsFreeAndSystemVmPublicIsFalse() throws No 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 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 void setParametersTestWhenStateIsNullAndSystemVmPublicIsFalse() throws No 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 void setParametersTestWhenStateIsNullAndSystemVmPublicIsTrue() throws NoS 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 void testGetExternalVmConsole() { 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 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 result = spy.getStatesForIpAddressSearch(cmd); + Assert.assertTrue(result.isEmpty()); + } + + @Test + public void getStatesForIpAddressSearchReturnsEmptyListForBlankState() { + ListPublicIpAddressesCmd cmd = Mockito.mock(ListPublicIpAddressesCmd.class); + Mockito.when(cmd.getState()).thenReturn(" "); + List 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()); + } + } } diff --git a/ui/src/components/widgets/InfiniteScrollSelect.vue b/ui/src/components/widgets/InfiniteScrollSelect.vue index 608eeebf1332..122feafb2a00 100644 --- a/ui/src/components/widgets/InfiniteScrollSelect.vue +++ b/ui/src/components/widgets/InfiniteScrollSelect.vue @@ -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 @@ - {{ option[optionLabelKey] }} + {{ optionLabelFn ? optionLabelFn(option) : option[optionLabelKey] }} @@ -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) } }, diff --git a/ui/src/views/network/IpAddressesTab.vue b/ui/src/views/network/IpAddressesTab.vue index ee9d1eff42bc..37ebc29f19d9 100644 --- a/ui/src/views/network/IpAddressesTab.vue +++ b/ui/src/views/network/IpAddressesTab.vue @@ -148,20 +148,17 @@ - - {{ ip.ipaddress }} ({{ ip.state }}) - + 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" />
{{ $t('label.cancel') }} @@ -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 diff --git a/utils/src/main/java/com/cloud/utils/EnumUtils.java b/utils/src/main/java/com/cloud/utils/EnumUtils.java index 02b6a25b8955..380b595a0ad1 100644 --- a/utils/src/main/java/com/cloud/utils/EnumUtils.java +++ b/utils/src/main/java/com/cloud/utils/EnumUtils.java @@ -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("[");