Skip to content

Commit dab7088

Browse files
committed
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.
1 parent ada1852 commit dab7088

3 files changed

Lines changed: 30 additions & 6 deletions

File tree

docs/changelog/89336.yaml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
pr: 89336
2+
summary: Report better error for GCS credentials load failure
3+
area: Snapshot/Restore
4+
type: bug
5+
issues: []

plugins/repository-gcs/src/main/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageClientSettings.java

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,7 @@
1616
import org.elasticsearch.common.settings.Settings;
1717
import org.elasticsearch.core.TimeValue;
1818

19-
import java.io.IOException;
2019
import java.io.InputStream;
21-
import java.io.UncheckedIOException;
2220
import java.net.URI;
2321
import java.util.Collection;
2422
import java.util.Collections;
@@ -195,13 +193,14 @@ static GoogleCloudStorageClientSettings getClientSettings(final Settings setting
195193
* {@code null} if no service account is defined.
196194
*/
197195
static ServiceAccountCredentials loadCredential(final Settings settings, final String clientName) {
196+
final Setting<InputStream> credentialsFileSetting = CREDENTIALS_FILE_SETTING.getConcreteSettingForNamespace(clientName);
198197
try {
199-
if (CREDENTIALS_FILE_SETTING.getConcreteSettingForNamespace(clientName).exists(settings) == false) {
198+
if (credentialsFileSetting.exists(settings) == false) {
200199
// explicitly returning null here so that the default credential
201200
// can be loaded later when creating the Storage client
202201
return null;
203202
}
204-
try (InputStream credStream = CREDENTIALS_FILE_SETTING.getConcreteSettingForNamespace(clientName).get(settings)) {
203+
try (InputStream credStream = credentialsFileSetting.get(settings)) {
205204
final Collection<String> scopes = Collections.singleton(StorageScopes.DEVSTORAGE_FULL_CONTROL);
206205
return SocketAccess.doPrivilegedIOException(() -> {
207206
final ServiceAccountCredentials credentials = ServiceAccountCredentials.fromStream(credStream);
@@ -211,8 +210,8 @@ static ServiceAccountCredentials loadCredential(final Settings settings, final S
211210
return credentials;
212211
});
213212
}
214-
} catch (final IOException e) {
215-
throw new UncheckedIOException(e);
213+
} catch (final Exception e) {
214+
throw new IllegalArgumentException("failed to load GCS client credentials from [" + credentialsFileSetting.getKey() + "]", e);
216215
}
217216
}
218217

plugins/repository-gcs/src/test/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageClientSettingsTests.java

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
import static org.elasticsearch.repositories.gcs.GoogleCloudStorageClientSettings.READ_TIMEOUT_SETTING;
3838
import static org.elasticsearch.repositories.gcs.GoogleCloudStorageClientSettings.getClientSettings;
3939
import static org.elasticsearch.repositories.gcs.GoogleCloudStorageClientSettings.loadCredential;
40+
import static org.hamcrest.Matchers.equalTo;
4041

4142
public class GoogleCloudStorageClientSettingsTests extends ESTestCase {
4243

@@ -83,6 +84,25 @@ public void testLoadCredential() throws Exception {
8384
assertGoogleCredential(expectedClientSettings.getCredential(), loadCredential(randomClient.v2(), clientName));
8485
}
8586

87+
public void testLoadInvalidCredential() throws Exception {
88+
final List<Setting<?>> deprecationWarnings = new ArrayList<>();
89+
final Settings.Builder settings = Settings.builder();
90+
final MockSecureSettings secureSettings = new MockSecureSettings();
91+
final String clientName = randomBoolean() ? "default" : randomAlphaOfLength(5).toLowerCase(Locale.ROOT);
92+
randomClient(clientName, settings, secureSettings, deprecationWarnings);
93+
secureSettings.setFile(
94+
CREDENTIALS_FILE_SETTING.getConcreteSettingForNamespace(clientName).getKey(),
95+
"invalid".getBytes(StandardCharsets.UTF_8)
96+
);
97+
assertThat(
98+
expectThrows(
99+
IllegalArgumentException.class,
100+
() -> loadCredential(settings.setSecureSettings(secureSettings).build(), clientName)
101+
).getMessage(),
102+
equalTo("failed to load GCS client credentials from [gcs.client." + clientName + ".credentials_file]")
103+
);
104+
}
105+
86106
public void testProjectIdDefaultsToCredentials() throws Exception {
87107
final String clientName = randomAlphaOfLength(5);
88108
final Tuple<ServiceAccountCredentials, byte[]> credentials = randomCredential(clientName);

0 commit comments

Comments
 (0)