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

api: provide new merge option: upsert #16483

Closed
Dentrax opened this issue Jul 28, 2022 · 1 comment
Closed

api: provide new merge option: upsert #16483

Dentrax opened this issue Jul 28, 2022 · 1 comment
Labels
core/api devex Developer Experience

Comments

@Dentrax
Copy link

Dentrax commented Jul 28, 2022

Is your feature request related to a problem? Please describe.

There should be a new MergeMethod that should put the secret if does not exist, otherwise, it should merge the secret by using patch method like as is.

foo, err := client.KVv2("foo").Patch(context.Background(), "bar", data)
if err != nil {
  // in case an error fallback to the patch way - key does not exist!
  foo, err = client.KVv2("foo").Put(context.Background(), "bar", data)
  if err != nil {
    log.Printf("could not write to Vault: %s: %v\n", variable.Key, err)
     
  }
}

I've tried to pass KVMergeMethodPatch and cas=0 but doesn't solve the problem:

client.KVv2("foo").Patch(context.Background(), "bar", data, vault.WithMergeMethod(vault.KVMergeMethodPatch), vault.WithCheckAndSet(0))

Describe the solution you'd like

Provide a new MergeMethod:

KVMergeMethodUpsert     = "upsert"

Eventually, I would write the following instead:

client.KVv2("foo").Patch(context.Background(), "bar", data, vault.WithMergeMethod(vault.KVMergeMethodUpsert)

Describe alternatives you've considered

-

Explain any additional use-cases
-

Additional context
-

@averche
Copy link
Contributor

averche commented Aug 13, 2022

Hi @Dentrax!

We've discussed this request internally and we think that adding an "upsert" option to the Patch method could lead to potentially non-standard UX. The Patch method internally translates into a PATCH request, which traditionally only modifies an existing resource and returns a 404 error if the resource is not found. Adding the "upsert" functionality would make Patch function as a mixture of PATCH and PUT.

The code example you posted above is a good way to achieve the upsert behaviour. I've added a sentinel error in #16699 to make the workflow a bit more clear:

_, err := client.KVv2("foo").Patch(ctx, "bar", data)
if errors.Is(err, vault.ErrSecretNotFound) {
    _, err = client.KVv2("foo").Put(ctx, "bar", data)
    if err != nil {
        // handle err
    }
} else if err != nil {
    // something else was wrong with the patch request
}

Hope this helps and thank you for creating the issue!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core/api devex Developer Experience
Projects
None yet
Development

No branches or pull requests

3 participants