[solr] 01/02: SOLR-15501: Read GCS creds more permissively

[ge...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

gerlowskija pushed a commit to branch branch_9_0
in repository https://gitbox.apache.org/repos/asf/solr.git

commit c38125f1a78a95794f6152233b504785b263f47d
Author: Jason Gerlowski <ge...@apache.org>
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.
    
    Co-authored-by: Jacek Kikiewicz <pu...@kikiewicz.com>
    Co-authored-by: Martin Stocker <ma...@gmail.com>
---
 .../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 ++-
 4 files changed, 41 insertions(+), 17 deletions(-)

diff --git a/solr/modules/gcs-repository/src/java/org/apache/solr/gcs/GCSBackupRepository.java b/solr/modules/gcs-repository/src/java/org/apache/solr/gcs/GCSBackupRepository.java
index 5fba087..fb3b5a4 100644
--- a/solr/modules/gcs-repository/src/java/org/apache/solr/gcs/GCSBackupRepository.java
+++ b/solr/modules/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/modules/gcs-repository/src/java/org/apache/solr/gcs/GCSConfigParser.java b/solr/modules/gcs-repository/src/java/org/apache/solr/gcs/GCSConfigParser.java
index 17ec5dd..ad7f3de 100644
--- a/solr/modules/gcs-repository/src/java/org/apache/solr/gcs/GCSConfigParser.java
+++ b/solr/modules/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/modules/gcs-repository/src/test/org/apache/solr/gcs/GCSBackupRepositoryTest.java b/solr/modules/gcs-repository/src/test/org/apache/solr/gcs/GCSBackupRepositoryTest.java
index 69f92be..5cba2ad 100644
--- a/solr/modules/gcs-repository/src/test/org/apache/solr/gcs/GCSBackupRepositoryTest.java
+++ b/solr/modules/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`::
 +