This is an automated email from the ASF dual-hosted git repository.
gerlowskija pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/solr.git
The following commit(s) were added to refs/heads/main by this push:
new 32a0fbd SOLR-15501: Read GCS creds more permissively
32a0fbd is described below
commit 32a0fbd69af0a9a943e2d5e5ae78334df5e09f0a
Author: Jason Gerlowski <[email protected]>
AuthorDate: Tue Jan 11 13:29:17 2022 -0500
SOLR-15501: Read GCS creds more permissively
Prior to this commit, GCSBackupRepository required all users to provide
a path to a file containing GCS credentials. It turns out that this was
overly strict, as GCP allows hosted code to authenticate implicitly with
whatever roles/permissions assigned to the the hosting server, VM, or pod.
Solr was unintentionally blocking this usecase.
This commit makes the `gcsCredentialPath` setting optional to better
support this usecase. If the credential path is absent, instead of
throwing an error, a warning is now logged to alert users that they
_might_ be missing this value if they're outside GCP.
Closes: #465
Co-authored-by: Jacek Kikiewicz <[email protected]>
Co-authored-by: Martin Stocker <[email protected]>
---
solr/CHANGES.txt | 4 ++++
.../org/apache/solr/gcs/GCSBackupRepository.java | 15 +++++++-------
.../java/org/apache/solr/gcs/GCSConfigParser.java | 16 ++++++++-------
.../apache/solr/gcs/GCSBackupRepositoryTest.java | 24 ++++++++++++++++++++--
solr/solr-ref-guide/src/backup-restore.adoc | 3 ++-
5 files changed, 45 insertions(+), 17 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 260b277..9b94c44 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -501,6 +501,10 @@ Bug Fixes
* SOLR-15854: Let RealtimeGet component support negative filters (Tomás
Fernández Löbbe)
+* SOLR-15501: GCSBackupRepository no longer strictly requires a pointer to a
service account JSON file, allowing users running within GCP
+ to take advantage of it's "Workload Identity" and other
role-based access feature. (Jacek Kikiewicz, Martin Stocker
+ via Jason Gerlowski)
+
================== 8.11.2 ==================
Bug Fixes
diff --git
a/solr/contrib/gcs-repository/src/java/org/apache/solr/gcs/GCSBackupRepository.java
b/solr/contrib/gcs-repository/src/java/org/apache/solr/gcs/GCSBackupRepository.java
index 5fba087..fb3b5a4 100644
---
a/solr/contrib/gcs-repository/src/java/org/apache/solr/gcs/GCSBackupRepository.java
+++
b/solr/contrib/gcs-repository/src/java/org/apache/solr/gcs/GCSBackupRepository.java
@@ -83,14 +83,15 @@ public class GCSBackupRepository implements
BackupRepository {
return storage;
try {
- if (credentialPath == null) {
- throw new
IllegalArgumentException(GCSConfigParser.missingCredentialErrorMsg());
+ if (credentialPath != null) {
+ log.info("Creating GCS client using credential at {}",
credentialPath);
+ // 'GoogleCredentials.fromStream' closes the input stream, so
we don't
+ GoogleCredentials credential =
GoogleCredentials.fromStream(new FileInputStream(credentialPath));
+ storageOptionsBuilder.setCredentials(credential);
+ } else {
+ // nowarn compile time string concatenation
+ log.warn(GCSConfigParser.potentiallyMissingCredentialMsg());
//nowarn
}
-
- log.info("Creating GCS client using credential at {}",
credentialPath);
- // 'GoogleCredentials.fromStream' closes the input stream, so we
don't
- GoogleCredentials credential = GoogleCredentials.fromStream(new
FileInputStream(credentialPath));
- storageOptionsBuilder.setCredentials(credential);
storage = storageOptionsBuilder.build().getService();
} catch (IOException e) {
throw new IllegalStateException(e);
diff --git
a/solr/contrib/gcs-repository/src/java/org/apache/solr/gcs/GCSConfigParser.java
b/solr/contrib/gcs-repository/src/java/org/apache/solr/gcs/GCSConfigParser.java
index 17ec5dd..ad7f3de 100644
---
a/solr/contrib/gcs-repository/src/java/org/apache/solr/gcs/GCSConfigParser.java
+++
b/solr/contrib/gcs-repository/src/java/org/apache/solr/gcs/GCSConfigParser.java
@@ -28,8 +28,8 @@ import java.util.Map;
* Parses configuration for {@link GCSBackupRepository} from NamedList and
environment variables
*/
public class GCSConfigParser {
- private static final String GCS_BUCKET_ENV_VAR_NAME = "GCS_BUCKET";
- private static final String GCS_CREDENTIAL_ENV_VAR_NAME =
"GCS_CREDENTIAL_PATH";
+ protected static final String GCS_BUCKET_ENV_VAR_NAME = "GCS_BUCKET";
+ protected static final String GCS_CREDENTIAL_ENV_VAR_NAME =
"GCS_CREDENTIAL_PATH";
private static final String GCS_BUCKET_PARAM_NAME = "gcsBucket";
private static final String GCS_CREDENTIAL_PARAM_NAME = "gcsCredentialPath";
@@ -93,11 +93,13 @@ public class GCSConfigParser {
return envVars.get(GCS_CREDENTIAL_ENV_VAR_NAME);
}
- public static String missingCredentialErrorMsg() {
- return "GCSBackupRepository requires a credential for GCS communication,
but none was provided. Please specify a " +
- "path to this GCS credential by adding a '" +
GCS_CREDENTIAL_PARAM_NAME + "' property to the repository " +
- "definition in your solrconfig, or by setting the path value in an
env-var named '" +
- GCS_CREDENTIAL_ENV_VAR_NAME + "'";
+ public static String potentiallyMissingCredentialMsg() {
+ return "No explicit credential path was provided for this
GCSBackupRepository. If Solr is running inside GCP, this " +
+ "may be expected if GCP's \"Workload Identity\" or a related
feature is configured. Solr instances " +
+ "running outside of GCP however must provide a valid credential
path in order to access GCS. These users " +
+ "should specify their credential path by adding a '" +
GCS_CREDENTIAL_PARAM_NAME + "' property to the " +
+ "repository definition in solconfig, or by setting the path value
in an env-var named '" +
+ GCS_CREDENTIAL_ENV_VAR_NAME + "'.";
}
private int getIntOrDefault(NamedList<?> config, String propName, int
defaultValue) {
diff --git
a/solr/contrib/gcs-repository/src/test/org/apache/solr/gcs/GCSBackupRepositoryTest.java
b/solr/contrib/gcs-repository/src/test/org/apache/solr/gcs/GCSBackupRepositoryTest.java
index 69f92be..5cba2ad 100644
---
a/solr/contrib/gcs-repository/src/test/org/apache/solr/gcs/GCSBackupRepositoryTest.java
+++
b/solr/contrib/gcs-repository/src/test/org/apache/solr/gcs/GCSBackupRepositoryTest.java
@@ -19,16 +19,22 @@ package org.apache.solr.gcs;
import com.google.common.collect.Lists;
import org.apache.solr.cloud.api.collections.AbstractBackupRepositoryTest;
-import org.apache.solr.common.params.CoreAdminParams;
import org.apache.solr.common.util.NamedList;
import org.apache.solr.core.backup.repository.BackupRepository;
import org.junit.AfterClass;
import org.junit.BeforeClass;
+import org.junit.Test;
import java.net.URI;
import java.net.URISyntaxException;
+import java.util.HashMap;
import java.util.List;
import java.util.Locale;
+import java.util.Map;
+
+import static org.apache.solr.common.params.CoreAdminParams.BACKUP_LOCATION;
+import static org.apache.solr.gcs.GCSConfigParser.GCS_BUCKET_ENV_VAR_NAME;
+import static org.apache.solr.gcs.GCSConfigParser.GCS_CREDENTIAL_ENV_VAR_NAME;
/**
* Unit tests for {@link GCSBackupRepository} that use an in-memory Storage
object
@@ -56,7 +62,7 @@ public class GCSBackupRepositoryTest extends
AbstractBackupRepositoryTest {
@Override
protected BackupRepository getRepository() {
final NamedList<Object> config = new NamedList<>();
- config.add(CoreAdminParams.BACKUP_LOCATION, "backup1");
+ config.add(BACKUP_LOCATION, "backup1");
final GCSBackupRepository repository = new
LocalStorageGCSBackupRepository();
repository.init(config);
@@ -67,4 +73,18 @@ public class GCSBackupRepositoryTest extends
AbstractBackupRepositoryTest {
protected URI getBaseUri() throws URISyntaxException {
return new URI("tmp");
}
+
+ @Test
+ public void testInitStoreDoesNotFailWithMissingCredentials()
+ {
+ Map<String, String> config = new HashMap<>();
+ config.put(GCS_BUCKET_ENV_VAR_NAME, "a_bucket_name");
+ // explicitly setting credential name to null; will work inside
google-cloud project
+ config.put(GCS_CREDENTIAL_ENV_VAR_NAME, null);
+ config.put(BACKUP_LOCATION, "/==");
+
+ BackupRepository gcsBackupRepository = getRepository();
+
+ gcsBackupRepository.init(new NamedList<>(config));
+ }
}
diff --git a/solr/solr-ref-guide/src/backup-restore.adoc
b/solr/solr-ref-guide/src/backup-restore.adoc
index b35eebc..22d0ccf 100644
--- a/solr/solr-ref-guide/src/backup-restore.adoc
+++ b/solr/solr-ref-guide/src/backup-restore.adoc
@@ -485,7 +485,8 @@ If both values are absent, the value `solrBackupsBucket`
will be used as a defau
+
A path on the local filesystem (accessible by Solr) to a
https://cloud.google.com/iam/docs/creating-managing-service-account-keys[Google
Cloud service account key] file.
If not specified, GCSBackupRepository will use the value of the
`GCS_CREDENTIAL_PATH` environment variable.
-If both values are absent, an error will be thrown as GCS requires credentials
for most usage.
+If both values are absent and Solr is running inside GCP, the GCS client will
attempt to authenticate using GCP's "Compute Engine Metadata Serve"r or
https://cloud.google.com/kubernetes-engine/docs/how-to/workload-identity[Workload
Identity] features.
+If both values are absent and Solr is running outside of GCP, it will be
unable to authenticate and any backup or restore operations will fail.
`location`::
+
