pieterlouw / caddy-grpc

grpc plugin for Caddy Server
Apache License 2.0
49 stars 15 forks source link

Caddy v0.10.12 cannot build with this plugin #3

Closed met-pub closed 6 years ago

met-pub commented 6 years ago

$ wget "https://caddyserver.com/download/darwin/amd64?license=personal" --2018-03-30 08:38:42-- https://caddyserver.com/download/darwin/amd64?license=personal Resolving caddyserver.com (caddyserver.com)... 138.68.240.78 Connecting to caddyserver.com (caddyserver.com)|138.68.240.78|:443... connected. HTTP request sent, awaiting response... 200 OK Length: unspecified [application/zip] ...

$ wget "https://caddyserver.com/download/darwin/amd64?plugins=http.grpc&license=personal" --2018-03-30 08:39:14-- https://caddyserver.com/download/darwin/amd64?plugins=http.grpc&license=personal Resolving caddyserver.com (caddyserver.com)... 138.68.240.78 Connecting to caddyserver.com (caddyserver.com)|138.68.240.78|:443... connected. HTTP request sent, awaiting response... 500 Internal Server Error 2018-03-30 08:39:42 ERROR 500: Internal Server Error.

shaxbee commented 6 years ago

This was fixed in #2, maybe caddy is hardcoded to partical version of caddy-grcp plugin?

met-pub commented 6 years ago

thanks @shaxbee, I'm newbie in Go, is it possible to download caddy and the fixed plugin separately?

webwurst commented 6 years ago

@bianbian-org I have a Dockerfile that shows how to build Caddy and plugins on your own: giantswarm/docker-caddy/Dockerfile

@shaxbee Thanks for the fix! \o/ I also hit that before. But now there is another one:

../../../mwitkow/grpc-proxy/proxy/handler.go:63:30: undefined: transport.StreamFromContext

webwurst commented 6 years ago

Ok, it's probably because of this change, in a dependency introduced in November:

// We require that the director's returned context inherits from the serverStream.Context().

mholt commented 6 years ago

(And now Cloudflare's new DNS service broke one of our tests, go figure -- so deploying an update to the plugin will fail, but as soon as I get the next Caddy release out, later this week or early next, it should work.)

pieterlouw commented 6 years ago

@mholt

How will it affect the Caddy build server if I use a vendor directory for the plugin. I'm thinking of telling dep to point to an older version of the upstream grpc-go to avoid surprises like these: https://github.com/mwitkow/grpc-proxy/issues/24

mholt commented 6 years ago

@pieterlouw As long as you don't vendor a lib used by Caddy, it should be OK. However, I haven't thoroughly tested it, but I think it works. (Or at least, it used to!)

pieterlouw commented 6 years ago

Thanks @mholt. I just ran dep init on the plugin and it added caddy as one of the constraints.

[[constraint]]
  name = "github.com/mholt/caddy"
  version = "0.10.12"

Also, it added quite alot of packages to the vendor directory which might be included in caddy as well. I'll see if there's a way around this.

mholt commented 6 years ago

Hmm, I'm not sure I would vendor Caddy itself either... 🤔 💣

pieterlouw commented 6 years ago

I made some changes (added vendor then removed again :) don't ask), but the main change I made was to hard fork the proxy package from https://github.com/mwitkow/grpc-proxy into the plugin. There is a PR hanging there that will fix the mwitkow/grpc-proxy/proxy/handler.go:63:30: undefined: transport.StreamFromContext error mentioned above but I made that changes inside the plugin until the PR has been merged.

However now it seems the build is broken because of the new Cloudflare DNS as @mholt mentioned above.

mholt commented 6 years ago

Sorry, I've been a little delayed on that release, (end of semester stuff), but I will try to get it out soon.

pieterlouw commented 6 years ago

Hi @bianbian-org. I've manage to deploy the latest code to Caddy (Thanks @mholt!), please check to see if this issue is resolved.

met-pub commented 6 years ago

Caddy v0.10.13 is ok! thanks!