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

Serve from the path of the public hostname #61

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jleedev
Copy link

@jleedev jleedev commented Aug 26, 2023

Passing public-hostname with a non-empty path will use that path as the prefix for the HTTP handler.

This is technically a breaking change, in that anybody who was using a non-empty path behind a reverse proxy must have had it configured to remove the path prefix on that end. Now they need to remove that. However, that's generally a brittle thing to do; it's simpler all around for the backend server to know what its root path should be.

Moreover, Firebase Hosting doesn't have such a capability, so it would require running an additional backend server.

Passing public-hostname with a non-empty path will use that path as the
prefix for the HTTP handler.

This is technically a breaking change, in that anybody who was using a
non-empty path behind a reverse proxy must have had it configured to
remove the path prefix on that end. Now they need to remove that.
However, that's generally a brittle thing to do; it's simpler all around
for the backend server to know what its root path should be.

Moreover, Firebase Hosting doesn't have such a capability, so it would
require running an additional backend server.
@bdon
Copy link
Member

bdon commented Aug 28, 2023

The intention before was for publicHostname to affect the string embedded in TileJSON and nothing else.

If you have pmtiles serve running behind, for example, a NGINX reverse proxy on a server, the reverse proxy should handle the removal of the prefix, along with other things (headers, SSL termination). Does this patch only make sense where pmtiles serve is used without any reverse proxy?

Can you link to how this is deployed to Firebase Hosting so I can reproduce it?

@jleedev
Copy link
Author

jleedev commented Aug 28, 2023

Really, I was surprised when I was trying to get this to work. I supplied the public URL on the command line, and the server did not respond to that URL. Having to actually remove the prefix at the proxy layer is weird.

For instance, there could be no proxy, just running on an external port, but we still need to tell the server what its hostname is, so accepting a path as part of that option, but ignoring it, is weird.

I think it is pretty standard for the external URL flag to alter the URL paths that the backend accepts, and not to require the user to remove them at the proxy. I'm not aware of a useful guide "how to write a well-behaved web backend", but it's my conviction that this is the correct way.

https://github.com/prometheus/prometheus/blob/1b9a53b5eea62e82a65bb65a8139c38dbd8f259e/cmd/prometheus/main.go#L266-L272
https://tegola.io/documentation/configuration/#webserver has a dedicated uri_prefix flag, etc.

Anyway, the Firebase setup isn't checked in anywhere, but it looks like this:

 ¶ cat firebase.json
{
  "hosting": {
    "rewrites": [
      {
        "source": "/tiles/**",
        "run": {
          "serviceId": "tiles",
          "region": "us-central1"
        }
      }
    ]
  }
}
 ¶ firebase deploy --only hosting
 ¶ gcloud run deploy tiles \
    --image=us-docker.pkg.dev/${PROJECT_ID}/my-docker-repo/go-pmtiles \
    --region us-central1 \
    --allow-unauthenticated \
    --args 'serve,.,--bucket=gs://my-pmtiles-bucket,--public-hostname,https://asdf-f6040.web.app/tiles,--cors,*'

Running a version of the server which contains this patch. Firebase can match a glob or regex and dispatch it to a Cloud Run, and that's it.

My file -- https://asdf-f6040.web.app/tiles/NationalFile.json
404 from Firebase -- https://asdf-f6040.web.app/tiles (no trailing /, didn't bother to match it)
404 from go-pmtiles -- https://asdf-f6040.web.app/tiles/

(Perhaps related, the positional path argument and the bucket flag seem mutually exclusive? Hard to say)

There's also no index page to say yes, you correctly reached the server's homepage; that would require a bucket scan operation of course, but an empty filename could perhaps instead return "Hello, go-pmtiles!" since that can't possible match anything real.

There are other routing approaches, e.g. https://github.com/consbio/mbtileserver/blob/c4c18f5b60c22559df36c1c57db1d22d17415f5b/main.go#L98 allows configuring the path prefix only, and determines its hostname on the fly via X-Forwarded-Host || Host.

https://github.com/maplibre/martin/blob/550a46bd0f58c6a7054f121bf103cf9e886fcd47/demo/frontend/nginx.conf#L69 seems to encourage removing the url prefix at the proxy and adding the original in an X-Rewrite-URL header, I'm not sure how common that is.

@bdon
Copy link
Member

bdon commented Aug 28, 2023

OK, thanks for the detail. This dovetails with protomaps/PMTiles#174 since it looks like Firebase Deploy is just Google Cloud Run? I'll give it a spin myself soon

  1. if you are deploying this using gcloud run why not just just deploy at the root of the domain instead of a /path? The ideal convention is that the GCP deployment is as close as possible to the Cloudflare and AWS deployments.
  2. Can you accomplish this by prefixing your objects with tiles/ in your GCP bucket?
  3. should we rename the publicHostname to option to publicPath, or have separate publicHostname and path? publicHostname implies the non-path parts of the URL, so the patch as is can be confusing.

If we automatically derive the hostname via Host or X-Forwarded-Host that influences the cached contents of TileJSON in any CDN in front of the server - this is true of publicHostname as well but at least that setting is explicit to the developer.

@jleedev
Copy link
Author

jleedev commented Aug 28, 2023

it looks like Firebase Deploy is just Google Cloud Run?

Firebase Hosting can be paired with either Cloud Functions or Cloud Run, and can also serve static content.

  1. if you are deploying this using gcloud run why not just just deploy at the root of the domain instead of a /path? The ideal convention is that the GCP deployment is as close as possible to the Cloudflare and AWS deployments.

Mainly because the configuration doesn't quite line up. Google Cloud has a few options but in my opinion, Cloud Run + Firebase Hosting is cheap and simple.

(For instance, AWS Lambda supports the JavaScript version as native, but Cloud Functions sort of has a long build step that turns it into a container image. Cloud Run can deploy any HTTP service while AWS Lambda requires an accompanying host that connects to a special endpoint to shuttle requests.)

And I don't actually want to map my Cloud Run Service to a domain, I want a CDN, in order to minimize the container startup latency, bucket latency, etc. The Google options are Cloud CDN and Firebase Hosting. The latter has a generous free tier; the former has none, so that determines my choice.

However, the way Firebase Hosting works is "one per project", as opposed to a resource that you can create n of. So for "all my ad-hoc hobby cloud stuff", I would configure it in this way.

  1. Can you accomplish this by prefixing your objects with tiles/ in your GCP bucket?

Yes, that's a great workaround, if a bit unintuitive without a nice tutorial to follow.

  1. should we rename the publicHostname to option to publicPath, or have separate publicHostname and path? publicHostname implies the non-path parts of the URL, so the patch as is can be confusing.

For sure. I'd probably go with externalUrl as a single flag.

If we automatically derive the hostname via Host or X-Forwarded-Host that influences the cached contents of TileJSON in any CDN in front of the server - this is true of publicHostname as well but at least that setting is explicit to the developer.

Right, it would require adding a Vary header. But it is an interesting way that this can be done automatically.

@bdon
Copy link
Member

bdon commented Sep 1, 2023

@jleedev has this been running pretty well for you? My plan is to knock out some docs related to both GCP and running pmtiles serve simultaneously

@jleedev
Copy link
Author

jleedev commented Sep 1, 2023

Yes, it seems to work fine.

  • Correct the flag name and/or split into two
  • Verify that a trailing slash is not allowed (I think?), either refuse to start up or remove it automatically
  • Verify that this does in fact work when no value is passed (since we're calling url.Parse and erroring)
  • Ideally add a unit test, there's a complicated relationship between the global bucket prefix, the incoming url, the prefix to remove, and so on.
  • Write a guide for setting things up and how these parameters (prefix and bucket path) relate to the end user URLs.

@bdon
Copy link
Member

bdon commented Sep 5, 2023

Related https://github.com/orgs/protomaps/discussions/1 for potentially loading go-pmtiles as a caddy module

I need to investigate if this lets Caddy handle URL prefixes while the pmtiles HTTP handler is prefix-unaware. Caddy may also be able to pass the correct path and hostname (it has it for Let's Encrypt)

@bdon bdon mentioned this pull request Sep 24, 2023
@bdon
Copy link
Member

bdon commented Sep 26, 2023

see #85 for directly related discussion of prefixes and tilejson hostnames

@bdon
Copy link
Member

bdon commented Mar 11, 2024

basic Google Cloud Run instructions up: https://docs.protomaps.com/deploy/google-cloud

This deployment pattern is straightforward compared to Firebase but doesn't yet include the Cloud CDN component

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants