diff --git a/src/api-umbrella/cli/read_config.lua b/src/api-umbrella/cli/read_config.lua index 13bf56a4f..4045c032e 100644 --- a/src/api-umbrella/cli/read_config.lua +++ b/src/api-umbrella/cli/read_config.lua @@ -202,14 +202,23 @@ local function set_computed_config() end end - -- Add a default fallback host that will match any hostname, but doesn't - -- include any host-specific settings in nginx (like rewrites). This host can - -- still then be used to match APIs for unknown hosts. - table.insert(config["hosts"], { - hostname = "*", - _nginx_server_name = "_", - default = (not default_host_exists), - }) + -- If a default host hasn't been explicitly defined, then add a default + -- fallback host that will match any hostname (but doesn't include any + -- host-specific settings in nginx, like rewrites). A default host is + -- necessary so nginx handles all hostnames, allowing APIs to be matched for + -- hosts that are only defined in the API backend configuration. + if not default_host_exists then + table.insert(config["hosts"], { + hostname = "*", + -- Use a slightly different nginx server name to avoid any conflicts with + -- explicitly defined wildcard hosts (but aren't the default, which + -- doesn't seem particularly likely). There's nothing actually special + -- about "_" in nginx, it's just a hostname that won't match anything + -- real. + _nginx_server_name = "__", + default = true, + }) + end local default_hostname if config["hosts"] then diff --git a/templates/etc/nginx/router.conf.mustache b/templates/etc/nginx/router.conf.mustache index 2b77cf841..ab342ecfc 100644 --- a/templates/etc/nginx/router.conf.mustache +++ b/templates/etc/nginx/router.conf.mustache @@ -221,12 +221,10 @@ http { server { {{#listen.addresses}} listen {{.}}:{{http_port}}{{#default}} default_server so_keepalive=on{{/default}}; + listen {{.}}:{{https_port}} ssl{{#default}} default_server so_keepalive=on{{/default}}; {{/listen.addresses}} server_name {{_nginx_server_name}}; - {{#listen.addresses}} - listen {{.}}:{{https_port}} ssl{{#default}} default_server so_keepalive=on{{/default}}; - {{/listen.addresses}} {{#ssl_cert}} ssl_certificate {{ssl_cert}}; ssl_certificate_key {{ssl_cert_key}}; diff --git a/test/proxy/test_nginx_rewrites.rb b/test/proxy/test_nginx_rewrites.rb index 7c2f0ab77..d96a4a8d0 100644 --- a/test/proxy/test_nginx_rewrites.rb +++ b/test/proxy/test_nginx_rewrites.rb @@ -9,6 +9,7 @@ def setup end def test_basic_rewrites + log_tail = LogTail.new("nginx/current") override_config({ "hosts" => [ { @@ -24,6 +25,8 @@ def test_basic_rewrites }, ], }, "--router") do + refute_nginx_duplicate_server_name_warnings(log_tail) + # Basic rewrite response = Typhoeus.get("https://127.0.0.1:9081/#{unique_test_id}/hello/rewrite?foo=bar", http_options) assert_response_code(301, response) @@ -37,6 +40,7 @@ def test_basic_rewrites end def test_default_host + log_tail = LogTail.new("nginx/current") override_config({ "hosts" => [ { @@ -51,6 +55,8 @@ def test_default_host }, ], }, "--router") do + refute_nginx_duplicate_server_name_warnings(log_tail) + # Known host without rewrites response = Typhoeus.get("https://127.0.0.1:9081/#{unique_test_id}/hello/rewrite?foo=bar", http_options.deep_merge({ :headers => { "Host" => "default.foo" }, @@ -74,6 +80,7 @@ def test_default_host end def test_no_default_host + log_tail = LogTail.new("nginx/current") override_config({ "hosts" => [ { @@ -84,6 +91,8 @@ def test_no_default_host }, ], }, "--router") do + refute_nginx_duplicate_server_name_warnings(log_tail) + # Known host without rewrites response = Typhoeus.get("https://127.0.0.1:9081/#{unique_test_id}/hello/rewrite?foo=bar", http_options.deep_merge({ :headers => { "Host" => "default.foo" }, @@ -99,7 +108,138 @@ def test_no_default_host end end + # When nginx has no default_server defined, then the first server defined + # becomes the default one. So test to see how this ordering also interacts + # with a wildcard host. + def test_no_default_host_wildcard_first + log_tail = LogTail.new("nginx/current") + override_config({ + "hosts" => [ + { + "hostname" => "*", + "rewrites" => [ + "^/#{unique_test_id}/hello/rewrite https://example.com/something/ permanent", + ], + }, + { + "hostname" => "known.foo", + }, + ], + }, "--router") do + refute_nginx_duplicate_server_name_warnings(log_tail) + + # Known host without rewrites + response = Typhoeus.get("https://127.0.0.1:9081/#{unique_test_id}/hello/rewrite?foo=bar", http_options.deep_merge({ + :headers => { "Host" => "known.foo" }, + })) + assert_response_code(404, response) + + # Unknown host + response = Typhoeus.get("https://127.0.0.1:9081/#{unique_test_id}/hello/rewrite?foo=bar", http_options.deep_merge({ + :headers => { "Host" => "unknown.foo" }, + })) + assert_response_code(404, response) + end + end + + def test_no_default_host_wildcard_last + log_tail = LogTail.new("nginx/current") + override_config({ + "hosts" => [ + { + "hostname" => "known.foo", + }, + { + "hostname" => "*", + "rewrites" => [ + "^/#{unique_test_id}/hello/rewrite https://example.com/something/ permanent", + ], + }, + ], + }, "--router") do + refute_nginx_duplicate_server_name_warnings(log_tail) + + # Known host without rewrites + response = Typhoeus.get("https://127.0.0.1:9081/#{unique_test_id}/hello/rewrite?foo=bar", http_options.deep_merge({ + :headers => { "Host" => "known.foo" }, + })) + assert_response_code(404, response) + + # Unknown host + response = Typhoeus.get("https://127.0.0.1:9081/#{unique_test_id}/hello/rewrite?foo=bar", http_options.deep_merge({ + :headers => { "Host" => "unknown.foo" }, + })) + assert_response_code(404, response) + end + end + + def test_wildcard_is_default + log_tail = LogTail.new("nginx/current") + override_config({ + "hosts" => [ + { + "hostname" => "*", + "default" => true, + "rewrites" => [ + "^/#{unique_test_id}/hello/rewrite https://example.com/something/ permanent", + ], + }, + { + "hostname" => "known.foo", + }, + ], + }, "--router") do + refute_nginx_duplicate_server_name_warnings(log_tail) + + # Known host without rewrites + response = Typhoeus.get("https://127.0.0.1:9081/#{unique_test_id}/hello/rewrite?foo=bar", http_options.deep_merge({ + :headers => { "Host" => "known.foo" }, + })) + assert_response_code(404, response) + + # Unknown host + response = Typhoeus.get("https://127.0.0.1:9081/#{unique_test_id}/hello/rewrite?foo=bar", http_options.deep_merge({ + :headers => { "Host" => "unknown.foo" }, + })) + assert_response_code(301, response) + assert_equal("https://example.com/something/?foo=bar", response.headers["location"]) + end + end + + def test_wildcard_is_not_default + log_tail = LogTail.new("nginx/current") + override_config({ + "hosts" => [ + { + "hostname" => "*", + "rewrites" => [ + "^/#{unique_test_id}/hello/rewrite https://example.com/something/ permanent", + ], + }, + { + "hostname" => "known.foo", + "default" => true, + }, + ], + }, "--router") do + refute_nginx_duplicate_server_name_warnings(log_tail) + + # Known host without rewrites + response = Typhoeus.get("https://127.0.0.1:9081/#{unique_test_id}/hello/rewrite?foo=bar", http_options.deep_merge({ + :headers => { "Host" => "known.foo" }, + })) + assert_response_code(404, response) + + # Unknown host + response = Typhoeus.get("https://127.0.0.1:9081/#{unique_test_id}/hello/rewrite?foo=bar", http_options.deep_merge({ + :headers => { "Host" => "unknown.foo" }, + })) + assert_response_code(404, response) + end + end + def test_precedence + log_tail = LogTail.new("nginx/current") override_config({ "apis" => [ { @@ -129,6 +269,8 @@ def test_precedence }, ], }, "--router") do + refute_nginx_duplicate_server_name_warnings(log_tail) + http_opts = http_options.deep_merge({ :headers => { "Host" => "with-apis-and-website.foo" }, }) @@ -166,4 +308,13 @@ def test_precedence assert_match("
API Umbrella
", response.body) end end + + private + + def refute_nginx_duplicate_server_name_warnings(log_tail) + log_output = log_tail.read + assert_match(/\[notice\].*: reconfiguring/, log_output) + refute_match("conflicting server name", log_output) + refute_match("warn", log_output) + end end diff --git a/test/support/log_tail.rb b/test/support/log_tail.rb new file mode 100644 index 000000000..43021005a --- /dev/null +++ b/test/support/log_tail.rb @@ -0,0 +1,20 @@ +class LogTail + def initialize(filename) + @path = File.join($config["log_dir"], filename) + File.open(@path) do |file| + file.seek(0, IO::SEEK_END) + @pos = file.pos + end + end + + def read + output = nil + File.open(@path) do |file| + file.seek(@pos) + output = file.read + @pos = file.pos + end + + output + end +end