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

Make the proxy and registry return a VCS redirect on a cache miss #241

Closed
arschles opened this issue Jul 10, 2018 · 10 comments
Closed

Make the proxy and registry return a VCS redirect on a cache miss #241

arschles opened this issue Jul 10, 2018 · 10 comments
Labels
proxy Work to do on the module proxy

Comments

@arschles
Copy link
Member

arschles commented Jul 10, 2018 •
edited
Loading

A vgo API redirect looks like this (notice the mod in the <meta> tag):

arschles ~ $ curl -sSL 'https://swtch.com/testmod?go-get=1'
<!DOCTYPE html>
<meta name="go-import" content="swtch.com/testmod mod https://swtch.com/testmodproxy">
Nothing to see here.

And a vcs redirect looks like this (notice the "git" in the <meta> tag):

arschles ~ $ curl https://gopkg.in/yaml.v1?go-get=1

<html>
<head>
<meta name="go-import" content="gopkg.in/yaml.v1 git https://gopkg.in/yaml.v1">
<meta name="go-source" content="gopkg.in/yaml.v1 _ https://github.com/go-yaml/yaml/tree/v1{/dir} https://github.com/go-yaml/yaml/blob/v1{/dir}/{file}#L{line}">
</head>
<body>
go get gopkg.in/yaml.v1
</body>
</html>

When the proxy or the registry see a cache miss, they currently return a 404 Not Found HTTP response, but this breaks vgo and fails builds.

@marwan-at-work is investigating whether vgo will follow a vcs redirect from a proxy server. If that works, we should be returning vcs redirects from the registry and proxy when a cache miss happens.

Otherwise, we'll need to figure out another way!
@arschles arschles added registry proxy Work to do on the module proxy labels Jul 10, 2018
@marwan-at-work
Copy link
Contributor

Moving some of our slack conversation findings here:

If GOPROXY is set, then vgo does not handle redirects. It assumes that whatever the proxy gives it is the right module data.

Furthermore, it seems that it is not strict about incorrect /list data. If the lines are not semver, they are simply not added. So vgo ends up returning this error: https://github.com/golang/vgo/blob/master/vendor/cmd/go/internal/modfetch/proxy.go#L81

My ideal solution is to have vgo be able to handle 301s but happy to discuss further.

@arschles
Copy link
Member Author

arschles commented Jul 10, 2018 •
edited
Loading

More slack discussion moved here:

The situation:

  • vgo may not support exactly what we need, which is returning a "redirect" to a vcs
  • we needed that because it would give us the opportunity to fill caches in the background
  • it would be great to be able to do that in vgo b/c putting downloads in the critical path would add a lot of load to our webservers
  • we could get around doing it in the critical path, but we should cross that bridge if we have no other options

A few options:

  1. modify vgo to support 404s or redirects
  2. look into downloading code when it's requested (this is what I called "in the critical path")
  3. modify vgo to automatically prefix imports with a (overridable) stable hostname, so vgo get my/module actually becomes vgo get registry.golang.org/my/module
    • that command will respect the 'meta tag redirect', so we can redirect to a vcs

@marwan-at-work
Copy link
Contributor

From the Athens side of things, whether vgo handles redirects or not, we still need to get rid of the worker logic because things need to be synchronous. Particularly the cacheMiss/cacheFetch jobs. Because otherwise vgo will fail and would have to say "try again in 5 minutes" 😃

@arschles
Copy link
Member Author

arschles commented Jul 10, 2018 via email

@marwan-at-work
Copy link
Contributor

ah, yes that makes sense. So the handler will return a redirect, and queue a cache fill.

But in case vgo does not end up handling redirects and we're left with the "critical path" solution, then we can just copy the data from the pipe into our own cache. But maybe im missing other ways too :D

@arschles
Copy link
Member Author

arschles commented Jul 10, 2018 via email

@michalpristas
Copy link
Member

@arschles isnt somebody from CDA working on vgo directly as well? maybe they have more insight about how it should be handled or any future plans

@marwan-at-work
Copy link
Contributor

After doing some digging into vgo itself, it turns out the net/http Client handles redirects automatically (up to 10 redirects https://golang.org/pkg/net/http/#Get)

Therefore, vgo can handle Proxy Redirects but not in the same HTML way described above.

From athens side of things, we still need to make sure we propagate redirects appropriately so we can track that in this issue or create a new one.

Thanks!

@arschles
Copy link
Member Author

Ref golang/go#26334

@arschles
Copy link
Member Author

We decided in #290 that we're going to fill caches synchronously, so we don't need to do redirects for that purpose. We still may need to have the proxy redirect the go CLI to the registry, but I am going to open a separate issue for that.

Also, the go CLI will not respect the HTML I referenced in the OP, when GOPROXY is set. I'll open a new issue for that too

This was referenced Jul 28, 2018
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
proxy Work to do on the module proxy
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@arschles @michalpristas @marwan-at-work