Skip to content

Commit b1a543e

Browse files
chenjian2664wendigolosipiuk
committed
Add gcs.auth-type configuration
This makes it easier to add new GCS authentication types in the future. The commit also deprecate `gcs.use-access-token` Co-Authored-By: Mateusz "Serafin" Gajewski <[email protected]> Co-Authored-By: Łukasz Osipiuk <[email protected]>
1 parent 4071360 commit b1a543e

File tree

7 files changed

+137
-23
lines changed

7 files changed

+137
-23
lines changed

‎docs/src/main/sphinx/object-storage/file-system-gcs.md‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,15 @@ Cloud Storage:
7070
- Description
7171
* - `gcs.use-access-token`
7272
- Flag to set usage of a client-provided OAuth 2.0 token to access Google
73-
Cloud Storage. Defaults to `false`.
73+
Cloud Storage. Defaults to `false`, deprecated to use `gcs.auth-type` instead.
74+
* - `gcs.auth-type`
75+
- Authentication type to use for Google Cloud Storage access. Default to `SERVICE_ACCOUNT`.
76+
Supported values are:
77+
* `SERVICE_ACCOUNT`: loads credentials from the environment. Either `gcs.json-key` or
78+
`gcs.json-key-file-path` can be set in addition to override the default
79+
credentials provider.
80+
* `ACCESS_TOKEN`: usage of client-provided OAuth 2.0 token to access Google
81+
Cloud Storage.
7482
* - `gcs.json-key`
7583
- Your Google Cloud service account key in JSON format. Not to be set together
7684
with `gcs.json-key-file-path`.
@@ -103,7 +111,7 @@ Cloud Storage, make the following edits to your catalog configuration:
103111
- Native property
104112
- Notes
105113
* - `hive.gcs.use-access-token`
106-
- `gcs.use-access-token`
114+
- `gcs.auth-type`
107115
-
108116
* - `hive.gcs.json-key-file-path`
109117
- `gcs.json-key-file-path`

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

Lines changed: 37 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import io.airlift.units.Duration;
2222
import io.airlift.units.MinDuration;
2323
import jakarta.annotation.Nullable;
24+
import jakarta.validation.constraints.AssertFalse;
2425
import jakarta.validation.constraints.AssertTrue;
2526
import jakarta.validation.constraints.Min;
2627
import jakarta.validation.constraints.NotNull;
@@ -33,6 +34,12 @@
3334

3435
public class GcsFileSystemConfig
3536
{
37+
public enum AuthType
38+
{
39+
ACCESS_TOKEN,
40+
SERVICE_ACCOUNT;
41+
}
42+
3643
private DataSize readBlockSize = DataSize.of(2, MEGABYTE);
3744
private DataSize writeBlockSize = DataSize.of(16, MEGABYTE);
3845
private int pageSize = 100;
@@ -41,7 +48,8 @@ public class GcsFileSystemConfig
4148
private String projectId;
4249
private Optional<String> endpoint = Optional.empty();
4350

44-
private boolean useGcsAccessToken;
51+
private Optional<Boolean> useGcsAccessToken = Optional.empty();
52+
private Optional<AuthType> authType = Optional.empty();
4553
private String jsonKey;
4654
private String jsonKeyFilePath;
4755
private int maxRetries = 20;
@@ -134,15 +142,33 @@ public GcsFileSystemConfig setEndpoint(Optional<String> endpoint)
134142
return this;
135143
}
136144

145+
@NotNull
146+
public AuthType getAuthType()
147+
{
148+
if (useGcsAccessToken.isPresent() && useGcsAccessToken.get()) {
149+
return AuthType.ACCESS_TOKEN;
150+
}
151+
return authType.orElse(AuthType.SERVICE_ACCOUNT);
152+
}
153+
154+
@Config("gcs.auth-type")
155+
public GcsFileSystemConfig setAuthType(AuthType authType)
156+
{
157+
this.authType = Optional.of(authType);
158+
return this;
159+
}
160+
161+
@Deprecated
137162
public boolean isUseGcsAccessToken()
138163
{
139-
return useGcsAccessToken;
164+
return useGcsAccessToken.orElse(false);
140165
}
141166

167+
@Deprecated
142168
@Config("gcs.use-access-token")
143169
public GcsFileSystemConfig setUseGcsAccessToken(boolean useGcsAccessToken)
144170
{
145-
this.useGcsAccessToken = useGcsAccessToken;
171+
this.useGcsAccessToken = Optional.of(useGcsAccessToken);
146172
return this;
147173
}
148174

@@ -268,13 +294,19 @@ public boolean isRetryDelayValid()
268294
return minBackoffDelay.compareTo(maxBackoffDelay) <= 0;
269295
}
270296

271-
@AssertTrue(message = "Either gcs.use-access-token or gcs.json-key or gcs.json-key-file-path must be set")
297+
@AssertTrue(message = "Either gcs.auth-type or gcs.json-key or gcs.json-key-file-path must be set")
272298
public boolean isAuthMethodValid()
273299
{
274-
if (useGcsAccessToken) {
300+
if (getAuthType() == AuthType.ACCESS_TOKEN) {
275301
return jsonKey == null && jsonKeyFilePath == null;
276302
}
277303

278304
return (jsonKey == null) ^ (jsonKeyFilePath == null);
279305
}
306+
307+
@AssertFalse(message = "Cannot set both gcs.use-access-token and gcs.auth-type")
308+
public boolean isAuthTypeAndGcsAccessTokenConfigured()
309+
{
310+
return authType.isPresent() && useGcsAccessToken.isPresent();
311+
}
280312
}

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

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@
1717
import com.google.inject.Scopes;
1818
import io.airlift.configuration.AbstractConfigurationAwareModule;
1919

20-
import static io.airlift.configuration.ConditionalModule.conditionalModule;
2120
import static io.airlift.configuration.ConfigBinder.configBinder;
21+
import static io.airlift.configuration.SwitchModule.switchModule;
2222

2323
public class GcsFileSystemModule
2424
extends AbstractConfigurationAwareModule
@@ -30,10 +30,12 @@ protected void setup(Binder binder)
3030
binder.bind(GcsStorageFactory.class).in(Scopes.SINGLETON);
3131
binder.bind(GcsFileSystemFactory.class).in(Scopes.SINGLETON);
3232

33-
install(conditionalModule(
33+
install(switchModule(
3434
GcsFileSystemConfig.class,
35-
GcsFileSystemConfig::isUseGcsAccessToken,
36-
_ -> binder.bind(GcsAuth.class).to(GcsAccessTokenAuth.class).in(Scopes.SINGLETON),
37-
_ -> binder.bind(GcsAuth.class).to(GcsDefaultAuth.class).in(Scopes.SINGLETON)));
35+
GcsFileSystemConfig::getAuthType,
36+
type -> switch (type) {
37+
case ACCESS_TOKEN -> _ -> binder.bind(GcsAuth.class).to(GcsAccessTokenAuth.class).in(Scopes.SINGLETON);
38+
case SERVICE_ACCOUNT -> _ -> binder.bind(GcsAuth.class).to(GcsServiceAccountAuth.class).in(Scopes.SINGLETON);
39+
}));
3840
}
3941
}

‎lib/trino-filesystem-gcs/src/main/java/io/trino/filesystem/gcs/GcsDefaultAuth.java‎ renamed to ‎lib/trino-filesystem-gcs/src/main/java/io/trino/filesystem/gcs/GcsServiceAccountAuth.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,13 @@
2626

2727
import static java.nio.charset.StandardCharsets.UTF_8;
2828

29-
public class GcsDefaultAuth
29+
public class GcsServiceAccountAuth
3030
implements GcsAuth
3131
{
3232
private final Optional<GoogleCredentials> jsonGoogleCredential;
3333

3434
@Inject
35-
public GcsDefaultAuth(GcsFileSystemConfig config)
35+
public GcsServiceAccountAuth(GcsFileSystemConfig config)
3636
throws IOException
3737
{
3838
String jsonKey = config.getJsonKey();

‎lib/trino-filesystem-gcs/src/test/java/io/trino/filesystem/gcs/AbstractTestGcsFileSystem.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ protected void initialize(String gcpCredentialKey)
5656
// For gcp testing this corresponds to the Cluster Storage Admin and Cluster Storage Object Admin roles
5757
byte[] jsonKeyBytes = Base64.getDecoder().decode(gcpCredentialKey);
5858
GcsFileSystemConfig config = new GcsFileSystemConfig().setJsonKey(new String(jsonKeyBytes, UTF_8));
59-
GcsStorageFactory storageFactory = new GcsStorageFactory(config, new GcsDefaultAuth(config));
59+
GcsStorageFactory storageFactory = new GcsStorageFactory(config, new GcsServiceAccountAuth(config));
6060
this.gcsFileSystemFactory = new GcsFileSystemFactory(config, storageFactory);
6161
this.storage = storageFactory.create(ConnectorIdentity.ofUser("test"));
6262
String bucket = RemoteStorageHelper.generateBucketName();

‎lib/trino-filesystem-gcs/src/test/java/io/trino/filesystem/gcs/TestGcsFileSystemConfig.java‎

Lines changed: 79 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616
import com.google.common.collect.ImmutableMap;
1717
import io.airlift.units.DataSize;
1818
import io.airlift.units.Duration;
19+
import io.trino.filesystem.gcs.GcsFileSystemConfig.AuthType;
20+
import jakarta.validation.constraints.AssertFalse;
1921
import jakarta.validation.constraints.AssertTrue;
2022
import org.junit.jupiter.api.Test;
2123

@@ -41,9 +43,10 @@ void testDefaults()
4143
.setWriteBlockSize(DataSize.of(16, MEGABYTE))
4244
.setPageSize(100)
4345
.setBatchSize(100)
46+
.setUseGcsAccessToken(false)
4447
.setProjectId(null)
4548
.setEndpoint(Optional.empty())
46-
.setUseGcsAccessToken(false)
49+
.setAuthType(AuthType.SERVICE_ACCOUNT)
4750
.setJsonKey(null)
4851
.setJsonKeyFilePath(null)
4952
.setMaxRetries(20)
@@ -56,6 +59,43 @@ void testDefaults()
5659

5760
@Test
5861
void testExplicitPropertyMappings()
62+
{
63+
Map<String, String> properties = ImmutableMap.<String, String>builder()
64+
.put("gcs.read-block-size", "51MB")
65+
.put("gcs.write-block-size", "52MB")
66+
.put("gcs.page-size", "10")
67+
.put("gcs.batch-size", "11")
68+
.put("gcs.project-id", "project")
69+
.put("gcs.endpoint", "http://custom.dns.org:8000")
70+
.put("gcs.auth-type", "access_token")
71+
.put("gcs.client.max-retries", "10")
72+
.put("gcs.client.backoff-scale-factor", "4.0")
73+
.put("gcs.client.max-retry-time", "10s")
74+
.put("gcs.client.min-backoff-delay", "20ms")
75+
.put("gcs.client.max-backoff-delay", "20ms")
76+
.put("gcs.application-id", "application id")
77+
.buildOrThrow();
78+
79+
GcsFileSystemConfig expected = new GcsFileSystemConfig()
80+
.setReadBlockSize(DataSize.of(51, MEGABYTE))
81+
.setWriteBlockSize(DataSize.of(52, MEGABYTE))
82+
.setPageSize(10)
83+
.setBatchSize(11)
84+
.setProjectId("project")
85+
.setEndpoint(Optional.of("http://custom.dns.org:8000"))
86+
.setAuthType(AuthType.ACCESS_TOKEN)
87+
.setMaxRetries(10)
88+
.setBackoffScaleFactor(4.0)
89+
.setMaxRetryTime(new Duration(10, SECONDS))
90+
.setMinBackoffDelay(new Duration(20, MILLISECONDS))
91+
.setMaxBackoffDelay(new Duration(20, MILLISECONDS))
92+
.setApplicationId("application id");
93+
assertFullMapping(properties, expected, Set.of("gcs.json-key", "gcs.json-key-file-path", "gcs.use-access-token"));
94+
}
95+
96+
// backwards compatibility test, remove if use-access-token is removed
97+
@Test
98+
void testExplicitPropertyMappingsWithDeprecatedUseAccessToken()
5999
{
60100
Map<String, String> properties = ImmutableMap.<String, String>builder()
61101
.put("gcs.read-block-size", "51MB")
@@ -87,34 +127,34 @@ void testExplicitPropertyMappings()
87127
.setMinBackoffDelay(new Duration(20, MILLISECONDS))
88128
.setMaxBackoffDelay(new Duration(20, MILLISECONDS))
89129
.setApplicationId("application id");
90-
assertFullMapping(properties, expected, Set.of("gcs.json-key", "gcs.json-key-file-path"));
130+
assertFullMapping(properties, expected, Set.of("gcs.json-key", "gcs.json-key-file-path", "gcs.auth-type"));
91131
}
92132

93133
@Test
94134
public void testValidation()
95135
{
96136
assertFailsValidation(
97137
new GcsFileSystemConfig()
98-
.setUseGcsAccessToken(true)
138+
.setAuthType(AuthType.ACCESS_TOKEN)
99139
.setJsonKey("{}}"),
100140
"authMethodValid",
101-
"Either gcs.use-access-token or gcs.json-key or gcs.json-key-file-path must be set",
141+
"Either gcs.auth-type or gcs.json-key or gcs.json-key-file-path must be set",
102142
AssertTrue.class);
103143

104144
assertFailsValidation(
105145
new GcsFileSystemConfig()
106-
.setUseGcsAccessToken(true)
146+
.setAuthType(AuthType.ACCESS_TOKEN)
107147
.setJsonKeyFilePath("/dev/null"),
108148
"authMethodValid",
109-
"Either gcs.use-access-token or gcs.json-key or gcs.json-key-file-path must be set",
149+
"Either gcs.auth-type or gcs.json-key or gcs.json-key-file-path must be set",
110150
AssertTrue.class);
111151

112152
assertFailsValidation(
113153
new GcsFileSystemConfig()
114154
.setJsonKey("{}")
115155
.setJsonKeyFilePath("/dev/null"),
116156
"authMethodValid",
117-
"Either gcs.use-access-token or gcs.json-key or gcs.json-key-file-path must be set",
157+
"Either gcs.auth-type or gcs.json-key or gcs.json-key-file-path must be set",
118158
AssertTrue.class);
119159

120160
assertFailsValidation(
@@ -125,5 +165,37 @@ public void testValidation()
125165
"retryDelayValid",
126166
"gcs.client.min-backoff-delay must be less than or equal to gcs.client.max-backoff-delay",
127167
AssertTrue.class);
168+
169+
assertFailsValidation(
170+
new GcsFileSystemConfig()
171+
.setAuthType(AuthType.ACCESS_TOKEN)
172+
.setUseGcsAccessToken(true),
173+
"authTypeAndGcsAccessTokenConfigured",
174+
"Cannot set both gcs.use-access-token and gcs.auth-type",
175+
AssertFalse.class);
176+
177+
assertFailsValidation(
178+
new GcsFileSystemConfig()
179+
.setAuthType(AuthType.ACCESS_TOKEN)
180+
.setUseGcsAccessToken(false),
181+
"authTypeAndGcsAccessTokenConfigured",
182+
"Cannot set both gcs.use-access-token and gcs.auth-type",
183+
AssertFalse.class);
184+
185+
assertFailsValidation(
186+
new GcsFileSystemConfig()
187+
.setUseGcsAccessToken(true)
188+
.setAuthType(AuthType.SERVICE_ACCOUNT),
189+
"authTypeAndGcsAccessTokenConfigured",
190+
"Cannot set both gcs.use-access-token and gcs.auth-type",
191+
AssertFalse.class);
192+
193+
assertFailsValidation(
194+
new GcsFileSystemConfig()
195+
.setUseGcsAccessToken(false)
196+
.setAuthType(AuthType.SERVICE_ACCOUNT),
197+
"authTypeAndGcsAccessTokenConfigured",
198+
"Cannot set both gcs.use-access-token and gcs.auth-type",
199+
AssertFalse.class);
128200
}
129201
}

‎lib/trino-filesystem-gcs/src/test/java/io/trino/filesystem/gcs/TestGcsStorageFactory.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ void testDefaultCredentials()
3232
// No credentials options are set
3333
GcsFileSystemConfig config = new GcsFileSystemConfig();
3434

35-
GcsStorageFactory storageFactory = new GcsStorageFactory(config, new GcsDefaultAuth(config));
35+
GcsStorageFactory storageFactory = new GcsStorageFactory(config, new GcsServiceAccountAuth(config));
3636

3737
Credentials actualCredentials;
3838
try (Storage storage = storageFactory.create(ConnectorIdentity.ofUser("test"))) {

0 commit comments

Comments
 (0)