Backport of Remove pinned builtin plugin versions from storage into release/1.12.x #18102
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
hc-github-team-secure-vault-core
force-pushed
the
backport/remove-builtin-version-info-from-mount-table/logically-innocent-frog
branch
2 times, most recently
from
f8786e5 to
ab60aef
Compare
|
|
* Removes _builtin_ versions from mount storage where it already exists * Stops new builtin versions being put into storage on mount creation/tuning * Stops the plugin catalog from returning a builtin plugin that has been overridden, so it more accurately reflects the plugins that are available to actually run Conflicts: vault/mount.go
tomhjp
force-pushed
the
backport/remove-builtin-version-info-from-mount-table/logically-innocent-frog
branch
from
5010597 to
7d529e7
Compare
swenson
approved these changes
Nov 23, 2022
|
Thanks! |
tomhjp
deleted the
backport/remove-builtin-version-info-from-mount-table/logically-innocent-frog
branch
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.
Backport
This PR is auto-generated from #18051 to be assessed for backporting due to the inclusion of the label backport/1.12.x.
WARNING automatic cherry-pick of commits failed. Commits will require human attention.
The below text is copied from the body of the original PR.
Addresses bug described in the 1.12 upgrade guide: https://developer.hashicorp.com/vault/docs/upgrading/upgrade-to-1.12.x#pinning-to-builtin-plugin-versions-may-cause-failure-on-upgrade.
In a nutshell, when we try to start a plugin after upgrading, if it has a builtin version stored in the mount table and that version subsequently disappeared from catalog due to the upgrade, we won't be able to mount it. To fix this, the main idea is to stop storing version information for builtin plugins, and rely on the pre-existing behaviour that not specifying a version selects the builtin (unless it's been shadowed by an unversioned external plugin).
2 main changes, that need to be done for each of the 3 plugin types:
Stopping storage for new mounts is easy enough. Cleaning up existing auth and secrets mounts also seems relatively straightforward thanks to some existing logic for upgrading their storage.
Unfortunately, database plugins don't seem to have any such existing capability, instead this relies on fixing storage whenever it's next written to for each db plugin. That means databases also need an extra shim to stop mounting from failing, because we may mount a plugin before the config has been fixed in storage. We also delete builtin plugin versions from a config read response, so the out of date storage is invisible to users unless they poke around in raw endpoints.
The approach also means users who have a builtin version pinned, and an unversioned plugin of the same name registered, will start running the unversioned plugin on upgrading. That deserves a callout in the changelog, but I don't expect it to be problematic for many people, as most people using versions for builtins will probably also use versions for externals. Plus it's still a very new feature, and the delta between the builtin and external versions is likely small.
I've also updated the plugin catalog to not return a builtin if it has been shadowed by an unversioned external plugin of the same name.
Overview of commits