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.

Sign up for GitHub

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

Jump to bottom

Entrypoint cache should expire items #332

Closed
imjasonh opened this issue Dec 14, 2018 · 2 comments
Closed

Entrypoint cache should expire items #332

imjasonh opened this issue Dec 14, 2018 · 2 comments
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. meaty-juicy-coding-work This task is mostly about implementation!!! And docs and tests of course but that's a given

Comments

@imjasonh
Copy link
Member

When we lookup entrypoints for remote images, we cache it for later lookups. Items are never removed from the cache, so if the controller runs for long enough and sees enough unique builder images, it will consume more and more memory. This could present a risk of DoS, since a malicious user can craft valid build requests that fill the cache, especially with large values for entrypoints.

We should use a LRU cache (here's a popular one in Go) that will only keep the most recently-used entrypoints cached and drop entrypoints we haven't used recently.
@imjasonh imjasonh added good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. meaty-juicy-coding-work This task is mostly about implementation!!! And docs and tests of course but that's a given labels Dec 14, 2018
@mathspanda mathspanda mentioned this issue Dec 26, 2018
Merged
knative-prow-robot pushed a commit that referenced this issue Jan 15, 2019
@mathspanda @knative-prow-robot
Use lru cache to expire entrypoint items(#332)
8e06fa0 
Use a lru cache that will only keep the most recently-used entrypoints
cached and drop entrypoints haven't been used recently, in order to
present a risk of DoS.

1. replace internal map in original Cache with lru.Cache
2. fix incorrect entrypoint_test package name

Signed-off-by: mathspanda <[email protected]>
@afrittoli
Copy link
Member

Since #350 is merged, I think this could be closed?

I added #418 as a follow-up, the deps update pops up when running ./hack/update-deps.sh

@bobcatfish
Copy link
Collaborator

Since #350 is merged, I think this could be closed?

Makes sense, thanks for the vigilance @afrittoli !!

pradeepitm12 pushed a commit to pradeepitm12/pipeline that referenced this issue Jan 28, 2021
@dibyom @tekton-robot
Switch API group for triggers to triggers.tekton.dev
ea6c193 
BREAKING CHANGE: Users will have to delete, and recreate
resources after updating its API group.

Fixes tektoncd#332

Signed-off-by: Dibyo Mukherjee <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. meaty-juicy-coding-work This task is mostly about implementation!!! And docs and tests of course but that's a given
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@imjasonh @bobcatfish @afrittoli