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

Add HTTP PATCH support to KV #12687

Merged
merged 35 commits into from Oct 13, 2021
Merged
Show file tree
Hide file tree
Changes from 34 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
e64bfec
handle HTTP PATCH requests as logical.PatchOperation
ccapurso Sep 20, 2021
11eaec8
update go.mod, go.sum
ccapurso Sep 20, 2021
10ebff6
a nil response for logical.PatchOperation should result in 404
ccapurso Sep 20, 2021
0d6fab2
respond with 415 for incorrect MIME type in PATCH Content-Type header
ccapurso Sep 20, 2021
1ab8d4c
add abstraction to handle PatchOperation requests
ccapurso Sep 20, 2021
dc649aa
add ACLs for patch
raskchanky Sep 20, 2021
8607481
Merge remote-tracking branch 'origin/kv-patch' into kv-patch
raskchanky Sep 20, 2021
6257873
Adding JSON Merge support to the API client
raskchanky Sep 21, 2021
8bd064d
add HTTP PATCH tests to check high level response logic
ccapurso Sep 22, 2021
8bab71d
add permission-based 'kv patch' tests in prep to add HTTP PATCH
ccapurso Sep 23, 2021
67f7a60
adding more 'kv patch' CLI command tests
ccapurso Sep 24, 2021
ecb1efe
fix TestHandler_Patch_NotFound
ccapurso Sep 27, 2021
0bfec35
Fix TestKvPatchCommand_StdinValue
ccapurso Sep 27, 2021
6b9aeca
add audit log test for HTTP PATCH
ccapurso Sep 27, 2021
4f9b112
patch CLI changes
raskchanky Sep 28, 2021
0dd646a
Merge remote-tracking branch 'origin/kv-patch' into kv-patch
raskchanky Sep 28, 2021
aeeff0e
add patch CLI tests
raskchanky Sep 29, 2021
af0e962
change JSONMergePatch func to accept a ctx
ccapurso Sep 30, 2021
07ae4ea
fix TestKVPatchCommand_RWMethodNotExists and TestKVPatchCommand_RWMet…
ccapurso Sep 30, 2021
d18f9e3
go fmt
ccapurso Sep 30, 2021
1cd8bf4
add a test to verify patching works by default with the root token
raskchanky Oct 1, 2021
a8c7e13
add changelog entry
ccapurso Oct 4, 2021
723fb6b
get vault-plugin-secrets-kv@add-patch-support
ccapurso Oct 7, 2021
f309ea8
PR feedback
raskchanky Oct 7, 2021
eb00941
Merge remote-tracking branch 'origin/kv-patch' into kv-patch
raskchanky Oct 7, 2021
de33fd6
reorder some imports; go fmt
ccapurso Oct 7, 2021
2fe0402
add doc comment for HandlePatchOperation
ccapurso Oct 7, 2021
90c5725
add json-patch@v5.5.0 to go.mod
ccapurso Oct 7, 2021
5d04fff
remove unnecessary cancelFunc for WriteBytes
ccapurso Oct 7, 2021
29b9d69
remove default for -method
raskchanky Oct 7, 2021
8f908d5
Merge remote-tracking branch 'origin/kv-patch' into kv-patch
raskchanky Oct 7, 2021
16135ef
use stable version of json-patch; go mod tidy
ccapurso Oct 7, 2021
6fcc723
more PR feedback
raskchanky Oct 8, 2021
802fd02
Merge branch 'main' into kv-patch
ccapurso Oct 11, 2021
aee77e3
temp go get vault-plugin-secrets-kv@master until official release
ccapurso Oct 13, 2021
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 1 addition & 2 deletions api/api_test.go
Expand Up @@ -9,8 +9,7 @@ import (

// testHTTPServer creates a test HTTP server that handles requests until
// the listener returned is closed.
func testHTTPServer(
t *testing.T, handler http.Handler) (*Config, net.Listener) {
func testHTTPServer(t *testing.T, handler http.Handler) (*Config, net.Listener) {
ln, err := net.Listen("tcp", "127.0.0.1:0")
if err != nil {
t.Fatalf("err: %s", err)
Expand Down
24 changes: 19 additions & 5 deletions api/logical.go
Expand Up @@ -5,6 +5,7 @@ import (
"context"
"fmt"
"io"
"net/http"
"net/url"
"os"

Expand Down Expand Up @@ -130,24 +131,37 @@ func (c *Logical) List(path string) (*Secret, error) {
}

func (c *Logical) Write(path string, data map[string]interface{}) (*Secret, error) {
ctx, cancelFunc := context.WithCancel(context.Background())
defer cancelFunc()

r := c.c.NewRequest("PUT", "/v1/"+path)
if err := r.SetJSONBody(data); err != nil {
return nil, err
}

return c.write(path, r)
return c.write(ctx, path, r)
}

func (c *Logical) JSONMergePatch(ctx context.Context, path string, data map[string]interface{}) (*Secret, error) {
pmmukh marked this conversation as resolved.
Show resolved Hide resolved
r := c.c.NewRequest("PATCH", "/v1/"+path)
r.Headers = http.Header{
"Content-Type": []string{"application/merge-patch+json"},
}
if err := r.SetJSONBody(data); err != nil {
return nil, err
}

return c.write(ctx, path, r)
}

func (c *Logical) WriteBytes(path string, data []byte) (*Secret, error) {
r := c.c.NewRequest("PUT", "/v1/"+path)
r.BodyBytes = data

return c.write(path, r)
return c.write(context.Background(), path, r)
}

func (c *Logical) write(path string, request *Request) (*Secret, error) {
ctx, cancelFunc := context.WithCancel(context.Background())
defer cancelFunc()
func (c *Logical) write(ctx context.Context, path string, request *Request) (*Secret, error) {
resp, err := c.c.RawRequestWithContext(ctx, request)
if resp != nil {
defer resp.Body.Close()
Expand Down
5 changes: 5 additions & 0 deletions changelog/12687.txt
@@ -0,0 +1,5 @@
```release-note:feature
**KV patch**: Add partial update support the for the `/<mount>/data/:path` kv-v2
ccapurso marked this conversation as resolved.
Show resolved Hide resolved
endpoint through HTTP `PATCH`. A new `patch` ACL capability has been added and
is required to make such requests.
```
133 changes: 120 additions & 13 deletions command/kv_patch.go
@@ -1,11 +1,13 @@
package command

import (
"context"
"fmt"
"io"
"os"
"strings"

"github.com/hashicorp/vault/api"
"github.com/mitchellh/cli"
"github.com/posener/complete"
)
Expand All @@ -18,7 +20,9 @@ var (
type KVPatchCommand struct {
*BaseCommand

testStdin io.Reader // for tests
flagCAS int
flagMethod string
testStdin io.Reader // for tests
}

func (c *KVPatchCommand) Synopsis() string {
Expand All @@ -45,6 +49,25 @@ Usage: vault kv patch [options] KEY [DATA]

$ echo "abcd1234" | vault kv patch secret/foo bar=-

To perform a Check-And-Set operation, specify the -cas flag with the
appropriate version number corresponding to the key you want to perform
the CAS operation on:

$ vault kv patch -cas=1 secret/foo bar=baz

By default, this operation will attempt an HTTP PATCH operation. If your
policy does not allow that, it will fall back to a read/local update/write approach.
If you wish to specify which method this command should use, you may do so
with the -method flag. When -method=patch is specified, only an HTTP PATCH
operation will be tried. If it fails, the entire command will fail.

$ vault kv patch -method=patch secret/foo bar=baz

When -method=rw is specified, only a read/local update/write approach will be tried.
This was the default behavior previous to Vault 1.9.

$ vault kv patch -method=rw secret/foo bar=baz

Additional flags and more advanced use cases are detailed below.

` + c.Flags().Help()
Expand All @@ -54,6 +77,27 @@ Usage: vault kv patch [options] KEY [DATA]
func (c *KVPatchCommand) Flags() *FlagSets {
set := c.flagSet(FlagSetHTTP | FlagSetOutputField | FlagSetOutputFormat)

// Patch specific options
f := set.NewFlagSet("Common Options")

f.IntVar(&IntVar{
Name: "cas",
Target: &c.flagCAS,
Default: 0,
Usage: `Specifies to use a Check-And-Set operation. If set to 0 or not
raskchanky marked this conversation as resolved.
Show resolved Hide resolved
set, the patch will be allowed. If the index is non-zero the patch will
only be allowed if the key’s current version matches the version
specified in the cas parameter.`,
})

f.StringVar(&StringVar{
Name: "method",
Target: &c.flagMethod,
Usage: `Specifies which method of patching to use. If set to "patch", then
an HTTP PATCH request will be issued. If set to "rw", then a read will be
performed, then a local update, followed by a remote update.`,
})

return set
}

Expand Down Expand Up @@ -121,6 +165,30 @@ func (c *KVPatchCommand) Run(args []string) int {
return 2
}

// Check the method and behave accordingly
var secret *api.Secret
var code int

switch c.flagMethod {
case "rw":
secret, code = c.readThenWrite(client, path, newData)
case "patch":
secret, code = c.mergePatch(client, path, newData, false)
case "":
secret, code = c.mergePatch(client, path, newData, true)
default:
c.UI.Error(fmt.Sprintf("Unsupported method provided to -method flag: %s", c.flagMethod))
return 2
}

if code != 0 {
return code
}

return OutputSecret(c.UI, secret)
}

func (c *KVPatchCommand) readThenWrite(client *api.Client, path string, newData map[string]interface{}) (*api.Secret, int) {
// First, do a read.
// Note that we don't want to see curl output for the read request.
curOutputCurl := client.OutputCurlString()
Expand All @@ -129,45 +197,45 @@ func (c *KVPatchCommand) Run(args []string) int {
client.SetOutputCurlString(curOutputCurl)
if err != nil {
c.UI.Error(fmt.Sprintf("Error doing pre-read at %s: %s", path, err))
return 2
return nil, 2
}

// Make sure a value already exists
if secret == nil || secret.Data == nil {
c.UI.Error(fmt.Sprintf("No value found at %s", path))
return 2
return nil, 2
}

// Verify metadata found
rawMeta, ok := secret.Data["metadata"]
if !ok || rawMeta == nil {
c.UI.Error(fmt.Sprintf("No metadata found at %s; patch only works on existing data", path))
return 2
return nil, 2
}
meta, ok := rawMeta.(map[string]interface{})
if !ok {
c.UI.Error(fmt.Sprintf("Metadata found at %s is not the expected type (JSON object)", path))
return 2
return nil, 2
}
if meta == nil {
c.UI.Error(fmt.Sprintf("No metadata found at %s; patch only works on existing data", path))
return 2
return nil, 2
}

// Verify old data found
rawData, ok := secret.Data["data"]
if !ok || rawData == nil {
c.UI.Error(fmt.Sprintf("No data found at %s; patch only works on existing data", path))
return 2
return nil, 2
}
data, ok := rawData.(map[string]interface{})
if !ok {
c.UI.Error(fmt.Sprintf("Data found at %s is not the expected type (JSON object)", path))
return 2
return nil, 2
}
if data == nil {
c.UI.Error(fmt.Sprintf("No data found at %s; patch only works on existing data", path))
return 2
return nil, 2
}

// Copy new data over
Expand All @@ -183,19 +251,58 @@ func (c *KVPatchCommand) Run(args []string) int {
})
if err != nil {
c.UI.Error(fmt.Sprintf("Error writing data to %s: %s", path, err))
return 2
return nil, 2
}

if secret == nil {
// Don't output anything unless using the "table" format
if Format(c.UI) == "table" {
c.UI.Info(fmt.Sprintf("Success! Data written to: %s", path))
}
return 0
return nil, 0
}

if c.flagField != "" {
return PrintRawField(c.UI, secret, c.flagField)
return nil, PrintRawField(c.UI, secret, c.flagField)
}

return OutputSecret(c.UI, secret)
return secret, 0
}

func (c *KVPatchCommand) mergePatch(client *api.Client, path string, newData map[string]interface{}, rwFallback bool) (*api.Secret, int) {
data := map[string]interface{}{
"data": newData,
"options": map[string]interface{}{},
}

if c.flagCAS > 0 {
data["options"].(map[string]interface{})["cas"] = c.flagCAS
}

secret, err := client.Logical().JSONMergePatch(context.Background(), path, data)
if err != nil {
// If it's a 403, that probably means they don't have the patch capability in their policy. Fall back to
// the old way of doing it if the user didn't specify a -method. If they did, and it was "patch", then just error.
if re, ok := err.(*api.ResponseError); ok && re.StatusCode == 403 && rwFallback {
c.UI.Warn(fmt.Sprintf("Data was written to %s but we recommend that you add the \"patch\" capability to your ACL policy in order to use HTTP PATCH in the future.", path))
return c.readThenWrite(client, path, newData)
}

c.UI.Error(fmt.Sprintf("Error writing data to %s: %s", path, err))
return nil, 2
}

if secret == nil {
// Don't output anything unless using the "table" format
if Format(c.UI) == "table" {
c.UI.Info(fmt.Sprintf("Success! Data written to: %s", path))
}
return nil, 0
}

if c.flagField != "" {
return nil, PrintRawField(c.UI, secret, c.flagField)
}

return secret, 0
raskchanky marked this conversation as resolved.
Show resolved Hide resolved
}