Skip to content

Commit

Permalink
fix(redis): add default port for standardized redis config
Browse files Browse the repository at this point in the history
The default port should be 6379. This was how RateLimiting
and Response-RateLimiting worked before redis config standardization.

KAG-3618
  • Loading branch information
nowNick authored and jschmid1 committed Jan 29, 2024
1 parent 15aa0e4 commit cb1a3c1
Show file tree
Hide file tree
Showing 7 changed files with 198 additions and 8 deletions.
8 changes: 8 additions & 0 deletions kong/plugins/acme/schema.lua
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,14 @@ local schema = {
then_err = "terms of service must be accepted, see https://letsencrypt.org/repository/",
}
},
{ conditional = {
if_field = "config.storage", if_match = { eq = "redis" },
then_field = "config.storage_config.redis.host", then_match = { required = true },
} },
{ conditional = {
if_field = "config.storage", if_match = { eq = "redis" },
then_field = "config.storage_config.redis.port", then_match = { required = true },
} },
{
custom_entity_check = {
field_sources = { "config.storage", },
Expand Down
7 changes: 2 additions & 5 deletions kong/tools/redis/schema.lua
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ return {
description = "Redis configuration",
fields = {
{ host = typedefs.host },
{ port = typedefs.port },
{ port = typedefs.port({ default = 6379 }), },
{ timeout = typedefs.timeout { default = DEFAULT_TIMEOUT } },
{ username = { description = "Username to use for Redis connections. If undefined, ACL authentication won't be performed. This requires Redis v6.0.0+. To be compatible with Redis v5.x.y, you can set it to `default`.", type = "string",
referenceable = true
Expand All @@ -31,9 +31,6 @@ return {
default = false
} },
{ server_name = typedefs.sni { required = false } }
},
entity_checks = {
{ mutually_required = { "host", "port" }, },
},
}
}
}
106 changes: 106 additions & 0 deletions spec/01-unit/30-standardized_redis_config_spec.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
local schema_def = require "spec.fixtures.custom_plugins.kong.plugins.redis-dummy.schema"
local v = require("spec.helpers").validate_plugin_config_schema


describe("Validate standardized redis config schema", function()
describe("valid config", function()
it("accepts minimal redis config (populates defaults)", function()
local config = {
redis = {
host = "localhost"
}
}
local ok, err = v(config, schema_def)
assert.truthy(ok)
assert.same({
host = "localhost",
port = 6379,
timeout = 2000,
username = ngx.null,
password = ngx.null,
database = 0,
ssl = false,
ssl_verify = false,
server_name = ngx.null,
}, ok.config.redis)
assert.is_nil(err)
end)

it("full redis config", function()
local config = {
redis = {
host = "localhost",
port = 9900,
timeout = 3333,
username = "test",
password = "testXXX",
database = 5,
ssl = true,
ssl_verify = true,
server_name = "example.test"
}
}
local ok, err = v(config, schema_def)
assert.truthy(ok)
assert.same(config.redis, ok.config.redis)
assert.is_nil(err)
end)

it("allows empty strings on password", function()
local config = {
redis = {
host = "localhost",
password = "",
}
}
local ok, err = v(config, schema_def)
assert.truthy(ok)
assert.same({
host = "localhost",
port = 6379,
timeout = 2000,
username = ngx.null,
password = "",
database = 0,
ssl = false,
ssl_verify = false,
server_name = ngx.null,
}, ok.config.redis)
assert.is_nil(err)
end)
end)

describe("invalid config", function()
it("rejects invalid config", function()
local config = {
redis = {
host = "",
port = -5,
timeout = -5,
username = 1,
password = 4,
database = "abc",
ssl = "abc",
ssl_verify = "xyz",
server_name = "test-test"
}
}
local ok, err = v(config, schema_def)
assert.falsy(ok)
assert.same({
config = {
redis = {
database = 'expected an integer',
host = 'length must be at least 1',
password = 'expected a string',
port = 'value should be between 0 and 65535',
ssl = 'expected a boolean',
ssl_verify = 'expected a boolean',
timeout = 'value should be between 0 and 2147483646',
username = 'expected a string',
}
}
}, err)
end)
end)
end)
24 changes: 21 additions & 3 deletions spec/02-integration/02-cmd/11-config_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,12 @@ describe("kong config", function()
config:
port: 10000
host: 127.0.0.1
- name: rate-limiting
config:
minute: 200
policy: redis
redis:
host: 127.0.0.1
plugins:
- name: correlation-id
id: 467f719f-a544-4a8f-bc4b-7cd12913a9d4
Expand Down Expand Up @@ -130,7 +136,7 @@ describe("kong config", function()
local res = client:get("/services/bar/plugins")
local body = assert.res_status(200, res)
local json = cjson.decode(body)
assert.equals(2, #json.data)
assert.equals(3, #json.data)

local res = client:get("/plugins/467f719f-a544-4a8f-bc4b-7cd12913a9d4")
local body = assert.res_status(200, res)
Expand Down Expand Up @@ -532,7 +538,17 @@ describe("kong config", function()

local service2 = bp.services:insert({ name = "service2" }, { nulls = true })
local route2 = bp.routes:insert({ service = service2, methods = { "GET" }, name = "b" }, { nulls = true })
local plugin3 = bp.tcp_log_plugins:insert({
local plugin3 = bp.rate_limiting_plugins:insert({
service = service2,
config = {
minute = 100,
policy = "redis",
redis = {
host = "localhost"
}
}
}, { nulls = true })
local plugin4 = bp.tcp_log_plugins:insert({
service = service2,
}, { nulls = true })
local consumer = bp.consumers:insert(nil, { nulls = true })
Expand Down Expand Up @@ -603,7 +619,7 @@ describe("kong config", function()
assert.equals(route2.name, yaml.routes[2].name)
assert.equals(service2.id, yaml.routes[2].service)

assert.equals(3, #yaml.plugins)
assert.equals(4, #yaml.plugins)
table.sort(yaml.plugins, sort_by_name)
assert.equals(plugin1.id, yaml.plugins[1].id)
assert.equals(plugin1.name, yaml.plugins[1].name)
Expand All @@ -615,6 +631,8 @@ describe("kong config", function()

assert.equals(plugin3.id, yaml.plugins[3].id)
assert.equals(plugin3.name, yaml.plugins[3].name)
assert.equals(plugin4.id, yaml.plugins[4].id)
assert.equals(plugin4.name, yaml.plugins[4].name)
assert.equals(service2.id, yaml.plugins[3].service)

assert.equals(1, #yaml.consumers)
Expand Down
34 changes: 34 additions & 0 deletions spec/03-plugins/29-acme/04-schema_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,40 @@ describe("Plugin: acme (schema)", function()
}
},
},
----------------------------------------
{
name = "accepts valid redis config",
input = {
account_email = "[email protected]",
storage = "redis",
storage_config = {
redis = {
host = "localhost"
},
}
},
},
----------------------------------------
{
name = "rejects invalid redis config",
input = {
account_email = "[email protected]",
storage = "redis",
storage_config = {
redis = { },
}
},
error = {
["@entity"] = { "failed conditional validation given value of field 'config.storage'" },
config = {
storage_config = {
redis = {
host = "required field missing",
}
}
},
},
},
}

for _, t in ipairs(tests) do
Expand Down
12 changes: 12 additions & 0 deletions spec/fixtures/custom_plugins/kong/plugins/redis-dummy/handler.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
local kong = kong

local RedisDummy = {
PRIORITY = 1000,
VERSION = "0.1.0",
}

function RedisDummy:access(conf)
kong.log("access phase")
end

return RedisDummy
15 changes: 15 additions & 0 deletions spec/fixtures/custom_plugins/kong/plugins/redis-dummy/schema.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
local redis_schema = require "kong.tools.redis.schema"

return {
name = "redis-dummy",
fields = {
{
config = {
type = "record",
fields = {
{ redis = redis_schema.config_schema },
},
},
},
},
}

0 comments on commit cb1a3c1

Please sign in to comment.