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

operator: Explicitly init the FQDN regex LRU cache #37366

Merged

Conversation

christarazi
Copy link
Member

In #37355, it is possible that
the LRU is not initialized at the time of the policy validation logic.
Fix this by explicitly calling InitRegexCompileLRU() in the policy
validator cell.

FWIW, during my attempt to locally reproduce, I was not able to.
Inspecting the stacktrace [1] when InitRegexCompileLRU() was called which I
obtained by adding debug statements reveals that the cache is
initialized via init() of the cilium-dbg package. I'm not sure why
there is a mismatch between what I observed and the aforementioned
report, but regardless, it is worth adding an explicit call to
InitRegexCompileLRU() in case the init() is no longer called in the
future.

[1]:

goroutine 1 [running, locked to thread]:
runtime/debug.Stack()
	/usr/local/go/src/runtime/debug/stack.go:26 +0x5e
github.com/cilium/cilium/pkg/fqdn/re.InitRegexCompileLRU(0x1)
	/go/src/github.com/cilium/cilium/pkg/fqdn/re/re.go:57 +0x73
github.com/cilium/cilium/cilium-dbg/cmd.init.119()
	/go/src/github.com/cilium/cilium/cilium-dbg/cmd/policy.go:47 +0x46

Fixes: #37355
Signed-off-by: Chris Tarazi chris@isovalent.com

@christarazi christarazi added kind/bug This is a bug in the Cilium logic. sig/policy Impacts whether traffic is allowed or denied based on user-defined policies. area/operator Impacts the cilium-operator component release-note/misc This PR makes changes that have no direct user impact. sig/agent Cilium agent related. labels Jan 30, 2025
@christarazi christarazi force-pushed the pr/christarazi/init-fqdn-re-operation branch from a9ed50e to 41c83f8 Compare January 30, 2025 19:59
@christarazi christarazi added the needs-backport/1.17 This PR / issue needs backporting to the v1.17 branch label Jan 30, 2025
@christarazi

This comment was marked as outdated.

In cilium#37355, it is possible that
the LRU is not initialized at the time of the policy validation logic.
Fix this by explicitly calling InitRegexCompileLRU() in the policy
validator cell.

FWIW, during my attempt to locally reproduce, I was not able to.
Inspecting the stacktrace [1] when InitRegexCompileLRU() was called which I
obtained by adding debug statements reveals that the cache is
initialized via `init()` of the cilium-dbg package. I'm not sure why
there is a mismatch between what I observed and the aforementioned
report, but regardless, it is worth adding an explicit call to
InitRegexCompileLRU() in case the `init()` is no longer called in the
future.

[1]:

```
goroutine 1 [running, locked to thread]:
runtime/debug.Stack()
	/usr/local/go/src/runtime/debug/stack.go:26 +0x5e
github.com/cilium/cilium/pkg/fqdn/re.InitRegexCompileLRU(0x1)
	/go/src/github.com/cilium/cilium/pkg/fqdn/re/re.go:57 +0x73
github.com/cilium/cilium/cilium-dbg/cmd.init.119()
	/go/src/github.com/cilium/cilium/cilium-dbg/cmd/policy.go:47 +0x46
```

Fixes: cilium#37355
Signed-off-by: Chris Tarazi <chris@isovalent.com>
@christarazi christarazi force-pushed the pr/christarazi/init-fqdn-re-operation branch from 41c83f8 to cece3d5 Compare January 30, 2025 20:30
@christarazi
Copy link
Member Author

/test

@christarazi christarazi marked this pull request as ready for review January 30, 2025 23:05
@christarazi christarazi requested a review from a team as a code owner January 30, 2025 23:05
@christarazi christarazi requested a review from derailed January 30, 2025 23:05
@christarazi christarazi added this pull request to the merge queue Feb 11, 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 Feb 11, 2025
Merged via the queue into cilium:main with commit 8983c4d Feb 11, 2025
69 of 70 checks passed
@christarazi christarazi deleted the pr/christarazi/init-fqdn-re-operation branch February 11, 2025 00:56
@nbusseneau nbusseneau mentioned this pull request Feb 14, 2025
22 tasks
@nbusseneau nbusseneau added backport-pending/1.17 The backport for Cilium 1.17.x for this PR is in progress. and removed needs-backport/1.17 This PR / issue needs backporting to the v1.17 branch labels Feb 14, 2025
@github-actions github-actions bot added backport-done/1.17 The backport for Cilium 1.17.x for this PR is done. and removed backport-pending/1.17 The backport for Cilium 1.17.x for this PR is in progress. labels Feb 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/operator Impacts the cilium-operator component backport-done/1.17 The backport for Cilium 1.17.x for this PR is done. kind/bug This is a bug in the Cilium logic. 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/agent Cilium agent related. 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.

CiliumNetworkPolicy CRD validation marks DNS policies with matchPattern as invalid
3 participants