improve errors and streamline cli error output

Signed-off-by: Kristoffer Dalby <[email protected]>
juanfont · Sep 6, 2024 · 9a09cee · 9a09cee
1 parent 42f9cdc
commit 9a09cee
Show file tree

Hide file tree

Showing 10 changed files with 30 additions and 152 deletions.
diff --git a/cmd/headscale/cli/api_key.go b/cmd/headscale/cli/api_key.go
@@ -67,14 +67,10 @@ var listAPIKeys = &cobra.Command{
 				fmt.Sprintf("Error getting the list of keys: %s", err),
 				output,
 			)
-
-			return
 		}
 
 		if output != "" {
 			SuccessOutput(response.GetApiKeys(), "", output)
-
-			return
 		}
 
 		tableData := pterm.TableData{
@@ -102,8 +98,6 @@ var listAPIKeys = &cobra.Command{
 				fmt.Sprintf("Failed to render pterm table: %s", err),
 				output,
 			)
-
-			return
 		}
 	},
 }
@@ -119,9 +113,6 @@ If you loose a key, create a new one and revoke (expire) the old one.`,
 	Run: func(cmd *cobra.Command, args []string) {
 		output, _ := cmd.Flags().GetString("output")
 
-		log.Trace().
-			Msg("Preparing to create ApiKey")
-
 		request := &v1.CreateApiKeyRequest{}
 
 		durationStr, _ := cmd.Flags().GetString("expiration")
@@ -133,16 +124,10 @@ If you loose a key, create a new one and revoke (expire) the old one.`,
 				fmt.Sprintf("Could not parse duration: %s\n", err),
 				output,
 			)
-
-			return
 		}
 
 		expiration := time.Now().UTC().Add(time.Duration(duration))
 
-		log.Trace().
-			Dur("expiration", time.Duration(duration)).
-			Msg("expiration has been set")
-
 		request.Expiration = timestamppb.New(expiration)
 
 		ctx, client, conn, cancel := newHeadscaleCLIWithConfig()
@@ -156,8 +141,6 @@ If you loose a key, create a new one and revoke (expire) the old one.`,
 				fmt.Sprintf("Cannot create Api Key: %s\n", err),
 				output,
 			)
-
-			return
 		}
 
 		SuccessOutput(response.GetApiKey(), response.GetApiKey(), output)
@@ -178,8 +161,6 @@ var expireAPIKeyCmd = &cobra.Command{
 				fmt.Sprintf("Error getting prefix from CLI flag: %s", err),
 				output,
 			)
-
-			return
 		}
 
 		ctx, client, conn, cancel := newHeadscaleCLIWithConfig()
@@ -197,8 +178,6 @@ var expireAPIKeyCmd = &cobra.Command{
 				fmt.Sprintf("Cannot expire Api Key: %s\n", err),
 				output,
 			)
-
-			return
 		}
 
 		SuccessOutput(response, "Key expired", output)
@@ -219,8 +198,6 @@ var deleteAPIKeyCmd = &cobra.Command{
 				fmt.Sprintf("Error getting prefix from CLI flag: %s", err),
 				output,
 			)
-
-			return
 		}
 
 		ctx, client, conn, cancel := newHeadscaleCLIWithConfig()
@@ -238,8 +215,6 @@ var deleteAPIKeyCmd = &cobra.Command{
 				fmt.Sprintf("Cannot delete Api Key: %s\n", err),
 				output,
 			)
-
-			return
 		}
 
 		SuccessOutput(response, "Key deleted", output)

diff --git a/cmd/headscale/cli/debug.go b/cmd/headscale/cli/debug.go
@@ -64,8 +64,6 @@ var createNodeCmd = &cobra.Command{
 		user, err := cmd.Flags().GetString("user")
 		if err != nil {
 			ErrorOutput(err, fmt.Sprintf("Error getting user: %s", err), output)
-
-			return
 		}
 
 		ctx, client, conn, cancel := newHeadscaleCLIWithConfig()
@@ -79,8 +77,6 @@ var createNodeCmd = &cobra.Command{
 				fmt.Sprintf("Error getting node from flag: %s", err),
 				output,
 			)
-
-			return
 		}
 
 		machineKey, err := cmd.Flags().GetString("key")
@@ -90,8 +86,6 @@ var createNodeCmd = &cobra.Command{
 				fmt.Sprintf("Error getting key from flag: %s", err),
 				output,
 			)
-
-			return
 		}
 
 		var mkey key.MachinePublic
@@ -102,8 +96,6 @@ var createNodeCmd = &cobra.Command{
 				fmt.Sprintf("Failed to parse machine key from flag: %s", err),
 				output,
 			)
-
-			return
 		}
 
 		routes, err := cmd.Flags().GetStringSlice("route")
@@ -113,8 +105,6 @@ var createNodeCmd = &cobra.Command{
 				fmt.Sprintf("Error getting routes from flag: %s", err),
 				output,
 			)
-
-			return
 		}
 
 		request := &v1.DebugCreateNodeRequest{
@@ -131,8 +121,6 @@ var createNodeCmd = &cobra.Command{
 				fmt.Sprintf("Cannot create node: %s", status.Convert(err).Message()),
 				output,
 			)
-
-			return
 		}
 
 		SuccessOutput(response.GetNode(), "Node created", output)

diff --git a/cmd/headscale/cli/nodes.go b/cmd/headscale/cli/nodes.go
@@ -116,8 +116,6 @@ var registerNodeCmd = &cobra.Command{
 		user, err := cmd.Flags().GetString("user")
 		if err != nil {
 			ErrorOutput(err, fmt.Sprintf("Error getting user: %s", err), output)
-
-			return
 		}
 
 		ctx, client, conn, cancel := newHeadscaleCLIWithConfig()
@@ -131,8 +129,6 @@ var registerNodeCmd = &cobra.Command{
 				fmt.Sprintf("Error getting node key from flag: %s", err),
 				output,
 			)
-
-			return
 		}
 
 		request := &v1.RegisterNodeRequest{
@@ -150,8 +146,6 @@ var registerNodeCmd = &cobra.Command{
 				),
 				output,
 			)
-
-			return
 		}
 
 		SuccessOutput(
@@ -169,14 +163,10 @@ var listNodesCmd = &cobra.Command{
 		user, err := cmd.Flags().GetString("user")
 		if err != nil {
 			ErrorOutput(err, fmt.Sprintf("Error getting user: %s", err), output)
-
-			return
 		}
 		showTags, err := cmd.Flags().GetBool("tags")
 		if err != nil {
 			ErrorOutput(err, fmt.Sprintf("Error getting tags flag: %s", err), output)
-
-			return
 		}
 
 		ctx, client, conn, cancel := newHeadscaleCLIWithConfig()
@@ -194,21 +184,15 @@ var listNodesCmd = &cobra.Command{
 				fmt.Sprintf("Cannot get nodes: %s", status.Convert(err).Message()),
 				output,
 			)
-
-			return
 		}
 
 		if output != "" {
 			SuccessOutput(response.GetNodes(), "", output)
-
-			return
 		}
 
 		tableData, err := nodesToPtables(user, showTags, response.GetNodes())
 		if err != nil {
 			ErrorOutput(err, fmt.Sprintf("Error converting to table: %s", err), output)
-
-			return
 		}
 
 		err = pterm.DefaultTable.WithHasHeader().WithData(tableData).Render()
@@ -218,8 +202,6 @@ var listNodesCmd = &cobra.Command{
 				fmt.Sprintf("Failed to render pterm table: %s", err),
 				output,
 			)
-
-			return
 		}
 	},
 }

diff --git a/cmd/headscale/cli/policy.go b/cmd/headscale/cli/policy.go
@@ -1,6 +1,7 @@
 package cli
 
 import (
+	"fmt"
 	"io"
 	"os"
 
@@ -30,6 +31,7 @@ var getPolicy = &cobra.Command{
 	Short:   "Print the current ACL Policy",
 	Aliases: []string{"show", "view", "fetch"},
 	Run: func(cmd *cobra.Command, args []string) {
+		output, _ := cmd.Flags().GetString("output")
 		ctx, client, conn, cancel := newHeadscaleCLIWithConfig()
 		defer cancel()
 		defer conn.Close()
@@ -38,13 +40,13 @@ var getPolicy = &cobra.Command{
 
 		response, err := client.GetPolicy(ctx, request)
 		if err != nil {
-			log.Fatal().Err(err).Msg("Failed to get the policy")
-
-			return
+			ErrorOutput(err, fmt.Sprintf("Failed loading ACL Policy: %s", err), output)
 		}
 
 		// TODO(pallabpain): Maybe print this better?
-		SuccessOutput("", response.GetPolicy(), "hujson")
+		// This does not pass output as we dont support yaml, json or json-line
+		// output for this command. It is HuJSON already.
+		SuccessOutput("", response.GetPolicy(), "")
 	},
 }
 
@@ -56,21 +58,18 @@ var setPolicy = &cobra.Command{
 	This command only works when the acl.policy_mode is set to "db", and the policy will be stored in the database.`,
 	Aliases: []string{"put", "update"},
 	Run: func(cmd *cobra.Command, args []string) {
+		output, _ := cmd.Flags().GetString("output")
 		policyPath, _ := cmd.Flags().GetString("file")
 
 		f, err := os.Open(policyPath)
 		if err != nil {
-			log.Fatal().Err(err).Msg("Error opening the policy file")
-
-			return
+			ErrorOutput(err, fmt.Sprintf("Error opening the policy file: %s", err), output)
 		}
 		defer f.Close()
 
 		policyBytes, err := io.ReadAll(f)
 		if err != nil {
-			log.Fatal().Err(err).Msg("Error reading the policy file")
-
-			return
+			ErrorOutput(err, fmt.Sprintf("Error reading the policy file: %s", err), output)
 		}
 
 		request := &v1.SetPolicyRequest{Policy: string(policyBytes)}
@@ -80,9 +79,7 @@ var setPolicy = &cobra.Command{
 		defer conn.Close()
 
 		if _, err := client.SetPolicy(ctx, request); err != nil {
-			log.Fatal().Err(err).Msg("Failed to set ACL Policy")
-
-			return
+			ErrorOutput(err, fmt.Sprintf("Failed to set ACL Policy: %s", err), output)
 		}
 
 		SuccessOutput(nil, "Policy updated.", "")

diff --git a/cmd/headscale/cli/preauthkeys.go b/cmd/headscale/cli/preauthkeys.go
@@ -60,8 +60,6 @@ var listPreAuthKeys = &cobra.Command{
 		user, err := cmd.Flags().GetString("user")
 		if err != nil {
 			ErrorOutput(err, fmt.Sprintf("Error getting user: %s", err), output)
-
-			return
 		}
 
 		ctx, client, conn, cancel := newHeadscaleCLIWithConfig()
@@ -85,8 +83,6 @@ var listPreAuthKeys = &cobra.Command{
 
 		if output != "" {
 			SuccessOutput(response.GetPreAuthKeys(), "", output)
-
-			return
 		}
 
 		tableData := pterm.TableData{
@@ -134,8 +130,6 @@ var listPreAuthKeys = &cobra.Command{
 				fmt.Sprintf("Failed to render pterm table: %s", err),
 				output,
 			)
-
-			return
 		}
 	},
 }
@@ -150,20 +144,12 @@ var createPreAuthKeyCmd = &cobra.Command{
 		user, err := cmd.Flags().GetString("user")
 		if err != nil {
 			ErrorOutput(err, fmt.Sprintf("Error getting user: %s", err), output)
-
-			return
 		}
 
 		reusable, _ := cmd.Flags().GetBool("reusable")
 		ephemeral, _ := cmd.Flags().GetBool("ephemeral")
 		tags, _ := cmd.Flags().GetStringSlice("tags")
 
-		log.Trace().
-			Bool("reusable", reusable).
-			Bool("ephemeral", ephemeral).
-			Str("user", user).
-			Msg("Preparing to create preauthkey")
-
 		request := &v1.CreatePreAuthKeyRequest{
 			User:      user,
 			Reusable:  reusable,
@@ -180,8 +166,6 @@ var createPreAuthKeyCmd = &cobra.Command{
 				fmt.Sprintf("Could not parse duration: %s\n", err),
 				output,
 			)
-
-			return
 		}
 
 		expiration := time.Now().UTC().Add(time.Duration(duration))
@@ -203,8 +187,6 @@ var createPreAuthKeyCmd = &cobra.Command{
 				fmt.Sprintf("Cannot create Pre Auth Key: %s\n", err),
 				output,
 			)
-
-			return
 		}
 
 		SuccessOutput(response.GetPreAuthKey(), response.GetPreAuthKey().GetKey(), output)
@@ -227,8 +209,6 @@ var expirePreAuthKeyCmd = &cobra.Command{
 		user, err := cmd.Flags().GetString("user")
 		if err != nil {
 			ErrorOutput(err, fmt.Sprintf("Error getting user: %s", err), output)
-
-			return
 		}
 
 		ctx, client, conn, cancel := newHeadscaleCLIWithConfig()
@@ -247,8 +227,6 @@ var expirePreAuthKeyCmd = &cobra.Command{
 				fmt.Sprintf("Cannot expire Pre Auth Key: %s\n", err),
 				output,
 			)
-
-			return
 		}
 
 		SuccessOutput(response, "Key expired", output)

diff --git a/cmd/headscale/cli/root.go b/cmd/headscale/cli/root.go
@@ -58,11 +58,10 @@ func initConfig() {
 		zerolog.SetGlobalLevel(zerolog.Disabled)
 	}
 
-	logFormat := viper.GetString("log.format")
-
-	if logFormat == types.JSONLogFormat {
-		log.Logger = log.Output(os.Stdout)
-	}
+	// logFormat := viper.GetString("log.format")
+	// if logFormat == types.JSONLogFormat {
+	// 	log.Logger = log.Output(os.Stdout)
+	// }
 
 	disableUpdateCheck := viper.GetBool("disable_check_updates")
 	if !disableUpdateCheck && !machineOutput {