You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by zw...@apache.org on 2022/05/16 15:28:19 UTC
[trafficserver] branch 9.2.x updated: Fix parent_select optional scheme (#8831)
This is an automated email from the ASF dual-hosted git repository.
zwoop pushed a commit to branch 9.2.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/9.2.x by this push:
new 1005a6c1b Fix parent_select optional scheme (#8831)
1005a6c1b is described below
commit 1005a6c1b655cacd8eb0e0087e73a2f1829714c9
Author: Robert O Butts <ro...@users.noreply.github.com>
AuthorDate: Mon May 16 09:23:24 2022 -0600
Fix parent_select optional scheme (#8831)
(cherry picked from commit 695f9f6c40e5cecae594f8f7e2b41e8b92e2ad49)
---
plugins/experimental/parent_select/strategy.cc | 15 ++-
proxy/http/remap/NextHopSelectionStrategy.cc | 15 ++-
.../parent_select_optional_scheme_matching.test.py | 134 +++++++++++++++++++++
3 files changed, 148 insertions(+), 16 deletions(-)
diff --git a/plugins/experimental/parent_select/strategy.cc b/plugins/experimental/parent_select/strategy.cc
index ddefc67f3..270ff8cc8 100644
--- a/plugins/experimental/parent_select/strategy.cc
+++ b/plugins/experimental/parent_select/strategy.cc
@@ -71,6 +71,7 @@ PLNextHopSelectionStrategy::Init(const YAML::Node &n)
bool self_host_used = false;
try {
+ // scheme is optional, and strategies with no scheme will match hosts with no scheme
if (n["scheme"]) {
auto scheme_val = n["scheme"].Scalar();
if (scheme_val == "http") {
@@ -78,9 +79,7 @@ PLNextHopSelectionStrategy::Init(const YAML::Node &n)
} else if (scheme_val == "https") {
scheme = PL_NH_SCHEME_HTTPS;
} else {
- scheme = PL_NH_SCHEME_NONE;
- PL_NH_Note("Invalid 'scheme' value, '%s', for the strategy named '%s', setting to PL_NH_SCHEME_NONE", scheme_val.c_str(),
- strategy_name.c_str());
+ PL_NH_Note("Invalid scheme '%s' for strategy '%s', setting to NONE", scheme_val.c_str(), strategy_name.c_str());
}
}
@@ -395,16 +394,16 @@ template <> struct convert<PLNHProtocol> {
static bool
decode(const Node &node, PLNHProtocol &nh)
{
+ // scheme is optional, and strategies with no scheme will match hosts with no scheme
if (node["scheme"]) {
- if (node["scheme"].Scalar() == "http") {
+ const auto scheme_val = node["scheme"].Scalar();
+ if (scheme_val == "http") {
nh.scheme = PL_NH_SCHEME_HTTP;
- } else if (node["scheme"].Scalar() == "https") {
+ } else if (scheme_val == "https") {
nh.scheme = PL_NH_SCHEME_HTTPS;
} else {
- throw YAML::ParserException(node["scheme"].Mark(), "no valid scheme defined, valid schemes are http or https");
+ PL_NH_Note("Invalid scheme '%s' for protocol, setting to NONE", scheme_val.c_str());
}
- } else {
- throw YAML::ParserException(node["scheme"].Mark(), "no scheme defined, valid schemes are http or https");
}
if (node["port"]) {
nh.port = node["port"].as<int>();
diff --git a/proxy/http/remap/NextHopSelectionStrategy.cc b/proxy/http/remap/NextHopSelectionStrategy.cc
index b0cb85314..0ae5677f3 100644
--- a/proxy/http/remap/NextHopSelectionStrategy.cc
+++ b/proxy/http/remap/NextHopSelectionStrategy.cc
@@ -60,6 +60,7 @@ NextHopSelectionStrategy::Init(ts::Yaml::Map &n)
bool self_host_used = false;
try {
+ // scheme is optional, and strategies with no scheme will match hosts with no scheme
if (n["scheme"]) {
auto scheme_val = n["scheme"].Scalar();
if (scheme_val == "http") {
@@ -67,9 +68,7 @@ NextHopSelectionStrategy::Init(ts::Yaml::Map &n)
} else if (scheme_val == "https") {
scheme = NH_SCHEME_HTTPS;
} else {
- scheme = NH_SCHEME_NONE;
- NH_Note("Invalid 'scheme' value, '%s', for the strategy named '%s', setting to NH_SCHEME_NONE", scheme_val.c_str(),
- strategy_name.c_str());
+ NH_Note("Invalid scheme '%s' for strategy '%s', setting to NONE", scheme_val.c_str(), strategy_name.c_str());
}
}
@@ -387,16 +386,16 @@ template <> struct convert<NHProtocol> {
{
ts::Yaml::Map map{node};
+ // scheme is optional, and strategies with no scheme will match hosts with no scheme
if (map["scheme"]) {
- if (map["scheme"].Scalar() == "http") {
+ const auto scheme_val = map["scheme"].Scalar();
+ if (scheme_val == "http") {
nh.scheme = NH_SCHEME_HTTP;
- } else if (map["scheme"].Scalar() == "https") {
+ } else if (scheme_val == "https") {
nh.scheme = NH_SCHEME_HTTPS;
} else {
- throw YAML::ParserException(map["scheme"].Mark(), "no valid scheme defined, valid schemes are http or https");
+ NH_Note("Invalid scheme '%s' for protocol, setting to NONE", scheme_val.c_str());
}
- } else {
- throw YAML::ParserException(map["scheme"].Mark(), "no scheme defined, valid schemes are http or https");
}
if (map["port"]) {
nh.port = map["port"].as<int>();
diff --git a/tests/gold_tests/pluginTest/parent_select/parent_select_optional_scheme_matching.test.py b/tests/gold_tests/pluginTest/parent_select/parent_select_optional_scheme_matching.test.py
new file mode 100755
index 000000000..c1754da0f
--- /dev/null
+++ b/tests/gold_tests/pluginTest/parent_select/parent_select_optional_scheme_matching.test.py
@@ -0,0 +1,134 @@
+'''
+'''
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+Test.Summary = '''
+Basic parent_select plugin test
+'''
+
+Test.SkipUnless(
+ Condition.PluginExists('parent_select.so'),
+)
+Test.ContinueOnFail = False
+
+# Define and populate MicroServer.
+#
+server = Test.MakeOriginServer("server")
+response_header = {
+ "headers":
+ "HTTP/1.1 200 OK\r\n"
+ "Connection: close\r\n"
+ "Cache-control: max-age=85000\r\n"
+ "\r\n",
+ "timestamp": "1469733493.993",
+ "body": "This is the body.\n"
+}
+num_objects = 32
+for i in range(num_objects):
+ request_header = {
+ "headers":
+ f"GET /obj{i} HTTP/1.1\r\n"
+ "Host: does.not.matter\r\n" # But cannot be omitted.
+ "\r\n",
+ "timestamp": "1469733493.993",
+ "body": ""
+ }
+ server.addResponse("sessionlog.json", request_header, response_header)
+
+dns = Test.MakeDNServer("dns")
+
+# Define next hop trafficserver instances.
+#
+num_nh = 8
+ts_nh = []
+for i in range(num_nh):
+ ts = Test.MakeATSProcess(f"ts_nh{i}", command=f"traffic_server 2>nh_trace{i}.log")
+ ts.Disk.records_config.update({
+ 'proxy.config.diags.debug.enabled': 1,
+ 'proxy.config.diags.debug.tags': 'http|dns',
+ 'proxy.config.dns.nameservers': f"127.0.0.1:{dns.Variables.Port}",
+ 'proxy.config.dns.resolv_conf': "NULL",
+ })
+ ts.Disk.remap_config.AddLine(
+ f"map / http://127.0.0.1:{server.Variables.Port}"
+ )
+ ts_nh.append(ts)
+
+ts = Test.MakeATSProcess("ts")
+
+ts.Disk.records_config.update({
+ 'proxy.config.diags.debug.enabled': 1,
+ 'proxy.config.diags.debug.tags': 'http|dns|parent|next_hop|host_statuses|hostdb',
+ 'proxy.config.dns.nameservers': f"127.0.0.1:{dns.Variables.Port}", # Only nameservers if resolv_conf NULL.
+ 'proxy.config.dns.resolv_conf': "NULL", # This defaults to /etc/resvolv.conf (OS namesevers) if not NULL.
+ 'proxy.config.http.cache.http': 0,
+ 'proxy.config.http.uncacheable_requests_bypass_parent': 0,
+ 'proxy.config.http.no_dns_just_forward_to_parent': 1,
+ 'proxy.config.http.parent_proxy.mark_down_hostdb': 0,
+ 'proxy.config.http.parent_proxy.self_detect': 0,
+})
+
+ts.Disk.File(ts.Variables.CONFIGDIR + "/strategies.yaml", id="strategies", typename="ats:config")
+s = ts.Disk.strategies
+s.AddLine("groups:")
+s.AddLine(" - &g1")
+for i in range(num_nh):
+ dns.addRecords(records={f"next_hop{i}": ["127.0.0.1"]})
+ s.AddLine(f" - host: next_hop{i}")
+ s.AddLine(f" protocol:")
+ s.AddLine(f" - port: {ts_nh[i].Variables.port}")
+ # The health check URL does not seem to be used currently.
+ # s.AddLine(f" health_check_url: http://next_hop{i}:{ts_nh[i].Variables.port}")
+ s.AddLine(f" weight: 1.0")
+s.AddLines([
+ "strategies:",
+ " - strategy: the-strategy",
+ " policy: consistent_hash",
+ " hash_key: path",
+ " go_direct: false",
+ " parent_is_proxy: true",
+ " ignore_self_detect: true",
+ " groups:",
+ " - *g1"])
+
+ts.Disk.remap_config.AddLine(
+ "map http://dummy.com http://not_used @plugin=parent_select.so @pparam=" +
+ ts.Variables.CONFIGDIR +
+ "/strategies.yaml @pparam=the-strategy")
+
+tr = Test.AddTestRun()
+tr.Processes.Default.StartBefore(server)
+tr.Processes.Default.StartBefore(dns)
+for i in range(num_nh):
+ tr.Processes.Default.StartBefore(ts_nh[i])
+tr.Processes.Default.StartBefore(Test.Processes.ts)
+tr.Processes.Default.Command = 'echo start TS, HTTP server, DNS server and next hop TSes'
+tr.Processes.Default.ReturnCode = 0
+
+for i in range(num_objects):
+ tr = Test.AddTestRun()
+ tr.Processes.Default.Command = (
+ f'curl --verbose --proxy 127.0.0.1:{ts.Variables.port} http://dummy.com/obj{i}'
+ )
+ tr.Processes.Default.Streams.stdout = "body.gold"
+ tr.Processes.Default.ReturnCode = 0
+
+tr = Test.AddTestRun()
+# For some reason, the * won't be expanded when the command is executed, if stdout is not piped through "cat".
+tr.Processes.Default.Command = "grep -F '200 OK' nh_trace*.log | cat"
+tr.Processes.Default.Streams.stdout = "trace.gold"
+tr.Processes.Default.ReturnCode = 0