Browse Source

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.
David Turner 3 years ago
parent
commit
621c38cde5

+ 5 - 0
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: []

+ 5 - 6
modules/repository-gcs/src/main/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageClientSettings.java

@@ -18,9 +18,7 @@ import org.elasticsearch.common.settings.SettingsException;
 import org.elasticsearch.core.Nullable;
 import org.elasticsearch.core.Nullable;
 import org.elasticsearch.core.TimeValue;
 import org.elasticsearch.core.TimeValue;
 
 
-import java.io.IOException;
 import java.io.InputStream;
 import java.io.InputStream;
-import java.io.UncheckedIOException;
 import java.net.InetAddress;
 import java.net.InetAddress;
 import java.net.InetSocketAddress;
 import java.net.InetSocketAddress;
 import java.net.Proxy;
 import java.net.Proxy;
@@ -246,13 +244,14 @@ public class GoogleCloudStorageClientSettings {
      *         {@code null} if no service account is defined.
      *         {@code null} if no service account is defined.
      */
      */
     static ServiceAccountCredentials loadCredential(final Settings settings, final String clientName) {
     static ServiceAccountCredentials loadCredential(final Settings settings, final String clientName) {
+        final var credentialsFileSetting = CREDENTIALS_FILE_SETTING.getConcreteSettingForNamespace(clientName);
         try {
         try {
-            if (CREDENTIALS_FILE_SETTING.getConcreteSettingForNamespace(clientName).exists(settings) == false) {
+            if (credentialsFileSetting.exists(settings) == false) {
                 // explicitly returning null here so that the default credential
                 // explicitly returning null here so that the default credential
                 // can be loaded later when creating the Storage client
                 // can be loaded later when creating the Storage client
                 return null;
                 return null;
             }
             }
-            try (InputStream credStream = CREDENTIALS_FILE_SETTING.getConcreteSettingForNamespace(clientName).get(settings)) {
+            try (InputStream credStream = credentialsFileSetting.get(settings)) {
                 final Collection<String> scopes = Collections.singleton(StorageScopes.DEVSTORAGE_FULL_CONTROL);
                 final Collection<String> scopes = Collections.singleton(StorageScopes.DEVSTORAGE_FULL_CONTROL);
                 return SocketAccess.doPrivilegedIOException(() -> {
                 return SocketAccess.doPrivilegedIOException(() -> {
                     final ServiceAccountCredentials credentials = ServiceAccountCredentials.fromStream(credStream);
                     final ServiceAccountCredentials credentials = ServiceAccountCredentials.fromStream(credStream);
@@ -262,8 +261,8 @@ public class GoogleCloudStorageClientSettings {
                     return credentials;
                     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);
         }
         }
     }
     }
 
 

+ 20 - 0
modules/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageClientSettingsTests.java

@@ -43,6 +43,7 @@ import static org.elasticsearch.repositories.gcs.GoogleCloudStorageClientSetting
 import static org.elasticsearch.repositories.gcs.GoogleCloudStorageClientSettings.READ_TIMEOUT_SETTING;
 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.getClientSettings;
 import static org.elasticsearch.repositories.gcs.GoogleCloudStorageClientSettings.loadCredential;
 import static org.elasticsearch.repositories.gcs.GoogleCloudStorageClientSettings.loadCredential;
+import static org.hamcrest.Matchers.equalTo;
 
 
 public class GoogleCloudStorageClientSettingsTests extends ESTestCase {
 public class GoogleCloudStorageClientSettingsTests extends ESTestCase {
 
 
@@ -89,6 +90,25 @@ public class GoogleCloudStorageClientSettingsTests extends ESTestCase {
         assertGoogleCredential(expectedClientSettings.getCredential(), loadCredential(randomClient.v2(), clientName));
         assertGoogleCredential(expectedClientSettings.getCredential(), loadCredential(randomClient.v2(), clientName));
     }
     }
 
 
+    public void testLoadInvalidCredential() throws Exception {
+        final List<Setting<?>> 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 {
     public void testProjectIdDefaultsToCredentials() throws Exception {
         final String clientName = randomAlphaOfLength(5);
         final String clientName = randomAlphaOfLength(5);
         final Tuple<ServiceAccountCredentials, byte[]> credentials = randomCredential(clientName);
         final Tuple<ServiceAccountCredentials, byte[]> credentials = randomCredential(clientName);