Skip to content

Commit 26201f0

Browse files
kavetikaveti
committed
Refactor Iceberg GCS vending tests and remove encryption key mechanism
- Rewrite TestIcebergGcsVendingRestCatalog as TestIcebergGcsVendingRestCatalogConnectorSmokeTest extending BaseIcebergConnectorSmokeTest with real GCS credentials - Create IcebergGcsRestCatalogBackendContainer for GCS-specific REST catalog backend - Rename IcebergRestCatalogBackendContainer to IcebergS3RestCatalogBackendContainer - Remove old test class and GcsCredentialVendingCatalogAdapter - Remove default encryption/decryption key mechanism from GcsFileSystem and GcsFileSystemFactory to align with S3FileSystem and AzureFileSystem (only per-call newEncrypted* methods) - Remove encryption key constants from GcsFileSystemConstants - Remove encryption key mapping from IcebergRestCatalogFileSystemFactory - Add new GCS vending test to cloud-tests profile in pom.xml
1 parent b4492a9 commit 26201f0

File tree

12 files changed

+393
-529
lines changed

12 files changed

+393
-529
lines changed

‎lib/trino-filesystem-gcs/src/main/java/io/trino/filesystem/gcs/GcsFileSystem.java‎

Lines changed: 4 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -76,40 +76,23 @@ public class GcsFileSystem
7676
private final long writeBlockSizeBytes;
7777
private final int pageSize;
7878
private final int batchSize;
79-
private final Optional<EncryptionKey> defaultEncryptionKey;
80-
private final Optional<EncryptionKey> defaultDecryptionKey;
8179

8280
public GcsFileSystem(ListeningExecutorService executorService, Storage storage, int readBlockSizeBytes, long writeBlockSizeBytes, int pageSize, int batchSize)
83-
{
84-
this(executorService, storage, readBlockSizeBytes, writeBlockSizeBytes, pageSize, batchSize, Optional.empty(), Optional.empty());
85-
}
86-
87-
public GcsFileSystem(
88-
ListeningExecutorService executorService,
89-
Storage storage,
90-
int readBlockSizeBytes,
91-
long writeBlockSizeBytes,
92-
int pageSize,
93-
int batchSize,
94-
Optional<EncryptionKey> defaultEncryptionKey,
95-
Optional<EncryptionKey> defaultDecryptionKey)
9681
{
9782
this.executorService = requireNonNull(executorService, "executorService is null");
9883
this.storage = requireNonNull(storage, "storage is null");
9984
this.readBlockSizeBytes = readBlockSizeBytes;
10085
this.writeBlockSizeBytes = writeBlockSizeBytes;
10186
this.pageSize = pageSize;
10287
this.batchSize = batchSize;
103-
this.defaultEncryptionKey = requireNonNull(defaultEncryptionKey, "defaultEncryptionKey is null");
104-
this.defaultDecryptionKey = requireNonNull(defaultDecryptionKey, "defaultDecryptionKey is null");
10588
}
10689

10790
@Override
10891
public TrinoInputFile newInputFile(Location location)
10992
{
11093
GcsLocation gcsLocation = new GcsLocation(location);
11194
checkIsValidFile(gcsLocation);
112-
return new GcsInputFile(gcsLocation, storage, readBlockSizeBytes, OptionalLong.empty(), Optional.empty(), defaultDecryptionKey);
95+
return new GcsInputFile(gcsLocation, storage, readBlockSizeBytes, OptionalLong.empty(), Optional.empty(), Optional.empty());
11396
}
11497

11598
@Override
@@ -125,7 +108,7 @@ public TrinoInputFile newInputFile(Location location, long length)
125108
{
126109
GcsLocation gcsLocation = new GcsLocation(location);
127110
checkIsValidFile(gcsLocation);
128-
return new GcsInputFile(gcsLocation, storage, readBlockSizeBytes, OptionalLong.of(length), Optional.empty(), defaultDecryptionKey);
111+
return new GcsInputFile(gcsLocation, storage, readBlockSizeBytes, OptionalLong.of(length), Optional.empty(), Optional.empty());
129112
}
130113

131114
@Override
@@ -141,7 +124,7 @@ public TrinoInputFile newInputFile(Location location, long length, Instant lastM
141124
{
142125
GcsLocation gcsLocation = new GcsLocation(location);
143126
checkIsValidFile(gcsLocation);
144-
return new GcsInputFile(gcsLocation, storage, readBlockSizeBytes, OptionalLong.of(length), Optional.of(lastModified), defaultDecryptionKey);
127+
return new GcsInputFile(gcsLocation, storage, readBlockSizeBytes, OptionalLong.of(length), Optional.of(lastModified), Optional.empty());
145128
}
146129

147130
@Override
@@ -157,7 +140,7 @@ public TrinoOutputFile newOutputFile(Location location)
157140
{
158141
GcsLocation gcsLocation = new GcsLocation(location);
159142
checkIsValidFile(gcsLocation);
160-
return new GcsOutputFile(gcsLocation, storage, writeBlockSizeBytes, defaultEncryptionKey);
143+
return new GcsOutputFile(gcsLocation, storage, writeBlockSizeBytes, Optional.empty());
161144
}
162145

163146
@Override

‎lib/trino-filesystem-gcs/src/main/java/io/trino/filesystem/gcs/GcsFileSystemFactory.java‎

Lines changed: 1 addition & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -17,17 +17,11 @@
1717
import com.google.inject.Inject;
1818
import io.trino.filesystem.TrinoFileSystem;
1919
import io.trino.filesystem.TrinoFileSystemFactory;
20-
import io.trino.filesystem.encryption.EncryptionKey;
2120
import io.trino.spi.security.ConnectorIdentity;
2221
import jakarta.annotation.PreDestroy;
2322

24-
import java.util.Base64;
25-
import java.util.Optional;
26-
2723
import static com.google.common.util.concurrent.MoreExecutors.listeningDecorator;
2824
import static io.airlift.concurrent.Threads.daemonThreadsNamed;
29-
import static io.trino.filesystem.gcs.GcsFileSystemConstants.EXTRA_CREDENTIALS_GCS_DECRYPTION_KEY_PROPERTY;
30-
import static io.trino.filesystem.gcs.GcsFileSystemConstants.EXTRA_CREDENTIALS_GCS_ENCRYPTION_KEY_PROPERTY;
3125
import static java.lang.Math.toIntExact;
3226
import static java.util.Objects.requireNonNull;
3327
import static java.util.concurrent.Executors.newCachedThreadPool;
@@ -62,18 +56,6 @@ public void stop()
6256
@Override
6357
public TrinoFileSystem create(ConnectorIdentity identity)
6458
{
65-
Optional<EncryptionKey> defaultEncryptionKey = extractEncryptionKey(identity, EXTRA_CREDENTIALS_GCS_ENCRYPTION_KEY_PROPERTY);
66-
Optional<EncryptionKey> defaultDecryptionKey = extractEncryptionKey(identity, EXTRA_CREDENTIALS_GCS_DECRYPTION_KEY_PROPERTY);
67-
return new GcsFileSystem(executorService, storageFactory.create(identity), readBlockSizeBytes, writeBlockSizeBytes, pageSize, batchSize, defaultEncryptionKey, defaultDecryptionKey);
68-
}
69-
70-
private static Optional<EncryptionKey> extractEncryptionKey(ConnectorIdentity identity, String extraCredentialKey)
71-
{
72-
String base64Key = identity.getExtraCredentials().get(extraCredentialKey);
73-
if (base64Key == null) {
74-
return Optional.empty();
75-
}
76-
byte[] keyBytes = Base64.getDecoder().decode(base64Key);
77-
return Optional.of(new EncryptionKey(keyBytes, "AES256"));
59+
return new GcsFileSystem(executorService, storageFactory.create(identity), readBlockSizeBytes, writeBlockSizeBytes, pageSize, batchSize);
7860
}
7961
}

‎lib/trino-filesystem/src/main/java/io/trino/filesystem/gcs/GcsFileSystemConstants.java‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,6 @@ public final class GcsFileSystemConstants
2121
public static final String EXTRA_CREDENTIALS_GCS_SERVICE_HOST_PROPERTY = "internal$gcs_service_host";
2222
public static final String EXTRA_CREDENTIALS_GCS_NO_AUTH_PROPERTY = "internal$gcs_no_auth";
2323
public static final String EXTRA_CREDENTIALS_GCS_USER_PROJECT_PROPERTY = "internal$gcs_user_project";
24-
public static final String EXTRA_CREDENTIALS_GCS_ENCRYPTION_KEY_PROPERTY = "internal$gcs_encryption_key";
25-
public static final String EXTRA_CREDENTIALS_GCS_DECRYPTION_KEY_PROPERTY = "internal$gcs_decryption_key";
2624

2725
private GcsFileSystemConstants() {}
2826
}

‎plugin/trino-iceberg/pom.xml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -845,6 +845,7 @@
845845
<include>**/TestIcebergS3TablesConnectorSmokeTest.java</include>
846846
<include>**/TestIcebergBigLakeMetastoreConnectorSmokeTest.java</include>
847847
<include>**/TestIcebergGcsConnectorSmokeTest.java</include>
848+
<include>**/TestIcebergGcsVendingRestCatalogConnectorSmokeTest.java</include>
848849
<include>**/TestIcebergAbfsConnectorSmokeTest.java</include>
849850
<include>**/TestIcebergSnowflakeCatalogConnectorSmokeTest.java</include>
850851
<include>**/TestTrinoSnowflakeCatalog.java</include>

‎plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/rest/IcebergRestCatalogFileSystemFactory.java‎

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,6 @@
2222

2323
import java.util.Map;
2424

25-
import static io.trino.filesystem.gcs.GcsFileSystemConstants.EXTRA_CREDENTIALS_GCS_DECRYPTION_KEY_PROPERTY;
26-
import static io.trino.filesystem.gcs.GcsFileSystemConstants.EXTRA_CREDENTIALS_GCS_ENCRYPTION_KEY_PROPERTY;
2725
import static io.trino.filesystem.gcs.GcsFileSystemConstants.EXTRA_CREDENTIALS_GCS_NO_AUTH_PROPERTY;
2826
import static io.trino.filesystem.gcs.GcsFileSystemConstants.EXTRA_CREDENTIALS_GCS_OAUTH_TOKEN_EXPIRES_AT_PROPERTY;
2927
import static io.trino.filesystem.gcs.GcsFileSystemConstants.EXTRA_CREDENTIALS_GCS_OAUTH_TOKEN_PROPERTY;
@@ -48,8 +46,6 @@ public class IcebergRestCatalogFileSystemFactory
4846
private static final String VENDED_GCS_SERVICE_HOST = "gcs.service.host";
4947
private static final String VENDED_GCS_NO_AUTH = "gcs.no-auth";
5048
private static final String VENDED_GCS_USER_PROJECT = "gcs.user-project";
51-
private static final String VENDED_GCS_ENCRYPTION_KEY = "gcs.encryption-key";
52-
private static final String VENDED_GCS_DECRYPTION_KEY = "gcs.decryption-key";
5349

5450
private final TrinoFileSystemFactory fileSystemFactory;
5551
private final boolean vendedCredentialsEnabled;
@@ -92,9 +88,6 @@ public TrinoFileSystem create(ConnectorIdentity identity, Map<String, String> fi
9288
addOptionalProperty(extraCredentialsBuilder, fileIoProperties, VENDED_GCS_SERVICE_HOST, EXTRA_CREDENTIALS_GCS_SERVICE_HOST_PROPERTY);
9389
addOptionalProperty(extraCredentialsBuilder, fileIoProperties, VENDED_GCS_NO_AUTH, EXTRA_CREDENTIALS_GCS_NO_AUTH_PROPERTY);
9490
addOptionalProperty(extraCredentialsBuilder, fileIoProperties, VENDED_GCS_USER_PROJECT, EXTRA_CREDENTIALS_GCS_USER_PROJECT_PROPERTY);
95-
addOptionalProperty(extraCredentialsBuilder, fileIoProperties, VENDED_GCS_ENCRYPTION_KEY, EXTRA_CREDENTIALS_GCS_ENCRYPTION_KEY_PROPERTY);
96-
addOptionalProperty(extraCredentialsBuilder, fileIoProperties, VENDED_GCS_DECRYPTION_KEY, EXTRA_CREDENTIALS_GCS_DECRYPTION_KEY_PROPERTY);
97-
9891
ConnectorIdentity identityWithExtraCredentials = ConnectorIdentity.forUser(identity.getUser())
9992
.withGroups(identity.getGroups())
10093
.withPrincipal(identity.getPrincipal())
@@ -114,9 +107,7 @@ private static boolean hasAnyGcsVendedProperty(Map<String, String> fileIoPropert
114107
fileIoProperties.containsKey(VENDED_GCS_NO_AUTH) ||
115108
fileIoProperties.containsKey(VENDED_GCS_PROJECT_ID) ||
116109
fileIoProperties.containsKey(VENDED_GCS_SERVICE_HOST) ||
117-
fileIoProperties.containsKey(VENDED_GCS_USER_PROJECT) ||
118-
fileIoProperties.containsKey(VENDED_GCS_ENCRYPTION_KEY) ||
119-
fileIoProperties.containsKey(VENDED_GCS_DECRYPTION_KEY);
110+
fileIoProperties.containsKey(VENDED_GCS_USER_PROJECT);
120111
}
121112

122113
private static void addOptionalProperty(

0 commit comments

Comments
 (0)