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.

Sign up for GitHub

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

feat(response-ratelimiting): add support for secret rotation with redis connection #10570

Merged
merged 1 commit into from
Sep 21, 2023
Merged

feat(response-ratelimiting): add support for secret rotation with redis connection #10570

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 6 additions & 0 deletions CHANGELOG/unreleased/kong/10570.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
message: "**response-ratelimiting**: add support for secret rotation with redis connection "
type: feature
scope: Plugin
prs:
- 10570

8 changes: 6 additions & 2 deletions kong/plugins/response-ratelimiting/policies/init.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,9 +85,13 @@ local function get_redis_connection(conf)
if is_present(conf.redis_password) then
local ok, err
if is_present(conf.redis_username) then
ok, err = red:auth(conf.redis_username, conf.redis_password)
ok, err = kong.vault.try(function(cfg)
return red:auth(cfg.redis_username, cfg.redis_password)
end, conf)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aapo, was the plan not to keep this transparent to the plugin authors and resolve the referenced username/password before calling any plugin handler code?

I'm surprised to see this code here.

Copy link
Member Author

@bungle bungle Apr 19, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hbagdi, this is the resolve-secret-on-failure approach. Most of the cases we have with secrets are resolve-on-secret-failure. But we decided to focus on more generic ttl, but that has limitations. E.g. with try, you can just go to vault and change secret and then change db password, and Kong should be able to pick it up. The TTL approach requires a more coordinated approach. That is when rotating, you should keep the old one at least for TTL amount of time, and after that you may drop the old credentials. The TTL approach can be seen in action here (the update function): 83bd52b

But the TTL we currently only re-resolve secrets only every minute as most:
20c2da8#diff-212b3488c5eb070820f7bc5178fbaa56e9435fe49cf050feb29091ca9aa1540bR49

There is no way to do fail-and-resolve anywhere else than where the failure happens.

else
ok, err = red:auth(conf.redis_password)
ok, err = kong.vault.try(function(cfg)
return red:auth(cfg.redis_password)
end, conf)
end
if not ok then
kong.log.err("failed to auth Redis: ", err)
Expand Down