appengine: implement AppEngineTokenSource for 2nd gen runtimes #341
Conversation
Go 1.11 on App Engine standard is a "second generation" runtime, and second generation runtimes do not set the appengine build tag. appengine_hook.go was behind the appengine build tag, meaning that AppEngineTokenSource panicked on the go111 runtime, saying, "AppEngineTokenSource can only be used on App Engine." The second gen runtimes should use ComputeTokenSource, which is also what flex does [1]. This commit does two things to remedy the situation: 1. Put the pre-existing implementation of AppEngineTokenSource behind the appengine build tag since it only works on first gen App Engine runtimes. This leaves first gen behavior unchanged. 2. Add a new implementation of AppEngineTokenSource and tag it !appengine. This implementation will therefore be used by second gen App Engine standard runtimes and App Engine flexible. It delegates to ComputeTokenSource. The new AppEngineTokenSource implementation emits a log message informing the user that AppEngineTokenSource is deprecated for second gen runtimes and flex, instructing them to use DefaultTokenSource or ComputeTokenSource instead. The documentation is updated to say the same. In this way users will not break when upgrading from Go 1.9 to Go 1.11 on App Engine but they will be nudged toward the world where App Engine runtimes have less special behavior. findDefaultCredentials still calls AppEngineTokenSource for first gen runtimes and ComputeTokenSource for flex. Fixes #334 Test: I deployed an app that uses AppEngineTokenSource to Go 1.9 and Go 1.11 on App Engine standard and to Go 1.11 on App Engine flexible and it worked in all cases. Also verified that the log message is present on go111 and flex. [1] DefaultTokenSource did use ComputeTokenSource for flex but AppEngineTokenSource did not. AppEngineTokenSource is supported on flex, in the sense that it doesn't panic when used on flex in the way it does when used outside App Engine. However, AppEngineTokenSource makes an API call internally that isn't supported by default on flex, which emits a log instructing the user to enable the compat runtime. The compat runtimes are deprecated and deploys are blocked. This is a bad experience. This commit has the side effect of fixing this.
|
This PR (HEAD: be148ec) has been imported to Gerrit for code review. Please visit https://go-review.googlesource.com/c/oauth2/+/146177 to see it. Tip: You can toggle comments from me using the |
bradfitz
changed the title
|
Message from Gerrit User 12446: Uploaded patch set 2: Commit message was updated. Please don’t reply on this GitHub thread. Visit golang.org/cl/146177. |
|
Message from Gerrit User 5065: Patch Set 2: (4 comments) Please don’t reply on this GitHub thread. Visit golang.org/cl/146177. |
|
Message from Gerrit User 7935: Patch Set 2: Code-Review+1 Please don’t reply on this GitHub thread. Visit golang.org/cl/146177. |
|
Message from Gerrit User 17555: Patch Set 2: (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/146177. |
- Move AppEngineTokenSource documentation to one location - Add file comments - Log the deprecation message exactly once in the second generation and flex AppEngineTokenSource.
|
This PR (HEAD: 0f03d90) has been imported to Gerrit for code review. Please visit https://go-review.googlesource.com/c/oauth2/+/146177 to see it. Tip: You can toggle comments from me using the |
|
Message from Gerrit User 27052: Patch Set 3: (5 comments) Please don’t reply on this GitHub thread. Visit golang.org/cl/146177. |
|
Message from Gerrit User 5065: Patch Set 3: (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/146177. |
|
Message from Gerrit User 27052: Patch Set 3: (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/146177. |
|
Message from Gerrit User 7440: Patch Set 3: Code-Review+1 (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/146177. |
|
Message from Gerrit User 27052: Patch Set 3: (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/146177. |
appEngineTokenSource -> gaeTokenSource getAppEngineTokenSource -> appEngineTokenSource
|
This PR (HEAD: 75f52a8) has been imported to Gerrit for code review. Please visit https://go-review.googlesource.com/c/oauth2/+/146177 to see it. Tip: You can toggle comments from me using the |
|
Message from Gerrit User 27052: Patch Set 4: (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/146177. |
|
Message from Gerrit User 5065: Patch Set 4: Code-Review+2 (3 comments) Thanks! LGTM after minor tweak below. Please don’t reply on this GitHub thread. Visit golang.org/cl/146177. |
|
This PR (HEAD: 5779afb) has been imported to Gerrit for code review. Please visit https://go-review.googlesource.com/c/oauth2/+/146177 to see it. Tip: You can toggle comments from me using the |
|
Message from Gerrit User 27052: Patch Set 5: (3 comments) Please don’t reply on this GitHub thread. Visit golang.org/cl/146177. |
Go 1.11 on App Engine standard is a "second generation" runtime, and second generation runtimes do not set the appengine build tag. appengine_hook.go was behind the appengine build tag, meaning that AppEngineTokenSource panicked on the go111 runtime, saying, "AppEngineTokenSource can only be used on App Engine." The second gen runtimes should use ComputeTokenSource, which is also what flex does [1]. This commit does two things to remedy the situation: 1. Put the pre-existing implementation of AppEngineTokenSource behind the appengine build tag since it only works on first gen App Engine runtimes. This leaves first gen behavior unchanged. 2. Add a new implementation of AppEngineTokenSource and tag it !appengine. This implementation will therefore be used by second gen App Engine standard runtimes and App Engine flexible. It delegates to ComputeTokenSource. The new AppEngineTokenSource implementation emits a log message informing the user that AppEngineTokenSource is deprecated for second gen runtimes and flex, instructing them to use DefaultTokenSource or ComputeTokenSource instead. The documentation is updated to say the same. In this way users will not break when upgrading from Go 1.9 to Go 1.11 on App Engine but they will be nudged toward the world where App Engine runtimes have less special behavior. findDefaultCredentials still calls AppEngineTokenSource for first gen runtimes and ComputeTokenSource for flex. Fixes #334 Test: I deployed an app that uses AppEngineTokenSource to Go 1.9 and Go 1.11 on App Engine standard and to Go 1.11 on App Engine flexible and it worked in all cases. Also verified that the log message is present on go111 and flex. [1] DefaultTokenSource did use ComputeTokenSource for flex but AppEngineTokenSource did not. AppEngineTokenSource is supported on flex, in the sense that it doesn't panic when used on flex in the way it does when used outside App Engine. However, AppEngineTokenSource makes an API call internally that isn't supported by default on flex, which emits a log instructing the user to enable the compat runtime. The compat runtimes are deprecated and deploys are blocked. This is a bad experience. This commit has the side effect of fixing this. Change-Id: Iab63547b410535db60dcf204782d5b6b599a4e0c GitHub-Last-Rev: 5779afb GitHub-Pull-Request: #341 Reviewed-on: https://go-review.googlesource.com/c/146177 Reviewed-by: Brad Fitzpatrick <[email protected]>
|
This PR is being closed because golang.org/cl/146177 has been merged. |
PR #341 introduce some new import `x/net/context` in parallel of PR #339 replacing them with the standard context. This quick PR rename those imports. Change-Id: I94f7edbee851a733b8a307c2ea60923dd990bdb4 GitHub-Last-Rev: fbe7944 GitHub-Pull-Request: #342 Reviewed-on: https://go-review.googlesource.com/c/146837 Reviewed-by: Brad Fitzpatrick <[email protected]>
Go 1.11 on App Engine standard is a "second generation" runtime, and
second generation runtimes do not set the appengine build tag.
appengine_hook.go was behind the appengine build tag, meaning that
AppEngineTokenSource panicked on the go111 runtime, saying,
"AppEngineTokenSource can only be used on App Engine."
The second gen runtimes should use ComputeTokenSource, which is also
what flex does [1]. This commit does two things to remedy the situation:
the appengine build tag since it only works on first gen App Engine
runtimes. This leaves first gen behavior unchanged.
!appengine. This implementation will therefore be used by second gen
App Engine standard runtimes and App Engine flexible. It delegates
to ComputeTokenSource.
The new AppEngineTokenSource implementation emits a log message
informing the user that AppEngineTokenSource is deprecated for second
gen runtimes and flex, instructing them to use DefaultTokenSource or
ComputeTokenSource instead. The documentation is updated to say the
same.
In this way users will not break when upgrading from Go 1.9 to Go 1.11
on App Engine but they will be nudged toward the world where App Engine
runtimes have less special behavior.
findDefaultCredentials still calls AppEngineTokenSource for first gen
runtimes and ComputeTokenSource for flex.
Fixes #334
Test: I deployed an app that uses AppEngineTokenSource to Go 1.9 and
Go 1.11 on App Engine standard and to Go 1.11 on App Engine
flexible and it worked in all cases. Also verified that the log
message is present on go111 and flex.
[1] DefaultTokenSource did use ComputeTokenSource for flex but
AppEngineTokenSource did not. AppEngineTokenSource is supported on flex,
in the sense that it doesn't panic when used on flex in the way it does
when used outside App Engine. However, AppEngineTokenSource makes an API
call internally that isn't supported by default on flex, which emits a
log instructing the user to enable the compat runtime. The compat
runtimes are deprecated and deploys are blocked. This is a bad
experience. This commit has the side effect of fixing this.