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 <[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. Co-authored-by: Jacek Kikiewicz <[email protected]> Co-authored-by: Martin Stocker <[email protected]> --- .../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`:: +
