pass map representation of proto duration for patch of delete_version_after #62
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
briankassouf
approved these changes
Jun 3, 2022
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.
Overview
Attempting to PATCH KVv2 metadata
delete_version_afterfield with a value of0sfor an entry whose value is non-zero does not work. This ultimately is due to the fact thatdurationpb.Durationstruct containsomitemptyjson tags (thejson-patchlibrary requires marshaled JSON byte arrays) for its underlying fields. With that provided0swill marshal into{}rather than{"seconds": 0, "nanos": 0}. An empty map is treated as a NOOP when it comes to JSON merge patches.Design of Change
A fix has been added to the patch preprocessor for the KVv2 metadata endpoint. Rather than provided the incoming
delete_version_afterfield as adurationpb.Duration, that duration is converted into a map inline to match the resulting structure of marshaling as JSON minus the omission of empty values.Attempting to treat both the
delete_version_aftervalue of both the input data and existing resource as integers (as provided byframework.TypeDurationSecond) resulted in more complex logic as it required both pre and post processing of the existing resource data. Due to the added complexity, the fix provided in this PR seemed more ideal.A changelog entry will be added to Vault once a new version is of this plugin is cut and thus upgraded in that repo.