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

Auto host rewrite option for strict_dns and logical_dns clusters #504

Merged
merged 19 commits into from
Feb 27, 2017
10 changes: 10 additions & 0 deletions docs/configuration/http_conn_man/route_config/route.rst
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ next (e.g., redirect, forward, rewrite, etc.).
"path_redirect": "...",
"prefix_rewrite": "...",
"host_rewrite": "...",
"auto_host_rewrite": "...",
"case_sensitive": "...",
"timeout_ms": "...",
"runtime": "{...}",
Expand Down Expand Up @@ -98,6 +99,15 @@ host_rewrite
*(optional, string)* Indicates that during forwarding, the host header will be swapped with this
value.

.. _config_http_conn_man_route_table_route_auto_host_rewrite:
Copy link
Member

Choose a reason for hiding this comment

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

nit: don't need the anchor if its not referenced anywhere (though you might want to reference from the router arch overview page).

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, that is a todo on my end. Need to provide an example as well. Will add it to router arch


auto_host_rewrite
*(optional, boolean)* Indicates that during forwarding, the host header will be swapped with the
hostname of the upstream host chosen by the cluster manager. This option is applicable only when
the destination cluster for a route is of type *strict_dns* or *logical_dns*. Setting this to true
with other cluster types has no effect. *auto_host_rewrite* and *host_rewrite* are mutually exclusive
options. Only one can be specified.

.. _config_http_conn_man_route_table_route_case_sensitive:

case_sensitive
Expand Down
4 changes: 3 additions & 1 deletion docs/intro/arch_overview/http_routing.rst
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@ request. The router filter supports the following features:
level.
* :ref:`Path <config_http_conn_man_route_table_route_path_redirect>`/:ref:`host
<config_http_conn_man_route_table_route_host_redirect>` redirection at the route level.
* :ref:`Host rewriting <config_http_conn_man_route_table_route_host_rewrite>`.
* :ref:`Explicit host rewriting <config_http_conn_man_route_table_route_host_rewrite>`.
* :ref:`Automatic host rewriting <config_http_conn_man_route_table_route_auto_host_rewrite>` based on
the DNS name of the selected upstream host.
* :ref:`Prefix rewriting <config_http_conn_man_route_table_route_prefix_rewrite>`.
* :ref:`Request retries <arch_overview_http_routing_retry>` specified either via HTTP header or via
route configuration.
Expand Down
5 changes: 5 additions & 0 deletions include/envoy/router/router.h
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,11 @@ class RouteEntry {
* @return const VirtualHost& the virtual host that owns the route.
*/
virtual const VirtualHost& virtualHost() const PURE;

/**
* @return bool true if the :authority header should be overwritten with the upstream hostname.
*/
virtual bool autoHostRewrite() const PURE;
};

/**
Expand Down
6 changes: 6 additions & 0 deletions include/envoy/upstream/host_description.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,12 @@ class HostDescription {
*/
virtual Outlier::DetectorHostSink& outlierDetector() const PURE;

/**
* @return the hostname associated with the host if any.
* Empty string "" indicates that hostname is not a DNS name.
*/
virtual const std::string& hostname() const PURE;

/**
* @return the address used to connect to the host.
*/
Expand Down
1 change: 1 addition & 0 deletions source/common/http/async_client_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ class AsyncStreamImpl : public AsyncClient::Stream,
return nullptr;
}
const Router::VirtualHost& virtualHost() const override { return virtual_host_; }
bool autoHostRewrite() const override { return false; }

static const NullRateLimitPolicy rate_limit_policy_;
static const NullRetryPolicy retry_policy_;
Expand Down
1 change: 1 addition & 0 deletions source/common/json/config_schemas.cc
Original file line number Diff line number Diff line change
Expand Up @@ -488,6 +488,7 @@ const std::string Json::Schema::ROUTE_ENTRY_CONFIGURATION_SCHEMA(R"EOF(
"path_redirect" : {"type" : "string"},
"prefix_rewrite" : {"type" : "string"},
"host_rewrite" : {"type" : "string"},
"auto_host_rewrite" : {"type" : "boolean"},
"case_sensitive" : {"type" : "boolean"},
"timeout_ms" : {"type" : "integer"},
"runtime" : {
Expand Down
7 changes: 7 additions & 0 deletions source/common/router/config_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ RouteEntryImplBase::RouteEntryImplBase(const VirtualHostImpl& vhost, const Json:
: case_sensitive_(route.getBoolean("case_sensitive", true)),
prefix_rewrite_(route.getString("prefix_rewrite", "")),
host_rewrite_(route.getString("host_rewrite", "")), vhost_(vhost),
auto_host_rewrite_(route.getBoolean("auto_host_rewrite", false)),
cluster_name_(route.getString("cluster", "")),
cluster_header_name_(route.getString("cluster_header", "")),
timeout_(route.getInteger("timeout_ms", DEFAULT_ROUTE_TIMEOUT_MS)),
Expand All @@ -105,6 +106,12 @@ RouteEntryImplBase::RouteEntryImplBase(const VirtualHostImpl& vhost, const Json:

route.validateSchema(Json::Schema::ROUTE_ENTRY_CONFIGURATION_SCHEMA);

// Route can either have a host_rewrite with fixed host header or automatic host rewrite
// based on the DNS name of the instance in the backing cluster.
if (auto_host_rewrite_ && !host_rewrite_.empty()) {
throw EnvoyException("routes cannot have both auto_host_rewrite and host_rewrite options set");
}

bool have_weighted_clusters = route.hasObject("weighted_clusters");
bool have_cluster =
!cluster_name_.empty() || !cluster_header_name_.get().empty() || have_weighted_clusters;
Expand Down
3 changes: 3 additions & 0 deletions source/common/router/config_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,7 @@ class RouteEntryImplBase : public RouteEntry,
}
std::chrono::milliseconds timeout() const override { return timeout_; }
const VirtualHost& virtualHost() const override { return vhost_; }
bool autoHostRewrite() const override { return auto_host_rewrite_; }

// Router::RedirectEntry
std::string newPath(const Http::HeaderMap& headers) const override;
Expand Down Expand Up @@ -258,6 +259,7 @@ class RouteEntryImplBase : public RouteEntry,
}

const VirtualHost& virtualHost() const override { return parent_->virtualHost(); }
bool autoHostRewrite() const override { return parent_->autoHostRewrite(); }

// Router::Route
const RedirectEntry* redirectEntry() const override { return nullptr; }
Expand Down Expand Up @@ -301,6 +303,7 @@ class RouteEntryImplBase : public RouteEntry,
static const uint64_t DEFAULT_ROUTE_TIMEOUT_MS = 15000;

const VirtualHostImpl& vhost_;
const bool auto_host_rewrite_;
const std::string cluster_name_;
const Http::LowerCaseString cluster_header_name_;
const std::chrono::milliseconds timeout_;
Expand Down
4 changes: 4 additions & 0 deletions source/common/router/router.cc
Original file line number Diff line number Diff line change
Expand Up @@ -704,6 +704,10 @@ void Filter::UpstreamRequest::onPoolReady(Http::StreamEncoder& request_encoder,
conn_pool_stream_handle_ = nullptr;
request_encoder_ = &request_encoder;
calling_encode_headers_ = true;
if (parent_.route_entry_->autoHostRewrite() && !host->hostname().empty()) {
parent_.downstream_headers_->Host()->value(host->hostname());
}

request_encoder.encodeHeaders(*parent_.downstream_headers_,
!buffered_request_body_ && encode_complete_ && !encode_trailers_);
calling_encode_headers_ = false;
Expand Down
6 changes: 3 additions & 3 deletions source/common/upstream/logical_dns_cluster.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ LogicalDnsCluster::LogicalDnsCluster(const Json::Object& config, Runtime::Loader
}

dns_url_ = hosts_json[0]->getString("url");
Network::Utility::hostFromTcpUrl(dns_url_);
hostname_ = Network::Utility::hostFromTcpUrl(dns_url_);
Copy link
Member

Choose a reason for hiding this comment

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

I would leave those 2 lines, they make sure the URL is well formed, which hostAndPortFromTcpUrl does not. Otherwise we might get an exception later on. Since it's probably not clear why these lines were here, please add a comment to that effect.

Network::Utility::portFromTcpUrl(dns_url_);

// This must come before startResolve(), since the resolve callback relies on
Expand Down Expand Up @@ -70,8 +70,8 @@ void LogicalDnsCluster::startResolve() {
// the friendly DNS name in that output, but currently there is no way to
// express a DNS name inside of an Address::Instance. For now this is OK but
// we might want to do better again later.
logical_host_.reset(
new LogicalHost(info_, Network::Utility::resolveUrl("tcp://0.0.0.0:0"), *this));
logical_host_.reset(new LogicalHost(
info_, hostname_, Network::Utility::resolveUrl("tcp://0.0.0.0:0"), *this));
HostVectorPtr new_hosts(new std::vector<HostPtr>());
new_hosts->emplace_back(logical_host_);
updateHosts(new_hosts, createHealthyHostList(*new_hosts), empty_host_lists_,
Expand Down
8 changes: 5 additions & 3 deletions source/common/upstream/logical_dns_cluster.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,9 @@ class LogicalDnsCluster : public ClusterImplBase {

private:
struct LogicalHost : public HostImpl {
LogicalHost(ClusterInfoPtr cluster, Network::Address::InstancePtr address,
LogicalDnsCluster& parent)
: HostImpl(cluster, address, false, 1, ""), parent_(parent) {}
LogicalHost(ClusterInfoPtr cluster, const std::string& hostname,
Network::Address::InstancePtr address, LogicalDnsCluster& parent)
: HostImpl(cluster, hostname, address, false, 1, ""), parent_(parent) {}

// Upstream::Host
CreateConnectionData createConnection(Event::Dispatcher& dispatcher) const override;
Expand All @@ -63,6 +63,7 @@ class LogicalDnsCluster : public ClusterImplBase {
return logical_host_->outlierDetector();
}
const HostStats& stats() const override { return logical_host_->stats(); }
const std::string& hostname() const override { return logical_host_->hostname(); }
Network::Address::InstancePtr address() const override { return address_; }
const std::string& zone() const override { return EMPTY_STRING; }

Expand All @@ -88,6 +89,7 @@ class LogicalDnsCluster : public ClusterImplBase {
bool initialized_;
Event::TimerPtr resolve_timer_;
std::string dns_url_;
std::string hostname_;
Network::Address::InstancePtr current_resolved_address_;
HostPtr logical_host_;
Network::ActiveDnsQuery* active_dns_query_{};
Expand Down
4 changes: 2 additions & 2 deletions source/common/upstream/sds.cc
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ void SdsClusterImpl::parseResponse(const Http::Message& response) {
}

new_hosts.emplace_back(
new HostImpl(info_, Network::Address::InstancePtr{new Network::Address::Ipv4Instance(
host->getString("ip_address"), host->getInteger("port"))},
new HostImpl(info_, "", Network::Address::InstancePtr{new Network::Address::Ipv4Instance(
host->getString("ip_address"), host->getInteger("port"))},
canary, weight, zone));
}

Expand Down
13 changes: 7 additions & 6 deletions source/common/upstream/upstream_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -267,8 +267,8 @@ StaticClusterImpl::StaticClusterImpl(const Json::Object& config, Runtime::Loader
std::vector<Json::ObjectPtr> hosts_json = config.getObjectArray("hosts");
HostVectorPtr new_hosts(new std::vector<HostPtr>());
for (Json::ObjectPtr& host : hosts_json) {
new_hosts->emplace_back(HostPtr{
new HostImpl(info_, Network::Utility::resolveUrl(host->getString("url")), false, 1, "")});
new_hosts->emplace_back(HostPtr{new HostImpl(
info_, "", Network::Utility::resolveUrl(host->getString("url")), false, 1, "")});
}

updateHosts(new_hosts, createHealthyHostList(*new_hosts), empty_host_lists_, empty_host_lists_,
Expand Down Expand Up @@ -420,10 +420,11 @@ void StrictDnsClusterImpl::ResolveTarget::startResolve() {
// address that has port in it. We need to both support IPv6 as well as potentially
// move port handling into the DNS interface itself, which would work better for
// SRV.
new_hosts.emplace_back(new HostImpl(
parent_.info_, Network::Address::InstancePtr{new Network::Address::Ipv4Instance(
address->ip()->addressAsString(), port_)},
false, 1, ""));
new_hosts.emplace_back(
new HostImpl(parent_.info_, dns_address_,
Network::Address::InstancePtr{new Network::Address::Ipv4Instance(
address->ip()->addressAsString(), port_)},
false, 1, ""));
}

std::vector<HostPtr> hosts_added;
Expand Down
15 changes: 9 additions & 6 deletions source/common/upstream/upstream_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,9 @@ namespace Upstream {
*/
class HostDescriptionImpl : virtual public HostDescription {
public:
HostDescriptionImpl(ClusterInfoPtr cluster, Network::Address::InstancePtr address, bool canary,
const std::string& zone)
: cluster_(cluster), address_(address), canary_(canary), zone_(zone),
HostDescriptionImpl(ClusterInfoPtr cluster, const std::string& hostname,
Network::Address::InstancePtr address, bool canary, const std::string& zone)
: cluster_(cluster), hostname_(hostname), address_(address), canary_(canary), zone_(zone),
stats_{ALL_HOST_STATS(POOL_COUNTER(stats_store_), POOL_GAUGE(stats_store_))} {}

// Upstream::HostDescription
Expand All @@ -41,11 +41,13 @@ class HostDescriptionImpl : virtual public HostDescription {
}
}
const HostStats& stats() const override { return stats_; }
const std::string& hostname() const override { return hostname_; }
Network::Address::InstancePtr address() const override { return address_; }
const std::string& zone() const override { return zone_; }

protected:
ClusterInfoPtr cluster_;
const std::string hostname_;
Network::Address::InstancePtr address_;
const bool canary_;
const std::string zone_;
Expand All @@ -64,9 +66,10 @@ class HostImpl : public HostDescriptionImpl,
public Host,
public std::enable_shared_from_this<HostImpl> {
public:
HostImpl(ClusterInfoPtr cluster, Network::Address::InstancePtr address, bool canary,
uint32_t initial_weight, const std::string& zone)
: HostDescriptionImpl(cluster, address, canary, zone) {
HostImpl(ClusterInfoPtr cluster, const std::string& hostname,
Network::Address::InstancePtr address, bool canary, uint32_t initial_weight,
const std::string& zone)
: HostDescriptionImpl(cluster, hostname, address, canary, zone) {
weight(initial_weight);
}

Expand Down
2 changes: 1 addition & 1 deletion test/common/filter/tcp_proxy_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,7 @@ class TcpProxyTest : public testing::Test {
Upstream::MockHost::MockCreateConnectionData conn_info;
conn_info.connection_ = upstream_connection_;
conn_info.host_.reset(
new Upstream::HostImpl(cluster_manager_.cluster_.info_,
new Upstream::HostImpl(cluster_manager_.cluster_.info_, "",
Network::Utility::resolveUrl("tcp://127.0.0.1:80"), false, 1, ""));
EXPECT_CALL(cluster_manager_, tcpConnForCluster_("fake_cluster")).WillOnce(Return(conn_info));
EXPECT_CALL(*upstream_connection_, addReadFilter(_))
Expand Down
2 changes: 1 addition & 1 deletion test/common/http/access_log/access_log_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ TEST_F(AccessLogImplTest, NoFilter) {
TEST_F(AccessLogImplTest, UpstreamHost) {
std::shared_ptr<Upstream::MockClusterInfo> cluster{new Upstream::MockClusterInfo()};
request_info_.upstream_host_ = std::make_shared<Upstream::HostDescriptionImpl>(
cluster, Network::Utility::resolveUrl("tcp://10.0.0.5:1234"), false, "");
cluster, "", Network::Utility::resolveUrl("tcp://10.0.0.5:1234"), false, "");

std::string json = R"EOF(
{
Expand Down
2 changes: 1 addition & 1 deletion test/common/http/codec_client_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ class CodecClientTest : public testing::Test {
Network::ReadFilterPtr filter_;
std::shared_ptr<Upstream::MockClusterInfo> cluster_{new NiceMock<Upstream::MockClusterInfo>()};
Upstream::HostDescriptionPtr host_{new Upstream::HostDescriptionImpl(
cluster_, Network::Utility::resolveUrl("tcp://127.0.0.1:80"), false, "")};
cluster_, "", Network::Utility::resolveUrl("tcp://127.0.0.1:80"), false, "")};
};

TEST_F(CodecClientTest, BasicHeaderOnlyResponse) {
Expand Down
9 changes: 5 additions & 4 deletions test/common/http/http1/conn_pool_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,11 @@ namespace Http1 {
class ConnPoolImplForTest : public ConnPoolImpl {
public:
ConnPoolImplForTest(Event::MockDispatcher& dispatcher, Upstream::ClusterInfoPtr cluster)
: ConnPoolImpl(dispatcher, Upstream::HostPtr{new Upstream::HostImpl(
cluster, Network::Utility::resolveUrl("tcp://127.0.0.1:9000"),
false, 1, "")},
Upstream::ResourcePriority::Default),
: ConnPoolImpl(
dispatcher,
Upstream::HostPtr{new Upstream::HostImpl(
cluster, "", Network::Utility::resolveUrl("tcp://127.0.0.1:9000"), false, 1, "")},
Upstream::ResourcePriority::Default),
mock_dispatcher_(dispatcher) {}

~ConnPoolImplForTest() {
Expand Down
2 changes: 1 addition & 1 deletion test/common/http/http2/conn_pool_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ class Http2ConnPoolImplTest : public testing::Test {
NiceMock<Event::MockDispatcher> dispatcher_;
std::shared_ptr<Upstream::MockClusterInfo> cluster_{new NiceMock<Upstream::MockClusterInfo>()};
Upstream::HostPtr host_{new Upstream::HostImpl(
cluster_, Network::Utility::resolveUrl("tcp://127.0.0.1:80"), false, 1, "")};
cluster_, "", Network::Utility::resolveUrl("tcp://127.0.0.1:80"), false, 1, "")};
TestConnPoolImpl pool_;
std::vector<TestCodecClient> test_clients_;
NiceMock<Runtime::MockLoader> runtime_;
Expand Down
2 changes: 1 addition & 1 deletion test/common/network/filter_manager_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ TEST_F(NetworkFilterManagerTest, RateLimitAndTcpProxy) {
Upstream::MockHost::MockCreateConnectionData conn_info;
conn_info.connection_ = upstream_connection;
conn_info.host_.reset(new Upstream::HostImpl(
cm.cluster_.info_, Utility::resolveUrl("tcp://127.0.0.1:80"), false, 1, ""));
cm.cluster_.info_, "", Utility::resolveUrl("tcp://127.0.0.1:80"), false, 1, ""));
EXPECT_CALL(cm, tcpConnForCluster_("fake_cluster")).WillOnce(Return(conn_info));

request_callbacks->complete(RateLimit::LimitStatus::OK);
Expand Down
2 changes: 1 addition & 1 deletion test/common/redis/conn_pool_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ class RedisClientImplTest : public testing::Test, public DecoderFactory {
Upstream::MockHost::MockCreateConnectionData conn_info;
conn_info.connection_ = upstream_connection_;
conn_info.host_.reset(new Upstream::HostImpl(
cm_.cluster_.info_, Network::Utility::resolveUrl("tcp://127.0.0.1:80"), false, 1, ""));
cm_.cluster_.info_, "", Network::Utility::resolveUrl("tcp://127.0.0.1:80"), false, 1, ""));
EXPECT_CALL(cm_, tcpConnForCluster_("foo")).WillOnce(Return(conn_info));
EXPECT_CALL(*upstream_connection_, addReadFilter(_))
.WillOnce(SaveArg<0>(&upstream_read_filter_));
Expand Down
26 changes: 26 additions & 0 deletions test/common/router/config_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -412,6 +412,32 @@ TEST(RouteMatcherTest, Priority) {
}
}

TEST(RouteMatcherTest, NoHostRewriteAndAutoRewrite) {
std::string json = R"EOF(
{
"virtual_hosts": [
{
"name": "local_service",
"domains": ["*"],
"routes": [
{
"prefix": "/",
"cluster": "local_service",
"host_rewrite": "foo",
"auto_host_rewrite" : true
}
]
}
]
}
)EOF";

Json::ObjectPtr loader = Json::Factory::LoadFromString(json);
NiceMock<Runtime::MockLoader> runtime;
NiceMock<Upstream::MockClusterManager> cm;
EXPECT_THROW(ConfigImpl(*loader, runtime, cm, true), EnvoyException);
}

TEST(RouteMatcherTest, HeaderMatchedRouting) {
std::string json = R"EOF(
{
Expand Down
Loading