Skip to content

Remove github.com/gravitational/ttlmap dependency#46899

Merged
rosstimothy merged 12 commits intomasterfrom
joerger/remove-ttl-map
Oct 8, 2024
Merged

Remove github.com/gravitational/ttlmap dependency#46899
rosstimothy merged 12 commits intomasterfrom
joerger/remove-ttl-map

Conversation

@Joerger
Copy link
Copy Markdown
Contributor

@Joerger Joerger commented Sep 24, 2024 •
edited
Loading

Replace github.com/gravitational/ttlmap with utils.FnCache.

@Joerger Joerger added the no-changelog Indicates that a PR does not require a changelog entry label Sep 24, 2024
@Joerger Joerger marked this pull request as ready for review September 24, 2024 23:19
@github-actions github-actions Bot added bpf Used to bugs with bpf and enhanced session recording. kubernetes-access size/sm labels Sep 24, 2024
@zmb3 zmb3 requested a review from fspmarshall September 24, 2024 23:25
fspmarshall
fspmarshall approved these changes Sep 24, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@fspmarshall fspmarshall left a comment

Choose a reason for hiding this comment

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

LGTM once the new ttl cache methods have decent test coverage.

Comment thread lib/utils/fncache.go Outdated
Comment thread lib/kube/proxy/forwarder_test.go Outdated
Comment thread lib/bpf/bpf.go Outdated
@rosstimothy rosstimothy force-pushed the joerger/remove-ttl-map branch from 045eb9b to 33102ce Compare October 4, 2024 14:53
@rosstimothy rosstimothy enabled auto-merge October 4, 2024 15:07
@Joerger Joerger force-pushed the joerger/remove-ttl-map branch 2 times, most recently from 705525f to 6109070 Compare October 4, 2024 20:12
@Joerger
Copy link
Copy Markdown
Contributor Author

Joerger commented Oct 4, 2024

@fspmarshall We had to enabled ReloadOnErr for the app session cache due to potentially transient errors creating a new session when no app servers were matched. It doesn't seem like you intended ReloadOnErr to be used this way, but IMO it does exactly what we need to handle transient errors.

Alternatively we could validate the session each time before checking the session cache in order to avoid recording failed entries - d82d439

WDYT?

@fspmarshall
Copy link
Copy Markdown
Contributor

fspmarshall commented Oct 4, 2024

@Joerger avoiding retention of a transient error sounds like a good use of it to me. I'm all for it.

@rosstimothy rosstimothy force-pushed the joerger/remove-ttl-map branch from 8e6ceab to 89e6906 Compare October 8, 2024 18:43
@rosstimothy rosstimothy force-pushed the joerger/remove-ttl-map branch from 37a22ac to 5b0e772 Compare October 8, 2024 20:27
@rosstimothy rosstimothy added this pull request to the merge queue Oct 8, 2024
Merged via the queue into master with commit d40c0db Oct 8, 2024
@rosstimothy rosstimothy deleted the joerger/remove-ttl-map branch October 8, 2024 22:09
mvbrock pushed a commit that referenced this pull request Oct 16, 2024
@Joerger @rosstimothy @mvbrock
Remove github.com/gravitational/ttlmap dependency (#46899)
d2663a7 
* Replace sessionCache with FnCache.

* Use FnCache in hostCertificate cache.

* Use FnCache for forwarder transport cache.

* Use a normal map for bpf arg collection.

* Remove github.com/gravitational/ttlmap dependency.

* Use FnCache for bfp.

* fix: add test coverage to new FnCache methods

* fix: handle cache error for bpf args

* fix: bring back transport cache ttl const

* fix: restore comments and fix spacing

* fix: tidy integrations/terraform module

* fix: app session cache should reload on transient errors

---------

Co-authored-by: Tim Ross <tim.ross@goteleport.com>
tigrato added a commit that referenced this pull request Jan 30, 2025
@tigrato
kube: fix transport being incorrectly cached by kube service
c64ef7e 
PR #46899 introduced a subtle change in behavior to Kubernetes service
where the direct transport - kube agent to Kubernetes API - is being
cached for 5 hours.

This breaks auto discovery because tokens are valid for 15 min but the
HTTP transport that adds them is valid for 5, leaving the kubernetes
service reusing the same expired token for 4h:45m.

This PR fixes the incorrect behavior by ensuring the token is never
cached.

Fixes #51639

Signed-off-by: Tiago Silva <tiago.silva@goteleport.com>
tigrato added a commit that referenced this pull request Jan 30, 2025
@tigrato
kube: fix transport being incorrectly cached by kube service
9898e9a 
PR #46899 introduced a subtle change in Kubernetes service behavior, where the direct transport—kube agent to Kubernetes API—is now cached for five hours.

This disrupts auto-discovery because tokens are only valid for 15 minutes, while the HTTP transport that applies them remains valid for five hours. As a result, the Kubernetes service continues using the same expired token for 4 hours and 45 minutes.

This PR resolves the issue by preventing token caching altogether.

Fixes #51639

Signed-off-by: Tiago Silva <tiago.silva@goteleport.com>
@tigrato tigrato mentioned this pull request Jan 30, 2025
Merged
tigrato added a commit that referenced this pull request Jan 30, 2025
@tigrato
kube: fix transport being incorrectly cached by kube service
1b87d7e 
PR #46899 introduced a subtle change in Kubernetes service behavior, where the direct transport—kube agent to Kubernetes API—is now cached for five hours.

This disrupts auto-discovery because tokens are only valid for 15 minutes, while the HTTP transport that applies them remains valid for five hours. As a result, the Kubernetes service continues using the same expired token for 4 hours and 45 minutes.

This PR resolves the issue by preventing token caching altogether.

Fixes #51639

Signed-off-by: Tiago Silva <tiago.silva@goteleport.com>
github-merge-queue Bot pushed a commit that referenced this pull request Jan 30, 2025
@tigrato
kube: fix transport being incorrectly cached by kube service (#51640)
0380866 
PR #46899 introduced a subtle change in Kubernetes service behavior, where the direct transport—kube agent to Kubernetes API—is now cached for five hours.

This disrupts auto-discovery because tokens are only valid for 15 minutes, while the HTTP transport that applies them remains valid for five hours. As a result, the Kubernetes service continues using the same expired token for 4 hours and 45 minutes.

This PR resolves the issue by preventing token caching altogether.

Fixes #51639

Signed-off-by: Tiago Silva <tiago.silva@goteleport.com>
github-actions Bot pushed a commit that referenced this pull request Jan 30, 2025
@tigrato
kube: fix transport being incorrectly cached by kube service
2dbb8ea 
PR #46899 introduced a subtle change in Kubernetes service behavior, where the direct transport—kube agent to Kubernetes API—is now cached for five hours.

This disrupts auto-discovery because tokens are only valid for 15 minutes, while the HTTP transport that applies them remains valid for five hours. As a result, the Kubernetes service continues using the same expired token for 4 hours and 45 minutes.

This PR resolves the issue by preventing token caching altogether.

Fixes #51639

Signed-off-by: Tiago Silva <tiago.silva@goteleport.com>
github-merge-queue Bot pushed a commit that referenced this pull request Jan 30, 2025
@tigrato
kube: fix transport being incorrectly cached by kube service (#51652)
c16ef82 
PR #46899 introduced a subtle change in Kubernetes service behavior, where the direct transport—kube agent to Kubernetes API—is now cached for five hours.

This disrupts auto-discovery because tokens are only valid for 15 minutes, while the HTTP transport that applies them remains valid for five hours. As a result, the Kubernetes service continues using the same expired token for 4 hours and 45 minutes.

This PR resolves the issue by preventing token caching altogether.

Fixes #51639

Signed-off-by: Tiago Silva <tiago.silva@goteleport.com>
carloscastrojumo pushed a commit to carloscastrojumo/teleport that referenced this pull request Feb 19, 2025
@tigrato @carloscastrojumo
kube: fix transport being incorrectly cached by kube service (gravita…
6647231 
…tional#51640)

PR gravitational#46899 introduced a subtle change in Kubernetes service behavior, where the direct transport—kube agent to Kubernetes API—is now cached for five hours.

This disrupts auto-discovery because tokens are only valid for 15 minutes, while the HTTP transport that applies them remains valid for five hours. As a result, the Kubernetes service continues using the same expired token for 4 hours and 45 minutes.

This PR resolves the issue by preventing token caching altogether.

Fixes gravitational#51639

Signed-off-by: Tiago Silva <tiago.silva@goteleport.com>
@greedy52 greedy52 mentioned this pull request Mar 20, 2025
Merged
@espadolini espadolini mentioned this pull request Mar 16, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rosstimothy rosstimothy rosstimothy left review comments

@fspmarshall fspmarshall fspmarshall approved these changes

+1 more reviewer

@EdwardDowling EdwardDowling EdwardDowling approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

bpf Used to bugs with bpf and enhanced session recording. kubernetes-access no-changelog Indicates that a PR does not require a changelog entry size/sm

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@Joerger @fspmarshall @EdwardDowling @rosstimothy