From 621c38cde5e0795d2ab96dca9618f251a6d8e354 Mon Sep 17 00:00:00 2001 From: David Turner Date: Mon, 15 Aug 2022 10:42:50 +0100 Subject: [PATCH] Report better error for GCS credentials load failure (#89336) Today if the GCS credentials file setting is invalid we report some kind of JSON parsing error but it's not clear what JSON is being parsed so the error is hard to track down. This commit adds the problematic setting name to the exception message. --- docs/changelog/89336.yaml | 5 +++++ .../gcs/GoogleCloudStorageClientSettings.java | 11 +++++----- ...GoogleCloudStorageClientSettingsTests.java | 20 +++++++++++++++++++ 3 files changed, 30 insertions(+), 6 deletions(-) create mode 100644 docs/changelog/89336.yaml diff --git a/docs/changelog/89336.yaml b/docs/changelog/89336.yaml new file mode 100644 index 0000000000000..4dde7e4545c47 --- /dev/null +++ b/docs/changelog/89336.yaml @@ -0,0 +1,5 @@ +pr: 89336 +summary: Report better error for GCS credentials load failure +area: Snapshot/Restore +type: bug +issues: [] diff --git a/modules/repository-gcs/src/main/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageClientSettings.java b/modules/repository-gcs/src/main/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageClientSettings.java index 7a510e8215bbb..e52fa91a61ade 100644 --- a/modules/repository-gcs/src/main/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageClientSettings.java +++ b/modules/repository-gcs/src/main/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageClientSettings.java @@ -18,9 +18,7 @@ import org.elasticsearch.core.Nullable; import org.elasticsearch.core.TimeValue; -import java.io.IOException; import java.io.InputStream; -import java.io.UncheckedIOException; import java.net.InetAddress; import java.net.InetSocketAddress; import java.net.Proxy; @@ -246,13 +244,14 @@ static GoogleCloudStorageClientSettings getClientSettings(final Settings setting * {@code null} if no service account is defined. */ static ServiceAccountCredentials loadCredential(final Settings settings, final String clientName) { + final var credentialsFileSetting = CREDENTIALS_FILE_SETTING.getConcreteSettingForNamespace(clientName); try { - if (CREDENTIALS_FILE_SETTING.getConcreteSettingForNamespace(clientName).exists(settings) == false) { + if (credentialsFileSetting.exists(settings) == false) { // explicitly returning null here so that the default credential // can be loaded later when creating the Storage client return null; } - try (InputStream credStream = CREDENTIALS_FILE_SETTING.getConcreteSettingForNamespace(clientName).get(settings)) { + try (InputStream credStream = credentialsFileSetting.get(settings)) { final Collection scopes = Collections.singleton(StorageScopes.DEVSTORAGE_FULL_CONTROL); return SocketAccess.doPrivilegedIOException(() -> { final ServiceAccountCredentials credentials = ServiceAccountCredentials.fromStream(credStream); @@ -262,8 +261,8 @@ static ServiceAccountCredentials loadCredential(final Settings settings, final S return credentials; }); } - } catch (final IOException e) { - throw new UncheckedIOException(e); + } catch (final Exception e) { + throw new IllegalArgumentException("failed to load GCS client credentials from [" + credentialsFileSetting.getKey() + "]", e); } } diff --git a/modules/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageClientSettingsTests.java b/modules/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageClientSettingsTests.java index 0ce4932ad4edd..d2813ff3c56c8 100644 --- a/modules/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageClientSettingsTests.java +++ b/modules/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageClientSettingsTests.java @@ -43,6 +43,7 @@ import static org.elasticsearch.repositories.gcs.GoogleCloudStorageClientSettings.READ_TIMEOUT_SETTING; import static org.elasticsearch.repositories.gcs.GoogleCloudStorageClientSettings.getClientSettings; import static org.elasticsearch.repositories.gcs.GoogleCloudStorageClientSettings.loadCredential; +import static org.hamcrest.Matchers.equalTo; public class GoogleCloudStorageClientSettingsTests extends ESTestCase { @@ -89,6 +90,25 @@ public void testLoadCredential() throws Exception { assertGoogleCredential(expectedClientSettings.getCredential(), loadCredential(randomClient.v2(), clientName)); } + public void testLoadInvalidCredential() throws Exception { + final List> deprecationWarnings = new ArrayList<>(); + final Settings.Builder settings = Settings.builder(); + final MockSecureSettings secureSettings = new MockSecureSettings(); + final String clientName = randomBoolean() ? "default" : randomAlphaOfLength(5).toLowerCase(Locale.ROOT); + randomClient(clientName, settings, secureSettings, deprecationWarnings); + secureSettings.setFile( + CREDENTIALS_FILE_SETTING.getConcreteSettingForNamespace(clientName).getKey(), + "invalid".getBytes(StandardCharsets.UTF_8) + ); + assertThat( + expectThrows( + IllegalArgumentException.class, + () -> loadCredential(settings.setSecureSettings(secureSettings).build(), clientName) + ).getMessage(), + equalTo("failed to load GCS client credentials from [gcs.client." + clientName + ".credentials_file]") + ); + } + public void testProjectIdDefaultsToCredentials() throws Exception { final String clientName = randomAlphaOfLength(5); final Tuple credentials = randomCredential(clientName);