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

CLI Secrets management: Re-consider upsert logic #4936

Open
theduke opened this issue Jul 16, 2024 · 3 comments
Open

CLI Secrets management: Re-consider upsert logic #4936

theduke opened this issue Jul 16, 2024 · 3 comments
Assignees

Comments

@theduke
Copy link
Contributor

theduke commented Jul 16, 2024

Should this kind of logic really be in the CLI?
That makes it hard to update.

This also opens the door to race conditions, so it's not actually a guarantee that create actually creates.

I feel like the backend should handle this.
Eg with a "createNew" argument to the mutation.

Originally posted by @theduke in #4930 (comment)

@theduke
Copy link
Contributor Author

theduke commented Jul 16, 2024

@xdoardo @ayys

@syrusakbary
Copy link
Member

This also opens the door to race conditions

What race conditions? I see upsert being less prone to race conditions than the other one (basically because it will not fail if it already exists while the other strategy will require always two checks - and doesn't guarantee that other client doesn't do an operation in between). But perhaps I'm missing something

@theduke
Copy link
Contributor Author

theduke commented Jul 17, 2024

@syrusakbary

So the current implementation of the "secret create" command does a check if a secret with the same name already exists. Doing that in CLI is only a best effort sanity check, it's not a guarantee of any kind. Only the backend can ensure that it is actually a "create" operation, not an update. Currently if someone else were to create a secret in the meantime the previous value would be overwritten due to the upsert.

Supporting a "create new" operation makes sense, because it prevents overwriting a secret, but if we want that to actually have guaranteed semantics that has to be done in the backend.

It's a marginal concern, because secrets will be updated rarely. It's just a question of if we actually want to ensure the behaviour.

A side concern is that putting such logic in the CLI makes it hard to update. The backend can be modified at any time without requiring users to update the CLI.

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

No branches or pull requests

3 participants