From 30a8a56934f06cbc0b3bae61a1efb2fc8793e9d3 Mon Sep 17 00:00:00 2001 From: Anurag Mittal <1321012+anurag4DSB@users.noreply.github.com> Date: Fri, 26 Jun 2026 10:57:16 +0200 Subject: [PATCH 1/2] OSIS-156: log empty bucket list at DEBUG, classify on AmazonS3Exception 404 --- .../service/impl/ScalityOsisServiceImpl.java | 14 ++- .../impl/ScalityOsisServiceMiscTests.java | 111 +++++++++++++++++- 2 files changed, 122 insertions(+), 3 deletions(-) diff --git a/osis-core/src/main/java/com/scality/osis/service/impl/ScalityOsisServiceImpl.java b/osis-core/src/main/java/com/scality/osis/service/impl/ScalityOsisServiceImpl.java index c17bf70..2d87dae 100644 --- a/osis-core/src/main/java/com/scality/osis/service/impl/ScalityOsisServiceImpl.java +++ b/osis-core/src/main/java/com/scality/osis/service/impl/ScalityOsisServiceImpl.java @@ -1216,7 +1216,19 @@ public PageOfOsisBucketMeta getBucketList(String tenantId, long offset, long lim } } - logger.error("GetBucketList error. Returning empty list. Error details: ", e); + if (e instanceof AmazonS3Exception + && ((AmazonS3Exception) e).getStatusCode() == HttpStatus.NOT_FOUND.value()) { + // A tenant with no buckets is an expected, recoverable condition: the platform + // answers with a 404 and OSIS returns an empty list. Log it at DEBUG with a + // concise message and no stack trace so the normal "no buckets yet" case does + // not read as a failure. + logger.debug("Get Bucket List returned no buckets; returning empty list: {}", e.getMessage()); + } else { + // A genuine fault (Vault, S3, network, or auth) stays visible at ERROR with its + // stack trace, exactly as before. Only the expected empty-list case above is + // quieted; the contract still requires returning an empty page here. + logger.error("GetBucketList error. Returning empty list. Error details: ", e); + } // For errors, GetBucketList should return empty PageOfOsisBucketMeta PageInfo pageInfo = new PageInfo(limit, offset); diff --git a/osis-core/src/test/java/com/scality/osis/service/impl/ScalityOsisServiceMiscTests.java b/osis-core/src/test/java/com/scality/osis/service/impl/ScalityOsisServiceMiscTests.java index 30e5d00..e3a64ed 100644 --- a/osis-core/src/test/java/com/scality/osis/service/impl/ScalityOsisServiceMiscTests.java +++ b/osis-core/src/test/java/com/scality/osis/service/impl/ScalityOsisServiceMiscTests.java @@ -1,14 +1,18 @@ package com.scality.osis.service.impl; +import ch.qos.logback.classic.Level; +import ch.qos.logback.classic.Logger; +import ch.qos.logback.classic.spi.ILoggingEvent; +import ch.qos.logback.core.read.ListAppender; import com.amazonaws.Response; import com.amazonaws.services.identitymanagement.model.*; +import com.amazonaws.services.s3.model.AmazonS3Exception; import com.amazonaws.services.s3.model.Bucket; import com.amazonaws.services.s3.model.Owner; import com.amazonaws.services.securitytoken.model.AssumeRoleRequest; import com.amazonaws.services.securitytoken.model.Credentials; import com.scality.osis.model.*; import com.scality.osis.model.exception.NotImplementedException; -import com.scality.osis.s3.impl.S3ServiceException; import com.scality.osis.utapi.impl.UtapiServiceException; import com.scality.osis.utapiclient.dto.ListMetricsRequestDTO; import com.scality.osis.utapiclient.dto.MetricsData; @@ -19,6 +23,7 @@ import org.junit.jupiter.api.Test; import org.mockito.invocation.InvocationOnMock; import org.mockito.stubbing.Answer; +import org.slf4j.LoggerFactory; import org.springframework.http.HttpStatus; import java.util.ArrayList; @@ -164,8 +169,10 @@ void testGetBucketListErr() { final long limit = 1000L; when(s3ClientMock.listBuckets()) .thenAnswer((Answer>) invocation -> { - throw new S3ServiceException(HttpStatus.BAD_REQUEST, + final AmazonS3Exception ex = new AmazonS3Exception( "Requested offset is outside the total available items"); + ex.setStatusCode(HttpStatus.BAD_REQUEST.value()); + throw ex; }); final PageOfOsisBucketMeta response = scalityOsisServiceUnderTest.getBucketList(SAMPLE_TENANT_ID, offset, limit); @@ -177,6 +184,106 @@ void testGetBucketListErr() { assertEquals(0L, response.getItems().size()); } + /** + * A tenant that legitimately has no buckets is an expected, recoverable condition. The + * empty-list contract must hold and the recovery must not be logged as an error or + * warning, nor dump a stack trace, so operators do not chase a non-failure. + */ + @Test + void testGetBucketListEmptyLogsAtDebugWithoutErrorOrTrace() { + // Setup: tenant with no buckets surfaces as a 404 from the storage platform + final long offset = 0L; + final long limit = 1000L; + when(s3ClientMock.listBuckets()) + .thenAnswer((Answer>) invocation -> { + final AmazonS3Exception ex = new AmazonS3Exception("The specified bucket does not exist"); + ex.setStatusCode(HttpStatus.NOT_FOUND.value()); + throw ex; + }); + + final Logger serviceLogger = (Logger) LoggerFactory.getLogger(ScalityOsisServiceImpl.class); + final Level originalLevel = serviceLogger.getLevel(); + // Capture DEBUG so we can assert the expected case is logged there, not at WARN/ERROR. + serviceLogger.setLevel(Level.DEBUG); + final ListAppender appender = new ListAppender<>(); + appender.start(); + serviceLogger.addAppender(appender); + + try { + // Run: empty-list contract must be preserved + final PageOfOsisBucketMeta response = + scalityOsisServiceUnderTest.getBucketList(SAMPLE_TENANT_ID, offset, limit); + + assertEquals(0L, response.getPageInfo().getTotal()); + assertEquals(offset, response.getPageInfo().getOffset()); + assertEquals(limit, response.getPageInfo().getLimit()); + assertEquals(0L, response.getItems().size()); + + // The expected "no buckets" case must never be logged at ERROR or WARN, and must + // not carry a stack trace. + assertTrue(appender.list.stream().noneMatch(e -> e.getLevel() == Level.ERROR), + "the expected empty-bucket case must not be logged at ERROR"); + assertTrue(appender.list.stream().noneMatch(e -> e.getLevel() == Level.WARN), + "the expected empty-bucket case must not be logged at WARN"); + assertTrue(appender.list.stream().allMatch(e -> e.getThrowableProxy() == null), + "the expected empty-bucket case must not dump a stack trace"); + + // The recovery line is emitted, at DEBUG, with the concise message. + assertTrue(appender.list.stream() + .anyMatch(e -> e.getLevel() == Level.DEBUG + && e.getFormattedMessage().contains("returning empty list")), + "the empty-bucket recovery should be logged once at DEBUG"); + } finally { + serviceLogger.detachAppender(appender); + serviceLogger.setLevel(originalLevel); + } + } + + /** + * Only the expected empty-bucket 404 is quieted to DEBUG. A genuine fault (here a 500 from + * the storage platform) stays visible at ERROR with its stack trace, exactly as before, while + * the empty-page contract is still honored. + */ + @Test + void testGetBucketListGenuineFaultLogsAtError() { + // Setup: a non-404 fault from the storage platform (genuine failure, not "no buckets") + final long offset = 0L; + final long limit = 1000L; + when(s3ClientMock.listBuckets()) + .thenAnswer((Answer>) invocation -> { + final AmazonS3Exception ex = new AmazonS3Exception("storage platform unavailable"); + ex.setStatusCode(HttpStatus.INTERNAL_SERVER_ERROR.value()); + throw ex; + }); + + final Logger serviceLogger = (Logger) LoggerFactory.getLogger(ScalityOsisServiceImpl.class); + final Level originalLevel = serviceLogger.getLevel(); + serviceLogger.setLevel(Level.DEBUG); + final ListAppender appender = new ListAppender<>(); + appender.start(); + serviceLogger.addAppender(appender); + + try { + // Contract: still an empty page for the caller + final PageOfOsisBucketMeta response = + scalityOsisServiceUnderTest.getBucketList(SAMPLE_TENANT_ID, offset, limit); + assertEquals(0L, response.getItems().size()); + + // A genuine (non-404) fault must surface at ERROR with its stack trace, not be hidden at DEBUG. + assertTrue(appender.list.stream() + .anyMatch(e -> e.getLevel() == Level.ERROR + && e.getThrowableProxy() != null), + "a genuine bucket-list fault must be logged at ERROR with a stack trace"); + assertTrue(appender.list.stream() + .noneMatch(e -> e.getLevel() == Level.DEBUG + && e.getFormattedMessage().contains("no buckets")), + "a genuine fault must not be misreported as the expected no-buckets case"); + } finally { + serviceLogger.detachAppender(appender); + serviceLogger.setLevel(originalLevel); + } + } + // test to check if getUsage API throws an error not implemented // this will be removed as a part of S3C-8266 From 9f62d5fcdfbc2bfc12a065101dac71b93f0dfdc5 Mon Sep 17 00:00:00 2001 From: Anurag Mittal <1321012+anurag4DSB@users.noreply.github.com> Date: Mon, 29 Jun 2026 12:39:28 +0200 Subject: [PATCH 2/2] OSIS-156: restore wildcard imports and s3Exception name to satisfy PMD after rebase A rebase onto main re-expanded the wildcard imports (29 -> 34, over PMD's ExcessiveImports limit of 30) and renamed s3Exception back to ex (ShortVariable), silently reverting the PMD fix that had already passed CI. Restore both. --- .../impl/ScalityOsisServiceMiscTests.java | 31 ++++++++----------- 1 file changed, 13 insertions(+), 18 deletions(-) diff --git a/osis-core/src/test/java/com/scality/osis/service/impl/ScalityOsisServiceMiscTests.java b/osis-core/src/test/java/com/scality/osis/service/impl/ScalityOsisServiceMiscTests.java index e3a64ed..f51c071 100644 --- a/osis-core/src/test/java/com/scality/osis/service/impl/ScalityOsisServiceMiscTests.java +++ b/osis-core/src/test/java/com/scality/osis/service/impl/ScalityOsisServiceMiscTests.java @@ -1,21 +1,16 @@ package com.scality.osis.service.impl; -import ch.qos.logback.classic.Level; -import ch.qos.logback.classic.Logger; +import ch.qos.logback.classic.*; import ch.qos.logback.classic.spi.ILoggingEvent; import ch.qos.logback.core.read.ListAppender; import com.amazonaws.Response; import com.amazonaws.services.identitymanagement.model.*; -import com.amazonaws.services.s3.model.AmazonS3Exception; -import com.amazonaws.services.s3.model.Bucket; -import com.amazonaws.services.s3.model.Owner; -import com.amazonaws.services.securitytoken.model.AssumeRoleRequest; -import com.amazonaws.services.securitytoken.model.Credentials; +import com.amazonaws.services.s3.model.*; +import com.amazonaws.services.securitytoken.model.*; import com.scality.osis.model.*; import com.scality.osis.model.exception.NotImplementedException; import com.scality.osis.utapi.impl.UtapiServiceException; -import com.scality.osis.utapiclient.dto.ListMetricsRequestDTO; -import com.scality.osis.utapiclient.dto.MetricsData; +import com.scality.osis.utapiclient.dto.*; import com.scality.osis.vaultadmin.impl.VaultServiceException; import com.scality.vaultclient.dto.*; @@ -169,10 +164,10 @@ void testGetBucketListErr() { final long limit = 1000L; when(s3ClientMock.listBuckets()) .thenAnswer((Answer>) invocation -> { - final AmazonS3Exception ex = new AmazonS3Exception( + final AmazonS3Exception s3Exception = new AmazonS3Exception( "Requested offset is outside the total available items"); - ex.setStatusCode(HttpStatus.BAD_REQUEST.value()); - throw ex; + s3Exception.setStatusCode(HttpStatus.BAD_REQUEST.value()); + throw s3Exception; }); final PageOfOsisBucketMeta response = scalityOsisServiceUnderTest.getBucketList(SAMPLE_TENANT_ID, offset, limit); @@ -196,9 +191,9 @@ void testGetBucketListEmptyLogsAtDebugWithoutErrorOrTrace() { final long limit = 1000L; when(s3ClientMock.listBuckets()) .thenAnswer((Answer>) invocation -> { - final AmazonS3Exception ex = new AmazonS3Exception("The specified bucket does not exist"); - ex.setStatusCode(HttpStatus.NOT_FOUND.value()); - throw ex; + final AmazonS3Exception s3Exception = new AmazonS3Exception("The specified bucket does not exist"); + s3Exception.setStatusCode(HttpStatus.NOT_FOUND.value()); + throw s3Exception; }); final Logger serviceLogger = (Logger) LoggerFactory.getLogger(ScalityOsisServiceImpl.class); @@ -251,9 +246,9 @@ void testGetBucketListGenuineFaultLogsAtError() { final long limit = 1000L; when(s3ClientMock.listBuckets()) .thenAnswer((Answer>) invocation -> { - final AmazonS3Exception ex = new AmazonS3Exception("storage platform unavailable"); - ex.setStatusCode(HttpStatus.INTERNAL_SERVER_ERROR.value()); - throw ex; + final AmazonS3Exception s3Exception = new AmazonS3Exception("storage platform unavailable"); + s3Exception.setStatusCode(HttpStatus.INTERNAL_SERVER_ERROR.value()); + throw s3Exception; }); final Logger serviceLogger = (Logger) LoggerFactory.getLogger(ScalityOsisServiceImpl.class);