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/website/cmd/golangorg: loading and sharing Go Playground snippets on go.dev/play isn't working #65081

Closed
dmitshur opened this issue Jan 12, 2024 · 15 comments
Assignees
Labels
NeedsFix The path to resolution is known, but the work has not been done. website
Milestone

Comments

@dmitshur
Copy link
Contributor

dmitshur commented Jan 12, 2024

Update: See #65081 (comment) and later comments.


testdata/live.txt has these test cases to check that POSTing to the playground share endpoint works:

POST https://golang.org/share
postbody
	package main
body !contains UA-

POST https://go.dev/_/share
postbody
	package main
body !contains UA-

(source)

They sometimes (but seemingly not always) fail with with an unexpected status code 451:

webtest.CheckHandler failed:
# testdata/live.txt
## POST https://go.dev/_/share
testdata/live.txt:55: POST https://go.dev/_/share
testdata/live.txt:58: code = "451", want "200"

When it happens during a deployment, it prevents the new version from being promoted to serve traffic.

CC @golang/tools-team.

@dmitshur dmitshur added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. website labels Jan 12, 2024
@dmitshur dmitshur added this to the Unreleased milestone Jan 12, 2024
@findleyr
Copy link
Contributor

Indeed, playground share seems flaky today.

Per https://cs.opensource.google/go/x/playground/+/master:share.go;l=55;drc=bb2a4ed2d77c5ebb58ace5e1466acec69a729a68, I believe this should only happen if r.Header.Get("X-AppEngine-Country") == "CN". Hmm.

@fis
Copy link

fis commented Jan 13, 2024

Just as a data point, this seems to be pretty consistently happening to me today on the current live Playground version, from the UK. (And other IP-based geolocation services don't seem to think I'm in China.)

@jub0bs
Copy link

jub0bs commented Jan 13, 2024

I'm observing the same issue from a French IP address.

@nalgeon
Copy link

nalgeon commented Jan 13, 2024

Same problem with German IP (207.154.221.x). Both sharing and loading shared snippets does not work:

POST https://go.dev/_/share
451 (Unavailable For Legal Reasons)

GET https://go.dev/_/share?id=YJeYpEjdBF
451 (Unavailable For Legal Reasons)

The problem is not intermittent for me. Sharing stopped working completely yesterday.

@dmitshur dmitshur changed the title x/website/cmd/golangorg: failures in playground share POST live.txt test case, possibly intermittent x/website/cmd/golangorg: loading and sharing Go Playground snippets on go.dev/play isn't working Jan 13, 2024
@dmitshur
Copy link
Contributor Author

dmitshur commented Jan 13, 2024

Thanks for the reports. This is indeed now happening on each request, and affects website visitors in the US and many other locations. Only snippet loading and sharing is affected, other parts of the Go Playground work okay.

I've updated the issue and pinned it for visibility, but it's unlikely someone will have a chance to look into it until after the weekend.

@dmitshur dmitshur pinned this issue Jan 13, 2024
@x1unix
Copy link

x1unix commented Jan 14, 2024

I got the same issue. I host a frontend for Go Playground - https://goplay.tools which is hosted in the US and get 451 Unavailable For Legal Reasons on each share call.

The same occurs from Polish, Ukrainian and French IPs.

@findleyr
Copy link
Contributor

findleyr commented Jan 14, 2024

I looked into this briefly today. It appears that one of the internal IPs used for proxied traffic from the go.dev frontend to the playground backend was briefly categorized as CN, causing these spurious failures.

I think the IP in question has since been recategorized as US, but it will probably take several more days for that to propagate. Additionally, go.dev has its own mechanism for handling CN-based traffic, so this check on internal IPs is both pointless and unnecessary. We should remove it for proxied traffic, though we still need it for direct traffic to the playground backend.

Unfortunately, due to the timing around the holiday weekend in the US, we're unlikely to be able to mitigate this until Tuesday at the earliest. It may resolve naturally before then. In the meantime, if you're up for a blast from the past, the old frontend is still available at https://play-dot-golang-org.appspot.com/.

@nalgeon
Copy link

nalgeon commented Jan 16, 2024

So the sharing feature has been down since Friday, and as of Tuesday, it still appears to be unresolved.

Understanding that the team doesn't work on weekends, I'm concerned about the length of downtime for such a critical component.

Could we get an update on the expected timeline for a fix?

@mateusz834 mateusz834 unpinned this issue Jan 16, 2024
@mateusz834 mateusz834 pinned this issue Jan 16, 2024
@gopherbot
Copy link

Change https://go.dev/cl/556096 mentions this issue: cmd/golangorg/testdata: comment out playground snippet tests

gopherbot pushed a commit to golang/website that referenced this issue Jan 16, 2024
These tests are causing the go.dev deployment to fail due to
golang/go#65081.

Change-Id: I3585079098064680ef1a65494e7bc7c272956fa8
Reviewed-on: https://go-review.googlesource.com/c/website/+/556096
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
Auto-Submit: Robert Findley <rfindley@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
@findleyr
Copy link
Contributor

@nalgeon the Go team is primarily based in the US, where Monday was a holiday. We're working on this now.

@gopherbot
Copy link

Change https://go.dev/cl/556157 mentions this issue: share: hard-code an allow list of IP prefixes for proxied traffic

gopherbot pushed a commit to golang/playground that referenced this issue Jan 16, 2024
While we continue to investigate the miscategorization of internal IPs
in playground traffic, add an allow list of a few known problematic IP
prefixes that have been manually verified to be US Google IPs.

Based on history over the past week, this should get playground snippets
working again, at least temporarily.

For golang/go#65081

Change-Id: Iccb16e9f6afbdad271198a4e3f23c8adf8b0fe8f
Reviewed-on: https://go-review.googlesource.com/c/playground/+/556157
Auto-Submit: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Commit-Queue: Robert Findley <rfindley@google.com>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
Run-TryBot: Robert Findley <rfindley@google.com>
@gopherbot
Copy link

Change https://go.dev/cl/556195 mentions this issue: share: use forwarded IP for geoIP prefix allow list comparison

gopherbot pushed a commit to golang/playground that referenced this issue Jan 16, 2024
CL 556157 used the http.Request.RemoteAddr for comparing with the allow
list of miscategorized IP prefixes. This is incorrect as it is generally
127.0.0.1 for App Engine traffic.

Use X-Forwarded-For instead:
https://cloud.google.com/appengine/docs/standard/reference/request-headers

For golang/go#65081

Change-Id: Ia0861bdf76dd401c8fa1cd0871c09ae901f5a089
Reviewed-on: https://go-review.googlesource.com/c/playground/+/556195
TryBot-Result: Gopher Robot <gobot@golang.org>
Commit-Queue: Robert Findley <rfindley@google.com>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
Run-TryBot: Robert Findley <rfindley@google.com>
Auto-Submit: Robert Findley <rfindley@google.com>
@findleyr
Copy link
Contributor

Update for today: the above CLs temporarily mitigate the problem by allow listing several Google IP prefixes that have been observed to follow the problematic pattern: a transient recategorization as CN in early January, since reverted back to US. This short list covers all the traffic we've seen over the past week, and playground snippet sharing seems to be working again. However, there's no guarantee that this list will remain valid.

We're still pursuing several more permanent fixes.

@gopherbot
Copy link

Change https://go.dev/cl/556315 mentions this issue: share: remove the allowShare guard

@hyangah
Copy link
Contributor

hyangah commented Jan 17, 2024

This is mitigated and now @findleyr is actively working on a longer term solution. Unpinning.

@hyangah hyangah unpinned this issue Jan 17, 2024
@hyangah hyangah added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Jan 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsFix The path to resolution is known, but the work has not been done. website
Projects
None yet
Development

No branches or pull requests

8 participants