Skip to content

Conversation

ARolek
Copy link
Member

@ARolek ARolek commented Aug 28, 2019

Added the webserver config property uri_prefix which allows for
setting an API route prefix. This is commonly used when tegola
is run behind a reverse proxy (i.e. example.com/tegola). Certain
routes (i.e. capabilities) build URLs which are included in the
response and then passed back to the proxy. Without the ability
to set a URI prefix, the URLs in the capabilities responses
will not be useable by the consuming client.

Setting the webserver hostname now ignores the server port. It's
assumed that if the user is setting the hostname they want full
control of it.

closes #136

Added the webserver config property uri_prefix which allows for
setting an API route prefix. This is commonly used when tegola
is run behind a reverse proxy (i.e. example.com/tegola). Certain
routes (i.e. capabilities) build URLs which are included in the
response and then passed back to the proxy. Without the ability
to set a URI prefix, the URLs in the capabilities responses
will not be useable by the consuming client.

Setting the webserver hostname now ignores the server port. It's
assumed that if the user is setting the hostname they want full
control of it.

closes #136
@ARolek ARolek requested review from gdey and ear7h August 28, 2019 22:57
@coveralls
Copy link

coveralls commented Aug 28, 2019

Pull Request Test Coverage Report for Build 1752

  • 31 of 41 (75.61%) changed or added relevant lines in 6 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.01%) to 45.258%

Changes Missing Coverage Covered Lines Changed/Added Lines %
cmd/tegola/cmd/server.go 0 3 0.0%
config/errors.go 0 3 0.0%
config/config.go 1 5 20.0%
Totals Coverage Status
Change from base Build 1701: -0.01%
Covered Lines: 5545
Relevant Lines: 12252

💛 - Coveralls

The buildout of the capabilities URLs had too many moving
parts and was being repeated in too many places. The
logic has been centralized into a single function.
@ear7h
Copy link
Contributor

ear7h commented Aug 29, 2019

Could you elaborate on the following?

Setting the webserver hostname now ignores the server port. It's
assumed that if the user is setting the hostname they want full
control of it.

@ARolek
Copy link
Member Author

ARolek commented Aug 29, 2019

@ear7h previously if the hostname was set and a port was included in the request, the the port was appended to the hostname. For example, if I set the hostname to tegola.io and a request came in on port :8080 then the host would be modified to tegola.io:8080. This is problematic when tegola sits behind a reverse proxy as tegola will likely be listening on an internal port and have no idea what port the request originated on. The user configuring the service would likely know.

Copy link
Contributor

@ear7h ear7h left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, I have a few nit picky change requests and some clarifications/improvements to test cases. Thank you!

@ARolek
Copy link
Member Author

ARolek commented Aug 30, 2019

@ear7h great code review. I have incorporated all your suggestions except for the last one.

@ear7h
Copy link
Contributor

ear7h commented Aug 30, 2019

LGTM

Copy link
Member

@gdey gdey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@gdey gdey merged commit e45ccbd into v0.10.x Aug 30, 2019
@gdey gdey deleted the issue-136 branch August 30, 2019 11:54
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.

4 participants