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(health-checker): get certificate with upstream.ws_id #13882

Merged
merged 2 commits into from
Nov 18, 2024

Conversation

Oyami-Srk
Copy link
Member

Summary

This is to avoid calling get_certificatewith workspace id from ngx.ctx.workspace which is volatile when in a timer callback.

Checklist

Issue reference

Fix KAG-5821

@github-actions github-actions bot added core/balancer core/proxy cherry-pick kong-ee schedule this PR for cherry-picking to kong/kong-ee labels Nov 18, 2024
@Oyami-Srk Oyami-Srk removed the cherry-pick kong-ee schedule this PR for cherry-picking to kong/kong-ee label Nov 18, 2024
@Oyami-Srk Oyami-Srk added the cherry-pick kong-ee schedule this PR for cherry-picking to kong/kong-ee label Nov 18, 2024
@chobits chobits requested a review from oowl November 18, 2024 03:06
@Oyami-Srk Oyami-Srk force-pushed the haoxuan/fix-healthcheck branch from e107aa3 to 9b4c224 Compare November 18, 2024 03:07
Copy link
Contributor

@chobits chobits left a comment

Choose a reason for hiding this comment

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

Need a changelog entry for this PR, its a bugfix for balancer system

@chobits
Copy link
Contributor

chobits commented Nov 18, 2024

From this fix, I guess there might be similar bugs for other parts of the code that call select(pk) without appending ws_id. Therefore I searched the source code:

$ grep 'select(pk' -R kong -n
kong/runloop/certificate.lua:192:  local certificate, err = kong.db.certificates:select(pk)
kong/db/dao/snis.lua:133:  local _, err, err_t = invalidate_cache(self, self:select(pk))
kong/db/dao/snis.lua:155:  local _, err, err_t = invalidate_cache(self, self:select(pk))

We need to add another internal ticket(KAG) to track and verify whether calling select(pk) without ws_id also causes similar problems.

@Oyami-Srk
Copy link
Member Author

@chobits Every DAO operations for workspace-able entities without ws_id in timer context are dangerous. Not only the select.

@Oyami-Srk Oyami-Srk requested a review from chobits November 18, 2024 03:13
@chobits
Copy link
Contributor

chobits commented Nov 18, 2024

@chobits Every DAO operations for workspace-able entities without ws_id in timer context are dangerous. Not only the select.

yep, I'll file a ticket to track this.

updated: the internal ticket https://konghq.atlassian.net/browse/KAG-5842

@Oyami-Srk Oyami-Srk force-pushed the haoxuan/fix-healthcheck branch 2 times, most recently from 01b6ad3 to baea106 Compare November 18, 2024 06:27
@Oyami-Srk Oyami-Srk force-pushed the haoxuan/fix-healthcheck branch from baea106 to 08f114f Compare November 18, 2024 06:35
@ADD-SP ADD-SP merged commit e862d9c into master Nov 18, 2024
26 checks passed
@ADD-SP ADD-SP deleted the haoxuan/fix-healthcheck branch November 18, 2024 06:54
@team-gateway-bot
Copy link
Collaborator

Successfully created cherry-pick PR for master:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-pick kong-ee schedule this PR for cherry-picking to kong/kong-ee core/balancer core/proxy size/S
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants