From e54bb9763c5edfe8a9abe87d60940dfe9bf54de0 Mon Sep 17 00:00:00 2001 From: Tevin Flores Date: Wed, 4 Jun 2025 13:26:56 -0400 Subject: [PATCH] This change standardizes the command line flag naming conventions across the headscale CLI: User-related operations now consistently use -u/--user (changed from -n/--name) Node-related operations now consistently use -n/--node-id (changed from -N/--identifier) User ID operations continue to use -i/--identifier The error message for user identification has also been simplified for better clarity. Integration tests have been updated to use the new flag naming conventions, ensuring all test cases properly exercise the updated CLI interface. This includes modifying test fixtures and command invocations to align with the standardized flags. --- cmd/headscale/cli/debug.go | 21 +++++------ cmd/headscale/cli/nodes.go | 16 ++++----- cmd/headscale/cli/user_helpers.go | 8 ++--- cmd/headscale/cli/users.go | 2 +- integration/auth_key_test.go | 2 +- integration/cli_test.go | 58 +++++++++++++++---------------- integration/general_test.go | 6 ++-- integration/hsic/hsic.go | 14 ++++++-- integration/scenario.go | 4 +-- 9 files changed, 67 insertions(+), 64 deletions(-) diff --git a/cmd/headscale/cli/debug.go b/cmd/headscale/cli/debug.go index 41b46fb0..6f9eb865 100644 --- a/cmd/headscale/cli/debug.go +++ b/cmd/headscale/cli/debug.go @@ -27,17 +27,11 @@ func init() { if err != nil { log.Fatal().Err(err).Msg("") } - createNodeCmd.Flags().StringP("user", "u", "", "User") - + usernameAndIDFlag(createNodeCmd) createNodeCmd.Flags().StringP("namespace", "n", "", "User") createNodeNamespaceFlag := createNodeCmd.Flags().Lookup("namespace") createNodeNamespaceFlag.Deprecated = deprecateNamespaceMessage createNodeNamespaceFlag.Hidden = true - - err = createNodeCmd.MarkFlagRequired("user") - if err != nil { - log.Fatal().Err(err).Msg("") - } createNodeCmd.Flags().StringP("key", "k", "", "Key") err = createNodeCmd.MarkFlagRequired("key") if err != nil { @@ -61,15 +55,16 @@ var createNodeCmd = &cobra.Command{ Run: func(cmd *cobra.Command, args []string) { output, _ := cmd.Flags().GetString("output") - user, err := cmd.Flags().GetString("user") - if err != nil { - ErrorOutput(err, fmt.Sprintf("Error getting user: %s", err), output) - } - ctx, client, conn, cancel := newHeadscaleCLIWithConfig() defer cancel() defer conn.Close() + user, err := findSingleUser(ctx, client, cmd, "debug-createNodeCmd", output) + if err != nil { + // The helper already calls ErrorOutput, so we can just return + return + } + name, err := cmd.Flags().GetString("name") if err != nil { ErrorOutput( @@ -109,7 +104,7 @@ var createNodeCmd = &cobra.Command{ request := &v1.DebugCreateNodeRequest{ Key: registrationID, Name: name, - User: user, + User: user.GetName(), Routes: routes, } diff --git a/cmd/headscale/cli/nodes.go b/cmd/headscale/cli/nodes.go index cf2914a3..952b9a57 100644 --- a/cmd/headscale/cli/nodes.go +++ b/cmd/headscale/cli/nodes.go @@ -24,7 +24,7 @@ func init() { listNodesCmd.Flags().BoolP("tags", "t", false, "Show tags") usernameAndIDFlag(listNodesCmd) nodeCmd.AddCommand(listNodesCmd) - listNodeRoutesCmd.Flags().Uint64P("node-id", "N", 0, "Node identifier (ID)") + listNodeRoutesCmd.Flags().Uint64P("node-id", "n", 0, "Node identifier (ID)") nodeCmd.AddCommand(listNodeRoutesCmd) usernameAndIDFlag(registerNodeCmd) registerNodeCmd.Flags().StringP("key", "k", "", "Key") @@ -34,28 +34,28 @@ func init() { } nodeCmd.AddCommand(registerNodeCmd) - expireNodeCmd.Flags().Uint64P("node-id", "N", 0, "Node identifier (ID)") + expireNodeCmd.Flags().Uint64P("node-id", "n", 0, "Node identifier (ID)") err = expireNodeCmd.MarkFlagRequired("node-id") if err != nil { log.Fatal(err.Error()) } nodeCmd.AddCommand(expireNodeCmd) - renameNodeCmd.Flags().Uint64P("node-id", "N", 0, "Node identifier (ID)") + renameNodeCmd.Flags().Uint64P("node-id", "n", 0, "Node identifier (ID)") err = renameNodeCmd.MarkFlagRequired("node-id") if err != nil { log.Fatal(err.Error()) } nodeCmd.AddCommand(renameNodeCmd) - deleteNodeCmd.Flags().Uint64P("node-id", "N", 0, "Node identifier (ID)") + deleteNodeCmd.Flags().Uint64P("node-id", "n", 0, "Node identifier (ID)") err = deleteNodeCmd.MarkFlagRequired("node-id") if err != nil { log.Fatal(err.Error()) } nodeCmd.AddCommand(deleteNodeCmd) - moveNodeCmd.Flags().Uint64P("node-id", "N", 0, "Node identifier (ID)") + moveNodeCmd.Flags().Uint64P("node-id", "n", 0, "Node identifier (ID)") err = moveNodeCmd.MarkFlagRequired("node-id") if err != nil { @@ -66,12 +66,12 @@ func init() { nodeCmd.AddCommand(moveNodeCmd) - tagCmd.Flags().Uint64P("node-id", "N", 0, "Node identifier (ID)") + tagCmd.Flags().Uint64P("node-id", "n", 0, "Node identifier (ID)") tagCmd.MarkFlagRequired("node-id") tagCmd.Flags().StringSliceP("tags", "t", []string{}, "List of tags to add to the node") nodeCmd.AddCommand(tagCmd) - approveRoutesCmd.Flags().Uint64P("node-id", "N", 0, "Node identifier (ID)") + approveRoutesCmd.Flags().Uint64P("node-id", "n", 0, "Node identifier (ID)") approveRoutesCmd.MarkFlagRequired("node-id") approveRoutesCmd.Flags().StringSliceP("routes", "r", []string{}, `List of routes that will be approved (comma-separated, e.g. "10.0.0.0/8,192.168.0.0/24" or empty string to remove all approved routes)`) nodeCmd.AddCommand(approveRoutesCmd) @@ -150,7 +150,7 @@ var listNodesCmd = &cobra.Command{ // Check if user identifier flags are provided id, _ := cmd.Flags().GetInt64("identifier") - username, _ := cmd.Flags().GetString("name") + username, _ := cmd.Flags().GetString("user") request := &v1.ListNodesRequest{} diff --git a/cmd/headscale/cli/user_helpers.go b/cmd/headscale/cli/user_helpers.go index 678cdabd..6e5f6909 100644 --- a/cmd/headscale/cli/user_helpers.go +++ b/cmd/headscale/cli/user_helpers.go @@ -22,16 +22,16 @@ func usernameAndIDFlag(cmd *cobra.Command, opts ...string) { nameHelp = opts[1] } cmd.PersistentFlags().Int64P("identifier", "i", -1, idHelp) - cmd.PersistentFlags().StringP("name", "n", "", nameHelp) + cmd.PersistentFlags().StringP("user", "u", "", nameHelp) } // usernameAndIDFromFlag returns the username and ID from the flags of the command. // If both are empty, it will exit the program with an error. func usernameAndIDFromFlag(cmd *cobra.Command) (uint64, string) { - username, _ := cmd.Flags().GetString("name") + username, _ := cmd.Flags().GetString("user") identifier, _ := cmd.Flags().GetInt64("identifier") if username == "" && identifier < 0 { - err := errors.New("--name or --identifier flag is required") + err := errors.New("--user or --identifier flag is required") ErrorOutput( err, fmt.Sprintf( @@ -71,7 +71,7 @@ func findSingleUser( } if len(users.GetUsers()) != 1 { - err := fmt.Errorf("Unable to determine user to %s, query returned multiple users, use ID", operationName) + err := fmt.Errorf("Unable to determine user for %s", operationName) ErrorOutput( err, fmt.Sprintf("Error: %s", status.Convert(err).Message()), diff --git a/cmd/headscale/cli/users.go b/cmd/headscale/cli/users.go index 638c077d..e957ddba 100644 --- a/cmd/headscale/cli/users.go +++ b/cmd/headscale/cli/users.go @@ -168,7 +168,7 @@ var listUsersCmd = &cobra.Command{ request := &v1.ListUsersRequest{} id, _ := cmd.Flags().GetInt64("identifier") - username, _ := cmd.Flags().GetString("name") + username, _ := cmd.Flags().GetString("user") email, _ := cmd.Flags().GetString("email") // filter by one param at most diff --git a/integration/auth_key_test.go b/integration/auth_key_test.go index d54ff593..4e0d30ac 100644 --- a/integration/auth_key_test.go +++ b/integration/auth_key_test.go @@ -342,7 +342,7 @@ func TestAuthKeyLogoutAndReloginSameUserExpiredKey(t *testing.T) { []string{ "headscale", "preauthkeys", - "--user", + "--identifier", strconv.FormatUint(userMap[userName].GetId(), 10), "expire", key.Key, diff --git a/integration/cli_test.go b/integration/cli_test.go index 2cff0500..9a7fd113 100644 --- a/integration/cli_test.go +++ b/integration/cli_test.go @@ -18,8 +18,8 @@ import ( "github.com/juanfont/headscale/integration/tsic" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "tailscale.com/tailcfg" "golang.org/x/exp/slices" + "tailscale.com/tailcfg" ) func executeAndUnmarshal[T any](headscale ControlServer, command []string, result T) error { @@ -128,7 +128,7 @@ func TestUserCommand(t *testing.T) { "list", "--output", "json", - "--name=user1", + "--user=user1", }, &listByUsername, ) @@ -219,7 +219,7 @@ func TestUserCommand(t *testing.T) { "users", "destroy", "--force", - "--name=newname", + "--user=newname", }, ) assert.Nil(t, err) @@ -272,7 +272,7 @@ func TestPreAuthKeyCommand(t *testing.T) { []string{ "headscale", "preauthkeys", - "--user", + "-i", "1", "create", "--reusable", @@ -298,7 +298,7 @@ func TestPreAuthKeyCommand(t *testing.T) { []string{ "headscale", "preauthkeys", - "--user", + "-i", "1", "list", "--output", @@ -355,7 +355,7 @@ func TestPreAuthKeyCommand(t *testing.T) { []string{ "headscale", "preauthkeys", - "--user", + "-i", "1", "expire", listedPreAuthKeys[1].GetKey(), @@ -369,7 +369,7 @@ func TestPreAuthKeyCommand(t *testing.T) { []string{ "headscale", "preauthkeys", - "--user", + "-i", "1", "list", "--output", @@ -409,7 +409,7 @@ func TestPreAuthKeyCommandWithoutExpiry(t *testing.T) { []string{ "headscale", "preauthkeys", - "--user", + "-i", "1", "create", "--reusable", @@ -426,7 +426,7 @@ func TestPreAuthKeyCommandWithoutExpiry(t *testing.T) { []string{ "headscale", "preauthkeys", - "--user", + "-i", "1", "list", "--output", @@ -471,7 +471,7 @@ func TestPreAuthKeyCommandReusableEphemeral(t *testing.T) { []string{ "headscale", "preauthkeys", - "--user", + "-i", "1", "create", "--reusable=true", @@ -488,7 +488,7 @@ func TestPreAuthKeyCommandReusableEphemeral(t *testing.T) { []string{ "headscale", "preauthkeys", - "--user", + "-i", "1", "create", "--ephemeral=true", @@ -508,7 +508,7 @@ func TestPreAuthKeyCommandReusableEphemeral(t *testing.T) { []string{ "headscale", "preauthkeys", - "--user", + "-i", "1", "list", "--output", @@ -559,7 +559,7 @@ func TestPreAuthKeyCorrectUserLoggedInCommand(t *testing.T) { []string{ "headscale", "preauthkeys", - "--user", + "-i", strconv.FormatUint(u2.GetId(), 10), "create", "--reusable", @@ -860,7 +860,7 @@ func TestNodeTagCommand(t *testing.T) { "headscale", "nodes", "tag", - "-i", "1", + "-n", "1", "-t", "tag:test", "--output", "json", }, @@ -875,7 +875,7 @@ func TestNodeTagCommand(t *testing.T) { "headscale", "nodes", "tag", - "-i", "2", + "-n", "2", "-t", "wrong-tag", "--output", "json", }, @@ -913,8 +913,6 @@ func TestNodeTagCommand(t *testing.T) { ) } - - func TestNodeAdvertiseTagCommand(t *testing.T) { IntegrationSkip(t) t.Parallel() @@ -1250,7 +1248,7 @@ func TestNodeCommand(t *testing.T) { "headscale", "nodes", "delete", - "--identifier", + "--node-id", // Delete the last added machine "4", "--output", @@ -1376,7 +1374,7 @@ func TestNodeExpireCommand(t *testing.T) { "headscale", "nodes", "expire", - "--identifier", + "--node-id", fmt.Sprintf("%d", listAll[idx].GetId()), }, ) @@ -1503,7 +1501,7 @@ func TestNodeRenameCommand(t *testing.T) { "headscale", "nodes", "rename", - "--identifier", + "--node-id", fmt.Sprintf("%d", listAll[idx].GetId()), fmt.Sprintf("newnode-%d", idx+1), }, @@ -1541,7 +1539,7 @@ func TestNodeRenameCommand(t *testing.T) { "headscale", "nodes", "rename", - "--identifier", + "--node-id", fmt.Sprintf("%d", listAll[4].GetId()), strings.Repeat("t", 64), }, @@ -1642,9 +1640,9 @@ func TestNodeMoveCommand(t *testing.T) { "headscale", "nodes", "move", - "--identifier", + "--node-id", strconv.FormatUint(node.GetId(), 10), - "--user", + "--identifier", strconv.FormatUint(userMap["new-user"].GetId(), 10), "--output", "json", @@ -1680,9 +1678,9 @@ func TestNodeMoveCommand(t *testing.T) { "headscale", "nodes", "move", - "--identifier", + "--node-id", nodeID, - "--user", + "--identifier", "999", "--output", "json", @@ -1691,7 +1689,7 @@ func TestNodeMoveCommand(t *testing.T) { assert.ErrorContains( t, err, - "user not found", + "Unable to determine user", ) assert.Equal(t, node.GetUser().GetName(), "new-user") @@ -1701,9 +1699,9 @@ func TestNodeMoveCommand(t *testing.T) { "headscale", "nodes", "move", - "--identifier", + "--node-id", nodeID, - "--user", + "--identifier", strconv.FormatUint(userMap["old-user"].GetId(), 10), "--output", "json", @@ -1720,9 +1718,9 @@ func TestNodeMoveCommand(t *testing.T) { "headscale", "nodes", "move", - "--identifier", + "--node-id", nodeID, - "--user", + "--identifier", strconv.FormatUint(userMap["old-user"].GetId(), 10), "--output", "json", diff --git a/integration/general_test.go b/integration/general_test.go index 292eb5ca..772533e4 100644 --- a/integration/general_test.go +++ b/integration/general_test.go @@ -597,7 +597,7 @@ func TestUpdateHostnameFromClient(t *testing.T) { "node", "rename", givenName, - "--identifier", + "--node-id", strconv.FormatUint(node.GetId(), 10), }) assertNoErr(t, err) @@ -693,7 +693,7 @@ func TestExpireNode(t *testing.T) { // TODO(kradalby): This is Headscale specific and would not play nicely // with other implementations of the ControlServer interface result, err := headscale.Execute([]string{ - "headscale", "nodes", "expire", "--identifier", "1", "--output", "json", + "headscale", "nodes", "expire", "--node-id", "1", "--output", "json", }) assertNoErr(t, err) @@ -1032,7 +1032,7 @@ func Test2118DeletingOnlineNodePanics(t *testing.T) { "headscale", "nodes", "delete", - "--identifier", + "--node-id", // Delete the last added machine fmt.Sprintf("%d", nodeList[0].GetId()), "--output", diff --git a/integration/hsic/hsic.go b/integration/hsic/hsic.go index 35550c65..0f922402 100644 --- a/integration/hsic/hsic.go +++ b/integration/hsic/hsic.go @@ -447,6 +447,16 @@ func New( } } + // Create necessary directories in the container + _, _, err = dockertestutil.ExecuteCommand( + container, + []string{"mkdir", "-p", "/tmp/mapresponses"}, + []string{}, + ) + if err != nil { + log.Printf("Warning: Could not create /tmp/mapresponses directory: %v", err) + } + return hsic, nil } @@ -727,7 +737,7 @@ func (t *HeadscaleInContainer) CreateAuthKey( ) (*v1.PreAuthKey, error) { command := []string{ "headscale", - "--user", + "-i", strconv.FormatUint(user, 10), "preauthkeys", "create", @@ -997,7 +1007,7 @@ func (t *HeadscaleInContainer) ApproveRoutes(id uint64, routes []netip.Prefix) ( command := []string{ "headscale", "nodes", "approve-routes", "--output", "json", - "--identifier", strconv.FormatUint(id, 10), + "--node-id", strconv.FormatUint(id, 10), fmt.Sprintf("--routes=%s", strings.Join(util.PrefixesToString(routes), ",")), } diff --git a/integration/scenario.go b/integration/scenario.go index 0af1956b..61283f00 100644 --- a/integration/scenario.go +++ b/integration/scenario.go @@ -438,13 +438,13 @@ func (s *Scenario) CreatePreAuthKey( if headscale, err := s.Headscale(); err == nil { key, err := headscale.CreateAuthKey(user, reusable, ephemeral) if err != nil { - return nil, fmt.Errorf("failed to create user: %w", err) + return nil, fmt.Errorf("failed to create preauthkey: %w", err) } return key, nil } - return nil, fmt.Errorf("failed to create user: %w", errNoHeadscaleAvailable) + return nil, fmt.Errorf("failed to create preauthkey: %w", errNoHeadscaleAvailable) } // CreateUser creates a User to be created in the