From 9d8320de9def774a8f91c80b01f72a2e1eec7a59 Mon Sep 17 00:00:00 2001 From: Laura Brehm Date: Fri, 19 Apr 2024 12:32:18 +0100 Subject: [PATCH] hooks: include full configured command Before, for plugin commands, only the plugin name (such as `buildx`) would be both included as `RootCmd` when passed to the hook plugin, which isn't enough information for a plugin to decide whether to execute a hook or not since plugins implement multiple varied commands (`buildx build`, `buildx prune`, etc.). This commit changes the hook logic to account for this situation, so that the the entire configured hook is passed, i.e., if a user has a hook configured for `buildx imagetools inspect` and the command `docker buildx imagetools inspect alpine` is called, then the plugin hooks will be passed `buildx imagetools inspect`. This logic works for aliased commands too, so whether `docker build ...` or `docker buildx build` is executed (unless Buildx is disabled) the hook will be invoked with `buildx build`. Signed-off-by: Laura Brehm hooks: include full match when invoking plugins Signed-off-by: Laura Brehm --- cli-plugins/manager/hooks.go | 88 +++++++++++++++++++++---------- cli-plugins/manager/hooks_test.go | 72 +++++++++++++++++++++++++ cmd/docker/docker.go | 8 +-- 3 files changed, 137 insertions(+), 31 deletions(-) diff --git a/cli-plugins/manager/hooks.go b/cli-plugins/manager/hooks.go index 09c49397f623..7b45e2afd72e 100644 --- a/cli-plugins/manager/hooks.go +++ b/cli-plugins/manager/hooks.go @@ -14,31 +14,41 @@ import ( // that plugins declaring support for hooks get passed when // being invoked following a CLI command execution. type HookPluginData struct { + // RootCmd is a string representing the matching hook configuration + // which is currently being invoked. If a hook for `docker context` is + // configured and the user executes `docker context ls`, the plugin will + // be invoked with `context`. RootCmd string Flags map[string]string } -// RunPluginHooks calls the hook subcommand for all present -// CLI plugins that declare support for hooks in their metadata -// and parses/prints their responses. -func RunPluginHooks(dockerCli command.Cli, rootCmd, subCommand *cobra.Command, plugin string, args []string) error { - subCmdName := subCommand.Name() - if plugin != "" { - subCmdName = plugin - } - var flags map[string]string - if plugin == "" { - flags = getCommandFlags(subCommand) - } else { - flags = getNaiveFlags(args) - } - nextSteps := invokeAndCollectHooks(dockerCli, rootCmd, subCommand, subCmdName, flags) +// RunCLICommandHooks is the entrypoint into the hooks execution flow after +// a main CLI command was executed. It calls the hook subcommand for all +// present CLI plugins that declare support for hooks in their metadata and +// parses/prints their responses. +func RunCLICommandHooks(dockerCli command.Cli, rootCmd, subCommand *cobra.Command) { + commandName := strings.TrimPrefix(subCommand.CommandPath(), rootCmd.Name()+" ") + flags := getCommandFlags(subCommand) + + runHooks(dockerCli, rootCmd, subCommand, commandName, flags) +} + +// RunPluginHooks is the entrypoint for the hooks execution flow +// after a plugin command was just executed by the CLI. +func RunPluginHooks(dockerCli command.Cli, rootCmd, subCommand *cobra.Command, args []string) { + commandName := strings.Join(args, " ") + flags := getNaiveFlags(args) + + runHooks(dockerCli, rootCmd, subCommand, commandName, flags) +} + +func runHooks(dockerCli command.Cli, rootCmd, subCommand *cobra.Command, invokedCommand string, flags map[string]string) { + nextSteps := invokeAndCollectHooks(dockerCli, rootCmd, subCommand, invokedCommand, flags) hooks.PrintNextSteps(dockerCli.Err(), nextSteps) - return nil } -func invokeAndCollectHooks(dockerCli command.Cli, rootCmd, subCmd *cobra.Command, hookCmdName string, flags map[string]string) []string { +func invokeAndCollectHooks(dockerCli command.Cli, rootCmd, subCmd *cobra.Command, subCmdStr string, flags map[string]string) []string { pluginsCfg := dockerCli.ConfigFile().Plugins if pluginsCfg == nil { return nil @@ -46,7 +56,8 @@ func invokeAndCollectHooks(dockerCli command.Cli, rootCmd, subCmd *cobra.Command nextSteps := make([]string, 0, len(pluginsCfg)) for pluginName, cfg := range pluginsCfg { - if !registersHook(cfg, hookCmdName) { + match, ok := pluginMatch(cfg, subCmdStr) + if !ok { continue } @@ -55,7 +66,7 @@ func invokeAndCollectHooks(dockerCli command.Cli, rootCmd, subCmd *cobra.Command continue } - hookReturn, err := p.RunHook(hookCmdName, flags) + hookReturn, err := p.RunHook(match, flags) if err != nil { // skip misbehaving plugins, but don't halt execution continue @@ -81,18 +92,41 @@ func invokeAndCollectHooks(dockerCli command.Cli, rootCmd, subCmd *cobra.Command return nextSteps } -func registersHook(pluginCfg map[string]string, subCmdName string) bool { - hookCmdStr, ok := pluginCfg["hooks"] - if !ok { - return false +// pluginMatch takes a plugin configuration and a string representing the +// command being executed (such as 'image ls' – the root 'docker' is omitted) +// and, if the configuration includes a hook for the invoked command, returns +// the configured hook string. +func pluginMatch(pluginCfg map[string]string, subCmd string) (string, bool) { + configuredPluginHooks, ok := pluginCfg["hooks"] + if !ok || configuredPluginHooks == "" { + return "", false } - commands := strings.Split(hookCmdStr, ",") + + commands := strings.Split(configuredPluginHooks, ",") for _, hookCmd := range commands { - if hookCmd == subCmdName { - return true + if hookMatch(hookCmd, subCmd) { + return hookCmd, true } } - return false + + return "", false +} + +func hookMatch(hookCmd, subCmd string) bool { + hookCmdTokens := strings.Split(hookCmd, " ") + subCmdTokens := strings.Split(subCmd, " ") + + if len(hookCmdTokens) > len(subCmdTokens) { + return false + } + + for i, v := range hookCmdTokens { + if v != subCmdTokens[i] { + return false + } + } + + return true } func getCommandFlags(cmd *cobra.Command) map[string]string { diff --git a/cli-plugins/manager/hooks_test.go b/cli-plugins/manager/hooks_test.go index d3ef24dabf8c..c532702f7b04 100644 --- a/cli-plugins/manager/hooks_test.go +++ b/cli-plugins/manager/hooks_test.go @@ -36,3 +36,75 @@ func TestGetNaiveFlags(t *testing.T) { assert.DeepEqual(t, getNaiveFlags(tc.args), tc.expectedFlags) } } + +func TestPluginMatch(t *testing.T) { + testCases := []struct { + commandString string + pluginConfig map[string]string + expectedMatch string + expectedOk bool + }{ + { + commandString: "image ls", + pluginConfig: map[string]string{ + "hooks": "image", + }, + expectedMatch: "image", + expectedOk: true, + }, + { + commandString: "context ls", + pluginConfig: map[string]string{ + "hooks": "build", + }, + expectedMatch: "", + expectedOk: false, + }, + { + commandString: "context ls", + pluginConfig: map[string]string{ + "hooks": "context ls", + }, + expectedMatch: "context ls", + expectedOk: true, + }, + { + commandString: "image ls", + pluginConfig: map[string]string{ + "hooks": "image ls,image", + }, + expectedMatch: "image ls", + expectedOk: true, + }, + { + commandString: "image ls", + pluginConfig: map[string]string{ + "hooks": "", + }, + expectedMatch: "", + expectedOk: false, + }, + { + commandString: "image inspect", + pluginConfig: map[string]string{ + "hooks": "image i", + }, + expectedMatch: "", + expectedOk: false, + }, + { + commandString: "image inspect", + pluginConfig: map[string]string{ + "hooks": "image", + }, + expectedMatch: "image", + expectedOk: true, + }, + } + + for _, tc := range testCases { + match, ok := pluginMatch(tc.pluginConfig, tc.commandString) + assert.Equal(t, ok, tc.expectedOk) + assert.Equal(t, match, tc.expectedMatch) + } +} diff --git a/cmd/docker/docker.go b/cmd/docker/docker.go index 6a766ee50bf9..e6bba05622a4 100644 --- a/cmd/docker/docker.go +++ b/cmd/docker/docker.go @@ -336,7 +336,7 @@ func runDocker(ctx context.Context, dockerCli *command.DockerCli) error { err := tryPluginRun(dockerCli, cmd, args[0], envs) if err == nil { if dockerCli.HooksEnabled() && dockerCli.Out().IsTerminal() && ccmd != nil { - _ = pluginmanager.RunPluginHooks(dockerCli, cmd, ccmd, args[0], args) + pluginmanager.RunPluginHooks(dockerCli, cmd, ccmd, args) } return nil } @@ -354,10 +354,10 @@ func runDocker(ctx context.Context, dockerCli *command.DockerCli) error { cmd.SetArgs(args) err = cmd.Execute() - // If the command is being executed in an interactive terminal, - // run the plugin hooks (but don't throw an error if something misbehaves) + // If the command is being executed in an interactive terminal + // and hook are enabled, run the plugin hooks. if dockerCli.HooksEnabled() && dockerCli.Out().IsTerminal() && subCommand != nil { - _ = pluginmanager.RunPluginHooks(dockerCli, cmd, subCommand, "", args) + pluginmanager.RunCLICommandHooks(dockerCli, cmd, subCommand) } return err