Visitar URL original
fix: do not use the GAE APIs on gen2+ runtimes by zevdg · Pull Request #807 · googleapis/google-auth-library-python · GitHub
Skip to content
This repository was archived by the owner on Mar 6, 2026. It is now read-only.

fix: do not use the GAE APIs on gen2+ runtimes - #807

Merged
arithmetic1728 merged 3 commits into
googleapis:masterfrom
zevdg:gae-gen2-apis
Jul 23, 2021
Merged

fix: do not use the GAE APIs on gen2+ runtimes#807
arithmetic1728 merged 3 commits into
googleapis:masterfrom
zevdg:gae-gen2-apis

Conversation

@zevdg

@zevdg zevdg commented Jul 20, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor

Currently, this library uses the App Engine credentials in all environments if
the APIs can be imported successfully. This assumption made sense when
the API was only available on gen1, but this is no longer the case.
See https://github.com/GoogleCloudPlatform/appengine-python-standard

In order to comply with AIP-4115, we must treat GAE gen2+ as a "compute
engine equivalent environment" even if the GAE APIs are importable.
In other words, google.auth.default() should not return an
app_engine.Credental on GAE gen2+.

@zevdg
zevdg requested a review from a team July 20, 2021 16:01
@google-cla google-cla Bot added the cla: yes This human has signed the Contributor License Agreement. label Jul 20, 2021
@zevdg
zevdg force-pushed the gae-gen2-apis branch 2 times, most recently from d0490ce to f4f3f0f Compare July 20, 2021 16:08
@zevdg

zevdg commented Jul 20, 2021

Copy link
Copy Markdown
Contributor Author

It's worth noting that this change could be disruptive if a user migrates their app from GAE gen1 to GAE gen2 with a version of google.auth that doesn't include this change. Then, instead of the ADC changing when they upgrade runtimes as the spec dictates, their ADC would change when they upgrade this library to include this change. As such, any migration guides should encourage users to upgrade this library to the latest py2-compatible version BEFORE migrating to gen2.

Conversely, if we do not make this change, then gen2 users could run into the opposite problem. Their ADC could suddenly start returning an app_engine.Credential instead of a compute_engine.Credential if they (or one of their dependencies) adds appengine-python-standard to their requirements.txt.

The sooner we land this change, the less people will be affected by either of those scenarios.

busunkim96
busunkim96 reviewed Jul 20, 2021
View reviewed changes
Comment thread google/auth/_default.py Resolved
@busunkim96

busunkim96 commented Jul 20, 2021

Copy link
Copy Markdown
Contributor

CC @wescpy and @engelke since they're listed as DevRel contacts for this project.

Asking folks to update to a minimum version of google-auth is captured in a comment in a documentation bug (186268245).

@busunkim96 busunkim96 added the kokoro:run Add this label to force Kokoro to re-run the tests. label Jul 20, 2021
@yoshi-kokoro yoshi-kokoro removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Jul 20, 2021
@busunkim96 busunkim96 added the kokoro:run Add this label to force Kokoro to re-run the tests. label Jul 20, 2021
@yoshi-kokoro yoshi-kokoro removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Jul 20, 2021
@busunkim96 busunkim96 changed the title do not use the GAE APIs on gen2+ runtimes fix: do not use the GAE APIs on gen2+ runtimes Jul 21, 2021
@busunkim96 busunkim96 added automerge Merge the pull request once unit tests and other checks pass. kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Jul 21, 2021
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 21, 2021
@busunkim96 busunkim96 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 21, 2021
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 21, 2021
@arithmetic1728

arithmetic1728 commented Jul 21, 2021

Copy link
Copy Markdown
Contributor

@zevdg Kokoro build failed because coverage test failed. It seems we need some additional unit tests to cover google/auth/_default.py line 243-245 and 251-255.

@gcf-merge-on-green

gcf-merge-on-green Bot commented Jul 22, 2021

Copy link
Copy Markdown
Contributor

Merge-on-green attempted to merge your PR for 6 hours, but it was not mergeable because either one of your required status checks failed, one of your required reviews was not approved, or there is a do not merge label. Learn more about your required status checks here: https://help.github.com/en/github/administering-a-repository/enabling-required-status-checks. You can remove and reapply the label to re-run the bot.

@gcf-merge-on-green gcf-merge-on-green Bot removed the automerge Merge the pull request once unit tests and other checks pass. label Jul 22, 2021
tseaver
tseaver reviewed Jul 22, 2021
View reviewed changes
Comment thread tests/test__default.py Resolved
@zevdg
fix: do not use the GAE APIs on gen2+ runtimes
162ee03 
Currently, this library uses the App Engine API in all environments if
it can be imported successfully. This assumption made sense when the API
was only available on gen1, but this is no longer the case.
See https://github.com/GoogleCloudPlatform/appengine-python-standard

In order to comply with AIP-4115, we must treat GAE gen2+ as a "compute
engine equivalent environment" even if the GAE APIs are importable.
In other words, google.auth.default() must never return an
app_engine.Credental on GAE gen2+.Currently, this library uses the App Engine API in all environments if
it can be imported successfully. This assumption made sense when the API
was only available on gen1, but this is no longer the case.
See https://github.com/GoogleCloudPlatform/appengine-python-standard

In order to comply with AIP-4115, we must treat GAE gen2+ as a "compute
engine equivalent environment" even if the GAE APIs are importable.
In other words, google.auth.default() should not return an
app_engine.Credental on GAE gen2+.
@busunkim96 busunkim96 added automerge Merge the pull request once unit tests and other checks pass. kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Jul 23, 2021
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 23, 2021
@busunkim96 busunkim96 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 23, 2021
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 23, 2021
@busunkim96 busunkim96 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 23, 2021
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 23, 2021
@arithmetic1728
arithmetic1728 merged commit 7f7d92d into googleapis:master Jul 23, 2021
@gcf-merge-on-green gcf-merge-on-green Bot removed the automerge Merge the pull request once unit tests and other checks pass. label Jul 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@arithmetic1728 arithmetic1728 Awaiting requested review from arithmetic1728
4 more reviewers
@tseaver tseaver tseaver left review comments
@silvolu silvolu silvolu approved these changes
@crwilcox crwilcox crwilcox approved these changes
@busunkim96 busunkim96 busunkim96 approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@zevdg @busunkim96 @arithmetic1728 @tseaver @silvolu @crwilcox @yoshi-kokoro