diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b905c857b1d6..d9a01e238e23 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -720,9 +720,10 @@ jobs: - name: GCS FileSystem Cloud Tests env: GCP_CREDENTIALS_KEY: ${{ secrets.GCP_CREDENTIALS_KEY }} + GCP_STORAGE_BUCKET: ${{ vars.GCP_STORAGE_BUCKET }} if: >- contains(matrix.modules, 'trino-filesystem-gcs') && contains(matrix.profile, 'cloud-tests') && - (env.CI_SKIP_SECRETS_PRESENCE_CHECKS != '' || env.GCP_CREDENTIALS_KEY != '') + (env.CI_SKIP_SECRETS_PRESENCE_CHECKS != '' || env.GCP_CREDENTIALS_KEY != '' || env.GCP_STORAGE_BUCKET != '') run: | $MAVEN test ${MAVEN_TEST} -pl ${{ matrix.modules }} ${{ format('-P {0}', matrix.profile) }} - name: Cloud Delta Lake Tests diff --git a/lib/trino-filesystem-gcs/src/test/java/io/trino/filesystem/gcs/AbstractTestGcsFileSystem.java b/lib/trino-filesystem-gcs/src/test/java/io/trino/filesystem/gcs/AbstractTestGcsFileSystem.java index 805dea0176fc..d2c358a8a1f5 100644 --- a/lib/trino-filesystem-gcs/src/test/java/io/trino/filesystem/gcs/AbstractTestGcsFileSystem.java +++ b/lib/trino-filesystem-gcs/src/test/java/io/trino/filesystem/gcs/AbstractTestGcsFileSystem.java @@ -13,9 +13,8 @@ */ package io.trino.filesystem.gcs; -import com.google.cloud.storage.BucketInfo; import com.google.cloud.storage.Storage; -import com.google.cloud.storage.testing.RemoteStorageHelper; +import com.google.cloud.storage.Storage.BlobListOption; import io.trino.filesystem.AbstractTestTrinoFileSystem; import io.trino.filesystem.Location; import io.trino.filesystem.TrinoFileSystem; @@ -24,12 +23,11 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.TestInstance; -import java.io.ByteArrayInputStream; import java.io.IOException; -import java.io.InputStream; import java.util.Base64; -import java.util.concurrent.TimeUnit; +import java.util.UUID; +import static com.google.common.base.Preconditions.checkState; import static java.nio.charset.StandardCharsets.UTF_8; import static java.util.Objects.requireNonNull; import static org.assertj.core.api.Assertions.assertThat; @@ -48,31 +46,29 @@ protected static String getRequiredEnvironmentVariable(String name) return requireNonNull(System.getenv(name), "Environment variable not set: " + name); } - protected void initialize(String gcpCredentialKey) + protected void initialize(String gcpCredentialKey, String bucket) throws IOException { // Note: the account needs the following permissions: - // create/get/delete bucket + // get bucket // create/get/list/delete blob // For gcp testing this corresponds to the Cluster Storage Admin and Cluster Storage Object Admin roles byte[] jsonKeyBytes = Base64.getDecoder().decode(gcpCredentialKey); GcsFileSystemConfig config = new GcsFileSystemConfig().setJsonKey(new String(jsonKeyBytes, UTF_8)); - GcsStorageFactory storageFactory = new TestingGcsStorageFactory(config); + GcsStorageFactory storageFactory = new DefaultGcsStorageFactory(config); this.gcsFileSystemFactory = new GcsFileSystemFactory(config, storageFactory); this.storage = storageFactory.create(ConnectorIdentity.ofUser("test")); - String bucket = RemoteStorageHelper.generateBucketName(); - storage.create(BucketInfo.of(bucket)); - this.rootLocation = Location.of("gs://%s/".formatted(bucket)); + // The bucket must exist. + checkState(storage.get(bucket) != null, "Bucket not found: '%s'", bucket); + this.rootLocation = Location.of("gs://%s/%s/".formatted(bucket, UUID.randomUUID())); this.fileSystem = gcsFileSystemFactory.create(ConnectorIdentity.ofUser("test")); cleanupFiles(); } @AfterAll void tearDown() - throws Exception { try { - RemoteStorageHelper.forceDelete(storage, rootLocation.host().get(), 5, TimeUnit.SECONDS); gcsFileSystemFactory.stop(); } finally { @@ -117,8 +113,8 @@ protected Location getRootLocation() @Override protected void verifyFileSystemIsEmpty() { - String bucket = new GcsLocation(rootLocation).bucket(); - assertThat(storage.list(bucket).iterateAll()).isEmpty(); + GcsLocation gcsLocation = new GcsLocation(rootLocation); + assertThat(storage.list(gcsLocation.bucket(), BlobListOption.prefix(gcsLocation.path())).iterateAll()).isEmpty(); } @Override @@ -126,25 +122,4 @@ protected final boolean supportsRenameFile() { return false; } - - private static class TestingGcsStorageFactory - implements GcsStorageFactory - { - private final Storage storage; - - public TestingGcsStorageFactory(GcsFileSystemConfig config) - { - requireNonNull(config, "config is null"); - InputStream inputStream = new ByteArrayInputStream(config.getJsonKey().getBytes(UTF_8)); - // Note: the default project id from the credentials file will be used. See StorageOptions.setProjectId() - RemoteStorageHelper helper = RemoteStorageHelper.create(null, inputStream); - this.storage = helper.getOptions().getService(); - } - - @Override - public Storage create(ConnectorIdentity identity) - { - return storage; - } - } } diff --git a/lib/trino-filesystem-gcs/src/test/java/io/trino/filesystem/gcs/TestGcsFileSystemGcs.java b/lib/trino-filesystem-gcs/src/test/java/io/trino/filesystem/gcs/TestGcsFileSystemGcs.java index b9a056e487fc..bf6187847d8c 100644 --- a/lib/trino-filesystem-gcs/src/test/java/io/trino/filesystem/gcs/TestGcsFileSystemGcs.java +++ b/lib/trino-filesystem-gcs/src/test/java/io/trino/filesystem/gcs/TestGcsFileSystemGcs.java @@ -26,6 +26,6 @@ public class TestGcsFileSystemGcs void setup() throws IOException { - initialize(getRequiredEnvironmentVariable("GCP_CREDENTIALS_KEY")); + initialize(getRequiredEnvironmentVariable("GCP_CREDENTIALS_KEY"), getRequiredEnvironmentVariable("GCP_STORAGE_BUCKET")); } } diff --git a/lib/trino-filesystem/src/test/java/io/trino/filesystem/AbstractTestTrinoFileSystem.java b/lib/trino-filesystem/src/test/java/io/trino/filesystem/AbstractTestTrinoFileSystem.java index 82b42b052158..d80913c1df23 100644 --- a/lib/trino-filesystem/src/test/java/io/trino/filesystem/AbstractTestTrinoFileSystem.java +++ b/lib/trino-filesystem/src/test/java/io/trino/filesystem/AbstractTestTrinoFileSystem.java @@ -867,20 +867,21 @@ public void testDirectoryExists() try (Closer closer = Closer.create()) { String directoryName = "testDirectoryExistsDir"; String fileName = "file.csv"; - + String subPath = "%s/%s".formatted(directoryName, fileName); assertThat(listPath("")).isEmpty(); - assertThat(getFileSystem().directoryExists(getRootLocation())).contains(true); + Optional expectedExists = isHierarchical() || getRootLocation().path().isEmpty() ? Optional.of(true) : Optional.empty(); + assertThat(getFileSystem().directoryExists(getRootLocation())).isEqualTo(expectedExists); if (isHierarchical()) { assertThat(getFileSystem().directoryExists(createLocation(directoryName))).contains(false); - createBlob(closer, createLocation(directoryName).appendPath(fileName).path()); + createBlob(closer, subPath); assertThat(getFileSystem().directoryExists(createLocation(directoryName))).contains(true); assertThat(getFileSystem().directoryExists(createLocation(UUID.randomUUID().toString()))).contains(false); assertThat(getFileSystem().directoryExists(createLocation(directoryName).appendPath(fileName))).contains(false); } else { assertThat(getFileSystem().directoryExists(createLocation(directoryName))).isEmpty(); - createBlob(closer, createLocation(directoryName).appendPath(fileName).path()); + createBlob(closer, subPath); assertThat(getFileSystem().directoryExists(createLocation(directoryName))).contains(true); assertThat(getFileSystem().directoryExists(createLocation(UUID.randomUUID().toString()))).isEmpty(); assertThat(getFileSystem().directoryExists(createLocation(directoryName).appendPath(fileName))).isEmpty();