Merge pull request #1308 from ArangoGutierrez/i1225

Fix handling of unrecognised CDI hooks

Author: elezar

cmd/nvidia-cdi-hook/commands/commands.go

@@ -17,6 +17,9 @@
 package commands
 
 import (
+	"context"
+	"strings"
+
 	"github.com/urfave/cli/v3"
 
 	"github.com/NVIDIA/nvidia-container-toolkit/cmd/nvidia-cdi-hook/chmod"
@@ -27,27 +30,89 @@ import (
 	"github.com/NVIDIA/nvidia-container-toolkit/internal/logger"
 )
 
-// New creates the commands associated with supported CDI hooks.
-// These are shared by the nvidia-cdi-hook and nvidia-ctk hook commands.
-func New(logger logger.Interface) []*cli.Command {
-	return []*cli.Command{
+// ConfigureCDIHookCommand configures a base command with supported CDI hooks
+// and error handling for unsupported hooks.
+// This allows the same command to be used for the nvidia-cdi-hook and
+// nvidia-ctk hook commands.
+func ConfigureCDIHookCommand(logger logger.Interface, base *cli.Command) *cli.Command {
+	// We set the default action for the command to issue a warning and exit
+	// with no error.
+	// This means that if an unsupported hook is run, a container will not fail
+	// to launch. An unsupported hook could be the result of a CDI specification
+	// referring to a new hook that is not yet supported by an older NVIDIA
+	// Container Toolkit version or a hook that has been removed in newer
+	// version.
+	base.Action = func(ctx context.Context, cmd *cli.Command) error {
+		return issueUnsupportedHookWarning(logger, cmd)
+	}
+	// CommandNotFound is triggered when an unrecognised (sub)command is detected.
+	// We assume that an unrecognised (sub)command represents an unsupported hook
+	// (usually a hook that was added or removed)
+	base.CommandNotFound = func(ctx context.Context, cmd *cli.Command, commandName string) {
+		_ = issueUnsupportedHookWarning(logger, cmd)
+	}
+	// OnUsageError is triggered when an unexpected flag is detected.
+	// We check the invoked command to determine whether it is an expected
+	// hook, and assume that this is an unsupported hook otherwise.
+	base.OnUsageError = func(ctx context.Context, cmd *cli.Command, err error, isSubcommand bool) error {
+		// If this is not an error that comes from parsing an unrecognised flag,
+		// return it as is.
+		if !strings.HasPrefix(err.Error(), "flag provided but not defined: -") {
+			return err
+		}
+
+		// If the first argument is a recognised command, we return the error as
+		// is since it represents an incorrect argument to the specific hook.
+		var subcommandName string
+		for _, arg := range cmd.Args().Slice() {
+			if strings.HasPrefix(arg, "-") {
+				continue
+			}
+			subcommandName = arg
+			break
+		}
+		// If a subcommand is detected and is a recognised subcommand, we return
+		// the error as is.
+		if subcommandName != "" && cmd.Command(subcommandName) != nil {
+			return err
+		}
+
+		// At this point either no args have been supplied or the (sub)command
+		// (first arg) is not regognised.
+		// We issue a warning and returun nil.
+		return issueUnsupportedHookWarning(logger, cmd)
+	}
+
+	// Define the supported hooks.
+	base.Commands = []*cli.Command{
 		ldcache.NewCommand(logger),
 		symlinks.NewCommand(logger),
 		chmod.NewCommand(logger),
 		cudacompat.NewCommand(logger),
 		disabledevicenodemodification.NewCommand(logger),
+		{
+			Name:   "noop",
+			Usage:  "The noop hook performs no actions and is only added to facilitate basic testing of the CLI",
+			Hidden: true,
+			Action: func(_ context.Context, _ *cli.Command) error {
+				return nil
+			},
+		},
 	}
+
+	return base
 }
 
-// IssueUnsupportedHookWarning logs a warning that no hook or an unsupported
+// issueUnsupportedHookWarning logs a warning that no hook or an unsupported
 // hook has been specified.
 // This happens if a subcommand is provided that does not match one of the
 // subcommands that has been explicitly specified.
-func IssueUnsupportedHookWarning(logger logger.Interface, c *cli.Command) {
+func issueUnsupportedHookWarning(logger logger.Interface, c *cli.Command) error {
 	args := c.Args().Slice()
 	if len(args) == 0 {
 		logger.Warningf("No CDI hook specified")
 	} else {
 		logger.Warningf("Unsupported CDI hook: %v", args[0])
 	}
+	return nil
 }

cmd/nvidia-cdi-hook/main.go

@@ -45,7 +45,7 @@ func main() {
 	opts := options{}
 
 	// Create the top-level CLI
-	c := cli.Command{
+	c := commands.ConfigureCDIHookCommand(logger, &cli.Command{
 		Name:    "NVIDIA CDI Hook",
 		Usage:   "Command to structure files for usage inside a container, called as hooks from a container runtime, defined in a CDI yaml file",
 		Version: info.GetVersionString(),
@@ -61,19 +61,6 @@ func main() {
 			logger.SetLevel(logLevel)
 			return ctx, nil
 		},
-		// We set the default action for the `nvidia-cdi-hook` command to issue a
-		// warning and exit with no error.
-		// This means that if an unsupported hook is run, a container will not fail
-		// to launch. An unsupported hook could be the result of a CDI specification
-		// referring to a new hook that is not yet supported by an older NVIDIA
-		// Container Toolkit version or a hook that has been removed in newer
-		// version.
-		Action: func(ctx context.Context, cmd *cli.Command) error {
-			commands.IssueUnsupportedHookWarning(logger, cmd)
-			return nil
-		},
-		// Define the subcommands
-		Commands: commands.New(logger),
 		Flags: []cli.Flag{
 			&cli.BoolFlag{
 				Name:        "debug",
@@ -91,7 +78,7 @@ func main() {
 				Sources: cli.EnvVars("NVIDIA_CTK_QUIET", "NVIDIA_CDI_QUIET"),
 			},
 		},
-	}
+	})
 
 	// Run the CLI
 	err := c.Run(context.Background(), os.Args)

cmd/nvidia-ctk/hook/hook.go

@@ -17,8 +17,6 @@
 package hook
 
 import (
-	"context"
-
 	"github.com/NVIDIA/nvidia-container-toolkit/cmd/nvidia-cdi-hook/commands"
 	"github.com/NVIDIA/nvidia-container-toolkit/internal/logger"
 
@@ -39,23 +37,8 @@ func NewCommand(logger logger.Interface) *cli.Command {
 
 // build
 func (m hookCommand) build() *cli.Command {
-	// Create the 'hook' subcommand
-	hook := cli.Command{
+	return commands.ConfigureCDIHookCommand(m.logger, &cli.Command{
 		Name:  "hook",
 		Usage: "A collection of hooks that may be injected into an OCI spec",
-		// We set the default action for the `hook` subcommand to issue a
-		// warning and exit with no error.
-		// This means that if an unsupported hook is run, a container will not fail
-		// to launch. An unsupported hook could be the result of a CDI specification
-		// referring to a new hook that is not yet supported by an older NVIDIA
-		// Container Toolkit version or a hook that has been removed in newer
-		// version.
-		Action: func(ctx context.Context, cmd *cli.Command) error {
-			commands.IssueUnsupportedHookWarning(m.logger, cmd)
-			return nil
-		},
-		Commands: commands.New(m.logger),
-	}
-
-	return &hook
+	})
 }