Skip to content

Conversation

squeed
Copy link
Contributor

@squeed squeed commented Jul 4, 2025

This code path has always annoyed me. It has led to bugs before such as #37355. We have a global singleton that needs explicit initialization, always a recipe for errors.

Switch the regex cache to the maintained (and already imported) hashicorp/go-lru package. Initialize the cache in init(), rather than requiring every single test and binary to initialize it.

One nice property of go-lru is that it doesn't initially allocate a buffer of the given size, so we don't need to restrict the size to 1 for one-off CLI programs. The default size is fine.

This allows us to remove the initialization code from lots and lots and lots of unit tests. Hooray.

@squeed squeed requested review from a team as code owners July 4, 2025 10:40
@squeed squeed added the kind/cleanup This includes no functional changes. label Jul 4, 2025
@squeed squeed requested review from a team as code owners July 4, 2025 10:40
@squeed squeed added the release-note/misc This PR makes changes that have no direct user impact. label Jul 4, 2025
@squeed squeed requested review from a team as code owners July 4, 2025 10:40
@github-actions github-actions bot added the sig/policy Impacts whether traffic is allowed or denied based on user-defined policies. label Jul 4, 2025
@squeed squeed requested review from aanm and removed request for nathanjsweet July 4, 2025 10:51
@squeed
Copy link
Contributor Author

squeed commented Jul 4, 2025

/test

Copy link
Member

@tklauser tklauser left a comment

Choose a reason for hiding this comment

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

Very nice improvement!

Some very nit-picky comments inline.

@aanm aanm enabled auto-merge July 4, 2025 11:54
@squeed squeed force-pushed the fqdn-regex-cache branch from d4a7644 to 1145d13 Compare July 4, 2025 12:32
@squeed
Copy link
Contributor Author

squeed commented Jul 4, 2025

/test

Copy link
Member

@giorio94 giorio94 left a comment

Choose a reason for hiding this comment

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

/lgtm for the sig-agent changes.

Switch the regex cache to the maintained (and already imported)
hashicorp/go-lru package. Initialize the cache in `init()`, rather than
requiring every single test and binary to initialize it.

One nice property of go-lru is that it doesn't initially allocate a
buffer of the given size, so we don't need to restrict the size to 1
for one-off CLI programs. The default size is fine.

This allows us to remove the initialization code from lots and lots and
lots of unit tests. Hooray.

Signed-off-by: Casey Callendrello <cdc@isovalent.com>
@squeed squeed force-pushed the fqdn-regex-cache branch from 1145d13 to c4bbd03 Compare July 7, 2025 10:06
@squeed
Copy link
Contributor Author

squeed commented Jul 7, 2025

/test

@aanm aanm added this pull request to the merge queue Jul 7, 2025
@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jul 7, 2025
Merged via the queue into cilium:main with commit 67de01f Jul 7, 2025
68 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/cleanup This includes no functional changes. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact. sig/policy Impacts whether traffic is allowed or denied based on user-defined policies.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants