Merge pull request #5030 from laurazard/hooks-plugin-name

hooks: include plugin name in hook data

cli-plugins/manager/hooks.go

@@ -14,39 +14,50 @@ 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
 	}
 
 	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 {

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)
+	}
+}

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