Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,11 @@ public boolean isRetryDelayValid()
@AssertTrue(message = "Either gcs.auth-type or gcs.json-key or gcs.json-key-file-path must be set")
public boolean isAuthMethodValid()
{
// allow not specify any of configuration for SERVICE_ACCOUNT auth
if (getAuthType() == AuthType.SERVICE_ACCOUNT && jsonKey == null && jsonKeyFilePath == null) {
return true;
}

if (getAuthType() == AuthType.ACCESS_TOKEN) {
return jsonKey == null && jsonKeyFilePath == null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@
*/
package io.trino.filesystem.gcs;

import com.google.auth.Credentials;
import com.google.auth.oauth2.GoogleCredentials;
import com.google.cloud.NoCredentials;
import com.google.cloud.storage.StorageOptions;
import com.google.inject.Inject;
import io.trino.spi.security.ConnectorIdentity;
Expand All @@ -29,7 +31,7 @@
public class GcsServiceAccountAuth
implements GcsAuth
{
private final Optional<GoogleCredentials> jsonGoogleCredential;
private final Optional<Credentials> jsonGoogleCredential;

@Inject
public GcsServiceAccountAuth(GcsFileSystemConfig config)
Expand All @@ -56,18 +58,15 @@ else if (jsonKeyFilePath != null) {
@Override
public void setAuth(StorageOptions.Builder builder, ConnectorIdentity identity)
{
GoogleCredentials credentials = jsonGoogleCredential.orElseGet(() -> {
Credentials credentials = jsonGoogleCredential.orElseGet(() -> {
try {
return GoogleCredentials.getApplicationDefault();
}
catch (IOException e) {
// This is consistent with the GCP SDK when no credentials are available in the environment
return null;
Comment on lines -64 to -65
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd leave it as-is now.

return NoCredentials.getInstance();
}
});

if (credentials != null) {
builder.setCredentials(credentials);
}
builder.setCredentials(credentials);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import static io.airlift.units.DataSize.Unit.MEGABYTE;
import static java.util.concurrent.TimeUnit.MILLISECONDS;
import static java.util.concurrent.TimeUnit.SECONDS;
import static org.assertj.core.api.Assertions.assertThat;

public class TestGcsFileSystemConfig
{
Expand Down Expand Up @@ -136,7 +137,7 @@ public void testValidation()
assertFailsValidation(
new GcsFileSystemConfig()
.setAuthType(AuthType.ACCESS_TOKEN)
.setJsonKey("{}}"),
.setJsonKey("{}"),
"authMethodValid",
"Either gcs.auth-type or gcs.json-key or gcs.json-key-file-path must be set",
AssertTrue.class);
Expand Down Expand Up @@ -198,4 +199,10 @@ public void testValidation()
"Cannot set both gcs.use-access-token and gcs.auth-type",
AssertFalse.class);
}

@Test
void testAllowingNoConfigurationSpecifiedForServiceAccountAuth()
{
assertThat(new GcsFileSystemConfig().isAuthMethodValid()).isTrue();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
package io.trino.filesystem.gcs;

import com.google.auth.Credentials;
import com.google.cloud.NoCredentials;
import com.google.cloud.storage.Storage;
import com.google.cloud.storage.StorageOptions;
import io.trino.spi.security.ConnectorIdentity;
Expand All @@ -27,8 +28,6 @@ final class TestGcsStorageFactory
void testDefaultCredentials()
throws Exception
{
Credentials expectedCredentials = StorageOptions.newBuilder().build().getCredentials();

// No credentials options are set
GcsFileSystemConfig config = new GcsFileSystemConfig();

Expand All @@ -41,6 +40,8 @@ void testDefaultCredentials()

assertThat(actualCredentials)
.as("if credentials are not explicitly configured, should have same behavior as the GCS client")
.isEqualTo(expectedCredentials);
.isEqualTo(NoCredentials.getInstance());

assertThat(StorageOptions.newBuilder().build().getCredentials()).isNull();
}
}