Revert "Make GCP OAuth scopes configurable via pipeline options. (apa…

…che#23644)" This reverts commit fdb1223.
AnandInguva · Oct 17, 2022 · 62ef2c1 · 62ef2c1
1 parent 693725d
commit 62ef2c1
Show file tree

Hide file tree

Showing 6 changed files with 49 additions and 79 deletions.
diff --git a/...form-core/src/main/java/org/apache/beam/sdk/extensions/gcp/auth/GcpCredentialFactory.java b/...form-core/src/main/java/org/apache/beam/sdk/extensions/gcp/auth/GcpCredentialFactory.java
@@ -34,40 +34,55 @@
  * credential.
  */
 public class GcpCredentialFactory implements CredentialFactory {
-  // A list of OAuth scopes to request when creating a credential.
-  private List<String> oauthScopes;
+  /**
+   * The scope cloud-platform provides access to all Cloud Platform resources. cloud-platform isn't
+   * sufficient yet for talking to datastore so we request those resources separately.
+   *
+   * <p>Note that trusted scope relationships don't apply to OAuth tokens, so for services we access
+   * directly (GCS) as opposed to through the backend (BigQuery, GCE), we need to explicitly request
+   * that scope.
+   */
+  private static final List<String> SCOPES =
+      Arrays.asList(
+          "https://www.googleapis.com/auth/cloud-platform",
+          "https://www.googleapis.com/auth/devstorage.full_control",
+          "https://www.googleapis.com/auth/userinfo.email",
+          "https://www.googleapis.com/auth/datastore",
+          "https://www.googleapis.com/auth/bigquery",
+          "https://www.googleapis.com/auth/bigquery.insertdata",
+          "https://www.googleapis.com/auth/pubsub");
+
   // If non-null, a list of service account emails to be used as an impersonation chain.
   private @Nullable List<String> impersonateServiceAccountChain;
 
-  private GcpCredentialFactory(
-      List<String> oauthScopes, @Nullable List<String> impersonateServiceAccountChain) {
+  private GcpCredentialFactory(@Nullable List<String> impersonateServiceAccountChain) {
     if (impersonateServiceAccountChain != null) {
       checkArgument(impersonateServiceAccountChain.size() > 0);
     }
 
-    this.oauthScopes = oauthScopes;
     this.impersonateServiceAccountChain = impersonateServiceAccountChain;
   }
 
   public static GcpCredentialFactory fromOptions(PipelineOptions options) {
-    GcpOptions gcpOptions = options.as(GcpOptions.class);
-    @Nullable String impersonateServiceAccountArg = gcpOptions.getImpersonateServiceAccount();
+    @Nullable
+    String impersonateServiceAccountArg =
+        options.as(GcpOptions.class).getImpersonateServiceAccount();
 
     @Nullable
     List<String> impersonateServiceAccountChain =
         impersonateServiceAccountArg == null
             ? null
             : Arrays.asList(impersonateServiceAccountArg.split(","));
 
-    return new GcpCredentialFactory(gcpOptions.getGcpOauthScopes(), impersonateServiceAccountChain);
+    return new GcpCredentialFactory(impersonateServiceAccountChain);
   }
 
   /** Returns a default GCP {@link Credentials} or null when it fails. */
   @Override
   public @Nullable Credentials getCredential() {
     try {
       GoogleCredentials applicationDefaultCredentials =
-          GoogleCredentials.getApplicationDefault().createScoped(oauthScopes);
+          GoogleCredentials.getApplicationDefault().createScoped(SCOPES);
 
       if (impersonateServiceAccountChain == null) {
         return applicationDefaultCredentials;
@@ -79,7 +94,7 @@ public static GcpCredentialFactory fromOptions(PipelineOptions options) {
 
         GoogleCredentials impersonationCredentials =
             ImpersonatedCredentials.create(
-                applicationDefaultCredentials, targetPrincipal, delegationChain, oauthScopes, 0);
+                applicationDefaultCredentials, targetPrincipal, delegationChain, SCOPES, 0);
 
         return impersonationCredentials;
       }

diff --git a/...ud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/options/GcpOptions.java b/...ud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/options/GcpOptions.java
@@ -37,8 +37,6 @@
 import java.nio.charset.StandardCharsets;
 import java.nio.file.FileAlreadyExistsException;
 import java.security.GeneralSecurityException;
-import java.util.Arrays;
-import java.util.List;
 import java.util.Locale;
 import java.util.Map;
 import java.util.regex.Matcher;
@@ -142,43 +140,6 @@ public interface GcpOptions extends GoogleApiDebugOptions, PipelineOptions {
 
   void setWorkerZone(String workerZone);
 
-  /**
-   * Controls the OAuth scopes that will be requested when creating {@link Credentials} with the
-   * {@link GcpCredentialFactory} (which is the default {@link CredentialFactory}). If the {@link
-   * #setGcpCredential credential} or {@link #setCredentialFactoryClass credential factory} have
-   * been set then this field may do nothing.
-   */
-  @Default.InstanceFactory(GcpOAuthScopesFactory.class)
-  @Description(
-      "Controls the OAuth scopes that will be requested when creating credentials with the GcpCredentialFactory (which is the default credential factory). If the GCP credential or credential factory have been set then this property may do nothing.")
-  List<String> getGcpOauthScopes();
-
-  void setGcpOauthScopes(List<String> oauthScopes);
-
-  /** Returns the default set of OAuth scopes. */
-  class GcpOAuthScopesFactory implements DefaultValueFactory<List<String>> {
-
-    @Override
-    public List<String> create(PipelineOptions options) {
-      /**
-       * The scope cloud-platform provides access to all Cloud Platform resources. cloud-platform
-       * isn't sufficient yet for talking to datastore so we request those resources separately.
-       *
-       * <p>Note that trusted scope relationships don't apply to OAuth tokens, so for services we
-       * access directly (GCS) as opposed to through the backend (BigQuery, GCE), we need to
-       * explicitly request that scope.
-       */
-      return Arrays.asList(
-          "https://www.googleapis.com/auth/cloud-platform",
-          "https://www.googleapis.com/auth/devstorage.full_control",
-          "https://www.googleapis.com/auth/userinfo.email",
-          "https://www.googleapis.com/auth/datastore",
-          "https://www.googleapis.com/auth/bigquery",
-          "https://www.googleapis.com/auth/bigquery.insertdata",
-          "https://www.googleapis.com/auth/pubsub");
-    }
-  }
-
   /**
    * The class of the credential factory that should be created and used to create credentials. If
    * gcpCredential has not been set explicitly, an instance of this class will be constructed and
@@ -330,7 +291,7 @@ public Credentials create(PipelineOptions options) {
     }
   }
 
-  /** EnableStreamingEngine defaults to false unless one of the two experiments is set. */
+  /** EneableStreamingEngine defaults to false unless one of the two experiments is set. */
   class EnableStreamingEngineFactory implements DefaultValueFactory<Boolean> {
     @Override
     public Boolean create(PipelineOptions options) {

diff --git a/sdks/python/apache_beam/internal/gcp/auth.py b/sdks/python/apache_beam/internal/gcp/auth.py
@@ -24,6 +24,7 @@
 import threading
 
 from apache_beam.options.pipeline_options import GoogleCloudOptions
+from apache_beam.options.pipeline_options import PipelineOptions
 
 # google.auth is only available when Beam is installed with the gcp extra.
 try:
@@ -43,6 +44,16 @@
 
 _LOGGER = logging.getLogger(__name__)
 
+CLIENT_SCOPES = [
+    'https://www.googleapis.com/auth/bigquery',
+    'https://www.googleapis.com/auth/cloud-platform',
+    'https://www.googleapis.com/auth/devstorage.full_control',
+    'https://www.googleapis.com/auth/userinfo.email',
+    'https://www.googleapis.com/auth/datastore',
+    'https://www.googleapis.com/auth/spanner.admin',
+    'https://www.googleapis.com/auth/spanner.data'
+]
+
 
 def set_running_in_gce(worker_executing_project):
   """For internal use only; no backwards-compatibility guarantees.
@@ -142,9 +153,7 @@ def _get_service_credentials(pipeline_options):
       return None
 
     try:
-      # pylint: disable=c-extension-no-member
-      credentials, _ = google.auth.default(
-          scopes=pipeline_options.view_as(GoogleCloudOptions).gcp_oauth_scopes)
+      credentials, _ = google.auth.default(scopes=CLIENT_SCOPES)  # pylint: disable=c-extension-no-member
       credentials = _Credentials._add_impersonation_credentials(
           credentials, pipeline_options)
       credentials = _ApitoolsCredentialsAdapter(credentials)
@@ -161,9 +170,14 @@ def _get_service_credentials(pipeline_options):
 
   @staticmethod
   def _add_impersonation_credentials(credentials, pipeline_options):
-    gcs_options = pipeline_options.view_as(GoogleCloudOptions)
-    impersonate_service_account = gcs_options.impersonate_service_account
-    scopes = gcs_options.gcp_oauth_scopes
+    if isinstance(pipeline_options, PipelineOptions):
+      gcs_options = pipeline_options.view_as(GoogleCloudOptions)
+      impersonate_service_account = gcs_options.impersonate_service_account
+    elif isinstance(pipeline_options, dict):
+      impersonate_service_account = pipeline_options.get(
+          'impersonate_service_account')
+    else:
+      return credentials
     if impersonate_service_account:
       _LOGGER.info('Impersonating: %s', impersonate_service_account)
       impersonate_accounts = impersonate_service_account.split(',')
@@ -173,6 +187,6 @@ def _add_impersonation_credentials(credentials, pipeline_options):
           source_credentials=credentials,
           target_principal=target_principal,
           delegates=delegate_to,
-          target_scopes=scopes,
+          target_scopes=CLIENT_SCOPES,
       )
     return credentials
diff --git a/sdks/python/apache_beam/io/gcp/bigquery_tools.py b/sdks/python/apache_beam/io/gcp/bigquery_tools.py
@@ -56,7 +56,6 @@
 from apache_beam.io.gcp.internal.clients import bigquery
 from apache_beam.metrics import monitoring_infos
 from apache_beam.options import value_provider
-from apache_beam.options.pipeline_options import PipelineOptions
 from apache_beam.transforms import DoFn
 from apache_beam.typehints.typehints import Any
 from apache_beam.utils import retry
@@ -332,7 +331,7 @@ class BigQueryWrapper(object):
   def __init__(self, client=None, temp_dataset_id=None, temp_table_ref=None):
     self.client = client or bigquery.BigqueryV2(
         http=get_new_http(),
-        credentials=auth.get_service_credentials(PipelineOptions()),
+        credentials=auth.get_service_credentials(None),
         response_encoding='utf8',
         additional_http_headers={
             "user-agent": "apache-beam-%s" % apache_beam.__version__

diff --git a/sdks/python/apache_beam/options/pipeline_options.py b/sdks/python/apache_beam/options/pipeline_options.py
@@ -640,14 +640,6 @@ class GoogleCloudOptions(PipelineOptions):
   COMPUTE_API_SERVICE = 'compute.googleapis.com'
   STORAGE_API_SERVICE = 'storage.googleapis.com'
   DATAFLOW_ENDPOINT = 'https://dataflow.googleapis.com'
-  OAUTH_SCOPES = [
-      'https://www.googleapis.com/auth/bigquery',
-      'https://www.googleapis.com/auth/cloud-platform',
-      'https://www.googleapis.com/auth/devstorage.full_control',
-      'https://www.googleapis.com/auth/userinfo.email',
-      'https://www.googleapis.com/auth/datastore',
-      'https://www.googleapis.com/auth/spanner'
-  ]
 
   @classmethod
   def _add_argparse_args(cls, parser):
@@ -781,16 +773,6 @@ def _add_argparse_args(cls, parser):
         'either a single service account as the impersonator, or a '
         'comma-separated list of service accounts to create an '
         'impersonation delegation chain.')
-    parser.add_argument(
-        '--gcp_oauth_scope',
-        '--gcp_oauth_scopes',
-        dest='gcp_oauth_scopes',
-        action='append',
-        default=cls.OAUTH_SCOPES,
-        help=(
-            'Controls the OAuth scopes that will be requested when creating '
-            'GCP credentials. Note: If set programmatically, must be set as a '
-            'list of strings'))
 
   def _create_default_gcs_bucket(self):
     try:

diff --git a/sdks/python/apache_beam/runners/interactive/utils.py b/sdks/python/apache_beam/runners/interactive/utils.py
@@ -35,7 +35,6 @@
 from apache_beam.internal.gcp import auth
 from apache_beam.internal.http_client import get_new_http
 from apache_beam.io.gcp.internal.clients import storage
-from apache_beam.options.pipeline_options import PipelineOptions
 from apache_beam.pipeline import Pipeline
 from apache_beam.portability.api import beam_runner_api_pb2
 from apache_beam.runners.interactive.caching.cacheable import Cacheable
@@ -453,7 +452,7 @@ def assert_bucket_exists(bucket_name):
   try:
     from apitools.base.py.exceptions import HttpError
     storage_client = storage.StorageV1(
-        credentials=auth.get_service_credentials(PipelineOptions()),
+        credentials=auth.get_service_credentials(None),
         get_credentials=False,
         http=get_new_http(),
         response_encoding='utf8')