-
Notifications
You must be signed in to change notification settings - Fork 4.8k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix(logs): misleading acme deprecation logs in hybrid mode
When running kong in hybrid mode a plugin configuration is pushed from CP to DP. When introducing shorthand field expansion to Admin responses the deprecated fields (defined as shorthand fields) were also pushed from CP to DP which resulted in DP receiving config with both new fields and also old fields. It resulted with DP reporting in logs that the plugin is being configured with deprecated fields. It affected the plugins: - ACME - Rate-Limiting - Response-RateLimiting KAG-4515
- Loading branch information
Showing
6 changed files
with
239 additions
and
29 deletions.
There are no files selected for viewing
3 changes: 3 additions & 0 deletions
3
changelog/unreleased/kong/fix-acme-misleading-deprecation-logs.yml
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
message: "**ACME**: Fix an issue of DP reporting that deprecated config fields are used when configuration from DP is pushed" | ||
type: bugfix | ||
scope: Plugin |
3 changes: 3 additions & 0 deletions
3
changelog/unreleased/kong/fix-response-rl-misleading-deprecation-logs.yml
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
message: "**Response-RateLimiting**: Fix an issue of DP reporting that deprecated config fields are used when configuration from DP is pushed" | ||
type: bugfix | ||
scope: Plugin |
3 changes: 3 additions & 0 deletions
3
changelog/unreleased/kong/fix-rl-misleading-deprecation-logs.yml
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
message: "**Rate-Limiting**: Fix an issue of DP reporting that deprecated config fields are used when configuration from DP is pushed" | ||
type: bugfix | ||
scope: Plugin |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
114 changes: 114 additions & 0 deletions
114
spec/03-plugins/23-rate-limiting/07-hybrid_mode_spec.lua
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,114 @@ | ||
local helpers = require "spec.helpers" | ||
|
||
local REDIS_HOST = helpers.redis_host | ||
local REDIS_PORT = helpers.redis_port | ||
|
||
for _, strategy in helpers.each_strategy({"postgres"}) do | ||
describe("Plugin: rate-limiting (handler.access) worked with [#" .. strategy .. "]", function() | ||
local dp_prefix = "servroot2" | ||
local dp_logfile, bp, db, route | ||
|
||
lazy_setup(function() | ||
bp, db = helpers.get_db_utils(strategy, { | ||
"services", | ||
"routes", | ||
"plugins", | ||
}, { "rate-limiting", }) | ||
|
||
local service = assert(bp.services:insert { | ||
host = helpers.mock_upstream_host, | ||
port = helpers.mock_upstream_port, | ||
}) | ||
|
||
route = assert(bp.routes:insert { | ||
paths = { "/rate-limit-test" }, | ||
service = service | ||
}) | ||
|
||
assert(helpers.start_kong({ | ||
role = "control_plane", | ||
database = strategy, | ||
cluster_cert = "spec/fixtures/kong_clustering.crt", | ||
cluster_cert_key = "spec/fixtures/kong_clustering.key", | ||
lua_ssl_trusted_certificate = "spec/fixtures/kong_clustering.crt", | ||
cluster_listen = "127.0.0.1:9005", | ||
cluster_telemetry_listen = "127.0.0.1:9006", | ||
nginx_conf = "spec/fixtures/custom_nginx.template", | ||
admin_listen = "0.0.0.0:9001", | ||
proxy_listen = "off", | ||
})) | ||
|
||
assert(helpers.start_kong({ | ||
role = "data_plane", | ||
database = "off", | ||
prefix = dp_prefix, | ||
cluster_cert = "spec/fixtures/kong_clustering.crt", | ||
cluster_cert_key = "spec/fixtures/kong_clustering.key", | ||
lua_ssl_trusted_certificate = "spec/fixtures/kong_clustering.crt", | ||
cluster_control_plane = "127.0.0.1:9005", | ||
cluster_telemetry_endpoint = "127.0.0.1:9006", | ||
admin_listen = "off", | ||
proxy_listen = "0.0.0.0:9002", | ||
})) | ||
dp_logfile = helpers.get_running_conf(dp_prefix).nginx_err_logs | ||
end) | ||
|
||
lazy_teardown(function() | ||
helpers.stop_kong("servroot2") | ||
helpers.stop_kong() | ||
end) | ||
|
||
describe("\"redis\" storage mode in Hybrid mode", function() | ||
lazy_setup(function () | ||
local admin_client = helpers.admin_client(nil, 9001) | ||
local res = assert(admin_client:send { | ||
method = "POST", | ||
path = "/plugins", | ||
body = { | ||
name = "rate-limiting", | ||
route = { | ||
id = route.id | ||
}, | ||
config = { | ||
minute = 2, | ||
policy = "redis", | ||
redis = { | ||
host = REDIS_HOST, | ||
port = REDIS_PORT, | ||
} | ||
}, | ||
}, | ||
headers = { | ||
["Content-Type"] = "application/json", | ||
} | ||
}) | ||
assert.res_status(201, res) | ||
admin_client:close() | ||
end) | ||
|
||
lazy_teardown(function () | ||
db:truncate("plugins") | ||
end) | ||
|
||
before_each(function() | ||
helpers.clean_logfile(dp_logfile) | ||
end) | ||
|
||
it("sanity test - check if old fields are not pushed & visible in logs as deprecation warnings", function() | ||
helpers.wait_until(function() | ||
local proxy_client = helpers.proxy_client(nil, 9002) | ||
local res = assert(proxy_client:get("/rate-limit-test")) | ||
proxy_client:close() | ||
|
||
return res.status == 429 | ||
end, 10, 1) | ||
|
||
assert.logfile(dp_logfile).has.no.line( | ||
"rate-limiting: config.redis_host is deprecated", -- one of the deprecated old config lines | ||
true, | ||
10 | ||
) | ||
end) | ||
end) | ||
end) | ||
end |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,9 +3,11 @@ local helpers = require "spec.helpers" | |
for _, strategy in helpers.each_strategy({"postgres"}) do | ||
describe("Plugin: acme (handler.access) worked with [#" .. strategy .. "]", function() | ||
local domain = "mydomain.test" | ||
local dp_prefix = "servroot2" | ||
local dp_logfile, bp, db | ||
|
||
lazy_setup(function() | ||
local bp = helpers.get_db_utils(strategy, { | ||
bp, db = helpers.get_db_utils(strategy, { | ||
"services", | ||
"routes", | ||
"plugins", | ||
|
@@ -15,19 +17,6 @@ for _, strategy in helpers.each_strategy({"postgres"}) do | |
paths = { "/" }, | ||
}) | ||
|
||
assert(bp.plugins:insert { | ||
name = "acme", | ||
config = { | ||
account_email = "[email protected]", | ||
api_uri = "https://api.acme.org", | ||
domains = { domain }, | ||
storage = "kong", | ||
storage_config = { | ||
kong = {}, | ||
}, | ||
}, | ||
}) | ||
|
||
assert(helpers.start_kong({ | ||
role = "control_plane", | ||
database = strategy, | ||
|
@@ -37,43 +26,140 @@ for _, strategy in helpers.each_strategy({"postgres"}) do | |
cluster_listen = "127.0.0.1:9005", | ||
cluster_telemetry_listen = "127.0.0.1:9006", | ||
nginx_conf = "spec/fixtures/custom_nginx.template", | ||
admin_listen = "0.0.0.0:9001", | ||
proxy_listen = "off", | ||
})) | ||
|
||
assert(helpers.start_kong({ | ||
role = "data_plane", | ||
database = "off", | ||
prefix = "servroot2", | ||
prefix = dp_prefix, | ||
cluster_cert = "spec/fixtures/kong_clustering.crt", | ||
cluster_cert_key = "spec/fixtures/kong_clustering.key", | ||
lua_ssl_trusted_certificate = "spec/fixtures/kong_clustering.crt", | ||
cluster_control_plane = "127.0.0.1:9005", | ||
cluster_telemetry_endpoint = "127.0.0.1:9006", | ||
admin_listen = "off", | ||
proxy_listen = "0.0.0.0:9002", | ||
})) | ||
dp_logfile = helpers.get_running_conf(dp_prefix).nginx_err_logs | ||
end) | ||
|
||
lazy_teardown(function() | ||
helpers.stop_kong("servroot2") | ||
helpers.stop_kong() | ||
end) | ||
|
||
it("sanity test works with \"kong\" storage in Hybrid mode", function() | ||
local proxy_client = helpers.http_client("127.0.0.1", 9002) | ||
helpers.wait_until(function() | ||
local res = assert(proxy_client:send { | ||
method = "GET", | ||
path = "/.well-known/acme-challenge/x", | ||
headers = { host = domain } | ||
describe("\"kong\" storage mode in Hybrid mode", function() | ||
lazy_setup(function () | ||
local admin_client = helpers.admin_client(nil, 9001) | ||
local res = assert(admin_client:send { | ||
method = "POST", | ||
path = "/plugins", | ||
body = { | ||
name = "acme", | ||
config = { | ||
account_email = "[email protected]", | ||
api_uri = "https://api.acme.org", | ||
domains = { domain }, | ||
storage = "kong", | ||
storage_config = { | ||
kong = {}, | ||
}, | ||
}, | ||
}, | ||
headers = { | ||
["Content-Type"] = "application/json", | ||
} | ||
}) | ||
assert.res_status(201, res) | ||
admin_client:close() | ||
end) | ||
|
||
lazy_teardown(function () | ||
db:truncate("plugins") | ||
end) | ||
|
||
it("sanity test", function() | ||
local proxy_client = helpers.http_client("127.0.0.1", 9002) | ||
helpers.wait_until(function() | ||
local res = assert(proxy_client:send { | ||
method = "GET", | ||
path = "/.well-known/acme-challenge/x", | ||
headers = { host = domain } | ||
}) | ||
|
||
if res.status ~= 404 then | ||
return false | ||
end | ||
|
||
local body = res:read_body() | ||
return body == "Not found\n" | ||
end, 10) | ||
proxy_client:close() | ||
end) | ||
end) | ||
|
||
describe("\"redis\" storage mode in Hybrid mode", function() | ||
lazy_setup(function () | ||
local admin_client = helpers.admin_client(nil, 9001) | ||
local res = assert(admin_client:send { | ||
method = "POST", | ||
path = "/plugins", | ||
body = { | ||
name = "acme", | ||
config = { | ||
account_email = "[email protected]", | ||
api_uri = "https://api.acme.org", | ||
domains = { domain }, | ||
storage = "kong", | ||
storage_config = { | ||
redis = { | ||
host = helpers.redis_host, | ||
port = helpers.redis_port, | ||
}, | ||
}, | ||
}, | ||
}, | ||
headers = { | ||
["Content-Type"] = "application/json", | ||
} | ||
}) | ||
assert.res_status(201, res) | ||
admin_client:close() | ||
end) | ||
|
||
lazy_teardown(function () | ||
db:truncate("plugins") | ||
end) | ||
|
||
before_each(function() | ||
helpers.clean_logfile(dp_logfile) | ||
end) | ||
|
||
it("sanity test", function() | ||
local proxy_client = helpers.http_client("127.0.0.1", 9002) | ||
helpers.wait_until(function() | ||
local res = assert(proxy_client:send { | ||
method = "GET", | ||
path = "/.well-known/acme-challenge/x", | ||
headers = { host = domain } | ||
}) | ||
|
||
if res.status ~= 404 then | ||
return false | ||
end | ||
if res.status ~= 404 then | ||
return false | ||
end | ||
|
||
local body = res:read_body() | ||
return body == "Not found\n" | ||
end, 10) | ||
proxy_client:close() | ||
local body = res:read_body() | ||
return body == "Not found\n" | ||
end, 10) | ||
assert.logfile(dp_logfile).has.no.line( | ||
"config%.storage_config%.redis%.(namespace|scan_count) is deprecated", | ||
false, | ||
10 | ||
) | ||
proxy_client:close() | ||
end) | ||
end) | ||
end) | ||
end |