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

Add --base-path CLI option to override the URL path in the tilejson #1185

Closed
5 tasks
nyurik opened this issue Feb 8, 2024 · 6 comments · Fixed by #1205
Closed
5 tasks

Add --base-path CLI option to override the URL path in the tilejson #1185

nyurik opened this issue Feb 8, 2024 · 6 comments · Fixed by #1205
Assignees
Labels
cli Related to Command Line Interface good first issue help wanted

Comments

@nyurik
Copy link
Member

nyurik commented Feb 8, 2024

As discussed in #1184, Martin should have an option to override the base path when its being hosted behind a proxy that cannot easily set x-rewrite-url header.

CLI parameter

# Force Martin to ignore the x-rewrite-url header and set tilejson's tiles URL
# as   <protocol>://<host>/<source>/{z}/{x}/{y}
martin --base-path /

# Force it to be  <protocol>://<host>/foo/bar/<source>/{z}/{x}/{y}
martin --base-path /foo/bar

# This should work the same as above
martin --base-path /foo/bar/

# This should produce an error
# (must start with a slash, but could be relaxed in the future, e.g. can combine path with x-rewrite-uri?)
martin --base-path foo/bar

Configuration Files

Add base_path to the root (in the struct SrvConfig)

---
base_path: /foo/bar

Code changes

  • Add base_path: Option<String> to SrvArgs - similar to how listen_addresses is implemented
  • Add base_path to SrvConfig
  • Similar to catalog, pass Data::new(config) to the app_data in new_server
  • Modify get_source_info to additionally take config: Data<SrvConfig> and use it to override the x-rewrite-url handling.
  • Make sure finalize does all the needed validations of this value or it should return an error. You will need to add a new error to MartinError for this.
@sharkAndshark sharkAndshark added help wanted config Relates to Martin configuration good first issue cli Related to Command Line Interface and removed config Relates to Martin configuration labels Feb 8, 2024
@sharkAndshark
Copy link
Collaborator

Do we need to add it to config either?

@nyurik
Copy link
Member Author

nyurik commented Feb 8, 2024

@sharkAndshark I just updated this ticket with all the details needed

@sutanAE
Copy link

sutanAE commented Feb 15, 2024

Do we need to add it to config either?

It would be nice to add this on config.

In my experience, I put Martin behind NGINX and use the proxy_pass directive, and rewrite the basepath. the SSL can be handled on NGINX as well. This works with /tablename/{z}/{x}/{y}, however, this won't work with the tile.json because there is no base path for martin. If we could add this, then it would be so nice!

@nyurik
Copy link
Member Author

nyurik commented Feb 15, 2024

Yep, I documented it above for both CLI and config file case. @sutanAE, if you use NGINX, why not use rewriting urls config?

@nyurik
Copy link
Member Author

nyurik commented Feb 20, 2024

Should this feature ignore X-REWRITE-URL header if it is set? I.e. override it? Or should it only work if there is no x-rewrite-url header? What usecases would there be for its usage?

@sutanAE
Copy link

sutanAE commented Feb 20, 2024

Yep, I documented it above for both the CLI and config file cases. @sutanAE, if you use NGINX, why not use rewriting urls config?

Hi @nyurik thank you. That is because I did not read the documentation correctly; my apologies.

It seems like the problem is now solved for me. Either way, I think it would still be nice to have something that acts as a base path for Martin. The only thing I can think of is why the developer experience and local development imitates NGINX configuration.

I personally develop my app using Node.js with SvelteKit and have Martin running on another port. Martin's URL in localhost and production is different, so the SvelteKit's code is quite messy; there are lots of if statements to see if it's in dev or in prod. Of course, this could be avoided using environment variables, but this limits Martin to certain styles.

I hope I am contributing to the discussion—many thanks for developing such amazing software.

nyurik pushed a commit that referenced this issue Feb 27, 2024
…1205)

Override URL path in the tilejson's `tiles` field when used behind a proxy that is not setting `X-Rewrite-URL` header

Fixes #1185
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli Related to Command Line Interface good first issue help wanted
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants