VAULT-19237 Add mount_type to secret response #23047
Conversation
github-actions
bot
added
the
hashicorp-contributed-pr
|
CI Results: |
|
Build Results: |
|
Can we make this more generic? I see that Router.routeCommon is already setting MountType on the request, so Core.handleRequest could use that to populate the response field. |
I've been looking into this, and I'm not sure how to access the I agree, in principle, for what it's worth, and this would be a great option, I'm just missing how to do this in |
vault/external_tests/api/kv_test.go
Outdated
| coreConfig := &vault.CoreConfig{ | ||
| LogicalBackends: map[string]logical.Factory{ | ||
| "kv": logicalKv.Factory, | ||
| }, | ||
| } | ||
|
|
||
| cluster := vault.NewTestCluster(t, coreConfig, &vault.TestClusterOptions{ | ||
| HandlerFunc: vaulthttp.Handler, | ||
| }) | ||
|
|
||
| cluster.Start() | ||
| defer cluster.Cleanup() | ||
|
|
There was a problem hiding this comment.
🤔 Hey, do we care about having multiple cores in this test? If not, you could probably save some lines by using minimal.NewTestSoloCluster.
| @@ -3769,7 +3779,7 @@ var file_sdk_plugin_pb_backend_proto_rawDesc = []byte{ | |||
| 0x6c, 0x5f, 0x64, 0x61, 0x74, 0x61, 0x18, 0x02, 0x20, 0x01, 0x28, 0x09, 0x52, 0x0c, 0x69, 0x6e, | |||
| 0x74, 0x65, 0x72, 0x6e, 0x61, 0x6c, 0x44, 0x61, 0x74, 0x61, 0x12, 0x19, 0x0a, 0x08, 0x6c, 0x65, | |||
| 0x61, 0x73, 0x65, 0x5f, 0x69, 0x64, 0x18, 0x03, 0x20, 0x01, 0x28, 0x09, 0x52, 0x07, 0x6c, 0x65, | |||
| 0x61, 0x73, 0x65, 0x49, 0x64, 0x22, 0xc8, 0x02, 0x0a, 0x08, 0x52, 0x65, 0x73, 0x70, 0x6f, 0x6e, | |||
| 0x61, 0x73, 0x65, 0x49, 0x64, 0x22, 0xe7, 0x02, 0x0a, 0x08, 0x52, 0x65, 0x73, 0x70, 0x6f, 0x6e, | |||
There was a problem hiding this comment.
I don't know why there are a lot of changes in this area of the file, I didn't touch this area and I followed the instructions on "protobuf & gRPC generation". If this is cause for concern, let me know.
There was a problem hiding this comment.
Flyby comment: this looks perfectly normal to me. The protobuf compiler generates lots of diff here anytime the service or message definitions change.
There was a problem hiding this comment.
Awesome! Thanks for confirming, it puts my mind at ease, haha
vault/external_tests/api/kv_test.go
Outdated
|
|
||
| cluster := minimal.NewTestSoloCluster(t, nil) | ||
|
|
||
| cluster.Start() |
There was a problem hiding this comment.
FYI this is a no-op for any use of TestCluster (including NewTestCluster), the Start method exists only because we haven't got around to removing all uses of it.
vault/external_tests/api/kv_test.go
Outdated
| cluster := minimal.NewTestSoloCluster(t, nil) | ||
|
|
||
| cluster.Start() | ||
| defer cluster.Cleanup() |
There was a problem hiding this comment.
This is also unnecessary with NewTestSoloCluster.
This is the first step towards the Vault proxy static secret caching initiative. We return
kvfor KVV1/KVV2 responses.Surprisingly, I couldn't find any KV tests that were explicitly testing the Get method, so I made my own.