From da29efc4e966f31be2067690780d37c546a55090 Mon Sep 17 00:00:00 2001 From: Leopold Cramer Date: Wed, 2 Apr 2025 11:03:56 +0200 Subject: [PATCH 1/5] save workspaceFiles in s3 service instead of localStorage changed local storage saving to s3 saving added s3 tests capabilities added integration tests for mentionned changes --- .../helm-chart/templates/_helpers.tpl | 9 -- .../helm-chart/templates/deployment.yaml | 10 -- api/kubernetes/helm-chart/templates/pvc.yaml | 21 ---- api/kubernetes/helm-chart/values.yaml | 5 + .../api/exceptions/AwsExceptionHandling.kt | 31 ++++++ api/src/main/resources/application.yml | 20 +++- build.gradle.kts | 7 +- config/application-dev.sample.yml | 14 +++ .../resources/application-connector-test.yml | 7 +- .../resources/application-dataset-test.yml | 7 +- .../application-organization-test.yml | 7 +- .../resources/application-run-test.yml | 7 +- .../resources/application-runner-test.yml | 7 +- .../resources/application-solution-test.yml | 7 +- workspace/build.gradle.kts | 3 +- .../workspace/service/CsmS3TestBase.kt | 88 +++++++++++++++ .../WorkspaceServiceIntegrationTest.kt | 100 +++++++++++++++--- .../service/WorkspaceServiceRBACTest.kt | 32 ++---- .../resources/application-workspace-test.yml | 11 +- .../resources/test_workspace_file.txt | 1 + .../workspace/service/WorkspaceServiceImpl.kt | 90 ++++++++-------- .../workspace/utils/WorkspaceUtils.kt | 23 ---- .../service/WorkspaceServiceImplTests.kt | 10 +- 23 files changed, 349 insertions(+), 168 deletions(-) delete mode 100644 api/kubernetes/helm-chart/templates/pvc.yaml create mode 100644 api/src/main/kotlin/com/cosmotech/api/exceptions/AwsExceptionHandling.kt create mode 100644 workspace/src/integrationTest/kotlin/com/cosmotech/workspace/service/CsmS3TestBase.kt create mode 100644 workspace/src/integrationTest/resources/test_workspace_file.txt delete mode 100644 workspace/src/main/kotlin/com/cosmotech/workspace/utils/WorkspaceUtils.kt diff --git a/api/kubernetes/helm-chart/templates/_helpers.tpl b/api/kubernetes/helm-chart/templates/_helpers.tpl index 35202ed8e..598526175 100644 --- a/api/kubernetes/helm-chart/templates/_helpers.tpl +++ b/api/kubernetes/helm-chart/templates/_helpers.tpl @@ -108,13 +108,6 @@ E.g: {{- end }} {{- end }} -{{/* -Location of the persistence data -*/}} -{{- define "cosmotech-api.blobPersistencePath" -}} -"/var/lib/cosmotech-api/data" -{{- end }} - {{- define "cosmotech-api.custom-rootca-path" -}} /mnt/cosmotech/certificates {{- end }} @@ -197,8 +190,6 @@ csm: {{- else }} image-pull-secrets: [] {{- end }} - blobPersistence: - path: {{ include "cosmotech-api.blobPersistencePath" . }} identityProvider: tls: bundle: {{ include "cosmotech-api.custom-rootca-bundle" . }} diff --git a/api/kubernetes/helm-chart/templates/deployment.yaml b/api/kubernetes/helm-chart/templates/deployment.yaml index 16f2ed867..c6b8925c9 100644 --- a/api/kubernetes/helm-chart/templates/deployment.yaml +++ b/api/kubernetes/helm-chart/templates/deployment.yaml @@ -46,11 +46,6 @@ spec: - name: helm-config secret: secretName: {{ include "cosmotech-api.fullname" . }} - {{if .Values.persistence.enabled}} - - name: blob-storage - persistentVolumeClaim: - claimName: {{ include "cosmotech-api.fullname" . }}-blob-storage - {{end}} {{if .Values.api.tlsTruststore.enabled}} - name: custom-rootca secret: @@ -90,11 +85,6 @@ spec: - mountPath: /config name: helm-config readOnly: true - {{if .Values.persistence.enabled}} - - mountPath: {{ include "cosmotech-api.blobPersistencePath" . }} - name: blob-storage - readOnly: false - {{end}} {{if .Values.api.tlsTruststore.enabled }} - mountPath: {{ include "cosmotech-api.custom-rootca-path" . | quote }} name: custom-rootca diff --git a/api/kubernetes/helm-chart/templates/pvc.yaml b/api/kubernetes/helm-chart/templates/pvc.yaml deleted file mode 100644 index 545e91b7e..000000000 --- a/api/kubernetes/helm-chart/templates/pvc.yaml +++ /dev/null @@ -1,21 +0,0 @@ -{{if .Values.persistence.enabled}} -apiVersion: v1 -kind: PersistentVolumeClaim -metadata: - name: {{ include "cosmotech-api.fullname" . }}-blob-storage - namespace: {{ .Release.Namespace }} - labels: - {{- include "cosmotech-api.labels" . | nindent 4 }} - annotations: - email: "platform@cosmotech.com" -spec: - accessModes: - - ReadWriteMany - volumeMode: Filesystem - resources: - requests: - storage: {{ .Values.persistence.size }} - {{- if .Values.persistence.storageClass }} - storageClassName: {{ .Values.persistence.storageClass }} - {{- end }} -{{end}} diff --git a/api/kubernetes/helm-chart/values.yaml b/api/kubernetes/helm-chart/values.yaml index cf120b29c..1c952e720 100644 --- a/api/kubernetes/helm-chart/values.yaml +++ b/api/kubernetes/helm-chart/values.yaml @@ -192,6 +192,11 @@ config: requests: # File storage minimal claim is 100Gi for Premium classes storage: 100Gi + s3: + endpointUrl: "http://s3-server:9000" + bucketName: "changeme" + accessKeyId: "changeme" + secretAccessKey: "changeme" authorization: allowed-tenants: [] identityProvider: diff --git a/api/src/main/kotlin/com/cosmotech/api/exceptions/AwsExceptionHandling.kt b/api/src/main/kotlin/com/cosmotech/api/exceptions/AwsExceptionHandling.kt new file mode 100644 index 000000000..cea7a34fc --- /dev/null +++ b/api/src/main/kotlin/com/cosmotech/api/exceptions/AwsExceptionHandling.kt @@ -0,0 +1,31 @@ +// Copyright (c) Cosmo Tech. +// Licensed under the MIT license. +package com.cosmotech.api.exceptions + +import java.net.URI +import org.springframework.core.Ordered +import org.springframework.core.annotation.Order +import org.springframework.http.HttpStatus +import org.springframework.http.ProblemDetail +import org.springframework.web.bind.annotation.ExceptionHandler +import org.springframework.web.bind.annotation.RestControllerAdvice +import software.amazon.awssdk.awscore.exception.AwsServiceException + +@RestControllerAdvice +@Order(Ordered.HIGHEST_PRECEDENCE) +internal class AwsExceptionHandling : CsmExceptionHandling() { + private val httpStatusCodeTypePrefix = "https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/" + + @ExceptionHandler + fun handleBlobStorageException(exception: AwsServiceException): ProblemDetail { + val status = HttpStatus.INTERNAL_SERVER_ERROR + + val problemDetail = ProblemDetail.forStatus(status) + problemDetail.type = URI.create(httpStatusCodeTypePrefix + status.value()) + + if (exception.message != null) { + problemDetail.detail = exception.message + } + return problemDetail + } +} diff --git a/api/src/main/resources/application.yml b/api/src/main/resources/application.yml index d2ce69195..0e1756868 100644 --- a/api/src/main/resources/application.yml +++ b/api/src/main/resources/application.yml @@ -34,6 +34,19 @@ spring: enabled: false kubernetes: enabled: false + aws: + credentials: + access-key: ${csm.platform.s3.accessKeyId} + secret-key: ${csm.platform.s3.secretAccessKey} + s3: + # We don't need/have a region for our local S3 service but the AWS SDK requires one to be configured + region: ${csm.platform.s3.region} + # Enable path-style / disable DNS-style + # By default, and for AWS S3, the client crafts its URL with the bucket as sub-domain of the endpoint + # which is not how our current S3 implementation works as it expects the bucket in the path + # './' DNS vs Path '//' + path-style-access-enabled: true + endpoint: ${csm.platform.s3.endpointUrl} management: endpoints: @@ -119,8 +132,11 @@ csm: sender: username: "eventbus_sender_username" password: "eventbus_sender_password" - blobPersistence: - path: /tmp/cosmotech-api-data + s3: + endpointUrl: "http://localhost:9000" + bucketName: "cosmotech-api" + accessKeyId: "s3_username" + secretAccessKey: "s3_password" argo: base-uri: "https://localhost:2746" image-pull-secrets: [] diff --git a/build.gradle.kts b/build.gradle.kts index f5371db66..56327c134 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -61,7 +61,7 @@ val springWebVersion = "6.2.1" // Implementation val kotlinJvmTarget = 21 -val cosmotechApiCommonVersion = "2.1.0-SNAPSHOT" +val cosmotechApiCommonVersion = "2.1.1-LCRA-store_workspace_files_in_seaweed_PROD-14290-SNAPSHOT" val jedisVersion = "4.4.6" val springOauthVersion = "6.4.2" val redisOmSpringVersion = "0.9.7" @@ -79,6 +79,7 @@ val testNgVersion = "7.8.0" val testContainersRedisVersion = "1.6.4" val testContainersPostgreSQLVersion = "1.19.7" val commonCompressVersion = "1.27.1" +val awsSpringVersion = "3.1.1" // Checks val detektVersion = "1.23.7" @@ -140,6 +141,7 @@ allprojects { configurations { all { resolutionStrategy { force("com.redis.om:redis-om-spring:0.9.1") } } } repositories { + mavenLocal() maven { name = "GitHubPackages" url = uri("https://maven.pkg.github.com/Cosmo-Tech/cosmotech-api-common") @@ -312,6 +314,9 @@ subprojects { implementation("org.json:json:$orgJsonVersion") + implementation(platform("io.awspring.cloud:spring-cloud-aws-dependencies:$awsSpringVersion")) + implementation("io.awspring.cloud:spring-cloud-aws-starter-s3:$awsSpringVersion") + testImplementation(kotlin("test")) testImplementation(platform("org.junit:junit-bom:$jUnitBomVersion")) testImplementation("org.junit.jupiter:junit-jupiter") diff --git a/config/application-dev.sample.yml b/config/application-dev.sample.yml index 6fc9fd200..cf9d3248e 100644 --- a/config/application-dev.sample.yml +++ b/config/application-dev.sample.yml @@ -40,6 +40,15 @@ spring: keycloak: truststore: certificate: "classpath:[fill-this-value].pem" # certificate file + cloud: + aws: + credentials: + access-key: "s3_username" + secret-key: "s3_password" + s3: + region: "dummy" + path-style-access-enabled: true + endpoint: "http://localhost:9000" csm: platform: @@ -72,6 +81,11 @@ csm: tokenUrl: "[fill-this-value]" # eg. https://kubernetes.cosmotech.com/keycloak/realms/brewery/protocol/openid-connect/token metrics: enabled: false + s3: + endpointUrl: "http://localhost:9000" + bucketName: "cosmotech-api" + accessKeyId: "s3_username" + secretAccessKey: "s3_password" argo: base-uri: "http://localhost:2746" workflows: diff --git a/connector/src/integrationTest/resources/application-connector-test.yml b/connector/src/integrationTest/resources/application-connector-test.yml index 4c849fd27..e78585969 100644 --- a/connector/src/integrationTest/resources/application-connector-test.yml +++ b/connector/src/integrationTest/resources/application-connector-test.yml @@ -95,5 +95,8 @@ csm: default-page-size: 5 rbac: enabled: true - blobPersistence: - path: /tmp/cosmotech-api-connector-test-data + s3: + endpointUrl: "http://localhost:9000" + bucketName: "test-bucket" + accessKeyId: "s3_username" + secretAccessKey: "s3_password" diff --git a/dataset/src/integrationTest/resources/application-dataset-test.yml b/dataset/src/integrationTest/resources/application-dataset-test.yml index 22147e214..59ecf8d06 100644 --- a/dataset/src/integrationTest/resources/application-dataset-test.yml +++ b/dataset/src/integrationTest/resources/application-dataset-test.yml @@ -100,5 +100,8 @@ csm: default-page-size: 5 rbac: enabled: true - blobPersistence: - path: /tmp/cosmotech-api-dataset-test-data + s3: + endpointUrl: "http://localhost:9000" + bucketName: "test-bucket" + accessKeyId: "s3_username" + secretAccessKey: "s3_password" diff --git a/organization/src/integrationTest/resources/application-organization-test.yml b/organization/src/integrationTest/resources/application-organization-test.yml index 1afea781d..2e6d2a944 100644 --- a/organization/src/integrationTest/resources/application-organization-test.yml +++ b/organization/src/integrationTest/resources/application-organization-test.yml @@ -95,5 +95,8 @@ csm: default-page-size: 5 rbac: enabled: true - blobPersistence: - path: /tmp/cosmotech-api-organization-test-data + s3: + endpointUrl: "http://localhost:9000" + bucketName: "test-bucket" + accessKeyId: "s3_username" + secretAccessKey: "s3_password" diff --git a/run/src/integrationTest/resources/application-run-test.yml b/run/src/integrationTest/resources/application-run-test.yml index 397a09db4..652174093 100644 --- a/run/src/integrationTest/resources/application-run-test.yml +++ b/run/src/integrationTest/resources/application-run-test.yml @@ -139,5 +139,8 @@ csm: host: "localhost:5000" username: "default" password: "my-wonderful-password" - blobPersistence: - path: /tmp/cosmotech-api-run-test-data + s3: + endpointUrl: "http://localhost:9000" + bucketName: "test-bucket" + accessKeyId: "s3_username" + secretAccessKey: "s3_password" diff --git a/runner/src/integrationTest/resources/application-runner-test.yml b/runner/src/integrationTest/resources/application-runner-test.yml index 5505e9fb6..b882e26a8 100644 --- a/runner/src/integrationTest/resources/application-runner-test.yml +++ b/runner/src/integrationTest/resources/application-runner-test.yml @@ -95,5 +95,8 @@ csm: default-page-size: 20 rbac: enabled: true - blobPersistence: - path: /tmp/cosmotech-api-runner-test-data + s3: + endpointUrl: "http://localhost:9000" + bucketName: "test-bucket" + accessKeyId: "s3_username" + secretAccessKey: "s3_password" diff --git a/solution/src/integrationTest/resources/application-solution-test.yml b/solution/src/integrationTest/resources/application-solution-test.yml index d24a30e9f..60a82a003 100644 --- a/solution/src/integrationTest/resources/application-solution-test.yml +++ b/solution/src/integrationTest/resources/application-solution-test.yml @@ -99,5 +99,8 @@ csm: default-page-size: 20 rbac: enabled: true - blobPersistence: - path: /tmp/cosmotech-api-solution-test-data + s3: + endpointUrl: "http://localhost:9000" + bucketName: "test-bucket" + accessKeyId: "s3_username" + secretAccessKey: "s3_password" diff --git a/workspace/build.gradle.kts b/workspace/build.gradle.kts index 345201524..dddadd0f5 100644 --- a/workspace/build.gradle.kts +++ b/workspace/build.gradle.kts @@ -3,10 +3,11 @@ // no-import plugins { id("org.jetbrains.kotlinx.kover") } - +val testContainersLocalStackVersion = "1.20.6" dependencies { implementation(projects.cosmotechOrganizationApi) implementation(projects.cosmotechSolutionApi) + testImplementation("org.testcontainers:localstack:$testContainersLocalStackVersion") testImplementation(projects.cosmotechWorkspaceApi) testImplementation(projects.cosmotechDatasetApi) testImplementation(projects.cosmotechConnectorApi) diff --git a/workspace/src/integrationTest/kotlin/com/cosmotech/workspace/service/CsmS3TestBase.kt b/workspace/src/integrationTest/kotlin/com/cosmotech/workspace/service/CsmS3TestBase.kt new file mode 100644 index 000000000..4156973c0 --- /dev/null +++ b/workspace/src/integrationTest/kotlin/com/cosmotech/workspace/service/CsmS3TestBase.kt @@ -0,0 +1,88 @@ +// Copyright (c) Cosmo Tech. +// Licensed under the MIT license. +package com.cosmotech.workspace.service + +import com.redis.om.spring.annotations.EnableRedisDocumentRepositories +import com.redis.testcontainers.RedisServer +import com.redis.testcontainers.RedisStackContainer +import com.redis.testcontainers.junit.AbstractTestcontainersRedisTestBase +import com.redis.testcontainers.junit.RedisTestContext +import com.redis.testcontainers.junit.RedisTestContextsSource +import org.junit.jupiter.api.Assertions +import org.junit.jupiter.params.ParameterizedTest +import org.slf4j.LoggerFactory +import org.springframework.test.context.DynamicPropertyRegistry +import org.springframework.test.context.DynamicPropertySource +import org.testcontainers.containers.localstack.LocalStackContainer +import org.testcontainers.utility.DockerImageName + +@EnableRedisDocumentRepositories(basePackages = ["com.cosmotech"]) +open class CsmS3TestBase : AbstractTestcontainersRedisTestBase() { + + companion object { + + private const val DEFAULT_REDIS_PORT = 6379 + + private const val REDIS_STACK_LASTEST_TAG_WITH_GRAPH = "6.2.6-v9" + + @JvmStatic + val redisStackServer = + RedisStackContainer( + RedisStackContainer.DEFAULT_IMAGE_NAME.withTag(REDIS_STACK_LASTEST_TAG_WITH_GRAPH)) + + @JvmStatic + val localStackServer = + LocalStackContainer(DockerImageName.parse("localstack/localstack:3.5.0")) + .withServices(LocalStackContainer.Service.S3) + + private val logger = LoggerFactory.getLogger(CsmS3TestBase::class.java) + + init { + redisStackServer.start() + localStackServer.start() + localStackServer.execInContainer("awslocal", "s3", "mb", "s3://test-bucket") + } + + @JvmStatic + @DynamicPropertySource + fun connectionProperties(registry: DynamicPropertyRegistry) { + logger.error("Override properties to connect to Testcontainers:") + val containerIp = + redisStackServer.containerInfo.networkSettings.networks.entries + .elementAt(0) + .value + .ipAddress + logger.error( + "* Test-Container 'Redis': spring.data.redis.host = {} ; spring.data.redis.port = {}", + containerIp, + DEFAULT_REDIS_PORT) + + registry.add("spring.data.redis.host") { containerIp } + registry.add("spring.data.redis.port") { DEFAULT_REDIS_PORT } + registry.add("spring.cloud.aws.s3.endpoint") { localStackServer.endpoint } + registry.add("spring.cloud.aws.credentials.access-key") { localStackServer.accessKey } + registry.add("spring.cloud.aws.credentials.secret-key") { localStackServer.secretKey } + registry.add("spring.cloud.aws.s3.region") { localStackServer.region } + } + } + + override fun redisServers(): MutableCollection { + return mutableListOf(redisStackServer) + } + + @ParameterizedTest + @RedisTestContextsSource + fun canPing(context: RedisTestContext) { + Assertions.assertEquals("PONG", context.sync().ping()) + } + + @ParameterizedTest + @RedisTestContextsSource + fun canWrite(context: RedisTestContext) { + val hash = mutableMapOf() + hash["field1"] = "value1" + context.sync().hset("hash:test", hash) + val response = context.sync().hgetall("hash:test") + Assertions.assertEquals(hash, response) + } +} diff --git a/workspace/src/integrationTest/kotlin/com/cosmotech/workspace/service/WorkspaceServiceIntegrationTest.kt b/workspace/src/integrationTest/kotlin/com/cosmotech/workspace/service/WorkspaceServiceIntegrationTest.kt index a3e688287..3a57540cc 100644 --- a/workspace/src/integrationTest/kotlin/com/cosmotech/workspace/service/WorkspaceServiceIntegrationTest.kt +++ b/workspace/src/integrationTest/kotlin/com/cosmotech/workspace/service/WorkspaceServiceIntegrationTest.kt @@ -3,7 +3,6 @@ package com.cosmotech.workspace.service import com.cosmotech.api.config.CsmPlatformProperties -import com.cosmotech.api.containerregistry.ContainerRegistryService import com.cosmotech.api.exceptions.CsmAccessForbiddenException import com.cosmotech.api.exceptions.CsmResourceNotFoundException import com.cosmotech.api.rbac.ROLE_ADMIN @@ -11,7 +10,6 @@ import com.cosmotech.api.rbac.ROLE_EDITOR import com.cosmotech.api.rbac.ROLE_NONE import com.cosmotech.api.rbac.ROLE_USER import com.cosmotech.api.rbac.ROLE_VIEWER -import com.cosmotech.api.tests.CsmRedisTestBase import com.cosmotech.api.utils.getCurrentAccountIdentifier import com.cosmotech.api.utils.getCurrentAuthenticatedRoles import com.cosmotech.api.utils.getCurrentAuthenticatedUserName @@ -34,6 +32,7 @@ import com.cosmotech.workspace.WorkspaceApiServiceInterface import com.cosmotech.workspace.domain.Workspace import com.cosmotech.workspace.domain.WorkspaceAccessControl import com.cosmotech.workspace.domain.WorkspaceCreateRequest +import com.cosmotech.workspace.domain.WorkspaceFile import com.cosmotech.workspace.domain.WorkspaceRole import com.cosmotech.workspace.domain.WorkspaceSecurity import com.cosmotech.workspace.domain.WorkspaceSolution @@ -42,8 +41,8 @@ import com.cosmotech.workspace.domain.WorkspaceWebApp import com.redis.om.spring.RediSearchIndexer import io.mockk.every import io.mockk.junit5.MockKExtension -import io.mockk.mockk import io.mockk.mockkStatic +import org.junit.jupiter.api.AfterEach import java.util.* import kotlin.test.assertEquals import kotlin.test.assertNull @@ -57,10 +56,10 @@ import org.junit.runner.RunWith import org.slf4j.LoggerFactory import org.springframework.beans.factory.annotation.Autowired import org.springframework.boot.test.context.SpringBootTest +import org.springframework.core.io.UrlResource import org.springframework.test.context.ActiveProfiles import org.springframework.test.context.junit.jupiter.SpringExtension import org.springframework.test.context.junit4.SpringRunner -import org.springframework.test.util.ReflectionTestUtils @ActiveProfiles(profiles = ["workspace-test"]) @ExtendWith(MockKExtension::class) @@ -68,10 +67,11 @@ import org.springframework.test.util.ReflectionTestUtils @RunWith(SpringRunner::class) @SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT) @Suppress("FunctionName") -class WorkspaceServiceIntegrationTest : CsmRedisTestBase() { +class WorkspaceServiceIntegrationTest : CsmS3TestBase() { val TEST_USER_MAIL = "testuser@mail.fr" val CONNECTED_ADMIN_USER = "test.admin@cosmotech.com" val CONNECTED_DEFAULT_USER = "test.user@cosmotech.com" + val fileName = "test_workspace_file.txt" private val logger = LoggerFactory.getLogger(WorkspaceServiceIntegrationTest::class.java) @Autowired lateinit var rediSearchIndexer: RediSearchIndexer @@ -82,8 +82,6 @@ class WorkspaceServiceIntegrationTest : CsmRedisTestBase() { @Autowired lateinit var datasetApiService: DatasetApiService @Autowired lateinit var csmPlatformProperties: CsmPlatformProperties - private var containerRegistryService: ContainerRegistryService = mockk(relaxed = true) - lateinit var organization: OrganizationCreateRequest lateinit var solution: SolutionCreateRequest lateinit var workspace: WorkspaceCreateRequest @@ -96,11 +94,11 @@ class WorkspaceServiceIntegrationTest : CsmRedisTestBase() { lateinit var connectorSaved: Connector lateinit var datasetSaved: Dataset + val resourceTestFile = this::class.java.getResource("/$fileName") + @BeforeEach fun setUp() { mockkStatic("com.cosmotech.api.utils.SecurityUtilsKt") - ReflectionTestUtils.setField( - solutionApiService, "containerRegistryService", containerRegistryService) every { getCurrentAccountIdentifier(any()) } returns CONNECTED_ADMIN_USER every { getCurrentAuthenticatedUserName(csmPlatformProperties) } returns "test.user" every { getCurrentAuthenticatedRoles(any()) } returns listOf("user") @@ -127,6 +125,16 @@ class WorkspaceServiceIntegrationTest : CsmRedisTestBase() { datasetSaved = datasetApiService.createDataset(organizationSaved.id, dataset) } + @AfterEach + fun cleanUp(){ + every { getCurrentAuthenticatedRoles(any()) } returns listOf("Platform.Admin") + + val workspaces = workspaceApiService.listWorkspaces(organizationSaved.id, null, null) + workspaces.forEach { + workspaceApiService.deleteWorkspaceFiles(organizationSaved.id, it.id) + } + } + @Test fun `test CRUD operations on Workspace as User Admin`() { @@ -153,15 +161,75 @@ class WorkspaceServiceIntegrationTest : CsmRedisTestBase() { WorkspaceUpdateRequest(key = "key", name = "Workspace 1 updated")) assertEquals("Workspace 1 updated", updatedWorkspace.name) assertEquals(workspaceSaved.organizationId, updatedWorkspace.organizationId) + } + + @Test + fun `test create workspace file`(){ + every { getCurrentAuthenticatedRoles(any()) } returns listOf("Platform.Admin") + + logger.info("should create a workspace file") + + var savedFile = WorkspaceFile("") + assertDoesNotThrow { + savedFile = workspaceApiService.createWorkspaceFile(organizationSaved.id, workspaceSaved.id, UrlResource(resourceTestFile!!), true, null) + } + + assertEquals(fileName, savedFile.fileName) + } + + @Test + fun `test get workspace file`(){ + logger.info("should get a workspace file") + + workspaceApiService.createWorkspaceFile(organizationSaved.id, workspaceSaved.id, UrlResource(resourceTestFile!!), true, null) + assertDoesNotThrow { workspaceApiService.getWorkspaceFile(organizationSaved.id, workspaceSaved.id, fileName) } + + val fetchedFile = workspaceApiService.getWorkspaceFile(organizationSaved.id, workspaceSaved.id, fileName) + assertEquals(UrlResource(resourceTestFile!!).file, fetchedFile.file) + } + + @Test + fun `test list workspace files`() { + every { getCurrentAuthenticatedRoles(any()) } returns listOf("Platform.Admin") + + logger.info("should list all workspace file") + + var workspaceFiles = + workspaceApiService.listWorkspaceFiles(organizationSaved.id, workspaceSaved.id) + assertTrue(workspaceFiles.isEmpty()) + + + workspaceApiService.createWorkspaceFile( + organizationSaved.id, workspaceSaved.id, UrlResource(resourceTestFile!!), true, null) + + workspaceFiles = workspaceApiService.listWorkspaceFiles(organizationSaved.id, workspaceSaved.id) + assertEquals(1, workspaceFiles.size) + } + + @Test + fun `test delete workspace file`(){ + logger.info("should delete a workspace file") + + workspaceApiService.createWorkspaceFile( + organizationSaved.id, workspaceSaved.id, UrlResource(resourceTestFile!!), true, null) + + assertDoesNotThrow { workspaceApiService.deleteWorkspaceFile(organizationSaved.id, workspaceSaved.id, fileName) } + + assertThrows { workspaceApiService.getWorkspaceFile(organizationSaved.id, workspaceSaved.id, fileName) } + } + + @Test + fun `test deleteAll workspace file`(){ + logger.info("should delete all workspace files") + + workspaceApiService.createWorkspaceFile( + organizationSaved.id, workspaceSaved.id, UrlResource(resourceTestFile!!), true, null) + workspaceApiService.createWorkspaceFile( + organizationSaved.id, workspaceSaved.id, UrlResource(resourceTestFile!!), true, null) - /* - TODO : Fix the corountine effect + assertDoesNotThrow { workspaceApiService.deleteWorkspaceFiles(organizationSaved.id, workspaceSaved.id) } - logger.info("should delete the first workspace") - workspaceApiService.deleteWorkspace(organizationRegistered.id!!, workspaceRegistered2.id!!) - val workspacesListAfterDelete: List = - workspaceApiService.listWorkspaces(organizationRegistered.id!!, null, null) - assertTrue(workspacesListAfterDelete.size == 1)*/ + assertEquals(0, workspaceApiService.listWorkspaceFiles(organizationSaved.id, workspaceSaved.id).size) } @Test diff --git a/workspace/src/integrationTest/kotlin/com/cosmotech/workspace/service/WorkspaceServiceRBACTest.kt b/workspace/src/integrationTest/kotlin/com/cosmotech/workspace/service/WorkspaceServiceRBACTest.kt index 23433bf7b..2e8b971f1 100644 --- a/workspace/src/integrationTest/kotlin/com/cosmotech/workspace/service/WorkspaceServiceRBACTest.kt +++ b/workspace/src/integrationTest/kotlin/com/cosmotech/workspace/service/WorkspaceServiceRBACTest.kt @@ -37,17 +37,15 @@ import com.cosmotech.workspace.domain.WorkspaceSecurity import com.cosmotech.workspace.domain.WorkspaceSolution import com.cosmotech.workspace.domain.WorkspaceUpdateRequest import com.redis.om.spring.RediSearchIndexer +import io.awspring.cloud.s3.S3Template import io.mockk.every import io.mockk.impl.annotations.RelaxedMockK import io.mockk.junit5.MockKExtension import io.mockk.mockk import io.mockk.mockkStatic import java.io.File -import java.nio.file.Files -import java.nio.file.Path import java.util.* import kotlin.test.assertEquals -import org.junit.jupiter.api.AfterEach import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.DynamicTest import org.junit.jupiter.api.TestFactory @@ -62,6 +60,7 @@ import org.springframework.test.context.ActiveProfiles import org.springframework.test.context.junit.jupiter.SpringExtension import org.springframework.test.context.junit4.SpringRunner import org.springframework.test.util.ReflectionTestUtils +import software.amazon.awssdk.services.s3.S3Client @ActiveProfiles(profiles = ["workspace-test"]) @ExtendWith(MockKExtension::class) @@ -77,6 +76,8 @@ class WorkspaceServiceRBACTest : CsmRedisTestBase() { @RelaxedMockK private lateinit var resource: Resource @RelaxedMockK private lateinit var resourceScanner: ResourceScanner + @RelaxedMockK private lateinit var s3Client: S3Client + @RelaxedMockK private lateinit var s3Template: S3Template @Autowired lateinit var rediSearchIndexer: RediSearchIndexer @Autowired lateinit var organizationApiService: OrganizationApiServiceInterface @@ -91,23 +92,20 @@ class WorkspaceServiceRBACTest : CsmRedisTestBase() { mockkStatic("com.cosmotech.api.utils.SecurityUtilsKt") ReflectionTestUtils.setField( solutionApiService, "containerRegistryService", containerRegistryService) + ReflectionTestUtils.setField(workspaceApiService, "s3Client", s3Client) + ReflectionTestUtils.setField(workspaceApiService, "s3Template", s3Template) every { containerRegistryService.getImageLabel(any(), any(), any()) } returns null every { getCurrentAccountIdentifier(any()) } returns CONNECTED_ADMIN_USER every { getCurrentAuthenticatedUserName(csmPlatformProperties) } returns "test.user" every { getCurrentAuthenticatedRoles(any()) } returns listOf() - File(csmPlatformProperties.blobPersistence.path).deleteRecursively() + File(csmPlatformProperties.s3.endpointUrl).deleteRecursively() rediSearchIndexer.createIndexFor(Organization::class.java) rediSearchIndexer.createIndexFor(Solution::class.java) rediSearchIndexer.createIndexFor(Workspace::class.java) } - @AfterEach - fun afterEach() { - File(csmPlatformProperties.blobPersistence.path).deleteRecursively() - } - @TestFactory fun `test RBAC listWorkspaces`() = mapOf( @@ -790,14 +788,6 @@ class WorkspaceServiceRBACTest : CsmRedisTestBase() { "RBAC ${organizationSaved.id} - User does not have permission $PERMISSION_READ", exception.message) } else { - val filePath = - Path.of( - csmPlatformProperties.blobPersistence.path, - organizationSaved.id, - workspaceSaved.id, - "name") - Files.createDirectories(filePath.getParent()) - Files.createFile(filePath) assertDoesNotThrow { workspaceApiService.getWorkspaceFile( organizationSaved.id, workspaceSaved.id, "name") @@ -841,14 +831,6 @@ class WorkspaceServiceRBACTest : CsmRedisTestBase() { "RBAC ${workspaceSaved.id} - User does not have permission $PERMISSION_READ", exception.message) } else { - val filePath = - Path.of( - csmPlatformProperties.blobPersistence.path, - organizationSaved.id, - workspaceSaved.id, - "name") - Files.createDirectories(filePath.getParent()) - Files.createFile(filePath) assertDoesNotThrow { workspaceApiService.getWorkspaceFile( organizationSaved.id, workspaceSaved.id, "name") diff --git a/workspace/src/integrationTest/resources/application-workspace-test.yml b/workspace/src/integrationTest/resources/application-workspace-test.yml index 607941ae8..8e123c4d4 100644 --- a/workspace/src/integrationTest/resources/application-workspace-test.yml +++ b/workspace/src/integrationTest/resources/application-workspace-test.yml @@ -15,6 +15,10 @@ spring: csm: platform: + containerRegistry: + username: "test" + password: "test" + checkSolutionImage: false identityProvider: code: on_premise_one authorizationUrl: "http://fake_url:8080/authorize" @@ -100,5 +104,8 @@ csm: default-page-size: 5 rbac: enabled: true - blobPersistence: - path: /tmp/cosmotech-api-workspace-test-data + s3: + endpointUrl: "http://localhost:9000" + bucketName: "test-bucket" + accessKeyId: "s3_username" + secretAccessKey: "s3_password" diff --git a/workspace/src/integrationTest/resources/test_workspace_file.txt b/workspace/src/integrationTest/resources/test_workspace_file.txt new file mode 100644 index 000000000..0d6205935 --- /dev/null +++ b/workspace/src/integrationTest/resources/test_workspace_file.txt @@ -0,0 +1 @@ +This is a workspace file for test \ No newline at end of file diff --git a/workspace/src/main/kotlin/com/cosmotech/workspace/service/WorkspaceServiceImpl.kt b/workspace/src/main/kotlin/com/cosmotech/workspace/service/WorkspaceServiceImpl.kt index 494bbd2e9..18affa5c9 100644 --- a/workspace/src/main/kotlin/com/cosmotech/workspace/service/WorkspaceServiceImpl.kt +++ b/workspace/src/main/kotlin/com/cosmotech/workspace/service/WorkspaceServiceImpl.kt @@ -41,22 +41,19 @@ import com.cosmotech.workspace.domain.WorkspaceRole import com.cosmotech.workspace.domain.WorkspaceSecurity import com.cosmotech.workspace.domain.WorkspaceUpdateRequest import com.cosmotech.workspace.repository.WorkspaceRepository -import com.cosmotech.workspace.utils.getWorkspaceFilePath -import com.cosmotech.workspace.utils.getWorkspaceFilesDir -import java.io.File -import java.nio.file.Files -import java.nio.file.Path -import java.nio.file.StandardCopyOption.REPLACE_EXISTING +import io.awspring.cloud.s3.S3Template import kotlinx.coroutines.GlobalScope import kotlinx.coroutines.launch import org.springframework.beans.factory.config.ConfigurableBeanFactory import org.springframework.context.annotation.Scope import org.springframework.context.event.EventListener -import org.springframework.core.io.PathResource +import org.springframework.core.io.InputStreamResource import org.springframework.core.io.Resource import org.springframework.data.domain.Pageable import org.springframework.scheduling.annotation.Async import org.springframework.stereotype.Service +import software.amazon.awssdk.services.s3.S3Client +import software.amazon.awssdk.services.s3.model.ListObjectsV2Request @Service @Scope(value = ConfigurableBeanFactory.SCOPE_PROTOTYPE) @@ -64,6 +61,8 @@ import org.springframework.stereotype.Service internal class WorkspaceServiceImpl( private val organizationService: OrganizationApiServiceInterface, private val solutionService: SolutionApiService, + private val s3Client: S3Client, + private val s3Template: S3Template, private val csmRbac: CsmRbac, private val resourceScanner: ResourceScanner, private val workspaceRepository: WorkspaceRepository @@ -147,7 +146,7 @@ internal class WorkspaceServiceImpl( override fun deleteWorkspaceFiles(organizationId: String, workspaceId: String) { val workspace = getVerifiedWorkspace(organizationId, workspaceId, PERMISSION_WRITE) - deleteAllFilesystemWorkspaceFiles(organizationId, workspace) + deleteAllS3WorkspaceObjects(organizationId, workspace) } override fun updateWorkspace( @@ -205,7 +204,7 @@ internal class WorkspaceServiceImpl( val workspace = getVerifiedWorkspace(organizationId, workspaceId, PERMISSION_DELETE) try { - deleteAllFilesystemWorkspaceFiles(organizationId, workspace) + deleteAllS3WorkspaceObjects(organizationId, workspace) workspace.linkedDatasetIdList?.forEach { deleteDatasetLink(organizationId, workspaceId, it) } } finally { workspaceRepository.delete(workspace) @@ -218,7 +217,7 @@ internal class WorkspaceServiceImpl( override fun deleteWorkspaceFile(organizationId: String, workspaceId: String, fileName: String) { require(".." !in fileName) { "Invalid filename: '$fileName'. '..' is not allowed" } val workspace = getVerifiedWorkspace(organizationId, workspaceId, PERMISSION_WRITE) - deleteFilesystemWorkspaceFile(organizationId, workspace, fileName) + deleteS3WorkspaceObject(organizationId, workspace, fileName) } override fun getWorkspaceFile( @@ -233,12 +232,10 @@ internal class WorkspaceServiceImpl( workspace.id, workspace.name, fileName) - val filePath = - getWorkspaceFilePath(csmPlatformProperties, organizationId, workspaceId, fileName) - if (!Files.isRegularFile(filePath)) { - throw CsmResourceNotFoundException("$fileName not found") - } - return PathResource(filePath) + return InputStreamResource( + s3Template + .download(csmPlatformProperties.s3.bucketName, "$organizationId/$workspaceId/$fileName") + .inputStream) } override fun createWorkspaceFile( @@ -276,21 +273,14 @@ internal class WorkspaceServiceImpl( fileRelativeDestinationBuilder.append(file.filename) } } + val objectKey = "$organizationId/$workspaceId/$fileRelativeDestinationBuilder" - val filePath = - getWorkspaceFilePath( - csmPlatformProperties, - organizationId, - workspaceId, - fileRelativeDestinationBuilder.toString()) - - if (overwrite == false && Files.exists(filePath)) { + if (!overwrite && s3Template.objectExists(csmPlatformProperties.s3.bucketName, objectKey)) { throw IllegalArgumentException( "File '$fileRelativeDestinationBuilder' already exists, not overwriting it") } - Files.createDirectories(filePath.getParent()) - Files.copy(file.inputStream, filePath, REPLACE_EXISTING) + s3Template.upload(csmPlatformProperties.s3.bucketName, objectKey, file.inputStream) return WorkspaceFile(fileName = fileRelativeDestinationBuilder.toString()) } @@ -313,16 +303,12 @@ internal class WorkspaceServiceImpl( Pageable.ofSize(csmPlatformProperties.twincache.workspace.defaultPageSize) val workspaces = workspaceRepository.findByOrganizationId(organizationId, pageable).toList() workspaces.forEach { + deleteAllS3WorkspaceObjects(organizationId, it) workspaceRepository.delete(it) if (csmPlatformProperties.internalResultServices?.enabled == true) { this.eventPublisher.publishEvent(WorkspaceDeleted(this, organizationId, it.id)) } } - File( - Path.of(csmPlatformProperties.blobPersistence.path, organizationId) - .toAbsolutePath() - .toString()) - .deleteRecursively() } override fun createDatasetLink( @@ -384,17 +370,23 @@ internal class WorkspaceServiceImpl( } private fun getWorkspaceFiles(organizationId: String, workspaceId: String): List { - val rootPath = getWorkspaceFilesDir(csmPlatformProperties, organizationId, workspaceId) - if (!Files.exists(rootPath)) { - return listOf() - } - return Files.walk(rootPath) - .filter { Files.isRegularFile(it) } - .map { WorkspaceFile(fileName = rootPath.relativize(it).toString()) } + val prefix = "${organizationId}/${workspaceId}/" + val listObjectsRequest = + ListObjectsV2Request.builder() + .bucket(csmPlatformProperties.s3.bucketName) + .prefix(prefix) + .build() + + return s3Client + .listObjectsV2Paginator(listObjectsRequest) + .stream() + .flatMap { + it.contents().stream().map { WorkspaceFile(fileName = it.key().removePrefix(prefix)) } + } .toList() } - private fun deleteFilesystemWorkspaceFile( + private fun deleteS3WorkspaceObject( organizationId: String, workspace: Workspace, fileName: String @@ -404,16 +396,24 @@ internal class WorkspaceServiceImpl( workspace.id, workspace.name, fileName) - Files.deleteIfExists( - getWorkspaceFilePath(csmPlatformProperties, organizationId, workspace.id, fileName)) + + s3Template.deleteObject( + csmPlatformProperties.s3.bucketName, "$organizationId/${workspace.id}/$fileName") } - private fun deleteAllFilesystemWorkspaceFiles(organizationId: String, workspace: Workspace) { + private fun deleteAllS3WorkspaceObjects(organizationId: String, workspace: Workspace) { + logger.debug("Deleting all files for workspace #{} ({})", workspace.id, workspace.name) + GlobalScope.launch(SecurityCoroutineContext()) { // TODO Consider using a smaller coroutine scope - logger.debug("Deleting all files for workspace #{} ({})", workspace.id, workspace.name) - File(getWorkspaceFilesDir(csmPlatformProperties, organizationId, workspace.id).toString()) - .deleteRecursively() + val workspaceFiles = getWorkspaceFiles(organizationId, workspace.id) + if (workspaceFiles.isEmpty()) { + logger.debug("No file to delete for workspace #{} ({})", workspace.id, workspace.name) + } else { + workspaceFiles.forEach { file -> + deleteS3WorkspaceObject(organizationId, workspace, file.fileName) + } + } } } diff --git a/workspace/src/main/kotlin/com/cosmotech/workspace/utils/WorkspaceUtils.kt b/workspace/src/main/kotlin/com/cosmotech/workspace/utils/WorkspaceUtils.kt deleted file mode 100644 index 31b9314a2..000000000 --- a/workspace/src/main/kotlin/com/cosmotech/workspace/utils/WorkspaceUtils.kt +++ /dev/null @@ -1,23 +0,0 @@ -// Copyright (c) Cosmo Tech. -// Licensed under the MIT license. -package com.cosmotech.workspace.utils - -import com.cosmotech.api.config.CsmPlatformProperties -import java.nio.file.Path - -fun getWorkspaceFilesDir( - csmPlatformProperties: CsmPlatformProperties, - organizationId: String, - workspaceId: String -) = - Path.of(csmPlatformProperties.blobPersistence.path, organizationId, workspaceId) - .toAbsolutePath() - -fun getWorkspaceFilePath( - csmPlatformProperties: CsmPlatformProperties, - organizationId: String, - workspaceId: String, - fileName: String -) = - Path.of(csmPlatformProperties.blobPersistence.path, organizationId, workspaceId, fileName) - .toAbsolutePath() diff --git a/workspace/src/test/kotlin/com/cosmotech/workspace/service/WorkspaceServiceImplTests.kt b/workspace/src/test/kotlin/com/cosmotech/workspace/service/WorkspaceServiceImplTests.kt index fc236f940..3a21e00bf 100644 --- a/workspace/src/test/kotlin/com/cosmotech/workspace/service/WorkspaceServiceImplTests.kt +++ b/workspace/src/test/kotlin/com/cosmotech/workspace/service/WorkspaceServiceImplTests.kt @@ -42,6 +42,7 @@ import com.cosmotech.workspace.domain.WorkspaceSecurity import com.cosmotech.workspace.domain.WorkspaceSolution import com.cosmotech.workspace.domain.WorkspaceUpdateRequest import com.cosmotech.workspace.repository.WorkspaceRepository +import io.awspring.cloud.s3.S3Template import io.mockk.MockKAnnotations import io.mockk.confirmVerified import io.mockk.every @@ -71,11 +72,13 @@ import org.junit.jupiter.api.assertThrows import org.junit.jupiter.api.extension.ExtendWith import org.springframework.core.io.Resource import org.springframework.data.repository.findByIdOrNull +import software.amazon.awssdk.services.s3.S3Client const val ORGANIZATION_ID = "o-AbCdEf1234" const val WORKSPACE_ID = "w-BcDeFg1234" const val CONNECTED_ADMIN_USER = "test.admin@cosmotech.com" const val CONNECTED_DEFAULT_USER = "test.user@cosmotech.com" +const val S3_BUCKET_NAME = "test-bucket" @ExtendWith(MockKExtension::class) @Suppress("LargeClass") @@ -84,6 +87,9 @@ class WorkspaceServiceImplTests { @MockK private lateinit var solutionService: SolutionApiServiceInterface @RelaxedMockK private lateinit var organizationService: OrganizationApiServiceInterface + @Suppress("unused") @RelaxedMockK private lateinit var s3Client: S3Client + @RelaxedMockK private lateinit var s3Template: S3Template + private lateinit var blobPersistencePath: String @Suppress("unused") @MockK private var eventPublisher: CsmEventPublisher = mockk(relaxed = true) @@ -133,8 +139,10 @@ class WorkspaceServiceImplTests { every { csmPlatformProperties.rbac.enabled } returns true every { csmPlatformProperties.upload } returns csmPlatformPropertiesUpload + every { csmPlatformProperties.s3.bucketName } returns S3_BUCKET_NAME + every { s3Template.objectExists(any(), any()) } returns false + blobPersistencePath = Files.createTempDirectory("cosmotech-api-test-data-").toString() - every { csmPlatformProperties.blobPersistence.path } returns blobPersistencePath MockKAnnotations.init(this) } From d9d180ff56c62fc8969e83e0bee4931e2f7699ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=A9my=20Reynard?= Date: Fri, 4 Apr 2025 19:08:55 +0200 Subject: [PATCH 2/5] Use resourceLoader for workspace files --- .../WorkspaceServiceIntegrationTest.kt | 64 +++++++++++-------- 1 file changed, 38 insertions(+), 26 deletions(-) diff --git a/workspace/src/integrationTest/kotlin/com/cosmotech/workspace/service/WorkspaceServiceIntegrationTest.kt b/workspace/src/integrationTest/kotlin/com/cosmotech/workspace/service/WorkspaceServiceIntegrationTest.kt index 3a57540cc..877deb7e2 100644 --- a/workspace/src/integrationTest/kotlin/com/cosmotech/workspace/service/WorkspaceServiceIntegrationTest.kt +++ b/workspace/src/integrationTest/kotlin/com/cosmotech/workspace/service/WorkspaceServiceIntegrationTest.kt @@ -42,11 +42,11 @@ import com.redis.om.spring.RediSearchIndexer import io.mockk.every import io.mockk.junit5.MockKExtension import io.mockk.mockkStatic -import org.junit.jupiter.api.AfterEach import java.util.* import kotlin.test.assertEquals import kotlin.test.assertNull import kotlin.test.assertTrue +import org.junit.jupiter.api.AfterEach import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test import org.junit.jupiter.api.assertDoesNotThrow @@ -56,7 +56,7 @@ import org.junit.runner.RunWith import org.slf4j.LoggerFactory import org.springframework.beans.factory.annotation.Autowired import org.springframework.boot.test.context.SpringBootTest -import org.springframework.core.io.UrlResource +import org.springframework.core.io.ResourceLoader import org.springframework.test.context.ActiveProfiles import org.springframework.test.context.junit.jupiter.SpringExtension import org.springframework.test.context.junit4.SpringRunner @@ -81,6 +81,7 @@ class WorkspaceServiceIntegrationTest : CsmS3TestBase() { @Autowired lateinit var connectorApiService: ConnectorApiService @Autowired lateinit var datasetApiService: DatasetApiService @Autowired lateinit var csmPlatformProperties: CsmPlatformProperties + @Autowired lateinit var resourceLoader: ResourceLoader lateinit var organization: OrganizationCreateRequest lateinit var solution: SolutionCreateRequest @@ -94,8 +95,6 @@ class WorkspaceServiceIntegrationTest : CsmS3TestBase() { lateinit var connectorSaved: Connector lateinit var datasetSaved: Dataset - val resourceTestFile = this::class.java.getResource("/$fileName") - @BeforeEach fun setUp() { mockkStatic("com.cosmotech.api.utils.SecurityUtilsKt") @@ -126,13 +125,11 @@ class WorkspaceServiceIntegrationTest : CsmS3TestBase() { } @AfterEach - fun cleanUp(){ + fun cleanUp() { every { getCurrentAuthenticatedRoles(any()) } returns listOf("Platform.Admin") val workspaces = workspaceApiService.listWorkspaces(organizationSaved.id, null, null) - workspaces.forEach { - workspaceApiService.deleteWorkspaceFiles(organizationSaved.id, it.id) - } + workspaces.forEach { workspaceApiService.deleteWorkspaceFiles(organizationSaved.id, it.id) } } @Test @@ -164,28 +161,36 @@ class WorkspaceServiceIntegrationTest : CsmS3TestBase() { } @Test - fun `test create workspace file`(){ + fun `test create workspace file`() { + + val resourceTestFile = resourceLoader.getResource("classpath:/$fileName") every { getCurrentAuthenticatedRoles(any()) } returns listOf("Platform.Admin") logger.info("should create a workspace file") var savedFile = WorkspaceFile("") assertDoesNotThrow { - savedFile = workspaceApiService.createWorkspaceFile(organizationSaved.id, workspaceSaved.id, UrlResource(resourceTestFile!!), true, null) + savedFile = + workspaceApiService.createWorkspaceFile( + organizationSaved.id, workspaceSaved.id, resourceTestFile, true, null) } assertEquals(fileName, savedFile.fileName) } @Test - fun `test get workspace file`(){ + fun `test get workspace file`() { logger.info("should get a workspace file") + val resourceTestFile = resourceLoader.getResource("classpath:/$fileName") - workspaceApiService.createWorkspaceFile(organizationSaved.id, workspaceSaved.id, UrlResource(resourceTestFile!!), true, null) - assertDoesNotThrow { workspaceApiService.getWorkspaceFile(organizationSaved.id, workspaceSaved.id, fileName) } + workspaceApiService.createWorkspaceFile( + organizationSaved.id, workspaceSaved.id, resourceTestFile, true, null) - val fetchedFile = workspaceApiService.getWorkspaceFile(organizationSaved.id, workspaceSaved.id, fileName) - assertEquals(UrlResource(resourceTestFile!!).file, fetchedFile.file) + val fetchedFile = + workspaceApiService.getWorkspaceFile(organizationSaved.id, workspaceSaved.id, fileName) + val expectedText = resourceTestFile.inputStream.bufferedReader().use { it.readText() } + val retrievedText = fetchedFile.inputStream.bufferedReader().use { it.readText() } + assertEquals(expectedText, retrievedText) } @Test @@ -193,43 +198,50 @@ class WorkspaceServiceIntegrationTest : CsmS3TestBase() { every { getCurrentAuthenticatedRoles(any()) } returns listOf("Platform.Admin") logger.info("should list all workspace file") + val resourceTestFile = resourceLoader.getResource("classpath:/$fileName") var workspaceFiles = workspaceApiService.listWorkspaceFiles(organizationSaved.id, workspaceSaved.id) assertTrue(workspaceFiles.isEmpty()) - workspaceApiService.createWorkspaceFile( - organizationSaved.id, workspaceSaved.id, UrlResource(resourceTestFile!!), true, null) + organizationSaved.id, workspaceSaved.id, resourceTestFile, true, null) workspaceFiles = workspaceApiService.listWorkspaceFiles(organizationSaved.id, workspaceSaved.id) assertEquals(1, workspaceFiles.size) } @Test - fun `test delete workspace file`(){ + fun `test delete workspace file`() { logger.info("should delete a workspace file") + val resourceTestFile = resourceLoader.getResource("classpath:/$fileName") workspaceApiService.createWorkspaceFile( - organizationSaved.id, workspaceSaved.id, UrlResource(resourceTestFile!!), true, null) + organizationSaved.id, workspaceSaved.id, resourceTestFile, true, null) - assertDoesNotThrow { workspaceApiService.deleteWorkspaceFile(organizationSaved.id, workspaceSaved.id, fileName) } + assertDoesNotThrow { + workspaceApiService.deleteWorkspaceFile(organizationSaved.id, workspaceSaved.id, fileName) + } - assertThrows { workspaceApiService.getWorkspaceFile(organizationSaved.id, workspaceSaved.id, fileName) } + assertThrows { + workspaceApiService.getWorkspaceFile(organizationSaved.id, workspaceSaved.id, fileName) + } } @Test - fun `test deleteAll workspace file`(){ + fun `test deleteAll workspace file`() { logger.info("should delete all workspace files") + val resourceTestFile = resourceLoader.getResource("classpath:/$fileName") workspaceApiService.createWorkspaceFile( - organizationSaved.id, workspaceSaved.id, UrlResource(resourceTestFile!!), true, null) + organizationSaved.id, workspaceSaved.id, resourceTestFile, true, null) workspaceApiService.createWorkspaceFile( - organizationSaved.id, workspaceSaved.id, UrlResource(resourceTestFile!!), true, null) + organizationSaved.id, workspaceSaved.id, resourceTestFile, true, null) - assertDoesNotThrow { workspaceApiService.deleteWorkspaceFiles(organizationSaved.id, workspaceSaved.id) } + workspaceApiService.deleteWorkspaceFiles(organizationSaved.id, workspaceSaved.id) - assertEquals(0, workspaceApiService.listWorkspaceFiles(organizationSaved.id, workspaceSaved.id).size) + assertEquals( + 0, workspaceApiService.listWorkspaceFiles(organizationSaved.id, workspaceSaved.id).size) } @Test From 938de0b4c23bd957bf36375d7a43592a4d60b760 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=A9my=20Reynard?= Date: Thu, 10 Apr 2025 11:44:36 +0200 Subject: [PATCH 3/5] Fix tests - Add default spring configuration for integration tests - Add S3 configuration in ControllerTestBase - Change coroutines management in Workspace module --- api/build.gradle.kts | 4 --- .../cosmotech/api/home/ControllerTestBase.kt | 21 +++++++++-- .../resources/application-test.yml | 19 ++++++++++ build.gradle.kts | 7 ++-- .../resources/application-connector-test.yml | 14 ++++++++ .../resources/application-dataset-test.yml | 14 ++++++++ .../application-organization-test.yml | 14 ++++++++ .../resources/application-run-test.yml | 14 ++++++++ .../resources/application-runner-test.yml | 14 ++++++++ .../resources/application-solution-test.yml | 15 ++++++++ workspace/build.gradle.kts | 3 +- .../WorkspaceServiceIntegrationTest.kt | 36 +++++++------------ .../service/WorkspaceServiceRBACTest.kt | 3 +- .../resources/application-workspace-test.yml | 14 ++++++++ .../workspace/WorkspaceApiServiceInterface.kt | 2 ++ .../workspace/service/WorkspaceServiceImpl.kt | 32 ++++++++--------- 16 files changed, 172 insertions(+), 54 deletions(-) diff --git a/api/build.gradle.kts b/api/build.gradle.kts index e163b7439..9adefd0bd 100644 --- a/api/build.gradle.kts +++ b/api/build.gradle.kts @@ -11,10 +11,6 @@ plugins { id("org.jetbrains.kotlinx.kover") } -val testNgVersion = "7.8.0" -val testContainersRedisVersion = "1.6.4" -val testContainersPostgreSQLVersion = "1.19.7" - dependencies { implementation(projects.cosmotechMetaApi) implementation(projects.cosmotechConnectorApi) diff --git a/api/src/integrationTest/kotlin/com/cosmotech/api/home/ControllerTestBase.kt b/api/src/integrationTest/kotlin/com/cosmotech/api/home/ControllerTestBase.kt index aae588337..10b2319dc 100644 --- a/api/src/integrationTest/kotlin/com/cosmotech/api/home/ControllerTestBase.kt +++ b/api/src/integrationTest/kotlin/com/cosmotech/api/home/ControllerTestBase.kt @@ -34,7 +34,9 @@ import org.springframework.test.web.servlet.setup.MockMvcBuilders import org.springframework.web.context.WebApplicationContext import org.testcontainers.containers.PostgreSQLContainer import org.testcontainers.containers.PostgreSQLContainer.POSTGRESQL_PORT +import org.testcontainers.containers.localstack.LocalStackContainer import org.testcontainers.junit.jupiter.Testcontainers +import org.testcontainers.utility.DockerImageName import org.testcontainers.utility.MountableFile @Testcontainers @@ -99,7 +101,8 @@ abstract class ControllerTestBase : AbstractTestcontainersRedisTestBase() { private const val READER_USER_CREDENTIALS = "readusertest" private const val WRITER_USER_CREDENTIALS = "writeusertest" private const val DEFAULT_REDIS_PORT = 6379 - private const val REDIS_STACK_LASTEST_TAG_WITH_GRAPH = "6.2.6-v18" + private const val REDIS_STACK_LATEST_TAG_WITH_GRAPH = "6.2.6-v18" + private const val LOCALSTACK_FULL_IMAGE_NAME = "localstack/localstack:3.5.0" var postgres: PostgreSQLContainer<*> = PostgreSQLContainer("postgres:alpine3.19") @@ -108,11 +111,17 @@ abstract class ControllerTestBase : AbstractTestcontainersRedisTestBase() { var redisStackServer = RedisStackContainer( - RedisStackContainer.DEFAULT_IMAGE_NAME.withTag(REDIS_STACK_LASTEST_TAG_WITH_GRAPH)) + RedisStackContainer.DEFAULT_IMAGE_NAME.withTag(REDIS_STACK_LATEST_TAG_WITH_GRAPH)) + + val localStackServer = + LocalStackContainer(DockerImageName.parse(LOCALSTACK_FULL_IMAGE_NAME)) + .withServices(LocalStackContainer.Service.S3) init { redisStackServer.start() postgres.start() + localStackServer.start() + localStackServer.execInContainer("awslocal", "s3", "mb", "s3://test-bucket") } @JvmStatic @@ -120,6 +129,7 @@ abstract class ControllerTestBase : AbstractTestcontainersRedisTestBase() { fun connectionProperties(registry: DynamicPropertyRegistry) { initPostgresConfiguration(registry) initRedisConfiguration(registry) + initS3Configuration(registry) } private fun initRedisConfiguration(registry: DynamicPropertyRegistry) { @@ -133,6 +143,13 @@ abstract class ControllerTestBase : AbstractTestcontainersRedisTestBase() { registry.add("spring.data.redis.port") { DEFAULT_REDIS_PORT } } + private fun initS3Configuration(registry: DynamicPropertyRegistry) { + registry.add("spring.cloud.aws.s3.endpoint") { localStackServer.endpoint } + registry.add("spring.cloud.aws.credentials.access-key") { localStackServer.accessKey } + registry.add("spring.cloud.aws.credentials.secret-key") { localStackServer.secretKey } + registry.add("spring.cloud.aws.s3.region") { localStackServer.region } + } + private fun initPostgresConfiguration(registry: DynamicPropertyRegistry) { registry.add("csm.platform.internalResultServices.storage.host") { postgres.host } registry.add("csm.platform.internalResultServices.storage.port") { diff --git a/api/src/integrationTest/resources/application-test.yml b/api/src/integrationTest/resources/application-test.yml index 317122b45..23384f350 100644 --- a/api/src/integrationTest/resources/application-test.yml +++ b/api/src/integrationTest/resources/application-test.yml @@ -12,6 +12,20 @@ spring: client-type: jedis main: banner-mode: "off" + cloud: + aws: + credentials: + access-key: "accessKeyId" + secret-key: "secretAccessKey" + s3: + # We don't need/have a region for our local S3 service but the AWS SDK requires one to be configured + region: "region" + # Enable path-style / disable DNS-style + # By default, and for AWS S3, the client crafts its URL with the bucket as sub-domain of the endpoint + # which is not how our current S3 implementation works as it expects the bucket in the path + # './' DNS vs Path '//' + path-style-access-enabled: true + endpoint: "http://localhost:9000" csm: platform: @@ -105,5 +119,10 @@ csm: default-page-size: 5 rbac: enabled: true + s3: + endpointUrl: "http://localhost:9000" + bucketName: "test-bucket" + accessKeyId: "s3_username" + secretAccessKey: "s3_password" diff --git a/build.gradle.kts b/build.gradle.kts index 56327c134..74b6df9a5 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -61,7 +61,7 @@ val springWebVersion = "6.2.1" // Implementation val kotlinJvmTarget = 21 -val cosmotechApiCommonVersion = "2.1.1-LCRA-store_workspace_files_in_seaweed_PROD-14290-SNAPSHOT" +val cosmotechApiCommonVersion = "2.1.0-SNAPSHOT" val jedisVersion = "4.4.6" val springOauthVersion = "6.4.2" val redisOmSpringVersion = "0.9.7" @@ -77,7 +77,8 @@ val orgJsonVersion = "20240303" val jacksonModuleKotlinVersion = "2.18.3" val testNgVersion = "7.8.0" val testContainersRedisVersion = "1.6.4" -val testContainersPostgreSQLVersion = "1.19.7" +val testContainersPostgreSQLVersion = "1.20.6" +val testContainersLocalStackVersion = "1.20.6" val commonCompressVersion = "1.27.1" val awsSpringVersion = "3.1.1" @@ -141,7 +142,6 @@ allprojects { configurations { all { resolutionStrategy { force("com.redis.om:redis-om-spring:0.9.1") } } } repositories { - mavenLocal() maven { name = "GitHubPackages" url = uri("https://maven.pkg.github.com/Cosmo-Tech/cosmotech-api-common") @@ -327,6 +327,7 @@ subprojects { testImplementation( "com.redis.testcontainers:testcontainers-redis-junit:$testContainersRedisVersion") testImplementation("org.testcontainers:postgresql:$testContainersPostgreSQLVersion") + testImplementation("org.testcontainers:localstack:$testContainersLocalStackVersion") testImplementation("org.springframework.boot:spring-boot-starter-test") integrationTestImplementation("org.springframework.boot:spring-boot-starter-test") { diff --git a/connector/src/integrationTest/resources/application-connector-test.yml b/connector/src/integrationTest/resources/application-connector-test.yml index e78585969..b7c03a48e 100644 --- a/connector/src/integrationTest/resources/application-connector-test.yml +++ b/connector/src/integrationTest/resources/application-connector-test.yml @@ -12,6 +12,20 @@ spring: client-type: jedis main: banner-mode: "off" + cloud: + aws: + credentials: + access-key: "accessKeyId" + secret-key: "secretAccessKey" + s3: + # We don't need/have a region for our local S3 service but the AWS SDK requires one to be configured + region: "region" + # Enable path-style / disable DNS-style + # By default, and for AWS S3, the client crafts its URL with the bucket as sub-domain of the endpoint + # which is not how our current S3 implementation works as it expects the bucket in the path + # './' DNS vs Path '//' + path-style-access-enabled: true + endpoint: "http://localhost:9000" csm: platform: diff --git a/dataset/src/integrationTest/resources/application-dataset-test.yml b/dataset/src/integrationTest/resources/application-dataset-test.yml index 59ecf8d06..d177ad2a2 100644 --- a/dataset/src/integrationTest/resources/application-dataset-test.yml +++ b/dataset/src/integrationTest/resources/application-dataset-test.yml @@ -12,6 +12,20 @@ spring: client-type: jedis main: banner-mode: "off" + cloud: + aws: + credentials: + access-key: "accessKeyId" + secret-key: "secretAccessKey" + s3: + # We don't need/have a region for our local S3 service but the AWS SDK requires one to be configured + region: "region" + # Enable path-style / disable DNS-style + # By default, and for AWS S3, the client crafts its URL with the bucket as sub-domain of the endpoint + # which is not how our current S3 implementation works as it expects the bucket in the path + # './' DNS vs Path '//' + path-style-access-enabled: true + endpoint: "http://localhost:9000" csm: platform: diff --git a/organization/src/integrationTest/resources/application-organization-test.yml b/organization/src/integrationTest/resources/application-organization-test.yml index 2e6d2a944..11327e038 100644 --- a/organization/src/integrationTest/resources/application-organization-test.yml +++ b/organization/src/integrationTest/resources/application-organization-test.yml @@ -12,6 +12,20 @@ spring: client-type: jedis main: banner-mode: "off" + cloud: + aws: + credentials: + access-key: "accessKeyId" + secret-key: "secretAccessKey" + s3: + # We don't need/have a region for our local S3 service but the AWS SDK requires one to be configured + region: "region" + # Enable path-style / disable DNS-style + # By default, and for AWS S3, the client crafts its URL with the bucket as sub-domain of the endpoint + # which is not how our current S3 implementation works as it expects the bucket in the path + # './' DNS vs Path '//' + path-style-access-enabled: true + endpoint: "http://localhost:9000" csm: platform: diff --git a/run/src/integrationTest/resources/application-run-test.yml b/run/src/integrationTest/resources/application-run-test.yml index 652174093..ee206a2bf 100644 --- a/run/src/integrationTest/resources/application-run-test.yml +++ b/run/src/integrationTest/resources/application-run-test.yml @@ -12,6 +12,20 @@ spring: # Leave it as blank as there's no auth with test container password: client-type: jedis + cloud: + aws: + credentials: + access-key: "accessKeyId" + secret-key: "secretAccessKey" + s3: + # We don't need/have a region for our local S3 service but the AWS SDK requires one to be configured + region: "region" + # Enable path-style / disable DNS-style + # By default, and for AWS S3, the client crafts its URL with the bucket as sub-domain of the endpoint + # which is not how our current S3 implementation works as it expects the bucket in the path + # './' DNS vs Path '//' + path-style-access-enabled: true + endpoint: "http://localhost:9000" logging: level: diff --git a/runner/src/integrationTest/resources/application-runner-test.yml b/runner/src/integrationTest/resources/application-runner-test.yml index b882e26a8..604c8f76f 100644 --- a/runner/src/integrationTest/resources/application-runner-test.yml +++ b/runner/src/integrationTest/resources/application-runner-test.yml @@ -8,6 +8,20 @@ spring: client-type: jedis main: banner-mode: "off" + cloud: + aws: + credentials: + access-key: "accessKeyId" + secret-key: "secretAccessKey" + s3: + # We don't need/have a region for our local S3 service but the AWS SDK requires one to be configured + region: "region" + # Enable path-style / disable DNS-style + # By default, and for AWS S3, the client crafts its URL with the bucket as sub-domain of the endpoint + # which is not how our current S3 implementation works as it expects the bucket in the path + # './' DNS vs Path '//' + path-style-access-enabled: true + endpoint: "http://localhost:9000" management: endpoints: diff --git a/solution/src/integrationTest/resources/application-solution-test.yml b/solution/src/integrationTest/resources/application-solution-test.yml index 60a82a003..df1870f97 100644 --- a/solution/src/integrationTest/resources/application-solution-test.yml +++ b/solution/src/integrationTest/resources/application-solution-test.yml @@ -12,6 +12,20 @@ spring: client-type: jedis main: banner-mode: "off" + cloud: + aws: + credentials: + access-key: "accessKeyId" + secret-key: "secretAccessKey" + s3: + # We don't need/have a region for our local S3 service but the AWS SDK requires one to be configured + region: "region" + # Enable path-style / disable DNS-style + # By default, and for AWS S3, the client crafts its URL with the bucket as sub-domain of the endpoint + # which is not how our current S3 implementation works as it expects the bucket in the path + # './' DNS vs Path '//' + path-style-access-enabled: true + endpoint: "http://localhost:9000" csm: platform: @@ -104,3 +118,4 @@ csm: bucketName: "test-bucket" accessKeyId: "s3_username" secretAccessKey: "s3_password" + region: "us-east-1" diff --git a/workspace/build.gradle.kts b/workspace/build.gradle.kts index dddadd0f5..345201524 100644 --- a/workspace/build.gradle.kts +++ b/workspace/build.gradle.kts @@ -3,11 +3,10 @@ // no-import plugins { id("org.jetbrains.kotlinx.kover") } -val testContainersLocalStackVersion = "1.20.6" + dependencies { implementation(projects.cosmotechOrganizationApi) implementation(projects.cosmotechSolutionApi) - testImplementation("org.testcontainers:localstack:$testContainersLocalStackVersion") testImplementation(projects.cosmotechWorkspaceApi) testImplementation(projects.cosmotechDatasetApi) testImplementation(projects.cosmotechConnectorApi) diff --git a/workspace/src/integrationTest/kotlin/com/cosmotech/workspace/service/WorkspaceServiceIntegrationTest.kt b/workspace/src/integrationTest/kotlin/com/cosmotech/workspace/service/WorkspaceServiceIntegrationTest.kt index 877deb7e2..a341779e9 100644 --- a/workspace/src/integrationTest/kotlin/com/cosmotech/workspace/service/WorkspaceServiceIntegrationTest.kt +++ b/workspace/src/integrationTest/kotlin/com/cosmotech/workspace/service/WorkspaceServiceIntegrationTest.kt @@ -32,7 +32,6 @@ import com.cosmotech.workspace.WorkspaceApiServiceInterface import com.cosmotech.workspace.domain.Workspace import com.cosmotech.workspace.domain.WorkspaceAccessControl import com.cosmotech.workspace.domain.WorkspaceCreateRequest -import com.cosmotech.workspace.domain.WorkspaceFile import com.cosmotech.workspace.domain.WorkspaceRole import com.cosmotech.workspace.domain.WorkspaceSecurity import com.cosmotech.workspace.domain.WorkspaceSolution @@ -46,7 +45,6 @@ import java.util.* import kotlin.test.assertEquals import kotlin.test.assertNull import kotlin.test.assertTrue -import org.junit.jupiter.api.AfterEach import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test import org.junit.jupiter.api.assertDoesNotThrow @@ -60,6 +58,7 @@ import org.springframework.core.io.ResourceLoader import org.springframework.test.context.ActiveProfiles import org.springframework.test.context.junit.jupiter.SpringExtension import org.springframework.test.context.junit4.SpringRunner +import software.amazon.awssdk.services.s3.model.NoSuchKeyException @ActiveProfiles(profiles = ["workspace-test"]) @ExtendWith(MockKExtension::class) @@ -124,14 +123,6 @@ class WorkspaceServiceIntegrationTest : CsmS3TestBase() { datasetSaved = datasetApiService.createDataset(organizationSaved.id, dataset) } - @AfterEach - fun cleanUp() { - every { getCurrentAuthenticatedRoles(any()) } returns listOf("Platform.Admin") - - val workspaces = workspaceApiService.listWorkspaces(organizationSaved.id, null, null) - workspaces.forEach { workspaceApiService.deleteWorkspaceFiles(organizationSaved.id, it.id) } - } - @Test fun `test CRUD operations on Workspace as User Admin`() { @@ -167,13 +158,9 @@ class WorkspaceServiceIntegrationTest : CsmS3TestBase() { every { getCurrentAuthenticatedRoles(any()) } returns listOf("Platform.Admin") logger.info("should create a workspace file") - - var savedFile = WorkspaceFile("") - assertDoesNotThrow { - savedFile = - workspaceApiService.createWorkspaceFile( - organizationSaved.id, workspaceSaved.id, resourceTestFile, true, null) - } + val savedFile = + workspaceApiService.createWorkspaceFile( + organizationSaved.id, workspaceSaved.id, resourceTestFile, true, null) assertEquals(fileName, savedFile.fileName) } @@ -219,13 +206,14 @@ class WorkspaceServiceIntegrationTest : CsmS3TestBase() { workspaceApiService.createWorkspaceFile( organizationSaved.id, workspaceSaved.id, resourceTestFile, true, null) - assertDoesNotThrow { - workspaceApiService.deleteWorkspaceFile(organizationSaved.id, workspaceSaved.id, fileName) - } + workspaceApiService.deleteWorkspaceFile(organizationSaved.id, workspaceSaved.id, fileName) - assertThrows { - workspaceApiService.getWorkspaceFile(organizationSaved.id, workspaceSaved.id, fileName) - } + val exception = + assertThrows { + workspaceApiService.getWorkspaceFile(organizationSaved.id, workspaceSaved.id, fileName) + } + + assertEquals("The specified key does not exist.", exception.awsErrorDetails().errorMessage()) } @Test @@ -238,7 +226,7 @@ class WorkspaceServiceIntegrationTest : CsmS3TestBase() { workspaceApiService.createWorkspaceFile( organizationSaved.id, workspaceSaved.id, resourceTestFile, true, null) - workspaceApiService.deleteWorkspaceFiles(organizationSaved.id, workspaceSaved.id) + workspaceApiService.deleteAllS3WorkspaceObjects(organizationSaved.id, workspaceSaved) assertEquals( 0, workspaceApiService.listWorkspaceFiles(organizationSaved.id, workspaceSaved.id).size) diff --git a/workspace/src/integrationTest/kotlin/com/cosmotech/workspace/service/WorkspaceServiceRBACTest.kt b/workspace/src/integrationTest/kotlin/com/cosmotech/workspace/service/WorkspaceServiceRBACTest.kt index 2e8b971f1..c0875ecab 100644 --- a/workspace/src/integrationTest/kotlin/com/cosmotech/workspace/service/WorkspaceServiceRBACTest.kt +++ b/workspace/src/integrationTest/kotlin/com/cosmotech/workspace/service/WorkspaceServiceRBACTest.kt @@ -16,7 +16,6 @@ import com.cosmotech.api.rbac.ROLE_EDITOR import com.cosmotech.api.rbac.ROLE_NONE import com.cosmotech.api.rbac.ROLE_USER import com.cosmotech.api.rbac.ROLE_VIEWER -import com.cosmotech.api.tests.CsmRedisTestBase import com.cosmotech.api.utils.ResourceScanner import com.cosmotech.api.utils.getCurrentAccountIdentifier import com.cosmotech.api.utils.getCurrentAuthenticatedRoles @@ -68,7 +67,7 @@ import software.amazon.awssdk.services.s3.S3Client @RunWith(SpringRunner::class) @SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT) @Suppress("FunctionName") -class WorkspaceServiceRBACTest : CsmRedisTestBase() { +class WorkspaceServiceRBACTest : CsmS3TestBase() { val TEST_USER_MAIL = "testuser@mail.fr" val CONNECTED_ADMIN_USER = "test.admin@cosmotech.com" diff --git a/workspace/src/integrationTest/resources/application-workspace-test.yml b/workspace/src/integrationTest/resources/application-workspace-test.yml index 8e123c4d4..a39dab6f4 100644 --- a/workspace/src/integrationTest/resources/application-workspace-test.yml +++ b/workspace/src/integrationTest/resources/application-workspace-test.yml @@ -12,6 +12,20 @@ spring: client-type: jedis main: banner-mode: "off" + cloud: + aws: + credentials: + access-key: "accessKeyId" + secret-key: "secretAccessKey" + s3: + # We don't need/have a region for our local S3 service but the AWS SDK requires one to be configured + region: "region" + # Enable path-style / disable DNS-style + # By default, and for AWS S3, the client crafts its URL with the bucket as sub-domain of the endpoint + # which is not how our current S3 implementation works as it expects the bucket in the path + # './' DNS vs Path '//' + path-style-access-enabled: true + endpoint: "http://localhost:9000" csm: platform: diff --git a/workspace/src/main/kotlin/com/cosmotech/workspace/WorkspaceApiServiceInterface.kt b/workspace/src/main/kotlin/com/cosmotech/workspace/WorkspaceApiServiceInterface.kt index 1f145fe8b..052aad111 100644 --- a/workspace/src/main/kotlin/com/cosmotech/workspace/WorkspaceApiServiceInterface.kt +++ b/workspace/src/main/kotlin/com/cosmotech/workspace/WorkspaceApiServiceInterface.kt @@ -13,4 +13,6 @@ interface WorkspaceApiServiceInterface : WorkspaceApiService { workspaceId: String, requiredPermission: String = PERMISSION_READ ): Workspace + + fun deleteAllS3WorkspaceObjects(organizationId: String, workspace: Workspace) } diff --git a/workspace/src/main/kotlin/com/cosmotech/workspace/service/WorkspaceServiceImpl.kt b/workspace/src/main/kotlin/com/cosmotech/workspace/service/WorkspaceServiceImpl.kt index 18affa5c9..086d2b4bf 100644 --- a/workspace/src/main/kotlin/com/cosmotech/workspace/service/WorkspaceServiceImpl.kt +++ b/workspace/src/main/kotlin/com/cosmotech/workspace/service/WorkspaceServiceImpl.kt @@ -42,7 +42,7 @@ import com.cosmotech.workspace.domain.WorkspaceSecurity import com.cosmotech.workspace.domain.WorkspaceUpdateRequest import com.cosmotech.workspace.repository.WorkspaceRepository import io.awspring.cloud.s3.S3Template -import kotlinx.coroutines.GlobalScope +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.launch import org.springframework.beans.factory.config.ConfigurableBeanFactory import org.springframework.context.annotation.Scope @@ -146,7 +146,9 @@ internal class WorkspaceServiceImpl( override fun deleteWorkspaceFiles(organizationId: String, workspaceId: String) { val workspace = getVerifiedWorkspace(organizationId, workspaceId, PERMISSION_WRITE) - deleteAllS3WorkspaceObjects(organizationId, workspace) + CoroutineScope(SecurityCoroutineContext()).launch { + deleteAllS3WorkspaceObjects(organizationId, workspace) + } } override fun updateWorkspace( @@ -204,13 +206,13 @@ internal class WorkspaceServiceImpl( val workspace = getVerifiedWorkspace(organizationId, workspaceId, PERMISSION_DELETE) try { - deleteAllS3WorkspaceObjects(organizationId, workspace) + CoroutineScope(SecurityCoroutineContext()).launch { + deleteAllS3WorkspaceObjects(organizationId, workspace) + } workspace.linkedDatasetIdList?.forEach { deleteDatasetLink(organizationId, workspaceId, it) } } finally { workspaceRepository.delete(workspace) - if (csmPlatformProperties.internalResultServices?.enabled == true) { - this.eventPublisher.publishEvent(WorkspaceDeleted(this, organizationId, workspaceId)) - } + this.eventPublisher.publishEvent(WorkspaceDeleted(this, organizationId, workspaceId)) } } @@ -401,18 +403,14 @@ internal class WorkspaceServiceImpl( csmPlatformProperties.s3.bucketName, "$organizationId/${workspace.id}/$fileName") } - private fun deleteAllS3WorkspaceObjects(organizationId: String, workspace: Workspace) { + override fun deleteAllS3WorkspaceObjects(organizationId: String, workspace: Workspace) { logger.debug("Deleting all files for workspace #{} ({})", workspace.id, workspace.name) - - GlobalScope.launch(SecurityCoroutineContext()) { - // TODO Consider using a smaller coroutine scope - val workspaceFiles = getWorkspaceFiles(organizationId, workspace.id) - if (workspaceFiles.isEmpty()) { - logger.debug("No file to delete for workspace #{} ({})", workspace.id, workspace.name) - } else { - workspaceFiles.forEach { file -> - deleteS3WorkspaceObject(organizationId, workspace, file.fileName) - } + val workspaceFiles = getWorkspaceFiles(organizationId, workspace.id) + if (workspaceFiles.isEmpty()) { + logger.debug("No file to delete for workspace #{} ({})", workspace.id, workspace.name) + } else { + workspaceFiles.forEach { file -> + deleteS3WorkspaceObject(organizationId, workspace, file.fileName) } } } From e7e43ef7eb05c2c1276e9df6926a4641f7007da0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=A9my=20Reynard?= Date: Thu, 10 Apr 2025 12:38:04 +0200 Subject: [PATCH 4/5] Fix unit test in WorkspaceServiceImplTests --- .../service/WorkspaceServiceImplTests.kt | 36 ------------------- 1 file changed, 36 deletions(-) diff --git a/workspace/src/test/kotlin/com/cosmotech/workspace/service/WorkspaceServiceImplTests.kt b/workspace/src/test/kotlin/com/cosmotech/workspace/service/WorkspaceServiceImplTests.kt index 3a21e00bf..f2791e20f 100644 --- a/workspace/src/test/kotlin/com/cosmotech/workspace/service/WorkspaceServiceImplTests.kt +++ b/workspace/src/test/kotlin/com/cosmotech/workspace/service/WorkspaceServiceImplTests.kt @@ -54,16 +54,10 @@ import io.mockk.junit5.MockKExtension import io.mockk.mockk import io.mockk.mockkStatic import io.mockk.verify -import java.io.File -import java.nio.file.Files -import java.nio.file.Path import java.util.* import kotlin.test.Test import kotlin.test.assertEquals -import kotlin.test.assertFalse import kotlin.test.assertNotNull -import kotlin.test.assertTrue -import org.junit.jupiter.api.AfterEach import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.DynamicTest import org.junit.jupiter.api.TestFactory @@ -90,8 +84,6 @@ class WorkspaceServiceImplTests { @Suppress("unused") @RelaxedMockK private lateinit var s3Client: S3Client @RelaxedMockK private lateinit var s3Template: S3Template - private lateinit var blobPersistencePath: String - @Suppress("unused") @MockK private var eventPublisher: CsmEventPublisher = mockk(relaxed = true) @Suppress("unused") @MockK private var idGenerator: CsmIdGenerator = mockk(relaxed = true) @@ -142,16 +134,9 @@ class WorkspaceServiceImplTests { every { csmPlatformProperties.s3.bucketName } returns S3_BUCKET_NAME every { s3Template.objectExists(any(), any()) } returns false - blobPersistencePath = Files.createTempDirectory("cosmotech-api-test-data-").toString() - MockKAnnotations.init(this) } - @AfterEach - fun afterEach() { - File(blobPersistencePath).deleteRecursively() - } - @Test fun `In uploadWorkspaceFile, filename is used if no destination set`() { val workspace = mockk(relaxed = true) @@ -167,8 +152,6 @@ class WorkspaceServiceImplTests { workspaceServiceImpl.createWorkspaceFile(ORGANIZATION_ID, WORKSPACE_ID, file, false, null) assertNotNull(workspaceFile.fileName) assertEquals("my_file.txt", workspaceFile.fileName) - assertTrue( - Files.exists(Path.of(blobPersistencePath, ORGANIZATION_ID, WORKSPACE_ID, "my_file.txt"))) } @Test @@ -188,8 +171,6 @@ class WorkspaceServiceImplTests { workspaceServiceImpl.createWorkspaceFile(ORGANIZATION_ID, WORKSPACE_ID, file, false, " ") assertNotNull(workspaceFile.fileName) assertEquals("my_file.txt", workspaceFile.fileName) - assertTrue( - Files.exists(Path.of(blobPersistencePath, ORGANIZATION_ID, WORKSPACE_ID, "my_file.txt"))) } @Test @@ -208,10 +189,6 @@ class WorkspaceServiceImplTests { ORGANIZATION_ID, WORKSPACE_ID, file, false, "my/destination/") assertNotNull(workspaceFile.fileName) assertEquals("my/destination/my_file.txt", workspaceFile.fileName) - assertTrue( - Files.exists( - Path.of( - blobPersistencePath, ORGANIZATION_ID, WORKSPACE_ID, "my/destination/my_file.txt"))) } @Test @@ -230,9 +207,6 @@ class WorkspaceServiceImplTests { ORGANIZATION_ID, WORKSPACE_ID, file, false, "my/destination/file") assertNotNull(workspaceFile.fileName) assertEquals("my/destination/file", workspaceFile.fileName) - assertTrue( - Files.exists( - Path.of(blobPersistencePath, ORGANIZATION_ID, WORKSPACE_ID, "my/destination/file"))) } @Test @@ -251,10 +225,6 @@ class WorkspaceServiceImplTests { ORGANIZATION_ID, WORKSPACE_ID, file, false, "my//other/destination////////file") assertNotNull(workspaceFile.fileName) assertEquals("my/other/destination/file", workspaceFile.fileName) - assertTrue( - Files.exists( - Path.of( - blobPersistencePath, ORGANIZATION_ID, WORKSPACE_ID, "my/other/destination/file"))) } @Test @@ -263,7 +233,6 @@ class WorkspaceServiceImplTests { workspaceServiceImpl.createWorkspaceFile( ORGANIZATION_ID, WORKSPACE_ID, mockk(), false, "my/../other/destination/../../file") } - assertFalse(Files.exists(Path.of(blobPersistencePath, ORGANIZATION_ID, WORKSPACE_ID))) } @Test @@ -272,7 +241,6 @@ class WorkspaceServiceImplTests { workspaceServiceImpl.getWorkspaceFile( ORGANIZATION_ID, WORKSPACE_ID, "my/../../other/destination/file") } - assertFalse(Files.exists(Path.of(blobPersistencePath, ORGANIZATION_ID, WORKSPACE_ID))) } @Test @@ -442,10 +410,6 @@ class WorkspaceServiceImplTests { .map { (role, shouldThrow) -> rbacTest("Test RBAC download workspace file: $role", role, shouldThrow) { every { workspaceRepository.findBy(any(), any()) } returns Optional.of(it.workspace) - val filePath = - Path.of(blobPersistencePath, it.organization.id, it.workspace.id, "name") - Files.createDirectories(filePath.getParent()) - Files.createFile(filePath) workspaceServiceImpl.getWorkspaceFile(it.organization.id, it.workspace.id, "name") } } From 7be069793ec10e8224bb562c7d1f7daa5480cedd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=A9my=20Reynard?= Date: Thu, 10 Apr 2025 15:58:20 +0200 Subject: [PATCH 5/5] Dev review fix --- api/src/integrationTest/resources/application-test.yml | 7 ++++--- config/application-dev.sample.yml | 10 +--------- .../resources/application-connector-test.yml | 7 ++++--- .../resources/application-dataset-test.yml | 7 ++++--- .../resources/application-organization-test.yml | 7 ++++--- .../integrationTest/resources/application-run-test.yml | 7 ++++--- .../resources/application-runner-test.yml | 7 ++++--- .../resources/application-solution-test.yml | 8 ++++---- .../resources/application-workspace-test.yml | 7 ++++--- 9 files changed, 33 insertions(+), 34 deletions(-) diff --git a/api/src/integrationTest/resources/application-test.yml b/api/src/integrationTest/resources/application-test.yml index 23384f350..fca819e4a 100644 --- a/api/src/integrationTest/resources/application-test.yml +++ b/api/src/integrationTest/resources/application-test.yml @@ -15,11 +15,11 @@ spring: cloud: aws: credentials: - access-key: "accessKeyId" - secret-key: "secretAccessKey" + access-key: "s3_username" + secret-key: "s3_password" s3: # We don't need/have a region for our local S3 service but the AWS SDK requires one to be configured - region: "region" + region: "dummy" # Enable path-style / disable DNS-style # By default, and for AWS S3, the client crafts its URL with the bucket as sub-domain of the endpoint # which is not how our current S3 implementation works as it expects the bucket in the path @@ -124,5 +124,6 @@ csm: bucketName: "test-bucket" accessKeyId: "s3_username" secretAccessKey: "s3_password" + region: "dummy" diff --git a/config/application-dev.sample.yml b/config/application-dev.sample.yml index cf9d3248e..7b0dc29c0 100644 --- a/config/application-dev.sample.yml +++ b/config/application-dev.sample.yml @@ -40,15 +40,6 @@ spring: keycloak: truststore: certificate: "classpath:[fill-this-value].pem" # certificate file - cloud: - aws: - credentials: - access-key: "s3_username" - secret-key: "s3_password" - s3: - region: "dummy" - path-style-access-enabled: true - endpoint: "http://localhost:9000" csm: platform: @@ -86,6 +77,7 @@ csm: bucketName: "cosmotech-api" accessKeyId: "s3_username" secretAccessKey: "s3_password" + region: "dummy" argo: base-uri: "http://localhost:2746" workflows: diff --git a/connector/src/integrationTest/resources/application-connector-test.yml b/connector/src/integrationTest/resources/application-connector-test.yml index b7c03a48e..32875e946 100644 --- a/connector/src/integrationTest/resources/application-connector-test.yml +++ b/connector/src/integrationTest/resources/application-connector-test.yml @@ -15,11 +15,11 @@ spring: cloud: aws: credentials: - access-key: "accessKeyId" - secret-key: "secretAccessKey" + access-key: "s3_username" + secret-key: "s3_password" s3: # We don't need/have a region for our local S3 service but the AWS SDK requires one to be configured - region: "region" + region: "dummy" # Enable path-style / disable DNS-style # By default, and for AWS S3, the client crafts its URL with the bucket as sub-domain of the endpoint # which is not how our current S3 implementation works as it expects the bucket in the path @@ -114,3 +114,4 @@ csm: bucketName: "test-bucket" accessKeyId: "s3_username" secretAccessKey: "s3_password" + region: "dummy" diff --git a/dataset/src/integrationTest/resources/application-dataset-test.yml b/dataset/src/integrationTest/resources/application-dataset-test.yml index d177ad2a2..9e8f627f8 100644 --- a/dataset/src/integrationTest/resources/application-dataset-test.yml +++ b/dataset/src/integrationTest/resources/application-dataset-test.yml @@ -15,11 +15,11 @@ spring: cloud: aws: credentials: - access-key: "accessKeyId" - secret-key: "secretAccessKey" + access-key: "s3_username" + secret-key: "s3_password" s3: # We don't need/have a region for our local S3 service but the AWS SDK requires one to be configured - region: "region" + region: "dummy" # Enable path-style / disable DNS-style # By default, and for AWS S3, the client crafts its URL with the bucket as sub-domain of the endpoint # which is not how our current S3 implementation works as it expects the bucket in the path @@ -119,3 +119,4 @@ csm: bucketName: "test-bucket" accessKeyId: "s3_username" secretAccessKey: "s3_password" + region: "dummy" diff --git a/organization/src/integrationTest/resources/application-organization-test.yml b/organization/src/integrationTest/resources/application-organization-test.yml index 11327e038..70ac1b1c8 100644 --- a/organization/src/integrationTest/resources/application-organization-test.yml +++ b/organization/src/integrationTest/resources/application-organization-test.yml @@ -15,11 +15,11 @@ spring: cloud: aws: credentials: - access-key: "accessKeyId" - secret-key: "secretAccessKey" + access-key: "s3_username" + secret-key: "s3_password" s3: # We don't need/have a region for our local S3 service but the AWS SDK requires one to be configured - region: "region" + region: "dummy" # Enable path-style / disable DNS-style # By default, and for AWS S3, the client crafts its URL with the bucket as sub-domain of the endpoint # which is not how our current S3 implementation works as it expects the bucket in the path @@ -114,3 +114,4 @@ csm: bucketName: "test-bucket" accessKeyId: "s3_username" secretAccessKey: "s3_password" + region: "dummy" diff --git a/run/src/integrationTest/resources/application-run-test.yml b/run/src/integrationTest/resources/application-run-test.yml index ee206a2bf..91f3cfda5 100644 --- a/run/src/integrationTest/resources/application-run-test.yml +++ b/run/src/integrationTest/resources/application-run-test.yml @@ -15,11 +15,11 @@ spring: cloud: aws: credentials: - access-key: "accessKeyId" - secret-key: "secretAccessKey" + access-key: "s3_username" + secret-key: "s3_password" s3: # We don't need/have a region for our local S3 service but the AWS SDK requires one to be configured - region: "region" + region: "dummy" # Enable path-style / disable DNS-style # By default, and for AWS S3, the client crafts its URL with the bucket as sub-domain of the endpoint # which is not how our current S3 implementation works as it expects the bucket in the path @@ -158,3 +158,4 @@ csm: bucketName: "test-bucket" accessKeyId: "s3_username" secretAccessKey: "s3_password" + region: "dummy" diff --git a/runner/src/integrationTest/resources/application-runner-test.yml b/runner/src/integrationTest/resources/application-runner-test.yml index 604c8f76f..ef1da4c4c 100644 --- a/runner/src/integrationTest/resources/application-runner-test.yml +++ b/runner/src/integrationTest/resources/application-runner-test.yml @@ -11,11 +11,11 @@ spring: cloud: aws: credentials: - access-key: "accessKeyId" - secret-key: "secretAccessKey" + access-key: "s3_username" + secret-key: "s3_password" s3: # We don't need/have a region for our local S3 service but the AWS SDK requires one to be configured - region: "region" + region: "dummy" # Enable path-style / disable DNS-style # By default, and for AWS S3, the client crafts its URL with the bucket as sub-domain of the endpoint # which is not how our current S3 implementation works as it expects the bucket in the path @@ -114,3 +114,4 @@ csm: bucketName: "test-bucket" accessKeyId: "s3_username" secretAccessKey: "s3_password" + region: "dummy" diff --git a/solution/src/integrationTest/resources/application-solution-test.yml b/solution/src/integrationTest/resources/application-solution-test.yml index df1870f97..7d99a78fe 100644 --- a/solution/src/integrationTest/resources/application-solution-test.yml +++ b/solution/src/integrationTest/resources/application-solution-test.yml @@ -15,11 +15,11 @@ spring: cloud: aws: credentials: - access-key: "accessKeyId" - secret-key: "secretAccessKey" + access-key: "s3_username" + secret-key: "s3_password" s3: # We don't need/have a region for our local S3 service but the AWS SDK requires one to be configured - region: "region" + region: "dummy" # Enable path-style / disable DNS-style # By default, and for AWS S3, the client crafts its URL with the bucket as sub-domain of the endpoint # which is not how our current S3 implementation works as it expects the bucket in the path @@ -118,4 +118,4 @@ csm: bucketName: "test-bucket" accessKeyId: "s3_username" secretAccessKey: "s3_password" - region: "us-east-1" + region: "dummy" diff --git a/workspace/src/integrationTest/resources/application-workspace-test.yml b/workspace/src/integrationTest/resources/application-workspace-test.yml index a39dab6f4..cc4be4879 100644 --- a/workspace/src/integrationTest/resources/application-workspace-test.yml +++ b/workspace/src/integrationTest/resources/application-workspace-test.yml @@ -15,11 +15,11 @@ spring: cloud: aws: credentials: - access-key: "accessKeyId" - secret-key: "secretAccessKey" + access-key: "s3_username" + secret-key: "s3_password" s3: # We don't need/have a region for our local S3 service but the AWS SDK requires one to be configured - region: "region" + region: "dummy" # Enable path-style / disable DNS-style # By default, and for AWS S3, the client crafts its URL with the bucket as sub-domain of the endpoint # which is not how our current S3 implementation works as it expects the bucket in the path @@ -123,3 +123,4 @@ csm: bucketName: "test-bucket" accessKeyId: "s3_username" secretAccessKey: "s3_password" + region: "dummy"