diff --git a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java index 2f0e4f16797d..e5b567d1aa91 100644 --- a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java +++ b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java @@ -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"; diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/network/ListNetworksCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/network/ListNetworksCmd.java index 0e8425b14b4b..000ab48bb08f 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/network/ListNetworksCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/network/ListNetworksCmd.java @@ -233,7 +233,7 @@ public void execute() { private void updateNetworkResponse(List 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) { diff --git a/api/src/main/java/org/apache/cloudstack/api/response/IPAddressResponse.java b/api/src/main/java/org/apache/cloudstack/api/response/IPAddressResponse.java index 0018edc86388..06811bf7f8f4 100644 --- a/api/src/main/java/org/apache/cloudstack/api/response/IPAddressResponse.java +++ b/api/src/main/java/org/apache/cloudstack/api/response/IPAddressResponse.java @@ -143,6 +143,10 @@ public class IPAddressResponse extends BaseResponseWithAnnotations implements Co @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; @@ -297,6 +301,10 @@ public void setPurpose(String purpose) { this.purpose = purpose; } + public void setVpcAccess(boolean vpcAccess) { + this.vpcAccess = vpcAccess; + } + public void setVpcId(String vpcId) { this.vpcId = vpcId; } diff --git a/api/src/main/java/org/apache/cloudstack/api/response/NetworkResponse.java b/api/src/main/java/org/apache/cloudstack/api/response/NetworkResponse.java index 1049740bf55a..df52ea43011a 100644 --- a/api/src/main/java/org/apache/cloudstack/api/response/NetworkResponse.java +++ b/api/src/main/java/org/apache/cloudstack/api/response/NetworkResponse.java @@ -191,6 +191,10 @@ public class NetworkResponse extends BaseResponseWithAssociatedNetwork implement @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; @@ -491,6 +495,14 @@ public void setSpecifyIpRanges(Boolean specifyIpRanges) { this.specifyIpRanges = specifyIpRanges; } + public void setVpcAccess(boolean vpcAccess) { + this.vpcAccess = vpcAccess; + } + + public Boolean getVpcAccess() { + return vpcAccess; + } + public void setVpcId(String vpcId) { this.vpcId = vpcId; } diff --git a/server/src/main/java/com/cloud/api/ApiResponseHelper.java b/server/src/main/java/com/cloud/api/ApiResponseHelper.java index 01334c5d4c5a..b278616483e2 100644 --- a/server/src/main/java/com/cloud/api/ApiResponseHelper.java +++ b/server/src/main/java/com/cloud/api/ApiResponseHelper.java @@ -1072,7 +1072,7 @@ public IPAddressResponse createIPAddressResponse(ResponseView view, IpAddress ip } - setVpcIdInResponse(ipAddr.getVpcId(), ipResponse::setVpcId, ipResponse::setVpcName); + setVpcIdInResponse(ipAddr.getVpcId(), ipResponse::setVpcId, ipResponse::setVpcName, ipResponse::setVpcAccess); // Network id the ip is associated with (if associated networkId is @@ -1144,20 +1144,43 @@ public IPAddressResponse createIPAddressResponse(ResponseView view, IpAddress ip return ipResponse; } + protected void setVpcIdInResponse(Long vpcId, Consumer vpcUuidSetter, Consumer vpcNameSetter, Consumer vpcAccessSetter) { + if (vpcId == null) { + return; + } + Vpc vpc = ApiDBUtils.findVpcById(vpcId); + if (vpc == null) { + return; + } - private void setVpcIdInResponse(Long vpcId, Consumer vpcUuidSetter, Consumer 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); + } 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); + } + vpcNameSetter.accept(vpc.getName()); + vpcUuidSetter.accept(vpc.getUuid()); + } + + protected void setAclIdInResponse(Network network, NetworkResponse response) { + if (network.getNetworkACLId() == null) { + return; + } + + NetworkACL acl = ApiDBUtils.findByNetworkACLId(network.getNetworkACLId()); + if (acl == null) { + return; } + + if (Boolean.FALSE.equals(response.getVpcAccess()) && acl.getVpcId() != 0) { + 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) { @@ -2657,7 +2680,8 @@ public NetworkResponse createNetworkResponse(ResponseView view, Network network) response.setSpecifyIpRanges(network.getSpecifyIpRanges()); - setVpcIdInResponse(network.getVpcId(), response::setVpcId, response::setVpcName); + setVpcIdInResponse(network.getVpcId(), response::setVpcId, response::setVpcName, response::setVpcAccess); + setAclIdInResponse(network, response); setResponseAssociatedNetworkInformation(response, network.getId()); @@ -2674,14 +2698,6 @@ public NetworkResponse createNetworkResponse(ResponseView view, Network network) 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 networkSpannedZones = new HashSet(); diff --git a/server/src/test/java/com/cloud/api/ApiResponseHelperTest.java b/server/src/test/java/com/cloud/api/ApiResponseHelperTest.java index a68623aa1443..1c54f51e499d 100644 --- a/server/src/test/java/com/cloud/api/ApiResponseHelperTest.java +++ b/server/src/test/java/com/cloud/api/ApiResponseHelperTest.java @@ -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; @@ -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; @@ -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; @@ -105,6 +110,13 @@ public class ApiResponseHelperTest { @Mock IPAddressDao ipAddressDaoMock; + @Mock + VpcVO vpcVOMock; + + @Mock + NetworkACL networkACLMock; + + @Spy @InjectMocks ApiResponseHelper apiResponseHelper = new ApiResponseHelper(); @@ -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 { @@ -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 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 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 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 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 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 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 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()); + } + } diff --git a/ui/src/components/view/InfoCard.vue b/ui/src/components/view/InfoCard.vue index 24b7cce0feb2..ed1a8016a6b2 100644 --- a/ui/src/components/view/InfoCard.vue +++ b/ui/src/components/view/InfoCard.vue @@ -489,7 +489,8 @@ - {{ resource.vpcname || resource.vpcid }} + {{ resource.vpcname || resource.vpcid }} + {{ resource.vpcname || resource.vpcid }} diff --git a/ui/src/components/view/ListView.vue b/ui/src/components/view/ListView.vue index 2f94d6436a40..2b354dfdbd1d 100644 --- a/ui/src/components/view/ListView.vue +++ b/ui/src/components/view/ListView.vue @@ -270,7 +270,7 @@ {{ text }}