Skip to content

Commit 7bbd249

Browse files
Pearl1594nvazquez
Pearl1594
and
nvazquez
committed
List only Netris Public IPs for NAT operations (#26)
* List only Netris Public IPs for NAT operations * rename getter and change type * fix failing unit tests * list all IPs if forProvider is not passed * fix list public IPs for external providers with additional IP range * filter provider Ips in a zone with external provider setup * Prevent acquiring IP that is not from the external provider range * formating --------- Co-authored-by: nvazquez <nicovazquez90@gmail.com>
1 parent 1c2c7e3 commit 7bbd249

File tree

9 files changed

+129
-9
lines changed

9 files changed

+129
-9
lines changed

‎api/src/main/java/org/apache/cloudstack/api/ApiConstants.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,7 @@ public class ApiConstants {
212212
public static final String FORMAT = "format";
213213
public static final String FOR_VIRTUAL_NETWORK = "forvirtualnetwork";
214214
public static final String FOR_SYSTEM_VMS = "forsystemvms";
215+
public static final String FOR_PROVIDER = "forprovider";
215216
public static final String FULL_PATH = "fullpath";
216217
public static final String GATEWAY = "gateway";
217218
public static final String IP6_GATEWAY = "ip6gateway";

‎api/src/main/java/org/apache/cloudstack/api/command/user/address/ListPublicIpAddressesCmd.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,9 @@ public class ListPublicIpAddressesCmd extends BaseListRetrieveOnlyResourceCountC
108108
@Parameter(name = ApiConstants.FOR_SYSTEM_VMS, type = CommandType.BOOLEAN, description = "true if range is dedicated for system VMs", since = "4.20.0")
109109
private Boolean forSystemVMs;
110110

111+
@Parameter(name = ApiConstants.FOR_PROVIDER, type = CommandType.BOOLEAN, description = "true if range is dedicated for external network provider", since = "4.20.0")
112+
private Boolean forProvider;
113+
111114
/////////////////////////////////////////////////////
112115
/////////////////// Accessors ///////////////////////
113116
/////////////////////////////////////////////////////
@@ -183,6 +186,10 @@ public boolean getForSystemVMs() {
183186
return BooleanUtils.isTrue(forSystemVMs);
184187
}
185188

189+
public boolean isForProvider() {
190+
return BooleanUtils.isTrue(forProvider);
191+
}
192+
186193
/////////////////////////////////////////////////////
187194
/////////////// API Implementation///////////////////
188195
/////////////////////////////////////////////////////

‎api/src/main/java/org/apache/cloudstack/api/response/IPAddressResponse.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,10 @@ public class IPAddressResponse extends BaseResponseWithAnnotations implements Co
175175
@Param(description="true if range is dedicated for System VMs")
176176
private boolean forSystemVms;
177177

178+
@SerializedName(ApiConstants.FOR_PROVIDER)
179+
@Param(description="true if range is dedicated for external network providers")
180+
private boolean forProvider;
181+
178182
public void setIpAddress(String ipAddress) {
179183
this.ipAddress = ipAddress;
180184
}
@@ -332,4 +336,6 @@ public void setHasRules(final boolean hasRules) {
332336
public void setForSystemVms(boolean forSystemVms) {
333337
this.forSystemVms = forSystemVms;
334338
}
339+
340+
public void setForProvider(boolean forProvider) { this.forProvider = forProvider; }
335341
}

‎server/src/main/java/com/cloud/api/ApiResponseHelper.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1168,6 +1168,9 @@ public IPAddressResponse createIPAddressResponse(ResponseView view, IpAddress ip
11681168

11691169
ipResponse.setPortable(ipAddr.isPortable());
11701170
ipResponse.setForSystemVms(ipAddr.isForSystemVms());
1171+
if (Objects.nonNull(getProviderFromVlanDetailKey(vlan))) {
1172+
ipResponse.setForProvider(true);
1173+
}
11711174

11721175
//set tag information
11731176
List<? extends ResourceTag> tags = ApiDBUtils.listByResourceTypeAndId(ResourceObjectType.PublicIpAddress, ipAddr.getId());

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

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,13 @@
3333

3434
import javax.inject.Inject;
3535

36+
import com.cloud.dc.VlanDetailsVO;
37+
import com.cloud.dc.dao.VlanDetailsDao;
38+
import com.cloud.network.dao.NetrisProviderDao;
39+
import com.cloud.network.dao.NsxProviderDao;
3640
import com.cloud.network.dao.PublicIpQuarantineDao;
41+
import com.cloud.network.element.NetrisProviderVO;
42+
import com.cloud.network.element.NsxProviderVO;
3743
import com.cloud.network.vo.PublicIpQuarantineVO;
3844
import com.cloud.resourcelimit.CheckedReservation;
3945
import org.apache.cloudstack.acl.ControlledEntity.ACLType;
@@ -189,6 +195,7 @@
189195
import com.cloud.vm.dao.NicSecondaryIpDao;
190196
import com.cloud.vm.dao.UserVmDao;
191197
import com.cloud.vm.dao.VMInstanceDao;
198+
import org.apache.commons.lang3.ObjectUtils;
192199

193200
public class IpAddressManagerImpl extends ManagerBase implements IpAddressManager, Configurable {
194201

@@ -313,6 +320,12 @@ public class IpAddressManagerImpl extends ManagerBase implements IpAddressManage
313320
private AnnotationDao annotationDao;
314321
@Inject
315322
MessageBus messageBus;
323+
@Inject
324+
NsxProviderDao nsxProviderDao;
325+
@Inject
326+
NetrisProviderDao netrisProviderDao;
327+
@Inject
328+
VlanDetailsDao vlanDetailsDao;
316329

317330
@Inject
318331
PublicIpQuarantineDao publicIpQuarantineDao;
@@ -1303,6 +1316,30 @@ public void releasePodIp(Long id) throws CloudRuntimeException {
13031316
}
13041317
}
13051318

1319+
/**
1320+
* When the zone is linked to external provider NSX or Netris: check if the IP to be associated is from the suitable pool
1321+
* Otherwise, no checks are performed
1322+
*/
1323+
private void checkPublicIpOnExternalProviderZone(DataCenter zone, String ip) {
1324+
long zoneId = zone.getId();
1325+
NetrisProviderVO netrisProvider = netrisProviderDao.findByZoneId(zoneId);
1326+
NsxProviderVO nsxProvider = nsxProviderDao.findByZoneId(zoneId);
1327+
if (ObjectUtils.allNull(netrisProvider, nsxProvider)) {
1328+
return;
1329+
}
1330+
IPAddressVO ipAddress = _ipAddressDao.findByIpAndDcId(zoneId, ip);
1331+
if (ipAddress != null) {
1332+
String detailKey = nsxProvider != null ? ApiConstants.NSX_DETAIL_KEY : ApiConstants.NETRIS_DETAIL_KEY;
1333+
VlanDetailsVO vlanDetailVO = vlanDetailsDao.findDetail(ipAddress.getVlanId(), detailKey);
1334+
if (vlanDetailVO == null || vlanDetailVO.getValue().equalsIgnoreCase("false")) {
1335+
String msg = String.format("Cannot acquire IP %s on the zone %s as the IP is not from the reserved pool " +
1336+
"for the external provider", ip, zone.getName());
1337+
logger.error(msg);
1338+
throw new CloudRuntimeException(msg);
1339+
}
1340+
}
1341+
}
1342+
13061343
@DB
13071344
@Override
13081345
public IpAddress allocateIp(final Account ipOwner, final boolean isSystem, Account caller, User callerUser, final DataCenter zone, final Boolean displayIp, final String ipaddress)
@@ -1311,6 +1348,8 @@ public IpAddress allocateIp(final Account ipOwner, final boolean isSystem, Accou
13111348
final VlanType vlanType = VlanType.VirtualNetwork;
13121349
final boolean assign = false;
13131350

1351+
checkPublicIpOnExternalProviderZone(zone, ipaddress);
1352+
13141353
if (Grouping.AllocationState.Disabled == zone.getAllocationState() && !_accountMgr.isRootAdmin(caller.getId())) {
13151354
// zone is of type DataCenter. See DataCenterVO.java.
13161355
PermissionDeniedException ex = new PermissionDeniedException(generateErrorMessageForOperationOnDisabledZone("allocate IP addresses", zone));

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

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,12 @@
4545
import javax.naming.ConfigurationException;
4646

4747
import com.cloud.cpu.CPU;
48+
import com.cloud.dc.VlanDetailsVO;
49+
import com.cloud.dc.dao.VlanDetailsDao;
50+
import com.cloud.network.dao.NetrisProviderDao;
51+
import com.cloud.network.dao.NsxProviderDao;
52+
53+
import com.cloud.utils.security.CertificateHelper;
4854
import org.apache.cloudstack.acl.ControlledEntity;
4955
import org.apache.cloudstack.acl.SecurityChecker;
5056
import org.apache.cloudstack.affinity.AffinityGroupProcessor;
@@ -881,6 +887,8 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
881887
@Inject
882888
private VlanDao _vlanDao;
883889
@Inject
890+
private VlanDetailsDao vlanDetailsDao;
891+
@Inject
884892
private AccountVlanMapDao _accountVlanMapDao;
885893
@Inject
886894
private PodVlanMapDao _podVlanMapDao;
@@ -923,7 +931,7 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
923931
@Inject
924932
private StoragePoolJoinDao _poolJoinDao;
925933
@Inject
926-
private NetworkDao networkDao;
934+
protected NetworkDao networkDao;
927935
@Inject
928936
private StorageManager _storageMgr;
929937
@Inject
@@ -993,7 +1001,7 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
9931001
@Inject
9941002
private NetworkModel _networkMgr;
9951003
@Inject
996-
private VpcDao _vpcDao;
1004+
protected VpcDao _vpcDao;
9971005
@Inject
9981006
private DomainVlanMapDao _domainVlanMapDao;
9991007
@Inject
@@ -1023,6 +1031,10 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
10231031
protected AffinityGroupVMMapDao _affinityGroupVMMapDao;
10241032
@Inject
10251033
ResourceLimitService resourceLimitService;
1034+
@Inject
1035+
NsxProviderDao nsxProviderDao;
1036+
@Inject
1037+
NetrisProviderDao netrisProviderDao;
10261038

10271039
private LockControllerListener _lockControllerListener;
10281040
private final ScheduledExecutorService _eventExecutor = Executors.newScheduledThreadPool(1, new NamedThreadFactory("EventChecker"));
@@ -2575,6 +2587,7 @@ private void buildParameters(final SearchBuilder<IPAddressVO> sb, final ListPubl
25752587
final String address = cmd.getIpAddress();
25762588
final Boolean forLoadBalancing = cmd.isForLoadBalancing();
25772589
final Map<String, String> tags = cmd.getTags();
2590+
boolean forProvider = cmd.isForProvider();
25782591

25792592
sb.and("dataCenterId", sb.entity().getDataCenterId(), SearchCriteria.Op.EQ);
25802593
sb.and("address", sb.entity().getAddress(), SearchCriteria.Op.EQ);
@@ -2620,13 +2633,21 @@ private void buildParameters(final SearchBuilder<IPAddressVO> sb, final ListPubl
26202633
if (isAllocated != null && isAllocated) {
26212634
sb.and("allocated", sb.entity().getAllocatedTime(), SearchCriteria.Op.NNULL);
26222635
}
2636+
2637+
if (forProvider) {
2638+
SearchBuilder<VlanDetailsVO> vlanDetailsSearch = vlanDetailsDao.createSearchBuilder();
2639+
vlanDetailsSearch.and("name", vlanDetailsSearch.entity().getName(), SearchCriteria.Op.IN);
2640+
vlanDetailsSearch.and("value", vlanDetailsSearch.entity().getValue(), SearchCriteria.Op.EQ);
2641+
sb.join("vlanDetailSearch", vlanDetailsSearch, sb.entity().getVlanId(), vlanDetailsSearch.entity().getResourceId(), JoinType.LEFT);
2642+
}
26232643
}
26242644

26252645
protected void setParameters(SearchCriteria<IPAddressVO> sc, final ListPublicIpAddressesCmd cmd, VlanType vlanType, Boolean isAllocated) {
26262646
final Object keyword = cmd.getKeyword();
26272647
final Long physicalNetworkId = cmd.getPhysicalNetworkId();
26282648
final Long sourceNetworkId = cmd.getNetworkId();
2629-
final Long zone = cmd.getZoneId();
2649+
final Long vpcId = cmd.getVpcId();
2650+
Long zone = cmd.getZoneId();
26302651
final String address = cmd.getIpAddress();
26312652
final Long vlan = cmd.getVlanId();
26322653
final Long ipId = cmd.getId();
@@ -2635,6 +2656,7 @@ protected void setParameters(SearchCriteria<IPAddressVO> sc, final ListPublicIpA
26352656
final Boolean forDisplay = cmd.getDisplay();
26362657
final String state = cmd.getState();
26372658
final Boolean forSystemVms = cmd.getForSystemVMs();
2659+
final boolean forProvider = cmd.isForProvider();
26382660
final Map<String, String> tags = cmd.getTags();
26392661

26402662
sc.setJoinParameters("vlanSearch", "vlanType", vlanType);
@@ -2700,6 +2722,11 @@ protected void setParameters(SearchCriteria<IPAddressVO> sc, final ListPublicIpA
27002722
} else {
27012723
sc.setParameters(FOR_SYSTEMVMS, forSystemVms);
27022724
}
2725+
2726+
if (forProvider) {
2727+
sc.setJoinParameters("vlanDetailSearch", "name", ApiConstants.NETRIS_DETAIL_KEY, ApiConstants.NSX_DETAIL_KEY);
2728+
sc.setJoinParameters("vlanDetailSearch", "value", "true");
2729+
}
27032730
}
27042731

27052732
@Override

‎server/src/test/java/com/cloud/server/ManagementServerImplTest.java‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@
2525
import com.cloud.network.IpAddress;
2626
import com.cloud.network.IpAddressManagerImpl;
2727
import com.cloud.network.dao.IPAddressVO;
28+
import com.cloud.network.dao.NetworkDao;
29+
import com.cloud.network.vpc.dao.VpcDao;
2830
import com.cloud.storage.VMTemplateVO;
2931
import com.cloud.storage.dao.VMTemplateDao;
3032
import com.cloud.user.Account;
@@ -123,6 +125,12 @@ public class ManagementServerImplTest {
123125
@Mock
124126
UserDataManager userDataManager;
125127

128+
@Mock
129+
VpcDao vpcDao;
130+
131+
@Mock
132+
NetworkDao networkDao;
133+
126134
@Spy
127135
ManagementServerImpl spy = new ManagementServerImpl();
128136

@@ -145,6 +153,8 @@ public void setup() throws IllegalAccessException, NoSuchFieldException {
145153
spy._UserVmDetailsDao = userVmDetailsDao;
146154
spy._detailsDao = hostDetailsDao;
147155
spy.userDataManager = userDataManager;
156+
spy._vpcDao = vpcDao;
157+
spy.networkDao = networkDao;
148158
}
149159

150160
@After

‎ui/src/views/AutogenView.vue‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1004,6 +1004,19 @@ export default {
10041004
}
10051005
10061006
this.items = json[responseName][objectName]
1007+
var filteredItems = []
1008+
if (this.apiName === 'listPublicIpAddresses') {
1009+
for (var zone of this.$store.getters.zones) {
1010+
const zoneIps = this.items.filter(item => item.zoneid === zone.id)
1011+
const providerIps = zoneIps.filter(item => item.forprovider === true)
1012+
if (providerIps.length === 0) {
1013+
filteredItems.push(...zoneIps)
1014+
} else {
1015+
filteredItems.push(...providerIps)
1016+
}
1017+
}
1018+
this.items = filteredItems
1019+
}
10071020
if (!this.items || this.items.length === 0) {
10081021
this.items = []
10091022
}
@@ -1934,7 +1947,6 @@ export default {
19341947
this.rules[field.name].push(rule)
19351948
break
19361949
case (this.currentAction.mapping && field.name in this.currentAction.mapping && 'options' in this.currentAction.mapping[field.name]):
1937-
console.log('op: ' + field)
19381950
rule.required = field.required
19391951
rule.message = this.$t('message.error.select')
19401952
this.rules[field.name].push(rule)
@@ -1945,20 +1957,17 @@ export default {
19451957
this.rules[field.name].push(rule)
19461958
break
19471959
case (field.type === 'uuid'):
1948-
console.log('uuid: ' + field)
19491960
rule.required = field.required
19501961
rule.message = this.$t('message.error.select')
19511962
this.rules[field.name].push(rule)
19521963
break
19531964
case (field.type === 'list'):
1954-
console.log('list: ' + field)
19551965
rule.type = 'array'
19561966
rule.required = field.required
19571967
rule.message = this.$t('message.error.select')
19581968
this.rules[field.name].push(rule)
19591969
break
19601970
case (field.type === 'long'):
1961-
console.log(field)
19621971
rule.type = 'number'
19631972
rule.required = field.required
19641973
rule.message = this.$t('message.validate.number')

‎ui/src/views/network/IpAddressesTab.vue‎

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -282,10 +282,12 @@ export default {
282282
acquireLoading: false,
283283
acquireIp: null,
284284
listPublicIpAddress: [],
285-
changeSourceNat: false
285+
changeSourceNat: false,
286+
zoneExtNetProvider: ''
286287
}
287288
},
288-
created () {
289+
async created () {
290+
await this.fetchZones()
289291
this.fetchData()
290292
},
291293
watch: {
@@ -321,6 +323,9 @@ export default {
321323
} else {
322324
params.associatednetworkid = this.resource.id
323325
}
326+
if (['nsx', 'netris'].includes(this.zoneExtNetProvider?.toLowerCase())) {
327+
params.forprovider = true
328+
}
324329
this.fetchLoading = true
325330
api('listPublicIpAddresses', params).then(json => {
326331
this.totalIps = json.listpublicipaddressesresponse.count || 0
@@ -329,6 +334,16 @@ export default {
329334
this.fetchLoading = false
330335
})
331336
},
337+
fetchZones () {
338+
return new Promise((resolve, reject) => {
339+
api('listZones', {
340+
id: this.resource.zoneid
341+
}).then(json => {
342+
this.zoneExtNetProvider = json?.listzonesresponse?.zone?.[0]?.provider || null
343+
resolve(this.zoneExtNetProvider)
344+
}).catch(reject)
345+
})
346+
},
332347
fetchListPublicIpAddress () {
333348
return new Promise((resolve, reject) => {
334349
const params = {
@@ -338,6 +353,9 @@ export default {
338353
forvirtualnetwork: true,
339354
allocatedonly: false
340355
}
356+
if (['nsx', 'netris'].includes(this.zoneExtNetProvider?.toLowerCase())) {
357+
params.forprovider = true
358+
}
341359
api('listPublicIpAddresses', params).then(json => {
342360
const listPublicIps = json.listpublicipaddressesresponse.publicipaddress || []
343361
resolve(listPublicIps)

0 commit comments

Comments
 (0)