Skip to content

Commit

Permalink
fix(configuration): fixed an issue where debug_listen incorrectly u…
Browse files Browse the repository at this point in the history
…sed the SSL-related configuration of `status_listen` (#9815)

The SSL-related configuration for 'debug_listen' and 'status_listen' should be separate.

This PR also adjusted the indentation of the spec-ee/02-integration/11-cmd/03-debug_spec.lua.

Fix: [FTI-6123](https://konghq.atlassian.net/browse/FTI-6123)
  • Loading branch information
tzssangglass authored Aug 5, 2024
1 parent ebb235d commit a4e8c6c
Show file tree
Hide file tree
Showing 9 changed files with 89 additions and 6 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
message: |
Fixed an issue where `debug_listen` incorrectly used the SSL-related configuration of `status_listen`.
type: bugfix
scope: Configuration
6 changes: 5 additions & 1 deletion kong/cmd/utils/prefix_handler.lua
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,9 @@ local function gen_default_ssl_cert(kong_config, target)
elseif target == "portal_gui" then
ssl_cert = kong_config["portal_gui_ssl_cert_default" .. suffix]
ssl_cert_key = kong_config["portal_gui_ssl_cert_key_default" .. suffix]
elseif target == "debug" then
ssl_cert = kong_config["debug_ssl_cert_default" .. suffix]
ssl_cert_key = kong_config["debug_ssl_cert_key_default" .. suffix]
-- EE ]]

else
Expand Down Expand Up @@ -567,7 +570,7 @@ local function prepare_prefix(kong_config, nginx_custom_template_path, skip_writ
-- generate default SSL certs if needed
-- [[ XXX EE: adding portal_gui, and portal_api ]]
do
for _, target in ipairs({ "proxy", "admin", "admin_gui", "status", "portal_gui", "portal_api" }) do
for _, target in ipairs({ "proxy", "admin", "admin_gui", "status", "portal_gui", "portal_api", "debug" }) do
local ssl_enabled = kong_config[target .. "_ssl_enabled"]
if not ssl_enabled and target == "proxy" then
ssl_enabled = kong_config.stream_proxy_ssl_enabled
Expand Down Expand Up @@ -684,6 +687,7 @@ local function prepare_prefix(kong_config, nginx_custom_template_path, skip_writ
-- [[ XXX EE
"portal_gui",
"portal_api",
"debug",
"keyring_public",
"keyring_private",
"keyring_recovery_public"
Expand Down
2 changes: 1 addition & 1 deletion kong/conf_loader/init.lua
Original file line number Diff line number Diff line change
Expand Up @@ -869,7 +869,7 @@ local function load(path, custom_conf, opts)
conf.enabled_headers_upstream = setmetatable(enabled_headers_upstream, conf_constants._NOP_TOSTRING_MT)
end

for _, prefix in ipairs({ "ssl", "admin_ssl", "admin_gui_ssl", "status_ssl", "client_ssl", "cluster" }) do
for _, prefix in ipairs({ "ssl", "admin_ssl", "admin_gui_ssl", "status_ssl", "client_ssl", "cluster", "debug_ssl" }) do
local ssl_cert = conf[prefix .. "_cert"]
local ssl_cert_key = conf[prefix .. "_cert_key"]

Expand Down
2 changes: 1 addition & 1 deletion kong/conf_loader/parse.lua
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ local function check_and_parse(conf, opts)
end
end

for _, prefix in ipairs({ "proxy_", "admin_", "admin_gui_", "status_" }) do
for _, prefix in ipairs({ "proxy_", "admin_", "admin_gui_", "status_", "debug_" }) do
local listen = conf[prefix .. "listen"]

local ssl_enabled = find(concat(listen, ",") .. " ", "%sssl[%s,]") ~= nil
Expand Down
5 changes: 5 additions & 0 deletions kong/enterprise_edition/conf_loader.lua
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,11 @@ local EE_PREFIX_PATHS = {
portal_gui_ssl_cert_key_default = {"ssl", "portal-gui-kong-default.key"},
portal_api_ssl_cert_default_ecdsa = {"ssl", "portal-api-kong-default-ecdsa.crt"},
portal_api_ssl_cert_key_default_ecdsa = {"ssl", "portal-api-kong-default-ecdsa.key"},

debug_ssl_cert_default = {"ssl", "debug-kong-default.crt"},
debug_ssl_cert_key_default = {"ssl", "debug-kong-default.key"},
debug_ssl_cert_default_ecdsa = {"ssl", "debug-kong-default-ecdsa.crt"},
debug_ssl_cert_key_default_ecdsa = {"ssl", "debug-kong-default-ecdsa.key"},
}


Expand Down
4 changes: 2 additions & 2 deletions kong/templates/nginx_kong.lua
Original file line number Diff line number Diff line change
Expand Up @@ -795,8 +795,8 @@ server {
error_log ${{DEBUG_ERROR_LOG}} ${{LOG_LEVEL}};
> if #debug_listeners > 0 then
> if status_ssl_enabled then
> for i = 1, #status_ssl_cert do
> if debug_ssl_enabled then
> for i = 1, #debug_ssl_cert do
ssl_certificate $(debug_ssl_cert[i]);
ssl_certificate_key $(debug_ssl_cert_key[i]);
> end
Expand Down
27 changes: 27 additions & 0 deletions spec-ee/02-integration/11-cmd/03-debug_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -239,4 +239,31 @@ for __, deploy in ipairs({ "traditional", "hybrid" }) do
end)

end

describe("debug_listen ssl configuration", function()
after_each(function()
assert(helpers.stop_kong())
end)

it("the ssl configuration of debug_listen and status_listen is independent", function()
assert(helpers.start_kong({
database = strategy,
nginx_conf = "spec/fixtures/custom_nginx.template",
debug_listen = string.format("%s:%d", DEBUG_LISTEN_HOST, DEBUG_LISTEN_PORT),
debug_listen_local = false,
status_listen = "0.0.0.0:8100 ssl",
}))
end)

it("debug_listen ssl enable", function()
assert(helpers.start_kong({
database = strategy,
nginx_conf = "spec/fixtures/custom_nginx.template",
debug_listen = string.format("%s:%d", DEBUG_LISTEN_HOST, DEBUG_LISTEN_PORT) .. " ssl",
debug_listen_local = false,
status_listen = "0.0.0.0:8100 ssl",
}))
end)
end)

end
11 changes: 11 additions & 0 deletions spec/01-unit/03-conf_loader_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,8 @@ describe("Configuration loader", function()
assert.same({}, conf.admin_gui_ssl_cert_key)
assert.same({}, conf.status_ssl_cert)
assert.same({}, conf.status_ssl_cert_key)
assert.same({}, conf.debug_ssl_cert)
assert.same({}, conf.debug_ssl_cert_key)
assert.same(nil, conf.privileged_agent)
assert.same(true, conf.dedicated_config_processing)
assert.same(false, conf.allow_debug_header)
Expand Down Expand Up @@ -324,6 +326,8 @@ describe("Configuration loader", function()
assert.equal("/usr/local/kong/ssl/admin-gui-kong-default.key", conf.admin_gui_ssl_cert_key_default)
assert.equal("/usr/local/kong/ssl/status-kong-default.crt", conf.status_ssl_cert_default)
assert.equal("/usr/local/kong/ssl/status-kong-default.key", conf.status_ssl_cert_key_default)
assert.equal("/usr/local/kong/ssl/debug-kong-default.crt", conf.debug_ssl_cert_default)
assert.equal("/usr/local/kong/ssl/debug-kong-default.key", conf.debug_ssl_cert_key_default)
end)
it("should populate correct admin_gui_origin", function()
local conf, _, errors = conf_loader(nil, {})
Expand Down Expand Up @@ -900,6 +904,8 @@ describe("Configuration loader", function()
admin_gui_ssl_cert_key = key,
status_ssl_cert = cert,
status_ssl_cert_key = key,
debug_ssl_cert = cert,
debug_ssl_cert_key = key,
client_ssl_cert = cert,
client_ssl_cert_key = key,
cluster_cert = cert,
Expand All @@ -917,6 +923,7 @@ describe("Configuration loader", function()
proxy_listen = "127.0.0.1:456 ssl",
admin_listen = "127.0.0.1:789 ssl",
admin_gui_listen = "127.0.0.1:8445 ssl",
debug_listen = "127.0.0.1:890 ssl",
}

for n, v in pairs(properties) do
Expand Down Expand Up @@ -2722,6 +2729,10 @@ describe("Configuration loader", function()
portal_gui_ssl_cert_key_default = true,
portal_api_ssl_cert_default_ecdsa = true,
portal_api_ssl_cert_key_default_ecdsa = true,
debug_ssl_cert_default = true,
debug_ssl_cert_key_default = true,
debug_ssl_cert_default_ecdsa = true,
debug_ssl_cert_key_default_ecdsa = true,
}
local conf = assert(conf_loader(nil, nil, { pre_cmd = true }))
for k, _ in pairs(conf) do
Expand Down
34 changes: 33 additions & 1 deletion spec/01-unit/04-prefix_handler_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,26 @@ describe("NGINX conf compiler", function()
end
end)
end)

describe("debug", function()
it("auto-generates SSL certificate and key", function()
assert(prefix_handler.gen_default_ssl_cert(conf, "debug"))
for _, suffix in ipairs({ "", "_ecdsa" }) do
assert(exists(conf["debug_ssl_cert_default" .. suffix]))
assert(exists(conf["debug_ssl_cert_key_default" .. suffix]))
end
end)
it("does not re-generate if they already exist", function()
assert(prefix_handler.gen_default_ssl_cert(conf, "debug"))
for _, suffix in ipairs({ "", "_ecdsa" }) do
local cer = helpers.file.read(conf["debug_ssl_cert_default" .. suffix])
local key = helpers.file.read(conf["debug_ssl_cert_key_default" .. suffix])
assert(prefix_handler.gen_default_ssl_cert(conf, "debug"))
assert.equal(cer, helpers.file.read(conf["debug_ssl_cert_default" .. suffix]))
assert.equal(key, helpers.file.read(conf["debug_ssl_cert_key_default" .. suffix]))
end
end)
end)
--]]
end)

Expand Down Expand Up @@ -361,6 +381,7 @@ describe("NGINX conf compiler", function()
admin_gui_listen = "127.0.0.1:8002",
portal_gui_listen = "0.0.0.0:9003",
portal_api_listen = "0.0.0.0:9004",
debug_listen = "127.0.0.1:9005",
}))
local kong_nginx_conf = prefix_handler.compile_kong_conf(conf)
assert.not_matches("listen%s+%d+%.%d+%.%d+%.%d+:%d+ ssl;", kong_nginx_conf)
Expand Down Expand Up @@ -1377,6 +1398,9 @@ describe("NGINX conf compiler", function()
portal_api_listen = "0.0.0.0:9004, 0.0.0.0:9447 ssl",
portal_api_ssl_cert = "spec/fixtures/kong_spec.crt",
portal_api_ssl_cert_key = "spec/fixtures/kong_spec.key",
debug_listen = "127.0.0.1:8007 ssl",
debug_ssl_cert = "spec/fixtures/kong_spec.crt",
debug_ssl_cert_key = "spec/fixtures/kong_spec.key",
})

assert(prefix_handler.prepare_prefix(conf))
Expand All @@ -1399,6 +1423,7 @@ describe("NGINX conf compiler", function()
portal_and_vitals_key = get_portal_and_vitals_key(),
portal_api_listen = "127.0.0.1:8004 ssl",
portal_admin_listen = "127.0.0.1:8005 ssl",
debug_listen = "127.0.0.1:8006 ssl",
-- ]]
})

Expand All @@ -1419,6 +1444,8 @@ describe("NGINX conf compiler", function()
assert.truthy(exists(conf["portal_api_ssl_cert_key_default" .. suffix]))
assert.truthy(exists(conf["portal_gui_ssl_cert_default" .. suffix]))
assert.truthy(exists(conf["portal_gui_ssl_cert_key_default" .. suffix]))
assert.truthy(exists(conf["debug_ssl_cert_default" .. suffix]))
assert.truthy(exists(conf["debug_ssl_cert_key_default" .. suffix]))
-- ]]
end

Expand All @@ -1431,10 +1458,11 @@ describe("NGINX conf compiler", function()
admin_listen = "127.0.0.1:8001 ssl",
admin_gui_listen = "127.0.0.1:8002 ssl",
status_listen = "127.0.0.1:8003 ssl",
debug_listen = "127.0.0.1:8006 ssl",
})

assert(prefix_handler.prepare_prefix(conf))
for _, prefix in ipairs({ "", "status_", "admin_", "admin_gui_" }) do
for _, prefix in ipairs({ "", "status_", "admin_", "admin_gui_", "debug_" }) do
for _, suffix in ipairs({ "", "_ecdsa" }) do
local handle = io.popen("ls -l " .. conf[prefix .. "ssl_cert_default" .. suffix])
local result = handle:read("*a")
Expand Down Expand Up @@ -1498,6 +1526,8 @@ describe("NGINX conf compiler", function()
portal_gui_ssl_cert_key = key,
portal_api_ssl_cert = cert,
portal_api_ssl_cert_key = key,
debug_ssl_cert = cert,
debug_ssl_cert_key = key,
keyring_public_key = key,
keyring_private_key = key,
keyring_recovery_public_key = key
Expand Down Expand Up @@ -1576,6 +1606,8 @@ describe("NGINX conf compiler", function()
admin_gui_ssl_cert_key = key,
status_ssl_cert = cert,
status_ssl_cert_key = key,
debug_ssl_cert = cert,
debug_ssl_cert_key = key,
client_ssl_cert = cert,
client_ssl_cert_key = key,
cluster_cert = cert,
Expand Down

0 comments on commit a4e8c6c

Please sign in to comment.