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

HTTP API for pinning plugin versions #25105

Merged
merged 8 commits into from
Jan 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 8 additions & 6 deletions builtin/logical/database/backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,10 @@ type dbPluginInstance struct {
sync.RWMutex
database databaseVersionWrapper

id string
name string
closed bool
id string
name string
runningPluginVersion string
closed bool
}

func (dbi *dbPluginInstance) ID() string {
Expand Down Expand Up @@ -324,9 +325,10 @@ func (b *databaseBackend) GetConnectionWithConfig(ctx context.Context, name stri
}

dbi = &dbPluginInstance{
database: dbw,
id: id,
name: name,
database: dbw,
id: id,
name: name,
runningPluginVersion: pluginVersion,
}
oldConn := b.connections.Put(name, dbi)
if oldConn != nil {
Expand Down
31 changes: 23 additions & 8 deletions builtin/logical/database/path_config_connection.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,9 @@ var (
// DatabaseConfig is used by the Factory function to configure a Database
// object.
type DatabaseConfig struct {
PluginName string `json:"plugin_name" structs:"plugin_name" mapstructure:"plugin_name"`
PluginVersion string `json:"plugin_version" structs:"plugin_version" mapstructure:"plugin_version"`
PluginName string `json:"plugin_name" structs:"plugin_name" mapstructure:"plugin_name"`
PluginVersion string `json:"plugin_version" structs:"plugin_version" mapstructure:"plugin_version"`
RunningPluginVersion string `json:"running_plugin_version,omitempty" structs:"running_plugin_version,omitempty" mapstructure:"running_plugin_version,omitempty"`
// ConnectionDetails stores the database specific connection settings needed
// by each database type.
ConnectionDetails map[string]interface{} `json:"connection_details" structs:"connection_details" mapstructure:"connection_details"`
Expand Down Expand Up @@ -376,9 +377,22 @@ func (b *databaseBackend) connectionReadHandler() framework.OperationFunc {
delete(config.ConnectionDetails, "private_key")
delete(config.ConnectionDetails, "service_account_json")

return &logical.Response{
Data: structs.New(config).Map(),
}, nil
resp := &logical.Response{}
if dbi, err := b.GetConnection(ctx, req.Storage, name); err == nil {
config.RunningPluginVersion = dbi.runningPluginVersion
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be good to add running SHA256 as well like we have for secrets/auth plugins, but this requires a bit of refactoring in the sdk package, so to be pragmatic (and safe) I've left it as just the plugin version for now.

if config.PluginVersion != "" && config.PluginVersion != config.RunningPluginVersion {
warning := fmt.Sprintf("Plugin version is configured as %q, but running %q", config.PluginVersion, config.RunningPluginVersion)
if pinnedVersion, _ := b.getPinnedVersion(ctx, config.PluginName); pinnedVersion == config.RunningPluginVersion {
warning += " because that version is pinned"
} else {
warning += " either due to a pinned version or because the plugin was upgraded and not yet reloaded"
}
resp.AddWarning(warning)
}
}

resp.Data = structs.New(config).Map()
return resp, nil
}
}

Expand Down Expand Up @@ -507,9 +521,10 @@ func (b *databaseBackend) connectionWriteHandler() framework.OperationFunc {

// Close and remove the old connection
oldConn := b.connections.Put(name, &dbPluginInstance{
database: dbw,
name: name,
id: id,
database: dbw,
name: name,
id: id,
runningPluginVersion: pluginVersion,
})
if oldConn != nil {
oldConn.Close()
Expand Down
6 changes: 6 additions & 0 deletions changelog/25105.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
```release-note:change
plugins/database: Reading connection config at `database/config/:name` will now return a computed `running_plugin_version` field if a non-builtin version is running.
```
```release-note:improvement
plugins: Add new pin version APIs to enforce all plugins of a specific type and name to run the same version.
```
4 changes: 4 additions & 0 deletions scripts/go-helper.sh
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ check_fmt() {
echo "--> The following files need to be reformatted with gofumpt"
printf '%s\n' "${malformed[@]}"
echo "Run \`make fmt\` to reformat code."
for file in "${malformed[@]}"; do
gofumpt -w "$file"
echo "$(git diff --no-color "$file")"
done
exit 1
fi
}
Expand Down
93 changes: 0 additions & 93 deletions vault/external_plugin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import (
"github.com/hashicorp/vault/helper/testhelpers/pluginhelpers"
"github.com/hashicorp/vault/sdk/framework"
"github.com/hashicorp/vault/sdk/helper/consts"
"github.com/hashicorp/vault/sdk/helper/pluginutil"
"github.com/hashicorp/vault/sdk/logical"
"github.com/hashicorp/vault/sdk/plugin"
"github.com/hashicorp/vault/sdk/plugin/mock"
Expand Down Expand Up @@ -96,98 +95,6 @@ func TestCore_EnableExternalPlugin(t *testing.T) {
}
}

// TestCore_UpgradePluginUsingPinnedVersion tests a full workflow of upgrading
// an external plugin gated by pinned versions.
func TestCore_UpgradePluginUsingPinnedVersion(t *testing.T) {
cluster := NewTestCluster(t, &CoreConfig{}, &TestClusterOptions{
Plugins: []*TestPluginConfig{
{
Typ: consts.PluginTypeCredential,
Versions: []string{""},
},
{
Typ: consts.PluginTypeSecrets,
Versions: []string{""},
},
},
})

cluster.Start()
t.Cleanup(cluster.Cleanup)

c := cluster.Cores[0].Core
TestWaitActive(t, c)

for name, tc := range map[string]struct {
idx int
}{
"credential plugin": {
idx: 0,
},
"secrets plugin": {
idx: 1,
},
} {
t.Run(name, func(t *testing.T) {
plugin := cluster.Plugins[tc.idx]
for _, version := range []string{"v1.0.0", "v1.0.1"} {
registerPlugin(t, c.systemBackend, plugin.Name, plugin.Typ.String(), version, plugin.Sha256, plugin.FileName)
}

// Mount 1.0.0 then pin to 1.0.1
mountPlugin(t, c.systemBackend, plugin.Name, plugin.Typ, "v1.0.0", "")
err := c.pluginCatalog.SetPinnedVersion(context.Background(), &pluginutil.PinnedVersion{
Name: plugin.Name,
Type: plugin.Typ,
Version: "v1.0.1",
})
if err != nil {
t.Fatal(err)
}

mountedPath := "foo/"
if plugin.Typ == consts.PluginTypeCredential {
mountedPath = "auth/" + mountedPath
}
expectRunningVersion(t, c, mountedPath, "v1.0.0")

reloaded, err := c.reloadMatchingPlugin(context.Background(), nil, plugin.Typ, plugin.Name)
if reloaded != 1 || err != nil {
t.Fatal(reloaded, err)
}

// Pinned version should be in effect after reloading.
expectRunningVersion(t, c, mountedPath, "v1.0.1")

err = c.pluginCatalog.DeletePinnedVersion(context.Background(), plugin.Typ, plugin.Name)
if err != nil {
t.Fatal(err)
}

reloaded, err = c.reloadMatchingPlugin(context.Background(), nil, plugin.Typ, plugin.Name)
if reloaded != 1 || err != nil {
t.Fatal(reloaded, err)
}

// After pin is deleted, the previously configured version should stand.
expectRunningVersion(t, c, mountedPath, "v1.0.0")
})
}
}

func expectRunningVersion(t *testing.T, c *Core, path, expectedVersion string) {
t.Helper()
match := c.router.MatchingMount(namespace.RootContext(context.Background()), path)
if match != path {
t.Fatalf("missing mount for %s, match: %q", path, match)
}

raw, _ := c.router.root.Get(match)
if actual := raw.(*routeEntry).mountEntry.RunningVersion; expectedVersion != actual {
t.Fatalf("expected running_plugin_version to be %s but got %s", expectedVersion, actual)
}
}

func TestCore_EnableExternalPlugin_MultipleVersions(t *testing.T) {
for name, tc := range map[string]struct {
pluginType consts.PluginType
Expand Down
Loading
Loading