From 30363e22500cc85de6def3a52cbe3b3965e3ed71 Mon Sep 17 00:00:00 2001 From: IvanBorislavovDimitrov Date: Fri, 24 Apr 2026 17:15:11 +0300 Subject: [PATCH 1/4] Migrate from Jclouds to AWS SDK LMCROSSITXSADEPLOY-3315 --- .../multiapps/controller/api/v1/FilesApi.java | 37 +- multiapps-controller-persistence/pom.xml | 8 +- .../src/main/java/module-info.java | 11 +- .../controller/persistence/Messages.java | 4 + .../services/AwsS3ObjectStoreFileStorage.java | 316 +++++++++ .../persistence/services/FileStorage.java | 2 +- .../services/GcpObjectStoreFileStorage.java | 27 +- .../JCloudsObjectStoreFileStorage.java | 1 + .../util/ObjectStoreConstants.java | 12 +- .../AwsS3ObjectStoreFileStorageTest.java | 606 ++++++++++++++++++ .../OperationInErrorStateHandlerTest.java | 8 +- multiapps-controller-web/pom.xml | 8 + .../multiapps/controller/web/Constants.java | 5 +- .../multiapps/controller/web/Messages.java | 15 +- .../web/api/impl/FilesApiServiceImpl.java | 12 +- .../ObjectStoreFileStorageFactoryBean.java | 173 ++--- .../service/ObjectStoreServiceInfo.java | 23 - .../ObjectStoreServiceInfoCreator.java | 68 +- .../upload/AsyncUploadJobOrchestrator.java | 28 +- .../FileUploadResilientOperationExecutor.java | 68 ++ ...ObjectStoreFileStorageFactoryBeanTest.java | 185 ++++-- .../ObjectStoreServiceInfoCreatorTest.java | 44 +- .../AsyncUploadJobOrchestratorTest.java | 6 +- ...eUploadResilientOperationExecutorTest.java | 207 ++++++ pom.xml | 19 +- 25 files changed, 1590 insertions(+), 303 deletions(-) create mode 100644 multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/AwsS3ObjectStoreFileStorage.java create mode 100644 multiapps-controller-persistence/src/test/java/org/cloudfoundry/multiapps/controller/persistence/services/AwsS3ObjectStoreFileStorageTest.java create mode 100644 multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/upload/resilience/FileUploadResilientOperationExecutor.java create mode 100644 multiapps-controller-web/src/test/java/org/cloudfoundry/multiapps/controller/web/upload/FileUploadResilientOperationExecutorTest.java diff --git a/multiapps-controller-api/src/main/java/org/cloudfoundry/multiapps/controller/api/v1/FilesApi.java b/multiapps-controller-api/src/main/java/org/cloudfoundry/multiapps/controller/api/v1/FilesApi.java index beb6dc1a84..d6b813166c 100644 --- a/multiapps-controller-api/src/main/java/org/cloudfoundry/multiapps/controller/api/v1/FilesApi.java +++ b/multiapps-controller-api/src/main/java/org/cloudfoundry/multiapps/controller/api/v1/FilesApi.java @@ -1,9 +1,13 @@ package org.cloudfoundry.multiapps.controller.api.v1; import java.util.List; - +import io.swagger.annotations.Api; +import io.swagger.annotations.ApiOperation; +import io.swagger.annotations.ApiParam; +import io.swagger.annotations.ApiResponse; +import io.swagger.annotations.ApiResponses; +import io.swagger.annotations.Authorization; import jakarta.inject.Inject; - import org.cloudfoundry.multiapps.controller.api.Constants.Endpoints; import org.cloudfoundry.multiapps.controller.api.Constants.PathVariables; import org.cloudfoundry.multiapps.controller.api.Constants.RequestVariables; @@ -23,13 +27,6 @@ import org.springframework.web.bind.annotation.RestController; import org.springframework.web.multipart.MultipartHttpServletRequest; -import io.swagger.annotations.Api; -import io.swagger.annotations.ApiOperation; -import io.swagger.annotations.ApiParam; -import io.swagger.annotations.ApiResponse; -import io.swagger.annotations.ApiResponses; -import io.swagger.annotations.Authorization; - @Api @RestController @RequestMapping(Resources.FILES) @@ -45,8 +42,8 @@ public class FilesApi { }) }, tags = {}) @ApiResponses(value = { @ApiResponse(code = 200, message = "OK", response = FileMetadata.class, responseContainer = "List") }) public ResponseEntity> - getFiles(@ApiParam(value = "GUID of space with mtas") @PathVariable(PathVariables.SPACE_GUID) String spaceGuid, - @ApiParam(value = "Filter mtas by namespace") @RequestParam(name = RequestVariables.NAMESPACE, required = false) String namespace) { + getFiles(@ApiParam(value = "GUID of space with mtas") @PathVariable(PathVariables.SPACE_GUID) String spaceGuid, + @ApiParam(value = "Filter mtas by namespace") @RequestParam(name = RequestVariables.NAMESPACE, required = false) String namespace) { return delegate.getFiles(spaceGuid, namespace); } @@ -57,9 +54,9 @@ public class FilesApi { }) }, tags = {}) @ApiResponses(value = { @ApiResponse(code = 201, message = "Created", response = FileMetadata.class) }) public ResponseEntity - uploadFile(MultipartHttpServletRequest request, - @ApiParam(value = "GUID of space you wish to deploy in") @PathVariable(PathVariables.SPACE_GUID) String spaceGuid, - @ApiParam(value = "file namespace") @RequestParam(name = RequestVariables.NAMESPACE, required = false) String namespace) { + uploadFile(MultipartHttpServletRequest request, + @ApiParam(value = "GUID of space you wish to deploy in") @PathVariable(PathVariables.SPACE_GUID) String spaceGuid, + @ApiParam(value = "file namespace") @RequestParam(name = RequestVariables.NAMESPACE, required = false) String namespace) { return delegate.uploadFile(request, spaceGuid, namespace); } @@ -70,9 +67,9 @@ public class FilesApi { }) }, tags = {}) @ApiResponses(value = { @ApiResponse(code = 202, message = "Accepted") }) public ResponseEntity - startUploadFromUrl(@ApiParam(value = "GUID of space you wish to deploy in") @PathVariable(PathVariables.SPACE_GUID) String spaceGuid, - @ApiParam(value = "file namespace") @RequestParam(name = RequestVariables.NAMESPACE, required = false) String namespace, - @ApiParam(value = "URL reference to a remote file") @RequestBody FileUrl fileUrl) { + startUploadFromUrl(@ApiParam(value = "GUID of space you wish to deploy in") @PathVariable(PathVariables.SPACE_GUID) String spaceGuid, + @ApiParam(value = "file namespace") @RequestParam(name = RequestVariables.NAMESPACE, required = false) String namespace, + @ApiParam(value = "URL reference to a remote file") @RequestBody FileUrl fileUrl) { return delegate.startUploadFromUrl(spaceGuid, namespace, fileUrl); } @@ -84,9 +81,9 @@ public class FilesApi { @ApiResponses(value = { @ApiResponse(code = 200, message = "OK"), @ApiResponse(code = 201, message = "Created", response = AsyncUploadResult.class) }) public ResponseEntity - getUploadFromUrlJob(@ApiParam(value = "GUID of space you wish to deploy in") @PathVariable(PathVariables.SPACE_GUID) String spaceGuid, - @ApiParam(value = "file namespace") @RequestParam(name = RequestVariables.NAMESPACE, required = false) String namespace, - @ApiParam(value = "ID of the upload job") @PathVariable(PathVariables.JOB_ID) String jobId) { + getUploadFromUrlJob(@ApiParam(value = "GUID of space you wish to deploy in") @PathVariable(PathVariables.SPACE_GUID) String spaceGuid, + @ApiParam(value = "file namespace") @RequestParam(name = RequestVariables.NAMESPACE, required = false) String namespace, + @ApiParam(value = "ID of the upload job") @PathVariable(PathVariables.JOB_ID) String jobId) { return delegate.getUploadFromUrlJob(spaceGuid, namespace, jobId); } diff --git a/multiapps-controller-persistence/pom.xml b/multiapps-controller-persistence/pom.xml index 6d2cba2ee6..d19e05f7ab 100644 --- a/multiapps-controller-persistence/pom.xml +++ b/multiapps-controller-persistence/pom.xml @@ -100,8 +100,8 @@ test - org.apache.jclouds.provider - aws-s3 + software.amazon.awssdk + s3 com.azure @@ -111,6 +111,10 @@ com.azure azure-core-http-jdk-httpclient + + software.amazon.awssdk + url-connection-client + org.apache.jclouds jclouds-blobstore diff --git a/multiapps-controller-persistence/src/main/java/module-info.java b/multiapps-controller-persistence/src/main/java/module-info.java index 1eef421ec7..8b7da05942 100644 --- a/multiapps-controller-persistence/src/main/java/module-info.java +++ b/multiapps-controller-persistence/src/main/java/module-info.java @@ -63,7 +63,16 @@ requires spring.beans; requires spring.context; requires spring.core; - + requires software.amazon.awssdk.services.s3; + requires software.amazon.awssdk.core; + requires software.amazon.awssdk.awscore; + requires software.amazon.awssdk.regions; + requires software.amazon.awssdk.utils; + requires software.amazon.awssdk.auth; + requires software.amazon.awssdk.http; + requires software.amazon.awssdk.http.urlconnection; + requires software.amazon.awssdk.retries; + requires software.amazon.awssdk.retries.api; requires static java.compiler; requires static org.immutables.value; } diff --git a/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/Messages.java b/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/Messages.java index df989fd918..762895d73d 100644 --- a/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/Messages.java +++ b/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/Messages.java @@ -48,6 +48,7 @@ public final class Messages { public static final String APPLICATION_SHUTDOWN_WITH_APPLICATION_INSTANCE_ID_ALREADY_EXIST = "Application shutdown application instance ID \"{0}\" already exist"; public static final String SECRET_TOKEN_WITH_ID_NOT_EXIST = "Secret token with ID \"{0}\" does not exist"; public static final String DATABASE_HEALTH_CHECK_FAILED = "Database health check failed"; + public static final String UPLOAD_OF_FILE_WITH_NAMESPACE_FAILED = "Upload of file: \"{0}\" with namespace: \"{1}\" failed"; // ERROR log messages: public static final String UPLOAD_STREAM_FAILED_TO_CLOSE = "Cannot close file upload stream"; @@ -69,6 +70,7 @@ public final class Messages { // DEBUG log messages: public static final String STORED_FILE_0 = "Stored file: \"{0}\""; public static final String STORED_FILE_0_WITH_SIZE_1 = "Stored file \"{0}\" with size {1}"; + public static final String STORED_FILE_0_WITH_SIZE_1_LOG = "Stored file \"{}\" with size {}"; public static final String DELETED_0_FILES_WITH_SPACEIDS_1 = "Deleted {0} files with space ids \"{1}\"."; public static final String DELETED_0_FILES_WITH_SPACE_1_AND_NAMESPACE_2 = "Deleted {0} files with space \"{1}\" and namespace \"{2}\"."; public static final String DELETED_0_FILES_MODIFIED_BEFORE_1 = "Deleted {0} files modified before \"{1}\"."; @@ -79,6 +81,8 @@ public final class Messages { public static final String RETRIEVED_SECRET_TOKEN_WITH_ID_0_FOR_PROCESS_WITH_ID_1 = "Retrieved secret token with id \"{0}\" for process with id \"{1}\""; public static final String DELETED_0_SECRET_TOKENS_FOR_PROCESS_WITH_ID_1 = "Deleted \"{0}\" secret tokens for process with id \"{1}\""; public static final String DELETED_0_SECRET_TOKENS_WITH_EXPIRATION_DATE_1 = "Deleted secret tokens \"{0}\" with an expiration date \"{1}\""; + public static final String FAILED_TO_DELETE_FILE_0_IN_OBJECT_STORE_REASON_1 = "Failed to delete file \"{}\" in object store. Reason: {}"; + public static final String S3_UPLOAD_FAILED_FILE_0_SIZE_1 = "S3 upload failed for file \"{0}\" (size={1}). Root cause chain: {2}"; protected Messages() { } diff --git a/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/AwsS3ObjectStoreFileStorage.java b/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/AwsS3ObjectStoreFileStorage.java new file mode 100644 index 0000000000..9a19456d5a --- /dev/null +++ b/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/AwsS3ObjectStoreFileStorage.java @@ -0,0 +1,316 @@ +package org.cloudfoundry.multiapps.controller.persistence.services; + +import org.cloudfoundry.multiapps.controller.persistence.Messages; +import org.cloudfoundry.multiapps.controller.persistence.model.FileEntry; +import org.cloudfoundry.multiapps.controller.persistence.util.ObjectStoreConstants; +import org.cloudfoundry.multiapps.controller.persistence.util.ObjectStoreFilter; +import org.cloudfoundry.multiapps.controller.persistence.util.ObjectStoreMapper; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.http.MediaType; +import software.amazon.awssdk.auth.credentials.AwsBasicCredentials; +import software.amazon.awssdk.auth.credentials.StaticCredentialsProvider; +import software.amazon.awssdk.core.ResponseInputStream; +import software.amazon.awssdk.core.client.config.ClientOverrideConfiguration; +import software.amazon.awssdk.core.sync.RequestBody; +import software.amazon.awssdk.http.urlconnection.UrlConnectionHttpClient; +import software.amazon.awssdk.retries.StandardRetryStrategy; +import software.amazon.awssdk.retries.api.BackoffStrategy; +import software.amazon.awssdk.services.s3.S3Client; +import software.amazon.awssdk.services.s3.S3ClientBuilder; +import software.amazon.awssdk.services.s3.model.DeleteObjectRequest; +import software.amazon.awssdk.services.s3.model.DeleteObjectsRequest; +import software.amazon.awssdk.services.s3.model.DeleteObjectsResponse; +import software.amazon.awssdk.services.s3.model.GetObjectRequest; +import software.amazon.awssdk.services.s3.model.GetObjectResponse; +import software.amazon.awssdk.services.s3.model.HeadObjectRequest; +import software.amazon.awssdk.services.s3.model.HeadObjectResponse; +import software.amazon.awssdk.services.s3.model.ListObjectsV2Request; +import software.amazon.awssdk.services.s3.model.ListObjectsV2Response; +import software.amazon.awssdk.services.s3.model.NoSuchKeyException; +import software.amazon.awssdk.services.s3.model.ObjectIdentifier; +import software.amazon.awssdk.services.s3.model.PutObjectRequest; +import software.amazon.awssdk.services.s3.model.S3Error; +import software.amazon.awssdk.services.s3.model.S3Object; + +import java.io.BufferedInputStream; +import java.io.InputStream; +import java.net.URI; +import java.text.MessageFormat; +import java.time.LocalDateTime; +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Set; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.function.BiPredicate; +import java.util.function.Consumer; + +public class AwsS3ObjectStoreFileStorage extends ObjectStoreFileStorage { + + private static final Logger LOGGER = LoggerFactory.getLogger(AwsS3ObjectStoreFileStorage.class); + private static final String HTTPS_SCHEME = "https://"; + private static final int DELETE_BATCH_SIZE = 1000; + + private final S3Client s3Client; + private final String bucketName; + + public AwsS3ObjectStoreFileStorage(Map credentials) { + this.bucketName = (String) credentials.get(CredentialKeys.BUCKET); + this.s3Client = createS3Client(credentials); + } + + protected S3Client createS3Client(Map credentials) { + AwsBasicCredentials awsCredentials = AwsBasicCredentials.create((String) credentials.get(CredentialKeys.ACCESS_KEY_ID), + (String) credentials.get(CredentialKeys.SECRET_ACCESS_KEY)); + S3ClientBuilder builder = S3Client.builder() + .credentialsProvider(StaticCredentialsProvider.create(awsCredentials)) + .overrideConfiguration(buildClientOverrideConfig()) + .httpClientBuilder(UrlConnectionHttpClient.builder() + .socketTimeout( + ObjectStoreConstants.AWS_OBJECT_STORE_SOCKET_TIMEOUT_CONFIG_IN_MINUTES) + .connectionTimeout( + ObjectStoreConstants.AWS_OBJECT_STORE_CONNECTION_TIMEOUT_CONFIG_IN_SECONDS)); + builder.endpointOverride(URI.create(HTTPS_SCHEME + credentials.get(CredentialKeys.HOST))); + builder.region(software.amazon.awssdk.regions.Region.of((String) credentials.get(CredentialKeys.REGION))); + return builder.build(); + } + + protected ClientOverrideConfiguration buildClientOverrideConfig() { + StandardRetryStrategy retryStrategy = StandardRetryStrategy.builder() + .maxAttempts(ObjectStoreConstants.OBJECT_STORE_MAX_ATTEMPTS_CONFIG) + .backoffStrategy(BackoffStrategy.exponentialDelayHalfJitter( + ObjectStoreConstants.OBJECT_STORE_INITIAL_RETRY_DELAY_CONFIG_IN_MILLIS, + ObjectStoreConstants.OBJECT_STORE_MAX_RETRY_DELAY_CONFIG_IN_SECONDS)) + .build(); + return ClientOverrideConfiguration.builder() + .retryStrategy(retryStrategy) + .apiCallTimeout(ObjectStoreConstants.AWS_OBJECT_STORE_TOTAL_TIMEOUT_CONFIG_IN_MINUTES) + .apiCallAttemptTimeout(ObjectStoreConstants.OBJECT_STORE_TOTAL_TIMEOUT_CONFIG_IN_MINUTES) + .build(); + } + + @Override + public void addFile(FileEntry fileEntry, InputStream content) throws FileStorageException { + long fileSize = fileEntry.getSize() + .longValue(); + PutObjectRequest request = PutObjectRequest.builder() + .bucket(bucketName) + .key(fileEntry.getId()) + .contentType(MediaType.APPLICATION_OCTET_STREAM_VALUE) + .contentDisposition(fileEntry.getName()) + .metadata(ObjectStoreMapper.createFileEntryMetadata(fileEntry)) + .build(); + try { + InputStream markableContent = content.markSupported() ? content : new BufferedInputStream(content); + s3Client.putObject(request, RequestBody.fromInputStream(markableContent, fileSize)); + LOGGER.debug(Messages.STORED_FILE_0_WITH_SIZE_1_LOG, fileEntry.getId(), fileSize); + } catch (Exception e) { + LOGGER.error(MessageFormat.format(Messages.S3_UPLOAD_FAILED_FILE_0_SIZE_1, fileEntry.getName(), fileSize, e)); + throw new FileStorageException(MessageFormat.format(Messages.UPLOAD_OF_FILE_WITH_NAMESPACE_FAILED, fileEntry.getName(), + fileEntry.getNamespace()), e); + } + } + + @Override + public List getFileEntriesWithoutContent(List fileEntries) { + Set existingKeys = new HashSet<>(listAllObjectKeys()); + return fileEntries.stream() + .filter(fileEntry -> !existingKeys.contains(fileEntry.getId())) + .toList(); + } + + private List listAllObjectKeys() { + List keys = new ArrayList<>(); + forEachPage(page -> page.contents() + .stream() + .map(S3Object::key) + .forEach(keys::add)); + return keys; + } + + private void forEachPage(Consumer pageConsumer) { + ListObjectsV2Request request = ListObjectsV2Request.builder() + .bucket(bucketName) + .build(); + ListObjectsV2Response response; + do { + response = s3Client.listObjectsV2(request); + pageConsumer.accept(response); + request = request.toBuilder() + .continuationToken(response.nextContinuationToken()) + .build(); + } while (Boolean.TRUE.equals(response.isTruncated())); + } + + @Override + protected boolean existsInObjectStore(FileEntry fileEntry) { + try { + s3Client.headObject(HeadObjectRequest.builder() + .bucket(bucketName) + .key(fileEntry.getId()) + .build()); + return true; + } catch (NoSuchKeyException _) { + return false; + } + } + + @Override + public void deleteFile(String id, String space) { + s3Client.deleteObject(DeleteObjectRequest.builder() + .bucket(bucketName) + .key(id) + .build()); + } + + @Override + public void deleteFilesBySpaceIds(List spaceIds) { + deleteByFilterAndCount((key, metadata) -> ObjectStoreFilter.filterBySpaceIds(metadata, spaceIds)); + } + + @Override + public void deleteFilesBySpaceAndNamespace(String space, String namespace) { + deleteByFilterAndCount((key, metadata) -> ObjectStoreFilter.filterBySpaceAndNamespace(metadata, space, namespace)); + } + + @Override + public int deleteFilesModifiedBefore(LocalDateTime modificationTime) { + return deleteByFilterAndCount((key, metadata) -> ObjectStoreFilter.filterByModificationTime(metadata, key, modificationTime)); + } + + private int deleteByFilterAndCount(BiPredicate> predicate) { + List toDelete = new ArrayList<>(); + try (ExecutorService executor = Executors.newVirtualThreadPerTaskExecutor()) { + forEachPage(page -> toDelete.addAll(collectMatchingKeys(page, predicate, executor))); + } + batchDelete(toDelete); + return toDelete.size(); + } + + private List collectMatchingKeys(ListObjectsV2Response page, BiPredicate> predicate, + ExecutorService executor) { + return page.contents() + .stream() + .map(obj -> CompletableFuture.supplyAsync(() -> fetchMetadataAndFilter(obj.key(), predicate), executor)) + .toList() + .stream() + .map(CompletableFuture::join) + .filter(Objects::nonNull) + .toList(); + } + + private String fetchMetadataAndFilter(String key, BiPredicate> predicate) { + try { + HeadObjectResponse response = s3Client.headObject(HeadObjectRequest.builder() + .bucket(bucketName) + .key(key) + .build()); + Map metadata = response.metadata() != null ? response.metadata() : Map.of(); + return predicate.test(key, metadata) ? key : null; + } catch (NoSuchKeyException _) { + return null; + } + } + + private void batchDelete(List keys) { + for (int i = 0; i < keys.size(); i += DELETE_BATCH_SIZE) { + List identifiers = keys.subList(i, Math.min(i + DELETE_BATCH_SIZE, keys.size())) + .stream() + .map(k -> ObjectIdentifier.builder() + .key(k) + .build()) + .toList(); + DeleteObjectsResponse response = s3Client.deleteObjects(DeleteObjectsRequest.builder() + .bucket(bucketName) + .delete(d -> d.objects(identifiers)) + .build()); + for (S3Error error : response.errors()) { + LOGGER.warn(Messages.FAILED_TO_DELETE_FILE_0_IN_OBJECT_STORE_REASON_1, error.key(), error.message()); + } + } + } + + @Override + public T processFileContent(String space, String id, FileContentProcessor fileContentProcessor) throws FileStorageException { + GetObjectRequest request = GetObjectRequest.builder() + .bucket(bucketName) + .key(id) + .build(); + try (ResponseInputStream stream = getObjectStream(request, id, space)) { + return fileContentProcessor.process(stream); + } catch (Exception e) { + throw new FileStorageException(e); + } + } + + @Override + public InputStream openInputStream(String space, String id) throws FileStorageException { + GetObjectRequest request = GetObjectRequest.builder() + .bucket(bucketName) + .key(id) + .build(); + return getObjectStream(request, id, space); + } + + private ResponseInputStream getObjectStream(GetObjectRequest request, String id, + String space) throws FileStorageException { + try { + return s3Client.getObject(request); + } catch (NoSuchKeyException _) { + throw new FileStorageException(MessageFormat.format(Messages.FILE_WITH_ID_AND_SPACE_DOES_NOT_EXIST, id, space)); + } + } + + @Override + public void testConnection() { + s3Client.listObjectsV2(ListObjectsV2Request.builder() + .bucket(bucketName) + .maxKeys(1) + .build()); + } + + @Override + public void deleteFilesByIds(List fileIds) { + if (fileIds.isEmpty()) { + return; + } + batchDelete(fileIds); + } + + @Override + public T processArchiveEntryContent(FileContentToProcess fileContentToProcess, FileContentProcessor fileContentProcessor) + throws FileStorageException { + String id = fileContentToProcess.getGuid(); + String space = fileContentToProcess.getSpaceGuid(); + GetObjectRequest request = GetObjectRequest.builder() + .bucket(bucketName) + .key(id) + .range("bytes=" + fileContentToProcess.getStartOffset() + "-" + + fileContentToProcess.getEndOffset()) + .build(); + try (ResponseInputStream stream = getObjectStream(request, id, space)) { + return fileContentProcessor.process(stream); + } catch (Exception e) { + throw new FileStorageException(e); + } + } + + @Override + public void destroy() { + super.destroy(); + s3Client.close(); + } + + private static final class CredentialKeys { + static final String ACCESS_KEY_ID = "access_key_id"; + static final String SECRET_ACCESS_KEY = "secret_access_key"; + static final String BUCKET = "bucket"; + static final String HOST = "host"; + static final String REGION = "region"; + } +} diff --git a/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/FileStorage.java b/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/FileStorage.java index 0cdca9cf4f..c84abf119f 100644 --- a/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/FileStorage.java +++ b/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/FileStorage.java @@ -10,7 +10,7 @@ public interface FileStorage { void addFile(FileEntry fileEntry, InputStream content) throws FileStorageException; - @Deprecated // This method is not reliable for aws as BlobStore::list might not return a complete list + @Deprecated List getFileEntriesWithoutContent(List fileEntries) throws FileStorageException; List getExistingFileEntries(List fileEntries) throws FileStorageException; diff --git a/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/GcpObjectStoreFileStorage.java b/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/GcpObjectStoreFileStorage.java index 68ded40aab..24982d2a8f 100644 --- a/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/GcpObjectStoreFileStorage.java +++ b/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/GcpObjectStoreFileStorage.java @@ -1,19 +1,5 @@ package org.cloudfoundry.multiapps.controller.persistence.services; -import java.io.ByteArrayInputStream; -import java.io.IOException; -import java.io.InputStream; -import java.nio.channels.Channels; -import java.text.MessageFormat; -import java.time.LocalDateTime; -import java.util.ArrayList; -import java.util.Base64; -import java.util.List; -import java.util.Map; -import java.util.Set; -import java.util.function.Predicate; -import java.util.stream.Collectors; - import com.google.api.gax.retrying.RetrySettings; import com.google.auth.Credentials; import com.google.auth.oauth2.GoogleCredentials; @@ -51,11 +37,9 @@ public class GcpObjectStoreFileStorage implements FileStorage { private final String bucketName; private final Storage storage; - private static final String BASE_64_ENCODED_PRIVATE_KEY_DATA = "base64EncodedPrivateKeyData"; - private static final String BUCKET = "bucket"; public GcpObjectStoreFileStorage(Map credentials) { - this.bucketName = (String) credentials.get(BUCKET); + this.bucketName = (String) credentials.get(CredentialKeys.BUCKET); this.storage = createObjectStoreStorage(credentials); } @@ -77,11 +61,11 @@ protected Storage createObjectStoreStorage(Map credentials) { } private Credentials getGcpCredentialsSupplier(Map credentials) { - if (!credentials.containsKey(BASE_64_ENCODED_PRIVATE_KEY_DATA)) { + if (!credentials.containsKey(CredentialKeys.BASE_64_ENCODED_PRIVATE_KEY_DATA)) { return null; } byte[] decodedKey = Base64.getDecoder() - .decode((String) credentials.get(BASE_64_ENCODED_PRIVATE_KEY_DATA)); + .decode((String) credentials.get(CredentialKeys.BASE_64_ENCODED_PRIVATE_KEY_DATA)); try { return GoogleCredentials.fromStream(new ByteArrayInputStream(decodedKey)); } catch (IOException e) { @@ -280,4 +264,9 @@ private InputStream getBlobPayloadWithOffset(FileEntry fileEntry, long startOffs throw new FileStorageException(e); } } + + private static final class CredentialKeys { + static final String BASE_64_ENCODED_PRIVATE_KEY_DATA = "base64EncodedPrivateKeyData"; + static final String BUCKET = "bucket"; + } } diff --git a/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/JCloudsObjectStoreFileStorage.java b/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/JCloudsObjectStoreFileStorage.java index 2ba0ff70a7..25aa0fb28c 100644 --- a/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/JCloudsObjectStoreFileStorage.java +++ b/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/JCloudsObjectStoreFileStorage.java @@ -65,6 +65,7 @@ public void addFile(FileEntry fileEntry, InputStream content) throws FileStorage } } + // BlobStore::list may not return a complete list, making this method unreliable @Override public List getFileEntriesWithoutContent(List fileEntries) { Set existingFiles = getAllEntries(new ListContainerOptions()).stream() diff --git a/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/util/ObjectStoreConstants.java b/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/util/ObjectStoreConstants.java index 1777eafbb1..2316faf240 100644 --- a/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/util/ObjectStoreConstants.java +++ b/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/util/ObjectStoreConstants.java @@ -7,10 +7,18 @@ public class ObjectStoreConstants { private ObjectStoreConstants() { } + // Shared / default public static final int OBJECT_STORE_MAX_ATTEMPTS_CONFIG = 6; public static final double OBJECT_STORE_RETRY_DELAY_MULTIPLIER_CONFIG = 2.0; - public static final Duration OBJECT_STORE_TOTAL_TIMEOUT_CONFIG_IN_MINUTES = Duration.ofMinutes(10); - public static final Duration OBJECT_STORE_MAX_RETRY_DELAY_CONFIG_IN_SECONDS = Duration.ofSeconds(10); public static final Duration OBJECT_STORE_INITIAL_RETRY_DELAY_CONFIG_IN_MILLIS = Duration.ofMillis(250); + public static final Duration OBJECT_STORE_MAX_RETRY_DELAY_CONFIG_IN_SECONDS = Duration.ofSeconds(10); + public static final Duration OBJECT_STORE_TOTAL_TIMEOUT_CONFIG_IN_MINUTES = Duration.ofMinutes(10); + + // Azure Blob public static final Duration AZURE_OBJECT_STORE_TOTAL_TIMEOUT_CONFIG_IN_MINUTES = Duration.ofMinutes(30); + + // AWS S3 + public static final Duration AWS_OBJECT_STORE_TOTAL_TIMEOUT_CONFIG_IN_MINUTES = Duration.ofMinutes(30); + public static final Duration AWS_OBJECT_STORE_SOCKET_TIMEOUT_CONFIG_IN_MINUTES = Duration.ofMinutes(10); + public static final Duration AWS_OBJECT_STORE_CONNECTION_TIMEOUT_CONFIG_IN_SECONDS = Duration.ofSeconds(10); } diff --git a/multiapps-controller-persistence/src/test/java/org/cloudfoundry/multiapps/controller/persistence/services/AwsS3ObjectStoreFileStorageTest.java b/multiapps-controller-persistence/src/test/java/org/cloudfoundry/multiapps/controller/persistence/services/AwsS3ObjectStoreFileStorageTest.java new file mode 100644 index 0000000000..02bae97ce4 --- /dev/null +++ b/multiapps-controller-persistence/src/test/java/org/cloudfoundry/multiapps/controller/persistence/services/AwsS3ObjectStoreFileStorageTest.java @@ -0,0 +1,606 @@ +package org.cloudfoundry.multiapps.controller.persistence.services; + +import org.cloudfoundry.multiapps.controller.persistence.model.FileEntry; +import org.cloudfoundry.multiapps.controller.persistence.model.ImmutableFileEntry; +import org.cloudfoundry.multiapps.controller.persistence.util.ObjectStoreConstants; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockito.ArgumentCaptor; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; +import software.amazon.awssdk.core.ResponseInputStream; +import software.amazon.awssdk.core.client.config.ClientOverrideConfiguration; +import software.amazon.awssdk.retries.StandardRetryStrategy; +import software.amazon.awssdk.services.s3.S3Client; +import software.amazon.awssdk.services.s3.model.DeleteObjectRequest; +import software.amazon.awssdk.services.s3.model.DeleteObjectsRequest; +import software.amazon.awssdk.services.s3.model.DeleteObjectsResponse; +import software.amazon.awssdk.services.s3.model.GetObjectRequest; +import software.amazon.awssdk.services.s3.model.GetObjectResponse; +import software.amazon.awssdk.services.s3.model.HeadObjectRequest; +import software.amazon.awssdk.services.s3.model.HeadObjectResponse; +import software.amazon.awssdk.services.s3.model.ListObjectsV2Request; +import software.amazon.awssdk.services.s3.model.ListObjectsV2Response; +import software.amazon.awssdk.services.s3.model.NoSuchKeyException; +import software.amazon.awssdk.services.s3.model.PutObjectRequest; +import software.amazon.awssdk.services.s3.model.S3Exception; +import software.amazon.awssdk.services.s3.model.S3Object; + +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.io.InputStream; +import java.math.BigInteger; +import java.time.LocalDateTime; +import java.time.ZoneId; +import java.util.Arrays; +import java.util.List; +import java.util.Map; +import java.util.UUID; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +class AwsS3ObjectStoreFileStorageTest { + + private static final String BUCKET_NAME = "test-bucket"; + + @Mock + private S3Client s3Client; + + @Mock + private FileContentProcessor fileContentProcessor; + + private AwsS3ObjectStoreFileStorage fileStorage; + private final InputStream inputStream = new ByteArrayInputStream(new byte[] {}); + private static final String TEST_SPACE_ID = UUID.randomUUID() + .toString(); + private static final String TEST_SPACE_ID_2 = UUID.randomUUID() + .toString(); + private static final String TEST_ID = UUID.randomUUID() + .toString(); + private static final String TEST_ID_2 = UUID.randomUUID() + .toString(); + private static final String NAMESPACE = "namespace"; + private static final String NAMESPACE_2 = "namespace_2"; + + @BeforeEach + void setUp() throws Exception { + MockitoAnnotations.openMocks(this) + .close(); + + fileStorage = new AwsS3ObjectStoreFileStorage(Map.of("bucket", BUCKET_NAME)) { + + @Override + protected S3Client createS3Client(Map credentials) { + return s3Client; + } + }; + } + + @Test + void testAddFileWithSuccessfulUpload() throws FileStorageException { + FileEntry fileEntry = createFileEntry(TEST_SPACE_ID, TEST_ID); + + fileStorage.addFile(fileEntry, inputStream); + + ArgumentCaptor captor = ArgumentCaptor.forClass(PutObjectRequest.class); + verify(s3Client).putObject(captor.capture(), any(software.amazon.awssdk.core.sync.RequestBody.class)); + PutObjectRequest request = captor.getValue(); + assertEquals(BUCKET_NAME, request.bucket()); + assertEquals(TEST_ID, request.key()); + } + + @Test + void testAddFileWithFailedUpload() { + when(s3Client.putObject(any(PutObjectRequest.class), any(software.amazon.awssdk.core.sync.RequestBody.class))) + .thenThrow(S3Exception.builder() + .message("upload failed") + .build()); + FileEntry fileEntry = createFileEntry(TEST_SPACE_ID, TEST_ID); + + assertThrows(FileStorageException.class, () -> fileStorage.addFile(fileEntry, inputStream)); + } + + @Test + void testGetFileEntriesWithoutContent() { + setupListObjects(TEST_ID_2); + FileEntry fileEntry = createFileEntry(TEST_SPACE_ID, TEST_ID); + + List result = fileStorage.getFileEntriesWithoutContent(List.of(fileEntry)); + + assertEquals(1, result.size()); + assertEquals(TEST_ID, result.getFirst() + .getId()); + } + + @Test + void testGetFileEntriesWithoutContentWithoutMatches() { + setupListObjects(TEST_ID, TEST_ID_2); + FileEntry fileEntry = createFileEntry(TEST_SPACE_ID, TEST_ID); + + List result = fileStorage.getFileEntriesWithoutContent(List.of(fileEntry)); + + assertEquals(0, result.size()); + } + + @Test + void testExistsInObjectStoreWhenFileExists() { + when(s3Client.headObject(any(HeadObjectRequest.class))).thenReturn(HeadObjectResponse.builder() + .build()); + FileEntry fileEntry = createFileEntry(TEST_SPACE_ID, TEST_ID); + + List result = fileStorage.getExistingFileEntries(List.of(fileEntry)); + + assertEquals(1, result.size()); + assertEquals(TEST_ID, result.getFirst() + .getId()); + } + + @Test + void testExistsInObjectStoreWhenFileDoesNotExist() { + when(s3Client.headObject(any(HeadObjectRequest.class))).thenThrow(NoSuchKeyException.builder() + .build()); + FileEntry fileEntry = createFileEntry(TEST_SPACE_ID, TEST_ID); + + List result = fileStorage.getExistingFileEntries(List.of(fileEntry)); + + assertTrue(result.isEmpty()); + } + + @Test + void testDeleteFile() { + fileStorage.deleteFile(TEST_ID, TEST_SPACE_ID); + + ArgumentCaptor captor = ArgumentCaptor.forClass(DeleteObjectRequest.class); + verify(s3Client).deleteObject(captor.capture()); + assertEquals(BUCKET_NAME, captor.getValue() + .bucket()); + assertEquals(TEST_ID, captor.getValue() + .key()); + } + + @Test + void testDeleteFilesBySpaceIdsWithAllMatchingItems() { + setupListObjectsWithKeys(TEST_ID, TEST_ID_2); + setupHeadObjectWithMetadata(TEST_ID, TEST_SPACE_ID, NAMESPACE, LocalDateTime.now()); + setupHeadObjectWithMetadata(TEST_ID_2, TEST_SPACE_ID_2, NAMESPACE_2, LocalDateTime.now()); + + fileStorage.deleteFilesBySpaceIds(List.of(TEST_SPACE_ID, TEST_SPACE_ID_2)); + + verify(s3Client).deleteObjects(any(DeleteObjectsRequest.class)); + } + + @Test + void testDeleteFilesBySpaceIdsWithOneMatchingItem() { + setupListObjectsWithKeys(TEST_ID, TEST_ID_2); + setupHeadObjectWithMetadata(TEST_ID, TEST_SPACE_ID, NAMESPACE, LocalDateTime.now()); + setupHeadObjectWithMetadata(TEST_ID_2, TEST_SPACE_ID_2, NAMESPACE_2, LocalDateTime.now()); + + fileStorage.deleteFilesBySpaceIds(List.of(TEST_SPACE_ID)); + + ArgumentCaptor captor = ArgumentCaptor.forClass(DeleteObjectsRequest.class); + verify(s3Client).deleteObjects(captor.capture()); + assertEquals(1, captor.getValue() + .delete() + .objects() + .size()); + } + + @Test + void testDeleteFilesBySpaceIdsWithoutMatchingItem() { + setupListObjectsWithKeys(TEST_ID); + setupHeadObjectWithMetadata(TEST_ID, TEST_SPACE_ID, NAMESPACE, LocalDateTime.now()); + + fileStorage.deleteFilesBySpaceIds(List.of("non-existing-space")); + + verify(s3Client, never()).deleteObjects(any(DeleteObjectsRequest.class)); + } + + @Test + void testDeleteFilesBySpaceAndNamespaceWithOneMatch() { + setupListObjectsWithKeys(TEST_ID, TEST_ID_2); + setupHeadObjectWithMetadata(TEST_ID, TEST_SPACE_ID, NAMESPACE, LocalDateTime.now()); + setupHeadObjectWithMetadata(TEST_ID_2, TEST_SPACE_ID_2, NAMESPACE_2, LocalDateTime.now()); + + fileStorage.deleteFilesBySpaceAndNamespace(TEST_SPACE_ID, NAMESPACE); + + ArgumentCaptor captor = ArgumentCaptor.forClass(DeleteObjectsRequest.class); + verify(s3Client).deleteObjects(captor.capture()); + assertEquals(1, captor.getValue() + .delete() + .objects() + .size()); + assertEquals(TEST_ID, captor.getValue() + .delete() + .objects() + .getFirst() + .key()); + } + + @Test + void testDeleteFilesModifiedBefore() { + LocalDateTime oldModified = LocalDateTime.now() + .minusMinutes(15); + + setupListObjectsWithKeys(TEST_ID, TEST_ID_2); + setupHeadObjectWithMetadata(TEST_ID, TEST_SPACE_ID, NAMESPACE, oldModified); + setupHeadObjectWithMetadata(TEST_ID_2, TEST_SPACE_ID_2, NAMESPACE_2, oldModified); + + int deletedCount = fileStorage.deleteFilesModifiedBefore(LocalDateTime.now()); + + assertEquals(2, deletedCount); + verify(s3Client).deleteObjects(any(DeleteObjectsRequest.class)); + } + + @Test + void testDeleteFilesModifiedBeforeWithNoOldFiles() { + setupListObjectsWithKeys(TEST_ID); + setupHeadObjectWithMetadata(TEST_ID, TEST_SPACE_ID, NAMESPACE, LocalDateTime.now()); + + LocalDateTime cutoff = LocalDateTime.now() + .minusDays(1); + int deletedCount = fileStorage.deleteFilesModifiedBefore(cutoff); + + assertEquals(0, deletedCount); + verify(s3Client, never()).deleteObjects(any(DeleteObjectsRequest.class)); + } + + @SuppressWarnings("unchecked") + @Test + void testProcessFileContent() throws FileStorageException, IOException { + ResponseInputStream mockStream = mock(ResponseInputStream.class); + when(s3Client.getObject(any(GetObjectRequest.class))).thenReturn(mockStream); + when(fileContentProcessor.process(any(InputStream.class))).thenReturn("result"); + + String result = fileStorage.processFileContent(TEST_SPACE_ID, TEST_ID, fileContentProcessor); + + assertEquals("result", result); + verify(fileContentProcessor).process(mockStream); + } + + @Test + void testProcessFileContentWithNoSuchKeyException() { + when(s3Client.getObject(any(GetObjectRequest.class))).thenThrow(NoSuchKeyException.builder() + .build()); + + assertThrows(FileStorageException.class, () -> fileStorage.processFileContent(TEST_SPACE_ID, TEST_ID, fileContentProcessor)); + } + + @SuppressWarnings("unchecked") + @Test + void testProcessFileContentWithProcessorException() throws IOException { + ResponseInputStream mockStream = mock(ResponseInputStream.class); + when(s3Client.getObject(any(GetObjectRequest.class))).thenReturn(mockStream); + when(fileContentProcessor.process(any(InputStream.class))).thenThrow(new IOException("processing failed")); + + assertThrows(FileStorageException.class, () -> fileStorage.processFileContent(TEST_SPACE_ID, TEST_ID, fileContentProcessor)); + } + + @SuppressWarnings("unchecked") + @Test + void testOpenInputStream() throws FileStorageException { + ResponseInputStream mockStream = mock(ResponseInputStream.class); + when(s3Client.getObject(any(GetObjectRequest.class))).thenReturn(mockStream); + + InputStream result = fileStorage.openInputStream(TEST_SPACE_ID, TEST_ID); + + assertNotNull(result); + ArgumentCaptor captor = ArgumentCaptor.forClass(GetObjectRequest.class); + verify(s3Client).getObject(captor.capture()); + assertEquals(BUCKET_NAME, captor.getValue() + .bucket()); + assertEquals(TEST_ID, captor.getValue() + .key()); + } + + @Test + void testOpenInputStreamWithNoSuchKeyException() { + when(s3Client.getObject(any(GetObjectRequest.class))).thenThrow(NoSuchKeyException.builder() + .build()); + + assertThrows(FileStorageException.class, () -> fileStorage.openInputStream(TEST_SPACE_ID, TEST_ID)); + } + + @Test + void testTestConnection() { + when(s3Client.listObjectsV2(any(ListObjectsV2Request.class))).thenReturn(ListObjectsV2Response.builder() + .isTruncated(false) + .build()); + + fileStorage.testConnection(); + + ArgumentCaptor captor = ArgumentCaptor.forClass(ListObjectsV2Request.class); + verify(s3Client).listObjectsV2(captor.capture()); + assertEquals(BUCKET_NAME, captor.getValue() + .bucket()); + assertEquals(1, captor.getValue() + .maxKeys()); + } + + @Test + void testDeleteFilesByIds() { + when(s3Client.deleteObjects(any(DeleteObjectsRequest.class))).thenReturn(DeleteObjectsResponse.builder() + .build()); + + fileStorage.deleteFilesByIds(List.of(TEST_ID)); + + ArgumentCaptor captor = ArgumentCaptor.forClass(DeleteObjectsRequest.class); + verify(s3Client).deleteObjects(captor.capture()); + assertEquals(1, captor.getValue() + .delete() + .objects() + .size()); + assertEquals(TEST_ID, captor.getValue() + .delete() + .objects() + .getFirst() + .key()); + } + + @Test + void testDeleteFilesByIdsWithEmptyList() { + fileStorage.deleteFilesByIds(List.of()); + + verify(s3Client, never()).listObjectsV2(any(ListObjectsV2Request.class)); + verify(s3Client, never()).deleteObjects(any(DeleteObjectsRequest.class)); + } + + @Test + void testDeleteFilesByIdsWithAllMatching() { + when(s3Client.deleteObjects(any(DeleteObjectsRequest.class))).thenReturn(DeleteObjectsResponse.builder() + .build()); + + fileStorage.deleteFilesByIds(List.of(TEST_ID, TEST_ID_2)); + + ArgumentCaptor captor = ArgumentCaptor.forClass(DeleteObjectsRequest.class); + verify(s3Client).deleteObjects(captor.capture()); + assertEquals(2, captor.getValue() + .delete() + .objects() + .size()); + } + + @SuppressWarnings("unchecked") + @Test + void testProcessArchiveEntryContent() throws FileStorageException, IOException { + ResponseInputStream mockStream = mock(ResponseInputStream.class); + when(s3Client.getObject(any(GetObjectRequest.class))).thenReturn(mockStream); + when(fileContentProcessor.process(any(InputStream.class))).thenReturn("archive-result"); + + FileContentToProcess fileContentToProcess = ImmutableFileContentToProcess.builder() + .guid(TEST_ID) + .spaceGuid(TEST_SPACE_ID) + .startOffset(100L) + .endOffset(500L) + .build(); + + String result = fileStorage.processArchiveEntryContent(fileContentToProcess, fileContentProcessor); + + assertEquals("archive-result", result); + ArgumentCaptor captor = ArgumentCaptor.forClass(GetObjectRequest.class); + verify(s3Client).getObject(captor.capture()); + assertEquals(BUCKET_NAME, captor.getValue() + .bucket()); + assertEquals(TEST_ID, captor.getValue() + .key()); + assertEquals("bytes=100-500", captor.getValue() + .range()); + } + + @Test + void testProcessArchiveEntryContentWithNoSuchKeyException() { + when(s3Client.getObject(any(GetObjectRequest.class))).thenThrow(NoSuchKeyException.builder() + .build()); + FileContentToProcess fileContentToProcess = ImmutableFileContentToProcess.builder() + .guid(TEST_ID) + .spaceGuid(TEST_SPACE_ID) + .startOffset(0L) + .endOffset(100L) + .build(); + + assertThrows(FileStorageException.class, () -> fileStorage.processArchiveEntryContent(fileContentToProcess, fileContentProcessor)); + } + + @Test + void getExistingFileEntriesWhenAllEntriesExist() { + when(s3Client.headObject(any(HeadObjectRequest.class))).thenReturn(HeadObjectResponse.builder() + .build()); + FileEntry firstEntry = createFileEntry(TEST_SPACE_ID, TEST_ID); + FileEntry secondEntry = createFileEntry(TEST_SPACE_ID_2, TEST_ID_2); + + List result = fileStorage.getExistingFileEntries(List.of(firstEntry, secondEntry)); + + assertEquals(2, result.size()); + List returnedIds = result.stream() + .map(FileEntry::getId) + .toList(); + assertTrue(returnedIds.contains(TEST_ID)); + assertTrue(returnedIds.contains(TEST_ID_2)); + } + + @Test + void getExistingFileEntriesWhenNoEntriesExist() { + when(s3Client.headObject(any(HeadObjectRequest.class))).thenThrow(NoSuchKeyException.builder() + .build()); + FileEntry firstEntry = createFileEntry(TEST_SPACE_ID, TEST_ID); + FileEntry secondEntry = createFileEntry(TEST_SPACE_ID_2, TEST_ID_2); + + List result = fileStorage.getExistingFileEntries(List.of(firstEntry, secondEntry)); + + assertTrue(result.isEmpty()); + } + + @Test + void getExistingFileEntriesWhenSomeEntriesExist() { + when(s3Client.headObject(headObjectRequestForKey(TEST_ID))).thenReturn(HeadObjectResponse.builder() + .build()); + when(s3Client.headObject(headObjectRequestForKey(TEST_ID_2))).thenThrow(NoSuchKeyException.builder() + .build()); + FileEntry existingEntry = createFileEntry(TEST_SPACE_ID, TEST_ID); + FileEntry nonExistingEntry = createFileEntry(TEST_SPACE_ID_2, TEST_ID_2); + + List result = fileStorage.getExistingFileEntries(List.of(existingEntry, nonExistingEntry)); + + assertEquals(1, result.size()); + assertEquals(TEST_ID, result.getFirst() + .getId()); + } + + @Test + void testDeleteFilesBySpaceIdsWithPagination() { + String thirdId = UUID.randomUUID() + .toString(); + ListObjectsV2Response firstPage = ListObjectsV2Response.builder() + .contents(S3Object.builder() + .key(TEST_ID) + .build()) + .isTruncated(true) + .nextContinuationToken("token-1") + .build(); + ListObjectsV2Response secondPage = ListObjectsV2Response.builder() + .contents(S3Object.builder() + .key(TEST_ID_2) + .build(), + S3Object.builder() + .key(thirdId) + .build()) + .isTruncated(false) + .build(); + when(s3Client.listObjectsV2(any(ListObjectsV2Request.class))).thenReturn(firstPage, secondPage); + when(s3Client.deleteObjects(any(DeleteObjectsRequest.class))).thenReturn(DeleteObjectsResponse.builder() + .build()); + setupHeadObjectWithMetadata(TEST_ID, TEST_SPACE_ID, NAMESPACE, LocalDateTime.now()); + setupHeadObjectWithMetadata(TEST_ID_2, TEST_SPACE_ID, NAMESPACE, LocalDateTime.now()); + setupHeadObjectWithMetadata(thirdId, TEST_SPACE_ID_2, NAMESPACE_2, LocalDateTime.now()); + + fileStorage.deleteFilesBySpaceIds(List.of(TEST_SPACE_ID)); + + ArgumentCaptor captor = ArgumentCaptor.forClass(DeleteObjectsRequest.class); + verify(s3Client).deleteObjects(captor.capture()); + assertEquals(2, captor.getValue() + .delete() + .objects() + .size()); + verify(s3Client, times(2)).listObjectsV2(any(ListObjectsV2Request.class)); + } + + @Test + void testDeleteFilesBySpaceIdsWhenHeadObjectReturnsNoSuchKey() { + setupListObjectsWithKeys(TEST_ID); + when(s3Client.headObject(any(HeadObjectRequest.class))).thenThrow(NoSuchKeyException.builder() + .build()); + + fileStorage.deleteFilesBySpaceIds(List.of(TEST_SPACE_ID)); + + verify(s3Client, never()).deleteObjects(any(DeleteObjectsRequest.class)); + } + + @Test + void getExistingFileEntriesWithEmptyList() { + List result = fileStorage.getExistingFileEntries(List.of()); + + assertTrue(result.isEmpty()); + verify(s3Client, never()).headObject(any(HeadObjectRequest.class)); + } + + @Test + void testBuildClientOverrideConfigSetsApiCallTimeout() { + ClientOverrideConfiguration config = fileStorage.buildClientOverrideConfig(); + + assertTrue(config.apiCallTimeout() + .isPresent()); + assertEquals(ObjectStoreConstants.AWS_OBJECT_STORE_TOTAL_TIMEOUT_CONFIG_IN_MINUTES, config.apiCallTimeout() + .get()); + } + + @Test + void testBuildClientOverrideConfigSetsApiCallAttemptTimeout() { + ClientOverrideConfiguration config = fileStorage.buildClientOverrideConfig(); + + assertTrue(config.apiCallAttemptTimeout() + .isPresent()); + assertEquals(ObjectStoreConstants.OBJECT_STORE_TOTAL_TIMEOUT_CONFIG_IN_MINUTES, config.apiCallAttemptTimeout() + .get()); + } + + @Test + void testBuildClientOverrideConfigSetsRetryStrategy() { + ClientOverrideConfiguration config = fileStorage.buildClientOverrideConfig(); + + assertTrue(config.retryStrategy() + .isPresent()); + assertInstanceOf(StandardRetryStrategy.class, config.retryStrategy() + .get()); + assertEquals(ObjectStoreConstants.OBJECT_STORE_MAX_ATTEMPTS_CONFIG, config.retryStrategy() + .get() + .maxAttempts()); + } + + @Test + void testCreateS3ClientWithValidCredentials() { + Map credentials = Map.of("access_key_id", "test-key", + "secret_access_key", "test-secret", + "bucket", "test-bucket", + "host", "s3.amazonaws.com", + "region", "eu-central-1"); + AwsS3ObjectStoreFileStorage storage = new AwsS3ObjectStoreFileStorage(credentials); + + assertNotNull(storage); + storage.destroy(); + } + + private HeadObjectRequest headObjectRequestForKey(String key) { + return HeadObjectRequest.builder() + .bucket(BUCKET_NAME) + .key(key) + .build(); + } + + private void setupListObjects(String... keys) { + ListObjectsV2Response.Builder responseBuilder = ListObjectsV2Response.builder() + .isTruncated(false); + List s3Objects = Arrays.stream(keys) + .map(key -> S3Object.builder() + .key(key) + .build()) + .toList(); + responseBuilder.contents(s3Objects); + when(s3Client.listObjectsV2(any(ListObjectsV2Request.class))).thenReturn(responseBuilder.build()); + } + + private void setupListObjectsWithKeys(String... keys) { + setupListObjects(keys); + when(s3Client.deleteObjects(any(DeleteObjectsRequest.class))).thenReturn(DeleteObjectsResponse.builder() + .build()); + } + + private void setupHeadObjectWithMetadata(String key, String space, String namespace, LocalDateTime modified) { + long modifiedMillis = modified.atZone(ZoneId.systemDefault()) + .toInstant() + .toEpochMilli(); + HeadObjectResponse response = HeadObjectResponse.builder() + .metadata(Map.of("space", space, + "namespace", namespace, + "modified", Long.toString(modifiedMillis))) + .build(); + when(s3Client.headObject(headObjectRequestForKey(key))).thenReturn(response); + } + + private static FileEntry createFileEntry(String space, String id) { + return ImmutableFileEntry.builder() + .space(space) + .size(BigInteger.TEN) + .modified(LocalDateTime.now()) + .id(id) + .build(); + } +} diff --git a/multiapps-controller-process/src/test/java/org/cloudfoundry/multiapps/controller/process/util/OperationInErrorStateHandlerTest.java b/multiapps-controller-process/src/test/java/org/cloudfoundry/multiapps/controller/process/util/OperationInErrorStateHandlerTest.java index 862eb246eb..987b28d5f0 100644 --- a/multiapps-controller-process/src/test/java/org/cloudfoundry/multiapps/controller/process/util/OperationInErrorStateHandlerTest.java +++ b/multiapps-controller-process/src/test/java/org/cloudfoundry/multiapps/controller/process/util/OperationInErrorStateHandlerTest.java @@ -201,11 +201,11 @@ private void prepareOperationService() { } private void assertErrorStateSet() { - Operation updatedOperation = ImmutableOperation.builder() - .state(Operation.State.ERROR) - .build(); + Operation errorOperation = ImmutableOperation.builder() + .state(Operation.State.ERROR) + .build(); Mockito.verify(operationService) - .update(updatedOperation, updatedOperation); + .update(errorOperation, errorOperation); } private OperationInErrorStateHandlerMock mockHandler() { diff --git a/multiapps-controller-web/pom.xml b/multiapps-controller-web/pom.xml index aee088b489..ce4db1a8cc 100644 --- a/multiapps-controller-web/pom.xml +++ b/multiapps-controller-web/pom.xml @@ -207,5 +207,13 @@ com.google.cloud google-cloud-nio + + software.amazon.awssdk + s3 + + + software.amazon.awssdk + url-connection-client + diff --git a/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/Constants.java b/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/Constants.java index eea2d64634..f7fecd8d0c 100644 --- a/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/Constants.java +++ b/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/Constants.java @@ -32,13 +32,12 @@ private Constants() { public static final String REGION = "region"; public static final String ENDPOINT = "endpoint"; public static final String ENDPOINT_URL = "endpoint-url"; - public static final String ACCOUNT_NAME = "account_name"; public static final String SAS_TOKEN = "sas_token"; - public static final String CONTAINER_NAME = "container_name"; public static final String CONTAINER_NAME_WITH_DASH = "container-name"; - public static final String CONTAINER_URI = "container_uri"; public static final String BASE_64_ENCODED_PRIVATE_KEY_DATA = "base64EncodedPrivateKeyData"; public static final String HOST = "host"; + public static final Set OBJECT_STORE_CUSTOM_REGIONS = Set.of("eu-south-1"); + public static final String OBJECT_STORE_JCLOUDS_REGIONS = "jclouds.regions"; public static final String AWS_S_3 = "aws-s3"; public static final String AZUREBLOB = "azureblob"; diff --git a/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/Messages.java b/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/Messages.java index 2cce47657e..10701c71d9 100644 --- a/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/Messages.java +++ b/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/Messages.java @@ -18,7 +18,7 @@ public final class Messages { public static final String ERROR_FROM_REMOTE_MTAR_ENDPOINT_FOR_JOB_WITH_ID = "Error from remote MTAR endpoint {0} with status code {1}, message: {2}. Job id: {3}"; public static final String MTAR_ENDPOINT_NOT_SECURE_FOR_JOB_WITH_ID = "Remote MTAR endpoint is not a secure connection. HTTPS required. Job id: {0}"; public static final String REQUEST_0_1_FAILED_WITH_2 = "Request \"{0} {1}\" failed with \"{2}\""; - public static final String ERROR_OCCURRED_WHILE_DELETING_JOB_ENTRY = "Error occurred while deleting job entry"; + public static final String ERROR_OCCURRED_WHILE_DELETING_JOB_ENTRY = "Error occurred while deleting job entry with id: {}"; public static final String CANNOT_CREATE_OBJECT_STORE_CLIENT_WITH_PROVIDER_0 = "Cannot create Object Store client with provider: {0}"; public static final String NO_VALID_OBJECT_STORE_CONFIGURATION_FOUND = "No valid Object Store configuration found!"; public static final String MISSING_PROPERTIES_FOR_CREATING_THE_SPECIFIC_PROVIDER = "Missing properties for creating the specific provider!"; @@ -53,11 +53,15 @@ public final class Messages { public static final String UNSUPPORTED_TOKEN_TYPE = "Unsupported token type: \"{0}\"."; public static final String CLEARING_FLOWABLE_LOCK_OWNER_THREW_AN_EXCEPTION_0 = "Clearing Flowable lock owner on JVM shutdown threw an exception: {0}"; public static final String FETCHING_FILE_FAILED = "Fetching file {0} in space {1} failed with: {2}"; - public static final String ASYNC_UPLOAD_JOB_FAILED = "Async upload job {0} failed with: {1}"; + public static final String ASYNC_UPLOAD_JOB_FAILED = "Async upload job {0} for file \"{1}\" failed with: {2}"; + public static final String ASYNC_UPLOAD_JOB_ERROR = "{0} Async upload job id: {1}"; // WARN log messages public static final String NO_OBJECTSTORE_PROVIDER_FOUND_FOR_0 = "No ObjectStore provider found for {0}!"; + public static final String NO_OBJECT_STORE_PROVIDERS_DETECTED = "No object store providers detected from credentials. Service name: {}"; + public static final String FILE_UPLOAD_ATTEMPT_FAILED = "Upload attempt {0}/{1} failed. Retrying in {2} ms. Cause: {3}"; + public static final String FILE_UPLOAD_ALL_ATTEMPTS_EXHAUSTED = "All {0} upload attempts exhausted. Last error: {1}"; // INFO log messages public static final String ALM_SERVICE_ENV_INITIALIZED = "Deploy service environment initialized"; @@ -68,8 +72,12 @@ public final class Messages { public static final String OBJECTSTORE_FOR_BINARIES_STORAGE = "Objectstore will be used for binaries storage"; public static final String CLEARING_LOCK_OWNER = "Clearing lock owner {0}..."; public static final String CLEARED_LOCK_OWNER = "Cleared lock owner {0}"; + public static final String OBJECT_STORE_PROVIDERS_DETECTED = "Object store providers detected: {}"; + public static final String ATTEMPTING_TO_CREATE_OBJECT_STORE_CLIENT = "Attempting to create object store client with provider: {}, container: {}, region: {}, host: {}, endpoint: {}"; public static final String OBJECT_STORE_WITH_PROVIDER_0_CREATED = "Object store with provider: {0} created"; - public static final String JOB_WITH_ID_WAS_NOT_UPDATED_WITHIN_SECONDS = "Job with ID: {} was not updated within: {} seconds"; + public static final String JOB_WITH_ID_WAS_NOT_UPDATED_WITHIN_SECONDS_ON_START = "startUploadFromUrl: Job with ID: {} was not updated within: {} seconds, clearing and re-triggering"; + public static final String JOB_WITH_ID_WAS_NOT_UPDATED_WITHIN_SECONDS_ON_GET = "getUploadFromUrlJob: Job with ID: {} was not updated within: {} seconds, returning error"; + public static final String STALE_JOB_DETAILS = "Stale job details - id: {}, state: {}, updatedAt: {}, addedAt: {}, startedAt: {}, bytesRead: {}, url: {}, space: {}, namespace: {}, user: {}, instance: {}"; public static final String CLEARING_OLD_ENTRY = "Clearing old entry with id: {0}"; // DEBUG log messages @@ -84,6 +92,7 @@ public final class Messages { public static final String ASYNC_UPLOAD_JOB_FINISHED = "Async upload job {} finished"; public static final String UPLOADING_MTAR_STREAM_FROM_REMOTE_ENDPOINT_WITH_JOB_ID = "Uploading MTAR stream from remote endpoint: {}. Job id: {}"; public static final String CALLING_REMOTE_MTAR_ENDPOINT_FOR_JOB_WITH_ID = "Calling remote MTAR endpoint {}. Job id: {}"; + public static final String ASYNC_UPLOAD_JOB_MONITOR_UPDATE = "Job {} monitor update - state: {}, bytesRead: {}, updatedAt: {}"; private Messages() { } diff --git a/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/api/impl/FilesApiServiceImpl.java b/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/api/impl/FilesApiServiceImpl.java index 16e4721aff..3c8996c4b4 100644 --- a/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/api/impl/FilesApiServiceImpl.java +++ b/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/api/impl/FilesApiServiceImpl.java @@ -134,6 +134,11 @@ public ResponseEntity startUploadFromUrl(String spaceGuid, String namespac return triggerUploadFromUrl(spaceGuid, namespace, urlWithoutUserInfo, decodedUrl, fileUrl.getUserCredentials()); } if (hasJobStuck(existingJob)) { + LOGGER.warn(Messages.JOB_WITH_ID_WAS_NOT_UPDATED_WITHIN_SECONDS_ON_START, existingJob.getId(), UPDATE_JOB_TIMEOUT); + LOGGER.warn(Messages.STALE_JOB_DETAILS, existingJob.getId(), existingJob.getState(), existingJob.getUpdatedAt(), + existingJob.getAddedAt(), existingJob.getStartedAt(), existingJob.getBytesRead(), existingJob.getUrl(), + existingJob.getSpaceGuid(), existingJob.getNamespace(), existingJob.getUser(), + existingJob.getInstanceIndex()); deleteAsyncJobEntry(existingJob); return triggerUploadFromUrl(spaceGuid, namespace, urlWithoutUserInfo, decodedUrl, fileUrl.getUserCredentials()); } @@ -167,7 +172,10 @@ public ResponseEntity getUploadFromUrlJob(String spaceGuid, S private ResponseEntity getAsyncUploadResult(AsyncUploadJobEntry job) { if (job.getState() == State.RUNNING || job.getState() == State.INITIAL) { if (hasJobStuck(job)) { - LOGGER.info(Messages.JOB_WITH_ID_WAS_NOT_UPDATED_WITHIN_SECONDS, job.getId(), UPDATE_JOB_TIMEOUT); + LOGGER.warn(Messages.JOB_WITH_ID_WAS_NOT_UPDATED_WITHIN_SECONDS_ON_GET, job.getId(), UPDATE_JOB_TIMEOUT); + LOGGER.warn(Messages.STALE_JOB_DETAILS, job.getId(), job.getState(), job.getUpdatedAt(), job.getAddedAt(), + job.getStartedAt(), job.getBytesRead(), job.getUrl(), job.getSpaceGuid(), job.getNamespace(), + job.getUser(), job.getInstanceIndex()); return ResponseEntity.ok( createErrorResult(MessageFormat.format(Messages.JOB_NOT_UPDATED_FOR_0_SECONDS, UPDATE_JOB_TIMEOUT), AsyncUploadResult.ClientAction.RETRY_UPLOAD)); @@ -257,7 +265,7 @@ private void deleteAsyncJobEntry(AsyncUploadJobEntry entry) { .id(entry.getId()) .delete(); } catch (Exception e) { - LOGGER.error(Messages.ERROR_OCCURRED_WHILE_DELETING_JOB_ENTRY, e); + LOGGER.error(Messages.ERROR_OCCURRED_WHILE_DELETING_JOB_ENTRY, entry.getId(), e); } } diff --git a/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/configuration/bean/factory/ObjectStoreFileStorageFactoryBean.java b/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/configuration/bean/factory/ObjectStoreFileStorageFactoryBean.java index 18020e1a0a..18e473873b 100644 --- a/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/configuration/bean/factory/ObjectStoreFileStorageFactoryBean.java +++ b/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/configuration/bean/factory/ObjectStoreFileStorageFactoryBean.java @@ -16,6 +16,7 @@ import org.apache.commons.lang3.StringUtils; import org.cloudfoundry.multiapps.controller.core.util.ApplicationConfiguration; import org.cloudfoundry.multiapps.controller.core.util.UriUtil; +import org.cloudfoundry.multiapps.controller.persistence.services.AwsS3ObjectStoreFileStorage; import org.cloudfoundry.multiapps.controller.persistence.services.AzureObjectStoreFileStorage; import org.cloudfoundry.multiapps.controller.persistence.services.FileStorage; import org.cloudfoundry.multiapps.controller.persistence.services.GcpObjectStoreFileStorage; @@ -26,7 +27,6 @@ import org.cloudfoundry.multiapps.controller.web.configuration.service.ObjectStoreServiceInfo; import org.cloudfoundry.multiapps.controller.web.configuration.service.ObjectStoreServiceInfoCreator; import org.jclouds.ContextBuilder; -import org.jclouds.aws.domain.Region; import org.jclouds.blobstore.BlobStoreContext; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -36,8 +36,6 @@ public class ObjectStoreFileStorageFactoryBean implements FactoryBean, InitializingBean { private static final Logger LOGGER = LoggerFactory.getLogger(ObjectStoreFileStorageFactoryBean.class); - private static final Set CUSTOM_REGIONS = Set.of("eu-south-1"); - private static final String JCLOUDS_REGIONS = "jclouds.regions"; private final String serviceName; private final EnvironmentServicesFinder environmentServicesFinder; @@ -58,7 +56,13 @@ public void afterPropertiesSet() { private FileStorage createObjectStoreFileStorage() { List providersServiceInfo = getProvidersServiceInfo(); + if (LOGGER.isInfoEnabled()) { + LOGGER.info(Messages.OBJECT_STORE_PROVIDERS_DETECTED, providersServiceInfo.stream() + .map(ObjectStoreServiceInfo::getProvider) + .collect(Collectors.joining(", "))); + } if (providersServiceInfo.isEmpty()) { + LOGGER.warn(Messages.NO_OBJECT_STORE_PROVIDERS_DETECTED, serviceName); return null; } Map exceptions = new HashMap<>(); @@ -66,17 +70,36 @@ private FileStorage createObjectStoreFileStorage() { if (!isObjectStoreEnvValid(objectStoreProviderName)) { return createObjectStoreFromFirstReachableProvider(exceptions, providersServiceInfo); } - Optional optionalFileStorage = createObjectStoreBasedOnProvider(objectStoreProviderName, providersServiceInfo, exceptions); - if (optionalFileStorage.isPresent()) { return optionalFileStorage.get(); } - throw buildNoValidObjectStoreException(exceptions); } + public List getProvidersServiceInfo() { + Map credentials = getServiceCredentials(); + if (credentials.isEmpty()) { + return Collections.emptyList(); + } + return new ObjectStoreServiceInfoCreator().getAllProvidersServiceInfo(credentials); + } + + private Map getServiceCredentials() { + CfService service = environmentServicesFinder.findService(serviceName); + if (service == null) { + return Map.of(); + } + return service.getCredentials() + .getMap(); + } + + private boolean isObjectStoreEnvValid(String objectStoreProviderName) { + return objectStoreProviderName != null && !objectStoreProviderName.isEmpty() && Constants.ENV_TO_OS_PROVIDER.containsKey( + objectStoreProviderName); + } + public FileStorage createObjectStoreFromFirstReachableProvider(Map exceptions, List providersServiceInfo) { for (ObjectStoreServiceInfo objectStoreServiceInfo : providersServiceInfo) { @@ -88,29 +111,19 @@ public FileStorage createObjectStoreFromFirstReachableProvider(Map createObjectStoreBasedOnProvider(String objectStoreProviderName, - List providersServiceInfo, - Map exceptions) { - Optional objectStoreServiceInfoOptional = getAppropriateProvider(objectStoreProviderName, - providersServiceInfo); - if (objectStoreServiceInfoOptional.isEmpty()) { - LOGGER.warn(MessageFormat.format(Messages.NO_OBJECTSTORE_PROVIDER_FOUND_FOR_0, objectStoreProviderName)); - return Optional.empty(); - } - return tryToCreateObjectStore(objectStoreServiceInfoOptional.get(), exceptions); - } - - private Optional getAppropriateProvider(String objectStoreProviderName, - List providersServiceInfo) { - String appropriateProvider = Constants.ENV_TO_OS_PROVIDER.get(objectStoreProviderName); - return providersServiceInfo.stream() - .filter(provider -> appropriateProvider.equals(provider.getProvider())) - .findFirst(); - } - private Optional tryToCreateObjectStore(ObjectStoreServiceInfo objectStoreServiceInfo, Map exceptions) { try { + LOGGER.info(Messages.ATTEMPTING_TO_CREATE_OBJECT_STORE_CLIENT, + objectStoreServiceInfo.getProvider(), + objectStoreServiceInfo.getCredentials() + .get(Constants.BUCKET), + objectStoreServiceInfo.getCredentials() + .get(Constants.REGION), + objectStoreServiceInfo.getCredentials() + .get(Constants.HOST), + objectStoreServiceInfo.getCredentials() + .get(Constants.ENDPOINT)); FileStorage fileStorage = getFileStorageBasedOnProvider(objectStoreServiceInfo); fileStorage.testConnection(); LOGGER.info(MessageFormat.format(Messages.OBJECT_STORE_WITH_PROVIDER_0_CREATED, objectStoreServiceInfo.getProvider())); @@ -125,6 +138,7 @@ private FileStorage getFileStorageBasedOnProvider(ObjectStoreServiceInfo objectS return switch (objectStoreServiceInfo.getProvider()) { case Constants.GOOGLE_CLOUD_STORAGE -> createGcpFileStorage(objectStoreServiceInfo); case Constants.AZUREBLOB -> createAzureFileStorage(objectStoreServiceInfo); + case Constants.AWS_S_3 -> createAwsS3FileStorage(objectStoreServiceInfo); default -> { BlobStoreContext context = getBlobStoreContext(objectStoreServiceInfo); yield createFileStorage(objectStoreServiceInfo, context); @@ -132,33 +146,16 @@ private FileStorage getFileStorageBasedOnProvider(ObjectStoreServiceInfo objectS }; } - private boolean isObjectStoreEnvValid(String objectStoreProviderName) { - return objectStoreProviderName != null && !objectStoreProviderName.isEmpty() && Constants.ENV_TO_OS_PROVIDER.containsKey( - objectStoreProviderName); - } - - private IllegalStateException buildNoValidObjectStoreException(Map exceptions) { - exceptions.forEach((provider, exception) -> LOGGER.error( - MessageFormat.format(Messages.CANNOT_CREATE_OBJECT_STORE_CLIENT_WITH_PROVIDER_0, provider), - exception)); - return new IllegalStateException(Messages.NO_VALID_OBJECT_STORE_CONFIGURATION_FOUND); + protected GcpObjectStoreFileStorage createGcpFileStorage(ObjectStoreServiceInfo objectStoreServiceInfo) { + return new GcpObjectStoreFileStorage(objectStoreServiceInfo.getCredentials()); } - public List getProvidersServiceInfo() { - Map credentials = getServiceCredentials(); - if (credentials.isEmpty()) { - return Collections.emptyList(); - } - return new ObjectStoreServiceInfoCreator().getAllProvidersServiceInfo(credentials); + protected AzureObjectStoreFileStorage createAzureFileStorage(ObjectStoreServiceInfo objectStoreServiceInfo) { + return new AzureObjectStoreFileStorage(objectStoreServiceInfo.getCredentials()); } - private Map getServiceCredentials() { - CfService service = environmentServicesFinder.findService(serviceName); - if (service == null) { - return Map.of(); - } - return service.getCredentials() - .getMap(); + protected AwsS3ObjectStoreFileStorage createAwsS3FileStorage(ObjectStoreServiceInfo objectStoreServiceInfo) { + return new AwsS3ObjectStoreFileStorage(objectStoreServiceInfo.getCredentials()); } private BlobStoreContext getBlobStoreContext(ObjectStoreServiceInfo serviceInfo) { @@ -166,61 +163,77 @@ private BlobStoreContext getBlobStoreContext(ObjectStoreServiceInfo serviceInfo) applyCredentials(serviceInfo, contextBuilder); addCustomRegions(contextBuilder); resolveContextEndpoint(serviceInfo, contextBuilder); - BlobStoreContext context = contextBuilder.buildView(BlobStoreContext.class); if (context == null) { throw new IllegalStateException(Messages.FAILED_TO_CREATE_BLOB_STORE_CONTEXT); } - return context; } + private void applyCredentials(ObjectStoreServiceInfo serviceInfo, ContextBuilder contextBuilder) { + Map credentials = serviceInfo.getCredentials(); + String identity = (String) credentials.get(Constants.ACCESS_KEY_ID); + String credential = (String) credentials.get(Constants.SECRET_ACCESS_KEY); + if (StringUtils.isBlank(identity) || StringUtils.isBlank(credential)) { + throw new IllegalArgumentException(Messages.MISSING_PROPERTIES_FOR_CREATING_THE_SPECIFIC_PROVIDER); + } + contextBuilder.credentials(identity, credential); + } + private void addCustomRegions(ContextBuilder contextBuilder) { Properties properties = new Properties(); - Set mergedRegions = Stream.of(CUSTOM_REGIONS, Region.DEFAULT_REGIONS, applicationConfiguration.getObjectStoreRegions()) + Set mergedRegions = Stream.of(Constants.OBJECT_STORE_CUSTOM_REGIONS, applicationConfiguration.getObjectStoreRegions()) .flatMap(Set::stream) .collect(Collectors.toSet()); - properties.setProperty(JCLOUDS_REGIONS, Joiner.on(',') - .join(mergedRegions)); - + properties.setProperty(Constants.OBJECT_STORE_JCLOUDS_REGIONS, Joiner.on(',') + .join(mergedRegions)); contextBuilder.overrides(properties); } - private void applyCredentials(ObjectStoreServiceInfo serviceInfo, ContextBuilder contextBuilder) { - if (serviceInfo.getCredentialsSupplier() != null) { - contextBuilder.credentialsSupplier(serviceInfo.getCredentialsSupplier()); - } else { - String identity = serviceInfo.getIdentity(); - String credential = serviceInfo.getCredential(); - - if (StringUtils.isBlank(identity) || StringUtils.isBlank(credential)) { - throw new IllegalArgumentException(Messages.MISSING_PROPERTIES_FOR_CREATING_THE_SPECIFIC_PROVIDER); - } - - contextBuilder.credentials(identity, credential); - } - } - private void resolveContextEndpoint(ObjectStoreServiceInfo serviceInfo, ContextBuilder contextBuilder) { - if (StringUtils.isNotEmpty(serviceInfo.getEndpoint())) { - contextBuilder.endpoint(serviceInfo.getEndpoint()); + Map credentials = serviceInfo.getCredentials(); + String endpoint = (String) credentials.get(Constants.ENDPOINT); + String host = (String) credentials.get(Constants.HOST); + if (StringUtils.isNotEmpty(endpoint)) { + contextBuilder.endpoint(endpoint); return; } - if (StringUtils.isNotEmpty(serviceInfo.getHost())) { - contextBuilder.endpoint(UriUtil.HTTPS_PROTOCOL + UriUtil.DEFAULT_SCHEME_SEPARATOR + serviceInfo.getHost()); + if (StringUtils.isNotEmpty(host)) { + contextBuilder.endpoint(UriUtil.HTTPS_PROTOCOL + UriUtil.DEFAULT_SCHEME_SEPARATOR + host); } } protected JCloudsObjectStoreFileStorage createFileStorage(ObjectStoreServiceInfo objectStoreServiceInfo, BlobStoreContext context) { - return new JCloudsObjectStoreFileStorage(context.getBlobStore(), objectStoreServiceInfo.getContainer()); + return new JCloudsObjectStoreFileStorage(context.getBlobStore(), + (String) objectStoreServiceInfo.getCredentials() + .get(Constants.BUCKET)); } - protected GcpObjectStoreFileStorage createGcpFileStorage(ObjectStoreServiceInfo objectStoreServiceInfo) { - return new GcpObjectStoreFileStorage(objectStoreServiceInfo.getCredentials()); + private Optional createObjectStoreBasedOnProvider(String objectStoreProviderName, + List providersServiceInfo, + Map exceptions) { + Optional objectStoreServiceInfoOptional = getAppropriateProvider(objectStoreProviderName, + providersServiceInfo); + if (objectStoreServiceInfoOptional.isEmpty()) { + LOGGER.warn(MessageFormat.format(Messages.NO_OBJECTSTORE_PROVIDER_FOUND_FOR_0, objectStoreProviderName)); + return Optional.empty(); + } + return tryToCreateObjectStore(objectStoreServiceInfoOptional.get(), exceptions); } - protected AzureObjectStoreFileStorage createAzureFileStorage(ObjectStoreServiceInfo objectStoreServiceInfo) { - return new AzureObjectStoreFileStorage(objectStoreServiceInfo.getCredentials()); + private Optional getAppropriateProvider(String objectStoreProviderName, + List providersServiceInfo) { + String appropriateProvider = Constants.ENV_TO_OS_PROVIDER.get(objectStoreProviderName); + return providersServiceInfo.stream() + .filter(provider -> appropriateProvider.equals(provider.getProvider())) + .findFirst(); + } + + private IllegalStateException buildNoValidObjectStoreException(Map exceptions) { + exceptions.forEach((provider, exception) -> LOGGER.error( + MessageFormat.format(Messages.CANNOT_CREATE_OBJECT_STORE_CLIENT_WITH_PROVIDER_0, provider), + exception)); + return new IllegalStateException(Messages.NO_VALID_OBJECT_STORE_CONFIGURATION_FOUND); } @Override @@ -233,4 +246,4 @@ public Class getObjectType() { return FileStorage.class; } -} \ No newline at end of file +} diff --git a/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/configuration/service/ObjectStoreServiceInfo.java b/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/configuration/service/ObjectStoreServiceInfo.java index a822250a15..1d093527ad 100644 --- a/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/configuration/service/ObjectStoreServiceInfo.java +++ b/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/configuration/service/ObjectStoreServiceInfo.java @@ -2,37 +2,14 @@ import java.util.Map; -import com.google.common.base.Supplier; import org.cloudfoundry.multiapps.common.Nullable; import org.immutables.value.Value; -import org.jclouds.domain.Credentials; @Value.Immutable public interface ObjectStoreServiceInfo { String getProvider(); - @Nullable - String getIdentity(); - - @Nullable - Supplier getCredentialsSupplier(); - - @Nullable - String getCredential(); - - @Nullable - String getContainer(); - - @Nullable - String getEndpoint(); - - @Nullable - String getRegion(); - - @Nullable - String getHost(); - @Nullable Map getCredentials(); } diff --git a/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/configuration/service/ObjectStoreServiceInfoCreator.java b/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/configuration/service/ObjectStoreServiceInfoCreator.java index b12f629d51..0f0914b1b6 100644 --- a/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/configuration/service/ObjectStoreServiceInfoCreator.java +++ b/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/configuration/service/ObjectStoreServiceInfoCreator.java @@ -1,62 +1,42 @@ package org.cloudfoundry.multiapps.controller.web.configuration.service; +import org.cloudfoundry.multiapps.controller.web.Constants; + +import java.util.HashMap; import java.util.List; import java.util.Map; -import org.cloudfoundry.multiapps.controller.persistence.Messages; -import org.cloudfoundry.multiapps.controller.web.Constants; - public class ObjectStoreServiceInfoCreator { public List getAllProvidersServiceInfo(Map credentials) { - return List.of(createServiceInfoForAws(credentials), createServiceInfoForAliCloud(credentials), - createServiceInfoForCcee(credentials), createServiceInfoForAzure(credentials), createServiceInfoForGcp(credentials)); + return List.of( + createServiceInfoForAws(credentials), + createServiceInfoForCcee(credentials), + createServiceInfoForAliCloud(credentials), + createServiceInfoForAzure(credentials), + createServiceInfoForGcp(credentials) + ); } private ObjectStoreServiceInfo createServiceInfoForAws(Map credentials) { - String accessKeyId = (String) credentials.get(Constants.ACCESS_KEY_ID); - String secretAccessKey = (String) credentials.get(Constants.SECRET_ACCESS_KEY); - String bucket = (String) credentials.get(Constants.BUCKET); - String host = (String) credentials.get(Constants.HOST); - return ImmutableObjectStoreServiceInfo.builder() - .provider(Constants.AWS_S_3) - .identity(accessKeyId) - .credential(secretAccessKey) - .container(bucket) - .host(host) - .build(); + return createObjectStoreServiceInfo(Constants.AWS_S_3, credentials); } - private ObjectStoreServiceInfo createServiceInfoForAliCloud(Map credentials) { - String accessKeyId = (String) credentials.get(Constants.ACCESS_KEY_ID); - String secretAccessKey = (String) credentials.get(Constants.SECRET_ACCESS_KEY); - String bucket = (String) credentials.get(Constants.BUCKET); - String region = (String) credentials.get(Constants.REGION); - String endpoint = (String) credentials.get(Constants.ENDPOINT); - return ImmutableObjectStoreServiceInfo.builder() - .provider(Constants.ALIYUN_OSS) - .identity(accessKeyId) - .credential(secretAccessKey) - .container(bucket) - .endpoint(endpoint) - .region(region) - .build(); + private ObjectStoreServiceInfo createServiceInfoForCcee(Map credentials) { + Map translated = new HashMap<>(credentials); + Object containerName = credentials.get(Constants.CONTAINER_NAME_WITH_DASH); + if (containerName != null) { + translated.put(Constants.BUCKET, containerName); + } + Object endpointUrl = credentials.get(Constants.ENDPOINT_URL); + if (endpointUrl != null) { + translated.put(Constants.ENDPOINT, endpointUrl); + } + return createObjectStoreServiceInfo(Constants.AWS_S_3, translated); } - private ObjectStoreServiceInfo createServiceInfoForCcee(Map credentials) { - String accessKeyId = (String) credentials.get(Constants.ACCESS_KEY_ID); - String containerName = (String) credentials.get(Constants.CONTAINER_NAME_WITH_DASH); - String endpointUrl = (String) credentials.get(Constants.ENDPOINT_URL); - String region = (String) credentials.get(Constants.REGION); - String secretAccessKey = (String) credentials.get(Constants.SECRET_ACCESS_KEY); - return ImmutableObjectStoreServiceInfo.builder() - .provider(Constants.AWS_S_3) - .identity(accessKeyId) - .container(containerName) - .endpoint(endpointUrl) - .region(region) - .credential(secretAccessKey) - .build(); + private ObjectStoreServiceInfo createServiceInfoForAliCloud(Map credentials) { + return createObjectStoreServiceInfo(Constants.ALIYUN_OSS, credentials); } private ObjectStoreServiceInfo createServiceInfoForAzure(Map credentials) { diff --git a/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/upload/AsyncUploadJobOrchestrator.java b/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/upload/AsyncUploadJobOrchestrator.java index 7fc4a38d56..b7a3d71b46 100644 --- a/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/upload/AsyncUploadJobOrchestrator.java +++ b/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/upload/AsyncUploadJobOrchestrator.java @@ -20,8 +20,6 @@ import org.cloudfoundry.multiapps.common.SLException; import org.cloudfoundry.multiapps.common.util.MiscUtil; import org.cloudfoundry.multiapps.controller.api.model.UserCredentials; -import org.cloudfoundry.multiapps.controller.client.util.CheckedSupplier; -import org.cloudfoundry.multiapps.controller.client.util.ResilientOperationExecutor; import org.cloudfoundry.multiapps.controller.core.helpers.DescriptorParserFacadeFactory; import org.cloudfoundry.multiapps.controller.core.util.ApplicationConfiguration; import org.cloudfoundry.multiapps.controller.core.util.FileUtils; @@ -37,6 +35,7 @@ import org.cloudfoundry.multiapps.controller.web.upload.client.DeployFromUrlRemoteClient; import org.cloudfoundry.multiapps.controller.web.upload.client.FileFromUrlData; import org.cloudfoundry.multiapps.controller.web.upload.exception.RejectedAsyncUploadJobException; +import org.cloudfoundry.multiapps.controller.web.upload.resilience.FileUploadResilientOperationExecutor; import org.cloudfoundry.multiapps.controller.web.util.SecurityContextUtil; import org.cloudfoundry.multiapps.mta.handlers.ArchiveHandler; import org.cloudfoundry.multiapps.mta.handlers.DescriptorParserFacade; @@ -54,7 +53,7 @@ public class AsyncUploadJobOrchestrator { private static final Logger LOGGER = LoggerFactory.getLogger(AsyncUploadJobOrchestrator.class); - private final ResilientOperationExecutor resilientOperationExecutor = getResilientOperationExecutor(); + private final FileUploadResilientOperationExecutor fileUploadResilientOperationExecutor = getFileUploadResilientOperationExecutor(); private final ExecutorService asyncFileUploadExecutor; private final ExecutorService deployFromUrlExecutor; @@ -130,7 +129,8 @@ private void deployFromUrl(AsyncUploadJobEntry jobEntry, String fileUrl, UserCre .singleResult(); monitorAsyncUploadJob(updatedJobEntry, lock, counterRef); } catch (Exception e) { - LOGGER.error(MessageFormat.format(Messages.ASYNC_UPLOAD_JOB_FAILED, jobEntry.getId(), e.getMessage()), e); + LOGGER.error(MessageFormat.format(Messages.ASYNC_UPLOAD_JOB_FAILED, jobEntry.getId(), + extractFileName(jobEntry.getUrl()), e.getMessage()), e); updateFailedAsyncUploadJob(jobEntry, e, lock); } } @@ -140,7 +140,11 @@ private void startAsyncUploadFromUrlUpload(UploadFromUrlContext uploadFromUrlCon try { startSyncUploadFromUrlUpload(uploadFromUrlContext, lock); } catch (Exception e) { - LOGGER.error(e.getMessage(), e); + LOGGER.error(MessageFormat.format(Messages.ASYNC_UPLOAD_JOB_FAILED, uploadFromUrlContext.getJobEntry() + .getId(), + extractFileName(uploadFromUrlContext.getJobEntry() + .getUrl()), + e.getMessage()), e); updateFailedAsyncUploadJob(uploadFromUrlContext.getJobEntry(), e, lock); throw new SLException(e, e.getMessage()); } @@ -148,8 +152,7 @@ private void startAsyncUploadFromUrlUpload(UploadFromUrlContext uploadFromUrlCon } private void startSyncUploadFromUrlUpload(UploadFromUrlContext uploadFromUrlContext, Lock lock) throws Exception { - FileEntry fileEntry = resilientOperationExecutor.execute( - (CheckedSupplier) () -> doUploadMtarFromUrl(uploadFromUrlContext, lock)); + FileEntry fileEntry = fileUploadResilientOperationExecutor.execute(() -> doUploadMtarFromUrl(uploadFromUrlContext, lock)); LOGGER.trace(Messages.UPLOADED_MTAR_FROM_REMOTE_ENDPOINT_AND_JOB_ID, uploadFromUrlContext.getJobEntry() .getUrl(), uploadFromUrlContext.getJobEntry() @@ -249,6 +252,8 @@ private void monitorAsyncUploadJob(AsyncUploadJobEntry updatedJobEntry, Lock loc } finally { lock.unlock(); } + LOGGER.info(Messages.ASYNC_UPLOAD_JOB_MONITOR_UPDATE, updatedJobEntry.getId(), updatedJobEntry.getState(), + updatedJobEntry.getBytesRead(), updatedJobEntry.getUpdatedAt()); waitBetweenUpdates(); } } @@ -266,15 +271,18 @@ private void updateFailedAsyncUploadJob(AsyncUploadJobEntry jobEntry, Exception asyncUploadJobService.update(failedEntry, ImmutableAsyncUploadJobEntry.copyOf(failedEntry) .withUpdatedAt(LocalDateTime.now()) .withFinishedAt(LocalDateTime.now()) - .withError(e.getMessage()) + .withError( + MessageFormat.format(Messages.ASYNC_UPLOAD_JOB_ERROR, + e.getMessage(), + jobEntry.getId())) .withState(AsyncUploadJobEntry.State.ERROR)); } finally { lock.unlock(); } } - protected ResilientOperationExecutor getResilientOperationExecutor() { - return new ResilientOperationExecutor(); + protected FileUploadResilientOperationExecutor getFileUploadResilientOperationExecutor() { + return new FileUploadResilientOperationExecutor(); } } diff --git a/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/upload/resilience/FileUploadResilientOperationExecutor.java b/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/upload/resilience/FileUploadResilientOperationExecutor.java new file mode 100644 index 0000000000..96626294e3 --- /dev/null +++ b/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/upload/resilience/FileUploadResilientOperationExecutor.java @@ -0,0 +1,68 @@ +package org.cloudfoundry.multiapps.controller.web.upload.resilience; + +import org.cloudfoundry.multiapps.common.util.MiscUtil; +import org.cloudfoundry.multiapps.controller.client.util.CheckedSupplier; +import org.cloudfoundry.multiapps.controller.persistence.services.FileStorageException; +import org.cloudfoundry.multiapps.controller.web.Messages; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.io.IOException; +import java.io.UncheckedIOException; +import java.text.MessageFormat; +import java.time.Duration; + +public class FileUploadResilientOperationExecutor { + + private static final Logger LOGGER = LoggerFactory.getLogger(FileUploadResilientOperationExecutor.class); + + private static final int MAX_ATTEMPTS = 7; + private static final int BACKOFF_FACTOR = 2; + private static final long INITIAL_WAIT_TIME_IN_MILLIS = Duration.ofSeconds(10) + .toMillis(); + private static final long MAX_WAIT_TIME_IN_MILLIS = Duration.ofSeconds(300) + .toMillis(); + + public T execute(CheckedSupplier operation) throws Exception { + Exception lastException = null; + for (int attempt = 1; attempt <= MAX_ATTEMPTS; attempt++) { + try { + return operation.get(); + } catch (Exception e) { + if (!isTransientError(e)) { + throw e; + } + lastException = e; + if (attempt == MAX_ATTEMPTS) { + LOGGER.error(MessageFormat.format(Messages.FILE_UPLOAD_ALL_ATTEMPTS_EXHAUSTED, MAX_ATTEMPTS, e.getMessage()), e); + throw e; + } + long waitTime = calculateWaitTime(attempt); + LOGGER.warn(MessageFormat.format(Messages.FILE_UPLOAD_ATTEMPT_FAILED, attempt, MAX_ATTEMPTS, waitTime, e.getMessage()), e); + sleep(waitTime); + } + } + throw lastException; + } + + protected void sleep(long millis) { + MiscUtil.sleep(millis); + } + + private boolean isTransientError(Exception e) { + Throwable cause = e instanceof FileStorageException ? e.getCause() : e; + while (cause != null) { + if (cause instanceof IOException || cause instanceof UncheckedIOException) { + return true; + } + cause = cause.getCause(); + } + return false; + } + + private long calculateWaitTime(int attempt) { + long waitTime = (long) (INITIAL_WAIT_TIME_IN_MILLIS * Math.pow(BACKOFF_FACTOR, (double) attempt - 1)); + return Math.min(waitTime, MAX_WAIT_TIME_IN_MILLIS); + } + +} diff --git a/multiapps-controller-web/src/test/java/org/cloudfoundry/multiapps/controller/web/configuration/bean/factory/ObjectStoreFileStorageFactoryBeanTest.java b/multiapps-controller-web/src/test/java/org/cloudfoundry/multiapps/controller/web/configuration/bean/factory/ObjectStoreFileStorageFactoryBeanTest.java index bcc01c7235..7cfdb49aab 100644 --- a/multiapps-controller-web/src/test/java/org/cloudfoundry/multiapps/controller/web/configuration/bean/factory/ObjectStoreFileStorageFactoryBeanTest.java +++ b/multiapps-controller-web/src/test/java/org/cloudfoundry/multiapps/controller/web/configuration/bean/factory/ObjectStoreFileStorageFactoryBeanTest.java @@ -1,13 +1,9 @@ package org.cloudfoundry.multiapps.controller.web.configuration.bean.factory; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.Set; - import io.pivotal.cfenv.core.CfCredentials; import io.pivotal.cfenv.core.CfService; import org.cloudfoundry.multiapps.controller.core.util.ApplicationConfiguration; +import org.cloudfoundry.multiapps.controller.persistence.services.AwsS3ObjectStoreFileStorage; import org.cloudfoundry.multiapps.controller.persistence.services.AzureObjectStoreFileStorage; import org.cloudfoundry.multiapps.controller.persistence.services.FileStorage; import org.cloudfoundry.multiapps.controller.persistence.services.GcpObjectStoreFileStorage; @@ -21,26 +17,36 @@ import org.jclouds.blobstore.BlobStoreContext; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.MockitoAnnotations; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.stream.Stream; + import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.anyList; import static org.mockito.ArgumentMatchers.anyMap; +import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.never; import static org.mockito.Mockito.spy; -import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; class ObjectStoreFileStorageFactoryBeanTest { + private static final String SERVICE_NAME = "deploy-service-os"; private static final String ACCESS_KEY_ID_VALUE = "access_key_id_value"; private static final String SECRET_ACCESS_KEY_VALUE = "secret_access_key_value"; private static final String BUCKET_VALUE = "bucket_value"; @@ -53,9 +59,10 @@ class ObjectStoreFileStorageFactoryBeanTest { private ApplicationConfiguration applicationConfiguration; @Mock private JCloudsObjectStoreFileStorage jCloudsObjectStoreFileStorage; - @Mock private GcpObjectStoreFileStorage gcpObjectStoreFileStorage; + @Mock + private AwsS3ObjectStoreFileStorage awsS3ObjectStoreFileStorage; @Mock private AzureObjectStoreFileStorage azureObjectStoreFileStorage; @@ -68,89 +75,115 @@ void setUp() throws Exception { MockitoAnnotations.openMocks(this) .close(); when(applicationConfiguration.getObjectStoreRegions()).thenReturn(Set.of()); - objectStoreFileStorageFactoryBean = new ObjectStoreFileStorageFactoryBeanMock("deploy-service-os", environmentServicesFinder, + objectStoreFileStorageFactoryBean = new ObjectStoreFileStorageFactoryBeanMock(SERVICE_NAME, environmentServicesFinder, applicationConfiguration); } + @Test + void testGetObjectType() { + assertEquals(FileStorage.class, objectStoreFileStorageFactoryBean.getObjectType()); + } + + @Test + void testGetObjectBeforeInitialization() { + assertNull(objectStoreFileStorageFactoryBean.getObject()); + } + @Test void testObjectStoreCreationWithoutServiceInstance() { objectStoreFileStorageFactoryBean.afterPropertiesSet(); - FileStorage objectStoreFileStorage = objectStoreFileStorageFactoryBean.getObject(); - assertNull(objectStoreFileStorage); + assertNull(objectStoreFileStorageFactoryBean.getObject()); } @Test void testObjectStoreCreationWithValidServiceInstance() { mockCfService(); objectStoreFileStorageFactoryBean.afterPropertiesSet(); - FileStorage objectStoreFileStorage = objectStoreFileStorageFactoryBean.getObject(); - assertNotNull(objectStoreFileStorage); + assertNotNull(objectStoreFileStorageFactoryBean.getObject()); } @Test void testObjectStoreCreationWhenEnvIsValidForAws() { mockCfService(); when(applicationConfiguration.getObjectStoreClientType()).thenReturn(Constants.AWS); - ObjectStoreFileStorageFactoryBean spy = spy(objectStoreFileStorageFactoryBean); + var factoryBeanSpy = spy(objectStoreFileStorageFactoryBean); - spy.afterPropertiesSet(); - FileStorage createdObjectStoreFileStorage = spy.getObject(); + factoryBeanSpy.afterPropertiesSet(); + var result = factoryBeanSpy.getObject(); - assertNotNull(createdObjectStoreFileStorage); - verify(spy, never()) - .createObjectStoreFromFirstReachableProvider(anyMap(), anyList()); - verify(jCloudsObjectStoreFileStorage, times(1)) - .testConnection(); - assertTrue(createdObjectStoreFileStorage instanceof JCloudsObjectStoreFileStorage); + assertInstanceOf(AwsS3ObjectStoreFileStorage.class, result); + verify(factoryBeanSpy, never()).createObjectStoreFromFirstReachableProvider(anyMap(), anyList()); + verify(awsS3ObjectStoreFileStorage).testConnection(); } @Test void testObjectStoreCreationWhenEnvIsValidForAzure() { mockCfService(); when(applicationConfiguration.getObjectStoreClientType()).thenReturn(Constants.AZURE); - ObjectStoreFileStorageFactoryBean spy = spy(objectStoreFileStorageFactoryBean); + var factoryBeanSpy = spy(objectStoreFileStorageFactoryBean); - spy.afterPropertiesSet(); - FileStorage createdObjectStoreFileStorage = spy.getObject(); + factoryBeanSpy.afterPropertiesSet(); + var result = factoryBeanSpy.getObject(); - assertNotNull(createdObjectStoreFileStorage); - verify(spy, never()) - .createObjectStoreFromFirstReachableProvider(anyMap(), anyList()); - verify(azureObjectStoreFileStorage, times(1)) - .testConnection(); - assertTrue(createdObjectStoreFileStorage instanceof AzureObjectStoreFileStorage); + assertInstanceOf(AzureObjectStoreFileStorage.class, result); + verify(factoryBeanSpy, never()).createObjectStoreFromFirstReachableProvider(anyMap(), anyList()); + verify(azureObjectStoreFileStorage).testConnection(); } @Test - void testObjectStoreCreationWhenEnvIsValid() { + void testObjectStoreCreationWhenEnvIsValidForGcp() { mockCfService(); when(applicationConfiguration.getObjectStoreClientType()).thenReturn(Constants.GCP); - ObjectStoreFileStorageFactoryBean spy = spy(objectStoreFileStorageFactoryBean); + var factoryBeanSpy = spy(objectStoreFileStorageFactoryBean); - spy.afterPropertiesSet(); - FileStorage createdObjectStoreFileStorage = spy.getObject(); + factoryBeanSpy.afterPropertiesSet(); + var result = factoryBeanSpy.getObject(); - assertNotNull(createdObjectStoreFileStorage); - verify(spy, never()) - .createObjectStoreFromFirstReachableProvider(anyMap(), anyList()); - verify(gcpObjectStoreFileStorage, times(1)) - .testConnection(); - assertTrue(createdObjectStoreFileStorage instanceof GcpObjectStoreFileStorage); + assertInstanceOf(GcpObjectStoreFileStorage.class, result); + verify(factoryBeanSpy, never()).createObjectStoreFromFirstReachableProvider(anyMap(), anyList()); + verify(gcpObjectStoreFileStorage).testConnection(); } - @Test - void testObjectStoreCreationWhenEnvIsInvalid() { + static Stream testObjectStoreCreationFallsBackToFirstReachableProviderWhenEnvIsInvalid() { + return Stream.of( + // @formatter:off + // (0) Unknown provider name: + Arguments.of("WRONG_PROVIDER"), + // (1) Null env value: + Arguments.of((String) null), + // (2) Empty env value: + Arguments.of("") + // @formatter:on + ); + } + + @ParameterizedTest + @MethodSource + void testObjectStoreCreationFallsBackToFirstReachableProviderWhenEnvIsInvalid(String envValue) { mockCfService(); - when(applicationConfiguration.getObjectStoreClientType()).thenReturn("WRONG_PROVIDER"); + when(applicationConfiguration.getObjectStoreClientType()).thenReturn(envValue); + var factoryBeanSpy = spy(objectStoreFileStorageFactoryBean); - ObjectStoreFileStorageFactoryBean spy = spy(objectStoreFileStorageFactoryBean); + factoryBeanSpy.afterPropertiesSet(); + var result = factoryBeanSpy.getObject(); - spy.afterPropertiesSet(); - FileStorage createdObjectStoreFileStorage = spy.getObject(); + assertInstanceOf(AwsS3ObjectStoreFileStorage.class, result); + verify(factoryBeanSpy).createObjectStoreFromFirstReachableProvider(anyMap(), anyList()); + } - assertNotNull(createdObjectStoreFileStorage); - verify(spy, times(1)) - .createObjectStoreFromFirstReachableProvider(anyMap(), anyList()); + @Test + void testObjectStoreCreationFallsBackToNextProviderWhenFirstFails() { + mockCfService(); + when(applicationConfiguration.getObjectStoreClientType()).thenReturn(null); + doThrow(new IllegalStateException("AWS connection failed")).when(awsS3ObjectStoreFileStorage) + .testConnection(); + + objectStoreFileStorageFactoryBean.afterPropertiesSet(); + var result = objectStoreFileStorageFactoryBean.getObject(); + + // AWS (two entries) fails, then Aliyun (JClouds default path) succeeds + assertInstanceOf(JCloudsObjectStoreFileStorage.class, result); + verify(jCloudsObjectStoreFileStorage).testConnection(); } @Test @@ -196,9 +229,7 @@ void testGcpEnvWithNoMatchingProviderInfoFallsThroughToNoValidStore() { public List getProvidersServiceInfo() { return List.of(ImmutableObjectStoreServiceInfo.builder() .provider(Constants.AWS_S_3) - .identity(ACCESS_KEY_ID_VALUE) - .credential(SECRET_ACCESS_KEY_VALUE) - .container(BUCKET_VALUE) + .credentials(buildCredentials()) .build()); } }; @@ -212,32 +243,54 @@ public List getProvidersServiceInfo() { void testObjectStoreCreationWhenEnvProviderFailsToConnect() { mockCfService(); when(applicationConfiguration.getObjectStoreClientType()).thenReturn(Constants.AWS); - doThrow(new IllegalStateException("Cannot create object store")).when(jCloudsObjectStoreFileStorage) + doThrow(new IllegalStateException("Cannot create object store")).when(awsS3ObjectStoreFileStorage) .testConnection(); - Exception exception = assertThrows(IllegalStateException.class, () -> objectStoreFileStorageFactoryBean.afterPropertiesSet()); + var exception = assertThrows(IllegalStateException.class, () -> objectStoreFileStorageFactoryBean.afterPropertiesSet()); assertEquals(Messages.NO_VALID_OBJECT_STORE_CONFIGURATION_FOUND, exception.getMessage()); } @Test - void testObjectStoreCreationWithoutValidServiceInstance() { + void testObjectStoreCreationWhenEnvProviderNotFoundInAvailableProviders() { mockCfService(); + when(applicationConfiguration.getObjectStoreClientType()).thenReturn(Constants.GCP); + var factoryBeanSpy = spy(objectStoreFileStorageFactoryBean); + doReturn(List.of(buildServiceInfo(Constants.AWS_S_3))).when(factoryBeanSpy) + .getProvidersServiceInfo(); + + var exception = assertThrows(IllegalStateException.class, factoryBeanSpy::afterPropertiesSet); + assertEquals(Messages.NO_VALID_OBJECT_STORE_CONFIGURATION_FOUND, exception.getMessage()); + } + + @Test + void testObjectStoreCreationWhenAllProvidersFail() { + mockCfService(); + doThrow(new IllegalStateException("Cannot create object store")).when(awsS3ObjectStoreFileStorage) + .testConnection(); doThrow(new IllegalStateException("Cannot create object store")).when(jCloudsObjectStoreFileStorage) .testConnection(); doThrow(new IllegalStateException("Cannot create object store")).when(gcpObjectStoreFileStorage) .testConnection(); doThrow(new IllegalStateException("Cannot create object store")).when(azureObjectStoreFileStorage) .testConnection(); - Exception exception = assertThrows(IllegalStateException.class, () -> objectStoreFileStorageFactoryBean.afterPropertiesSet()); + + var exception = assertThrows(IllegalStateException.class, () -> objectStoreFileStorageFactoryBean.afterPropertiesSet()); assertEquals(Messages.NO_VALID_OBJECT_STORE_CONFIGURATION_FOUND, exception.getMessage()); } + private ObjectStoreServiceInfo buildServiceInfo(String provider) { + return ImmutableObjectStoreServiceInfo.builder() + .provider(provider) + .credentials(buildCredentials()) + .build(); + } + private void mockCfService() { CfService cfService = Mockito.mock(CfService.class); CfCredentials cfCredentials = Mockito.mock(CfCredentials.class); when(cfCredentials.getMap()).thenReturn(buildCredentials()); when(cfService.getCredentials()).thenReturn(cfCredentials); - when(environmentServicesFinder.findService("deploy-service-os")).thenReturn(cfService); + when(environmentServicesFinder.findService(SERVICE_NAME)).thenReturn(cfService); } private static Map buildCredentials() { @@ -272,18 +325,20 @@ protected AzureObjectStoreFileStorage createAzureFileStorage(ObjectStoreServiceI return ObjectStoreFileStorageFactoryBeanTest.this.azureObjectStoreFileStorage; } + @Override + protected AwsS3ObjectStoreFileStorage createAwsS3FileStorage(ObjectStoreServiceInfo objectStoreServiceInfo) { + return ObjectStoreFileStorageFactoryBeanTest.this.awsS3ObjectStoreFileStorage; + } + @Override public List getProvidersServiceInfo() { - CfService service = environmentServicesFinder.findService("deploy-service-os"); + CfService service = environmentServicesFinder.findService(SERVICE_NAME); if (service != null) { - return new ObjectStoreServiceInfoCreatorMock().getAllProvidersServiceInfo(service.getCredentials() - .getMap()); - } else { - return List.of(); + return new ObjectStoreServiceInfoCreator().getAllProvidersServiceInfo(service.getCredentials() + .getMap()); } + return List.of(); } } - private class ObjectStoreServiceInfoCreatorMock extends ObjectStoreServiceInfoCreator { - } } diff --git a/multiapps-controller-web/src/test/java/org/cloudfoundry/multiapps/controller/web/configuration/service/ObjectStoreServiceInfoCreatorTest.java b/multiapps-controller-web/src/test/java/org/cloudfoundry/multiapps/controller/web/configuration/service/ObjectStoreServiceInfoCreatorTest.java index 346fc8698e..fc2b5533cf 100644 --- a/multiapps-controller-web/src/test/java/org/cloudfoundry/multiapps/controller/web/configuration/service/ObjectStoreServiceInfoCreatorTest.java +++ b/multiapps-controller-web/src/test/java/org/cloudfoundry/multiapps/controller/web/configuration/service/ObjectStoreServiceInfoCreatorTest.java @@ -24,7 +24,10 @@ class ObjectStoreServiceInfoCreatorTest { private static final String BUCKET_VALUE = "bucket_value"; private static final String REGION_VALUE = "region_value"; private static final String ENDPOINT_VALUE = "endpoint_value"; - private static final Map CREDENTIALS = Map.of("test", "test1"); + private static final String SAS_TOKEN_VALUE = "sas_token_value"; + private static final String BASE64_PRIVATE_KEY_VALUE = "base64EncodedPrivateKeyDataValue"; + private static final Map AZURE_CREDENTIALS = Map.of(Constants.SAS_TOKEN, SAS_TOKEN_VALUE); + private static final Map GCP_CREDENTIALS = Map.of(Constants.BASE_64_ENCODED_PRIVATE_KEY_DATA, BASE64_PRIVATE_KEY_VALUE); private ObjectStoreServiceInfoCreator objectStoreServiceInfoCreator; @@ -36,8 +39,9 @@ void setUp() { static Stream testDifferentProviders() { return Stream.of(Arguments.of(buildCfService(buildAliCloudCredentials()), buildAliCloudObjectStoreServiceInfo()), Arguments.of(buildCfService(buildAwsCredentials()), buildAwsObjectStoreServiceInfo()), - Arguments.of(buildCfService(buildSdkCredentials()), buildAzureObjectStoreServiceInfo()), - Arguments.of(buildCfService(buildSdkCredentials()), buildGcpObjectStoreServiceInfo())); + Arguments.of(buildCfService(buildCceeCredentials()), buildCceeObjectStoreServiceInfo()), + Arguments.of(buildCfService(AZURE_CREDENTIALS), buildAzureObjectStoreServiceInfo()), + Arguments.of(buildCfService(GCP_CREDENTIALS), buildGcpObjectStoreServiceInfo())); } @ParameterizedTest @@ -70,11 +74,7 @@ private static Map buildAliCloudCredentials() { private static ObjectStoreServiceInfo buildAliCloudObjectStoreServiceInfo() { return ImmutableObjectStoreServiceInfo.builder() .provider(Constants.ALIYUN_OSS) - .identity(ACCESS_KEY_ID_VALUE) - .credential(SECRET_ACCESS_KEY_VALUE) - .container(BUCKET_VALUE) - .endpoint(ENDPOINT_VALUE) - .region(REGION_VALUE) + .credentials(buildAliCloudCredentials()) .build(); } @@ -89,27 +89,41 @@ private static Map buildAwsCredentials() { private static ObjectStoreServiceInfo buildAwsObjectStoreServiceInfo() { return ImmutableObjectStoreServiceInfo.builder() .provider(Constants.AWS_S_3) - .identity(ACCESS_KEY_ID_VALUE) - .credential(SECRET_ACCESS_KEY_VALUE) - .container(BUCKET_VALUE) + .credentials(buildAwsCredentials()) .build(); } - private static Map buildSdkCredentials() { - return CREDENTIALS; + private static Map buildCceeCredentials() { + Map credentials = new HashMap<>(); + credentials.put(Constants.ACCESS_KEY_ID, ACCESS_KEY_ID_VALUE); + credentials.put(Constants.SECRET_ACCESS_KEY, SECRET_ACCESS_KEY_VALUE); + credentials.put(Constants.CONTAINER_NAME_WITH_DASH, BUCKET_VALUE); + credentials.put(Constants.ENDPOINT_URL, ENDPOINT_VALUE); + credentials.put(Constants.REGION, REGION_VALUE); + return credentials; + } + + private static ObjectStoreServiceInfo buildCceeObjectStoreServiceInfo() { + Map expectedCredentials = new HashMap<>(buildCceeCredentials()); + expectedCredentials.put(Constants.BUCKET, BUCKET_VALUE); + expectedCredentials.put(Constants.ENDPOINT, ENDPOINT_VALUE); + return ImmutableObjectStoreServiceInfo.builder() + .provider(Constants.AWS_S_3) + .credentials(expectedCredentials) + .build(); } private static ObjectStoreServiceInfo buildAzureObjectStoreServiceInfo() { return ImmutableObjectStoreServiceInfo.builder() .provider(Constants.AZUREBLOB) - .credentials(CREDENTIALS) + .credentials(AZURE_CREDENTIALS) .build(); } private static ObjectStoreServiceInfo buildGcpObjectStoreServiceInfo() { return ImmutableObjectStoreServiceInfo.builder() .provider(Constants.GOOGLE_CLOUD_STORAGE) - .credentials(CREDENTIALS) + .credentials(GCP_CREDENTIALS) .build(); } diff --git a/multiapps-controller-web/src/test/java/org/cloudfoundry/multiapps/controller/web/upload/AsyncUploadJobOrchestratorTest.java b/multiapps-controller-web/src/test/java/org/cloudfoundry/multiapps/controller/web/upload/AsyncUploadJobOrchestratorTest.java index c0533e5ac2..edd8576e9a 100644 --- a/multiapps-controller-web/src/test/java/org/cloudfoundry/multiapps/controller/web/upload/AsyncUploadJobOrchestratorTest.java +++ b/multiapps-controller-web/src/test/java/org/cloudfoundry/multiapps/controller/web/upload/AsyncUploadJobOrchestratorTest.java @@ -10,7 +10,6 @@ import org.cloudfoundry.multiapps.common.SLException; import org.cloudfoundry.multiapps.controller.api.model.UserCredentials; import org.cloudfoundry.multiapps.controller.client.util.CheckedSupplier; -import org.cloudfoundry.multiapps.controller.client.util.ResilientOperationExecutor; import org.cloudfoundry.multiapps.controller.core.helpers.DescriptorParserFacadeFactory; import org.cloudfoundry.multiapps.controller.core.util.ApplicationConfiguration; import org.cloudfoundry.multiapps.controller.persistence.model.AsyncUploadJobEntry; @@ -22,6 +21,7 @@ import org.cloudfoundry.multiapps.controller.persistence.services.FileService; import org.cloudfoundry.multiapps.controller.web.upload.client.DeployFromUrlRemoteClient; import org.cloudfoundry.multiapps.controller.web.upload.client.FileFromUrlData; +import org.cloudfoundry.multiapps.controller.web.upload.resilience.FileUploadResilientOperationExecutor; import org.cloudfoundry.multiapps.controller.web.util.SecurityContextUtil; import org.cloudfoundry.multiapps.mta.handlers.DescriptorParserFacade; import org.cloudfoundry.multiapps.mta.model.DeploymentDescriptor; @@ -445,8 +445,8 @@ protected void waitBetweenUpdates() { } @Override - protected ResilientOperationExecutor getResilientOperationExecutor() { - return new ResilientOperationExecutor() { + protected FileUploadResilientOperationExecutor getFileUploadResilientOperationExecutor() { + return new FileUploadResilientOperationExecutor() { @Override public T execute(CheckedSupplier operation) throws Exception { return operation.get(); diff --git a/multiapps-controller-web/src/test/java/org/cloudfoundry/multiapps/controller/web/upload/FileUploadResilientOperationExecutorTest.java b/multiapps-controller-web/src/test/java/org/cloudfoundry/multiapps/controller/web/upload/FileUploadResilientOperationExecutorTest.java new file mode 100644 index 0000000000..b00f9e038c --- /dev/null +++ b/multiapps-controller-web/src/test/java/org/cloudfoundry/multiapps/controller/web/upload/FileUploadResilientOperationExecutorTest.java @@ -0,0 +1,207 @@ +package org.cloudfoundry.multiapps.controller.web.upload; + +import org.cloudfoundry.multiapps.common.SLException; +import org.cloudfoundry.multiapps.controller.persistence.services.FileStorageException; +import org.cloudfoundry.multiapps.controller.web.upload.resilience.FileUploadResilientOperationExecutor; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import software.amazon.awssdk.core.exception.SdkException; + +import java.io.IOException; +import java.io.UncheckedIOException; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.atomic.AtomicInteger; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +class FileUploadResilientOperationExecutorTest { + + private List recordedSleepDurations; + private FileUploadResilientOperationExecutor executor; + + @BeforeEach + void setUp() { + recordedSleepDurations = new ArrayList<>(); + executor = new FileUploadResilientOperationExecutor() { + @Override + protected void sleep(long millis) { + recordedSleepDurations.add(millis); + } + }; + } + + @Test + void testSuccessfulFirstAttempt() throws Exception { + var result = executor.execute(() -> "success"); + + assertEquals("success", result); + assertTrue(recordedSleepDurations.isEmpty()); + } + + @Test + void testRetryOnFileStorageExceptionWithIOExceptionCause() throws Exception { + var attempts = new AtomicInteger(0); + + var result = executor.execute(() -> { + if (attempts.incrementAndGet() < 3) { + throw new FileStorageException("Storage failed", new IOException("Connection reset")); + } + return "recovered"; + }); + + assertEquals("recovered", result); + assertEquals(2, recordedSleepDurations.size()); + assertEquals(10000L, recordedSleepDurations.get(0)); + assertEquals(20000L, recordedSleepDurations.get(1)); + } + + @Test + void testNoRetryOnFileStorageExceptionWithSdkExceptionCause() { + var attempts = new AtomicInteger(0); + + assertThrows(FileStorageException.class, () -> executor.execute(() -> { + attempts.incrementAndGet(); + throw new FileStorageException("S3 error", SdkException.builder() + .message("Service unavailable") + .build()); + })); + + assertEquals(1, attempts.get()); + assertTrue(recordedSleepDurations.isEmpty()); + } + + @Test + void testRetryOnFileStorageExceptionWithSdkExceptionWrappingIOException() throws Exception { + var attempts = new AtomicInteger(0); + + var result = executor.execute(() -> { + if (attempts.incrementAndGet() < 2) { + // Mirrors what AWS SDK v2 does: SdkClientException wrapping a network IOException + var networkError = new IOException("Connection reset by peer"); + var sdkException = SdkException.builder() + .message("network error") + .cause(networkError) + .build(); + throw new FileStorageException("S3 upload failed", sdkException); + } + return "recovered"; + }); + + assertEquals("recovered", result); + assertEquals(1, recordedSleepDurations.size()); + assertEquals(10000L, recordedSleepDurations.get(0)); + } + + @Test + void testRetryOnFileStorageExceptionWithUncheckedIOExceptionCause() throws Exception { + var attempts = new AtomicInteger(0); + + var result = executor.execute(() -> { + if (attempts.incrementAndGet() < 2) { + throw new FileStorageException("Wrapped IO", new UncheckedIOException(new IOException("broken pipe"))); + } + return "recovered"; + }); + + assertEquals("recovered", result); + assertEquals(1, recordedSleepDurations.size()); + assertEquals(10000L, recordedSleepDurations.get(0)); + } + + @Test + void testRetryOnDirectIOException() throws Exception { + var attempts = new AtomicInteger(0); + + var result = executor.execute(() -> { + if (attempts.incrementAndGet() < 2) { + throw new IOException("Network failure"); + } + return "recovered"; + }); + + assertEquals("recovered", result); + assertEquals(1, recordedSleepDurations.size()); + assertEquals(10000L, recordedSleepDurations.get(0)); + } + + @Test + void testRetryOnDirectUncheckedIOException() throws Exception { + var attempts = new AtomicInteger(0); + + var result = executor.execute(() -> { + if (attempts.incrementAndGet() < 2) { + throw new UncheckedIOException(new IOException("timeout")); + } + return "recovered"; + }); + + assertEquals("recovered", result); + assertEquals(1, recordedSleepDurations.size()); + assertEquals(10000L, recordedSleepDurations.get(0)); + } + + @Test + void testNoRetryOnSLException() { + var attempts = new AtomicInteger(0); + + var exception = assertThrows(SLException.class, () -> executor.execute(() -> { + attempts.incrementAndGet(); + throw new SLException("Validation error"); + })); + + assertEquals(1, attempts.get()); + assertEquals("Validation error", exception.getMessage()); + assertTrue(recordedSleepDurations.isEmpty()); + } + + @Test + void testNoRetryOnFileStorageExceptionWithNonTransientCause() { + var attempts = new AtomicInteger(0); + + assertThrows(FileStorageException.class, () -> executor.execute(() -> { + attempts.incrementAndGet(); + throw new FileStorageException("Bad request", new IllegalArgumentException("invalid key")); + })); + + assertEquals(1, attempts.get()); + assertTrue(recordedSleepDurations.isEmpty()); + } + + @Test + void testExponentialBackoffTiming() throws Exception { + var attempts = new AtomicInteger(0); + + var result = executor.execute(() -> { + if (attempts.incrementAndGet() < 6) { + throw new IOException("transient"); + } + return "finally"; + }); + + assertEquals("finally", result); + assertEquals(5, recordedSleepDurations.size()); + assertEquals(10000L, recordedSleepDurations.get(0)); + assertEquals(20000L, recordedSleepDurations.get(1)); + assertEquals(40000L, recordedSleepDurations.get(2)); + assertEquals(80000L, recordedSleepDurations.get(3)); + assertEquals(160000L, recordedSleepDurations.get(4)); + } + + @Test + void testAllAttemptsExhausted() { + var attempts = new AtomicInteger(0); + + var exception = assertThrows(IOException.class, () -> executor.execute(() -> { + attempts.incrementAndGet(); + throw new IOException("persistent failure"); + })); + + assertEquals(7, attempts.get()); + assertEquals("persistent failure", exception.getMessage()); + assertEquals(6, recordedSleepDurations.size()); + } + +} diff --git a/pom.xml b/pom.xml index c9ff270b06..c82bedbfba 100644 --- a/pom.xml +++ b/pom.xml @@ -48,6 +48,7 @@ 2.12.1 1.16.4 3.18.5 + 2.44.12 1.1.1 1.21.0 @@ -757,12 +758,6 @@ jclouds-blobstore ${jclouds.version} - - - org.apache.jclouds.provider - aws-s3 - ${jclouds.version} - @@ -818,6 +813,18 @@ auto-service-annotations ${auto-service.version} + + + software.amazon.awssdk + s3 + ${aws.sdk.version} + + + + software.amazon.awssdk + url-connection-client + ${aws.sdk.version} + From 02194940fec3b32626cffe5da04bdbc09d19c77d Mon Sep 17 00:00:00 2001 From: IvanBorislavovDimitrov Date: Wed, 27 May 2026 17:08:05 +0300 Subject: [PATCH 2/4] Refactor testConnection to verify bucket/container existence Previously testConnection probed for a non-existent test object on the underlying store, which produced misleading results when the bucket or container itself was missing. The check now explicitly verifies that the target bucket/container exists and throws IllegalStateException with a clear OBJECT_STORE_BUCKET_NOT_FOUND message when it does not, making configuration errors fail fast with an actionable diagnostic. LMCROSSITXSADEPLOY-3315 --- .../controller/persistence/Messages.java | 1 + .../services/GcpObjectStoreFileStorage.java | 5 +++- .../JCloudsObjectStoreFileStorage.java | 4 ++- .../GcpObjectStoreFileStorageTest.java | 30 +++++++++++++++++++ .../JCloudsObjectStoreFileStorageTest.java | 24 +++++++++++++++ 5 files changed, 62 insertions(+), 2 deletions(-) diff --git a/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/Messages.java b/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/Messages.java index 762895d73d..81ae57da66 100644 --- a/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/Messages.java +++ b/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/Messages.java @@ -48,6 +48,7 @@ public final class Messages { public static final String APPLICATION_SHUTDOWN_WITH_APPLICATION_INSTANCE_ID_ALREADY_EXIST = "Application shutdown application instance ID \"{0}\" already exist"; public static final String SECRET_TOKEN_WITH_ID_NOT_EXIST = "Secret token with ID \"{0}\" does not exist"; public static final String DATABASE_HEALTH_CHECK_FAILED = "Database health check failed"; + public static final String OBJECT_STORE_BUCKET_NOT_FOUND = "Object store bucket \"{0}\" not found"; public static final String UPLOAD_OF_FILE_WITH_NAMESPACE_FAILED = "Upload of file: \"{0}\" with namespace: \"{1}\" failed"; // ERROR log messages: diff --git a/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/GcpObjectStoreFileStorage.java b/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/GcpObjectStoreFileStorage.java index 24982d2a8f..511ad1fbfc 100644 --- a/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/GcpObjectStoreFileStorage.java +++ b/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/GcpObjectStoreFileStorage.java @@ -192,7 +192,10 @@ public InputStream openInputStream(String space, String id) throws FileStorageEx @Override public void testConnection() { - storage.get(bucketName, "test"); + var bucket = storage.get(bucketName); + if (bucket == null) { + throw new IllegalStateException(MessageFormat.format(Messages.OBJECT_STORE_BUCKET_NOT_FOUND, bucketName)); + } } @Override diff --git a/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/JCloudsObjectStoreFileStorage.java b/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/JCloudsObjectStoreFileStorage.java index 25aa0fb28c..21f748e49e 100644 --- a/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/JCloudsObjectStoreFileStorage.java +++ b/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/JCloudsObjectStoreFileStorage.java @@ -177,7 +177,9 @@ private InputStream openPayloadInputStream(Payload payload) throws FileStorageEx @Override public void testConnection() { - blobStore.blobExists(container, "test"); + if (!blobStore.containerExists(container)) { + throw new IllegalStateException(MessageFormat.format(Messages.OBJECT_STORE_BUCKET_NOT_FOUND, container)); + } } @Override diff --git a/multiapps-controller-persistence/src/test/java/org/cloudfoundry/multiapps/controller/persistence/services/GcpObjectStoreFileStorageTest.java b/multiapps-controller-persistence/src/test/java/org/cloudfoundry/multiapps/controller/persistence/services/GcpObjectStoreFileStorageTest.java index 3d2cfeeb19..14264ffa72 100644 --- a/multiapps-controller-persistence/src/test/java/org/cloudfoundry/multiapps/controller/persistence/services/GcpObjectStoreFileStorageTest.java +++ b/multiapps-controller-persistence/src/test/java/org/cloudfoundry/multiapps/controller/persistence/services/GcpObjectStoreFileStorageTest.java @@ -3,6 +3,7 @@ import com.google.cloud.storage.Blob; import com.google.cloud.storage.BlobId; import com.google.cloud.storage.BlobInfo; +import com.google.cloud.storage.Bucket; import com.google.cloud.storage.Storage; import com.google.cloud.storage.contrib.nio.testing.LocalStorageHelper; import org.cloudfoundry.multiapps.controller.persistence.model.FileEntry; @@ -20,8 +21,10 @@ import java.util.Map; import java.util.UUID; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.anyList; import static org.mockito.Mockito.mock; @@ -151,6 +154,33 @@ void getExistingFileEntriesPassesCorrectBlobIdsToStorage() { verify(mockedStorage).get(List.of(BlobId.of(CONTAINER, entry.getId()))); } + @Override + @Test + void testConnection() { + Bucket mockBucket = mock(Bucket.class); + when(mockedStorage.get(CONTAINER)).thenReturn(mockBucket); + assertDoesNotThrow(mockedGcpFileStorage::testConnection); + } + + @Test + void testTestConnectionWhenBucketExists() { + Bucket mockBucket = mock(Bucket.class); + when(mockedStorage.get(CONTAINER)).thenReturn(mockBucket); + + assertDoesNotThrow(mockedGcpFileStorage::testConnection); + verify(mockedStorage).get(CONTAINER); + } + + @Test + void testTestConnectionWhenBucketDoesNotExist() { + when(mockedStorage.get(CONTAINER)).thenReturn(null); + + var exception = assertThrows(IllegalStateException.class, mockedGcpFileStorage::testConnection); + assertTrue(exception.getMessage() + .contains(CONTAINER)); + verify(mockedStorage).get(CONTAINER); + } + private void mockStorageGetToReturn(List blobs) { when(mockedStorage.get(anyList())).thenReturn(blobs); } diff --git a/multiapps-controller-persistence/src/test/java/org/cloudfoundry/multiapps/controller/persistence/services/JCloudsObjectStoreFileStorageTest.java b/multiapps-controller-persistence/src/test/java/org/cloudfoundry/multiapps/controller/persistence/services/JCloudsObjectStoreFileStorageTest.java index 0addf3485f..11bcddac57 100644 --- a/multiapps-controller-persistence/src/test/java/org/cloudfoundry/multiapps/controller/persistence/services/JCloudsObjectStoreFileStorageTest.java +++ b/multiapps-controller-persistence/src/test/java/org/cloudfoundry/multiapps/controller/persistence/services/JCloudsObjectStoreFileStorageTest.java @@ -230,6 +230,30 @@ void testConnection() { assertDoesNotThrow(() -> fileStorage.testConnection()); } + @Test + void testTestConnectionWhenContainerExists() { + BlobStore mockBlobStore = mock(BlobStore.class); + JCloudsObjectStoreFileStorage testFileStorage = new JCloudsObjectStoreFileStorage(mockBlobStore, CONTAINER); + + when(mockBlobStore.containerExists(CONTAINER)).thenReturn(true); + + assertDoesNotThrow(testFileStorage::testConnection); + verify(mockBlobStore).containerExists(CONTAINER); + } + + @Test + void testTestConnectionWhenContainerDoesNotExist() { + BlobStore mockBlobStore = mock(BlobStore.class); + JCloudsObjectStoreFileStorage testFileStorage = new JCloudsObjectStoreFileStorage(mockBlobStore, CONTAINER); + + when(mockBlobStore.containerExists(CONTAINER)).thenReturn(false); + + var exception = assertThrows(IllegalStateException.class, testFileStorage::testConnection); + assertTrue(exception.getMessage() + .contains(CONTAINER)); + verify(mockBlobStore).containerExists(CONTAINER); + } + @Test void testDeleteFilesByIds() throws Exception { FileEntry fileEntry = addFile(TEST_FILE_LOCATION); From d3c636c2e4a3d127dc5c7091c73f8e915044aaac Mon Sep 17 00:00:00 2001 From: IvanBorislavovDimitrov Date: Fri, 5 Jun 2026 11:46:11 +0300 Subject: [PATCH 3/4] Fix comments --- .../src/main/java/module-info.java | 2 - .../controller/persistence/Messages.java | 3 +- .../model/AsyncUploadJobEntry.java | 8 ++ .../services/AwsS3ObjectStoreFileStorage.java | 56 ++++++----- .../persistence/services/FileStorage.java | 1 - .../services/GcpObjectStoreFileStorage.java | 32 +++---- .../JCloudsObjectStoreFileStorage.java | 1 + .../util/ObjectStoreConstants.java | 1 + .../AwsS3ObjectStoreFileStorageTest.java | 33 ++++--- .../GcpObjectStoreFileStorageTest.java | 8 -- .../JCloudsObjectStoreFileStorageTest.java | 32 +++---- .../multiapps/controller/web/Messages.java | 15 ++- .../web/api/impl/FilesApiServiceImpl.java | 46 +++++---- .../ObjectStoreFileStorageFactoryBean.java | 31 +++--- .../ObjectStoreServiceInfoCreator.java | 3 +- .../upload/AsyncUploadJobOrchestrator.java | 31 +++--- .../FileUploadResilientOperationExecutor.java | 41 ++++++-- .../web/api/impl/FilesApiServiceImplTest.java | 73 ++++++-------- .../AsyncUploadJobOrchestratorTest.java | 27 +++--- ...eUploadResilientOperationExecutorTest.java | 94 ++++++++++++++++++- pom.xml | 2 +- 21 files changed, 331 insertions(+), 209 deletions(-) diff --git a/multiapps-controller-persistence/src/main/java/module-info.java b/multiapps-controller-persistence/src/main/java/module-info.java index 8b7da05942..ca1b6e20a1 100644 --- a/multiapps-controller-persistence/src/main/java/module-info.java +++ b/multiapps-controller-persistence/src/main/java/module-info.java @@ -65,9 +65,7 @@ requires spring.core; requires software.amazon.awssdk.services.s3; requires software.amazon.awssdk.core; - requires software.amazon.awssdk.awscore; requires software.amazon.awssdk.regions; - requires software.amazon.awssdk.utils; requires software.amazon.awssdk.auth; requires software.amazon.awssdk.http; requires software.amazon.awssdk.http.urlconnection; diff --git a/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/Messages.java b/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/Messages.java index 81ae57da66..145bdb521f 100644 --- a/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/Messages.java +++ b/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/Messages.java @@ -71,7 +71,6 @@ public final class Messages { // DEBUG log messages: public static final String STORED_FILE_0 = "Stored file: \"{0}\""; public static final String STORED_FILE_0_WITH_SIZE_1 = "Stored file \"{0}\" with size {1}"; - public static final String STORED_FILE_0_WITH_SIZE_1_LOG = "Stored file \"{}\" with size {}"; public static final String DELETED_0_FILES_WITH_SPACEIDS_1 = "Deleted {0} files with space ids \"{1}\"."; public static final String DELETED_0_FILES_WITH_SPACE_1_AND_NAMESPACE_2 = "Deleted {0} files with space \"{1}\" and namespace \"{2}\"."; public static final String DELETED_0_FILES_MODIFIED_BEFORE_1 = "Deleted {0} files modified before \"{1}\"."; @@ -82,7 +81,7 @@ public final class Messages { public static final String RETRIEVED_SECRET_TOKEN_WITH_ID_0_FOR_PROCESS_WITH_ID_1 = "Retrieved secret token with id \"{0}\" for process with id \"{1}\""; public static final String DELETED_0_SECRET_TOKENS_FOR_PROCESS_WITH_ID_1 = "Deleted \"{0}\" secret tokens for process with id \"{1}\""; public static final String DELETED_0_SECRET_TOKENS_WITH_EXPIRATION_DATE_1 = "Deleted secret tokens \"{0}\" with an expiration date \"{1}\""; - public static final String FAILED_TO_DELETE_FILE_0_IN_OBJECT_STORE_REASON_1 = "Failed to delete file \"{}\" in object store. Reason: {}"; + public static final String FAILED_TO_DELETE_FILE_0_IN_OBJECT_STORE_REASON_1 = "Failed to delete file \"{0}\" in object store. Reason: {1}"; public static final String S3_UPLOAD_FAILED_FILE_0_SIZE_1 = "S3 upload failed for file \"{0}\" (size={1}). Root cause chain: {2}"; protected Messages() { diff --git a/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/model/AsyncUploadJobEntry.java b/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/model/AsyncUploadJobEntry.java index e28d8344a3..a74c152602 100644 --- a/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/model/AsyncUploadJobEntry.java +++ b/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/model/AsyncUploadJobEntry.java @@ -1,5 +1,6 @@ package org.cloudfoundry.multiapps.controller.persistence.model; +import java.text.MessageFormat; import java.time.LocalDateTime; import org.cloudfoundry.multiapps.common.Nullable; @@ -8,6 +9,8 @@ @Value.Immutable public interface AsyncUploadJobEntry { + String STALE_JOB_DETAILS_FORMAT = "Stale job details - id: {0}, state: {1}, updatedAt: {2}, addedAt: {3}, startedAt: {4}, bytesRead: {5}, url: {6}, space: {7}, namespace: {8}, user: {9}, instance: {10}"; + enum State { INITIAL, RUNNING, FINISHED, ERROR } @@ -53,4 +56,9 @@ enum State { @Nullable LocalDateTime getUpdatedAt(); + + default String buildStaleDetailsLogMessage() { + return MessageFormat.format(STALE_JOB_DETAILS_FORMAT, getId(), getState(), getUpdatedAt(), getAddedAt(), getStartedAt(), + getBytesRead(), getUrl(), getSpaceGuid(), getNamespace(), getUser(), getInstanceIndex()); + } } diff --git a/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/AwsS3ObjectStoreFileStorage.java b/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/AwsS3ObjectStoreFileStorage.java index 9a19456d5a..f0e5415cb3 100644 --- a/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/AwsS3ObjectStoreFileStorage.java +++ b/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/AwsS3ObjectStoreFileStorage.java @@ -1,5 +1,21 @@ package org.cloudfoundry.multiapps.controller.persistence.services; +import java.io.BufferedInputStream; +import java.io.InputStream; +import java.net.URI; +import java.text.MessageFormat; +import java.time.LocalDateTime; +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Set; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.function.BiPredicate; +import java.util.function.Consumer; import org.cloudfoundry.multiapps.controller.persistence.Messages; import org.cloudfoundry.multiapps.controller.persistence.model.FileEntry; import org.cloudfoundry.multiapps.controller.persistence.util.ObjectStoreConstants; @@ -33,23 +49,6 @@ import software.amazon.awssdk.services.s3.model.S3Error; import software.amazon.awssdk.services.s3.model.S3Object; -import java.io.BufferedInputStream; -import java.io.InputStream; -import java.net.URI; -import java.text.MessageFormat; -import java.time.LocalDateTime; -import java.util.ArrayList; -import java.util.HashSet; -import java.util.List; -import java.util.Map; -import java.util.Objects; -import java.util.Set; -import java.util.concurrent.CompletableFuture; -import java.util.concurrent.ExecutorService; -import java.util.concurrent.Executors; -import java.util.function.BiPredicate; -import java.util.function.Consumer; - public class AwsS3ObjectStoreFileStorage extends ObjectStoreFileStorage { private static final Logger LOGGER = LoggerFactory.getLogger(AwsS3ObjectStoreFileStorage.class); @@ -75,11 +74,19 @@ protected S3Client createS3Client(Map credentials) { ObjectStoreConstants.AWS_OBJECT_STORE_SOCKET_TIMEOUT_CONFIG_IN_MINUTES) .connectionTimeout( ObjectStoreConstants.AWS_OBJECT_STORE_CONNECTION_TIMEOUT_CONFIG_IN_SECONDS)); - builder.endpointOverride(URI.create(HTTPS_SCHEME + credentials.get(CredentialKeys.HOST))); + builder.endpointOverride(URI.create(resolveEndpoint(credentials))); builder.region(software.amazon.awssdk.regions.Region.of((String) credentials.get(CredentialKeys.REGION))); return builder.build(); } + private String resolveEndpoint(Map credentials) { + String endpoint = (String) credentials.get(CredentialKeys.ENDPOINT); + if (endpoint != null) { + return endpoint; + } + return HTTPS_SCHEME + credentials.get(CredentialKeys.HOST); + } + protected ClientOverrideConfiguration buildClientOverrideConfig() { StandardRetryStrategy retryStrategy = StandardRetryStrategy.builder() .maxAttempts(ObjectStoreConstants.OBJECT_STORE_MAX_ATTEMPTS_CONFIG) @@ -106,9 +113,8 @@ public void addFile(FileEntry fileEntry, InputStream content) throws FileStorage .metadata(ObjectStoreMapper.createFileEntryMetadata(fileEntry)) .build(); try { - InputStream markableContent = content.markSupported() ? content : new BufferedInputStream(content); - s3Client.putObject(request, RequestBody.fromInputStream(markableContent, fileSize)); - LOGGER.debug(Messages.STORED_FILE_0_WITH_SIZE_1_LOG, fileEntry.getId(), fileSize); + s3Client.putObject(request, RequestBody.fromInputStream(new BufferedInputStream(content), fileSize)); + LOGGER.debug(MessageFormat.format(Messages.STORED_FILE_0_WITH_SIZE_1, fileEntry.getId(), fileSize)); } catch (Exception e) { LOGGER.error(MessageFormat.format(Messages.S3_UPLOAD_FAILED_FILE_0_SIZE_1, fileEntry.getName(), fileSize, e)); throw new FileStorageException(MessageFormat.format(Messages.UPLOAD_OF_FILE_WITH_NAMESPACE_FAILED, fileEntry.getName(), @@ -230,7 +236,7 @@ private void batchDelete(List keys) { .delete(d -> d.objects(identifiers)) .build()); for (S3Error error : response.errors()) { - LOGGER.warn(Messages.FAILED_TO_DELETE_FILE_0_IN_OBJECT_STORE_REASON_1, error.key(), error.message()); + LOGGER.warn(MessageFormat.format(Messages.FAILED_TO_DELETE_FILE_0_IN_OBJECT_STORE_REASON_1, error.key(), error.message())); } } } @@ -290,8 +296,9 @@ public T processArchiveEntryContent(FileContentToProcess fileContentToProces GetObjectRequest request = GetObjectRequest.builder() .bucket(bucketName) .key(id) - .range("bytes=" + fileContentToProcess.getStartOffset() + "-" - + fileContentToProcess.getEndOffset()) + .range(String.format(ObjectStoreConstants.BYTES_RANGE_FORMAT, + fileContentToProcess.getStartOffset(), + fileContentToProcess.getEndOffset())) .build(); try (ResponseInputStream stream = getObjectStream(request, id, space)) { return fileContentProcessor.process(stream); @@ -311,6 +318,7 @@ private static final class CredentialKeys { static final String SECRET_ACCESS_KEY = "secret_access_key"; static final String BUCKET = "bucket"; static final String HOST = "host"; + static final String ENDPOINT = "endpoint"; static final String REGION = "region"; } } diff --git a/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/FileStorage.java b/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/FileStorage.java index c84abf119f..67098c18d0 100644 --- a/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/FileStorage.java +++ b/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/FileStorage.java @@ -10,7 +10,6 @@ public interface FileStorage { void addFile(FileEntry fileEntry, InputStream content) throws FileStorageException; - @Deprecated List getFileEntriesWithoutContent(List fileEntries) throws FileStorageException; List getExistingFileEntries(List fileEntries) throws FileStorageException; diff --git a/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/GcpObjectStoreFileStorage.java b/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/GcpObjectStoreFileStorage.java index 511ad1fbfc..7e1dafaa61 100644 --- a/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/GcpObjectStoreFileStorage.java +++ b/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/GcpObjectStoreFileStorage.java @@ -1,5 +1,19 @@ package org.cloudfoundry.multiapps.controller.persistence.services; +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.io.InputStream; +import java.nio.channels.Channels; +import java.text.MessageFormat; +import java.time.LocalDateTime; +import java.util.ArrayList; +import java.util.Base64; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Set; +import java.util.function.Predicate; +import java.util.stream.Collectors; import com.google.api.gax.retrying.RetrySettings; import com.google.auth.Credentials; import com.google.auth.oauth2.GoogleCredentials; @@ -7,6 +21,7 @@ import com.google.cloud.storage.Blob; import com.google.cloud.storage.BlobId; import com.google.cloud.storage.BlobInfo; +import com.google.cloud.storage.Bucket; import com.google.cloud.storage.Storage; import com.google.cloud.storage.StorageException; import com.google.cloud.storage.StorageOptions; @@ -18,21 +33,6 @@ import org.cloudfoundry.multiapps.controller.persistence.util.ObjectStoreMapper; import org.springframework.http.MediaType; -import java.io.ByteArrayInputStream; -import java.io.IOException; -import java.io.InputStream; -import java.nio.channels.Channels; -import java.text.MessageFormat; -import java.time.LocalDateTime; -import java.util.ArrayList; -import java.util.Base64; -import java.util.List; -import java.util.Map; -import java.util.Objects; -import java.util.Set; -import java.util.function.Predicate; -import java.util.stream.Collectors; - public class GcpObjectStoreFileStorage implements FileStorage { private final String bucketName; @@ -192,7 +192,7 @@ public InputStream openInputStream(String space, String id) throws FileStorageEx @Override public void testConnection() { - var bucket = storage.get(bucketName); + Bucket bucket = storage.get(bucketName); if (bucket == null) { throw new IllegalStateException(MessageFormat.format(Messages.OBJECT_STORE_BUCKET_NOT_FOUND, bucketName)); } diff --git a/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/JCloudsObjectStoreFileStorage.java b/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/JCloudsObjectStoreFileStorage.java index 21f748e49e..d239875573 100644 --- a/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/JCloudsObjectStoreFileStorage.java +++ b/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/JCloudsObjectStoreFileStorage.java @@ -67,6 +67,7 @@ public void addFile(FileEntry fileEntry, InputStream content) throws FileStorage // BlobStore::list may not return a complete list, making this method unreliable @Override + @Deprecated public List getFileEntriesWithoutContent(List fileEntries) { Set existingFiles = getAllEntries(new ListContainerOptions()).stream() .map(StorageMetadata::getName) diff --git a/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/util/ObjectStoreConstants.java b/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/util/ObjectStoreConstants.java index 2316faf240..c5c1d2fb22 100644 --- a/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/util/ObjectStoreConstants.java +++ b/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/util/ObjectStoreConstants.java @@ -21,4 +21,5 @@ private ObjectStoreConstants() { public static final Duration AWS_OBJECT_STORE_TOTAL_TIMEOUT_CONFIG_IN_MINUTES = Duration.ofMinutes(30); public static final Duration AWS_OBJECT_STORE_SOCKET_TIMEOUT_CONFIG_IN_MINUTES = Duration.ofMinutes(10); public static final Duration AWS_OBJECT_STORE_CONNECTION_TIMEOUT_CONFIG_IN_SECONDS = Duration.ofSeconds(10); + public static final String BYTES_RANGE_FORMAT = "bytes=%d-%d"; } diff --git a/multiapps-controller-persistence/src/test/java/org/cloudfoundry/multiapps/controller/persistence/services/AwsS3ObjectStoreFileStorageTest.java b/multiapps-controller-persistence/src/test/java/org/cloudfoundry/multiapps/controller/persistence/services/AwsS3ObjectStoreFileStorageTest.java index 02bae97ce4..b5b5a08011 100644 --- a/multiapps-controller-persistence/src/test/java/org/cloudfoundry/multiapps/controller/persistence/services/AwsS3ObjectStoreFileStorageTest.java +++ b/multiapps-controller-persistence/src/test/java/org/cloudfoundry/multiapps/controller/persistence/services/AwsS3ObjectStoreFileStorageTest.java @@ -31,6 +31,7 @@ import java.io.InputStream; import java.math.BigInteger; import java.time.LocalDateTime; +import java.time.Month; import java.time.ZoneId; import java.util.Arrays; import java.util.List; @@ -51,6 +52,8 @@ class AwsS3ObjectStoreFileStorageTest { + private static final LocalDateTime FILE_TIMESTAMP = LocalDateTime.of(2025, Month.JANUARY, 1, 0, 0); + private static final String BUCKET_NAME = "test-bucket"; @Mock @@ -171,8 +174,8 @@ void testDeleteFile() { @Test void testDeleteFilesBySpaceIdsWithAllMatchingItems() { setupListObjectsWithKeys(TEST_ID, TEST_ID_2); - setupHeadObjectWithMetadata(TEST_ID, TEST_SPACE_ID, NAMESPACE, LocalDateTime.now()); - setupHeadObjectWithMetadata(TEST_ID_2, TEST_SPACE_ID_2, NAMESPACE_2, LocalDateTime.now()); + setupHeadObjectWithMetadata(TEST_ID, TEST_SPACE_ID, NAMESPACE, FILE_TIMESTAMP); + setupHeadObjectWithMetadata(TEST_ID_2, TEST_SPACE_ID_2, NAMESPACE_2, FILE_TIMESTAMP); fileStorage.deleteFilesBySpaceIds(List.of(TEST_SPACE_ID, TEST_SPACE_ID_2)); @@ -182,8 +185,8 @@ void testDeleteFilesBySpaceIdsWithAllMatchingItems() { @Test void testDeleteFilesBySpaceIdsWithOneMatchingItem() { setupListObjectsWithKeys(TEST_ID, TEST_ID_2); - setupHeadObjectWithMetadata(TEST_ID, TEST_SPACE_ID, NAMESPACE, LocalDateTime.now()); - setupHeadObjectWithMetadata(TEST_ID_2, TEST_SPACE_ID_2, NAMESPACE_2, LocalDateTime.now()); + setupHeadObjectWithMetadata(TEST_ID, TEST_SPACE_ID, NAMESPACE, FILE_TIMESTAMP); + setupHeadObjectWithMetadata(TEST_ID_2, TEST_SPACE_ID_2, NAMESPACE_2, FILE_TIMESTAMP); fileStorage.deleteFilesBySpaceIds(List.of(TEST_SPACE_ID)); @@ -198,7 +201,7 @@ void testDeleteFilesBySpaceIdsWithOneMatchingItem() { @Test void testDeleteFilesBySpaceIdsWithoutMatchingItem() { setupListObjectsWithKeys(TEST_ID); - setupHeadObjectWithMetadata(TEST_ID, TEST_SPACE_ID, NAMESPACE, LocalDateTime.now()); + setupHeadObjectWithMetadata(TEST_ID, TEST_SPACE_ID, NAMESPACE, FILE_TIMESTAMP); fileStorage.deleteFilesBySpaceIds(List.of("non-existing-space")); @@ -208,8 +211,8 @@ void testDeleteFilesBySpaceIdsWithoutMatchingItem() { @Test void testDeleteFilesBySpaceAndNamespaceWithOneMatch() { setupListObjectsWithKeys(TEST_ID, TEST_ID_2); - setupHeadObjectWithMetadata(TEST_ID, TEST_SPACE_ID, NAMESPACE, LocalDateTime.now()); - setupHeadObjectWithMetadata(TEST_ID_2, TEST_SPACE_ID_2, NAMESPACE_2, LocalDateTime.now()); + setupHeadObjectWithMetadata(TEST_ID, TEST_SPACE_ID, NAMESPACE, FILE_TIMESTAMP); + setupHeadObjectWithMetadata(TEST_ID_2, TEST_SPACE_ID_2, NAMESPACE_2, FILE_TIMESTAMP); fileStorage.deleteFilesBySpaceAndNamespace(TEST_SPACE_ID, NAMESPACE); @@ -228,14 +231,14 @@ void testDeleteFilesBySpaceAndNamespaceWithOneMatch() { @Test void testDeleteFilesModifiedBefore() { - LocalDateTime oldModified = LocalDateTime.now() + LocalDateTime oldModified = FILE_TIMESTAMP .minusMinutes(15); setupListObjectsWithKeys(TEST_ID, TEST_ID_2); setupHeadObjectWithMetadata(TEST_ID, TEST_SPACE_ID, NAMESPACE, oldModified); setupHeadObjectWithMetadata(TEST_ID_2, TEST_SPACE_ID_2, NAMESPACE_2, oldModified); - int deletedCount = fileStorage.deleteFilesModifiedBefore(LocalDateTime.now()); + int deletedCount = fileStorage.deleteFilesModifiedBefore(FILE_TIMESTAMP); assertEquals(2, deletedCount); verify(s3Client).deleteObjects(any(DeleteObjectsRequest.class)); @@ -244,9 +247,9 @@ void testDeleteFilesModifiedBefore() { @Test void testDeleteFilesModifiedBeforeWithNoOldFiles() { setupListObjectsWithKeys(TEST_ID); - setupHeadObjectWithMetadata(TEST_ID, TEST_SPACE_ID, NAMESPACE, LocalDateTime.now()); + setupHeadObjectWithMetadata(TEST_ID, TEST_SPACE_ID, NAMESPACE, FILE_TIMESTAMP); - LocalDateTime cutoff = LocalDateTime.now() + LocalDateTime cutoff = FILE_TIMESTAMP .minusDays(1); int deletedCount = fileStorage.deleteFilesModifiedBefore(cutoff); @@ -478,9 +481,9 @@ void testDeleteFilesBySpaceIdsWithPagination() { when(s3Client.listObjectsV2(any(ListObjectsV2Request.class))).thenReturn(firstPage, secondPage); when(s3Client.deleteObjects(any(DeleteObjectsRequest.class))).thenReturn(DeleteObjectsResponse.builder() .build()); - setupHeadObjectWithMetadata(TEST_ID, TEST_SPACE_ID, NAMESPACE, LocalDateTime.now()); - setupHeadObjectWithMetadata(TEST_ID_2, TEST_SPACE_ID, NAMESPACE, LocalDateTime.now()); - setupHeadObjectWithMetadata(thirdId, TEST_SPACE_ID_2, NAMESPACE_2, LocalDateTime.now()); + setupHeadObjectWithMetadata(TEST_ID, TEST_SPACE_ID, NAMESPACE, FILE_TIMESTAMP); + setupHeadObjectWithMetadata(TEST_ID_2, TEST_SPACE_ID, NAMESPACE, FILE_TIMESTAMP); + setupHeadObjectWithMetadata(thirdId, TEST_SPACE_ID_2, NAMESPACE_2, FILE_TIMESTAMP); fileStorage.deleteFilesBySpaceIds(List.of(TEST_SPACE_ID)); @@ -599,7 +602,7 @@ private static FileEntry createFileEntry(String space, String id) { return ImmutableFileEntry.builder() .space(space) .size(BigInteger.TEN) - .modified(LocalDateTime.now()) + .modified(FILE_TIMESTAMP) .id(id) .build(); } diff --git a/multiapps-controller-persistence/src/test/java/org/cloudfoundry/multiapps/controller/persistence/services/GcpObjectStoreFileStorageTest.java b/multiapps-controller-persistence/src/test/java/org/cloudfoundry/multiapps/controller/persistence/services/GcpObjectStoreFileStorageTest.java index 14264ffa72..1a0042cbc4 100644 --- a/multiapps-controller-persistence/src/test/java/org/cloudfoundry/multiapps/controller/persistence/services/GcpObjectStoreFileStorageTest.java +++ b/multiapps-controller-persistence/src/test/java/org/cloudfoundry/multiapps/controller/persistence/services/GcpObjectStoreFileStorageTest.java @@ -160,14 +160,6 @@ void testConnection() { Bucket mockBucket = mock(Bucket.class); when(mockedStorage.get(CONTAINER)).thenReturn(mockBucket); assertDoesNotThrow(mockedGcpFileStorage::testConnection); - } - - @Test - void testTestConnectionWhenBucketExists() { - Bucket mockBucket = mock(Bucket.class); - when(mockedStorage.get(CONTAINER)).thenReturn(mockBucket); - - assertDoesNotThrow(mockedGcpFileStorage::testConnection); verify(mockedStorage).get(CONTAINER); } diff --git a/multiapps-controller-persistence/src/test/java/org/cloudfoundry/multiapps/controller/persistence/services/JCloudsObjectStoreFileStorageTest.java b/multiapps-controller-persistence/src/test/java/org/cloudfoundry/multiapps/controller/persistence/services/JCloudsObjectStoreFileStorageTest.java index 11bcddac57..896cf90ee1 100644 --- a/multiapps-controller-persistence/src/test/java/org/cloudfoundry/multiapps/controller/persistence/services/JCloudsObjectStoreFileStorageTest.java +++ b/multiapps-controller-persistence/src/test/java/org/cloudfoundry/multiapps/controller/persistence/services/JCloudsObjectStoreFileStorageTest.java @@ -1,18 +1,5 @@ package org.cloudfoundry.multiapps.controller.persistence.services; -import jakarta.xml.bind.DatatypeConverter; -import org.cloudfoundry.multiapps.common.util.DigestHelper; -import org.cloudfoundry.multiapps.controller.persistence.model.FileEntry; -import org.cloudfoundry.multiapps.controller.persistence.model.ImmutableFileEntry; -import org.jclouds.ContextBuilder; -import org.jclouds.blobstore.BlobStore; -import org.jclouds.blobstore.BlobStoreContext; -import org.jclouds.blobstore.domain.Blob; -import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; -import org.springframework.http.MediaType; - import java.io.ByteArrayInputStream; import java.io.IOException; import java.io.InputStream; @@ -24,11 +11,23 @@ import java.security.NoSuchAlgorithmException; import java.time.Instant; import java.time.LocalDateTime; +import java.time.Month; import java.time.ZoneId; import java.util.ArrayList; import java.util.List; import java.util.UUID; - +import jakarta.xml.bind.DatatypeConverter; +import org.cloudfoundry.multiapps.common.util.DigestHelper; +import org.cloudfoundry.multiapps.controller.persistence.model.FileEntry; +import org.cloudfoundry.multiapps.controller.persistence.model.ImmutableFileEntry; +import org.jclouds.ContextBuilder; +import org.jclouds.blobstore.BlobStore; +import org.jclouds.blobstore.BlobStoreContext; +import org.jclouds.blobstore.domain.Blob; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.springframework.http.MediaType; import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; @@ -49,6 +48,7 @@ class JCloudsObjectStoreFileStorageTest { protected static final String SECOND_FILE_TEST_LOCATION = "src/test/resources/pexels-photo-463467.jpeg"; protected static final String DIGEST_METHOD = "MD5"; protected static final String CONTAINER = "container4e"; + private static final LocalDateTime FAR_FUTURE = LocalDateTime.of(9999, Month.JANUARY, 1, 0, 0); protected String spaceId; protected String namespace; @@ -366,7 +366,7 @@ private FileEntry createFileEntry(String entryName, byte[] content) { .toString()) .space(spaceId) .size(BigInteger.valueOf(content.length)) - .modified(LocalDateTime.now()) + .modified(FAR_FUTURE) .name(entryName) .build(); } @@ -386,7 +386,7 @@ private FileEntry enrichFileEntry(FileEntry fileEntry, Path path, LocalDateTime return ImmutableFileEntry.builder() .from(fileEntry) .size(bigInteger) - .modified(date != null ? date : LocalDateTime.now()) + .modified(date != null ? date : FAR_FUTURE) .name(path.getFileName() .toString()) .build(); diff --git a/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/Messages.java b/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/Messages.java index 10701c71d9..10156b8082 100644 --- a/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/Messages.java +++ b/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/Messages.java @@ -18,7 +18,7 @@ public final class Messages { public static final String ERROR_FROM_REMOTE_MTAR_ENDPOINT_FOR_JOB_WITH_ID = "Error from remote MTAR endpoint {0} with status code {1}, message: {2}. Job id: {3}"; public static final String MTAR_ENDPOINT_NOT_SECURE_FOR_JOB_WITH_ID = "Remote MTAR endpoint is not a secure connection. HTTPS required. Job id: {0}"; public static final String REQUEST_0_1_FAILED_WITH_2 = "Request \"{0} {1}\" failed with \"{2}\""; - public static final String ERROR_OCCURRED_WHILE_DELETING_JOB_ENTRY = "Error occurred while deleting job entry with id: {}"; + public static final String ERROR_OCCURRED_WHILE_DELETING_JOB_ENTRY = "Error occurred while deleting job entry with id: {0}"; public static final String CANNOT_CREATE_OBJECT_STORE_CLIENT_WITH_PROVIDER_0 = "Cannot create Object Store client with provider: {0}"; public static final String NO_VALID_OBJECT_STORE_CONFIGURATION_FOUND = "No valid Object Store configuration found!"; public static final String MISSING_PROPERTIES_FOR_CREATING_THE_SPECIFIC_PROVIDER = "Missing properties for creating the specific provider!"; @@ -59,7 +59,7 @@ public final class Messages { // WARN log messages public static final String NO_OBJECTSTORE_PROVIDER_FOUND_FOR_0 = "No ObjectStore provider found for {0}!"; - public static final String NO_OBJECT_STORE_PROVIDERS_DETECTED = "No object store providers detected from credentials. Service name: {}"; + public static final String NO_OBJECT_STORE_PROVIDERS_DETECTED_FOR_SERVICE_0 = "No object store providers detected from credentials. Service name: {0}"; public static final String FILE_UPLOAD_ATTEMPT_FAILED = "Upload attempt {0}/{1} failed. Retrying in {2} ms. Cause: {3}"; public static final String FILE_UPLOAD_ALL_ATTEMPTS_EXHAUSTED = "All {0} upload attempts exhausted. Last error: {1}"; @@ -72,12 +72,11 @@ public final class Messages { public static final String OBJECTSTORE_FOR_BINARIES_STORAGE = "Objectstore will be used for binaries storage"; public static final String CLEARING_LOCK_OWNER = "Clearing lock owner {0}..."; public static final String CLEARED_LOCK_OWNER = "Cleared lock owner {0}"; - public static final String OBJECT_STORE_PROVIDERS_DETECTED = "Object store providers detected: {}"; - public static final String ATTEMPTING_TO_CREATE_OBJECT_STORE_CLIENT = "Attempting to create object store client with provider: {}, container: {}, region: {}, host: {}, endpoint: {}"; + public static final String OBJECT_STORE_PROVIDERS_DETECTED_0 = "Object store providers detected: {0}"; + public static final String ATTEMPTING_TO_CREATE_OBJECT_STORE_CLIENT = "Attempting to create object store client with provider: {0}, container: {1}, region: {2}, host: {3}, endpoint: {4}"; public static final String OBJECT_STORE_WITH_PROVIDER_0_CREATED = "Object store with provider: {0} created"; - public static final String JOB_WITH_ID_WAS_NOT_UPDATED_WITHIN_SECONDS_ON_START = "startUploadFromUrl: Job with ID: {} was not updated within: {} seconds, clearing and re-triggering"; - public static final String JOB_WITH_ID_WAS_NOT_UPDATED_WITHIN_SECONDS_ON_GET = "getUploadFromUrlJob: Job with ID: {} was not updated within: {} seconds, returning error"; - public static final String STALE_JOB_DETAILS = "Stale job details - id: {}, state: {}, updatedAt: {}, addedAt: {}, startedAt: {}, bytesRead: {}, url: {}, space: {}, namespace: {}, user: {}, instance: {}"; + public static final String JOB_WITH_ID_WAS_NOT_UPDATED_WITHIN_SECONDS_ON_START_0_1 = "startUploadFromUrl: Job with ID: {0} was not updated within: {1} seconds, clearing and re-triggering"; + public static final String JOB_WITH_ID_WAS_NOT_UPDATED_WITHIN_SECONDS_ON_GET_0_1 = "getUploadFromUrlJob: Job with ID: {0} was not updated within: {1} seconds, returning error"; public static final String CLEARING_OLD_ENTRY = "Clearing old entry with id: {0}"; // DEBUG log messages @@ -92,7 +91,7 @@ public final class Messages { public static final String ASYNC_UPLOAD_JOB_FINISHED = "Async upload job {} finished"; public static final String UPLOADING_MTAR_STREAM_FROM_REMOTE_ENDPOINT_WITH_JOB_ID = "Uploading MTAR stream from remote endpoint: {}. Job id: {}"; public static final String CALLING_REMOTE_MTAR_ENDPOINT_FOR_JOB_WITH_ID = "Calling remote MTAR endpoint {}. Job id: {}"; - public static final String ASYNC_UPLOAD_JOB_MONITOR_UPDATE = "Job {} monitor update - state: {}, bytesRead: {}, updatedAt: {}"; + public static final String ASYNC_UPLOAD_JOB_MONITOR_UPDATE_0_1_2_3 = "Job {0} monitor update - state: {1}, bytesRead: {2}, updatedAt: {3}"; private Messages() { } diff --git a/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/api/impl/FilesApiServiceImpl.java b/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/api/impl/FilesApiServiceImpl.java index 3c8996c4b4..8a52541155 100644 --- a/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/api/impl/FilesApiServiceImpl.java +++ b/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/api/impl/FilesApiServiceImpl.java @@ -1,5 +1,17 @@ package org.cloudfoundry.multiapps.controller.web.api.impl; +import java.io.BufferedInputStream; +import java.io.InputStream; +import java.math.BigInteger; +import java.text.MessageFormat; +import java.time.Instant; +import java.time.LocalDateTime; +import java.time.ZoneOffset; +import java.time.temporal.ChronoUnit; +import java.util.Base64; +import java.util.List; +import java.util.concurrent.ExecutorService; +import java.util.stream.Collectors; import jakarta.inject.Inject; import jakarta.inject.Named; import jakarta.servlet.http.HttpServletRequest; @@ -38,17 +50,6 @@ import org.springframework.web.multipart.MultipartFile; import org.springframework.web.multipart.MultipartHttpServletRequest; -import java.io.BufferedInputStream; -import java.io.InputStream; -import java.math.BigInteger; -import java.text.MessageFormat; -import java.time.LocalDateTime; -import java.time.temporal.ChronoUnit; -import java.util.Base64; -import java.util.List; -import java.util.concurrent.ExecutorService; -import java.util.stream.Collectors; - @Named public class FilesApiServiceImpl implements FilesApiService { @@ -105,12 +106,12 @@ public ResponseEntity uploadFile(MultipartHttpServletRequest reque LOGGER.trace(Messages.RECEIVED_UPLOAD_REQUEST, ServletUtil.decodeUri(request)); var multipartFile = getFileFromRequest(request); try (InputStream in = new BufferedInputStream(multipartFile.getInputStream(), INPUT_STREAM_BUFFER_SIZE)) { - var startTime = LocalDateTime.now(); + var startTime = Instant.now(); FileEntry fileEntry = fileStorageThreadPool.submit(createUploadFileTask(spaceGuid, namespace, multipartFile, in)) .get(); FileMetadata file = parseFileEntry(fileEntry); filesApiServiceAuditLog.logUploadFile(SecurityContextUtil.getUsername(), spaceGuid, file); - var endTime = LocalDateTime.now(); + var endTime = Instant.now(); LOGGER.trace(Messages.UPLOADED_FILE, file.getId(), LogSanitizer.sanitize(file.getName()), file.getSize(), file.getSpace(), file.getDigest(), file.getDigestAlgorithm(), ChronoUnit.MILLIS.between(startTime, endTime)); return ResponseEntity.status(HttpStatus.CREATED) @@ -134,11 +135,9 @@ public ResponseEntity startUploadFromUrl(String spaceGuid, String namespac return triggerUploadFromUrl(spaceGuid, namespace, urlWithoutUserInfo, decodedUrl, fileUrl.getUserCredentials()); } if (hasJobStuck(existingJob)) { - LOGGER.warn(Messages.JOB_WITH_ID_WAS_NOT_UPDATED_WITHIN_SECONDS_ON_START, existingJob.getId(), UPDATE_JOB_TIMEOUT); - LOGGER.warn(Messages.STALE_JOB_DETAILS, existingJob.getId(), existingJob.getState(), existingJob.getUpdatedAt(), - existingJob.getAddedAt(), existingJob.getStartedAt(), existingJob.getBytesRead(), existingJob.getUrl(), - existingJob.getSpaceGuid(), existingJob.getNamespace(), existingJob.getUser(), - existingJob.getInstanceIndex()); + LOGGER.warn(MessageFormat.format(Messages.JOB_WITH_ID_WAS_NOT_UPDATED_WITHIN_SECONDS_ON_START_0_1, existingJob.getId(), + UPDATE_JOB_TIMEOUT)); + LOGGER.warn(existingJob.buildStaleDetailsLogMessage()); deleteAsyncJobEntry(existingJob); return triggerUploadFromUrl(spaceGuid, namespace, urlWithoutUserInfo, decodedUrl, fileUrl.getUserCredentials()); } @@ -150,7 +149,7 @@ public ResponseEntity startUploadFromUrl(String spaceGuid, String namespac private boolean hasJobStuck(AsyncUploadJobEntry existingJob) { return existingJob.getUpdatedAt() - .isBefore(LocalDateTime.now() + .isBefore(LocalDateTime.now(ZoneOffset.UTC) .minusSeconds(UPDATE_JOB_TIMEOUT)); } @@ -172,10 +171,9 @@ public ResponseEntity getUploadFromUrlJob(String spaceGuid, S private ResponseEntity getAsyncUploadResult(AsyncUploadJobEntry job) { if (job.getState() == State.RUNNING || job.getState() == State.INITIAL) { if (hasJobStuck(job)) { - LOGGER.warn(Messages.JOB_WITH_ID_WAS_NOT_UPDATED_WITHIN_SECONDS_ON_GET, job.getId(), UPDATE_JOB_TIMEOUT); - LOGGER.warn(Messages.STALE_JOB_DETAILS, job.getId(), job.getState(), job.getUpdatedAt(), job.getAddedAt(), - job.getStartedAt(), job.getBytesRead(), job.getUrl(), job.getSpaceGuid(), job.getNamespace(), - job.getUser(), job.getInstanceIndex()); + LOGGER.warn(MessageFormat.format(Messages.JOB_WITH_ID_WAS_NOT_UPDATED_WITHIN_SECONDS_ON_GET_0_1, job.getId(), + UPDATE_JOB_TIMEOUT)); + LOGGER.warn(job.buildStaleDetailsLogMessage()); return ResponseEntity.ok( createErrorResult(MessageFormat.format(Messages.JOB_NOT_UPDATED_FOR_0_SECONDS, UPDATE_JOB_TIMEOUT), AsyncUploadResult.ClientAction.RETRY_UPLOAD)); @@ -265,7 +263,7 @@ private void deleteAsyncJobEntry(AsyncUploadJobEntry entry) { .id(entry.getId()) .delete(); } catch (Exception e) { - LOGGER.error(Messages.ERROR_OCCURRED_WHILE_DELETING_JOB_ENTRY, entry.getId(), e); + LOGGER.error(MessageFormat.format(Messages.ERROR_OCCURRED_WHILE_DELETING_JOB_ENTRY, entry.getId()), e); } } diff --git a/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/configuration/bean/factory/ObjectStoreFileStorageFactoryBean.java b/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/configuration/bean/factory/ObjectStoreFileStorageFactoryBean.java index 18e473873b..a9ed5e3354 100644 --- a/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/configuration/bean/factory/ObjectStoreFileStorageFactoryBean.java +++ b/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/configuration/bean/factory/ObjectStoreFileStorageFactoryBean.java @@ -10,7 +10,6 @@ import java.util.Set; import java.util.stream.Collectors; import java.util.stream.Stream; - import com.google.common.base.Joiner; import io.pivotal.cfenv.core.CfService; import org.apache.commons.lang3.StringUtils; @@ -57,12 +56,14 @@ public void afterPropertiesSet() { private FileStorage createObjectStoreFileStorage() { List providersServiceInfo = getProvidersServiceInfo(); if (LOGGER.isInfoEnabled()) { - LOGGER.info(Messages.OBJECT_STORE_PROVIDERS_DETECTED, providersServiceInfo.stream() - .map(ObjectStoreServiceInfo::getProvider) - .collect(Collectors.joining(", "))); + LOGGER.info(MessageFormat.format(Messages.OBJECT_STORE_PROVIDERS_DETECTED_0, providersServiceInfo.stream() + .map( + ObjectStoreServiceInfo::getProvider) + .collect(Collectors.joining( + ", ")))); } if (providersServiceInfo.isEmpty()) { - LOGGER.warn(Messages.NO_OBJECT_STORE_PROVIDERS_DETECTED, serviceName); + LOGGER.warn(MessageFormat.format(Messages.NO_OBJECT_STORE_PROVIDERS_DETECTED_FOR_SERVICE_0, serviceName)); return null; } Map exceptions = new HashMap<>(); @@ -114,16 +115,16 @@ public FileStorage createObjectStoreFromFirstReachableProvider(Map tryToCreateObjectStore(ObjectStoreServiceInfo objectStoreServiceInfo, Map exceptions) { try { - LOGGER.info(Messages.ATTEMPTING_TO_CREATE_OBJECT_STORE_CLIENT, - objectStoreServiceInfo.getProvider(), - objectStoreServiceInfo.getCredentials() - .get(Constants.BUCKET), - objectStoreServiceInfo.getCredentials() - .get(Constants.REGION), - objectStoreServiceInfo.getCredentials() - .get(Constants.HOST), - objectStoreServiceInfo.getCredentials() - .get(Constants.ENDPOINT)); + LOGGER.info(MessageFormat.format(Messages.ATTEMPTING_TO_CREATE_OBJECT_STORE_CLIENT, + objectStoreServiceInfo.getProvider(), + objectStoreServiceInfo.getCredentials() + .get(Constants.BUCKET), + objectStoreServiceInfo.getCredentials() + .get(Constants.REGION), + objectStoreServiceInfo.getCredentials() + .get(Constants.HOST), + objectStoreServiceInfo.getCredentials() + .get(Constants.ENDPOINT))); FileStorage fileStorage = getFileStorageBasedOnProvider(objectStoreServiceInfo); fileStorage.testConnection(); LOGGER.info(MessageFormat.format(Messages.OBJECT_STORE_WITH_PROVIDER_0_CREATED, objectStoreServiceInfo.getProvider())); diff --git a/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/configuration/service/ObjectStoreServiceInfoCreator.java b/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/configuration/service/ObjectStoreServiceInfoCreator.java index 0f0914b1b6..b9d2a45583 100644 --- a/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/configuration/service/ObjectStoreServiceInfoCreator.java +++ b/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/configuration/service/ObjectStoreServiceInfoCreator.java @@ -1,10 +1,9 @@ package org.cloudfoundry.multiapps.controller.web.configuration.service; -import org.cloudfoundry.multiapps.controller.web.Constants; - import java.util.HashMap; import java.util.List; import java.util.Map; +import org.cloudfoundry.multiapps.controller.web.Constants; public class ObjectStoreServiceInfoCreator { diff --git a/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/upload/AsyncUploadJobOrchestrator.java b/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/upload/AsyncUploadJobOrchestrator.java index b7a3d71b46..faebc87071 100644 --- a/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/upload/AsyncUploadJobOrchestrator.java +++ b/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/upload/AsyncUploadJobOrchestrator.java @@ -6,7 +6,9 @@ import java.net.URI; import java.text.MessageFormat; import java.time.Duration; +import java.time.Instant; import java.time.LocalDateTime; +import java.time.ZoneOffset; import java.time.temporal.ChronoUnit; import java.util.UUID; import java.util.concurrent.ExecutorService; @@ -95,27 +97,27 @@ private AsyncUploadJobEntry createJobEntry(String spaceGuid, String namespace, S .id(UUID.randomUUID() .toString()) .user(SecurityContextUtil.getUsername()) - .addedAt(LocalDateTime.now()) + .addedAt(LocalDateTime.now(ZoneOffset.UTC)) .spaceGuid(spaceGuid) .namespace(namespace) .instanceIndex(applicationConfiguration.getApplicationInstanceIndex()) .url(url) .state(AsyncUploadJobEntry.State.INITIAL) - .updatedAt(LocalDateTime.now()) + .updatedAt(LocalDateTime.now(ZoneOffset.UTC)) .bytesRead(0L) .build(); } private void deployFromUrl(AsyncUploadJobEntry jobEntry, String fileUrl, UserCredentials userCredentials) { LOGGER.info(Messages.STARTING_DOWNLOAD_OF_MTAR_WITH_JOB_ID, LogSanitizer.sanitize(jobEntry.getUrl()), jobEntry.getId()); - var startTime = LocalDateTime.now(); + var startTime = LocalDateTime.now(ZoneOffset.UTC); Lock lock = new ReentrantLock(); AtomicLong counterRef = new AtomicLong(); try { var updatedJobEntry = asyncUploadJobService.update(jobEntry, ImmutableAsyncUploadJobEntry.copyOf(jobEntry) .withState( AsyncUploadJobEntry.State.RUNNING) - .withUpdatedAt(LocalDateTime.now()) + .withUpdatedAt(LocalDateTime.now(ZoneOffset.UTC)) .withStartedAt(startTime)); startAsyncUploadFromUrlUpload(ImmutableUploadFromUrlContext.builder() .jobEntry(updatedJobEntry) @@ -156,7 +158,9 @@ private void startSyncUploadFromUrlUpload(UploadFromUrlContext uploadFromUrlCont LOGGER.trace(Messages.UPLOADED_MTAR_FROM_REMOTE_ENDPOINT_AND_JOB_ID, uploadFromUrlContext.getJobEntry() .getUrl(), uploadFromUrlContext.getJobEntry() - .getId(), ChronoUnit.MILLIS.between(uploadFromUrlContext.getStartTime(), LocalDateTime.now())); + .getId(), + ChronoUnit.MILLIS.between(uploadFromUrlContext.getStartTime() + .toInstant(ZoneOffset.UTC), Instant.now())); var descriptor = fileService.processFileContent(uploadFromUrlContext.getJobEntry() .getSpaceGuid(), fileEntry.getId(), this::extractDeploymentDescriptor); @@ -172,8 +176,8 @@ private void startSyncUploadFromUrlUpload(UploadFromUrlContext uploadFromUrlCont .withFileId(fileEntry.getId()) .withMtaId(descriptor.getId()) .withSchemaVersion(descriptor.getSchemaVersion()) - .withUpdatedAt(LocalDateTime.now()) - .withFinishedAt(LocalDateTime.now()) + .withUpdatedAt(LocalDateTime.now(ZoneOffset.UTC)) + .withFinishedAt(LocalDateTime.now(ZoneOffset.UTC)) .withBytesRead(uploadFromUrlContext.getCounterRef() .get()) .withState(AsyncUploadJobEntry.State.FINISHED)); @@ -215,7 +219,7 @@ private void resetCounterOnRetry(UploadFromUrlContext upload, Lock lock) { .getId()) .singleResult(); asyncUploadJobService.update(asyncUploadJobEntry, ImmutableAsyncUploadJobEntry.copyOf(asyncUploadJobEntry) - .withUpdatedAt(LocalDateTime.now()) + .withUpdatedAt(LocalDateTime.now(ZoneOffset.UTC)) .withBytesRead(0L)); } finally { lock.unlock(); @@ -248,12 +252,13 @@ private void monitorAsyncUploadJob(AsyncUploadJobEntry updatedJobEntry, Lock loc updatedJobEntry = asyncUploadJobService.update(updatedJobEntry, ImmutableAsyncUploadJobEntry.copyOf(updatedJobEntry) .withBytesRead(counterRef.get()) .withUpdatedAt( - LocalDateTime.now())); + LocalDateTime.now(ZoneOffset.UTC))); } finally { lock.unlock(); } - LOGGER.info(Messages.ASYNC_UPLOAD_JOB_MONITOR_UPDATE, updatedJobEntry.getId(), updatedJobEntry.getState(), - updatedJobEntry.getBytesRead(), updatedJobEntry.getUpdatedAt()); + LOGGER.info(MessageFormat.format(Messages.ASYNC_UPLOAD_JOB_MONITOR_UPDATE_0_1_2_3, updatedJobEntry.getId(), + updatedJobEntry.getState(), updatedJobEntry.getBytesRead(), + updatedJobEntry.getUpdatedAt())); waitBetweenUpdates(); } } @@ -269,8 +274,8 @@ private void updateFailedAsyncUploadJob(AsyncUploadJobEntry jobEntry, Exception .id(jobEntry.getId()) .singleResult(); asyncUploadJobService.update(failedEntry, ImmutableAsyncUploadJobEntry.copyOf(failedEntry) - .withUpdatedAt(LocalDateTime.now()) - .withFinishedAt(LocalDateTime.now()) + .withUpdatedAt(LocalDateTime.now(ZoneOffset.UTC)) + .withFinishedAt(LocalDateTime.now(ZoneOffset.UTC)) .withError( MessageFormat.format(Messages.ASYNC_UPLOAD_JOB_ERROR, e.getMessage(), diff --git a/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/upload/resilience/FileUploadResilientOperationExecutor.java b/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/upload/resilience/FileUploadResilientOperationExecutor.java index 96626294e3..1a0dd2223c 100644 --- a/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/upload/resilience/FileUploadResilientOperationExecutor.java +++ b/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/upload/resilience/FileUploadResilientOperationExecutor.java @@ -1,16 +1,21 @@ package org.cloudfoundry.multiapps.controller.web.upload.resilience; +import java.io.IOException; +import java.io.UncheckedIOException; +import java.text.MessageFormat; +import java.time.Duration; +import java.util.Set; import org.cloudfoundry.multiapps.common.util.MiscUtil; import org.cloudfoundry.multiapps.controller.client.util.CheckedSupplier; import org.cloudfoundry.multiapps.controller.persistence.services.FileStorageException; import org.cloudfoundry.multiapps.controller.web.Messages; import org.slf4j.Logger; import org.slf4j.LoggerFactory; - -import java.io.IOException; -import java.io.UncheckedIOException; -import java.text.MessageFormat; -import java.time.Duration; +import org.springframework.http.HttpStatus; +import software.amazon.awssdk.core.exception.ApiCallAttemptTimeoutException; +import software.amazon.awssdk.core.exception.ApiCallTimeoutException; +import software.amazon.awssdk.core.exception.SdkException; +import software.amazon.awssdk.core.exception.SdkServiceException; public class FileUploadResilientOperationExecutor { @@ -22,6 +27,12 @@ public class FileUploadResilientOperationExecutor { .toMillis(); private static final long MAX_WAIT_TIME_IN_MILLIS = Duration.ofSeconds(300) .toMillis(); + private static final Set RETRYABLE_HTTP_STATUSES = Set.of(HttpStatus.REQUEST_TIMEOUT, + HttpStatus.TOO_MANY_REQUESTS, + HttpStatus.INTERNAL_SERVER_ERROR, + HttpStatus.BAD_GATEWAY, + HttpStatus.SERVICE_UNAVAILABLE, + HttpStatus.GATEWAY_TIMEOUT); public T execute(CheckedSupplier operation) throws Exception { Exception lastException = null; @@ -52,7 +63,7 @@ protected void sleep(long millis) { private boolean isTransientError(Exception e) { Throwable cause = e instanceof FileStorageException ? e.getCause() : e; while (cause != null) { - if (cause instanceof IOException || cause instanceof UncheckedIOException) { + if (isTransientCause(cause)) { return true; } cause = cause.getCause(); @@ -60,6 +71,24 @@ private boolean isTransientError(Exception e) { return false; } + private boolean isTransientCause(Throwable cause) { + if (cause instanceof IOException || cause instanceof UncheckedIOException) { + return true; + } + if (cause instanceof ApiCallAttemptTimeoutException || cause instanceof ApiCallTimeoutException) { + return true; + } + if (cause instanceof SdkServiceException serviceException && isRetryableStatusCode(serviceException.statusCode())) { + return true; + } + return cause instanceof SdkException sdkException && sdkException.retryable(); + } + + private boolean isRetryableStatusCode(int statusCode) { + HttpStatus status = HttpStatus.resolve(statusCode); + return status != null && RETRYABLE_HTTP_STATUSES.contains(status); + } + private long calculateWaitTime(int attempt) { long waitTime = (long) (INITIAL_WAIT_TIME_IN_MILLIS * Math.pow(BACKOFF_FACTOR, (double) attempt - 1)); return Math.min(waitTime, MAX_WAIT_TIME_IN_MILLIS); diff --git a/multiapps-controller-web/src/test/java/org/cloudfoundry/multiapps/controller/web/api/impl/FilesApiServiceImplTest.java b/multiapps-controller-web/src/test/java/org/cloudfoundry/multiapps/controller/web/api/impl/FilesApiServiceImplTest.java index 530e09b0e1..1860062893 100644 --- a/multiapps-controller-web/src/test/java/org/cloudfoundry/multiapps/controller/web/api/impl/FilesApiServiceImplTest.java +++ b/multiapps-controller-web/src/test/java/org/cloudfoundry/multiapps/controller/web/api/impl/FilesApiServiceImplTest.java @@ -1,5 +1,19 @@ package org.cloudfoundry.multiapps.controller.web.api.impl; +import java.io.InputStream; +import java.math.BigInteger; +import java.nio.charset.StandardCharsets; +import java.time.LocalDateTime; +import java.time.Month; +import java.util.Base64; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.Random; +import java.util.UUID; +import java.util.concurrent.Callable; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.FutureTask; import jakarta.servlet.http.HttpServletRequest; import org.apache.commons.lang3.RandomStringUtils; import org.cloudfoundry.multiapps.common.SLException; @@ -34,21 +48,6 @@ import org.springframework.security.oauth2.core.user.DefaultOAuth2User; import org.springframework.web.multipart.MultipartFile; import org.springframework.web.multipart.MultipartHttpServletRequest; - -import java.io.InputStream; -import java.math.BigInteger; -import java.nio.charset.StandardCharsets; -import java.time.LocalDateTime; -import java.util.Base64; -import java.util.Collections; -import java.util.List; -import java.util.Map; -import java.util.Random; -import java.util.UUID; -import java.util.concurrent.Callable; -import java.util.concurrent.ExecutorService; -import java.util.concurrent.FutureTask; - import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -63,6 +62,8 @@ class FilesApiServiceImplTest { private static final String SPACE_GUID = "896e6be9-8217-4a1c-b938-09b30966157a"; private static final String NAMESPACE = "custom-namespace"; private static final String DIGEST_CHARACTER_TABLE = "123456789ABCDEF"; + private static final LocalDateTime JOB_TIMESTAMP = LocalDateTime.of(2025, Month.JANUARY, 1, 0, 0); + private static final LocalDateTime ACTIVE_UPDATED_AT = LocalDateTime.of(9999, Month.DECEMBER, 31, 23, 59); private static final String FILE_URL = Base64.getUrlEncoder() .encodeToString( @@ -95,7 +96,7 @@ public void initialize() throws Exception { MockitoAnnotations.openMocks(this) .close(); testedClass = new FilesApiServiceImpl(fileService, uploadJobService, filesApiServiceAuditLog, asyncUploadJobOrchestrator, - apiUsageLogger, fileStorageThreadPool, httpServletRequest); + apiUsageLogger, fileStorageThreadPool, httpServletRequest); SecurityContextHolder.clearContext(); var user = new UserInfo("user1", "user1", null); var token = new DefaultOAuth2User(Collections.emptyList(), Map.of("user_info", user), "user_info"); @@ -215,7 +216,7 @@ void testStartDeployFromUrl() { ImmutableAsyncUploadJobEntry.builder() .id(expectedJobId) .url(DECODED_URL_WITH_CREDENTIALS_IN_THE_URL) - .startedAt(LocalDateTime.now()) + .startedAt(JOB_TIMESTAMP) .state(State.INITIAL) .user("user1") .spaceGuid(SPACE_GUID) @@ -269,10 +270,8 @@ void testStartDeployFromUrlWhenStuckJobExists() { AsyncUploadJobEntry stuckJob = ImmutableAsyncUploadJobEntry.builder() .id(existingJobId) .url(DECODED_URL_WITH_CREDENTIALS_IN_THE_URL) - .startedAt(LocalDateTime.now() - .minusMinutes(5)) - .updatedAt(LocalDateTime.now() - .minusMinutes(2)) + .startedAt(JOB_TIMESTAMP) + .updatedAt(JOB_TIMESTAMP) .state(State.RUNNING) .user("user1") .spaceGuid(SPACE_GUID) @@ -288,7 +287,7 @@ void testStartDeployFromUrlWhenStuckJobExists() { ImmutableAsyncUploadJobEntry.builder() .id(newJobId) .url(DECODED_URL_WITH_CREDENTIALS_IN_THE_URL) - .startedAt(LocalDateTime.now()) + .startedAt(JOB_TIMESTAMP) .state(State.INITIAL) .user("user1") .spaceGuid(SPACE_GUID) @@ -322,10 +321,8 @@ void testStartDeployFromUrlWhenActiveJobExists() { AsyncUploadJobEntry activeJob = ImmutableAsyncUploadJobEntry.builder() .id(existingJobId) .url(DECODED_URL_WITH_CREDENTIALS_IN_THE_URL) - .startedAt(LocalDateTime.now() - .minusMinutes(1)) - .updatedAt(LocalDateTime.now() - .minusSeconds(10)) + .startedAt(JOB_TIMESTAMP) + .updatedAt(ACTIVE_UPDATED_AT) .state(State.RUNNING) .user("user1") .spaceGuid(SPACE_GUID) @@ -382,10 +379,8 @@ void testGetUploadFromUrlJobWhenJobIsSuccessful() throws FileStorageException { AsyncUploadJobEntry successfulJob = ImmutableAsyncUploadJobEntry.builder() .id(jobId) .url(DECODED_URL_WITH_CREDENTIALS_IN_THE_URL) - .startedAt(LocalDateTime.now() - .minusMinutes(5)) - .updatedAt(LocalDateTime.now() - .minusMinutes(1)) + .startedAt(JOB_TIMESTAMP) + .updatedAt(JOB_TIMESTAMP) .state(State.FINISHED) .user("user1") .spaceGuid(SPACE_GUID) @@ -421,10 +416,8 @@ void testGetUploadFromUrlJobWhenJobIsStuck() { AsyncUploadJobEntry stuckJob = ImmutableAsyncUploadJobEntry.builder() .id(jobId) .url(DECODED_URL_WITH_CREDENTIALS_IN_THE_URL) - .startedAt(LocalDateTime.now() - .minusMinutes(5)) - .updatedAt(LocalDateTime.now() - .minusMinutes(2)) + .startedAt(JOB_TIMESTAMP) + .updatedAt(JOB_TIMESTAMP) .state(State.RUNNING) .user("user1") .spaceGuid(SPACE_GUID) @@ -454,10 +447,8 @@ void testGetUploadFromUrlJobWhenJobIsRunning() { AsyncUploadJobEntry runningJob = ImmutableAsyncUploadJobEntry.builder() .id(jobId) .url(DECODED_URL_WITH_CREDENTIALS_IN_THE_URL) - .startedAt(LocalDateTime.now() - .minusMinutes(1)) - .updatedAt(LocalDateTime.now() - .minusSeconds(10)) + .startedAt(JOB_TIMESTAMP) + .updatedAt(ACTIVE_UPDATED_AT) .state(State.RUNNING) .user("user1") .spaceGuid(SPACE_GUID) @@ -486,10 +477,8 @@ void testGetUploadFromUrlJobWhenJobIsInErrorState() { AsyncUploadJobEntry errorJob = ImmutableAsyncUploadJobEntry.builder() .id(jobId) .url(DECODED_URL_WITH_CREDENTIALS_IN_THE_URL) - .startedAt(LocalDateTime.now() - .minusMinutes(5)) - .updatedAt(LocalDateTime.now() - .minusMinutes(1)) + .startedAt(JOB_TIMESTAMP) + .updatedAt(JOB_TIMESTAMP) .state(State.ERROR) .user("user1") .spaceGuid(SPACE_GUID) diff --git a/multiapps-controller-web/src/test/java/org/cloudfoundry/multiapps/controller/web/upload/AsyncUploadJobOrchestratorTest.java b/multiapps-controller-web/src/test/java/org/cloudfoundry/multiapps/controller/web/upload/AsyncUploadJobOrchestratorTest.java index edd8576e9a..89041565da 100644 --- a/multiapps-controller-web/src/test/java/org/cloudfoundry/multiapps/controller/web/upload/AsyncUploadJobOrchestratorTest.java +++ b/multiapps-controller-web/src/test/java/org/cloudfoundry/multiapps/controller/web/upload/AsyncUploadJobOrchestratorTest.java @@ -5,6 +5,7 @@ import java.math.BigInteger; import java.net.URI; import java.time.LocalDateTime; +import java.time.Month; import java.util.concurrent.ExecutorService; import java.util.concurrent.atomic.AtomicBoolean; import org.cloudfoundry.multiapps.common.SLException; @@ -41,6 +42,8 @@ class AsyncUploadJobOrchestratorTest { + private static final LocalDateTime JOB_TIMESTAMP = LocalDateTime.of(2025, Month.JANUARY, 1, 0, 0); + private static final String SPACE_GUID = "space-123"; private static final String NAMESPACE = "test-namespace"; private static final String FILE_URL = "https://example.com/file.mtar"; @@ -139,12 +142,12 @@ void testSuccessfulJobExecution() throws Exception { AsyncUploadJobEntry initialEntry = createInitialJobEntry(); AsyncUploadJobEntry runningEntry = ImmutableAsyncUploadJobEntry.copyOf(initialEntry) .withState(AsyncUploadJobEntry.State.RUNNING) - .withStartedAt(LocalDateTime.now()); + .withStartedAt(JOB_TIMESTAMP); AsyncUploadJobEntry finishedEntry = ImmutableAsyncUploadJobEntry.copyOf(runningEntry) .withState(AsyncUploadJobEntry.State.FINISHED) .withFileId(FILE_ID) .withMtaId(MTA_ID) - .withFinishedAt(LocalDateTime.now()); + .withFinishedAt(JOB_TIMESTAMP); FileEntry fileEntry = createFileEntry(); @@ -188,7 +191,7 @@ void testJobExecutionWithRemoteClientException() throws Exception { AsyncUploadJobEntry initialEntry = createInitialJobEntry(); AsyncUploadJobEntry runningEntry = ImmutableAsyncUploadJobEntry.copyOf(initialEntry) .withState(AsyncUploadJobEntry.State.RUNNING) - .withStartedAt(LocalDateTime.now()); + .withStartedAt(JOB_TIMESTAMP); AsyncUploadJobEntry errorEntry = ImmutableAsyncUploadJobEntry.copyOf(runningEntry) .withState(AsyncUploadJobEntry.State.ERROR); @@ -219,7 +222,7 @@ void testJobExecutionWithFileServiceException() throws Exception { AsyncUploadJobEntry initialEntry = createInitialJobEntry(); AsyncUploadJobEntry runningEntry = ImmutableAsyncUploadJobEntry.copyOf(initialEntry) .withState(AsyncUploadJobEntry.State.RUNNING) - .withStartedAt(LocalDateTime.now()); + .withStartedAt(JOB_TIMESTAMP); AsyncUploadJobEntry errorEntry = ImmutableAsyncUploadJobEntry.copyOf(runningEntry) .withState(AsyncUploadJobEntry.State.ERROR); @@ -252,7 +255,7 @@ void testJobExecutionWithDescriptorParsingException() throws Exception { AsyncUploadJobEntry initialEntry = createInitialJobEntry(); AsyncUploadJobEntry runningEntry = ImmutableAsyncUploadJobEntry.copyOf(initialEntry) .withState(AsyncUploadJobEntry.State.RUNNING) - .withStartedAt(LocalDateTime.now()); + .withStartedAt(JOB_TIMESTAMP); AsyncUploadJobEntry errorEntry = ImmutableAsyncUploadJobEntry.copyOf(runningEntry) .withState(AsyncUploadJobEntry.State.ERROR); @@ -288,19 +291,19 @@ void testJobMonitoringWithMultipleUpdates() throws Exception { AsyncUploadJobEntry initialEntry = createInitialJobEntry(); AsyncUploadJobEntry runningEntry1 = ImmutableAsyncUploadJobEntry.copyOf(initialEntry) .withState(AsyncUploadJobEntry.State.RUNNING) - .withStartedAt(LocalDateTime.now()) + .withStartedAt(JOB_TIMESTAMP) .withBytesRead(100L); AsyncUploadJobEntry runningEntry2 = ImmutableAsyncUploadJobEntry.copyOf(runningEntry1) .withBytesRead(500L) - .withUpdatedAt(LocalDateTime.now()); + .withUpdatedAt(JOB_TIMESTAMP); AsyncUploadJobEntry runningEntry3 = ImmutableAsyncUploadJobEntry.copyOf(runningEntry2) .withBytesRead(800L) - .withUpdatedAt(LocalDateTime.now()); + .withUpdatedAt(JOB_TIMESTAMP); AsyncUploadJobEntry finishedEntry = ImmutableAsyncUploadJobEntry.copyOf(runningEntry3) .withState(AsyncUploadJobEntry.State.FINISHED) .withFileId(FILE_ID) .withMtaId(MTA_ID) - .withFinishedAt(LocalDateTime.now()); + .withFinishedAt(JOB_TIMESTAMP); FileEntry fileEntry = createFileEntry(); @@ -339,7 +342,7 @@ void testJobCreationWithCorrectProperties() { AsyncUploadJobEntry initialEntry = createInitialJobEntry(); AsyncUploadJobEntry runningEntry = ImmutableAsyncUploadJobEntry.copyOf(initialEntry) .withState(AsyncUploadJobEntry.State.RUNNING) - .withStartedAt(LocalDateTime.now()); + .withStartedAt(JOB_TIMESTAMP); when(asyncUploadJobService.add(any(AsyncUploadJobEntry.class))).thenReturn(initialEntry); when(asyncUploadJobService.update(any(AsyncUploadJobEntry.class), any(AsyncUploadJobEntry.class))) @@ -401,13 +404,13 @@ private AsyncUploadJobEntry createInitialJobEntry() { return ImmutableAsyncUploadJobEntry.builder() .id(JOB_ID) .user(USERNAME) - .addedAt(LocalDateTime.now()) + .addedAt(JOB_TIMESTAMP) .spaceGuid(SPACE_GUID) .namespace(NAMESPACE) .instanceIndex(0) .url(FILE_URL) .state(AsyncUploadJobEntry.State.INITIAL) - .updatedAt(LocalDateTime.now()) + .updatedAt(JOB_TIMESTAMP) .bytesRead(0L) .build(); } diff --git a/multiapps-controller-web/src/test/java/org/cloudfoundry/multiapps/controller/web/upload/FileUploadResilientOperationExecutorTest.java b/multiapps-controller-web/src/test/java/org/cloudfoundry/multiapps/controller/web/upload/FileUploadResilientOperationExecutorTest.java index b00f9e038c..728511e105 100644 --- a/multiapps-controller-web/src/test/java/org/cloudfoundry/multiapps/controller/web/upload/FileUploadResilientOperationExecutorTest.java +++ b/multiapps-controller-web/src/test/java/org/cloudfoundry/multiapps/controller/web/upload/FileUploadResilientOperationExecutorTest.java @@ -5,7 +5,11 @@ import org.cloudfoundry.multiapps.controller.web.upload.resilience.FileUploadResilientOperationExecutor; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import software.amazon.awssdk.core.exception.ApiCallAttemptTimeoutException; +import software.amazon.awssdk.core.exception.ApiCallTimeoutException; +import software.amazon.awssdk.core.exception.RetryableException; import software.amazon.awssdk.core.exception.SdkException; +import software.amazon.awssdk.services.s3.model.S3Exception; import java.io.IOException; import java.io.UncheckedIOException; @@ -59,13 +63,14 @@ void testRetryOnFileStorageExceptionWithIOExceptionCause() throws Exception { } @Test - void testNoRetryOnFileStorageExceptionWithSdkExceptionCause() { + void testNoRetryOnFileStorageExceptionWithBareSdkException() { var attempts = new AtomicInteger(0); + // A bare SdkException with no status code, no retryable cause, and no retryable subtype. assertThrows(FileStorageException.class, () -> executor.execute(() -> { attempts.incrementAndGet(); throw new FileStorageException("S3 error", SdkException.builder() - .message("Service unavailable") + .message("unspecified failure") .build()); })); @@ -73,6 +78,91 @@ void testNoRetryOnFileStorageExceptionWithSdkExceptionCause() { assertTrue(recordedSleepDurations.isEmpty()); } + @Test + void testRetryOnApiCallAttemptTimeoutException() throws Exception { + var attempts = new AtomicInteger(0); + + var result = executor.execute(() -> { + if (attempts.incrementAndGet() < 2) { + throw new FileStorageException("attempt timed out", ApiCallAttemptTimeoutException.builder() + .message("attempt timeout") + .build()); + } + return "recovered"; + }); + + assertEquals("recovered", result); + assertEquals(1, recordedSleepDurations.size()); + } + + @Test + void testRetryOnApiCallTimeoutException() throws Exception { + var attempts = new AtomicInteger(0); + + var result = executor.execute(() -> { + if (attempts.incrementAndGet() < 2) { + throw new FileStorageException("api call timed out", ApiCallTimeoutException.builder() + .message("api call timeout") + .build()); + } + return "recovered"; + }); + + assertEquals("recovered", result); + assertEquals(1, recordedSleepDurations.size()); + } + + @Test + void testRetryOnS3ExceptionWithRetryableStatusCode() throws Exception { + var attempts = new AtomicInteger(0); + + var result = executor.execute(() -> { + if (attempts.incrementAndGet() < 2) { + throw new FileStorageException("transient s3 error", S3Exception.builder() + .message("service unavailable") + .statusCode(503) + .build()); + } + return "recovered"; + }); + + assertEquals("recovered", result); + assertEquals(1, recordedSleepDurations.size()); + } + + @Test + void testNoRetryOnS3ExceptionWithPermanentStatusCode() { + var attempts = new AtomicInteger(0); + + assertThrows(FileStorageException.class, () -> executor.execute(() -> { + attempts.incrementAndGet(); + throw new FileStorageException("permanent s3 error", S3Exception.builder() + .message("not found") + .statusCode(404) + .build()); + })); + + assertEquals(1, attempts.get()); + assertTrue(recordedSleepDurations.isEmpty()); + } + + @Test + void testRetryOnRetryableException() throws Exception { + var attempts = new AtomicInteger(0); + + var result = executor.execute(() -> { + if (attempts.incrementAndGet() < 2) { + throw new FileStorageException("retryable", RetryableException.builder() + .message("retryable error") + .build()); + } + return "recovered"; + }); + + assertEquals("recovered", result); + assertEquals(1, recordedSleepDurations.size()); + } + @Test void testRetryOnFileStorageExceptionWithSdkExceptionWrappingIOException() throws Exception { var attempts = new AtomicInteger(0); diff --git a/pom.xml b/pom.xml index c82bedbfba..d71669d42e 100644 --- a/pom.xml +++ b/pom.xml @@ -48,7 +48,7 @@ 2.12.1 1.16.4 3.18.5 - 2.44.12 + 2.46.3 1.1.1 1.21.0 From d3d54f176906d537f5c460a6aa7ef274921627bf Mon Sep 17 00:00:00 2001 From: IvanBorislavovDimitrov Date: Mon, 15 Jun 2026 17:21:07 +0300 Subject: [PATCH 4/4] More refactoring --- .../JCloudsObjectStoreFileStorage.java | 22 +- .../AwsTransientErrorClassifier.java | 25 +++ .../AzureTransientErrorClassifier.java | 16 ++ .../GcpTransientErrorClassifier.java | 18 ++ .../JCloudsTransientErrorClassifier.java | 20 ++ .../resilience/NoRetryErrorClassifier.java | 15 ++ .../resilience/RetryableErrorClassifier.java | 14 ++ .../resilience/RetryableHttpStatuses.java | 23 ++ .../AwsTransientErrorClassifierTest.java | 69 ++++++ .../AzureTransientErrorClassifierTest.java | 47 +++++ .../GcpTransientErrorClassifierTest.java | 55 +++++ .../JCloudsTransientErrorClassifierTest.java | 60 ++++++ .../OperationInErrorStateHandlerTest.java | 59 +++--- .../multiapps/controller/web/Constants.java | 2 - .../configuration/DatabaseConfiguration.java | 2 +- .../FileStorageConfiguration.java | 22 +- .../CloudDataSourceFactoryBean.java | 2 +- .../{factory => }/FileServiceFactoryBean.java | 2 +- .../ObjectStoreSelectorFactory.java} | 99 ++++----- .../upload/AsyncUploadJobOrchestrator.java | 11 +- .../FileUploadResilientOperationExecutor.java | 70 +++--- .../web/api/impl/FilesApiServiceImplTest.java | 57 ++--- .../CloudDataSourceFactoryBeanTest.java | 18 +- ...va => ObjectStoreSelectorFactoryTest.java} | 199 +++++++++--------- .../ObjectStoreServiceInfoCreatorTest.java | 6 +- .../AsyncUploadJobOrchestratorTest.java | 31 +-- ...eUploadResilientOperationExecutorTest.java | 191 +++-------------- 27 files changed, 675 insertions(+), 480 deletions(-) create mode 100644 multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/resilience/AwsTransientErrorClassifier.java create mode 100644 multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/resilience/AzureTransientErrorClassifier.java create mode 100644 multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/resilience/GcpTransientErrorClassifier.java create mode 100644 multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/resilience/JCloudsTransientErrorClassifier.java create mode 100644 multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/resilience/NoRetryErrorClassifier.java create mode 100644 multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/resilience/RetryableErrorClassifier.java create mode 100644 multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/resilience/RetryableHttpStatuses.java create mode 100644 multiapps-controller-persistence/src/test/java/org/cloudfoundry/multiapps/controller/persistence/services/resilience/AwsTransientErrorClassifierTest.java create mode 100644 multiapps-controller-persistence/src/test/java/org/cloudfoundry/multiapps/controller/persistence/services/resilience/AzureTransientErrorClassifierTest.java create mode 100644 multiapps-controller-persistence/src/test/java/org/cloudfoundry/multiapps/controller/persistence/services/resilience/GcpTransientErrorClassifierTest.java create mode 100644 multiapps-controller-persistence/src/test/java/org/cloudfoundry/multiapps/controller/persistence/services/resilience/JCloudsTransientErrorClassifierTest.java rename multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/configuration/bean/{factory => }/CloudDataSourceFactoryBean.java (99%) rename multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/configuration/bean/{factory => }/FileServiceFactoryBean.java (99%) rename multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/configuration/{bean/factory/ObjectStoreFileStorageFactoryBean.java => factory/ObjectStoreSelectorFactory.java} (76%) rename multiapps-controller-web/src/test/java/org/cloudfoundry/multiapps/controller/web/configuration/bean/factory/{ObjectStoreFileStorageFactoryBeanTest.java => ObjectStoreSelectorFactoryTest.java} (65%) diff --git a/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/JCloudsObjectStoreFileStorage.java b/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/JCloudsObjectStoreFileStorage.java index d239875573..8880435814 100644 --- a/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/JCloudsObjectStoreFileStorage.java +++ b/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/JCloudsObjectStoreFileStorage.java @@ -1,5 +1,15 @@ package org.cloudfoundry.multiapps.controller.persistence.services; +import java.io.IOException; +import java.io.InputStream; +import java.text.MessageFormat; +import java.time.LocalDateTime; +import java.util.HashSet; +import java.util.List; +import java.util.Objects; +import java.util.Set; +import java.util.function.Predicate; +import java.util.stream.Collectors; import org.cloudfoundry.multiapps.common.util.MiscUtil; import org.cloudfoundry.multiapps.controller.persistence.Messages; import org.cloudfoundry.multiapps.controller.persistence.model.FileEntry; @@ -19,17 +29,6 @@ import org.slf4j.LoggerFactory; import org.springframework.http.MediaType; -import java.io.IOException; -import java.io.InputStream; -import java.text.MessageFormat; -import java.time.LocalDateTime; -import java.util.HashSet; -import java.util.List; -import java.util.Objects; -import java.util.Set; -import java.util.function.Predicate; -import java.util.stream.Collectors; - public class JCloudsObjectStoreFileStorage extends ObjectStoreFileStorage { private static final Logger LOGGER = LoggerFactory.getLogger(JCloudsObjectStoreFileStorage.class); @@ -65,7 +64,6 @@ public void addFile(FileEntry fileEntry, InputStream content) throws FileStorage } } - // BlobStore::list may not return a complete list, making this method unreliable @Override @Deprecated public List getFileEntriesWithoutContent(List fileEntries) { diff --git a/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/resilience/AwsTransientErrorClassifier.java b/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/resilience/AwsTransientErrorClassifier.java new file mode 100644 index 0000000000..888dc9914d --- /dev/null +++ b/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/resilience/AwsTransientErrorClassifier.java @@ -0,0 +1,25 @@ +package org.cloudfoundry.multiapps.controller.persistence.services.resilience; + +import software.amazon.awssdk.core.exception.ApiCallAttemptTimeoutException; +import software.amazon.awssdk.core.exception.ApiCallTimeoutException; +import software.amazon.awssdk.core.exception.SdkException; +import software.amazon.awssdk.core.exception.SdkServiceException; + +public class AwsTransientErrorClassifier implements RetryableErrorClassifier { + + @Override + public boolean isRetryable(Throwable cause) { + if (isIoFailure(cause)) { + return true; + } + if (cause instanceof ApiCallTimeoutException || cause instanceof ApiCallAttemptTimeoutException) { + return true; + } + if (cause instanceof SdkServiceException sdkServiceException + && RetryableHttpStatuses.contains(sdkServiceException.statusCode())) { + return true; + } + return cause instanceof SdkException sdkException && sdkException.retryable(); + } + +} diff --git a/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/resilience/AzureTransientErrorClassifier.java b/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/resilience/AzureTransientErrorClassifier.java new file mode 100644 index 0000000000..197affab5f --- /dev/null +++ b/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/resilience/AzureTransientErrorClassifier.java @@ -0,0 +1,16 @@ +package org.cloudfoundry.multiapps.controller.persistence.services.resilience; + +import com.azure.storage.blob.models.BlobStorageException; + +public class AzureTransientErrorClassifier implements RetryableErrorClassifier { + + @Override + public boolean isRetryable(Throwable cause) { + if (isIoFailure(cause)) { + return true; + } + return cause instanceof BlobStorageException blobStorageException + && RetryableHttpStatuses.contains(blobStorageException.getStatusCode()); + } + +} diff --git a/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/resilience/GcpTransientErrorClassifier.java b/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/resilience/GcpTransientErrorClassifier.java new file mode 100644 index 0000000000..77dd3fb226 --- /dev/null +++ b/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/resilience/GcpTransientErrorClassifier.java @@ -0,0 +1,18 @@ +package org.cloudfoundry.multiapps.controller.persistence.services.resilience; + +import com.google.cloud.storage.StorageException; + +public class GcpTransientErrorClassifier implements RetryableErrorClassifier { + + @Override + public boolean isRetryable(Throwable cause) { + if (isIoFailure(cause)) { + return true; + } + if (!(cause instanceof StorageException storageException)) { + return false; + } + return storageException.isRetryable() || RetryableHttpStatuses.contains(storageException.getCode()); + } + +} diff --git a/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/resilience/JCloudsTransientErrorClassifier.java b/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/resilience/JCloudsTransientErrorClassifier.java new file mode 100644 index 0000000000..404f45ccfb --- /dev/null +++ b/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/resilience/JCloudsTransientErrorClassifier.java @@ -0,0 +1,20 @@ +package org.cloudfoundry.multiapps.controller.persistence.services.resilience; + +import org.jclouds.http.HttpResponse; +import org.jclouds.http.HttpResponseException; + +public class JCloudsTransientErrorClassifier implements RetryableErrorClassifier { + + @Override + public boolean isRetryable(Throwable cause) { + if (isIoFailure(cause)) { + return true; + } + if (!(cause instanceof HttpResponseException httpResponseException)) { + return false; + } + HttpResponse response = httpResponseException.getResponse(); + return response != null && RetryableHttpStatuses.contains(response.getStatusCode()); + } + +} diff --git a/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/resilience/NoRetryErrorClassifier.java b/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/resilience/NoRetryErrorClassifier.java new file mode 100644 index 0000000000..65ce31d222 --- /dev/null +++ b/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/resilience/NoRetryErrorClassifier.java @@ -0,0 +1,15 @@ +package org.cloudfoundry.multiapps.controller.persistence.services.resilience; + +/** + * Fallback classifier used when no object store provider is bound. It treats every error as + * non-retryable so the resilient executor performs a single attempt — there is no transient + * cloud-store failure mode to retry against in that environment. + */ +public class NoRetryErrorClassifier implements RetryableErrorClassifier { + + @Override + public boolean isRetryable(Throwable cause) { + return false; + } + +} diff --git a/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/resilience/RetryableErrorClassifier.java b/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/resilience/RetryableErrorClassifier.java new file mode 100644 index 0000000000..879e8b42fa --- /dev/null +++ b/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/resilience/RetryableErrorClassifier.java @@ -0,0 +1,14 @@ +package org.cloudfoundry.multiapps.controller.persistence.services.resilience; + +import java.io.IOException; +import java.io.UncheckedIOException; + +public interface RetryableErrorClassifier { + + boolean isRetryable(Throwable cause); + + default boolean isIoFailure(Throwable cause) { + return cause instanceof IOException || cause instanceof UncheckedIOException; + } + +} diff --git a/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/resilience/RetryableHttpStatuses.java b/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/resilience/RetryableHttpStatuses.java new file mode 100644 index 0000000000..f630a2d0d0 --- /dev/null +++ b/multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/resilience/RetryableHttpStatuses.java @@ -0,0 +1,23 @@ +package org.cloudfoundry.multiapps.controller.persistence.services.resilience; + +import java.util.Set; +import org.springframework.http.HttpStatus; + +final class RetryableHttpStatuses { + + private static final Set VALUES = Set.of(HttpStatus.REQUEST_TIMEOUT, + HttpStatus.TOO_MANY_REQUESTS, + HttpStatus.INTERNAL_SERVER_ERROR, + HttpStatus.BAD_GATEWAY, + HttpStatus.SERVICE_UNAVAILABLE, + HttpStatus.GATEWAY_TIMEOUT); + + private RetryableHttpStatuses() { + } + + static boolean contains(int statusCode) { + HttpStatus status = HttpStatus.resolve(statusCode); + return status != null && VALUES.contains(status); + } + +} diff --git a/multiapps-controller-persistence/src/test/java/org/cloudfoundry/multiapps/controller/persistence/services/resilience/AwsTransientErrorClassifierTest.java b/multiapps-controller-persistence/src/test/java/org/cloudfoundry/multiapps/controller/persistence/services/resilience/AwsTransientErrorClassifierTest.java new file mode 100644 index 0000000000..ff52d655e4 --- /dev/null +++ b/multiapps-controller-persistence/src/test/java/org/cloudfoundry/multiapps/controller/persistence/services/resilience/AwsTransientErrorClassifierTest.java @@ -0,0 +1,69 @@ +package org.cloudfoundry.multiapps.controller.persistence.services.resilience; + +import java.io.IOException; +import java.io.UncheckedIOException; +import java.util.stream.Stream; +import org.cloudfoundry.multiapps.common.SLException; +import org.junit.jupiter.api.Named; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; +import software.amazon.awssdk.core.exception.ApiCallAttemptTimeoutException; +import software.amazon.awssdk.core.exception.ApiCallTimeoutException; +import software.amazon.awssdk.core.exception.RetryableException; +import software.amazon.awssdk.core.exception.SdkException; +import software.amazon.awssdk.services.s3.model.S3Exception; +import static org.junit.jupiter.api.Assertions.assertEquals; + +class AwsTransientErrorClassifierTest { + + private final AwsTransientErrorClassifier classifier = new AwsTransientErrorClassifier(); + + @ParameterizedTest(name = "{0}") + @MethodSource("testCases") + void isRetryable(Throwable cause, boolean expectedRetryable) { + assertEquals(expectedRetryable, classifier.isRetryable(cause)); + } + + static Stream testCases() { + return Stream.of(Arguments.of(Named.of("IOException is retryable", new IOException("network failure")), true), + Arguments.of(Named.of("UncheckedIOException is retryable", new UncheckedIOException(new IOException("timeout"))), + true), + Arguments.of(Named.of("ApiCallTimeoutException is retryable", + ApiCallTimeoutException.builder() + .message("api call timeout") + .build()), + true), + Arguments.of(Named.of("ApiCallAttemptTimeoutException is retryable", + ApiCallAttemptTimeoutException.builder() + .message("attempt timeout") + .build()), + true), + Arguments.of(Named.of("S3Exception with 503 status is retryable", + S3Exception.builder() + .message("service unavailable") + .statusCode(503) + .build()), + true), + Arguments.of(Named.of("S3Exception with 404 status is not retryable", + S3Exception.builder() + .message("not found") + .statusCode(404) + .build()), + false), + Arguments.of(Named.of("RetryableException is retryable", + RetryableException.builder() + .message("retryable error") + .build()), + true), + Arguments.of(Named.of("Bare SdkException is not retryable", + SdkException.builder() + .message("unspecified failure") + .build()), + false), + Arguments.of(Named.of("IllegalArgumentException is not retryable", new IllegalArgumentException("invalid key")), + false), + Arguments.of(Named.of("SLException is not retryable", new SLException("Validation error")), false)); + } + +} diff --git a/multiapps-controller-persistence/src/test/java/org/cloudfoundry/multiapps/controller/persistence/services/resilience/AzureTransientErrorClassifierTest.java b/multiapps-controller-persistence/src/test/java/org/cloudfoundry/multiapps/controller/persistence/services/resilience/AzureTransientErrorClassifierTest.java new file mode 100644 index 0000000000..efa4fcacfc --- /dev/null +++ b/multiapps-controller-persistence/src/test/java/org/cloudfoundry/multiapps/controller/persistence/services/resilience/AzureTransientErrorClassifierTest.java @@ -0,0 +1,47 @@ +package org.cloudfoundry.multiapps.controller.persistence.services.resilience; + +import java.io.IOException; +import java.io.UncheckedIOException; +import java.util.stream.Stream; +import com.azure.storage.blob.models.BlobStorageException; +import org.junit.jupiter.api.Named; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +class AzureTransientErrorClassifierTest { + + private final AzureTransientErrorClassifier classifier = new AzureTransientErrorClassifier(); + + @ParameterizedTest(name = "{0}") + @MethodSource("testCases") + void isRetryable(Throwable cause, boolean expectedRetryable) { + assertEquals(expectedRetryable, classifier.isRetryable(cause)); + } + + static Stream testCases() { + return Stream.of(Arguments.of(Named.of("IOException is retryable", new IOException("network failure")), true), + Arguments.of(Named.of("UncheckedIOException is retryable", new UncheckedIOException(new IOException("timeout"))), + true), + Arguments.of(Named.of("BlobStorageException with 503 status is retryable", mockBlobStorageException(503)), true), + Arguments.of(Named.of("BlobStorageException with 429 status is retryable", mockBlobStorageException(429)), true), + Arguments.of(Named.of("BlobStorageException with 404 status is not retryable", mockBlobStorageException(404)), + false), + Arguments.of(Named.of("BlobStorageException with 200 status is not retryable", mockBlobStorageException(200)), + false), + Arguments.of(Named.of("IllegalArgumentException is not retryable", new IllegalArgumentException("invalid key")), + false), + Arguments.of(Named.of("RuntimeException is not retryable", new RuntimeException("unrelated")), false)); + } + + private static BlobStorageException mockBlobStorageException(int statusCode) { + BlobStorageException exception = mock(BlobStorageException.class); + when(exception.getStatusCode()) + .thenReturn(statusCode); + return exception; + } + +} diff --git a/multiapps-controller-persistence/src/test/java/org/cloudfoundry/multiapps/controller/persistence/services/resilience/GcpTransientErrorClassifierTest.java b/multiapps-controller-persistence/src/test/java/org/cloudfoundry/multiapps/controller/persistence/services/resilience/GcpTransientErrorClassifierTest.java new file mode 100644 index 0000000000..3894ef9248 --- /dev/null +++ b/multiapps-controller-persistence/src/test/java/org/cloudfoundry/multiapps/controller/persistence/services/resilience/GcpTransientErrorClassifierTest.java @@ -0,0 +1,55 @@ +package org.cloudfoundry.multiapps.controller.persistence.services.resilience; + +import java.io.IOException; +import java.io.UncheckedIOException; +import java.util.stream.Stream; +import com.google.cloud.storage.StorageException; +import org.junit.jupiter.api.Named; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +class GcpTransientErrorClassifierTest { + + private final GcpTransientErrorClassifier classifier = new GcpTransientErrorClassifier(); + + @ParameterizedTest(name = "{0}") + @MethodSource("testCases") + void isRetryable(Throwable cause, boolean expectedRetryable) { + assertEquals(expectedRetryable, classifier.isRetryable(cause)); + } + + static Stream testCases() { + return Stream.of(Arguments.of(Named.of("IOException is retryable", new IOException("network failure")), true), + Arguments.of(Named.of("UncheckedIOException is retryable", new UncheckedIOException(new IOException("timeout"))), + true), + Arguments.of(Named.of("StorageException flagged retryable with non-retryable status is retryable", + mockStorageException(200, true)), + true), + Arguments.of(Named.of("StorageException with 503 status not flagged retryable is retryable", + mockStorageException(503, false)), + true), + Arguments.of(Named.of("StorageException with 503 status and flagged retryable is retryable", + mockStorageException(503, true)), + true), + Arguments.of(Named.of("StorageException with 404 status not flagged retryable is not retryable", + mockStorageException(404, false)), + false), + Arguments.of(Named.of("StorageException with 200 status not flagged retryable is not retryable", + mockStorageException(200, false)), + false), + Arguments.of(Named.of("IllegalArgumentException is not retryable", new IllegalArgumentException("invalid key")), + false)); + } + + private static StorageException mockStorageException(int code, boolean retryable) { + StorageException exception = mock(StorageException.class); + when(exception.getCode()).thenReturn(code); + when(exception.isRetryable()).thenReturn(retryable); + return exception; + } + +} diff --git a/multiapps-controller-persistence/src/test/java/org/cloudfoundry/multiapps/controller/persistence/services/resilience/JCloudsTransientErrorClassifierTest.java b/multiapps-controller-persistence/src/test/java/org/cloudfoundry/multiapps/controller/persistence/services/resilience/JCloudsTransientErrorClassifierTest.java new file mode 100644 index 0000000000..f786de90fd --- /dev/null +++ b/multiapps-controller-persistence/src/test/java/org/cloudfoundry/multiapps/controller/persistence/services/resilience/JCloudsTransientErrorClassifierTest.java @@ -0,0 +1,60 @@ +package org.cloudfoundry.multiapps.controller.persistence.services.resilience; + +import java.io.IOException; +import java.io.UncheckedIOException; +import java.util.stream.Stream; +import org.jclouds.http.HttpResponse; +import org.jclouds.http.HttpResponseException; +import org.junit.jupiter.api.Named; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +class JCloudsTransientErrorClassifierTest { + + private final JCloudsTransientErrorClassifier classifier = new JCloudsTransientErrorClassifier(); + + @ParameterizedTest(name = "{0}") + @MethodSource("testCases") + void isRetryable(Throwable cause, boolean expectedRetryable) { + assertEquals(expectedRetryable, classifier.isRetryable(cause)); + } + + static Stream testCases() { + return Stream.of(Arguments.of(Named.of("IOException is retryable", new IOException("network failure")), true), + Arguments.of(Named.of("UncheckedIOException is retryable", new UncheckedIOException(new IOException("timeout"))), + true), + Arguments.of(Named.of("HttpResponseException with 503 status is retryable", mockHttpResponseException(503)), + true), + Arguments.of(Named.of("HttpResponseException with 429 status is retryable", mockHttpResponseException(429)), + true), + Arguments.of(Named.of("HttpResponseException with 404 status is not retryable", mockHttpResponseException(404)), + false), + Arguments.of(Named.of("HttpResponseException with 200 status is not retryable", mockHttpResponseException(200)), + false), + Arguments.of( + Named.of("HttpResponseException with null response is not retryable", mockHttpResponseException(null)), + false), + Arguments.of(Named.of("IllegalArgumentException is not retryable", new IllegalArgumentException("invalid key")), + false)); + } + + private static HttpResponseException mockHttpResponseException(Integer statusCode) { + HttpResponseException exception = mock(HttpResponseException.class); + if (statusCode == null) { + when(exception.getResponse()) + .thenReturn(null); + } else { + HttpResponse response = mock(HttpResponse.class); + when(response.getStatusCode()) + .thenReturn(statusCode); + when(exception.getResponse()) + .thenReturn(response); + } + return exception; + } + +} diff --git a/multiapps-controller-process/src/test/java/org/cloudfoundry/multiapps/controller/process/util/OperationInErrorStateHandlerTest.java b/multiapps-controller-process/src/test/java/org/cloudfoundry/multiapps/controller/process/util/OperationInErrorStateHandlerTest.java index 987b28d5f0..e913c76d47 100644 --- a/multiapps-controller-process/src/test/java/org/cloudfoundry/multiapps/controller/process/util/OperationInErrorStateHandlerTest.java +++ b/multiapps-controller-process/src/test/java/org/cloudfoundry/multiapps/controller/process/util/OperationInErrorStateHandlerTest.java @@ -1,6 +1,14 @@ package org.cloudfoundry.multiapps.controller.process.util; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; import java.io.IOException; import java.util.Collections; @@ -32,7 +40,6 @@ import org.junit.jupiter.api.Test; import org.mockito.Answers; import org.mockito.Mock; -import org.mockito.Mockito; import org.mockito.MockitoAnnotations; class OperationInErrorStateHandlerTest { @@ -61,7 +68,7 @@ class OperationInErrorStateHandlerTest { void setUp() throws Exception { MockitoAnnotations.openMocks(this) .close(); - Mockito.when(progressMessageServiceMock.createQuery()) + when(progressMessageServiceMock.createQuery()) .thenReturn(progressMessageQuery); prepareOperationService(); } @@ -84,11 +91,11 @@ void testToEventTypeWithContentException() { @Test void testWithErrorMessageAlreadyPersisted() { - Mockito.when(flowableFacadeMock.getProcessInstanceId(Mockito.any())) + when(flowableFacadeMock.getProcessInstanceId(any())) .thenReturn("foo"); ProgressMessageQuery queryMock = new MockBuilder<>(progressMessageQuery).on(query -> query.processId("foo")) .build(); - Mockito.doReturn(List.of(ImmutableProgressMessage.builder() + doReturn(List.of(ImmutableProgressMessage.builder() .processId("foo") .taskId("") .text("") @@ -96,13 +103,13 @@ void testWithErrorMessageAlreadyPersisted() { .build())) .when(queryMock) .list(); - FlowableEngineEvent event = Mockito.mock(FlowableEngineEvent.class); + FlowableEngineEvent event = mock(FlowableEngineEvent.class); OperationInErrorStateHandler handler = mockHandler(); handler.handle(event, new Exception("test-message")); - Mockito.verify(progressMessageServiceMock, Mockito.never()) - .add(Mockito.any()); + verify(progressMessageServiceMock, never()) + .add(any()); assertErrorStateSet(); } @@ -113,50 +120,50 @@ void testWithNoErrorMessagePersistedAndTaskIdFromFlowableEngine() { @Test void testWithNoErrorMessageAndTaskIdFromContext() { - Mockito.when(flowableFacadeMock.getCurrentTaskId("bar")) + when(flowableFacadeMock.getCurrentTaskId("bar")) .thenReturn("barbar"); testWithNoErrorMessageWithExecutionEntity(false); } private void testWithNoErrorMessageWithExecutionEntity(boolean shouldUseExecutionEntity) { - Mockito.when(flowableFacadeMock.getProcessInstanceId(Mockito.anyString())) + when(flowableFacadeMock.getProcessInstanceId(anyString())) .thenReturn("foo"); ProgressMessageQuery queryMock = new MockBuilder<>(progressMessageQuery).on(query -> query.processId("foo")) .build(); - Mockito.doReturn(Collections.emptyList()) + doReturn(Collections.emptyList()) .when(queryMock) .list(); - FlowableEngineEvent engineEvent = Mockito.mock(FlowableEngineEvent.class); - Mockito.when(engineEvent.getExecutionId()) + FlowableEngineEvent engineEvent = mock(FlowableEngineEvent.class); + when(engineEvent.getExecutionId()) .thenReturn("bar"); - Mockito.when(engineEvent.getProcessInstanceId()) + when(engineEvent.getProcessInstanceId()) .thenReturn("foo"); - Mockito.when(engineEvent.getProcessDefinitionId()) + when(engineEvent.getProcessDefinitionId()) .thenReturn("testing"); - RuntimeService runtimeServiceMock = Mockito.mock(RuntimeService.class); - ExecutionQuery executionQueryMock = Mockito.mock(ExecutionQuery.class); - Mockito.when(executionQueryMock.executionId("bar")) + RuntimeService runtimeServiceMock = mock(RuntimeService.class); + ExecutionQuery executionQueryMock = mock(ExecutionQuery.class); + when(executionQueryMock.executionId("bar")) .thenReturn(executionQueryMock); - Mockito.when(executionQueryMock.processInstanceId("foo")) + when(executionQueryMock.processInstanceId("foo")) .thenReturn(executionQueryMock); getExecutionEntityMock(shouldUseExecutionEntity, executionQueryMock); - Mockito.when(runtimeServiceMock.createExecutionQuery()) + when(runtimeServiceMock.createExecutionQuery()) .thenReturn(executionQueryMock); - Mockito.when(processEngineConfigurationMock.getRuntimeService()) + when(processEngineConfigurationMock.getRuntimeService()) .thenReturn(runtimeServiceMock); OperationInErrorStateHandler handler = mockHandler(); handler.handle(engineEvent, new Exception("test-message")); - Mockito.verify(progressMessageServiceMock, Mockito.times(1)) + verify(progressMessageServiceMock, times(1)) .add(ImmutableProgressMessage.builder() .processId("foo") .taskId("barbar") @@ -170,7 +177,7 @@ private void testWithNoErrorMessageWithExecutionEntity(boolean shouldUseExecutio private void getExecutionEntityMock(boolean shouldUseExecutionEntity, ExecutionQuery executionQueryMock) { ExecutionEntityImpl executionEntity = getExecutionEntity(shouldUseExecutionEntity); - Mockito.when(executionQueryMock.list()) + when(executionQueryMock.list()) .thenReturn(getList(executionEntity)); } @@ -192,11 +199,11 @@ private void prepareOperationService() { Operation operation = ImmutableOperation.builder() .state(Operation.State.RUNNING) .build(); - Mockito.when(operationService.createQuery()) + when(operationService.createQuery()) .thenReturn(operationQuery); - Mockito.when(operationQuery.processId(Mockito.anyString())) + when(operationQuery.processId(anyString())) .thenReturn(operationQuery); - Mockito.when(operationQuery.singleResult()) + when(operationQuery.singleResult()) .thenReturn(operation); } @@ -204,7 +211,7 @@ private void assertErrorStateSet() { Operation errorOperation = ImmutableOperation.builder() .state(Operation.State.ERROR) .build(); - Mockito.verify(operationService) + verify(operationService) .update(errorOperation, errorOperation); } diff --git a/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/Constants.java b/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/Constants.java index f7fecd8d0c..fa5c89b3a7 100644 --- a/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/Constants.java +++ b/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/Constants.java @@ -36,8 +36,6 @@ private Constants() { public static final String CONTAINER_NAME_WITH_DASH = "container-name"; public static final String BASE_64_ENCODED_PRIVATE_KEY_DATA = "base64EncodedPrivateKeyData"; public static final String HOST = "host"; - public static final Set OBJECT_STORE_CUSTOM_REGIONS = Set.of("eu-south-1"); - public static final String OBJECT_STORE_JCLOUDS_REGIONS = "jclouds.regions"; public static final String AWS_S_3 = "aws-s3"; public static final String AZUREBLOB = "azureblob"; diff --git a/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/configuration/DatabaseConfiguration.java b/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/configuration/DatabaseConfiguration.java index cdc2a659fa..060dd692d1 100644 --- a/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/configuration/DatabaseConfiguration.java +++ b/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/configuration/DatabaseConfiguration.java @@ -9,7 +9,7 @@ import org.cloudfoundry.multiapps.controller.persistence.dialects.DefaultDataSourceDialect; import org.cloudfoundry.multiapps.controller.persistence.util.DataSourceFactory; import org.cloudfoundry.multiapps.controller.persistence.util.EnvironmentServicesFinder; -import org.cloudfoundry.multiapps.controller.web.configuration.bean.factory.CloudDataSourceFactoryBean; +import org.cloudfoundry.multiapps.controller.web.configuration.bean.CloudDataSourceFactoryBean; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Primary; diff --git a/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/configuration/FileStorageConfiguration.java b/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/configuration/FileStorageConfiguration.java index 00d84b1e45..c892373baa 100644 --- a/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/configuration/FileStorageConfiguration.java +++ b/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/configuration/FileStorageConfiguration.java @@ -1,8 +1,11 @@ package org.cloudfoundry.multiapps.controller.web.configuration; import org.cloudfoundry.multiapps.controller.core.util.ApplicationConfiguration; +import org.cloudfoundry.multiapps.controller.persistence.services.FileStorage; +import org.cloudfoundry.multiapps.controller.persistence.services.resilience.NoRetryErrorClassifier; +import org.cloudfoundry.multiapps.controller.persistence.services.resilience.RetryableErrorClassifier; import org.cloudfoundry.multiapps.controller.persistence.util.EnvironmentServicesFinder; -import org.cloudfoundry.multiapps.controller.web.configuration.bean.factory.ObjectStoreFileStorageFactoryBean; +import org.cloudfoundry.multiapps.controller.web.configuration.factory.ObjectStoreSelectorFactory; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.web.multipart.MultipartResolver; @@ -14,9 +17,20 @@ public class FileStorageConfiguration { private static final String OBJECT_STORE_SERVICE_NAME = "deploy-service-os"; @Bean - public ObjectStoreFileStorageFactoryBean objectStoreFileStorage(EnvironmentServicesFinder vcapServiceFinder, - ApplicationConfiguration applicationConfiguration) { - return new ObjectStoreFileStorageFactoryBean(OBJECT_STORE_SERVICE_NAME, vcapServiceFinder, applicationConfiguration); + public ObjectStoreSelectorFactory objectStoreSelector(EnvironmentServicesFinder vcapServiceFinder, + ApplicationConfiguration applicationConfiguration) { + return new ObjectStoreSelectorFactory(OBJECT_STORE_SERVICE_NAME, vcapServiceFinder, applicationConfiguration); + } + + @Bean + public FileStorage fileStorage(ObjectStoreSelectorFactory selector) { + return selector.fileStorage(); + } + + @Bean + public RetryableErrorClassifier retryableErrorClassifier(ObjectStoreSelectorFactory selector) { + RetryableErrorClassifier classifier = selector.classifier(); + return classifier != null ? classifier : new NoRetryErrorClassifier(); } @Bean(name = "filterMultipartResolver") diff --git a/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/configuration/bean/factory/CloudDataSourceFactoryBean.java b/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/configuration/bean/CloudDataSourceFactoryBean.java similarity index 99% rename from multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/configuration/bean/factory/CloudDataSourceFactoryBean.java rename to multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/configuration/bean/CloudDataSourceFactoryBean.java index 3ae6c082f8..18da1f9d86 100644 --- a/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/configuration/bean/factory/CloudDataSourceFactoryBean.java +++ b/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/configuration/bean/CloudDataSourceFactoryBean.java @@ -1,4 +1,4 @@ -package org.cloudfoundry.multiapps.controller.web.configuration.bean.factory; +package org.cloudfoundry.multiapps.controller.web.configuration.bean; import javax.sql.DataSource; diff --git a/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/configuration/bean/factory/FileServiceFactoryBean.java b/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/configuration/bean/FileServiceFactoryBean.java similarity index 99% rename from multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/configuration/bean/factory/FileServiceFactoryBean.java rename to multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/configuration/bean/FileServiceFactoryBean.java index 40d7728836..92d53b0da2 100644 --- a/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/configuration/bean/factory/FileServiceFactoryBean.java +++ b/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/configuration/bean/FileServiceFactoryBean.java @@ -1,4 +1,4 @@ -package org.cloudfoundry.multiapps.controller.web.configuration.bean.factory; +package org.cloudfoundry.multiapps.controller.web.configuration.bean; import jakarta.inject.Inject; import jakarta.inject.Named; diff --git a/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/configuration/bean/factory/ObjectStoreFileStorageFactoryBean.java b/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/configuration/factory/ObjectStoreSelectorFactory.java similarity index 76% rename from multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/configuration/bean/factory/ObjectStoreFileStorageFactoryBean.java rename to multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/configuration/factory/ObjectStoreSelectorFactory.java index a9ed5e3354..117ed389ad 100644 --- a/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/configuration/bean/factory/ObjectStoreFileStorageFactoryBean.java +++ b/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/configuration/factory/ObjectStoreSelectorFactory.java @@ -1,4 +1,4 @@ -package org.cloudfoundry.multiapps.controller.web.configuration.bean.factory; +package org.cloudfoundry.multiapps.controller.web.configuration.factory; import java.text.MessageFormat; import java.util.Collections; @@ -6,11 +6,7 @@ import java.util.List; import java.util.Map; import java.util.Optional; -import java.util.Properties; -import java.util.Set; import java.util.stream.Collectors; -import java.util.stream.Stream; -import com.google.common.base.Joiner; import io.pivotal.cfenv.core.CfService; import org.apache.commons.lang3.StringUtils; import org.cloudfoundry.multiapps.controller.core.util.ApplicationConfiguration; @@ -20,6 +16,11 @@ import org.cloudfoundry.multiapps.controller.persistence.services.FileStorage; import org.cloudfoundry.multiapps.controller.persistence.services.GcpObjectStoreFileStorage; import org.cloudfoundry.multiapps.controller.persistence.services.JCloudsObjectStoreFileStorage; +import org.cloudfoundry.multiapps.controller.persistence.services.resilience.AwsTransientErrorClassifier; +import org.cloudfoundry.multiapps.controller.persistence.services.resilience.AzureTransientErrorClassifier; +import org.cloudfoundry.multiapps.controller.persistence.services.resilience.GcpTransientErrorClassifier; +import org.cloudfoundry.multiapps.controller.persistence.services.resilience.JCloudsTransientErrorClassifier; +import org.cloudfoundry.multiapps.controller.persistence.services.resilience.RetryableErrorClassifier; import org.cloudfoundry.multiapps.controller.persistence.util.EnvironmentServicesFinder; import org.cloudfoundry.multiapps.controller.web.Constants; import org.cloudfoundry.multiapps.controller.web.Messages; @@ -29,31 +30,33 @@ import org.jclouds.blobstore.BlobStoreContext; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import org.springframework.beans.factory.FactoryBean; -import org.springframework.beans.factory.InitializingBean; -public class ObjectStoreFileStorageFactoryBean implements FactoryBean, InitializingBean { +public class ObjectStoreSelectorFactory { - private static final Logger LOGGER = LoggerFactory.getLogger(ObjectStoreFileStorageFactoryBean.class); + private static final Logger LOGGER = LoggerFactory.getLogger(ObjectStoreSelectorFactory.class); private final String serviceName; private final EnvironmentServicesFinder environmentServicesFinder; private final ApplicationConfiguration applicationConfiguration; - private FileStorage objectStoreFileStorage; + private final SelectedObjectStore selectedObjectStore; - public ObjectStoreFileStorageFactoryBean(String serviceName, EnvironmentServicesFinder environmentServicesFinder, - ApplicationConfiguration applicationConfiguration) { + public ObjectStoreSelectorFactory(String serviceName, EnvironmentServicesFinder environmentServicesFinder, + ApplicationConfiguration applicationConfiguration) { this.serviceName = serviceName; this.environmentServicesFinder = environmentServicesFinder; this.applicationConfiguration = applicationConfiguration; + this.selectedObjectStore = doSelect(); } - @Override - public void afterPropertiesSet() { - this.objectStoreFileStorage = createObjectStoreFileStorage(); + public FileStorage fileStorage() { + return selectedObjectStore == null ? null : selectedObjectStore.fileStorage(); } - private FileStorage createObjectStoreFileStorage() { + public RetryableErrorClassifier classifier() { + return selectedObjectStore == null ? null : selectedObjectStore.classifier(); + } + + private SelectedObjectStore doSelect() { List providersServiceInfo = getProvidersServiceInfo(); if (LOGGER.isInfoEnabled()) { LOGGER.info(MessageFormat.format(Messages.OBJECT_STORE_PROVIDERS_DETECTED_0, providersServiceInfo.stream() @@ -71,10 +74,10 @@ private FileStorage createObjectStoreFileStorage() { if (!isObjectStoreEnvValid(objectStoreProviderName)) { return createObjectStoreFromFirstReachableProvider(exceptions, providersServiceInfo); } - Optional optionalFileStorage = createObjectStoreBasedOnProvider(objectStoreProviderName, providersServiceInfo, - exceptions); - if (optionalFileStorage.isPresent()) { - return optionalFileStorage.get(); + Optional optionalSelected = createObjectStoreBasedOnProvider(objectStoreProviderName, providersServiceInfo, + exceptions); + if (optionalSelected.isPresent()) { + return optionalSelected.get(); } throw buildNoValidObjectStoreException(exceptions); } @@ -101,10 +104,10 @@ private boolean isObjectStoreEnvValid(String objectStoreProviderName) { objectStoreProviderName); } - public FileStorage createObjectStoreFromFirstReachableProvider(Map exceptions, - List providersServiceInfo) { + private SelectedObjectStore createObjectStoreFromFirstReachableProvider(Map exceptions, + List providersServiceInfo) { for (ObjectStoreServiceInfo objectStoreServiceInfo : providersServiceInfo) { - Optional createdObjectStore = tryToCreateObjectStore(objectStoreServiceInfo, exceptions); + Optional createdObjectStore = tryToCreateObjectStore(objectStoreServiceInfo, exceptions); if (createdObjectStore.isPresent()) { return createdObjectStore.get(); } @@ -112,8 +115,8 @@ public FileStorage createObjectStoreFromFirstReachableProvider(Map tryToCreateObjectStore(ObjectStoreServiceInfo objectStoreServiceInfo, - Map exceptions) { + private Optional tryToCreateObjectStore(ObjectStoreServiceInfo objectStoreServiceInfo, + Map exceptions) { try { LOGGER.info(MessageFormat.format(Messages.ATTEMPTING_TO_CREATE_OBJECT_STORE_CLIENT, objectStoreServiceInfo.getProvider(), @@ -125,24 +128,28 @@ private Optional tryToCreateObjectStore(ObjectStoreServiceInfo obje .get(Constants.HOST), objectStoreServiceInfo.getCredentials() .get(Constants.ENDPOINT))); - FileStorage fileStorage = getFileStorageBasedOnProvider(objectStoreServiceInfo); - fileStorage.testConnection(); + SelectedObjectStore selectedStore = selectFor(objectStoreServiceInfo); + selectedStore.fileStorage() + .testConnection(); LOGGER.info(MessageFormat.format(Messages.OBJECT_STORE_WITH_PROVIDER_0_CREATED, objectStoreServiceInfo.getProvider())); - return Optional.of(fileStorage); + return Optional.of(selectedStore); } catch (Exception e) { exceptions.put(objectStoreServiceInfo.getProvider(), e); return Optional.empty(); } } - private FileStorage getFileStorageBasedOnProvider(ObjectStoreServiceInfo objectStoreServiceInfo) { + private SelectedObjectStore selectFor(ObjectStoreServiceInfo objectStoreServiceInfo) { return switch (objectStoreServiceInfo.getProvider()) { - case Constants.GOOGLE_CLOUD_STORAGE -> createGcpFileStorage(objectStoreServiceInfo); - case Constants.AZUREBLOB -> createAzureFileStorage(objectStoreServiceInfo); - case Constants.AWS_S_3 -> createAwsS3FileStorage(objectStoreServiceInfo); + case Constants.GOOGLE_CLOUD_STORAGE -> + new SelectedObjectStore(createGcpFileStorage(objectStoreServiceInfo), new GcpTransientErrorClassifier()); + case Constants.AZUREBLOB -> + new SelectedObjectStore(createAzureFileStorage(objectStoreServiceInfo), new AzureTransientErrorClassifier()); + case Constants.AWS_S_3 -> + new SelectedObjectStore(createAwsS3FileStorage(objectStoreServiceInfo), new AwsTransientErrorClassifier()); default -> { BlobStoreContext context = getBlobStoreContext(objectStoreServiceInfo); - yield createFileStorage(objectStoreServiceInfo, context); + yield new SelectedObjectStore(createFileStorage(objectStoreServiceInfo, context), new JCloudsTransientErrorClassifier()); } }; } @@ -162,7 +169,6 @@ protected AwsS3ObjectStoreFileStorage createAwsS3FileStorage(ObjectStoreServiceI private BlobStoreContext getBlobStoreContext(ObjectStoreServiceInfo serviceInfo) { ContextBuilder contextBuilder = ContextBuilder.newBuilder(serviceInfo.getProvider()); applyCredentials(serviceInfo, contextBuilder); - addCustomRegions(contextBuilder); resolveContextEndpoint(serviceInfo, contextBuilder); BlobStoreContext context = contextBuilder.buildView(BlobStoreContext.class); if (context == null) { @@ -181,16 +187,6 @@ private void applyCredentials(ObjectStoreServiceInfo serviceInfo, ContextBuilder contextBuilder.credentials(identity, credential); } - private void addCustomRegions(ContextBuilder contextBuilder) { - Properties properties = new Properties(); - Set mergedRegions = Stream.of(Constants.OBJECT_STORE_CUSTOM_REGIONS, applicationConfiguration.getObjectStoreRegions()) - .flatMap(Set::stream) - .collect(Collectors.toSet()); - properties.setProperty(Constants.OBJECT_STORE_JCLOUDS_REGIONS, Joiner.on(',') - .join(mergedRegions)); - contextBuilder.overrides(properties); - } - private void resolveContextEndpoint(ObjectStoreServiceInfo serviceInfo, ContextBuilder contextBuilder) { Map credentials = serviceInfo.getCredentials(); String endpoint = (String) credentials.get(Constants.ENDPOINT); @@ -210,9 +206,9 @@ protected JCloudsObjectStoreFileStorage createFileStorage(ObjectStoreServiceInfo .get(Constants.BUCKET)); } - private Optional createObjectStoreBasedOnProvider(String objectStoreProviderName, - List providersServiceInfo, - Map exceptions) { + private Optional createObjectStoreBasedOnProvider(String objectStoreProviderName, + List providersServiceInfo, + Map exceptions) { Optional objectStoreServiceInfoOptional = getAppropriateProvider(objectStoreProviderName, providersServiceInfo); if (objectStoreServiceInfoOptional.isEmpty()) { @@ -237,14 +233,7 @@ private IllegalStateException buildNoValidObjectStoreException(Map getObjectType() { - return FileStorage.class; + record SelectedObjectStore(FileStorage fileStorage, RetryableErrorClassifier classifier) { } } diff --git a/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/upload/AsyncUploadJobOrchestrator.java b/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/upload/AsyncUploadJobOrchestrator.java index faebc87071..7e3bfc51e2 100644 --- a/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/upload/AsyncUploadJobOrchestrator.java +++ b/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/upload/AsyncUploadJobOrchestrator.java @@ -55,8 +55,6 @@ public class AsyncUploadJobOrchestrator { private static final Logger LOGGER = LoggerFactory.getLogger(AsyncUploadJobOrchestrator.class); - private final FileUploadResilientOperationExecutor fileUploadResilientOperationExecutor = getFileUploadResilientOperationExecutor(); - private final ExecutorService asyncFileUploadExecutor; private final ExecutorService deployFromUrlExecutor; private final ApplicationConfiguration applicationConfiguration; @@ -64,12 +62,14 @@ public class AsyncUploadJobOrchestrator { private final FileService fileService; private final DescriptorParserFacadeFactory descriptorParserFactory; private final DeployFromUrlRemoteClient deployFromUrlRemoteClient; + private final FileUploadResilientOperationExecutor fileUploadResilientOperationExecutor; @Inject public AsyncUploadJobOrchestrator(ExecutorService asyncFileUploadExecutor, ExecutorService deployFromUrlExecutor, ApplicationConfiguration applicationConfiguration, AsyncUploadJobService asyncUploadJobService, FileService fileService, DescriptorParserFacadeFactory descriptorParserFactory, - DeployFromUrlRemoteClient deployFromUrlRemoteClient) { + DeployFromUrlRemoteClient deployFromUrlRemoteClient, + FileUploadResilientOperationExecutor fileUploadResilientOperationExecutor) { this.asyncFileUploadExecutor = asyncFileUploadExecutor; this.deployFromUrlExecutor = deployFromUrlExecutor; this.applicationConfiguration = applicationConfiguration; @@ -77,6 +77,7 @@ public AsyncUploadJobOrchestrator(ExecutorService asyncFileUploadExecutor, Execu this.fileService = fileService; this.descriptorParserFactory = descriptorParserFactory; this.deployFromUrlRemoteClient = deployFromUrlRemoteClient; + this.fileUploadResilientOperationExecutor = fileUploadResilientOperationExecutor; } public AsyncUploadJobEntry executeUploadFromUrl(String spaceGuid, String namespace, String urlWithoutUserInfo, String decodedUrl, @@ -286,8 +287,4 @@ private void updateFailedAsyncUploadJob(AsyncUploadJobEntry jobEntry, Exception } } - protected FileUploadResilientOperationExecutor getFileUploadResilientOperationExecutor() { - return new FileUploadResilientOperationExecutor(); - } - } diff --git a/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/upload/resilience/FileUploadResilientOperationExecutor.java b/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/upload/resilience/FileUploadResilientOperationExecutor.java index 1a0dd2223c..c7fbcd0623 100644 --- a/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/upload/resilience/FileUploadResilientOperationExecutor.java +++ b/multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/upload/resilience/FileUploadResilientOperationExecutor.java @@ -1,22 +1,18 @@ package org.cloudfoundry.multiapps.controller.web.upload.resilience; -import java.io.IOException; -import java.io.UncheckedIOException; import java.text.MessageFormat; import java.time.Duration; -import java.util.Set; +import jakarta.inject.Inject; +import jakarta.inject.Named; import org.cloudfoundry.multiapps.common.util.MiscUtil; import org.cloudfoundry.multiapps.controller.client.util.CheckedSupplier; import org.cloudfoundry.multiapps.controller.persistence.services.FileStorageException; +import org.cloudfoundry.multiapps.controller.persistence.services.resilience.RetryableErrorClassifier; import org.cloudfoundry.multiapps.controller.web.Messages; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import org.springframework.http.HttpStatus; -import software.amazon.awssdk.core.exception.ApiCallAttemptTimeoutException; -import software.amazon.awssdk.core.exception.ApiCallTimeoutException; -import software.amazon.awssdk.core.exception.SdkException; -import software.amazon.awssdk.core.exception.SdkServiceException; +@Named public class FileUploadResilientOperationExecutor { private static final Logger LOGGER = LoggerFactory.getLogger(FileUploadResilientOperationExecutor.class); @@ -27,12 +23,13 @@ public class FileUploadResilientOperationExecutor { .toMillis(); private static final long MAX_WAIT_TIME_IN_MILLIS = Duration.ofSeconds(300) .toMillis(); - private static final Set RETRYABLE_HTTP_STATUSES = Set.of(HttpStatus.REQUEST_TIMEOUT, - HttpStatus.TOO_MANY_REQUESTS, - HttpStatus.INTERNAL_SERVER_ERROR, - HttpStatus.BAD_GATEWAY, - HttpStatus.SERVICE_UNAVAILABLE, - HttpStatus.GATEWAY_TIMEOUT); + + private final RetryableErrorClassifier classifier; + + @Inject + public FileUploadResilientOperationExecutor(RetryableErrorClassifier classifier) { + this.classifier = classifier; + } public T execute(CheckedSupplier operation) throws Exception { Exception lastException = null; @@ -40,30 +37,27 @@ public T execute(CheckedSupplier operation) throws Exception { try { return operation.get(); } catch (Exception e) { - if (!isTransientError(e)) { + if (!isRetryableError(e)) { throw e; } lastException = e; - if (attempt == MAX_ATTEMPTS) { - LOGGER.error(MessageFormat.format(Messages.FILE_UPLOAD_ALL_ATTEMPTS_EXHAUSTED, MAX_ATTEMPTS, e.getMessage()), e); - throw e; + if (attempt < MAX_ATTEMPTS) { + long waitTime = calculateWaitTime(attempt); + LOGGER.warn(MessageFormat.format(Messages.FILE_UPLOAD_ATTEMPT_FAILED, attempt, MAX_ATTEMPTS, waitTime, e.getMessage()), + e); + sleep(waitTime); } - long waitTime = calculateWaitTime(attempt); - LOGGER.warn(MessageFormat.format(Messages.FILE_UPLOAD_ATTEMPT_FAILED, attempt, MAX_ATTEMPTS, waitTime, e.getMessage()), e); - sleep(waitTime); } } + LOGGER.error(MessageFormat.format(Messages.FILE_UPLOAD_ALL_ATTEMPTS_EXHAUSTED, MAX_ATTEMPTS, lastException.getMessage()), + lastException); throw lastException; } - protected void sleep(long millis) { - MiscUtil.sleep(millis); - } - - private boolean isTransientError(Exception e) { + private boolean isRetryableError(Exception e) { Throwable cause = e instanceof FileStorageException ? e.getCause() : e; while (cause != null) { - if (isTransientCause(cause)) { + if (classifier.isRetryable(cause)) { return true; } cause = cause.getCause(); @@ -71,27 +65,13 @@ private boolean isTransientError(Exception e) { return false; } - private boolean isTransientCause(Throwable cause) { - if (cause instanceof IOException || cause instanceof UncheckedIOException) { - return true; - } - if (cause instanceof ApiCallAttemptTimeoutException || cause instanceof ApiCallTimeoutException) { - return true; - } - if (cause instanceof SdkServiceException serviceException && isRetryableStatusCode(serviceException.statusCode())) { - return true; - } - return cause instanceof SdkException sdkException && sdkException.retryable(); - } - - private boolean isRetryableStatusCode(int statusCode) { - HttpStatus status = HttpStatus.resolve(statusCode); - return status != null && RETRYABLE_HTTP_STATUSES.contains(status); - } - private long calculateWaitTime(int attempt) { long waitTime = (long) (INITIAL_WAIT_TIME_IN_MILLIS * Math.pow(BACKOFF_FACTOR, (double) attempt - 1)); return Math.min(waitTime, MAX_WAIT_TIME_IN_MILLIS); } + protected void sleep(long millis) { + MiscUtil.sleep(millis); + } + } diff --git a/multiapps-controller-web/src/test/java/org/cloudfoundry/multiapps/controller/web/api/impl/FilesApiServiceImplTest.java b/multiapps-controller-web/src/test/java/org/cloudfoundry/multiapps/controller/web/api/impl/FilesApiServiceImplTest.java index 1860062893..f89d48eac2 100644 --- a/multiapps-controller-web/src/test/java/org/cloudfoundry/multiapps/controller/web/api/impl/FilesApiServiceImplTest.java +++ b/multiapps-controller-web/src/test/java/org/cloudfoundry/multiapps/controller/web/api/impl/FilesApiServiceImplTest.java @@ -39,7 +39,6 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.mockito.Mock; -import org.mockito.Mockito; import org.mockito.MockitoAnnotations; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; @@ -55,6 +54,8 @@ import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.RETURNS_SELF; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; class FilesApiServiceImplTest { @@ -92,7 +93,7 @@ class FilesApiServiceImplTest { private FilesApiServiceImpl testedClass; @BeforeEach - public void initialize() throws Exception { + void initialize() throws Exception { MockitoAnnotations.openMocks(this) .close(); testedClass = new FilesApiServiceImpl(fileService, uploadJobService, filesApiServiceAuditLog, asyncUploadJobOrchestrator, @@ -102,13 +103,13 @@ public void initialize() throws Exception { var token = new DefaultOAuth2User(Collections.emptyList(), Map.of("user_info", user), "user_info"); SecurityContextHolder.getContext() .setAuthentication(new OAuth2AuthenticationToken(token, Collections.emptyList(), "id")); - Mockito.when(request.getRequestURI()) + when(request.getRequestURI()) .thenReturn(""); prepareFileStorageThreadPool(); } @AfterEach - public void cleanup() { + void cleanup() { SecurityContextHolder.clearContext(); } @@ -125,7 +126,7 @@ private void prepareFileStorageThreadPool() { void testGetMtaFiles() throws Exception { FileEntry entryOne = createFileEntry("test.mtar"); FileEntry entryTwo = createFileEntry("extension.mtaext"); - Mockito.when(fileService.listFiles(Mockito.eq(SPACE_GUID), Mockito.eq(NAMESPACE))) + when(fileService.listFiles(SPACE_GUID, NAMESPACE)) .thenReturn(List.of(entryOne, entryTwo)); ResponseEntity> response = testedClass.getFiles(SPACE_GUID, NAMESPACE); assertEquals(HttpStatus.OK, response.getStatusCode()); @@ -159,7 +160,7 @@ private void assertMetadataMatches(FileEntry expected, FileMetadata actual) { @Test void testGetMtaFilesError() throws Exception { - Mockito.when(fileService.listFiles(Mockito.eq(SPACE_GUID), Mockito.eq(null))) + when(fileService.listFiles(SPACE_GUID, null)) .thenThrow(new FileStorageException("error")); assertThrows(SLException.class, () -> testedClass.getFiles(SPACE_GUID, null)); } @@ -171,16 +172,16 @@ void testUploadMtaFile() throws Exception { long fileSize = fileEntry.getSize() .longValue(); - Mockito.when(file.getSize()) + when(file.getSize()) .thenReturn(fileSize); - Mockito.when(file.getOriginalFilename()) + when(file.getOriginalFilename()) .thenReturn(fileName); - Mockito.when(file.getInputStream()) - .thenReturn(Mockito.mock(InputStream.class)); - Mockito.when(request.getFileMap()) + when(file.getInputStream()) + .thenReturn(mock(InputStream.class)); + when(request.getFileMap()) .thenReturn(Map.of("file", file)); - Mockito.when(fileService.addFile(Mockito.eq(ImmutableFileEntry.builder() + when(fileService.addFile(eq(ImmutableFileEntry.builder() .space(SPACE_GUID) .namespace(NAMESPACE) .name(fileName) @@ -190,10 +191,10 @@ void testUploadMtaFile() throws Exception { ResponseEntity response = testedClass.uploadFile(request, SPACE_GUID, NAMESPACE); - Mockito.verify(file) + verify(file) .getInputStream(); - Mockito.verify(fileService) - .addFile(Mockito.eq(ImmutableFileEntry.builder() + verify(fileService) + .addFile(eq(ImmutableFileEntry.builder() .space(SPACE_GUID) .namespace(NAMESPACE) .name(fileName) @@ -229,7 +230,7 @@ void testStartDeployFromUrl() { assertEquals("spaces/" + SPACE_GUID + "/files/jobs/" + expectedJobId, response.getHeaders() .getLocation() .toString()); - Mockito.verify(asyncUploadJobOrchestrator) + verify(asyncUploadJobOrchestrator) .executeUploadFromUrl(eq(SPACE_GUID), eq(NAMESPACE), any(String.class), any(String.class), eq(null)); } @@ -253,10 +254,10 @@ void testStartDeployFromUrlWhenExecutorRejectsJob() { assertEquals("30", response.getHeaders() .getFirst("Retry-After")); - Mockito.verify(asyncUploadJobOrchestrator) + verify(asyncUploadJobOrchestrator) .executeUploadFromUrl(eq(SPACE_GUID), eq(NAMESPACE), any(String.class), any(String.class), eq(null)); - Mockito.verify(asyncUploadJobsQuery) + verify(asyncUploadJobsQuery) .delete(); } @@ -303,11 +304,11 @@ void testStartDeployFromUrlWhenStuckJobExists() { .getLocation() .toString()); - Mockito.verify(asyncUploadJobsQuery) + verify(asyncUploadJobsQuery) .id(existingJobId); - Mockito.verify(asyncUploadJobsQuery) + verify(asyncUploadJobsQuery) .delete(); - Mockito.verify(asyncUploadJobOrchestrator) + verify(asyncUploadJobOrchestrator) .executeUploadFromUrl(eq(SPACE_GUID), eq(NAMESPACE), any(String.class), any(String.class), eq(null)); } @@ -340,11 +341,11 @@ void testStartDeployFromUrlWhenActiveJobExists() { .getLocation() .toString()); - Mockito.verify(asyncUploadJobsQuery, Mockito.never()) + verify(asyncUploadJobsQuery, never()) .id(any()); - Mockito.verify(asyncUploadJobsQuery, Mockito.never()) + verify(asyncUploadJobsQuery, never()) .delete(); - Mockito.verify(asyncUploadJobOrchestrator, Mockito.never()) + verify(asyncUploadJobOrchestrator, never()) .executeUploadFromUrl(any(), any(), any(), any(), any()); } @@ -358,13 +359,13 @@ void testGetUploadFromUrlJobWhenJobNotFound() { assertEquals(HttpStatus.NOT_FOUND, response.getStatusCode()); - Mockito.verify(asyncUploadJobsQuery) + verify(asyncUploadJobsQuery) .id("non-existent-job-id"); - Mockito.verify(asyncUploadJobsQuery) + verify(asyncUploadJobsQuery) .spaceGuid(SPACE_GUID); - Mockito.verify(asyncUploadJobsQuery) + verify(asyncUploadJobsQuery) .user("user1"); - Mockito.verify(asyncUploadJobsQuery) + verify(asyncUploadJobsQuery) .namespace(NAMESPACE); } diff --git a/multiapps-controller-web/src/test/java/org/cloudfoundry/multiapps/controller/web/configuration/bean/factory/CloudDataSourceFactoryBeanTest.java b/multiapps-controller-web/src/test/java/org/cloudfoundry/multiapps/controller/web/configuration/bean/factory/CloudDataSourceFactoryBeanTest.java index 678e124855..c1ff391a32 100644 --- a/multiapps-controller-web/src/test/java/org/cloudfoundry/multiapps/controller/web/configuration/bean/factory/CloudDataSourceFactoryBeanTest.java +++ b/multiapps-controller-web/src/test/java/org/cloudfoundry/multiapps/controller/web/configuration/bean/factory/CloudDataSourceFactoryBeanTest.java @@ -2,16 +2,18 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNull; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; import javax.sql.DataSource; import org.cloudfoundry.multiapps.controller.core.util.ApplicationConfiguration; import org.cloudfoundry.multiapps.controller.persistence.util.DataSourceFactory; import org.cloudfoundry.multiapps.controller.persistence.util.EnvironmentServicesFinder; +import org.cloudfoundry.multiapps.controller.web.configuration.bean.CloudDataSourceFactoryBean; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.mockito.Mock; -import org.mockito.Mockito; import org.mockito.MockitoAnnotations; import io.pivotal.cfenv.jdbc.CfJdbcService; @@ -33,7 +35,7 @@ class CloudDataSourceFactoryBeanTest { private CloudDataSourceFactoryBean testedFactory; @BeforeEach - public void setUp() throws Exception { + void setUp() throws Exception { MockitoAnnotations.openMocks(this) .close(); testedFactory = new CloudDataSourceFactoryBean(SERVICE_NAME, dataSourceFactory, vcapServiceFinder, configuration); @@ -41,16 +43,16 @@ public void setUp() throws Exception { @Test void testWhenServiceExists() { - CfJdbcService service = Mockito.mock(CfJdbcService.class); - Mockito.when(vcapServiceFinder.findJdbcService(SERVICE_NAME)) + CfJdbcService service = mock(CfJdbcService.class); + when(vcapServiceFinder.findJdbcService(SERVICE_NAME)) .thenReturn(service); - Mockito.when(configuration.getDbConnectionThreads()) + when(configuration.getDbConnectionThreads()) .thenReturn(DB_CONNECTION_THREADS); - Mockito.when(configuration.getApplicationGuid()) + when(configuration.getApplicationGuid()) .thenReturn("1"); - Mockito.when(configuration.getApplicationInstanceIndex()) + when(configuration.getApplicationInstanceIndex()) .thenReturn(0); - Mockito.when(dataSourceFactory.createDataSource(service, DB_CONNECTION_THREADS, "ds-1/0")) + when(dataSourceFactory.createDataSource(service, DB_CONNECTION_THREADS, "ds-1/0")) .thenReturn(dataSource, null, null); testedFactory.afterPropertiesSet(); diff --git a/multiapps-controller-web/src/test/java/org/cloudfoundry/multiapps/controller/web/configuration/bean/factory/ObjectStoreFileStorageFactoryBeanTest.java b/multiapps-controller-web/src/test/java/org/cloudfoundry/multiapps/controller/web/configuration/bean/factory/ObjectStoreSelectorFactoryTest.java similarity index 65% rename from multiapps-controller-web/src/test/java/org/cloudfoundry/multiapps/controller/web/configuration/bean/factory/ObjectStoreFileStorageFactoryBeanTest.java rename to multiapps-controller-web/src/test/java/org/cloudfoundry/multiapps/controller/web/configuration/bean/factory/ObjectStoreSelectorFactoryTest.java index 7cfdb49aab..b28570c678 100644 --- a/multiapps-controller-web/src/test/java/org/cloudfoundry/multiapps/controller/web/configuration/bean/factory/ObjectStoreFileStorageFactoryBeanTest.java +++ b/multiapps-controller-web/src/test/java/org/cloudfoundry/multiapps/controller/web/configuration/bean/factory/ObjectStoreSelectorFactoryTest.java @@ -5,12 +5,16 @@ import org.cloudfoundry.multiapps.controller.core.util.ApplicationConfiguration; import org.cloudfoundry.multiapps.controller.persistence.services.AwsS3ObjectStoreFileStorage; import org.cloudfoundry.multiapps.controller.persistence.services.AzureObjectStoreFileStorage; -import org.cloudfoundry.multiapps.controller.persistence.services.FileStorage; import org.cloudfoundry.multiapps.controller.persistence.services.GcpObjectStoreFileStorage; import org.cloudfoundry.multiapps.controller.persistence.services.JCloudsObjectStoreFileStorage; +import org.cloudfoundry.multiapps.controller.persistence.services.resilience.AwsTransientErrorClassifier; +import org.cloudfoundry.multiapps.controller.persistence.services.resilience.AzureTransientErrorClassifier; +import org.cloudfoundry.multiapps.controller.persistence.services.resilience.GcpTransientErrorClassifier; +import org.cloudfoundry.multiapps.controller.persistence.services.resilience.JCloudsTransientErrorClassifier; import org.cloudfoundry.multiapps.controller.persistence.util.EnvironmentServicesFinder; import org.cloudfoundry.multiapps.controller.web.Constants; import org.cloudfoundry.multiapps.controller.web.Messages; +import org.cloudfoundry.multiapps.controller.web.configuration.factory.ObjectStoreSelectorFactory; import org.cloudfoundry.multiapps.controller.web.configuration.service.ImmutableObjectStoreServiceInfo; import org.cloudfoundry.multiapps.controller.web.configuration.service.ObjectStoreServiceInfo; import org.cloudfoundry.multiapps.controller.web.configuration.service.ObjectStoreServiceInfoCreator; @@ -21,7 +25,6 @@ import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; import org.mockito.Mock; -import org.mockito.Mockito; import org.mockito.MockitoAnnotations; import java.util.HashMap; @@ -35,23 +38,19 @@ import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.mockito.ArgumentMatchers.anyList; -import static org.mockito.ArgumentMatchers.anyMap; -import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.doThrow; -import static org.mockito.Mockito.never; -import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; -class ObjectStoreFileStorageFactoryBeanTest { +class ObjectStoreSelectorFactoryTest { private static final String SERVICE_NAME = "deploy-service-os"; private static final String ACCESS_KEY_ID_VALUE = "access_key_id_value"; private static final String SECRET_ACCESS_KEY_VALUE = "secret_access_key_value"; private static final String BUCKET_VALUE = "bucket_value"; - private ObjectStoreFileStorageFactoryBean objectStoreFileStorageFactoryBean; + private static final ThreadLocal> providersOverride = new ThreadLocal<>(); @Mock private EnvironmentServicesFinder environmentServicesFinder; @@ -63,7 +62,6 @@ class ObjectStoreFileStorageFactoryBeanTest { private GcpObjectStoreFileStorage gcpObjectStoreFileStorage; @Mock private AwsS3ObjectStoreFileStorage awsS3ObjectStoreFileStorage; - @Mock private AzureObjectStoreFileStorage azureObjectStoreFileStorage; @@ -75,73 +73,63 @@ void setUp() throws Exception { MockitoAnnotations.openMocks(this) .close(); when(applicationConfiguration.getObjectStoreRegions()).thenReturn(Set.of()); - objectStoreFileStorageFactoryBean = new ObjectStoreFileStorageFactoryBeanMock(SERVICE_NAME, environmentServicesFinder, - applicationConfiguration); - } - - @Test - void testGetObjectType() { - assertEquals(FileStorage.class, objectStoreFileStorageFactoryBean.getObjectType()); - } - - @Test - void testGetObjectBeforeInitialization() { - assertNull(objectStoreFileStorageFactoryBean.getObject()); } @Test void testObjectStoreCreationWithoutServiceInstance() { - objectStoreFileStorageFactoryBean.afterPropertiesSet(); - assertNull(objectStoreFileStorageFactoryBean.getObject()); + ObjectStoreSelectorFactory selector = createSelector(); + + assertNull(selector.fileStorage()); + assertNull(selector.classifier()); } @Test void testObjectStoreCreationWithValidServiceInstance() { mockCfService(); - objectStoreFileStorageFactoryBean.afterPropertiesSet(); - assertNotNull(objectStoreFileStorageFactoryBean.getObject()); + + ObjectStoreSelectorFactory selector = createSelector(); + + assertNotNull(selector.fileStorage()); + assertNotNull(selector.classifier()); } @Test void testObjectStoreCreationWhenEnvIsValidForAws() { mockCfService(); when(applicationConfiguration.getObjectStoreClientType()).thenReturn(Constants.AWS); - var factoryBeanSpy = spy(objectStoreFileStorageFactoryBean); - factoryBeanSpy.afterPropertiesSet(); - var result = factoryBeanSpy.getObject(); + ObjectStoreSelectorFactory selector = createSelector(); - assertInstanceOf(AwsS3ObjectStoreFileStorage.class, result); - verify(factoryBeanSpy, never()).createObjectStoreFromFirstReachableProvider(anyMap(), anyList()); - verify(awsS3ObjectStoreFileStorage).testConnection(); + assertInstanceOf(AwsS3ObjectStoreFileStorage.class, selector.fileStorage()); + assertInstanceOf(AwsTransientErrorClassifier.class, selector.classifier()); + verify(awsS3ObjectStoreFileStorage) + .testConnection(); } @Test void testObjectStoreCreationWhenEnvIsValidForAzure() { mockCfService(); when(applicationConfiguration.getObjectStoreClientType()).thenReturn(Constants.AZURE); - var factoryBeanSpy = spy(objectStoreFileStorageFactoryBean); - factoryBeanSpy.afterPropertiesSet(); - var result = factoryBeanSpy.getObject(); + ObjectStoreSelectorFactory selector = createSelector(); - assertInstanceOf(AzureObjectStoreFileStorage.class, result); - verify(factoryBeanSpy, never()).createObjectStoreFromFirstReachableProvider(anyMap(), anyList()); - verify(azureObjectStoreFileStorage).testConnection(); + assertInstanceOf(AzureObjectStoreFileStorage.class, selector.fileStorage()); + assertInstanceOf(AzureTransientErrorClassifier.class, selector.classifier()); + verify(azureObjectStoreFileStorage) + .testConnection(); } @Test void testObjectStoreCreationWhenEnvIsValidForGcp() { mockCfService(); when(applicationConfiguration.getObjectStoreClientType()).thenReturn(Constants.GCP); - var factoryBeanSpy = spy(objectStoreFileStorageFactoryBean); - factoryBeanSpy.afterPropertiesSet(); - var result = factoryBeanSpy.getObject(); + ObjectStoreSelectorFactory selector = createSelector(); - assertInstanceOf(GcpObjectStoreFileStorage.class, result); - verify(factoryBeanSpy, never()).createObjectStoreFromFirstReachableProvider(anyMap(), anyList()); - verify(gcpObjectStoreFileStorage).testConnection(); + assertInstanceOf(GcpObjectStoreFileStorage.class, selector.fileStorage()); + assertInstanceOf(GcpTransientErrorClassifier.class, selector.classifier()); + verify(gcpObjectStoreFileStorage) + .testConnection(); } static Stream testObjectStoreCreationFallsBackToFirstReachableProviderWhenEnvIsInvalid() { @@ -162,13 +150,11 @@ static Stream testObjectStoreCreationFallsBackToFirstReachableProvide void testObjectStoreCreationFallsBackToFirstReachableProviderWhenEnvIsInvalid(String envValue) { mockCfService(); when(applicationConfiguration.getObjectStoreClientType()).thenReturn(envValue); - var factoryBeanSpy = spy(objectStoreFileStorageFactoryBean); - factoryBeanSpy.afterPropertiesSet(); - var result = factoryBeanSpy.getObject(); + ObjectStoreSelectorFactory selector = createSelector(); - assertInstanceOf(AwsS3ObjectStoreFileStorage.class, result); - verify(factoryBeanSpy).createObjectStoreFromFirstReachableProvider(anyMap(), anyList()); + assertInstanceOf(AwsS3ObjectStoreFileStorage.class, selector.fileStorage()); + assertInstanceOf(AwsTransientErrorClassifier.class, selector.classifier()); } @Test @@ -178,12 +164,12 @@ void testObjectStoreCreationFallsBackToNextProviderWhenFirstFails() { doThrow(new IllegalStateException("AWS connection failed")).when(awsS3ObjectStoreFileStorage) .testConnection(); - objectStoreFileStorageFactoryBean.afterPropertiesSet(); - var result = objectStoreFileStorageFactoryBean.getObject(); + ObjectStoreSelectorFactory selector = createSelector(); - // AWS (two entries) fails, then Aliyun (JClouds default path) succeeds - assertInstanceOf(JCloudsObjectStoreFileStorage.class, result); - verify(jCloudsObjectStoreFileStorage).testConnection(); + assertInstanceOf(JCloudsObjectStoreFileStorage.class, selector.fileStorage()); + assertInstanceOf(JCloudsTransientErrorClassifier.class, selector.classifier()); + verify(jCloudsObjectStoreFileStorage) + .testConnection(); } @Test @@ -191,7 +177,7 @@ void testGcpFactoryReceivesPopulatedCredentialsWhenEnvIsGcp() { mockCfService(); when(applicationConfiguration.getObjectStoreClientType()).thenReturn(Constants.GCP); - objectStoreFileStorageFactoryBean.afterPropertiesSet(); + createSelector(); assertNotNull(capturedGcpServiceInfo); assertEquals(Constants.GOOGLE_CLOUD_STORAGE, capturedGcpServiceInfo.getProvider()); @@ -208,7 +194,7 @@ void testAzureFactoryReceivesPopulatedCredentialsWhenEnvIsAzure() { mockCfService(); when(applicationConfiguration.getObjectStoreClientType()).thenReturn(Constants.AZURE); - objectStoreFileStorageFactoryBean.afterPropertiesSet(); + createSelector(); assertNotNull(capturedAzureServiceInfo); assertEquals(Constants.AZUREBLOB, capturedAzureServiceInfo.getProvider()); @@ -222,21 +208,14 @@ void testAzureFactoryReceivesPopulatedCredentialsWhenEnvIsAzure() { void testGcpEnvWithNoMatchingProviderInfoFallsThroughToNoValidStore() { mockCfService(); when(applicationConfiguration.getObjectStoreClientType()).thenReturn(Constants.GCP); - - ObjectStoreFileStorageFactoryBean bean = new ObjectStoreFileStorageFactoryBean("deploy-service-os", environmentServicesFinder, - applicationConfiguration) { - @Override - public List getProvidersServiceInfo() { - return List.of(ImmutableObjectStoreServiceInfo.builder() - .provider(Constants.AWS_S_3) - .credentials(buildCredentials()) - .build()); - } - }; - - Exception exception = assertThrows(IllegalStateException.class, bean::afterPropertiesSet); - assertEquals(Messages.NO_VALID_OBJECT_STORE_CONFIGURATION_FOUND, exception.getMessage()); - assertNull(capturedGcpServiceInfo, "GCP factory must not be invoked when no matching provider info is bound"); + providersOverride.set(List.of(buildServiceInfo(Constants.AWS_S_3))); + try { + Exception exception = assertThrows(IllegalStateException.class, this::createSelector); + assertEquals(Messages.NO_VALID_OBJECT_STORE_CONFIGURATION_FOUND, exception.getMessage()); + assertNull(capturedGcpServiceInfo, "GCP factory must not be invoked when no matching provider info is bound"); + } finally { + providersOverride.remove(); + } } @Test @@ -246,7 +225,7 @@ void testObjectStoreCreationWhenEnvProviderFailsToConnect() { doThrow(new IllegalStateException("Cannot create object store")).when(awsS3ObjectStoreFileStorage) .testConnection(); - var exception = assertThrows(IllegalStateException.class, () -> objectStoreFileStorageFactoryBean.afterPropertiesSet()); + var exception = assertThrows(IllegalStateException.class, this::createSelector); assertEquals(Messages.NO_VALID_OBJECT_STORE_CONFIGURATION_FOUND, exception.getMessage()); } @@ -254,12 +233,13 @@ void testObjectStoreCreationWhenEnvProviderFailsToConnect() { void testObjectStoreCreationWhenEnvProviderNotFoundInAvailableProviders() { mockCfService(); when(applicationConfiguration.getObjectStoreClientType()).thenReturn(Constants.GCP); - var factoryBeanSpy = spy(objectStoreFileStorageFactoryBean); - doReturn(List.of(buildServiceInfo(Constants.AWS_S_3))).when(factoryBeanSpy) - .getProvidersServiceInfo(); - - var exception = assertThrows(IllegalStateException.class, factoryBeanSpy::afterPropertiesSet); - assertEquals(Messages.NO_VALID_OBJECT_STORE_CONFIGURATION_FOUND, exception.getMessage()); + providersOverride.set(List.of(buildServiceInfo(Constants.AWS_S_3))); + try { + var exception = assertThrows(IllegalStateException.class, this::createSelector); + assertEquals(Messages.NO_VALID_OBJECT_STORE_CONFIGURATION_FOUND, exception.getMessage()); + } finally { + providersOverride.remove(); + } } @Test @@ -274,20 +254,17 @@ void testObjectStoreCreationWhenAllProvidersFail() { doThrow(new IllegalStateException("Cannot create object store")).when(azureObjectStoreFileStorage) .testConnection(); - var exception = assertThrows(IllegalStateException.class, () -> objectStoreFileStorageFactoryBean.afterPropertiesSet()); + var exception = assertThrows(IllegalStateException.class, this::createSelector); assertEquals(Messages.NO_VALID_OBJECT_STORE_CONFIGURATION_FOUND, exception.getMessage()); } - private ObjectStoreServiceInfo buildServiceInfo(String provider) { - return ImmutableObjectStoreServiceInfo.builder() - .provider(provider) - .credentials(buildCredentials()) - .build(); + private ObjectStoreSelectorFactory createSelector() { + return new ObjectStoreSelectorFactoryMock(SERVICE_NAME, environmentServicesFinder, applicationConfiguration); } private void mockCfService() { - CfService cfService = Mockito.mock(CfService.class); - CfCredentials cfCredentials = Mockito.mock(CfCredentials.class); + CfService cfService = mock(CfService.class); + CfCredentials cfCredentials = mock(CfCredentials.class); when(cfCredentials.getMap()).thenReturn(buildCredentials()); when(cfService.getCredentials()).thenReturn(cfCredentials); when(environmentServicesFinder.findService(SERVICE_NAME)).thenReturn(cfService); @@ -301,44 +278,56 @@ private static Map buildCredentials() { return credentials; } - private class ObjectStoreFileStorageFactoryBeanMock extends ObjectStoreFileStorageFactoryBean { + private ObjectStoreServiceInfo buildServiceInfo(String provider) { + return ImmutableObjectStoreServiceInfo.builder() + .provider(provider) + .credentials(buildCredentials()) + .build(); + } + + private class ObjectStoreSelectorFactoryMock extends ObjectStoreSelectorFactory { - public ObjectStoreFileStorageFactoryBeanMock(String serviceName, EnvironmentServicesFinder environmentServicesFinder, - ApplicationConfiguration applicationConfiguration) { + ObjectStoreSelectorFactoryMock(String serviceName, EnvironmentServicesFinder environmentServicesFinder, + ApplicationConfiguration applicationConfiguration) { super(serviceName, environmentServicesFinder, applicationConfiguration); } + @Override + public List getProvidersServiceInfo() { + List override = providersOverride.get(); + if (override != null) { + return override; + } + CfService service = environmentServicesFinder.findService(SERVICE_NAME); + if (service != null) { + return new ObjectStoreServiceInfoCreator().getAllProvidersServiceInfo(service.getCredentials() + .getMap()); + } + return List.of(); + } + @Override protected JCloudsObjectStoreFileStorage createFileStorage(ObjectStoreServiceInfo objectStoreServiceInfo, BlobStoreContext context) { - return ObjectStoreFileStorageFactoryBeanTest.this.jCloudsObjectStoreFileStorage; + return ObjectStoreSelectorFactoryTest.this.jCloudsObjectStoreFileStorage; } @Override - protected GcpObjectStoreFileStorage createGcpFileStorage(ObjectStoreServiceInfo credentials) { - ObjectStoreFileStorageFactoryBeanTest.this.capturedGcpServiceInfo = credentials; - return ObjectStoreFileStorageFactoryBeanTest.this.gcpObjectStoreFileStorage; + protected GcpObjectStoreFileStorage createGcpFileStorage(ObjectStoreServiceInfo objectStoreServiceInfo) { + ObjectStoreSelectorFactoryTest.this.capturedGcpServiceInfo = objectStoreServiceInfo; + return ObjectStoreSelectorFactoryTest.this.gcpObjectStoreFileStorage; } @Override protected AzureObjectStoreFileStorage createAzureFileStorage(ObjectStoreServiceInfo objectStoreServiceInfo) { - ObjectStoreFileStorageFactoryBeanTest.this.capturedAzureServiceInfo = objectStoreServiceInfo; - return ObjectStoreFileStorageFactoryBeanTest.this.azureObjectStoreFileStorage; + ObjectStoreSelectorFactoryTest.this.capturedAzureServiceInfo = objectStoreServiceInfo; + return ObjectStoreSelectorFactoryTest.this.azureObjectStoreFileStorage; } @Override protected AwsS3ObjectStoreFileStorage createAwsS3FileStorage(ObjectStoreServiceInfo objectStoreServiceInfo) { - return ObjectStoreFileStorageFactoryBeanTest.this.awsS3ObjectStoreFileStorage; + return ObjectStoreSelectorFactoryTest.this.awsS3ObjectStoreFileStorage; } - @Override - public List getProvidersServiceInfo() { - CfService service = environmentServicesFinder.findService(SERVICE_NAME); - if (service != null) { - return new ObjectStoreServiceInfoCreator().getAllProvidersServiceInfo(service.getCredentials() - .getMap()); - } - return List.of(); - } } } diff --git a/multiapps-controller-web/src/test/java/org/cloudfoundry/multiapps/controller/web/configuration/service/ObjectStoreServiceInfoCreatorTest.java b/multiapps-controller-web/src/test/java/org/cloudfoundry/multiapps/controller/web/configuration/service/ObjectStoreServiceInfoCreatorTest.java index fc2b5533cf..38f18c3fb4 100644 --- a/multiapps-controller-web/src/test/java/org/cloudfoundry/multiapps/controller/web/configuration/service/ObjectStoreServiceInfoCreatorTest.java +++ b/multiapps-controller-web/src/test/java/org/cloudfoundry/multiapps/controller/web/configuration/service/ObjectStoreServiceInfoCreatorTest.java @@ -12,9 +12,9 @@ import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; -import org.mockito.Mockito; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; class ObjectStoreServiceInfoCreatorTest { @@ -54,8 +54,8 @@ void testDifferentProviders(CfService cfService, ObjectStoreServiceInfo exprecte } private static CfService buildCfService(Map credentials) { - CfService cfService = Mockito.mock(CfService.class); - CfCredentials cfCredentials = Mockito.mock(CfCredentials.class); + CfService cfService = mock(CfService.class); + CfCredentials cfCredentials = mock(CfCredentials.class); when(cfCredentials.getMap()).thenReturn(credentials); when(cfService.getCredentials()).thenReturn(cfCredentials); return cfService; diff --git a/multiapps-controller-web/src/test/java/org/cloudfoundry/multiapps/controller/web/upload/AsyncUploadJobOrchestratorTest.java b/multiapps-controller-web/src/test/java/org/cloudfoundry/multiapps/controller/web/upload/AsyncUploadJobOrchestratorTest.java index 89041565da..73c00cc562 100644 --- a/multiapps-controller-web/src/test/java/org/cloudfoundry/multiapps/controller/web/upload/AsyncUploadJobOrchestratorTest.java +++ b/multiapps-controller-web/src/test/java/org/cloudfoundry/multiapps/controller/web/upload/AsyncUploadJobOrchestratorTest.java @@ -10,7 +10,6 @@ import java.util.concurrent.atomic.AtomicBoolean; import org.cloudfoundry.multiapps.common.SLException; import org.cloudfoundry.multiapps.controller.api.model.UserCredentials; -import org.cloudfoundry.multiapps.controller.client.util.CheckedSupplier; import org.cloudfoundry.multiapps.controller.core.helpers.DescriptorParserFacadeFactory; import org.cloudfoundry.multiapps.controller.core.util.ApplicationConfiguration; import org.cloudfoundry.multiapps.controller.persistence.model.AsyncUploadJobEntry; @@ -122,7 +121,8 @@ void setUp() throws Exception { asyncUploadJobService, fileService, descriptorParserFactory, - deployFromUrlRemoteClient + deployFromUrlRemoteClient, + noRetryExecutor() ); when(applicationConfiguration.getApplicationInstanceIndex()).thenReturn(0); @@ -431,30 +431,31 @@ private void setupSuccessfulRemoteClientMocks() throws Exception { when(deployFromUrlRemoteClient.downloadFileFromUrl(any())).thenReturn(fileFromUrlData); } + private static FileUploadResilientOperationExecutor noRetryExecutor() { + return new FileUploadResilientOperationExecutor(cause -> false) { + @Override + protected void sleep(long millis) { + // intentionally empty — tests must not block on retry back-off delays + } + }; + } + private static class TestableAsyncUploadJobOrchestrator extends AsyncUploadJobOrchestrator { public TestableAsyncUploadJobOrchestrator(ExecutorService asyncFileUploadExecutor, ExecutorService deployFromUrlExecutor, ApplicationConfiguration applicationConfiguration, AsyncUploadJobService asyncUploadJobService, FileService fileService, DescriptorParserFacadeFactory descriptorParserFactory, - DeployFromUrlRemoteClient deployFromUrlRemoteClient) { + DeployFromUrlRemoteClient deployFromUrlRemoteClient, + FileUploadResilientOperationExecutor fileUploadResilientOperationExecutor) { super(asyncFileUploadExecutor, deployFromUrlExecutor, applicationConfiguration, - asyncUploadJobService, fileService, descriptorParserFactory, deployFromUrlRemoteClient); + asyncUploadJobService, fileService, descriptorParserFactory, deployFromUrlRemoteClient, + fileUploadResilientOperationExecutor); } @Override protected void waitBetweenUpdates() { - // do nothing - } - - @Override - protected FileUploadResilientOperationExecutor getFileUploadResilientOperationExecutor() { - return new FileUploadResilientOperationExecutor() { - @Override - public T execute(CheckedSupplier operation) throws Exception { - return operation.get(); - } - }; + // intentionally empty — tests must not sleep between polling iterations } } } diff --git a/multiapps-controller-web/src/test/java/org/cloudfoundry/multiapps/controller/web/upload/FileUploadResilientOperationExecutorTest.java b/multiapps-controller-web/src/test/java/org/cloudfoundry/multiapps/controller/web/upload/FileUploadResilientOperationExecutorTest.java index 728511e105..47aee5f3cf 100644 --- a/multiapps-controller-web/src/test/java/org/cloudfoundry/multiapps/controller/web/upload/FileUploadResilientOperationExecutorTest.java +++ b/multiapps-controller-web/src/test/java/org/cloudfoundry/multiapps/controller/web/upload/FileUploadResilientOperationExecutorTest.java @@ -1,35 +1,34 @@ package org.cloudfoundry.multiapps.controller.web.upload; -import org.cloudfoundry.multiapps.common.SLException; -import org.cloudfoundry.multiapps.controller.persistence.services.FileStorageException; -import org.cloudfoundry.multiapps.controller.web.upload.resilience.FileUploadResilientOperationExecutor; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; -import software.amazon.awssdk.core.exception.ApiCallAttemptTimeoutException; -import software.amazon.awssdk.core.exception.ApiCallTimeoutException; -import software.amazon.awssdk.core.exception.RetryableException; -import software.amazon.awssdk.core.exception.SdkException; -import software.amazon.awssdk.services.s3.model.S3Exception; - import java.io.IOException; import java.io.UncheckedIOException; import java.util.ArrayList; import java.util.List; import java.util.concurrent.atomic.AtomicInteger; +import org.cloudfoundry.multiapps.common.SLException; +import org.cloudfoundry.multiapps.controller.persistence.services.FileStorageException; +import org.cloudfoundry.multiapps.controller.persistence.services.resilience.RetryableErrorClassifier; +import org.cloudfoundry.multiapps.controller.web.upload.resilience.FileUploadResilientOperationExecutor; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; class FileUploadResilientOperationExecutorTest { + private static final RetryableErrorClassifier IO_RETRYABLE_CLASSIFIER = + cause -> cause instanceof IOException || cause instanceof UncheckedIOException; + private List recordedSleepDurations; private FileUploadResilientOperationExecutor executor; @BeforeEach void setUp() { recordedSleepDurations = new ArrayList<>(); - executor = new FileUploadResilientOperationExecutor() { + executor = new FileUploadResilientOperationExecutor(IO_RETRYABLE_CLASSIFIER) { @Override protected void sleep(long millis) { recordedSleepDurations.add(millis); @@ -46,7 +45,7 @@ void testSuccessfulFirstAttempt() throws Exception { } @Test - void testRetryOnFileStorageExceptionWithIOExceptionCause() throws Exception { + void testRetryOnTransientCauseInsideFileStorageException() throws Exception { var attempts = new AtomicInteger(0); var result = executor.execute(() -> { @@ -63,15 +62,12 @@ void testRetryOnFileStorageExceptionWithIOExceptionCause() throws Exception { } @Test - void testNoRetryOnFileStorageExceptionWithBareSdkException() { + void testNoRetryOnNonTransientCauseInsideFileStorageException() { var attempts = new AtomicInteger(0); - // A bare SdkException with no status code, no retryable cause, and no retryable subtype. assertThrows(FileStorageException.class, () -> executor.execute(() -> { attempts.incrementAndGet(); - throw new FileStorageException("S3 error", SdkException.builder() - .message("unspecified failure") - .build()); + throw new FileStorageException("Bad request", new IllegalArgumentException("invalid key")); })); assertEquals(1, attempts.get()); @@ -79,67 +75,12 @@ void testNoRetryOnFileStorageExceptionWithBareSdkException() { } @Test - void testRetryOnApiCallAttemptTimeoutException() throws Exception { - var attempts = new AtomicInteger(0); - - var result = executor.execute(() -> { - if (attempts.incrementAndGet() < 2) { - throw new FileStorageException("attempt timed out", ApiCallAttemptTimeoutException.builder() - .message("attempt timeout") - .build()); - } - return "recovered"; - }); - - assertEquals("recovered", result); - assertEquals(1, recordedSleepDurations.size()); - } - - @Test - void testRetryOnApiCallTimeoutException() throws Exception { - var attempts = new AtomicInteger(0); - - var result = executor.execute(() -> { - if (attempts.incrementAndGet() < 2) { - throw new FileStorageException("api call timed out", ApiCallTimeoutException.builder() - .message("api call timeout") - .build()); - } - return "recovered"; - }); - - assertEquals("recovered", result); - assertEquals(1, recordedSleepDurations.size()); - } - - @Test - void testRetryOnS3ExceptionWithRetryableStatusCode() throws Exception { - var attempts = new AtomicInteger(0); - - var result = executor.execute(() -> { - if (attempts.incrementAndGet() < 2) { - throw new FileStorageException("transient s3 error", S3Exception.builder() - .message("service unavailable") - .statusCode(503) - .build()); - } - return "recovered"; - }); - - assertEquals("recovered", result); - assertEquals(1, recordedSleepDurations.size()); - } - - @Test - void testNoRetryOnS3ExceptionWithPermanentStatusCode() { + void testNoRetryOnFileStorageExceptionWithoutCause() { var attempts = new AtomicInteger(0); assertThrows(FileStorageException.class, () -> executor.execute(() -> { attempts.incrementAndGet(); - throw new FileStorageException("permanent s3 error", S3Exception.builder() - .message("not found") - .statusCode(404) - .build()); + throw new FileStorageException("no cause"); })); assertEquals(1, attempts.get()); @@ -147,58 +88,17 @@ void testNoRetryOnS3ExceptionWithPermanentStatusCode() { } @Test - void testRetryOnRetryableException() throws Exception { - var attempts = new AtomicInteger(0); - - var result = executor.execute(() -> { - if (attempts.incrementAndGet() < 2) { - throw new FileStorageException("retryable", RetryableException.builder() - .message("retryable error") - .build()); - } - return "recovered"; - }); - - assertEquals("recovered", result); - assertEquals(1, recordedSleepDurations.size()); - } - - @Test - void testRetryOnFileStorageExceptionWithSdkExceptionWrappingIOException() throws Exception { - var attempts = new AtomicInteger(0); - - var result = executor.execute(() -> { - if (attempts.incrementAndGet() < 2) { - // Mirrors what AWS SDK v2 does: SdkClientException wrapping a network IOException - var networkError = new IOException("Connection reset by peer"); - var sdkException = SdkException.builder() - .message("network error") - .cause(networkError) - .build(); - throw new FileStorageException("S3 upload failed", sdkException); - } - return "recovered"; - }); - - assertEquals("recovered", result); - assertEquals(1, recordedSleepDurations.size()); - assertEquals(10000L, recordedSleepDurations.get(0)); - } - - @Test - void testRetryOnFileStorageExceptionWithUncheckedIOExceptionCause() throws Exception { + void testNoRetryOnSLException() { var attempts = new AtomicInteger(0); - var result = executor.execute(() -> { - if (attempts.incrementAndGet() < 2) { - throw new FileStorageException("Wrapped IO", new UncheckedIOException(new IOException("broken pipe"))); - } - return "recovered"; - }); + var exception = assertThrows(SLException.class, () -> executor.execute(() -> { + attempts.incrementAndGet(); + throw new SLException("Validation error"); + })); - assertEquals("recovered", result); - assertEquals(1, recordedSleepDurations.size()); - assertEquals(10000L, recordedSleepDurations.get(0)); + assertEquals(1, attempts.get()); + assertEquals("Validation error", exception.getMessage()); + assertTrue(recordedSleepDurations.isEmpty()); } @Test @@ -217,49 +117,6 @@ void testRetryOnDirectIOException() throws Exception { assertEquals(10000L, recordedSleepDurations.get(0)); } - @Test - void testRetryOnDirectUncheckedIOException() throws Exception { - var attempts = new AtomicInteger(0); - - var result = executor.execute(() -> { - if (attempts.incrementAndGet() < 2) { - throw new UncheckedIOException(new IOException("timeout")); - } - return "recovered"; - }); - - assertEquals("recovered", result); - assertEquals(1, recordedSleepDurations.size()); - assertEquals(10000L, recordedSleepDurations.get(0)); - } - - @Test - void testNoRetryOnSLException() { - var attempts = new AtomicInteger(0); - - var exception = assertThrows(SLException.class, () -> executor.execute(() -> { - attempts.incrementAndGet(); - throw new SLException("Validation error"); - })); - - assertEquals(1, attempts.get()); - assertEquals("Validation error", exception.getMessage()); - assertTrue(recordedSleepDurations.isEmpty()); - } - - @Test - void testNoRetryOnFileStorageExceptionWithNonTransientCause() { - var attempts = new AtomicInteger(0); - - assertThrows(FileStorageException.class, () -> executor.execute(() -> { - attempts.incrementAndGet(); - throw new FileStorageException("Bad request", new IllegalArgumentException("invalid key")); - })); - - assertEquals(1, attempts.get()); - assertTrue(recordedSleepDurations.isEmpty()); - } - @Test void testExponentialBackoffTiming() throws Exception { var attempts = new AtomicInteger(0);