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.

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

x/blog, x/tour, x/talks: deploy with Go 1.11 runtime of App Engine Standard #30486

Closed
3 tasks done
dmitshur opened this issue Feb 28, 2019 · 11 comments
Closed
3 tasks done
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@dmitshur
Copy link
Contributor

dmitshur commented Feb 28, 2019

The following services should to be updated to deploy using Go 1.11 runtime of App Engine Standard:

  • x/blog – currently using Go 1.9 runtime; see its app.yaml#L3
  • x/tour – currently using Go 1.9 runtime; see its app.yaml#L3
  • x/talks – already using Go 1.11 runtime; see its app.yaml#L2

Doing so will make it possible¹ to start using net/http package for making HTTP requests (instead of google.golang.org/appengine/urlfetch package²) in the golang.org/x/tools/playground package, and unblock CL 160837 from being able to be merged.

Merging that CL will help with #29981 and golang/lint#436.

/cc @matloob @andybons @bradfitz

¹: https://cloud.google.com/appengine/docs/standard/go111/go-differences#migrating-appengine-sdk
²: https://cloud.google.com/appengine/docs/standard/go/issue-requests

@dmitshur dmitshur added the NeedsFix The path to resolution is known, but the work has not been done. label Feb 28, 2019
@dmitshur dmitshur self-assigned this Feb 28, 2019
@gopherbot gopherbot added this to the Unreleased milestone Feb 28, 2019
@bradfitz
Copy link
Contributor

Or Go 1.12.

@bradfitz
Copy link
Contributor

Oh, of App Engine. Maybe they'll have Go 1.12 soon, but yeah, no reason to wait for it.

@dmitshur
Copy link
Contributor Author

dmitshur commented Mar 4, 2019

I'm close to being able to send a CL to convert x/tour.

I've deployed a test instance to a staging environment at https://tour-dot-go-dashboard-dev.appspot.com, if anyone would like to help test it out and report any problems.

Edit: I've also deployed a test instance of blog to https://blog-dot-go-dashboard-dev.appspot.com/, if anyone would like to test that too.

@gopherbot
Copy link

Change https://golang.org/cl/160837 mentions this issue: playground: use stdlib instead of appengine packages

@gopherbot
Copy link

Change https://golang.org/cl/165459 mentions this issue: godoc/env: replace with golangorgenv

@gopherbot
Copy link

Change https://golang.org/cl/165460 mentions this issue: blog: deploy with App Engine Standard on Go 1.11

@gopherbot
Copy link

Change https://golang.org/cl/165537 mentions this issue: tour: deploy with App Engine Standard on Go 1.11

@gopherbot
Copy link

Change https://golang.org/cl/165742 mentions this issue: talks: set CHECK_COUNTRY env var in app.yaml

gopherbot pushed a commit to golang/tools that referenced this issue Mar 7, 2019
This change replaces the env package with a new golangorgenv package.

The previous env package existed primarily to configure the godoc
instance that was running golang.org. By now, the golang.org website
has been factored out to x/website, which has its own env package,
but ends up still using this env package indirectly via x/tools/godoc.

The goal of this change is to make env available for other services
that run on subdomains of golang.org, so they can continue to safely
rely on the x/tools/playground, which will be modified in the next
commit to also use the new golangorgenv.

The golangorgenv package replaces the IsProd function with a more
specific one. Start using it in packages x/tools/{,cmd}/godoc. Also,
re-arrange the order of checks to give the host suffix check higher
priority than the environment variable check. This way, if the
environment variable isn't set, the host suffix check gets a chance
to run.

When getting the value of "X-AppEngine-Country" header, use its
canonical format "X-Appengine-Country" to avoid an allocation.
This does not change behavior.

Updates golang/go#30486

Change-Id: I97b47211a45ca0351f31fcb4fa6d408a4b0c4c7c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/165459
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
gopherbot pushed a commit to golang/tools that referenced this issue Mar 7, 2019
With modern versions of App Engine, it's no longer needed to use the
google.golang.org/appengine/... packages.

Package log from standard library can be used instead of the
google.golang.org/appengine/log package. Packages net/http and
context from standard library can be used instead of
google.golang.org/appengine/urlfetch.

This simplifies the code and reduces the number of dependences.

Start using the golangorgenv package from previous commit to
make the decision of whether to enforce sharing restrictions,
rather than relying on the appengine build tag. The appengine
build tag is no longer set in App Engine Standard with Go 1.11+
runtime. An alternative solution would be detect App Engine by
doing something like:

	// GAE_ENV environment variable is set to "standard" in App Engine environment, Go 1.11 runtime.
	// See https://cloud.google.com/appengine/docs/standard/go111/runtime#environment_variables.
	var onAppengine = os.Getenv("GAE_ENV") == "standard"

But we choose to upgrade to explicit app-scoped environment variable
configuration as part of this change. It provides better security
properties, and the value of adding an intermediate transitional step
is not high enough to justify doing it.

When getting the value of "X-AppEngine-Country" header, use its
canonical format "X-Appengine-Country" to avoid an allocation.
This does not change behavior.

Run go mod tidy (using Go 1.12).

Updates golang/go#29981
Updates golang/go#30486

Change-Id: I82a59e0f28623e06762b7ebdf3930b5ee243acda
Reviewed-on: https://go-review.googlesource.com/c/tools/+/160837
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
gopherbot pushed a commit to golang/talks that referenced this issue Mar 7, 2019
This uses the new golangorgenv package from CL 165459 to enable
country check in the /share endpoint registered by x/tools/playground
package. This improves consistency with other similar services that
also use the x/tools/playground package.

Updates golang/go#30486

Change-Id: Idc6db4e384a7392aa39967f3320c2158a9c7e99a
Reviewed-on: https://go-review.googlesource.com/c/talks/+/165742
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
gopherbot pushed a commit to golang/tour that referenced this issue Mar 7, 2019
This change upgrades the deployment of tour to use the newer
Go 1.11 runtime. As part of that, the appengine build tag is
removed (it's no longer set by App Engine), and the GAE_ENV
environment variable is used to detect when tour is being run
in App Engine mode.

Set an environment variable in app.yaml to configure the
x/tools/godoc/golangorgenv package appropriately.

Factor out the static file handlers in local.go, but keep
static file handlers in app.yaml for improved latency across
global regions.

Updates golang/go#30486

Change-Id: Ia5bc88aab34fd07bf6ff0785da831180f509156f
Reviewed-on: https://go-review.googlesource.com/c/tour/+/165537
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
gopherbot pushed a commit to golang/blog that referenced this issue Mar 7, 2019
This change upgrades the deployment of blog to use the newer
Go 1.11 runtime. As part of that, the appengine build tag is
removed (it's no longer set by App Engine), and the GAE_ENV
environment variable is used to detect when blog is being run
in App Engine mode.

Set an environment variable in app.yaml to configure the
x/tools/godoc/golangorgenv package appropriately.

Modify the static file server to also serve /favicon.ico,
but keep static file handlers in app.yaml for improved latency
across global regions.

Updates golang/go#30486

Change-Id: I63ca78a075d94d43a40f0b963b5f6d0d8270c34e
Reviewed-on: https://go-review.googlesource.com/c/blog/+/165460
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
@dmitshur
Copy link
Contributor Author

dmitshur commented Mar 7, 2019

This is done now, all 3 are on Go 1.11 runtime and deployed.

If you spot any issues, please let me know.

@dmitshur dmitshur closed this as completed Mar 7, 2019
@gopherbot
Copy link

Change https://golang.org/cl/169997 mentions this issue: [release-branch.go1.12] godoc/env: replace with golangorgenv

@gopherbot
Copy link

Change https://golang.org/cl/169998 mentions this issue: [release-branch.go1.12] playground: use stdlib instead of appengine packages

@golang golang locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

3 participants