server: report a domain's bucket, object storage and backup limits correctly

server/src/main/java/com/cloud/api/ApiDBUtils.java

@@ -986,8 +986,8 @@ public static long findCorrectResourceLimitForDomain(Long limit, ResourceType re
         if (domainId == Domain.ROOT_DOMAIN) {
             return Resource.RESOURCE_UNLIMITED;
         }
-        //--If limit doesn't have a value then fetch default limit from the configs
-        return (limit == null) ? s_resourceLimitMgr.findDefaultResourceLimitForDomain(resourceType) : limit;
+        //--If limit doesn't have a value then fetch the limit that is enforced: a parent domain's limit or the default from the configs
+        return (limit == null) ? findCorrectResourceLimitForDomain(resourceType, domainId) : limit;
     }
 
     public static long getResourceCount(ResourceType type, long accountId) {

server/src/main/java/com/cloud/api/query/dao/DomainJoinDaoImpl.java

@@ -217,24 +217,24 @@ public void setResourceLimits(DomainJoinVO domain, boolean fullView, ResourceLim
 
         //get resource limits for backups
         long backupLimit = ApiDBUtils.findCorrectResourceLimitForDomain(domain.getBackupLimit(), ResourceType.backup, domain.getId());
-        String backupLimitDisplay = (fullView || snapshotLimit == -1) ? Resource.UNLIMITED : String.valueOf(backupLimit);
+        String backupLimitDisplay = (fullView || backupLimit == -1) ? Resource.UNLIMITED : String.valueOf(backupLimit);
         long backupTotal = (domain.getBackupTotal() == null) ? 0 : domain.getBackupTotal();
-        String backupAvail = (fullView || snapshotLimit == -1) ? Resource.UNLIMITED : String.valueOf(backupLimit - backupTotal);
+        String backupAvail = (fullView || backupLimit == -1) ? Resource.UNLIMITED : String.valueOf(backupLimit - backupTotal);
         response.setBackupLimit(backupLimitDisplay);
         response.setBackupTotal(backupTotal);
         response.setBackupAvailable(backupAvail);
 
         //get resource limits for backup storage space and convert it from Bytes to GiB
         long backupStorageLimit = ApiDBUtils.findCorrectResourceLimitForDomain(domain.getBackupStorageLimit(), ResourceType.backup_storage, domain.getId());
-        String backupStorageLimitDisplay = (fullView || backupLimit == -1) ? Resource.UNLIMITED : String.valueOf(backupStorageLimit / ResourceType.bytesToGiB);
+        String backupStorageLimitDisplay = (fullView || backupStorageLimit == -1) ? Resource.UNLIMITED : String.valueOf(backupStorageLimit / ResourceType.bytesToGiB);
         long backupStorageTotal = (domain.getBackupStorageTotal() == null) ? 0 : (domain.getBackupStorageTotal() / ResourceType.bytesToGiB);
         String backupStorageAvail = (fullView || backupStorageLimit == -1) ? Resource.UNLIMITED : String.valueOf((backupStorageLimit / ResourceType.bytesToGiB) - backupStorageTotal);
         response.setBackupStorageLimit(backupStorageLimitDisplay);
         response.setBackupStorageTotal(backupStorageTotal);
         response.setBackupStorageAvailable(backupStorageAvail);
 
         //get resource limits for buckets
-        long bucketLimit = ApiDBUtils.findCorrectResourceLimit(domain.getBucketLimit(), domain.getId(), ResourceType.bucket);
+        long bucketLimit = ApiDBUtils.findCorrectResourceLimitForDomain(domain.getBucketLimit(), ResourceType.bucket, domain.getId());
         String bucketLimitDisplay = (fullView || bucketLimit == -1) ? Resource.UNLIMITED : String.valueOf(bucketLimit);
         long bucketTotal = (domain.getBucketTotal() == null) ? 0 : domain.getBucketTotal();
         String bucketAvail = (fullView || bucketLimit == -1) ? Resource.UNLIMITED : String.valueOf(bucketLimit - bucketTotal);
@@ -243,7 +243,7 @@ public void setResourceLimits(DomainJoinVO domain, boolean fullView, ResourceLim
         response.setBucketAvailable(bucketAvail);
 
         //get resource limits for object storage space and convert it from Bytes to GiB
-        long objectStorageLimit = ApiDBUtils.findCorrectResourceLimit(domain.getObjectStorageLimit(), domain.getId(), ResourceType.object_storage);
+        long objectStorageLimit = ApiDBUtils.findCorrectResourceLimitForDomain(domain.getObjectStorageLimit(), ResourceType.object_storage, domain.getId());
         String objectStorageLimitDisplay = (fullView || objectStorageLimit == -1) ? Resource.UNLIMITED : String.valueOf(objectStorageLimit / ResourceType.bytesToGiB);
         long objectStorageTotal = (domain.getObjectStorageTotal() == null) ? 0 : (domain.getObjectStorageTotal() / ResourceType.bytesToGiB);
         String objectStorageAvail = (fullView || objectStorageLimit == -1) ? Resource.UNLIMITED : String.valueOf((objectStorageLimit / ResourceType.bytesToGiB) - objectStorageTotal);

server/src/test/java/com/cloud/api/ApiDBUtilsTest.java

@@ -0,0 +1,84 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package com.cloud.api;
+
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.junit.MockitoJUnitRunner;
+
+import com.cloud.configuration.Resource;
+import com.cloud.configuration.Resource.ResourceType;
+import com.cloud.domain.Domain;
+import com.cloud.domain.DomainVO;
+import com.cloud.domain.dao.DomainDao;
+import com.cloud.user.ResourceLimitService;
+
+@RunWith(MockitoJUnitRunner.Silent.class)
+public class ApiDBUtilsTest {
+
+    private static final long DOMAIN_ID = 2L;
+
+    private DomainDao originalDomainDao;
+    private ResourceLimitService originalResourceLimitMgr;
+    private DomainVO domain;
+    private ResourceLimitService resourceLimitMgr;
+
+    @Before
+    public void setUp() {
+        originalDomainDao = ApiDBUtils.s_domainDao;
+        originalResourceLimitMgr = ApiDBUtils.s_resourceLimitMgr;
+
+        domain = mock(DomainVO.class);
+        DomainDao domainDao = mock(DomainDao.class);
+        when(domainDao.findById(DOMAIN_ID)).thenReturn(domain);
+        resourceLimitMgr = mock(ResourceLimitService.class);
+        ApiDBUtils.s_domainDao = domainDao;
+        ApiDBUtils.s_resourceLimitMgr = resourceLimitMgr;
+    }
+
+    @After
+    public void tearDown() {
+        ApiDBUtils.s_domainDao = originalDomainDao;
+        ApiDBUtils.s_resourceLimitMgr = originalResourceLimitMgr;
+    }
+
+    @Test
+    public void testDomainWithoutLimitGetsTheEnforcedLimit() {
+        // max.domain.buckets is enforced when the domain has no bucket limit of its own
+        when(resourceLimitMgr.findCorrectResourceLimitForDomain(domain, ResourceType.bucket, null)).thenReturn(20L);
+        when(resourceLimitMgr.findDefaultResourceLimitForDomain(ResourceType.bucket)).thenReturn((long) Resource.RESOURCE_UNLIMITED);
+
+        Assert.assertEquals(20L, ApiDBUtils.findCorrectResourceLimitForDomain(null, ResourceType.bucket, DOMAIN_ID));
+    }
+
+    @Test
+    public void testDomainLimitIsUsedWhenSet() {
+        Assert.assertEquals(7L, ApiDBUtils.findCorrectResourceLimitForDomain(7L, ResourceType.bucket, DOMAIN_ID));
+    }
+
+    @Test
+    public void testRootDomainIsUnlimited() {
+        Assert.assertEquals(Resource.RESOURCE_UNLIMITED,
+                ApiDBUtils.findCorrectResourceLimitForDomain(null, ResourceType.bucket, Domain.ROOT_DOMAIN));
+    }
+}

server/src/test/java/com/cloud/api/query/dao/DomainJoinDaoImplTest.java

@@ -0,0 +1,114 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package com.cloud.api.query.dao;
+
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyBoolean;
+import static org.mockito.ArgumentMatchers.anyLong;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.mockStatic;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import org.apache.cloudstack.api.response.ResourceLimitAndCountResponse;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.MockedStatic;
+import org.mockito.Mockito;
+import org.mockito.junit.MockitoJUnitRunner;
+
+import com.cloud.api.ApiDBUtils;
+import com.cloud.api.query.vo.DomainJoinVO;
+import com.cloud.configuration.Resource;
+import com.cloud.configuration.Resource.ResourceType;
+
+@RunWith(MockitoJUnitRunner.Silent.class)
+public class DomainJoinDaoImplTest {
+
+    private static final long DOMAIN_ID = 2L;
+
+    private DomainJoinDaoImpl domainJoinDao;
+    private DomainJoinVO domain;
+    private ResourceLimitAndCountResponse response;
+    private MockedStatic<ApiDBUtils> apiDBUtils;
+
+    @Before
+    public void setUp() {
+        // The constructor builds search builders against the database, which setResourceLimits does not need.
+        domainJoinDao = mock(DomainJoinDaoImpl.class, Mockito.CALLS_REAL_METHODS);
+        domain = mock(DomainJoinVO.class);
+        response = mock(ResourceLimitAndCountResponse.class);
+        when(domain.getId()).thenReturn(DOMAIN_ID);
+
+        apiDBUtils = mockStatic(ApiDBUtils.class);
+        // A domain lookup returns the domain's own limit, or unlimited when none is set.
+        apiDBUtils.when(() -> ApiDBUtils.findCorrectResourceLimitForDomain(any(), any(ResourceType.class), anyLong()))
+                .thenAnswer(invocation -> limitOrUnlimited(invocation.getArgument(0)));
+        apiDBUtils.when(() -> ApiDBUtils.findCorrectResourceLimitForDomain(any(), anyBoolean(), any(ResourceType.class), anyLong()))
+                .thenAnswer(invocation -> limitOrUnlimited(invocation.getArgument(0)));
+        // The account lookup resolves its id against the account table. Given a domain id it finds the root
+        // admin or no account at all, and in both cases answers unlimited.
+        apiDBUtils.when(() -> ApiDBUtils.findCorrectResourceLimit(any(), anyLong(), any(ResourceType.class)))
+                .thenReturn((long) Resource.RESOURCE_UNLIMITED);
+    }
+
+    @After
+    public void tearDown() {
+        apiDBUtils.close();
+    }
+
+    private static long limitOrUnlimited(Long limit) {
+        return limit == null ? Resource.RESOURCE_UNLIMITED : limit;
+    }
+
+    @Test
+    public void testBucketAndObjectStorageLimitsComeFromTheDomain() {
+        when(domain.getBucketLimit()).thenReturn(147L);
+        when(domain.getObjectStorageLimit()).thenReturn(157L * ResourceType.bytesToGiB);
+
+        domainJoinDao.setResourceLimits(domain, false, response);
+
+        verify(response).setBucketLimit("147");
+        verify(response).setBucketAvailable("147");
+        verify(response).setObjectStorageLimit("157");
+        verify(response).setObjectStorageAvailable("157");
+    }
+
+    @Test
+    public void testBackupLimitIsNotHiddenByAnUnlimitedSnapshotLimit() {
+        when(domain.getSnapshotLimit()).thenReturn((long) Resource.RESOURCE_UNLIMITED);
+        when(domain.getBackupLimit()).thenReturn(127L);
+
+        domainJoinDao.setResourceLimits(domain, false, response);
+
+        verify(response).setBackupLimit("127");
+        verify(response).setBackupAvailable("127");
+    }
+
+    @Test
+    public void testBackupStorageLimitIsNotHiddenByAnUnlimitedBackupLimit() {
+        when(domain.getBackupLimit()).thenReturn((long) Resource.RESOURCE_UNLIMITED);
+        when(domain.getBackupStorageLimit()).thenReturn(137L * ResourceType.bytesToGiB);
+
+        domainJoinDao.setResourceLimits(domain, false, response);
+
+        verify(response).setBackupStorageLimit("137");
+        verify(response).setBackupStorageAvailable("137");
+    }
+}