Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Bazel doesn't get Google Application Default Credentials correctly #3460

Closed
huangw5 opened this issue Jul 27, 2017 · 6 comments
Closed

Bazel doesn't get Google Application Default Credentials correctly #3460

huangw5 opened this issue Jul 27, 2017 · 6 comments
Assignees
@buchgr
Labels
P1 I'll work on this now. (Assignee required)

Comments

@huangw5
Copy link

huangw5 commented Jul 27, 2017

When getting the application default credentials, google-auth-library-java searches from five different sources in order (see doc and code) :

  1. Credentials file pointed to by the GOOGLE_APPLICATION_CREDENTIALS environment variable
  2. Credentials provided by the Google Cloud SDK gcloud auth application-default login command
  3. Google App Engine built-in credentials
  4. Google Cloud Shell built-in credentials
  5. Google Compute Engine built-in credentials

However, currently Bazel checks the first source only: If environment variable "GOOGLE_APPLICATION_CREDENTIALS" is empty, it simply returns null. This means we have to always provide the json credential file in order to upload build events to BES.

I did the following experiment:

  1. Built bazel from HEAD and ran bazel with --bes_backend set and --auth_credential unset: I always got UNAUTHENTICATED error:
ERROR: Build Event Protocol upload failed: UNAUTHENTICATED: UNAUTHENTICATED: Request is missing required authentication credential. Expected OAuth 2 access token, login cookie or other valid authentication crede
ntial. See https://developers.google.com/identity/sign-in/web/devconsole-project.
  1. Built bazel by removing the check against the environment variable !isNullOrEmpty(getenv(DEFAULT_APP_CREDENTIALS_ENV_VAR) and ran the above the same command.
INFO: Waiting for Build Event Protocol upload to finish.
INFO: Build Event Protocol upload finished successfully.
@buchgr buchgr self-assigned this Jul 27, 2017
@buchgr
Copy link
Contributor

buchgr commented Jul 27, 2017 •
edited
Loading

Thanks @huangw5! Will fix asap!

@damienmg we need this for 0.5.4

@buchgr buchgr added category: service APIs P1 I'll work on this now. (Assignee required) labels Jul 27, 2017
@buchgr
Copy link
Contributor

buchgr commented Jul 27, 2017

@huahang if you would like to fix this ... we love contributions :-)! Let me know!

@pmcalpine
Copy link

@buchgr Is there a reason that Bazel has any custom handling of the GOOGLE_APPLICATION_CREDENTIALS environment variable at all? I would have expected it to depend on GoogleCredentials.getApplicationDefault(...) to select the appropriate credentials source?

https://github.com/google/google-auth-library-java/blob/master/oauth2_http/java/com/google/auth/oauth2/GoogleCredentials.java#L100

@buchgr
Copy link
Contributor

buchgr commented Jul 27, 2017

@pmcalpine I don't think so. AFAIK it's been there since before I joined, so I can't say for sure, but we might not have been using google's auth library initially.

Anyway, I think it's safe to remove and we will include this in the next release.

@huangw5
Copy link
Author

huangw5 commented Jul 28, 2017

Great to hear that. Thanks a lot @buchgr!

@pmcalpine
Copy link

@buchgr: @huangw5 has created a pull request for this:
#3468

@pmcalpine pmcalpine mentioned this issue Aug 2, 2017
Closed
buchgr added a commit that referenced this issue Aug 7, 2017
@buchgr
grpc: Consolidate gRPC code from BES and Remote Execution. Fixes #3460,
38cacd0 
#3486

BES and Remote Execution have separate implementations of gRPC channel
creation, authentication and TLS. We should merge them, to avoid
duplication and bugs. One such bug is #3640, where the BES code had a
different implementation for Google Application Default Credentials.

RELNOTES: The Build Event Service (BES) client now properly supports
Google Applicaton Default Credentials.
PiperOrigin-RevId: 164253879
buchgr added a commit that referenced this issue Aug 11, 2017
@buchgr
grpc: Consolidate gRPC code from BES and Remote Execution. Fixes #3460,
37bd5c0 
#3486

BES and Remote Execution have separate implementations of gRPC channel
creation, authentication and TLS. We should merge them, to avoid
duplication and bugs. One such bug is #3640, where the BES code had a
different implementation for Google Application Default Credentials.

RELNOTES: The Build Event Service (BES) client now properly supports
Google Applicaton Default Credentials.
PiperOrigin-RevId: 164253879
buchgr added a commit that referenced this issue Aug 21, 2017
@buchgr
grpc: Consolidate gRPC code from BES and Remote Execution. Fixes #3460,
1dcf51e 
#3486

BES and Remote Execution have separate implementations of gRPC channel
creation, authentication and TLS. We should merge them, to avoid
duplication and bugs. One such bug is #3640, where the BES code had a
different implementation for Google Application Default Credentials.

RELNOTES: The Build Event Service (BES) client now properly supports
Google Applicaton Default Credentials.
PiperOrigin-RevId: 164253879
buchgr added a commit that referenced this issue Aug 22, 2017
@buchgr
grpc: Consolidate gRPC code from BES and Remote Execution. Fixes #3460,
7cfc436 
#3486

BES and Remote Execution have separate implementations of gRPC channel
creation, authentication and TLS. We should merge them, to avoid
duplication and bugs. One such bug is #3640, where the BES code had a
different implementation for Google Application Default Credentials.

RELNOTES: The Build Event Service (BES) client now properly supports
Google Applicaton Default Credentials.
PiperOrigin-RevId: 164253879
bazel-io pushed a commit that referenced this issue Aug 25, 2017
@buchgr
Release 0.5.4 (2017-08-25)
5371d13 
Baseline: 6563b2d

Cherry picks:
   + d4fa181:
     Use getExecPathString when getting bash_main_file
   + 837e1b3:
     Windows, sh_bin. launcher: export runfiles envvars
   + fe9ba89:
     grpc: Consolidate gRPC code from BES and Remote Execution. Fixes
     #3460, #3486
   + e8d4366:
     Automated rollback of commit
     496d3de.
   + 242a434:
     bes,remote: update default auth scope.
   + 793b409:
     Windows, sh_bin. launcher: fix manifest path
   + 7e4fbbe:
     Add --windows_exe_launcher option
   + 91fb38e:
     remote_worker: Serialize fork() calls. Fixes #3356
   + b79a9fc:
     Quote python_path and launcher in
     python_stub_template_windows.txt
   + 4a2e17f:
     Add build_windows_jni.sh back
   + ce61d63:
     don't use methods and classes removed in upstream dx RELNOTES:
     update dexing tools to Android SDK 26.0.1
   + 5393a49:
     Make Bazel enforce requirement on build-tools 26.0.1 or later.
   + 5fac03570f80856c063c6019f5beb3bdc1672dee:
     Fix --verbose_failures w/ sandboxing to print the full command
     line
   + f7bd1acf1f96bb7e3e19edb9483d9e07eb5af070:
     Only patch in C++ compile features when they are not already
     defined in crosstool
   + d7f5c12:
     Bump python-gflags to 3.1.0, take two
   + 3cb136d:
     Add python to bazel's dockerfiles

New features:

  - Do not disable fully dynamic linking with ThinLTO when invoked
    via LIPO options.

Important changes:

  - Ignore --glibc in the Android transition.
  - Remove --experimental_android_use_singlejar_for_multidex.
  - nocopts now also filter copts
  - The Build Event Service (BES) client now properly supports
    Google Applicaton Default Credentials.
  - update dexing tools to Android SDK 26.0.1
  - Bazel Android support now requires build-tools 26.0.1 or later.
  - Fix a bug in the remote_worker that would at times make it crash on Linux. See #3356
  - The java_proto_library rule now supports generated sources. See #2265
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@buchgr buchgr

Labels
P1 I'll work on this now. (Assignee required)
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@huangw5 @buchgr @pmcalpine