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 all 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
3 changes: 3 additions & 0 deletions changelog/24990.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
cli: fixes plugin register CLI failure to error when plugin image doesn't exist
```
5 changes: 4 additions & 1 deletion vault/logical_system.go
Original file line number Diff line number Diff line change
Expand Up @@ -596,9 +596,12 @@ 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) ||
errors.Is(err, plugincatalog.ErrPluginUnableToRun) {
return logical.ErrorResponse(err.Error()), nil
}

return nil, err
}

Expand Down
82 changes: 42 additions & 40 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 @@ -3879,6 +3880,10 @@ func TestSystemBackend_PluginCatalogPins_CRUD(t *testing.T) {
// TestSystemBackend_PluginCatalog_ContainerCRUD tests that plugins registered
// with oci_image set get recorded properly in the catalog.
func TestSystemBackend_PluginCatalog_ContainerCRUD(t *testing.T) {
if runtime.GOOS != "linux" {
t.Skip("Containerized plugins only supported on Linux")
}

sym, err := filepath.EvalSymlinks(os.TempDir())
if err != nil {
t.Fatalf("error: %v", err)
Expand All @@ -3888,18 +3893,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 +3920,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 All @@ -3935,15 +3947,6 @@ func TestSystemBackend_PluginCatalog_ContainerCRUD(t *testing.T) {

resp, err := b.HandleRequest(namespace.RootContext(nil), req)

// We should get a nice error back from the API if we're not on linux, but
// that's all we can test on non-linux.
if runtime.GOOS != "linux" {
if err != nil || resp.Error() == nil {
t.Fatalf("err: %v %v", err, resp.Error())
}
return
}

if err != nil || resp.Error() != nil {
t.Fatalf("err: %v %v", err, resp.Error())
}
Expand Down Expand Up @@ -6687,7 +6690,7 @@ func TestGetSealBackendStatus(t *testing.T) {

func TestSystemBackend_pluginRuntime_CannotDeleteRuntimeWithReferencingPlugins(t *testing.T) {
if runtime.GOOS != "linux" {
t.Skip("Currently plugincontainer only supports linux")
t.Skip("Containerized plugins only supported on Linux")
}
sym, err := filepath.EvalSymlinks(os.TempDir())
if err != nil {
Expand All @@ -6698,24 +6701,24 @@ func TestSystemBackend_pluginRuntime_CannotDeleteRuntimeWithReferencingPlugins(t
})
b := c.systemBackend

const runtime = "custom-runtime"
const ociRuntime = "runc"
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 +6729,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 +6750,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
32 changes: 23 additions & 9 deletions vault/plugincatalog/plugin_catalog.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ var (
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")
ErrPluginUnableToRun = errors.New("unable to run plugin")
)

// PluginCatalog keeps a record of plugins known to vault. External plugins need
Expand Down Expand Up @@ -631,16 +633,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: %w", ErrPluginUnableToRun, merr.ErrorOrNil())
}
return logical.EmptyPluginVersion, merr.ErrorOrNil()
}
c.logger.Debug("successfully dispensed v4 backend plugin", "name", pluginRunner.Name)
Expand Down Expand Up @@ -696,7 +701,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 @@ -715,8 +720,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: %w", ErrPluginUnableToRun, merr.ErrorOrNil())
}

return logical.EmptyPluginVersion, merr.ErrorOrNil()
}

// isDatabasePlugin returns an error if the plugin is not a database plugin.
Expand Down Expand Up @@ -1014,16 +1024,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, ErrPluginUnableToRun) {
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.Error("Plugin self-reported version did not match requested version",
"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