Implement Host header bypassing#155
Conversation
umputun
left a comment
There was a problem hiding this comment.
Sorry, completely missed the PR. Generally, it's looking good, I'll check it in detail on the weekend
| PingURL string | ||
| MatchType MatchType | ||
| RedirectType RedirectType | ||
| KeepHost *bool |
There was a problem hiding this comment.
is there a reason why this is a reference? I guess the goal is to have three-state bool here, but why? Default false for the normal bool should be fine i guess
There was a problem hiding this comment.
The primary reason we use a reference here, and in other discovery providers, is to enable the maintenance of a three-state (true/false/unset) value. If the value is not set, we'll default to the value specified in the global settings.
There was a problem hiding this comment.
got it, this is both on the global level as well on the provider level
One more thing - you don't have any tests for proxy.go changes, could you pls add it?
|
Is there any chance of being added? Still waiting for being able to use grafana :) |
# Conflicts: # README.md # app/discovery/discovery.go # app/discovery/provider/consulcatalog/consulcatalog.go # app/discovery/provider/consulcatalog/consulcatalog_test.go # app/discovery/provider/docker.go # app/discovery/provider/file.go # app/discovery/provider/file_test.go # app/discovery/provider/testdata/config.yml # app/main.go
|
@ffix - I have updated your branch with all the current changes from the master. It had a lot of conflicts, but hopefully, I have them adequately resolved. Pls, take a look and let me know if everything is fine |
# Conflicts: # README.md # app/main.go
This pull request adds the ability to bypass the Host header in Reproxy for compatibility with services like Grafana as mentioned in #133.
It includes the following changes:
It hasn't been tested with a consul catalog as me don't' have access to one.
I'm not familiar with the coding standards of this project, so I would appreciate any comments or fixes.