From dab70880f964a4042f39cb1af4f2895d917b7bb2 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/plugins/repository-gcs/src/main/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageClientSettings.java b/plugins/repository-gcs/src/main/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageClientSettings.java index 09ba4aed4a4e3..f7d8bc1678e72 100644 --- a/plugins/repository-gcs/src/main/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageClientSettings.java +++ b/plugins/repository-gcs/src/main/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageClientSettings.java @@ -16,9 +16,7 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.core.TimeValue; -import java.io.IOException; import java.io.InputStream; -import java.io.UncheckedIOException; import java.net.URI; import java.util.Collection; import java.util.Collections; @@ -195,13 +193,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 Setting 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); @@ -211,8 +210,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/plugins/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageClientSettingsTests.java b/plugins/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageClientSettingsTests.java index ffb142a85c59d..16229b9f4f924 100644 --- a/plugins/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageClientSettingsTests.java +++ b/plugins/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageClientSettingsTests.java @@ -37,6 +37,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 { @@ -83,6 +84,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);