Merge branch 'UDENG-5696-initial-client-core-cli' into collector_wip

cmd/insights/commands/collect.go

@@ -1,7 +1,9 @@
 package commands
 
 import (
+	"fmt"
 	"log/slog"
+	"os"
 
 	"github.com/spf13/cobra"
 )
@@ -26,29 +28,49 @@ func installCollectCmd(app *App) error {
 	app.collectConfig = defaultCollectConfig
 
 	collectCmd := &cobra.Command{
-		Use:   "collect [SOURCE](required argument)",
+		Use:   "collect [SOURCE] [SOURCE-METRICS-PATH](required if source provided)",
 		Short: "Collect system information",
-		Long:  "Collect system information and metrics and store it locally",
-		Args:  cobra.ExactArgs(1),
+		Long: `Collect system information and metrics and store it locally.
+		If SOURCE is not provided, then it is the source is assumed to be the currently detected platform. Additionally, there should be no SOURCE-METRICS-PATH provided.
+		If SOURCE is provided, then the SOURCE-METRICS-PATH should be provided as well.`,
+		Args: func(cmd *cobra.Command, args []string) error {
+			if err := cobra.MaximumNArgs(2)(cmd, args); err != nil {
+				return err
+			}
+
+			if len(args) != 0 {
+				if err := cobra.MatchAll(cobra.OnlyValidArgs, cobra.ExactArgs(2))(cmd, args); err != nil {
+					return fmt.Errorf("accepts no args, or exactly 2 args, recieved 1")
+				}
+
+				fileInfo, err := os.Stat(args[1])
+				if err != nil {
+					return fmt.Errorf("the second argument, SOURCE-METRICS-PATH, should be a valid JSON file. Error: %s", err.Error())
+				}
+
+				if fileInfo.IsDir() {
+					return fmt.Errorf("the second argument, SOURCE-METRICS-PATH, should be a valid JSON file, not a directory.")
+				}
+			}
+
+			return nil
+		},
 		RunE: func(cmd *cobra.Command, args []string) error {
 			// Set Sources to Args
-			app.collectConfig.source = args[0]
+			if len(args) == 2 {
+				app.collectConfig.source = args[0]
+				app.collectConfig.extraMetrics = args[1]
+			}
 
 			slog.Info("Running collect command")
 
 			return nil
 		},
 	}
 
-	collectCmd.Flags().UintVarP(&app.collectConfig.period, "period", "p", 1, "The minimum period between 2 collection periods for validation purposes in seconds")
-	collectCmd.Flags().BoolVarP(&app.collectConfig.force, "force", "f", false, "Force a collection, override the report if there are any conflicts. (Doesn't ignore consent)")
-	collectCmd.Flags().BoolVarP(&app.collectConfig.dryRun, "dry-run", "d", false, "Perform a dry-run where a report is collected, but not written to disk")
-	collectCmd.Flags().StringVarP(&app.collectConfig.extraMetrics, "extra-metrics", "e", "", "Path to JSON file to append extra metrics from")
-
-	if err := collectCmd.MarkFlagFilename("extra-metrics", "json"); err != nil {
-		slog.Error("An error occurred while initializing the collect command.", "error", err.Error())
-		return err
-	}
+	collectCmd.Flags().UintVarP(&app.collectConfig.period, "period", "p", 1, "the minimum period between 2 collection periods for validation purposes in seconds")
+	collectCmd.Flags().BoolVarP(&app.collectConfig.force, "force", "f", false, "force a collection, override the report if there are any conflicts (doesn't ignore consent)")
+	collectCmd.Flags().BoolVarP(&app.collectConfig.dryRun, "dry-run", "d", false, "perform a dry-run where a report is collected, but not written to disk")
 
 	app.rootCmd.AddCommand(collectCmd)
 	return nil

cmd/insights/commands/consent.go

@@ -42,7 +42,7 @@ func installConsentCmd(app *App) {
 		},
 	}
 
-	consentCmd.Flags().StringVarP(&app.consentConfig.consentState, "consent-state", "c", "", "The consent state to set (true or false). If not set, the current consent state is displayed.")
+	consentCmd.Flags().StringVarP(&app.consentConfig.consentState, "consent-state", "c", "", "the consent state to set (true or false), the current consent state is displayed if not set")
 
 	app.rootCmd.AddCommand(consentCmd)
 }

cmd/insights/commands/root.go

@@ -26,17 +26,18 @@ type rootConfig struct {
 
 var defaultRootConfig = rootConfig{
 	Verbose:     false,
-	ConsentDir:  constants.DefaultConfigPath,
-	InsightsDir: constants.DefaultCachePath,
+	ConsentDir:  constants.GetDefaultConfigPath(),
+	InsightsDir: constants.GetDefaultCachePath(),
 }
 
 // Registers commands and returns a new app
 func New() (*App, error) {
 	a := App{}
 	a.rootCmd = &cobra.Command{
-		Use:   "ubuntu-insights [COMMAND]",
-		Short: "",
-		Long:  "",
+		Use:           "ubuntu-insights [COMMAND]",
+		Short:         "",
+		Long:          "",
+		SilenceErrors: true,
 		PersistentPreRunE: func(cmd *cobra.Command, args []string) error {
 			// Command parsing has been successful. Returns to not print usage anymore.
 			a.rootCmd.SilenceUsage = true
@@ -60,7 +61,7 @@ func installRootCmd(app *App) error {
 	app.rootConfig = defaultRootConfig
 
 	cmd.PersistentFlags().BoolVarP(&app.rootConfig.Verbose, "verbose", "v", app.rootConfig.Verbose, "enable verbose logging")
-	cmd.PersistentFlags().StringVar(&app.rootConfig.ConsentDir, "consent-dir", app.rootConfig.ConsentDir, "the base directory to look for consent state files in")
+	cmd.PersistentFlags().StringVar(&app.rootConfig.ConsentDir, "consent-dir", app.rootConfig.ConsentDir, "the base directory of the consent state files")
 	cmd.PersistentFlags().StringVar(&app.rootConfig.InsightsDir, "insights-dir", app.rootConfig.InsightsDir, "the base directory of the insights report cache")
 
 	if err := cmd.MarkPersistentFlagDirname("consent-dir"); err != nil {

cmd/insights/commands/upload.go

@@ -41,9 +41,9 @@ func installUploadCmd(app *App) {
 	}
 
 	uploadCmd.Flags().StringVar(&app.uploadConfig.server, "server", app.uploadConfig.server, "the base URL of the server to upload the metrics to")
-	uploadCmd.Flags().UintVar(&app.uploadConfig.minAge, "min-age", app.uploadConfig.minAge, "the minimum age of the metrics to upload in seconds")
-	uploadCmd.Flags().BoolVarP(&app.uploadConfig.force, "force", "f", app.uploadConfig.force, "force upload even if the period has not elapsed, overriding any conflicts")
-	uploadCmd.Flags().BoolVarP(&app.uploadConfig.dryRun, "dry-run", "d", app.uploadConfig.dryRun, "perform a dry run of the upload without sending the data to the server")
+	uploadCmd.Flags().UintVar(&app.uploadConfig.minAge, "min-age", app.uploadConfig.minAge, "the minimum age (in seconds) of a report before the uploader will attempt to upload it")
+	uploadCmd.Flags().BoolVarP(&app.uploadConfig.force, "force", "f", app.uploadConfig.force, "force an upload, ignoring min age and clashes between the collected file and a file in the uploaded folder, replacing the clashing uploaded report if it exists")
+	uploadCmd.Flags().BoolVarP(&app.uploadConfig.dryRun, "dry-run", "d", app.uploadConfig.dryRun, "go through the motions of doing an upload, but do not communicate with the server or send the payload")
 
 	app.rootCmd.AddCommand(uploadCmd)
 }

cmd/insights/main_test.go

@@ -0,0 +1,72 @@
+package main
+
+import (
+	"errors"
+	"testing"
+	"time"
+)
+
+type testApp struct {
+	done            chan struct{}
+	runError        bool
+	userErrorReturn bool
+}
+
+func (a *testApp) Run() error {
+	<-a.done
+	if a.runError {
+		return errors.New(("run error!"))
+	}
+	return nil
+}
+
+func (a testApp) UsageError() bool {
+	return a.userErrorReturn
+}
+
+func (a testApp) Quit() {
+	close(a.done)
+}
+
+func TestRun(t *testing.T) {
+	t.Parallel()
+
+	tests := map[string]struct {
+		runError   bool
+		usageError bool
+
+		wantReturnCode int
+	}{
+		"Run and exit successfully":                        {},
+		"Run and exit error":                               {runError: true, wantReturnCode: 1},
+		"Run and exit with usage error":                    {usageError: true, runError: true, wantReturnCode: 2},
+		"Run and return with usage error but no run error": {usageError: true, wantReturnCode: 0},
+	}
+
+	for name, tc := range tests {
+		t.Run(name, func(t *testing.T) {
+			a := testApp{
+				done:            make(chan struct{}),
+				runError:        tc.runError,
+				userErrorReturn: tc.usageError,
+			}
+
+			var rc int
+			wait := make(chan struct{})
+
+			go func() {
+				rc = run(&a)
+				close(wait)
+			}()
+
+			time.Sleep(100 * time.Millisecond)
+
+			a.Quit()
+			<-wait
+
+			if rc != tc.wantReturnCode {
+				t.Errorf("run() = %v, want %v", rc, tc.wantReturnCode)
+			}
+		})
+	}
+}

internal/constants/constants.go

@@ -14,24 +14,35 @@ const (
 	DefaultLogLevel = slog.LevelInfo
 )
 
-var (
-	// DefaultConfigPath is the default path to the configuration file
-	DefaultConfigPath = userConfigDir() + string(os.PathSeparator) + DefaultAppFolder
+type options struct {
+	baseDir func() (string, error)
+}
 
-	// DefaultCachePath is the default path to the cache directory
-	DefaultCachePath = userCacheDir() + string(os.PathSeparator) + DefaultAppFolder
-)
+type option func(*options)
 
-func userConfigDir() string {
-	dir, err := os.UserConfigDir()
-	if err != nil {
-		return ""
+// GetDefaultConfigPath is the default path to the configuration file
+func GetDefaultConfigPath(opts ...option) string {
+	o := options{baseDir: os.UserCacheDir}
+	for _, opt := range opts {
+		opt(&o)
 	}
-	return dir
+
+	return getBaseDir(o.baseDir) + string(os.PathSeparator) + DefaultAppFolder
+}
+
+// GetDefaultCachePath is the default path to the cache directory
+func GetDefaultCachePath(opts ...option) string {
+	o := options{baseDir: os.UserConfigDir}
+	for _, opt := range opts {
+		opt(&o)
+	}
+
+	return getBaseDir(o.baseDir) + string(os.PathSeparator) + DefaultAppFolder
 }
 
-func userCacheDir() string {
-	dir, err := os.UserCacheDir()
+// getBaseDir is a helper function to handle the case where the baseDir function returns an error, and instead return an empty string
+func getBaseDir(baseDirFunc func() (string, error)) string {
+	dir, err := baseDirFunc()
 	if err != nil {
 		return ""
 	}

internal/constants/constants_test.go

@@ -0,0 +1,82 @@
+package constants_test
+
+import (
+	"fmt"
+	"os"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	"github.com/ubuntu/ubuntu-insights/internal/constants"
+)
+
+func Test_GetUserConfigDir(t *testing.T) {
+	t.Parallel()
+
+	tests := map[string]struct {
+		want string
+		mock func() (string, error)
+	}{
+		"os.UserConfigDir success": {
+			want: "abc/def" + string(os.PathSeparator) + constants.DefaultAppFolder,
+			mock: func() (string, error) {
+				return "abc/def", nil
+			},
+		},
+		"os.UserConfigDir error": {
+			want: string(os.PathSeparator) + constants.DefaultAppFolder,
+			mock: func() (string, error) {
+				return "", fmt.Errorf("error")
+			},
+		},
+		"os.UserConfigDir error 2": {
+			want: string(os.PathSeparator) + constants.DefaultAppFolder,
+			mock: func() (string, error) {
+				return "abc", fmt.Errorf("error")
+			},
+		},
+	}
+	for name, tt := range tests {
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+
+			opts := []constants.Option{constants.WithBaseDir(tt.mock)}
+			require.Equal(t, tt.want, constants.GetDefaultConfigPath(opts...))
+		})
+	}
+}
+
+func Test_userCacheDir(t *testing.T) {
+	t.Parallel()
+
+	tests := map[string]struct {
+		want string
+		mock func() (string, error)
+	}{
+		"os.UserCacheDir success": {
+			want: "def/abc" + string(os.PathSeparator) + constants.DefaultAppFolder,
+			mock: func() (string, error) {
+				return "def/abc", nil
+			},
+		},
+		"os.UserCacheDir error": {
+			want: string(os.PathSeparator) + constants.DefaultAppFolder,
+			mock: func() (string, error) {
+				return "", fmt.Errorf("error")
+			},
+		},
+		"os.UserCacheDir error 2": {
+			want: string(os.PathSeparator) + constants.DefaultAppFolder,
+			mock: func() (string, error) {
+				return "abc", fmt.Errorf("error")
+			},
+		},
+	}
+	for name, tt := range tests {
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+
+			opts := []constants.Option{constants.WithBaseDir(tt.mock)}
+			require.Equal(t, tt.want, constants.GetDefaultCachePath(opts...))
+		})
+	}
+}

internal/constants/export_test.go

@@ -0,0 +1,9 @@
+package constants
+
+type Option = option
+
+func WithBaseDir(baseDir func() (string, error)) option {
+	return func(o *options) {
+		o.baseDir = baseDir
+	}
+}