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

fix(redis): remove unnecessary redis config deprecation warnings in hybrid mode #13069

Merged
merged 1 commit into from
Jun 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
message: "**ACME**: Fixed an issue of DP reporting that deprecated config fields are used when configuration from CP is pushed"
type: bugfix
scope: Plugin
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
message: "**Response-RateLimiting**: Fixed an issue of DP reporting that deprecated config fields are used when configuration from CP is pushed"
type: bugfix
scope: Plugin
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
message: "**Rate-Limiting**: Fixed an issue of DP reporting that deprecated config fields are used when configuration from CP is pushed"
type: bugfix
scope: Plugin
5 changes: 3 additions & 2 deletions kong/db/schema/init.lua
Original file line number Diff line number Diff line change
Expand Up @@ -887,8 +887,9 @@ function Schema:validate_field(field, value)

if field.deprecation then
local old_default = field.deprecation.old_default
local should_warn = old_default == nil
or not deepcompare(value, old_default)
local should_warn = kong.configuration.role ~= "data_plane" and
(old_default == nil
or not deepcompare(value, old_default))
if should_warn then
deprecation(field.deprecation.message,
{ after = field.deprecation.removal_in_version, })
Expand Down
114 changes: 114 additions & 0 deletions spec/02-integration/09-hybrid_mode/13-deprecations_spec.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
local helpers = require("spec.helpers")
local join = require("pl.stringx").join

local ENABLED_PLUGINS = { "dummy" , "reconfiguration-completion"}

for _, strategy in helpers.each_strategy({"postgres"}) do
describe("deprecations are not reported on DP but on CP", function()
local cp_prefix = "servroot1"
local dp_prefix = "servroot2"
local cp_logfile, dp_logfile, route

lazy_setup(function()
local bp = helpers.get_db_utils(strategy, {
"services",
"routes",
"plugins",
}, ENABLED_PLUGINS)

local service = bp.services:insert {
name = "example",
host = helpers.mock_upstream_host,
port = helpers.mock_upstream_port,
}

route = assert(bp.routes:insert {
hosts = { "mock_upstream" },
protocols = { "http" },
service = service,
})

assert(helpers.start_kong({
role = "control_plane",
database = strategy,
prefix = cp_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_listen = "127.0.0.1:9005",
cluster_telemetry_listen = "127.0.0.1:9006",
plugins = "bundled," .. join(",", ENABLED_PLUGINS),
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",
plugins = "bundled," .. join(",", ENABLED_PLUGINS),
admin_listen = "off",
proxy_listen = "0.0.0.0:9002",
}))
dp_logfile = helpers.get_running_conf(dp_prefix).nginx_err_logs
cp_logfile = helpers.get_running_conf(cp_prefix).nginx_err_logs
end)

lazy_teardown(function()
helpers.stop_kong(dp_prefix)
helpers.stop_kong(cp_prefix)
end)

describe("deprecations are not reported on DP but on CP", function()
before_each(function()
helpers.clean_logfile(dp_logfile)
end)

it("deprecation warnings are only fired on CP not DP", function()
local proxy_client, admin_client = helpers.make_synchronized_clients({
proxy_client = helpers.proxy_client(nil, 9002),
admin_client = helpers.admin_client(nil, 9001)
})
local res = assert(admin_client:send {
method = "POST",
path = "/plugins",
body = {
name = "dummy",
route = { id = route.id },
config = {
old_field = 10,
append_body = "appended from body filtering"
},
},
headers = {
["Content-Type"] = "application/json",
}
})
assert.res_status(201, res)

local res = assert(proxy_client:send {
method = "GET",
path = "/status/200",
headers = {
["Host"] = "mock_upstream",
}
})
local body = assert.res_status(200, res)

-- TEST: ensure that the dummy plugin was executed by checking
-- that the body filtering phase has run

assert.matches("appended from body filtering", body, nil, true)

assert.logfile(cp_logfile).has.line("dummy: old_field is deprecated", true)
assert.logfile(dp_logfile).has.no.line("dummy: old_field is deprecated", true)
end)
end)
end)
end
108 changes: 108 additions & 0 deletions spec/03-plugins/23-rate-limiting/07-hybrid_mode_spec.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
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)
end)
end)
end)
end
Loading