Vault Raw Read Support (CLI & Client) #14945
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cipherboy
force-pushed
the
cipherboy-vault-read-raw
branch
from
a772c72 to
d289a7d
Compare
kitography
reviewed
Apr 12, 2022
|
@cipherboy @kitography I'm guessing the export use-case here would be to facilitate an import and subsequent rebuild for example of a pki mount anew? Also what's the status & when is it likely to be delivered? |
|
@aphorise This didn't get much traction internally. If you're interested in pushing it and working on it, feel free to do so. But this has been proposed in the past and rejected before, so I think it'll require a bit of convincing to get it accepted. |
Not all Vault API endpoints return well-formatted JSON objects. Sometimes, in the case of the PKI secrets engine, they're not even printable (/pki/ca returns a binary (DER-encoded) certificate). While this endpoint isn't authenticated, in general the API caller would either need to use Client.RawRequestWithContext(...) directly (which the docs advise against), or setup their own net/http client and re-create much of Client and/or Client.Logical. Instead, exposing the raw Request (via the new ReadRawWithData(...)) allows callers to directly consume these non-JSON endpoints like they would nearly any other endpoint. Signed-off-by: Alexander Scheel <[email protected]>
As mentioned in the previous commit, some API endpoints return non-JSON data. We get as far as fetching this data (via client.Logical().Read), but parsing it as an api.Secret fails (as in this case, it is non-JSON). Given that we intend to update `vault read` to support such endpoints, we'll need a "raw" formatter that accepts []byte-encoded data and simply writes it to the UI. Signed-off-by: Alexander Scheel <[email protected]>
Some endpoints, such as `pki/ca` and `pki/ca/pem` return non-JSON objects. When calling `vault read` on these endpoints, an error is returned because they cannot be parsed as api.Secret instances: > Error reading pki/ca/pem: invalid character '-' in numeric literal Indeed, we go to all the trouble of (successfully) fetching this value, only to be unable to Unmarshal into a Secrets value. Instead, add support for a new -format=raw option, allowing these endpoints to be consumed by callers of `vault read` directly. Signed-off-by: Alexander Scheel <[email protected]>
cipherboy
force-pushed
the
cipherboy-vault-read-raw
branch
from
d289a7d to
cd638de
Compare
|
@aphorise Steve convinced me to update this and I eventually figured out the issue with the contexts, so the tests should now pass. We'll see what others think :-) |
|
I'm still cool with it. Any UX concerns? |
Signed-off-by: Alexander Scheel <[email protected]>
|
@sgmiller Other than that we've returned obtuse errors in non-raw queries to the PKI endpoints, nope... I tagged you on a Slack thread, if you've got ideas, happy to look at them. |
raskchanky
reviewed
Oct 26, 2022
Signed-off-by: Alexander Scheel <[email protected]>
raskchanky
approved these changes
Oct 27, 2022
stevendpclark
approved these changes
Oct 28, 2022
|
Thanks all! Merging... |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Not really sure who should own this work :-) I realize the approach hasn't been popular in the past...
...but I figured I'd re-open a branch I had with this change to see where it'll go this time.
We expose a
ReadRaw...operations, which returns a rawResponserather than a parsedSecret. This allows us to hit endpoints (such as the various PKI endpoints -- which are ever-expanding) which return non-JSON formatted responses from both the Client and the API. These responses include DER and PEM-formatted bundles.From a CLI perspective, this lets the following succeed, which IMO, is a big improvement over the existing behavior:
Having access to the
Responseobject directly could also allow something like #14944 be implementable, wherein the rawerrorsvalue is accessible to the CLI to parse -- and (if not an error) -- optionally turn into aSecretif the response warrants it. It also allow JSON-formatted errors (which the API returns butvault read -format=jsondoesn't do something useful with).Who knows what else this could enable... :-)