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 0475600..b9971c2 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 @@ -84,6 +84,20 @@ public ScalityOsisServiceImpl(ScalityAppEnv appEnv, VaultAdmin vaultAdmin, this.scalityOsisCapsManager = scalityOsisCapsManager; } + /** + * Builds the exception to surface for a failed create/update operation. A genuine Vault server + * fault (a {@link VaultServiceException} with a 5xx status) is returned unchanged so the error + * boundary logs it once at ERROR with the stack trace. Every other failure is treated as a + * client-side rejection and mapped to {@code 400 BAD_REQUEST}, keeping the original as the cause. + */ + private VaultServiceException toResponseException(Exception e) { + if (e instanceof VaultServiceException + && ((VaultServiceException) e).getStatus().is5xxServerError()) { + return (VaultServiceException) e; + } + return new VaultServiceException(HttpStatus.BAD_REQUEST, e.getMessage(), e); + } + /** * Create a tenant in the platform * @@ -111,9 +125,9 @@ public OsisTenant createTenant(OsisTenant osisTenant) { return resOsisTenant; } catch (VaultServiceException e) { - // Create Tenant supports only 400:BAD_REQUEST error. Logged once at the error - // boundary; rethrow typed so the status is preserved. - throw new VaultServiceException(HttpStatus.BAD_REQUEST, e.getMessage(), e); + // A client-side Vault rejection maps to 400; a genuine Vault server fault (5xx) is + // preserved so the boundary logs it once at ERROR with the trace. + throw toResponseException(e); } } @@ -252,8 +266,9 @@ public OsisUser createUser(OsisUser osisUser) { return resOsisUser; }); } catch (Exception e) { - // Create User supports only 400:BAD_REQUEST error. Logged once at the boundary. - throw new VaultServiceException(HttpStatus.BAD_REQUEST, e.getMessage(), e); + // A client-side Vault rejection maps to 400; a genuine Vault server fault (5xx) is + // preserved so the boundary logs it once at ERROR with the trace. + throw toResponseException(e); } } @@ -396,8 +411,9 @@ public OsisS3Credential createS3Credential(String tenantId, String userId) { return credential; }); } catch (Exception e) { - // Create S3 Credential supports only 400:BAD_REQUEST error. Logged once at the boundary. - throw new VaultServiceException(HttpStatus.BAD_REQUEST, e.getMessage(), e); + // A client-side Vault rejection maps to 400; a genuine Vault server fault (5xx) is + // preserved so the boundary logs it once at ERROR with the trace. + throw toResponseException(e); } } @@ -551,8 +567,12 @@ public OsisTenant updateTenant(String tenantId, OsisTenant osisTenant) { return resOsisTenant; } catch (VaultServiceException e) { - // Update Tenant supports only 400:BAD_REQUEST error. Logged once at the boundary. - // Keep the original Vault failure as the cause so the trace survives to the log. + // A genuine Vault server fault (5xx) is preserved so the boundary logs it once at ERROR + // with the trace. A client-side rejection maps to 400 (errorCode retained for the + // response contract), keeping the original Vault failure as the cause. + if (e.getStatus().is5xxServerError()) { + throw e; + } throw new VaultServiceException(HttpStatus.BAD_REQUEST, e.getErrorCode(), e.getReason(), e); } } @@ -945,8 +965,9 @@ public OsisS3Credential updateCredentialStatus(String tenantId, String userId, S return newOsisS3Credential; }); } catch (Exception e) { - // Logged once at the boundary. - throw new VaultServiceException(HttpStatus.BAD_REQUEST, e.getMessage(), e); + // A client-side Vault rejection maps to 400; a genuine Vault server fault (5xx) is + // preserved so the boundary logs it once at ERROR with the trace. + throw toResponseException(e); } } @@ -1038,8 +1059,9 @@ public OsisUser updateUser(String tenantId, String userId, OsisUser osisUser) { return osisUser; }); } catch (Exception e) { - // Logged once at the boundary. - throw new VaultServiceException(HttpStatus.BAD_REQUEST, e.getMessage(), e); + // A client-side Vault rejection maps to 400; a genuine Vault server fault (5xx) is + // preserved so the boundary logs it once at ERROR with the trace. + throw toResponseException(e); } } diff --git a/osis-core/src/test/java/com/scality/osis/service/impl/ScalityOsisServiceTenantTests.java b/osis-core/src/test/java/com/scality/osis/service/impl/ScalityOsisServiceTenantTests.java index b13dc5f..90f378c 100644 --- a/osis-core/src/test/java/com/scality/osis/service/impl/ScalityOsisServiceTenantTests.java +++ b/osis-core/src/test/java/com/scality/osis/service/impl/ScalityOsisServiceTenantTests.java @@ -73,6 +73,22 @@ void testCreateTenant400() { } + @Test + void testCreateTenant500PreservesServerFault() { + + when(vaultAdminMock.createAccount(any(CreateAccountRequestDTO.class))) + .thenAnswer((Answer) invocation -> { + throw new VaultServiceException(HttpStatus.INTERNAL_SERVER_ERROR, "Vault unavailable"); + }); + + final VaultServiceException exception = assertThrows(VaultServiceException.class, () -> { + scalityOsisServiceUnderTest.createTenant(createSampleOsisTenantObj()); + }); + + assertEquals(500, exception.getStatus().value(), + "a genuine Vault server fault must not be downgraded to 400"); + } + @Test void testListTenants() { // Setup @@ -319,6 +335,22 @@ void testUpdateTenantRequestParametersConsistencyForName() { assertEquals("E_BAD_REQUEST", exception.getErrorCode()); } + @Test + void testUpdateTenant500PreservesServerFault() { + // A matching request passes the name/ID consistency check and reaches the Vault call. + when(vaultAdminMock.updateAccountAttributes(any(UpdateAccountAttributesRequestDTO.class))) + .thenAnswer((Answer) invocation -> { + throw new VaultServiceException(HttpStatus.INTERNAL_SERVER_ERROR, "Vault unavailable"); + }); + + final VaultServiceException exception = assertThrows(VaultServiceException.class, () -> { + scalityOsisServiceUnderTest.updateTenant(SAMPLE_ID, createSampleOsisTenantObj()); + }); + + assertEquals(500, exception.getStatus().value(), + "a genuine Vault server fault must not be downgraded to 400"); + } + @Test void testGetTenant() { // Setup