Issue 994 malformed hostnames

[ARolek]

Though this PR has a lot of code changes, a lot of them are clean up, adding consts, and formatting. The material changes are as follows:

• Removed internal function buildCapabilitiesURL from the server package. This function was handling too many cases and brittle.
• Added a new type to the server package: TileURLTemplate. This type is responsible for building up all the Tile template URLs. So any URL with the /{z}/{x}/{y}.pbf uri template suffix will now use this type.
• Added hostname parsing & validation to the config package. The hostname attribute does not support a protocol, but sometimes the protocol was configured and it would result in malformed URLs (putting "https://....." in hostname config directive results in a malformed URL #994). The validation and use of the url.URL package helps work with user supplied URLs.

Overall, it's a lot of code, but I got frustrated with the state of things and deemed a refactor necessary.

[coveralls]

Pull Request Test Coverage Report for Build 1919ce1db

Details

• 145 of 189 (76.72%) changed or added relevant lines in 12 files are covered.
• 1 unchanged line in 1 file lost coverage.
• Overall coverage increased (+0.3%) to 42.816%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
server/handle_map_style.go	7	8	87.5%
server/handle_capabilities.go	26	28	92.86%
server/server.go	5	7	71.43%
cmd/tegola_lambda/main.go	0	3	0.0%
server/errors.go	0	3	0.0%
cmd/tegola/cmd/server.go	0	4	0.0%
internal/env/parse.go	9	13	69.23%
internal/env/types.go	10	16	62.5%
server/tile_url_template.go	69	77	89.61%
config/errors.go	0	11	0.0%

Files with Coverage Reduction	New Missed Lines	%
cmd/tegola_lambda/main.go	1	0.0%

Totals
Change from base Build a58cd8965:	0.3%
Covered Lines:	7030
Relevant Lines:	16419

---

💛 - Coveralls

[gdey]

LGTM

Thank you for all the cleanup. It is good to get some of the magic values into constants.

[dBitech]

I took some time to test this out and seems to function as advertised. Look forward to it's commit bit

[iwpnd]

looks good. some nice cleanup. 👍

[iwpnd]

this is redundant. If you validate the hostname in config.validate() already it will throw early.

[ARolek]

@iwpnd you're right it is redundant. I just went back and looked at the code to determine why I did it this way and I think I'm hitting a weird issue that could probably be solved more elegantly:

• The config package has a ValidateAndRegisterParams routine which calls the url.Parse() method on the Hostname and throws an error if the Parse fails. It does not set the value of Webserver.Hostname, which is of type env.String.

tegola/config/config.go

Lines 337 to 345 in e38590a

	if string(c.Webserver.HostName) != "" {
	_, err := url.Parse(string(c.Webserver.HostName))
	if err != nil {
	return ErrInvalidHostName{
	HostName: string(c.Webserver.HostName),
	Err: err,
	}
	}
	}

• The server.Hostname value is now of type url.URL and since config.Webserver.Hostname is of type env.String we need to parse again.

I think I opted for the double Parse approach as this code path only gets hit during start up. A more elegant way to solve this would be to add a new type to the internal/env package called URL. This should be pretty straight forward to do so I will give it a shot.

Good call out.

[ARolek]

@iwpnd I added a new commit addressing this comment. LMK what you think.

[iwpnd]

this would have been caught in config.validate() already, no?

[ARolek]

@iwpnd I added a new commit addressing this comment. LMK what you think.

[ARolek]

@gdey / @iwpnd I have added another commit which implemented a new config type: env.URL. This removes the need for the URL parsing to happen at latter stages. I have tagged you two for re-review when you have a min.

[gdey]

LGTM