Skip to content

Ownership selection in VPC tiers and VPC public IPs #9692

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -829,6 +829,7 @@ public class ApiConstants {
public static final String VPC_OFF_ID = "vpcofferingid";
public static final String VPC_OFF_NAME = "vpcofferingname";
public static final String NETWORK = "network";
public static final String VPC_ACCESS = "vpcaccess";
public static final String VPC_ID = "vpcid";
public static final String VPC_NAME = "vpcname";
public static final String GATEWAY_ID = "gatewayid";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ public void execute() {
private void updateNetworkResponse(List<NetworkResponse> response) {
for (NetworkResponse networkResponse : response) {
ResourceIcon resourceIcon = resourceIconManager.getByResourceTypeAndUuid(ResourceTag.ResourceObjectType.Network, networkResponse.getId());
if (resourceIcon == null && networkResponse.getVpcId() != null) {
if (resourceIcon == null && networkResponse.getVpcId() != null && networkResponse.getVpcAccess()) {
resourceIcon = resourceIconManager.getByResourceTypeAndUuid(ResourceTag.ResourceObjectType.Vpc, networkResponse.getVpcId());
}
if (resourceIcon == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,10 @@
@Param(description = "purpose of the IP address. In Acton this value is not null for Ips with isSystem=true, and can have either StaticNat or LB value")
private String purpose;

@SerializedName(ApiConstants.VPC_ACCESS)
@Param(description = "Whether the calling account has access to this network's VPC", since = "4.21.0")
private boolean vpcAccess;

@SerializedName(ApiConstants.VPC_ID)
@Param(description = "VPC id the ip belongs to")
private String vpcId;
Expand Down Expand Up @@ -297,6 +301,10 @@
this.purpose = purpose;
}

public void setVpcAccess(boolean vpcAccess) {
this.vpcAccess = vpcAccess;
}

Check warning on line 306 in api/src/main/java/org/apache/cloudstack/api/response/IPAddressResponse.java

View check run for this annotation

Codecov / codecov/patch

api/src/main/java/org/apache/cloudstack/api/response/IPAddressResponse.java#L304-L306

Added lines #L304 - L306 were not covered by tests

public void setVpcId(String vpcId) {
this.vpcId = vpcId;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,10 @@
@Param(description = "true if network supports specifying ip ranges, false otherwise")
private Boolean specifyIpRanges;

@SerializedName(ApiConstants.VPC_ACCESS)
@Param(description = "Whether the calling account has access to this network's VPC", since = "4.21.0")
private Boolean vpcAccess;

@SerializedName(ApiConstants.VPC_ID)
@Param(description = "VPC the network belongs to")
private String vpcId;
Expand Down Expand Up @@ -491,6 +495,14 @@
this.specifyIpRanges = specifyIpRanges;
}

public void setVpcAccess(boolean vpcAccess) {
this.vpcAccess = vpcAccess;
}

Check warning on line 501 in api/src/main/java/org/apache/cloudstack/api/response/NetworkResponse.java

View check run for this annotation

Codecov / codecov/patch

api/src/main/java/org/apache/cloudstack/api/response/NetworkResponse.java#L499-L501

Added lines #L499 - L501 were not covered by tests
public Boolean getVpcAccess() {
return vpcAccess;
}

Check warning on line 505 in api/src/main/java/org/apache/cloudstack/api/response/NetworkResponse.java

View check run for this annotation

Codecov / codecov/patch

api/src/main/java/org/apache/cloudstack/api/response/NetworkResponse.java#L503-L505

Added lines #L503 - L505 were not covered by tests
public void setVpcId(String vpcId) {
this.vpcId = vpcId;
}
Expand Down
60 changes: 38 additions & 22 deletions server/src/main/java/com/cloud/api/ApiResponseHelper.java
Original file line number Diff line number Diff line change
Expand Up @@ -1072,7 +1072,7 @@
}


setVpcIdInResponse(ipAddr.getVpcId(), ipResponse::setVpcId, ipResponse::setVpcName);
setVpcIdInResponse(ipAddr.getVpcId(), ipResponse::setVpcId, ipResponse::setVpcName, ipResponse::setVpcAccess);

Check warning on line 1075 in server/src/main/java/com/cloud/api/ApiResponseHelper.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/api/ApiResponseHelper.java#L1075

Added line #L1075 was not covered by tests


// Network id the ip is associated with (if associated networkId is
Expand Down Expand Up @@ -1144,20 +1144,43 @@
return ipResponse;
}

protected void setVpcIdInResponse(Long vpcId, Consumer<String> vpcUuidSetter, Consumer<String> vpcNameSetter, Consumer<Boolean> vpcAccessSetter) {
if (vpcId == null) {
return;
}
Vpc vpc = ApiDBUtils.findVpcById(vpcId);
if (vpc == null) {

Check warning on line 1152 in server/src/main/java/com/cloud/api/ApiResponseHelper.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/api/ApiResponseHelper.java#L1152

Added line #L1152 was not covered by tests
return;
}

Check warning on line 1154 in server/src/main/java/com/cloud/api/ApiResponseHelper.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/api/ApiResponseHelper.java#L1154

Added line #L1154 was not covered by tests

private void setVpcIdInResponse(Long vpcId, Consumer<String> vpcUuidSetter, Consumer<String> vpcNameSetter) {
if (vpcId != null) {
Vpc vpc = ApiDBUtils.findVpcById(vpcId);
if (vpc != null) {
try {
_accountMgr.checkAccess(CallContext.current().getCallingAccount(), null, false, vpc);
vpcUuidSetter.accept(vpc.getUuid());
} catch (PermissionDeniedException e) {
logger.debug("Not setting the vpcId to the response because the caller does not have access to the VPC");
}
vpcNameSetter.accept(vpc.getName());
}
try {
_accountMgr.checkAccess(CallContext.current().getCallingAccount(), null, false, vpc);
vpcAccessSetter.accept(true);

Check warning on line 1158 in server/src/main/java/com/cloud/api/ApiResponseHelper.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/api/ApiResponseHelper.java#L1158

Added line #L1158 was not covered by tests
} catch (PermissionDeniedException e) {
vpcAccessSetter.accept(false);
logger.debug("Setting [{}] as false because the caller does not have access to the VPC [{}].", ApiConstants.VPC_ACCESS, vpc);

Check warning on line 1161 in server/src/main/java/com/cloud/api/ApiResponseHelper.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/api/ApiResponseHelper.java#L1160-L1161

Added lines #L1160 - L1161 were not covered by tests
}
vpcNameSetter.accept(vpc.getName());
vpcUuidSetter.accept(vpc.getUuid());
}

Check warning on line 1165 in server/src/main/java/com/cloud/api/ApiResponseHelper.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/api/ApiResponseHelper.java#L1164-L1165

Added lines #L1164 - L1165 were not covered by tests

protected void setAclIdInResponse(Network network, NetworkResponse response) {
if (network.getNetworkACLId() == null) {
return;
}

NetworkACL acl = ApiDBUtils.findByNetworkACLId(network.getNetworkACLId());

Check warning on line 1172 in server/src/main/java/com/cloud/api/ApiResponseHelper.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/api/ApiResponseHelper.java#L1167-L1172

Added lines #L1167 - L1172 were not covered by tests
if (acl == null) {
return;
}

if (Boolean.FALSE.equals(response.getVpcAccess()) && acl.getVpcId() != 0) {

Check warning on line 1177 in server/src/main/java/com/cloud/api/ApiResponseHelper.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/api/ApiResponseHelper.java#L1176-L1177

Added lines #L1176 - L1177 were not covered by tests
logger.debug("[{}] not set in response, since caller does not have access to it.", acl);
return;
}

response.setAclId(acl.getUuid());
response.setAclName(acl.getName());
}

private void showVmInfoForSharedNetworks(boolean forVirtualNetworks, IpAddress ipAddr, IPAddressResponse ipResponse) {
Expand Down Expand Up @@ -2657,7 +2680,8 @@
response.setSpecifyIpRanges(network.getSpecifyIpRanges());


setVpcIdInResponse(network.getVpcId(), response::setVpcId, response::setVpcName);
setVpcIdInResponse(network.getVpcId(), response::setVpcId, response::setVpcName, response::setVpcAccess);
setAclIdInResponse(network, response);

Check warning on line 2684 in server/src/main/java/com/cloud/api/ApiResponseHelper.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/api/ApiResponseHelper.java#L2683-L2684

Added lines #L2683 - L2684 were not covered by tests

setResponseAssociatedNetworkInformation(response, network.getId());

Expand All @@ -2674,14 +2698,6 @@
response.setHasAnnotation(annotationDao.hasAnnotations(network.getUuid(), AnnotationService.EntityType.NETWORK.name(),
_accountMgr.isRootAdmin(CallContext.current().getCallingAccount().getId())));

if (network.getNetworkACLId() != null) {
NetworkACL acl = ApiDBUtils.findByNetworkACLId(network.getNetworkACLId());
if (acl != null) {
response.setAclId(acl.getUuid());
response.setAclName(acl.getName());
}
}

response.setStrechedL2Subnet(network.isStrechedL2Network());
if (network.isStrechedL2Network()) {
Set<String> networkSpannedZones = new HashSet<String>();
Expand Down
147 changes: 147 additions & 0 deletions server/src/test/java/com/cloud/api/ApiResponseHelperTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
import com.cloud.capacity.Capacity;
import com.cloud.configuration.Resource;
import com.cloud.domain.DomainVO;
import com.cloud.exception.PermissionDeniedException;
import com.cloud.network.Network;
import com.cloud.network.PublicIpQuarantine;
import com.cloud.network.as.AutoScaleVmGroup;
import com.cloud.network.as.AutoScaleVmGroupVO;
Expand All @@ -29,6 +31,8 @@
import com.cloud.network.dao.LoadBalancerVO;
import com.cloud.network.dao.NetworkServiceMapDao;
import com.cloud.network.dao.NetworkVO;
import com.cloud.network.vpc.NetworkACL;
import com.cloud.network.vpc.VpcVO;
import com.cloud.storage.VMTemplateVO;
import com.cloud.usage.UsageVO;
import com.cloud.user.Account;
Expand All @@ -46,6 +50,7 @@
import org.apache.cloudstack.api.response.AutoScaleVmProfileResponse;
import org.apache.cloudstack.api.response.DirectDownloadCertificateResponse;
import org.apache.cloudstack.api.response.IpQuarantineResponse;
import org.apache.cloudstack.api.response.NetworkResponse;
import org.apache.cloudstack.api.response.NicSecondaryIpResponse;
import org.apache.cloudstack.api.response.UnmanagedInstanceResponse;
import org.apache.cloudstack.api.response.UsageRecordResponse;
Expand Down Expand Up @@ -105,6 +110,13 @@ public class ApiResponseHelperTest {
@Mock
IPAddressDao ipAddressDaoMock;

@Mock
VpcVO vpcVOMock;

@Mock
NetworkACL networkACLMock;


@Spy
@InjectMocks
ApiResponseHelper apiResponseHelper = new ApiResponseHelper();
Expand All @@ -123,6 +135,9 @@ public class ApiResponseHelperTest {

static long autoScaleUserId = 7L;

static final String A_NAME = "name";
static final String A_UUID = "021f94d4-73f9-4a9a-b003-1df9dd968a09";

@Before
public void injectMocks() throws SecurityException, NoSuchFieldException,
IllegalArgumentException, IllegalAccessException {
Expand Down Expand Up @@ -481,4 +496,136 @@ public void testCapacityListingForSingleNonGpuType() {
Assert.assertTrue(apiResponseHelper.capacityListingForSingleNonGpuType(List.of(c1, c2)));
Assert.assertFalse(apiResponseHelper.capacityListingForSingleNonGpuType(List.of(c1, c2, c3)));
}

@Test
public void setVpcIdInResponseTestNullVpcIdReturnNull() {
NetworkResponse networkResponse = new NetworkResponse();

apiResponseHelper.setVpcIdInResponse(null, networkResponse::setVpcId, networkResponse::setVpcName, networkResponse::setVpcAccess);
Assert.assertNull(networkResponse.getVpcId());
Assert.assertNull(networkResponse.getVpcName());
Assert.assertNull(networkResponse.getVpcAccess());
}

@Test
public void setVpcIdInResponseTestNullVpcReturnNull() {
NetworkResponse networkResponse = new NetworkResponse();

try (MockedStatic<ApiDBUtils> utils = Mockito.mockStatic(ApiDBUtils.class)) {
utils.when(() -> ApiDBUtils.findVpcById(1L)).thenReturn(null);
apiResponseHelper.setVpcIdInResponse(1L, networkResponse::setVpcId, networkResponse::setVpcName, networkResponse::setVpcAccess);
}
Assert.assertNull(networkResponse.getVpcId());
Assert.assertNull(networkResponse.getVpcName());
Assert.assertNull(networkResponse.getVpcAccess());
}

@Test
public void setVpcIdInResponseCallerHasAccessReturnVpcAccessTrueAndVpcIdAndVpcName() {
NetworkResponse networkResponse = new NetworkResponse();
Mockito.when(vpcVOMock.getName()).thenReturn(A_NAME);
Mockito.when(vpcVOMock.getUuid()).thenReturn(A_UUID);

try (MockedStatic<ApiDBUtils> utils = Mockito.mockStatic(ApiDBUtils.class)) {
utils.when(() -> ApiDBUtils.findVpcById(1L)).thenReturn(vpcVOMock);
apiResponseHelper.setVpcIdInResponse(1L, networkResponse::setVpcId, networkResponse::setVpcName, networkResponse::setVpcAccess);
};
Assert.assertEquals(A_UUID, networkResponse.getVpcId());
Assert.assertEquals(A_NAME, networkResponse.getVpcName());
Assert.assertTrue(networkResponse.getVpcAccess());
}

@Test
public void setVpcIdInResponseCallerDoesNotHaveAccessReturnVpcAccessFalseAndVpcIdAndVpcName() {
NetworkResponse networkResponse = new NetworkResponse();
Mockito.when(vpcVOMock.getName()).thenReturn(A_NAME);
Mockito.when(vpcVOMock.getUuid()).thenReturn(A_UUID);

try (MockedStatic<ApiDBUtils> utils = Mockito.mockStatic(ApiDBUtils.class)) {
utils.when(() -> ApiDBUtils.findVpcById(1L)).thenReturn(vpcVOMock);
Mockito.doThrow(PermissionDeniedException.class).when(accountManagerMock).checkAccess(Mockito.any(), Mockito.any(), Mockito.anyBoolean(), Mockito.any());
apiResponseHelper.setVpcIdInResponse(1L, networkResponse::setVpcId, networkResponse::setVpcName, networkResponse::setVpcAccess);
};
Assert.assertEquals(A_UUID, networkResponse.getVpcId());
Assert.assertEquals(A_NAME, networkResponse.getVpcName());
Assert.assertFalse(networkResponse.getVpcAccess());
}

@Test
public void setAclIdInResponseTestNullNetworkAclIdReturnNull() {
NetworkResponse networkResponse = new NetworkResponse();
Network networkMock = Mockito.mock(Network.class);
Mockito.when(networkMock.getNetworkACLId()).thenReturn(null);

apiResponseHelper.setAclIdInResponse(networkMock, networkResponse);
Assert.assertNull(networkResponse.getAclId());
Assert.assertNull(networkResponse.getAclName());
}

@Test
public void setAclIdInResponseTestNullAclReturnNull() {
NetworkResponse networkResponse = new NetworkResponse();
Network networkMock = Mockito.mock(Network.class);
Mockito.when(networkMock.getNetworkACLId()).thenReturn(1L);

try (MockedStatic<ApiDBUtils> utils = Mockito.mockStatic(ApiDBUtils.class)) {
utils.when(() -> ApiDBUtils.findByNetworkACLId(1L)).thenReturn(null);
apiResponseHelper.setAclIdInResponse(networkMock, networkResponse);
}
Assert.assertNull(networkResponse.getAclId());
Assert.assertNull(networkResponse.getAclName());
}

@Test
public void setAclIdInResponseTestCallerDoesNotHaveAccessReturnNull() {
NetworkResponse networkResponse = new NetworkResponse();
networkResponse.setVpcAccess(false);
Network networkMock = Mockito.mock(Network.class);
Mockito.when(networkMock.getNetworkACLId()).thenReturn(1L);
Mockito.when(networkACLMock.getVpcId()).thenReturn(2L);

try (MockedStatic<ApiDBUtils> utils = Mockito.mockStatic(ApiDBUtils.class)) {
utils.when(() -> ApiDBUtils.findByNetworkACLId(1L)).thenReturn(networkACLMock);
apiResponseHelper.setAclIdInResponse(networkMock, networkResponse);
}
Assert.assertNull(networkResponse.getAclId());
Assert.assertNull(networkResponse.getAclName());
}

@Test
public void setAclIdInResponseTestCallerDoesNotHaveAccessButAclIsGlobalReturnAclIdAndAclName() {
NetworkResponse networkResponse = new NetworkResponse();
networkResponse.setVpcAccess(false);
Network networkMock = Mockito.mock(Network.class);
Mockito.when(networkMock.getNetworkACLId()).thenReturn(1L);
Mockito.when(networkACLMock.getVpcId()).thenReturn(0L);
Mockito.when(networkACLMock.getName()).thenReturn(A_NAME);
Mockito.when(networkACLMock.getUuid()).thenReturn(A_UUID);

try (MockedStatic<ApiDBUtils> utils = Mockito.mockStatic(ApiDBUtils.class)) {
utils.when(() -> ApiDBUtils.findByNetworkACLId(1L)).thenReturn(networkACLMock);
apiResponseHelper.setAclIdInResponse(networkMock, networkResponse);
}
Assert.assertEquals(A_UUID, networkResponse.getAclId());
Assert.assertEquals(A_NAME, networkResponse.getAclName());
}

@Test
public void setAclIdInResponseTestCallerHasAccessReturnAclIdAndAclName() {
NetworkResponse networkResponse = new NetworkResponse();
networkResponse.setVpcAccess(true);
Network networkMock = Mockito.mock(Network.class);
Mockito.when(networkMock.getNetworkACLId()).thenReturn(1L);
Mockito.lenient().when(networkACLMock.getVpcId()).thenReturn(2L);
Mockito.when(networkACLMock.getName()).thenReturn(A_NAME);
Mockito.when(networkACLMock.getUuid()).thenReturn(A_UUID);

try (MockedStatic<ApiDBUtils> utils = Mockito.mockStatic(ApiDBUtils.class)) {
utils.when(() -> ApiDBUtils.findByNetworkACLId(1L)).thenReturn(networkACLMock);
apiResponseHelper.setAclIdInResponse(networkMock, networkResponse);
}
Assert.assertEquals(A_UUID, networkResponse.getAclId());
Assert.assertEquals(A_NAME, networkResponse.getAclName());
}

}
3 changes: 2 additions & 1 deletion ui/src/components/view/InfoCard.vue
Original file line number Diff line number Diff line change
Expand Up @@ -489,7 +489,8 @@
<resource-icon :image="getImage(images.vpc)" size="1x" style="margin-right: 5px"/>
</span>
<deployment-unit-outlined v-else />
<router-link :to="{ path: '/vpc/' + resource.vpcid }">{{ resource.vpcname || resource.vpcid }}</router-link>
<router-link v-if="resource.vpcaccess" :to="{ path: '/vpc/' + resource.vpcid }">{{ resource.vpcname || resource.vpcid }}</router-link>
<span v-else>{{ resource.vpcname || resource.vpcid }}</span>
</div>
</div>

Expand Down
2 changes: 1 addition & 1 deletion ui/src/components/view/ListView.vue
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@
<router-link :to="{ path: '/guestnetwork/' + record.associatednetworkid }">{{ text }}</router-link>
</template>
<template v-if="column.key === 'vpcname'">
<a v-if="record.vpcid">
<a v-if="record.vpcid && record.vpcaccess">
<router-link :to="{ path: '/vpc/' + record.vpcid }">{{ text || record.vpcid }}</router-link>
</a>
<span v-else>{{ text }}</span>
Expand Down
Loading
Loading