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

fix plugin register cli error on unfound oci images #24990

Merged
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
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
8 changes: 7 additions & 1 deletion vault/logical_system.go
Original file line number Diff line number Diff line change
Expand Up @@ -592,9 +592,15 @@ func (b *SystemBackend) handlePluginCatalogUpdate(ctx context.Context, _ *logica
Sha256: sha256Bytes,
})
if err != nil {
if errors.Is(err, plugincatalog.ErrPluginNotFound) || strings.HasPrefix(err.Error(), "plugin version mismatch") {
if errors.Is(err, plugincatalog.ErrPluginNotFound) ||
errors.Is(err, plugincatalog.ErrPluginVersionMismatch) {
return logical.ErrorResponse(err.Error()), nil
}

if errors.Is(err, plugincatalog.ErrContainerizedPluginUnableToRun) {
return logical.ErrorResponse(errors.Unwrap(err).Error()), nil
thyton marked this conversation as resolved.
Show resolved Hide resolved
}

return nil, err
}

Expand Down
67 changes: 37 additions & 30 deletions vault/logical_system_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
"github.com/hashicorp/vault/helper/namespace"
"github.com/hashicorp/vault/helper/random"
"github.com/hashicorp/vault/helper/testhelpers/corehelpers"
"github.com/hashicorp/vault/helper/testhelpers/pluginhelpers"
"github.com/hashicorp/vault/helper/versions"
"github.com/hashicorp/vault/internalshared/configutil"
"github.com/hashicorp/vault/sdk/framework"
Expand Down Expand Up @@ -3888,18 +3889,25 @@ func TestSystemBackend_PluginCatalog_ContainerCRUD(t *testing.T) {
})
b := c.systemBackend

latestPlugin := pluginhelpers.CompilePlugin(t, consts.PluginTypeDatabase, "", c.pluginDirectory)
tomhjp marked this conversation as resolved.
Show resolved Hide resolved
latestPlugin.Image, latestPlugin.ImageSha256 = pluginhelpers.BuildPluginContainerImage(t, latestPlugin, c.pluginDirectory)

const pluginVersion = "v1.0.0"
pluginV100 := pluginhelpers.CompilePlugin(t, consts.PluginTypeDatabase, pluginVersion, c.pluginDirectory)
pluginV100.Image, pluginV100.ImageSha256 = pluginhelpers.BuildPluginContainerImage(t, pluginV100, c.pluginDirectory)

for name, tc := range map[string]struct {
in, expected map[string]any
}{
"minimal": {
in: map[string]any{
"oci_image": "foo-image",
"sha256": hex.EncodeToString([]byte{'1'}),
"oci_image": latestPlugin.Image,
"sha_256": latestPlugin.ImageSha256,
},
expected: map[string]interface{}{
"name": "test-plugin",
"oci_image": "foo-image",
"sha256": "31",
"oci_image": latestPlugin.Image,
"sha256": latestPlugin.ImageSha256,
"command": "",
"args": []string{},
"builtin": false,
Expand All @@ -3908,21 +3916,21 @@ func TestSystemBackend_PluginCatalog_ContainerCRUD(t *testing.T) {
},
"fully specified": {
in: map[string]any{
"oci_image": "foo-image",
"sha256": hex.EncodeToString([]byte{'1'}),
"command": "foo-command",
"oci_image": pluginV100.Image,
"sha256": pluginV100.ImageSha256,
"command": "plugin",
"args": []string{"--a=1"},
"version": "v1.0.0",
"version": pluginVersion,
"env": []string{"X=2"},
},
expected: map[string]interface{}{
"name": "test-plugin",
"oci_image": "foo-image",
"sha256": "31",
"command": "foo-command",
"oci_image": pluginV100.Image,
"sha256": pluginV100.ImageSha256,
"command": "plugin",
"args": []string{"--a=1"},
"builtin": false,
"version": "v1.0.0",
"version": pluginVersion,
},
},
} {
Expand Down Expand Up @@ -6698,24 +6706,24 @@ func TestSystemBackend_pluginRuntime_CannotDeleteRuntimeWithReferencingPlugins(t
})
b := c.systemBackend

const runtime = "custom-runtime"
const ociRuntime = "runsc"
conf := pluginruntimeutil.PluginRuntimeConfig{
Name: "foo",
Type: consts.PluginRuntimeTypeContainer,
OCIRuntime: "some-oci-runtime",
CgroupParent: "/cpulimit/",
CPU: 1,
Memory: 10000,
Name: runtime,
Type: consts.PluginRuntimeTypeContainer,
OCIRuntime: ociRuntime,
}

// Register the plugin runtime
req := logical.TestRequest(t, logical.UpdateOperation, fmt.Sprintf("plugins/runtimes/catalog/%s/%s", conf.Type.String(), conf.Name))
req.Data = map[string]interface{}{
"oci_runtime": conf.OCIRuntime,
"cgroup_parent": conf.CgroupParent,
"cpu_nanos": conf.CPU,
"memory_bytes": conf.Memory,
"oci_runtime": conf.OCIRuntime,
}

const pluginVersion = "v1.16.0"
plugin := pluginhelpers.CompilePlugin(t, consts.PluginTypeDatabase, pluginVersion, c.pluginDirectory)
plugin.Image, plugin.ImageSha256 = pluginhelpers.BuildPluginContainerImage(t, plugin, c.pluginDirectory)

resp, err := b.HandleRequest(namespace.RootContext(nil), req)
if err != nil {
t.Fatalf("err: %v %#v", err, resp)
Expand All @@ -6726,18 +6734,17 @@ func TestSystemBackend_pluginRuntime_CannotDeleteRuntimeWithReferencingPlugins(t

// Register the plugin referencing the runtime.
req = logical.TestRequest(t, logical.UpdateOperation, "plugins/catalog/database/test-plugin")
req.Data["version"] = "v0.16.0"
req.Data["sha_256"] = hex.EncodeToString([]byte{'1'})
req.Data["command"] = ""
req.Data["oci_image"] = "hashicorp/vault-plugin-auth-jwt"
req.Data["runtime"] = "foo"
req.Data["version"] = pluginVersion
req.Data["sha_256"] = plugin.ImageSha256
req.Data["oci_image"] = plugin.Image
req.Data["runtime"] = runtime
resp, err = b.HandleRequest(namespace.RootContext(nil), req)
if err != nil || resp.Error() != nil {
t.Fatalf("err: %v %v", err, resp.Error())
}

// Expect to fail to delete the plugin runtime
req = logical.TestRequest(t, logical.DeleteOperation, "plugins/runtimes/catalog/container/foo")
req = logical.TestRequest(t, logical.DeleteOperation, fmt.Sprintf("plugins/runtimes/catalog/container/%s", runtime))
resp, err = b.HandleRequest(namespace.RootContext(nil), req)
if resp == nil || !resp.IsError() || resp.Error() == nil {
t.Errorf("expected logical error but got none, resp: %#v", resp)
Expand All @@ -6748,14 +6755,14 @@ func TestSystemBackend_pluginRuntime_CannotDeleteRuntimeWithReferencingPlugins(t

// Delete the plugin.
req = logical.TestRequest(t, logical.DeleteOperation, "plugins/catalog/database/test-plugin")
req.Data["version"] = "v0.16.0"
req.Data["version"] = pluginVersion
resp, err = b.HandleRequest(namespace.RootContext(nil), req)
if err != nil || resp.Error() != nil {
t.Fatalf("err: %v %v", err, resp.Error())
}

// This time deleting the runtime should work.
req = logical.TestRequest(t, logical.DeleteOperation, "plugins/runtimes/catalog/container/foo")
req = logical.TestRequest(t, logical.DeleteOperation, fmt.Sprintf("plugins/runtimes/catalog/container/%s", runtime))
resp, err = b.HandleRequest(namespace.RootContext(nil), req)
if err != nil || resp.Error() != nil {
t.Fatalf("err: %v %v", err, resp.Error())
Expand Down
42 changes: 28 additions & 14 deletions vault/plugincatalog/plugin_catalog.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,13 @@ import (
)

var (
ErrDirectoryNotConfigured = errors.New("could not set plugin, plugin directory is not configured")
ErrPluginNotFound = errors.New("plugin not found in the catalog")
ErrPluginConnectionNotFound = errors.New("plugin connection not found for client")
ErrPluginBadType = errors.New("unable to determine plugin type")
ErrPinnedVersion = errors.New("cannot delete a pinned version")
ErrDirectoryNotConfigured = errors.New("could not set plugin, plugin directory is not configured")
ErrPluginNotFound = errors.New("plugin not found in the catalog")
ErrPluginConnectionNotFound = errors.New("plugin connection not found for client")
ErrPluginBadType = errors.New("unable to determine plugin type")
ErrPinnedVersion = errors.New("cannot delete a pinned version")
ErrPluginVersionMismatch = errors.New("plugin version mismatch")
ErrContainerizedPluginUnableToRun = errors.New("unable to run containerized plugin")
thyton marked this conversation as resolved.
Show resolved Hide resolved
)

// PluginCatalog keeps a record of plugins known to vault. External plugins need
Expand Down Expand Up @@ -592,16 +594,19 @@ func (c *PluginCatalog) getBackendRunningVersion(ctx context.Context, pluginRunn
}
return logical.EmptyPluginVersion, nil
}
merr = multierror.Append(merr, fmt.Errorf("failed to dispense plugin as backend v5: %w", err))
merr = multierror.Append(merr, err)
}
c.logger.Debug("failed to dispense v5 backend plugin", "name", pluginRunner.Name, "error", err)
config.AutoMTLS = false
config.IsMetadataMode = true
// attempt to run as a v4 backend plugin
client, err = backendplugin.NewPluginClient(ctx, c.wrapper, pluginRunner, log.NewNullLogger(), true)
if err != nil {
merr = multierror.Append(merr, fmt.Errorf("failed to dispense v4 backend plugin: %w", err))
c.logger.Debug("failed to dispense v4 backend plugin", "name", pluginRunner.Name, "error", merr)
merr = multierror.Append(merr, err)
c.logger.Debug("failed to dispense v4 backend plugin", "name", pluginRunner.Name, "error", err)
if pluginRunner.OCIImage != "" {
tomhjp marked this conversation as resolved.
Show resolved Hide resolved
return logical.EmptyPluginVersion, fmt.Errorf("%w: %s", ErrContainerizedPluginUnableToRun, merr)
thyton marked this conversation as resolved.
Show resolved Hide resolved
}
return logical.EmptyPluginVersion, merr.ErrorOrNil()
}
c.logger.Debug("successfully dispensed v4 backend plugin", "name", pluginRunner.Name)
Expand Down Expand Up @@ -657,7 +662,7 @@ func (c *PluginCatalog) getDatabaseRunningVersion(ctx context.Context, pluginRun
}
return logical.EmptyPluginVersion, nil
}
merr = multierror.Append(merr, fmt.Errorf("failed to load plugin as database v5: %w", err))
merr = multierror.Append(merr, err)

c.logger.Debug("attempting to load database plugin as v4", "name", pluginRunner.Name)
v4Client, err := v4.NewPluginClient(ctx, c.wrapper, pluginRunner, log.NewNullLogger(), true)
Expand All @@ -676,8 +681,13 @@ func (c *PluginCatalog) getDatabaseRunningVersion(ctx context.Context, pluginRun

return logical.EmptyPluginVersion, nil
}
merr = multierror.Append(merr, fmt.Errorf("failed to load plugin as database v4: %w", err))
return logical.EmptyPluginVersion, merr

merr = multierror.Append(merr, err)
if pluginRunner.OCIImage != "" {
return logical.EmptyPluginVersion, fmt.Errorf("%w: %s", ErrContainerizedPluginUnableToRun, merr)
thyton marked this conversation as resolved.
Show resolved Hide resolved
}

return logical.EmptyPluginVersion, merr.ErrorOrNil()
}

// isDatabasePlugin returns an error if the plugin is not a database plugin.
Expand Down Expand Up @@ -975,16 +985,20 @@ func (c *PluginCatalog) setInternal(ctx context.Context, plugin pluginutil.SetPl
}
if versionErr != nil {
c.logger.Warn("Error determining plugin version", "error", versionErr)
if errors.Is(versionErr, ErrContainerizedPluginUnableToRun) {
return nil, versionErr
}
} else if plugin.Version != "" && runningVersion.Version != "" && plugin.Version != runningVersion.Version {
c.logger.Warn("Plugin self-reported version did not match requested version", "plugin", plugin.Name, "requestedVersion", plugin.Version, "reportedVersion", runningVersion.Version)
return nil, fmt.Errorf("plugin version mismatch: %s reported version (%s) did not match requested version (%s)", plugin.Name, runningVersion.Version, plugin.Version)
c.logger.Warn("Plugin self-reported version did not match requested version",
thyton marked this conversation as resolved.
Show resolved Hide resolved
"plugin", plugin.Name, "requestedVersion", plugin.Version, "reportedVersion", runningVersion.Version)
return nil, fmt.Errorf("%w: %s reported version (%s) did not match requested version (%s)",
ErrPluginVersionMismatch, plugin.Name, runningVersion.Version, plugin.Version)
} else if plugin.Version == "" && runningVersion.Version != "" {
plugin.Version = runningVersion.Version
_, err := semver.NewVersion(plugin.Version)
if err != nil {
return nil, fmt.Errorf("plugin self-reported version %q is not a valid semantic version: %w", plugin.Version, err)
}

}

entry := &pluginutil.PluginRunner{
Expand Down
Loading
Loading