juzhiyuan commented on PR #13707: URL: https://github.com/apache/apisix/pull/13707#issuecomment-5029407171
Thanks for working on this—avoiding an O(N) rebuild on every consumer change is valuable for large deployments. However, I found several consistency issues that should be addressed before merging: - Removing all auth plugins from a consumer is not queued by `filter()`, so the old cached credential can remain valid indefinitely. - Credential IDs are inconsistent: a full rebuild indexes credentials by the leaf `credential_id`, while incremental events use IDs such as `jack/credentials/cred-1`. This can leave an old key valid after rotation, cause collisions across consumers, and trigger repeated full rebuilds in the background check. - Updating a parent consumer does not rebuild its credential-derived entries, so changes to `group_id`, labels, or `custom_id` may never reach requests authenticated through credentials. - The new per-plugin `kv` map has no TTL, so secret-backed credentials may stop refreshing when the external secret changes. - The current Code Lint check also fails because the old `lrucache` variable is now unused. Please add regression coverage for removing auth plugins, updating a credential that existed before bootstrap, duplicate credential leaf IDs under different consumers, parent consumer group/label updates, and secret rotation. Attaching the benchmark harness would also make the performance results independently reproducible. The optimization goal is worthwhile, but authentication correctness needs to be preserved first. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
