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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support read-only mode to prevent unwanted cache update #199

Open
dvcolomban opened this issue Sep 13, 2023 · 9 comments
Open

Support read-only mode to prevent unwanted cache update #199

dvcolomban opened this issue Sep 13, 2023 · 9 comments
Labels
enhancement New feature or request

Comments

@dvcolomban
Copy link

dvcolomban commented Sep 13, 2023

馃殌 Feature Proposal

Support a read-only mode that allows cache read from clients but doesn't contribute new cache entries.

Motivation

In the context of a mono-repo ci integration, we would like to share ci cache artefacts with local machines, but we don't want users to be able to contribute new values.

The purpose is to speed up local work without risking contamination of the ci and cd artefacts from potentially broken or unsafe local clients.

Example

With such a feature, we would be able to generate cache read/write from the ci, where we can guarantee state and monitoring with a normal instance.

At the same time, another instance of the remote repo could allow local users to consume the same cache entry (backed by s3 storage in our case) without risking broken builds.

I've looked at the official turbo documentation, and it seems they don't natively offer such a feature.
The closest to it, are some cache management flags that allow to prevent cache uploads, but those are easily circumvented and necessite to split ci and dev scripts.

An open issue on turbo addresses similar needs but seems that nothing much came of it yet.

Do you have any plan to implement this ? And how it could be done ?

We're ready to contribute to a PR implementing this if you're open to it :)

@fox1t fox1t added the enhancement New feature or request label Sep 15, 2023
@fox1t
Copy link
Collaborator

fox1t commented Sep 15, 2023

I love this idea! We can add a deployment flag that will return a 401 to clients on PUT but will work on HEAD and GET! What do you think?

@matteovivona FYI.

@dvcolomban
Copy link
Author

I read that an issue with returning non 2xx return codes to clients can lead turbo to abandon remote caching and only relying on local cache (see this comment).

If we want to keep this working we would need to return an ok response.
Maybe a 202 to distinguish from non intercepted calls ?

@dvcolomban
Copy link
Author

If I'm not mistaken, any PUT filtering would have to be done here right?

Could we also use a URL whitelist instead of a separate read-only instance if we want only to allow PUT from trusted ci workflow ?

@fox1t
Copy link
Collaborator

fox1t commented Oct 13, 2023

Hey, that is the only part of the codebase that touches PUTs.
I think that having a whitelist is a great idea! You can add an onRequest hook to that route definition.

@matteovivona matteovivona linked a pull request Nov 14, 2023 that will close this issue
7 tasks
@anthonyshew
Copy link

anthonyshew commented Dec 6, 2023

Going to close this as a duplicate of #1188 since it has more reactions. If you'd like to signal boost with reactions/continue discussion, please do so there!

@NullVoxPopuli
Copy link

How do you configure it tho? do all devs still share the same secret-key for access? (this is dangerous)

@fox1t
Copy link
Collaborator

fox1t commented Feb 2, 2024

How do you configure it tho? do all devs still share the same secret-key for access? (this is dangerous)

Hey @NullVoxPopuli, can you elaborate? I am not sure I get it.

@NullVoxPopuli
Copy link

NullVoxPopuli commented Feb 2, 2024

so, in order to contact a cache server, turbo needs a TURBO_TOKEN to authenticate with the server.

Exposing that to all employees gives folks the chance to try to poison the cache, or manually upload invalid caches for a particular cache-hash.

But if we keep the TURBO_TOKEN secret (only for CI), and then somehow have employees authenticate in some other way (in read-only mode), the risk is mitigated - disgruntled employees can no longer try to poison the cache(s).

@fox1t
Copy link
Collaborator

fox1t commented Feb 5, 2024

We might add SQLite as a persistence layer and save it to the same storage where the cache is saved. At that point, we can start exposing an "admin" interface.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants