From ca49f84708fbbb12d2072a9c18a63bac3ad79b6f Mon Sep 17 00:00:00 2001 From: Jens Feodor Nielsen Date: Mon, 9 Dec 2024 11:56:23 +0100 Subject: [PATCH] fix(cli): show proper usage message on errors * Show proper sub-commands usage text instead of the root usage when errors occur * Store CLI arguments in objects, to avoid accidentally referring to them within the packages * Extract repeated code for app identifier arguments * Move logic for refreshing access tokens to the `auth` package * Move logic for validating app identifiers into the `appident` package * Move organization creation wizard logic into the `cmd/organization/create` package to simplify * Split several commands logic, so that `cmd.go` contains the actual command definition --- cmd/app/list/cmd.go | 28 +-- cmd/app/share/cmd.go | 24 +-- cmd/app/unshare/cmd.go | 25 +-- cmd/args/appident.go | 19 +++ cmd/config/cmd.go | 28 +-- cmd/deletecmd/cmd.go | 22 ++- cmd/deletecmd/delete.go | 2 +- cmd/deletecmd/delete_test.go | 6 +- cmd/deploy/cmd.go | 45 ++--- cmd/deploy/deploy.go | 23 ++- cmd/deploy/deploy_test.go | 28 +-- cmd/download/cmd.go | 20 +-- cmd/init/cmd.go | 52 +++--- cmd/legacy/initialize/cmd.go | 24 +-- cmd/login/cmd.go | 27 +++ cmd/login/login.go | 43 ----- cmd/login/login_test.go | 71 +------- cmd/logout/cmd.go | 18 ++ cmd/logout/logout.go | 14 -- cmd/logs/cmd.go | 55 +++--- cmd/logs/logs_test.go | 4 + cmd/organization/cmd.go | 4 +- cmd/organization/create/cmd.go | 23 +++ .../create/organization_create.go | 20 +-- .../create/{wizard => }/wizard.go | 4 +- cmd/organization/list/cmd.go | 28 +++ cmd/organization/list/organization_list.go | 20 +-- .../list/organization_list_test.go | 4 +- cmd/root.go | 122 +++++++------- cmd/token/create/cmd.go | 10 +- cmd/token/revoke/cmd.go | 6 +- ...get_appidentifier.go => app_identifier.go} | 5 +- ...ntifier_test.go => app_identifier_test.go} | 0 .../appident/validate.go | 2 +- .../appident/validate_test.go | 2 +- internal/auth/user.go | 22 +++ internal/auth/user_test.go | 159 ++++++++++++------ 37 files changed, 544 insertions(+), 465 deletions(-) create mode 100644 cmd/args/appident.go create mode 100644 cmd/login/cmd.go create mode 100644 cmd/logout/cmd.go create mode 100644 cmd/organization/create/cmd.go rename cmd/organization/create/{wizard => }/wizard.go (84%) create mode 100644 cmd/organization/list/cmd.go rename internal/appident/{get_appidentifier.go => app_identifier.go} (95%) rename internal/appident/{get_appidentifier_test.go => app_identifier_test.go} (100%) rename cmd/validate/identifier.go => internal/appident/validate.go (88%) rename cmd/validate/identifier_test.go => internal/appident/validate_test.go (97%) diff --git a/cmd/app/list/cmd.go b/cmd/app/list/cmd.go index c3fb1301..1f3b4b1b 100644 --- a/cmd/app/list/cmd.go +++ b/cmd/app/list/cmd.go @@ -4,32 +4,36 @@ import ( "net/http" "github.com/spf13/cobra" + "numerous.com/cli/cmd/args" "numerous.com/cli/cmd/errorhandling" "numerous.com/cli/cmd/output" "numerous.com/cli/internal/app" "numerous.com/cli/internal/gql" ) -var argOrganizationSlug string +var cmdArgs struct{ organizationSlug string } var Cmd = &cobra.Command{ Use: "list", Short: "List all your apps (login required)", - RunE: func(cmd *cobra.Command, args []string) error { - if argOrganizationSlug == "" { - output.PrintError("Missing organization argument.", "") - cmd.Usage() // nolint:errcheck + RunE: func(cmd *cobra.Command, args []string) error { return run(cmd) }, +} + +func run(cmd *cobra.Command) error { + if cmdArgs.organizationSlug == "" { + output.PrintError("Missing organization argument.", "") + cmd.Usage() // nolint:errcheck - return errorhandling.ErrAlreadyPrinted - } + return errorhandling.ErrAlreadyPrinted + } - service := app.New(gql.NewClient(), nil, http.DefaultClient) - err := list(cmd.Context(), service, AppListInput{OrganizationSlug: argOrganizationSlug}) + service := app.New(gql.NewClient(), nil, http.DefaultClient) + err := list(cmd.Context(), service, AppListInput{OrganizationSlug: cmdArgs.organizationSlug}) - return errorhandling.ErrorAlreadyPrinted(err) - }, + return errorhandling.ErrorAlreadyPrinted(err) } func init() { - Cmd.Flags().StringVarP(&argOrganizationSlug, "organization", "o", "", "The organization slug identifier to list app from.") + flags := Cmd.Flags() + args.AddOrganizationSlugFlag(flags, "to list apps from", &cmdArgs.organizationSlug) } diff --git a/cmd/app/share/cmd.go b/cmd/app/share/cmd.go index 2cd3f80e..4899a3c8 100644 --- a/cmd/app/share/cmd.go +++ b/cmd/app/share/cmd.go @@ -19,25 +19,30 @@ const longFormat string = `Creates a shared URL for the specified app. %s ` -var long string = fmt.Sprintf(longFormat, usage.AppIdentifier("to create a shared URL for"), usage.AppDirectoryArgument) +var cmdActionText = "to create a shared URL for" + +var long string = fmt.Sprintf(longFormat, usage.AppIdentifier(cmdActionText), usage.AppDirectoryArgument) var Cmd = &cobra.Command{ Use: "share [app directory]", RunE: run, Short: "Create app shared URL", Long: long, - Args: args.OptionalAppDir(&appDir), + Args: args.OptionalAppDir(&cmdArgs.appDir), } -var ( - orgSlug string - appSlug string - appDir string -) +var cmdArgs struct { + appIdent args.AppIdentifierArg + appDir string +} func run(cmd *cobra.Command, args []string) error { service := app.New(gql.NewClient(), nil, http.DefaultClient) - input := Input{AppDir: appDir, AppSlug: appSlug, OrgSlug: orgSlug} + input := Input{ + AppDir: cmdArgs.appDir, + AppSlug: cmdArgs.appIdent.AppSlug, + OrgSlug: cmdArgs.appIdent.OrganizationSlug, + } err := shareApp(cmd.Context(), service, input) @@ -46,6 +51,5 @@ func run(cmd *cobra.Command, args []string) error { func init() { flags := Cmd.Flags() - flags.StringVarP(&orgSlug, "organization", "o", "", "The organization slug identifier of the app to create a shared URL for. List available organizations with 'numerous organization list'.") - flags.StringVarP(&appSlug, "app", "a", "", "An app slug identifier of the app to create a shared URL for.") + cmdArgs.appIdent.AddAppIdentifierFlags(flags, cmdActionText) } diff --git a/cmd/app/unshare/cmd.go b/cmd/app/unshare/cmd.go index 4df57fac..e0166e84 100644 --- a/cmd/app/unshare/cmd.go +++ b/cmd/app/unshare/cmd.go @@ -19,25 +19,31 @@ const longFormat string = `Removes a shared URL for the specified app. %s ` -var long string = fmt.Sprintf(longFormat, usage.AppIdentifier("to remove a shared URL for"), usage.AppDirectoryArgument) +var ( + cmdActionText = "to remove a shared URL for" + long string = fmt.Sprintf(longFormat, usage.AppIdentifier(cmdActionText), usage.AppDirectoryArgument) +) var Cmd = &cobra.Command{ Use: "unshare [app directory]", RunE: run, Short: "Remove app shared URL", Long: long, - Args: args.OptionalAppDir(&appDir), + Args: args.OptionalAppDir(&cmdArgs.appDir), } -var ( - orgSlug string - appSlug string - appDir string -) +var cmdArgs struct { + appIdent args.AppIdentifierArg + appDir string +} func run(cmd *cobra.Command, args []string) error { service := app.New(gql.NewClient(), nil, http.DefaultClient) - input := Input{AppDir: appDir, AppSlug: appSlug, OrgSlug: orgSlug} + input := Input{ + AppDir: cmdArgs.appDir, + AppSlug: cmdArgs.appIdent.AppSlug, + OrgSlug: cmdArgs.appIdent.OrganizationSlug, + } err := unshareApp(cmd.Context(), service, input) @@ -46,6 +52,5 @@ func run(cmd *cobra.Command, args []string) error { func init() { flags := Cmd.Flags() - flags.StringVarP(&orgSlug, "organization", "o", "", "The organization slug identifier of the app to remove a shared URL for. List available organizations with 'numerous organization list'.") - flags.StringVarP(&appSlug, "app", "a", "", "An app slug identifier of the app to remove a shared URL for.") + cmdArgs.appIdent.AddAppIdentifierFlags(flags, cmdActionText) } diff --git a/cmd/args/appident.go b/cmd/args/appident.go new file mode 100644 index 00000000..3d5e64be --- /dev/null +++ b/cmd/args/appident.go @@ -0,0 +1,19 @@ +package args + +import ( + "github.com/spf13/pflag" +) + +type AppIdentifierArg struct { + OrganizationSlug string + AppSlug string +} + +func (a *AppIdentifierArg) AddAppIdentifierFlags(flags *pflag.FlagSet, action string) { + AddOrganizationSlugFlag(flags, action, &a.OrganizationSlug) + flags.StringVarP(&a.AppSlug, "app", "a", "", "An app slug identifier of the app "+action+".") +} + +func AddOrganizationSlugFlag(flags *pflag.FlagSet, action string, orgSlug *string) { + flags.StringVarP(orgSlug, "organization", "o", "", "The organization slug identifier of the app "+action+". List available organizations with 'numerous organization list'.") +} diff --git a/cmd/config/cmd.go b/cmd/config/cmd.go index a3a5eb9e..7b0868bb 100644 --- a/cmd/config/cmd.go +++ b/cmd/config/cmd.go @@ -13,22 +13,24 @@ var Cmd = &cobra.Command{ Use: "config", Short: "Configure the Numerous CLI", Long: "Set configuration values, or print the entire configuration.", - RunE: func(cmd *cobra.Command, args []string) error { - if len(args) != 0 { - // do nothing if subcommand is running - return nil - } + RunE: run, +} + +func run(cmd *cobra.Command, args []string) error { + if len(args) != 0 { + // do nothing if subcommand is running + return nil + } - cfg := config.Config{} - if err := cfg.Load(); err != nil { - output.PrintErrorDetails("Error loading configuration", err) - return err - } + cfg := config.Config{} + if err := cfg.Load(); err != nil { + output.PrintErrorDetails("Error loading configuration", err) + return err + } - cfg.Print() + cfg.Print() - return nil - }, + return nil } func init() { diff --git a/cmd/deletecmd/cmd.go b/cmd/deletecmd/cmd.go index 1e39d3a5..a79e5ec9 100644 --- a/cmd/deletecmd/cmd.go +++ b/cmd/deletecmd/cmd.go @@ -23,7 +23,7 @@ var Cmd = &cobra.Command{ GroupID: group.AppCommandsGroupID, Long: long, Example: example, - Args: args.OptionalAppDir(&appDir), + Args: args.OptionalAppDir(&cmdArgs.appDir), } const longFormat = `Deletes the specified app from the organization. @@ -45,26 +45,24 @@ Otherwise, assuming an app has been initialized in the directory numerous delete my_project/my_app ` -var ( - orgSlug string - appSlug string - appDir string = "." -) +var cmdArgs struct { + appIdent args.AppIdentifierArg + appDir string +} -func run(cmd *cobra.Command, args []string) error { - if exists, _ := dir.AppIDExists(appDir); exists { +func run(cmd *cobra.Command, _ []string) error { + if exists, _ := dir.AppIDExists(cmdArgs.appDir); exists { output.NotifyCmdChanged("numerous delete", "numerous legacy delete") println() } service := app.New(gql.NewClient(), nil, http.DefaultClient) - err := Delete(cmd.Context(), service, appDir, orgSlug, appSlug) + err := deleteApp(cmd.Context(), service, cmdArgs.appDir, cmdArgs.appIdent.OrganizationSlug, cmdArgs.appIdent.AppSlug) return errorhandling.ErrorAlreadyPrinted(err) } func init() { - flags := Cmd.Flags() - flags.StringVarP(&orgSlug, "organization", "o", "", "The organization slug identifier of the app to read logs from.") - flags.StringVarP(&appSlug, "app", "a", "", "The app slug identifier of the app to read logs from.") + f := Cmd.Flags() + cmdArgs.appIdent.AddAppIdentifierFlags(f, "") } diff --git a/cmd/deletecmd/delete.go b/cmd/deletecmd/delete.go index 72f67de3..67fd5919 100644 --- a/cmd/deletecmd/delete.go +++ b/cmd/deletecmd/delete.go @@ -12,7 +12,7 @@ type AppService interface { Delete(ctx context.Context, input app.DeleteAppInput) error } -func Delete(ctx context.Context, apps AppService, appDir, orgSlug, appSlug string) error { +func deleteApp(ctx context.Context, apps AppService, appDir, orgSlug, appSlug string) error { ai, err := appident.GetAppIdentifier(appDir, nil, orgSlug, appSlug) if err != nil { appident.PrintGetAppIdentifierError(err, appDir, ai) diff --git a/cmd/deletecmd/delete_test.go b/cmd/deletecmd/delete_test.go index 4cfc057d..e866c64a 100644 --- a/cmd/deletecmd/delete_test.go +++ b/cmd/deletecmd/delete_test.go @@ -25,7 +25,7 @@ func TestDelete(t *testing.T) { expectedInput := app.DeleteAppInput{OrganizationSlug: "organization-slug-in-manifest", AppSlug: "app-slug-in-manifest"} service.On("Delete", mock.Anything, expectedInput).Return(nil) - err := Delete(context.TODO(), service, appDir, "", "") + err := deleteApp(context.TODO(), service, appDir, "", "") assert.NoError(t, err) }) @@ -36,7 +36,7 @@ func TestDelete(t *testing.T) { expectedInput := app.DeleteAppInput{OrganizationSlug: slug, AppSlug: appSlug} service.On("Delete", mock.Anything, expectedInput).Return(nil) - err := Delete(context.TODO(), service, appDir, slug, appSlug) + err := deleteApp(context.TODO(), service, appDir, slug, appSlug) assert.NoError(t, err) }) @@ -47,7 +47,7 @@ func TestDelete(t *testing.T) { expectedInput := app.DeleteAppInput{OrganizationSlug: slug, AppSlug: appSlug} service.On("Delete", mock.Anything, expectedInput).Return(testError) - err := Delete(context.TODO(), service, appDir, slug, appSlug) + err := deleteApp(context.TODO(), service, appDir, slug, appSlug) assert.ErrorIs(t, err, testError) }) diff --git a/cmd/deploy/cmd.go b/cmd/deploy/cmd.go index 8b98fe10..2ba04e45 100644 --- a/cmd/deploy/cmd.go +++ b/cmd/deploy/cmd.go @@ -24,7 +24,10 @@ organization's apps page. %s ` -var long string = fmt.Sprintf(longFormat, usage.AppIdentifier("to deploy"), usage.AppDirectoryArgument) +var ( + cmdActionText = "to deploy" + long string = fmt.Sprintf(longFormat, usage.AppIdentifier(cmdActionText), usage.AppDirectoryArgument) +) var Cmd = &cobra.Command{ Use: "deploy [app directory]", @@ -39,43 +42,41 @@ be pushed to the organization "organization-slug-a2ecf59b", and the app slug numerous deploy --organization "organization-slug-a2ecf59b" --app "my-app" `, - Args: args.OptionalAppDir(&appDir), + Args: args.OptionalAppDir(&cmdArgs.appDir), } -var ( - orgSlug string - appSlug string +var cmdArgs struct { + appIdent args.AppIdentifierArg verbose bool - appDir string = "." - projectDir string = "" + appDir string + projectDir string message string version string follow bool -) +} func run(cmd *cobra.Command, args []string) error { sc := gql.NewSubscriptionClient().WithSyncMode(true) service := app.New(gql.NewClient(), sc, http.DefaultClient) input := DeployInput{ - AppDir: appDir, - ProjectDir: projectDir, - OrgSlug: orgSlug, - AppSlug: appSlug, - Message: message, - Version: version, - Verbose: verbose, - Follow: follow, + AppDir: cmdArgs.appDir, + ProjectDir: cmdArgs.projectDir, + OrgSlug: cmdArgs.appIdent.OrganizationSlug, + AppSlug: cmdArgs.appIdent.AppSlug, + Message: cmdArgs.message, + Version: cmdArgs.version, + Verbose: cmdArgs.verbose, + Follow: cmdArgs.follow, } - err := Deploy(cmd.Context(), service, input) + err := deploy(cmd.Context(), service, input) return errorhandling.ErrorAlreadyPrinted(err) } func init() { flags := Cmd.Flags() - flags.StringVarP(&orgSlug, "organization", "o", "", "The organization slug identifier of the app to deploy to. List available organizations with 'numerous organization list'.") - flags.StringVarP(&appSlug, "app", "a", "", "A app slug identifier of the app to deploy to.") - flags.BoolVarP(&verbose, "verbose", "v", false, "Display detailed information about the app deployment.") - flags.BoolVarP(&follow, "follow", "f", false, "Follow app deployment logs after deployment has succeeded.") - flags.StringVarP(&projectDir, "project-dir", "p", "", "The project directory, which is the build context if using a custom Dockerfile.") + cmdArgs.appIdent.AddAppIdentifierFlags(flags, cmdActionText) + flags.BoolVarP(&cmdArgs.verbose, "verbose", "v", false, "Display detailed information about the app deployment.") + flags.BoolVarP(&cmdArgs.follow, "follow", "f", false, "Follow app deployment logs after deployment has succeeded.") + flags.StringVarP(&cmdArgs.projectDir, "project-dir", "p", "", "The project directory, which is the build context if using a custom Dockerfile.") } diff --git a/cmd/deploy/deploy.go b/cmd/deploy/deploy.go index 7034b705..897ecff0 100644 --- a/cmd/deploy/deploy.go +++ b/cmd/deploy/deploy.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "io" + "log/slog" "os" "path" "path/filepath" @@ -22,7 +23,10 @@ import ( const maxUploadBytes int64 = 5368709120 -var ErrArchiveTooLarge = fmt.Errorf("archive exceeds maximum size %d bytes", maxUploadBytes) +var ( + ErrArchiveTooLarge = fmt.Errorf("archive exceeds maximum size %d bytes", maxUploadBytes) + ErrAppDirOutOfBounds = errors.New("app directory out of bounds of project directory") +) type deployBuildError struct { Message string @@ -54,7 +58,7 @@ type DeployInput struct { Follow bool } -func Deploy(ctx context.Context, apps AppService, input DeployInput) error { +func deploy(ctx context.Context, apps AppService, input DeployInput) error { appRelativePath, err := findAppRelativePath(input) if err != nil { output.PrintError("Project directory %q must be a parent of app directory %q", "", input.ProjectDir, input.AppDir) @@ -75,12 +79,19 @@ func Deploy(ctx context.Context, apps AppService, input DeployInput) error { if err != nil { return err } - defer os.Remove(archive.Name()) if err := uploadAppArchive(ctx, apps, archive, appVersionOutput.AppVersionID); err != nil { return err } + if err := archive.Close(); err != nil { + slog.Error("Error closing temporary app archive", slog.String("error", err.Error())) + } + + if err := os.Remove(archive.Name()); err != nil { + slog.Error("Error removing temporary app archive", slog.String("error", err.Error())) + } + if err := deployApp(ctx, appVersionOutput, secrets, apps, input, appRelativePath); err != nil { return err } @@ -130,10 +141,14 @@ func findAppRelativePath(input DeployInput) (string, error) { } rel, err := filepath.Rel(absProjectDir, absAppDir) - if err != nil || strings.HasPrefix(rel, "..") { + if err != nil { return "", err } + if strings.HasPrefix(rel, "..") { + return "", ErrAppDirOutOfBounds + } + return filepath.ToSlash(rel), nil } diff --git a/cmd/deploy/deploy_test.go b/cmd/deploy/deploy_test.go index 04b111cb..8376700b 100644 --- a/cmd/deploy/deploy_test.go +++ b/cmd/deploy/deploy_test.go @@ -64,7 +64,7 @@ func TestDeploy(t *testing.T) { apps := mockAppNotExists() input := DeployInput{AppDir: appDir, OrgSlug: slug, AppSlug: appSlug} - err := Deploy(context.TODO(), apps, input) + err := deploy(context.TODO(), apps, input) assert.NoError(t, err) }) @@ -75,7 +75,7 @@ func TestDeploy(t *testing.T) { apps := mockAppExists() input := DeployInput{AppDir: appDir, OrgSlug: slug, AppSlug: appSlug} - err := Deploy(context.TODO(), apps, input) + err := deploy(context.TODO(), apps, input) assert.NoError(t, err) apps.AssertNotCalled(t, "Create") @@ -85,7 +85,7 @@ func TestDeploy(t *testing.T) { dir := t.TempDir() input := DeployInput{AppDir: dir, OrgSlug: slug, AppSlug: appSlug} - err := Deploy(context.TODO(), nil, input) + err := deploy(context.TODO(), nil, input) assert.EqualError(t, err, "open "+dir+"/numerous.toml: no such file or directory") }) @@ -95,7 +95,7 @@ func TestDeploy(t *testing.T) { test.CopyDir(t, "../../testdata/streamlit_app", appDir) input := DeployInput{AppDir: appDir, OrgSlug: "Some Invalid Organization Slug", AppSlug: appSlug} - err := Deploy(context.TODO(), nil, input) + err := deploy(context.TODO(), nil, input) assert.ErrorIs(t, err, appident.ErrInvalidOrganizationSlug) }) @@ -110,7 +110,7 @@ func TestDeploy(t *testing.T) { test.CopyDir(t, "../../testdata/streamlit_app_without_deploy", appDir) input := DeployInput{AppDir: appDir, AppSlug: appSlug} - err := Deploy(context.TODO(), nil, input) + err := deploy(context.TODO(), nil, input) assert.ErrorIs(t, err, appident.ErrMissingOrganizationSlug) }) @@ -121,7 +121,7 @@ func TestDeploy(t *testing.T) { apps := mockAppNotExists() input := DeployInput{AppDir: appDir, AppSlug: "app-slug-in-argument", OrgSlug: "organization-slug-in-argument"} - err := Deploy(context.TODO(), apps, input) + err := deploy(context.TODO(), apps, input) if assert.NoError(t, err) { apps.AssertCalled(t, "ReadApp", mock.Anything, app.ReadAppInput{OrganizationSlug: "organization-slug-in-argument", AppSlug: "app-slug-in-argument"}) @@ -134,7 +134,7 @@ func TestDeploy(t *testing.T) { test.CopyDir(t, "../../testdata/streamlit_app", appDir) input := DeployInput{AppDir: appDir, OrgSlug: "organization-slug", AppSlug: "Some Invalid App Name"} - err := Deploy(context.TODO(), nil, input) + err := deploy(context.TODO(), nil, input) assert.ErrorIs(t, err, appident.ErrInvalidAppSlug) }) @@ -145,7 +145,7 @@ func TestDeploy(t *testing.T) { apps := mockAppNotExists() input := DeployInput{AppDir: appDir, OrgSlug: "organization-slug"} - err := Deploy(context.TODO(), apps, input) + err := deploy(context.TODO(), apps, input) expectedAppSlug := "streamlit-app-without-deploy" if assert.NoError(t, err) { @@ -159,7 +159,7 @@ func TestDeploy(t *testing.T) { test.CopyDir(t, "../../testdata/streamlit_app", appDir) apps := mockAppNotExists() - err := Deploy(context.TODO(), apps, DeployInput{AppDir: appDir}) + err := deploy(context.TODO(), apps, DeployInput{AppDir: appDir}) if assert.NoError(t, err) { expectedInput := app.CreateAppInput{OrganizationSlug: "organization-slug-in-manifest", AppSlug: "app-slug-in-manifest", DisplayName: "Streamlit App With Deploy"} @@ -173,7 +173,7 @@ func TestDeploy(t *testing.T) { apps := mockAppNotExists() input := DeployInput{AppDir: appDir, OrgSlug: "organization-slug-in-argument", AppSlug: "app-slug-in-argument"} - err := Deploy(context.TODO(), apps, input) + err := deploy(context.TODO(), apps, input) if assert.NoError(t, err) { apps.AssertCalled(t, "ReadApp", mock.Anything, app.ReadAppInput{OrganizationSlug: "organization-slug-in-argument", AppSlug: "app-slug-in-argument"}) @@ -189,7 +189,7 @@ func TestDeploy(t *testing.T) { apps := mockAppExists() input := DeployInput{AppDir: appDir, OrgSlug: slug, AppSlug: appSlug, Version: expectedVersion, Message: expectedMessage} - err := Deploy(context.TODO(), apps, input) + err := deploy(context.TODO(), apps, input) if assert.NoError(t, err) { expectedInput := app.CreateAppVersionInput{ @@ -207,7 +207,7 @@ func TestDeploy(t *testing.T) { apps := mockAppExists() input := DeployInput{AppDir: appDir, OrgSlug: slug, AppSlug: appSlug} - err := Deploy(context.TODO(), apps, input) + err := deploy(context.TODO(), apps, input) if assert.NoError(t, err) { expectedInput := app.CreateAppVersionInput{AppID: appID} @@ -232,7 +232,7 @@ func TestDeploy(t *testing.T) { input := DeployInput{AppDir: appDir, OrgSlug: slug, AppSlug: appSlug, Version: expectedVersion, Message: expectedMessage, Verbose: true} stdoutR, err := test.RunEWithPatchedStdout(t, func() error { - return Deploy(context.TODO(), apps, input) + return deploy(context.TODO(), apps, input) }) assert.NoError(t, err) @@ -281,7 +281,7 @@ func TestDeploy(t *testing.T) { input := DeployInput{AppDir: appDir, OrgSlug: slug, AppSlug: appSlug, Version: expectedVersion, Message: expectedMessage, Verbose: true, Follow: true} stdoutR, err := test.RunEWithPatchedStdout(t, func() error { - return Deploy(context.TODO(), apps, input) + return deploy(context.TODO(), apps, input) }) assert.NoError(t, err) diff --git a/cmd/download/cmd.go b/cmd/download/cmd.go index 30addbf0..c5591942 100644 --- a/cmd/download/cmd.go +++ b/cmd/download/cmd.go @@ -30,21 +30,20 @@ var Cmd = &cobra.Command{ Short: "Download app sources", Long: long, GroupID: group.AppCommandsGroupID, - Args: args.OptionalAppDir(&appDir), + Args: args.OptionalAppDir(&cmdArgs.appDir), } -var ( - orgSlug string - appSlug string - appDir string -) +var cmdArgs struct { + appIdent args.AppIdentifierArg + appDir string +} func run(cmd *cobra.Command, args []string) error { service := app.New(gql.NewClient(), nil, http.DefaultClient) input := Input{ - AppDir: appDir, - AppSlug: appSlug, - OrgSlug: orgSlug, + AppDir: cmdArgs.appDir, + AppSlug: cmdArgs.appIdent.AppSlug, + OrgSlug: cmdArgs.appIdent.OrganizationSlug, } err := Download(cmd.Context(), http.DefaultClient, service, input, surveyConfirmOverwrite) @@ -54,6 +53,5 @@ func run(cmd *cobra.Command, args []string) error { func init() { flags := Cmd.Flags() - flags.StringVarP(&orgSlug, "organization", "o", "", "The organization slug identifier of the app to download. List available organizations with 'numerous organization list'.") - flags.StringVarP(&appSlug, "app", "a", "", "A app slug identifier of the app to download.") + cmdArgs.appIdent.AddAppIdentifierFlags(flags, "to download") } diff --git a/cmd/init/cmd.go b/cmd/init/cmd.go index 6173f887..d4bb3b9d 100644 --- a/cmd/init/cmd.go +++ b/cmd/init/cmd.go @@ -24,16 +24,16 @@ var Cmd = &cobra.Command{ RunE: run, } -var ( - argName string - argDesc string - argLibraryKey string - argAppFile string - argRequirementsFile string - argDockerfile string - argDockerContext string - argDockerPort uint -) +var cmdArgs struct { + name string + description string + libraryKey string + appFilePath string + requirementsFilePath string + dockerfilePath string + dockerContextPath string + dockerPort uint +} var ( ErrGetWorkDir = errors.New("error getting working directory") @@ -52,14 +52,14 @@ func run(cmd *cobra.Command, args []string) error { params := InitializeParams{ AppDir: appDir, - Name: argName, - Desc: argDesc, - LibraryKey: argLibraryKey, - AppFile: argAppFile, - RequirementsFile: argRequirementsFile, - Dockerfile: argDockerfile, - DockerContext: argDockerContext, - DockerPort: int(argDockerPort), + Name: cmdArgs.name, + Desc: cmdArgs.description, + LibraryKey: cmdArgs.libraryKey, + AppFile: cmdArgs.appFilePath, + RequirementsFile: cmdArgs.requirementsFilePath, + Dockerfile: cmdArgs.dockerfilePath, + DockerContext: cmdArgs.dockerContextPath, + DockerPort: int(cmdArgs.dockerPort), } _, err = Initialize(&wizard.SurveyAsker{}, params) if errors.Is(err, wizard.ErrStopInit) { @@ -92,16 +92,16 @@ Next steps: } func init() { - Cmd.Flags().StringVarP(&argName, "name", "n", "", "Name of the app") - Cmd.Flags().StringVarP(&argDesc, "description", "d", "", "Description of the app") - Cmd.Flags().StringVarP(&argLibraryKey, "app-library", "t", "", "Library the app is made with") + Cmd.Flags().StringVarP(&cmdArgs.name, "name", "n", "", "Name of the app") + Cmd.Flags().StringVarP(&cmdArgs.description, "description", "d", "", "Description of the app") + Cmd.Flags().StringVarP(&cmdArgs.libraryKey, "app-library", "t", "", "Library the app is made with") - Cmd.Flags().StringVarP(&argAppFile, "app-file", "f", "", "Path to the entrypoint module of the python app") - Cmd.Flags().StringVarP(&argRequirementsFile, "requirements-file", "r", "", "Path to the requirements file of the python app") + Cmd.Flags().StringVarP(&cmdArgs.appFilePath, "app-file", "f", "", "Path to the entrypoint module of the python app") + Cmd.Flags().StringVarP(&cmdArgs.requirementsFilePath, "requirements-file", "r", "", "Path to the requirements file of the python app") - Cmd.Flags().StringVar(&argDockerfile, "dockerfile", "", "Path to the Dockerfile for the app") - Cmd.Flags().StringVar(&argDockerContext, "docker-context", "", "Path used as the context for building the app Dockerfile") - Cmd.Flags().UintVar(&argDockerPort, "docker-port", 0, "The port exposed in the Dockerfile") + Cmd.Flags().StringVar(&cmdArgs.dockerfilePath, "dockerfile", "", "Path to the Dockerfile for the app") + Cmd.Flags().StringVar(&cmdArgs.dockerContextPath, "docker-context", "", "Path used as the context for building the app Dockerfile") + Cmd.Flags().UintVar(&cmdArgs.dockerPort, "docker-port", 0, "The port exposed in the Dockerfile") } func PathArgumentHandler(providedPath string, currentPath string) string { diff --git a/cmd/legacy/initialize/cmd.go b/cmd/legacy/initialize/cmd.go index e4c96181..a7cc1db7 100644 --- a/cmd/legacy/initialize/cmd.go +++ b/cmd/legacy/initialize/cmd.go @@ -24,13 +24,13 @@ var InitCmd = &cobra.Command{ Run: run, } -var ( +var cmdArgs struct { name string desc string libraryKey string appFile string requirementsFile string -) +} func run(cmd *cobra.Command, args []string) { appDir, err := os.Getwd() @@ -53,11 +53,11 @@ func run(cmd *cobra.Command, args []string) { } params := cmdinit.InitializeParams{ - Name: name, - Desc: desc, - LibraryKey: libraryKey, - AppFile: appFile, - RequirementsFile: requirementsFile, + Name: cmdArgs.name, + Desc: cmdArgs.desc, + LibraryKey: cmdArgs.libraryKey, + AppFile: cmdArgs.appFile, + RequirementsFile: cmdArgs.requirementsFile, AppDir: appDir, } m, err := cmdinit.Initialize(&wizard.SurveyAsker{}, params) @@ -102,9 +102,9 @@ If you are logged in, you can use numerous list to find the App ID again. } func init() { - InitCmd.Flags().StringVarP(&name, "name", "n", "", "Name of the app") - InitCmd.Flags().StringVarP(&desc, "description", "d", "", "Description of your app") - InitCmd.Flags().StringVarP(&libraryKey, "app-library", "t", "", "Library the app is made with") - InitCmd.Flags().StringVarP(&appFile, "app-file", "f", "", "Path to that main file of the project") - InitCmd.Flags().StringVarP(&requirementsFile, "requirements-file", "r", "", "Requirements file of the project") + InitCmd.Flags().StringVarP(&cmdArgs.name, "name", "n", "", "Name of the app") + InitCmd.Flags().StringVarP(&cmdArgs.desc, "description", "d", "", "Description of your app") + InitCmd.Flags().StringVarP(&cmdArgs.libraryKey, "app-library", "t", "", "Library the app is made with") + InitCmd.Flags().StringVarP(&cmdArgs.appFile, "app-file", "f", "", "Path to that main file of the project") + InitCmd.Flags().StringVarP(&cmdArgs.requirementsFile, "requirements-file", "r", "", "Requirements file of the project") } diff --git a/cmd/login/cmd.go b/cmd/login/cmd.go new file mode 100644 index 00000000..5774289d --- /dev/null +++ b/cmd/login/cmd.go @@ -0,0 +1,27 @@ +package login + +import ( + "github.com/spf13/cobra" + "numerous.com/cli/cmd/errorhandling" + "numerous.com/cli/cmd/group" + "numerous.com/cli/cmd/output" + "numerous.com/cli/internal/auth" +) + +var Cmd = &cobra.Command{ + Use: "login", + Short: "Login to the Numerous CLI", + Args: cobra.NoArgs, + GroupID: group.AdditionalCommandsGroupID, + RunE: func(cmd *cobra.Command, args []string) error { + user := auth.NumerousTenantAuthenticator.GetLoggedInUserFromKeyring() + if user != nil { + output.PrintlnOK("Great, you are already logged in!") + return nil + } + + _, err := login(auth.NumerousTenantAuthenticator, cmd.Context()) + + return errorhandling.ErrorAlreadyPrinted(err) + }, +} diff --git a/cmd/login/login.go b/cmd/login/login.go index 689ea97f..6a862fb0 100644 --- a/cmd/login/login.go +++ b/cmd/login/login.go @@ -6,32 +6,10 @@ import ( "fmt" "net/http" - "numerous.com/cli/cmd/errorhandling" - "numerous.com/cli/cmd/group" "numerous.com/cli/cmd/output" "numerous.com/cli/internal/auth" - - "github.com/spf13/cobra" ) -var Cmd = &cobra.Command{ - Use: "login", - Short: "Login to the Numerous CLI", - Args: cobra.NoArgs, - GroupID: group.AdditionalCommandsGroupID, - RunE: func(cmd *cobra.Command, args []string) error { - user := auth.NumerousTenantAuthenticator.GetLoggedInUserFromKeyring() - if user != nil { - output.PrintlnOK("Great, you are already logged in!") - return nil - } - - _, err := login(auth.NumerousTenantAuthenticator, cmd.Context()) - - return errorhandling.ErrorAlreadyPrinted(err) - }, -} - const loggingInMessage = `You are logging into Numerous. When you press Enter, a browser window will automatically open. @@ -97,24 +75,3 @@ func login(a auth.Authenticator, ctx context.Context) (*auth.User, error) { return a.GetLoggedInUserFromKeyring(), nil } - -func RefreshAccessToken(user *auth.User, client *http.Client, a auth.Authenticator) error { - if err := user.CheckAuthenticationStatus(); err != auth.ErrExpiredToken { - if err != nil { - return err - } - - return nil - } - - newAccessToken, err := a.RegenerateAccessToken(client, user.RefreshToken) - if err != nil { - return err - } - if err := a.StoreAccessToken(newAccessToken); err != nil { - return err - } - user.AccessToken = newAccessToken - - return nil -} diff --git a/cmd/login/login_test.go b/cmd/login/login_test.go index a7fa71e6..f4d9aa2f 100644 --- a/cmd/login/login_test.go +++ b/cmd/login/login_test.go @@ -2,25 +2,17 @@ package login import ( "context" - "fmt" - "net/http" "testing" "time" "numerous.com/cli/internal/auth" - "numerous.com/cli/internal/test" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" - "github.com/stretchr/testify/require" -) - -var ( - testTenant = "test.domain.com" - testIssuer = fmt.Sprintf("https://%s/", testTenant) ) func TestLogin(t *testing.T) { + testTenant := "test.domain.com" state := auth.DeviceCodeState{ DeviceCode: "some-code", UserCode: "some-long-user-code", @@ -56,64 +48,3 @@ func TestLogin(t *testing.T) { m.AssertExpectations(t) assert.Equal(t, expectedUser, acutalUser) } - -func TestRefreshAccessToken(t *testing.T) { - t.Run("Refreshes access token if it has expired", func(t *testing.T) { - // Create test tokens - refreshToken := "refresh-token" - accessToken := test.GenerateJWT(t, testIssuer, time.Now().Add(-time.Hour)) - expectedNewAccessToken := test.GenerateJWT(t, testIssuer, time.Now().Add(time.Hour)) - - user := &auth.User{ - AccessToken: accessToken, - RefreshToken: refreshToken, - Tenant: testTenant, - } - - // Mock function from authenticator - client := &http.Client{} - m := new(auth.MockAuthenticator) - m.On("RegenerateAccessToken", client, refreshToken).Return(expectedNewAccessToken, nil) - m.On("StoreAccessToken", expectedNewAccessToken).Return(nil) - - // Execute function we test - err := RefreshAccessToken(user, client, m) - require.NoError(t, err) - - assert.Equal(t, expectedNewAccessToken, user.AccessToken) - m.AssertExpectations(t) - }) - - t.Run("Does not refresh if user not logged in", func(t *testing.T) { - var user *auth.User - m := new(auth.MockAuthenticator) - client := &http.Client{} - err := RefreshAccessToken(user, client, m) - - require.EqualError(t, err, auth.ErrUserNotLoggedIn.Error()) - m.AssertExpectations(t) - }) - - t.Run("Does not refresh if access token has not expired", func(t *testing.T) { - // Create test tokens - refreshToken := "refresh-token" - accessToken := test.GenerateJWT(t, testIssuer, time.Now().Add(time.Hour)) - - user := &auth.User{ - AccessToken: accessToken, - RefreshToken: refreshToken, - Tenant: testTenant, - } - - // Mock function from authenticator - client := &http.Client{} - m := new(auth.MockAuthenticator) - - // Execute function we test - err := RefreshAccessToken(user, client, m) - require.NoError(t, err) - - assert.Equal(t, user.AccessToken, accessToken) - m.AssertExpectations(t) - }) -} diff --git a/cmd/logout/cmd.go b/cmd/logout/cmd.go new file mode 100644 index 00000000..06553f75 --- /dev/null +++ b/cmd/logout/cmd.go @@ -0,0 +1,18 @@ +package logout + +import ( + "github.com/spf13/cobra" + "numerous.com/cli/cmd/errorhandling" + "numerous.com/cli/cmd/group" + "numerous.com/cli/internal/auth" +) + +var Cmd = &cobra.Command{ + Use: "logout", + Short: "Logout of the Numerous CLI", + GroupID: group.AdditionalCommandsGroupID, + RunE: func(cmd *cobra.Command, args []string) error { + err := logout(auth.NumerousTenantAuthenticator) + return errorhandling.ErrorAlreadyPrinted(err) + }, +} diff --git a/cmd/logout/logout.go b/cmd/logout/logout.go index 01aedeec..e9e1e6ab 100644 --- a/cmd/logout/logout.go +++ b/cmd/logout/logout.go @@ -3,24 +3,10 @@ package logout import ( "net/http" - "numerous.com/cli/cmd/errorhandling" - "numerous.com/cli/cmd/group" "numerous.com/cli/cmd/output" "numerous.com/cli/internal/auth" - - "github.com/spf13/cobra" ) -var Cmd = &cobra.Command{ - Use: "logout", - Short: "Logout of the Numerous CLI", - GroupID: group.AdditionalCommandsGroupID, - RunE: func(cmd *cobra.Command, args []string) error { - err := logout(auth.NumerousTenantAuthenticator) - return errorhandling.ErrorAlreadyPrinted(err) - }, -} - func logout(a auth.Authenticator) error { user := a.GetLoggedInUserFromKeyring() if user == nil { diff --git a/cmd/logs/cmd.go b/cmd/logs/cmd.go index 75ced828..b2a9a518 100644 --- a/cmd/logs/cmd.go +++ b/cmd/logs/cmd.go @@ -1,12 +1,14 @@ package logs import ( + "fmt" "net/http" "numerous.com/cli/cmd/args" "numerous.com/cli/cmd/errorhandling" "numerous.com/cli/cmd/group" "numerous.com/cli/cmd/output" + "numerous.com/cli/cmd/usage" "numerous.com/cli/internal/app" "numerous.com/cli/internal/dir" "numerous.com/cli/internal/gql" @@ -14,23 +16,18 @@ import ( "github.com/spf13/cobra" ) -var Cmd = &cobra.Command{ - Use: "logs [app directory]", - RunE: run, - Short: "Display running application logs", - GroupID: group.AppCommandsGroupID, - Long: `Read the logs of an application deployed to an organization on the +const longFormat string = `Read the logs of an application deployed to an organization on the Numerous platform. -If and flags are set, they define the app to read logs -from. If they are not, the default deployment section in the manifest is used, -if it is defined. +%s + +%s +` +const cmdActionText string = "to read logs from" -If [app directory] is specified, that directory will be used to read the -app manifest for the default deployment information. +var long string = fmt.Sprintf(longFormat, usage.AppIdentifier(cmdActionText), usage.AppDirectoryArgument) -If no [app directory] is specified, the current working directory is used.`, - Example: `To read the logs from a specific app deployment, use the following form: +const example string = `To read the logs from a specific app deployment, use the following form: numerous logs --organization "organization-slug-a2ecf59b" --app "my-app" @@ -38,42 +35,48 @@ Otherwise, assuming an app has been initialized in the directory "my_project/my_app" and has a default deployment defined in its manifest: numerous logs my_project/my_app -`, - Args: args.OptionalAppDir(&appDir), +` + +var Cmd = &cobra.Command{ + Use: "logs [app directory]", + RunE: run, + Short: "Display running application logs", + GroupID: group.AppCommandsGroupID, + Long: long, + Example: example, + Args: args.OptionalAppDir(&cmdArgs.appDir), } -var ( - orgSlug string - appSlug string +var cmdArgs struct { + appIdent args.AppIdentifierArg timestamps bool - appDir string = "." -) + appDir string +} func run(cmd *cobra.Command, args []string) error { // TODO: this is just here for users who expect the "old" log command in // this location, which will primarily be for apps initialized with an App // ID file - if exists, _ := dir.AppIDExists(appDir); exists { + if exists, _ := dir.AppIDExists(cmdArgs.appDir); exists { output.NotifyCmdMoved("numerous log", "numerous legacy log") println() } var printer func(app.AppDeployLogEntry) - if timestamps { + if cmdArgs.timestamps { printer = TimestampPrinter } else { printer = TextPrinter } sc := gql.NewSubscriptionClient().WithSyncMode(true) service := app.New(gql.NewClient(), sc, http.DefaultClient) - err := Logs(cmd.Context(), service, appDir, orgSlug, appSlug, printer) + err := Logs(cmd.Context(), service, cmdArgs.appDir, cmdArgs.appIdent.OrganizationSlug, cmdArgs.appIdent.AppSlug, printer) return errorhandling.ErrorAlreadyPrinted(err) } func init() { flags := Cmd.Flags() - flags.StringVarP(&orgSlug, "organization", "o", "", "The organization slug identifier of the app to read logs from.") - flags.StringVarP(&appSlug, "app", "a", "", "The app slug identifier of the app to read logs from.") - flags.BoolVarP(×tamps, "timestamps", "t", false, "Print a timestamp for each log entry.") + cmdArgs.appIdent.AddAppIdentifierFlags(flags, cmdActionText) + flags.BoolVarP(&cmdArgs.timestamps, "timestamps", "t", false, "Print a timestamp for each log entry.") } diff --git a/cmd/logs/logs_test.go b/cmd/logs/logs_test.go index f8713512..24aa245c 100644 --- a/cmd/logs/logs_test.go +++ b/cmd/logs/logs_test.go @@ -23,12 +23,16 @@ func TestLogs(t *testing.T) { testError := errors.New("test error") t.Run("given invalid slug then it returns error", func(t *testing.T) { + appDir := t.TempDir() + err := Logs(context.TODO(), nil, appDir, "Some Invalid Organization Slug", appSlug, dummyPrinter) assert.ErrorIs(t, err, appident.ErrInvalidOrganizationSlug) }) t.Run("given invalid app slug then it returns error", func(t *testing.T) { + appDir := t.TempDir() + err := Logs(context.TODO(), nil, appDir, slug, "Some Invalid App Name", dummyPrinter) assert.ErrorIs(t, err, appident.ErrInvalidAppSlug) diff --git a/cmd/organization/cmd.go b/cmd/organization/cmd.go index d73edc8f..8d99a69a 100644 --- a/cmd/organization/cmd.go +++ b/cmd/organization/cmd.go @@ -17,6 +17,6 @@ var Cmd = &cobra.Command{ } func init() { - Cmd.AddCommand(create.OrganizationCreateCmd) - Cmd.AddCommand(list.OrganizationListCmd) + Cmd.AddCommand(create.Cmd) + Cmd.AddCommand(list.Cmd) } diff --git a/cmd/organization/create/cmd.go b/cmd/organization/create/cmd.go new file mode 100644 index 00000000..0e2f7c4d --- /dev/null +++ b/cmd/organization/create/cmd.go @@ -0,0 +1,23 @@ +package create + +import ( + "github.com/spf13/cobra" + "numerous.com/cli/cmd/errorhandling" + "numerous.com/cli/cmd/output" + "numerous.com/cli/internal/auth" + "numerous.com/cli/internal/gql" +) + +var Cmd = &cobra.Command{ + Use: "create", + Short: "Creates an organization (login required)", + Long: "The organization feature is a structured space to keep the apps that you work on with team members. Organizations help to arrange your apps, manage who has access to them, and simplify the workflow with your team.", + RunE: func(cmd *cobra.Command, args []string) error { + err := organizationCreate(auth.NumerousTenantAuthenticator, gql.GetClient()) + if err != nil { + output.PrintErrorDetails("Error creating organization", err) + } + + return errorhandling.ErrorAlreadyPrinted(err) + }, +} diff --git a/cmd/organization/create/organization_create.go b/cmd/organization/create/organization_create.go index 02afb48c..67c5242f 100644 --- a/cmd/organization/create/organization_create.go +++ b/cmd/organization/create/organization_create.go @@ -4,32 +4,14 @@ import ( "errors" "fmt" - "numerous.com/cli/cmd/errorhandling" - "numerous.com/cli/cmd/organization/create/wizard" "numerous.com/cli/cmd/output" "numerous.com/cli/internal/auth" - "numerous.com/cli/internal/gql" "numerous.com/cli/internal/gql/organization" "git.sr.ht/~emersion/gqlclient" - "github.com/spf13/cobra" ) -var OrganizationCreateCmd = &cobra.Command{ - Use: "create", - Short: "Creates an organization (login required)", - Long: "The organization feature is a structured space to keep the apps that you work on with team members. Organizations help to arrange your apps, manage who has access to them, and simplify the workflow with your team.", - RunE: func(cmd *cobra.Command, args []string) error { - err := organizationCreate(auth.NumerousTenantAuthenticator, gql.GetClient()) - if err != nil { - output.PrintErrorDetails("Error creating organization", err) - } - - return errorhandling.ErrorAlreadyPrinted(err) - }, -} - func organizationCreate(a auth.Authenticator, c *gqlclient.Client) error { user := a.GetLoggedInUserFromKeyring() if user == nil { @@ -41,7 +23,7 @@ func organizationCreate(a auth.Authenticator, c *gqlclient.Client) error { wizard: for { - if err := wizard.RunOrganizationCreateWizard(&newOrganization.Name, *user); err != nil { + if err := runWizard(&newOrganization.Name, *user); err != nil { return err } diff --git a/cmd/organization/create/wizard/wizard.go b/cmd/organization/create/wizard.go similarity index 84% rename from cmd/organization/create/wizard/wizard.go rename to cmd/organization/create/wizard.go index 4e24d4ba..4ec5132d 100644 --- a/cmd/organization/create/wizard/wizard.go +++ b/cmd/organization/create/wizard.go @@ -1,4 +1,4 @@ -package wizard +package create import ( "numerous.com/cli/internal/auth" @@ -6,7 +6,7 @@ import ( "github.com/AlecAivazis/survey/v2" ) -func RunOrganizationCreateWizard(name *string, user auth.User) error { +func runWizard(name *string, user auth.User) error { questions := []*survey.Question{ { Name: "Name", diff --git a/cmd/organization/list/cmd.go b/cmd/organization/list/cmd.go new file mode 100644 index 00000000..a4c4bf13 --- /dev/null +++ b/cmd/organization/list/cmd.go @@ -0,0 +1,28 @@ +package list + +import ( + "github.com/spf13/cobra" + "numerous.com/cli/cmd/errorhandling" + "numerous.com/cli/internal/auth" + "numerous.com/cli/internal/gql" +) + +var cmdArgs = struct { + displayMode DisplayMode +}{ + displayMode: DisplayModeList, +} + +var Cmd = &cobra.Command{ + Use: "list", + Short: "List all your organizations (login required)", + RunE: func(cmd *cobra.Command, args []string) error { + err := list(auth.NumerousTenantAuthenticator, gql.GetClient(), cmdArgs.displayMode) + return errorhandling.ErrorAlreadyPrinted(err) + }, +} + +func init() { + flags := Cmd.Flags() + flags.VarP(&cmdArgs.displayMode, "display-mode", "d", "Display mode. Display organizations as a list or as a table. (\"list\", \"table\")") +} diff --git a/cmd/organization/list/organization_list.go b/cmd/organization/list/organization_list.go index 9a85f8a3..26ddf3ad 100644 --- a/cmd/organization/list/organization_list.go +++ b/cmd/organization/list/organization_list.go @@ -1,33 +1,15 @@ package list import ( - "numerous.com/cli/cmd/errorhandling" "numerous.com/cli/cmd/output" "numerous.com/cli/internal/auth" "numerous.com/cli/internal/config" - "numerous.com/cli/internal/gql" "numerous.com/cli/internal/gql/user" "git.sr.ht/~emersion/gqlclient" - "github.com/spf13/cobra" ) -var displayMode DisplayMode = DisplayModeList - -var OrganizationListCmd = &cobra.Command{ - Use: "list", - Short: "List all your organizations (login required)", - RunE: func(cmd *cobra.Command, args []string) error { - err := list(auth.NumerousTenantAuthenticator, gql.GetClient()) - return errorhandling.ErrorAlreadyPrinted(err) - }, -} - -func init() { - OrganizationListCmd.Flags().VarP(&displayMode, "display-mode", "d", "Display mode. Display organizations as a list or as a table. (\"list\", \"table\")") -} - -func list(a auth.Authenticator, g *gqlclient.Client) error { +func list(a auth.Authenticator, g *gqlclient.Client, displayMode DisplayMode) error { u := a.GetLoggedInUserFromKeyring() if u == nil { output.PrintErrorLogin() diff --git a/cmd/organization/list/organization_list_test.go b/cmd/organization/list/organization_list_test.go index 3fe25b93..3425f157 100644 --- a/cmd/organization/list/organization_list_test.go +++ b/cmd/organization/list/organization_list_test.go @@ -55,7 +55,7 @@ func TestList(t *testing.T) { }` c, transportMock := test.CreateMockGQLClient(response) - err := list(m, c) + err := list(m, c, DisplayModeList) assert.NoError(t, err) m.AssertExpectations(t) @@ -69,7 +69,7 @@ func TestList(t *testing.T) { c, transportMock := test.CreateMockGQLClient() - err := list(m, c) + err := list(m, c, DisplayModeList) assert.NoError(t, err) m.AssertExpectations(t) diff --git a/cmd/root.go b/cmd/root.go index aeb95d19..90a6a8e1 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -20,11 +20,11 @@ import ( "numerous.com/cli/cmd/organization" "numerous.com/cli/cmd/output" "numerous.com/cli/cmd/token" - "numerous.com/cli/cmd/version" + cmdversion "numerous.com/cli/cmd/version" "numerous.com/cli/internal/auth" "numerous.com/cli/internal/gql" "numerous.com/cli/internal/logging" - ver "numerous.com/cli/internal/version" + "numerous.com/cli/internal/version" "github.com/spf13/cobra" ) @@ -34,56 +34,59 @@ var ( ErrIncompatibleVersion = errors.New("incompatible version") ) -var ( - logLevel logging.Level = logging.LevelError - rootCmd = &cobra.Command{ - Use: "numerous", - Long: "\n ~~~ \n" + - " --- ~~~~~~~ \n" + - " ° ------- ~~~~~~~~~~ \n" + - " °°°° ----------- ~~~~~~~~~\n" + - " °°°°°°° ----------- ~~~~~~~~ _ _ \n" + - " °°°°°°°°°° ------- ~~~~~~~~ | \\ | | \n" + - " °°°°°°°°°°°°° ----- ~~~~~~~ | \\| |_ _ _ __ ___ ___ _ __ ___ _ _ ___\n" + - " °°°°°°°°°°°°° ----- ~~~~~~~ | . ` | | | | '_ ` _ \\ / _ \\ '__/ _ \\| | | / __|\n" + - " °°°°°°°°°°°°° ----- ~~~~ | |\\ | |_| | | | | | | __/ | | (_) | |_| \\__ \\\n" + - " °°°°°°°°°°°°° ---- ~~ |_| \\_|\\__,_|_| |_| |_|\\___|_| \\___/ \\__,_|___/\n" + - " °°°°°°°°°° --\n" + - " °°°°°°°° \n" + - " °°°°° \n" + - " °° \n" + - "", - SilenceErrors: true, - SilenceUsage: true, - PersistentPreRunE: func(cmd *cobra.Command, args []string) error { - output.NotifyFeedbackMaybe() - - if !version.Check(ver.NewService(gql.NewClient())) { - return errorhandling.ErrorAlreadyPrinted(ErrIncompatibleVersion) - } - - if !commandRequiresAuthentication(cmd.CommandPath()) { - return nil - } - - if os.Getenv("NUMEROUS_ACCESS_TOKEN") != "" { - return nil - } - - user := auth.NumerousTenantAuthenticator.GetLoggedInUserFromKeyring() - if user.CheckAuthenticationStatus() == auth.ErrUserNotLoggedIn { - output.PrintErrorLoginForCommand(cmd) - return ErrNotAuthorized - } - - if err := login.RefreshAccessToken(user, http.DefaultClient, auth.NumerousTenantAuthenticator); err != nil { - return err - } +var args = struct { + logLevel logging.Level +}{ + logLevel: logging.LevelError, +} +var cmd = &cobra.Command{ + Use: "numerous", + Long: "\n ~~~ \n" + + " --- ~~~~~~~ \n" + + " ° ------- ~~~~~~~~~~ \n" + + " °°°° ----------- ~~~~~~~~~\n" + + " °°°°°°° ----------- ~~~~~~~~ _ _ \n" + + " °°°°°°°°°° ------- ~~~~~~~~ | \\ | | \n" + + " °°°°°°°°°°°°° ----- ~~~~~~~ | \\| |_ _ _ __ ___ ___ _ __ ___ _ _ ___\n" + + " °°°°°°°°°°°°° ----- ~~~~~~~ | . ` | | | | '_ ` _ \\ / _ \\ '__/ _ \\| | | / __|\n" + + " °°°°°°°°°°°°° ----- ~~~~ | |\\ | |_| | | | | | | __/ | | (_) | |_| \\__ \\\n" + + " °°°°°°°°°°°°° ---- ~~ |_| \\_|\\__,_|_| |_| |_|\\___|_| \\___/ \\__,_|___/\n" + + " °°°°°°°°°° --\n" + + " °°°°°°°° \n" + + " °°°°° \n" + + " °° \n" + + "", + SilenceErrors: true, + SilenceUsage: true, + PersistentPreRunE: func(cmd *cobra.Command, args []string) error { + output.NotifyFeedbackMaybe() + + if !cmdversion.Check(version.NewService(gql.NewClient())) { + return errorhandling.ErrorAlreadyPrinted(ErrIncompatibleVersion) + } + + if !commandRequiresAuthentication(cmd.CommandPath()) { return nil - }, - } -) + } + + if os.Getenv("NUMEROUS_ACCESS_TOKEN") != "" { + return nil + } + + user := auth.NumerousTenantAuthenticator.GetLoggedInUserFromKeyring() + if user.CheckAuthenticationStatus() == auth.ErrUserNotLoggedIn { + output.PrintErrorLoginForCommand(cmd) + return ErrNotAuthorized + } + + if err := user.RefreshAccessToken(http.DefaultClient, auth.NumerousTenantAuthenticator); err != nil { + return err + } + + return nil + }, +} func commandRequiresAuthentication(invokedCommandName string) bool { commandsWithAuthRequired := []string{ @@ -114,30 +117,30 @@ func commandRequiresAuthentication(invokedCommandName string) bool { } func Execute() { - err := rootCmd.Execute() + executedCmd, err := cmd.ExecuteC() if err != nil { if !errors.Is(err, errorhandling.ErrAlreadyPrinted) { output.PrintError("Error: %s", "", err.Error()) println() - rootCmd.Usage() // nolint: errcheck + executedCmd.Usage() // nolint: errcheck } os.Exit(1) } } func init() { - rootCmd.PersistentFlags().VarP(&logLevel, "log-level", "l", "The log level, one of \"debug\", \"info\", \"warning\", or \"error\". Defaults to \"error\".") + cmd.PersistentFlags().VarP(&args.logLevel, "log-level", "l", "The log level, one of \"debug\", \"info\", \"warning\", or \"error\". Defaults to \"error\".") - rootCmd.AddGroup(&cobra.Group{ + cmd.AddGroup(&cobra.Group{ Title: "Numerous App Commands:", ID: "app-cmds", }) - rootCmd.AddGroup(&cobra.Group{ + cmd.AddGroup(&cobra.Group{ Title: "Additional Numerous Commands:", ID: "additional-cmds", }) - rootCmd.AddCommand( + cmd.AddCommand( cmdinit.Cmd, login.Cmd, logout.Cmd, @@ -148,7 +151,7 @@ func init() { logs.Cmd, download.Cmd, token.Cmd, - version.Cmd, + cmdversion.Cmd, app.Cmd, config.Cmd, @@ -161,7 +164,10 @@ func init() { ) cobra.OnInitialize(func() { - slog.SetDefault(slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: logLevel.ToSlogLevel()}))) + logOpts := &slog.HandlerOptions{Level: args.logLevel.ToSlogLevel()} + logHandler := slog.NewTextHandler(os.Stderr, logOpts) + logger := slog.New(logHandler) + slog.SetDefault(logger) }) } diff --git a/cmd/token/create/cmd.go b/cmd/token/create/cmd.go index ccb2c681..ed8bb04a 100644 --- a/cmd/token/create/cmd.go +++ b/cmd/token/create/cmd.go @@ -7,22 +7,22 @@ import ( "numerous.com/cli/internal/token" ) -var ( +var cmdArgs struct { name string desc string -) +} var Cmd = &cobra.Command{ Use: "create", Short: "Create a personal access token.", RunE: func(cmd *cobra.Command, args []string) error { - err := Create(cmd.Context(), token.NewService(gql.NewClient()), CreateInput{Name: name, Description: desc}) + err := Create(cmd.Context(), token.NewService(gql.NewClient()), CreateInput{Name: cmdArgs.name, Description: cmdArgs.desc}) return errorhandling.ErrorAlreadyPrinted(err) }, } func init() { flags := Cmd.Flags() - flags.StringVarP(&name, "name", "n", "", "The name of the personal access token. Must be unique, and no longer than 40 characters.") - flags.StringVarP(&desc, "description", "d", "", "The description of the personal access token.") + flags.StringVarP(&cmdArgs.name, "name", "n", "", "The name of the personal access token. Must be unique, and no longer than 40 characters.") + flags.StringVarP(&cmdArgs.desc, "description", "d", "", "The description of the personal access token.") } diff --git a/cmd/token/revoke/cmd.go b/cmd/token/revoke/cmd.go index 2dea904f..7388d429 100644 --- a/cmd/token/revoke/cmd.go +++ b/cmd/token/revoke/cmd.go @@ -7,18 +7,18 @@ import ( "numerous.com/cli/internal/token" ) -var id string +var cmdArgs struct{ id string } var Cmd = &cobra.Command{ Use: "revoke", Short: "Revoke a personal access token.", RunE: func(cmd *cobra.Command, args []string) error { - err := Revoke(cmd.Context(), token.NewService(gql.NewClient()), id) + err := Revoke(cmd.Context(), token.NewService(gql.NewClient()), cmdArgs.id) return errorhandling.ErrorAlreadyPrinted(err) }, } func init() { flags := Cmd.Flags() - flags.StringVarP(&id, "id", "", "", "The id of the personal access token.") + flags.StringVarP(&cmdArgs.id, "id", "", "", "The id of the personal access token.") } diff --git a/internal/appident/get_appidentifier.go b/internal/appident/app_identifier.go similarity index 95% rename from internal/appident/get_appidentifier.go rename to internal/appident/app_identifier.go index ad81d0cd..f1e4b24c 100644 --- a/internal/appident/get_appidentifier.go +++ b/internal/appident/app_identifier.go @@ -6,7 +6,6 @@ import ( "regexp" "strings" - "numerous.com/cli/cmd/validate" "numerous.com/cli/internal/config" "numerous.com/cli/internal/manifest" ) @@ -25,7 +24,7 @@ func (ai AppIdentifier) validate() error { return ErrMissingOrganizationSlug } - if !validate.IsValidIdentifier(ai.OrganizationSlug) { + if !IsValidIdentifier(ai.OrganizationSlug) { return ErrInvalidOrganizationSlug } @@ -33,7 +32,7 @@ func (ai AppIdentifier) validate() error { return ErrMissingAppSlug } - if !validate.IsValidIdentifier(ai.AppSlug) { + if !IsValidIdentifier(ai.AppSlug) { return ErrInvalidAppSlug } diff --git a/internal/appident/get_appidentifier_test.go b/internal/appident/app_identifier_test.go similarity index 100% rename from internal/appident/get_appidentifier_test.go rename to internal/appident/app_identifier_test.go diff --git a/cmd/validate/identifier.go b/internal/appident/validate.go similarity index 88% rename from cmd/validate/identifier.go rename to internal/appident/validate.go index 92477f2e..0950d4ae 100644 --- a/cmd/validate/identifier.go +++ b/internal/appident/validate.go @@ -1,4 +1,4 @@ -package validate +package appident import "regexp" diff --git a/cmd/validate/identifier_test.go b/internal/appident/validate_test.go similarity index 97% rename from cmd/validate/identifier_test.go rename to internal/appident/validate_test.go index 25d97e7d..64d25a09 100644 --- a/cmd/validate/identifier_test.go +++ b/internal/appident/validate_test.go @@ -1,4 +1,4 @@ -package validate +package appident import ( "testing" diff --git a/internal/auth/user.go b/internal/auth/user.go index 1aec6634..67e8e141 100644 --- a/internal/auth/user.go +++ b/internal/auth/user.go @@ -3,6 +3,7 @@ package auth import ( "errors" "fmt" + "net/http" "time" "numerous.com/cli/internal/keyring" @@ -65,6 +66,27 @@ func (u *User) HasExpiredToken() bool { return tokenExpired(token) } +func (u *User) RefreshAccessToken(client *http.Client, a Authenticator) error { + if err := u.CheckAuthenticationStatus(); err != ErrExpiredToken { + if err != nil { + return err + } + + return nil + } + + newAccessToken, err := a.RegenerateAccessToken(client, u.RefreshToken) + if err != nil { + return err + } + if err := a.StoreAccessToken(newAccessToken); err != nil { + return err + } + u.AccessToken = newAccessToken + + return nil +} + func validateToken(t jwt.Token, tenantName string) error { err := jwt.Validate(t, jwt.WithIssuer(fmt.Sprintf("https://%s/", tenantName))) switch err { diff --git a/internal/auth/user_test.go b/internal/auth/user_test.go index da496426..dfdf0a0a 100644 --- a/internal/auth/user_test.go +++ b/internal/auth/user_test.go @@ -2,6 +2,7 @@ package auth import ( "fmt" + "net/http" "testing" "time" @@ -16,6 +17,7 @@ import ( func TestUser(t *testing.T) { const testTenant string = "numerous-test.eu.com" testIssuer := fmt.Sprintf("https://%s/", testTenant) + t.Run("Can get logged in user, based on keyring", func(t *testing.T) { goKeyring.MockInit() expectedUser := User{ @@ -38,71 +40,134 @@ func TestUser(t *testing.T) { assert.Nil(t, actualUser) }) - t.Run("CheckAuthenticationStatus when user is logged in with wrong issuer returns error", func(t *testing.T) { - goKeyring.MockInit() + t.Run("CheckAuthenticationStatus", func(t *testing.T) { + t.Run("when user is logged in with wrong issuer returns error", func(t *testing.T) { + goKeyring.MockInit() - accessToken := test.GenerateJWT(t, "https://bad-issuer.com/", time.Now().Add(time.Hour)) - storeTestTokens(t, testTenant, accessToken, "refresh-token") + accessToken := test.GenerateJWT(t, "https://bad-issuer.com/", time.Now().Add(time.Hour)) + storeTestTokens(t, testTenant, accessToken, "refresh-token") - user := getLoggedInUserFromKeyring(testTenant) - actualError := user.CheckAuthenticationStatus() + user := getLoggedInUserFromKeyring(testTenant) + actualError := user.CheckAuthenticationStatus() - require.EqualError(t, actualError, "\"iss\" not satisfied: values do not match") - }) + require.EqualError(t, actualError, "\"iss\" not satisfied: values do not match") + }) - t.Run("CheckAuthenticationStatus with expired accessToken returns error", func(t *testing.T) { - goKeyring.MockInit() + t.Run("with expired accessToken returns error", func(t *testing.T) { + goKeyring.MockInit() - accessToken := test.GenerateJWT(t, testIssuer, time.Now().Add(-time.Hour)) - storeTestTokens(t, testTenant, accessToken, "refresh-token") + accessToken := test.GenerateJWT(t, testIssuer, time.Now().Add(-time.Hour)) + storeTestTokens(t, testTenant, accessToken, "refresh-token") - user := getLoggedInUserFromKeyring(testTenant) - actualError := user.CheckAuthenticationStatus() + user := getLoggedInUserFromKeyring(testTenant) + actualError := user.CheckAuthenticationStatus() - require.EqualError(t, actualError, ErrExpiredToken.Error()) - }) + require.EqualError(t, actualError, ErrExpiredToken.Error()) + }) - t.Run("CheckAuthenticationStatus when user is logged in returns nil", func(t *testing.T) { - goKeyring.MockInit() + t.Run("when user is logged in returns nil", func(t *testing.T) { + goKeyring.MockInit() - accessToken := test.GenerateJWT(t, testIssuer, time.Now().Add(time.Hour)) - storeTestTokens(t, testTenant, accessToken, "refresh-token") + accessToken := test.GenerateJWT(t, testIssuer, time.Now().Add(time.Hour)) + storeTestTokens(t, testTenant, accessToken, "refresh-token") - user := getLoggedInUserFromKeyring(testTenant) - actualError := user.CheckAuthenticationStatus() + user := getLoggedInUserFromKeyring(testTenant) + actualError := user.CheckAuthenticationStatus() - require.NoError(t, actualError) + require.NoError(t, actualError) + }) }) - hasExpiredTestCases := []struct { - name string - expiration time.Time - expectedHasExpired bool - }{ - { - name: "User.HasExpired returns false when expiration of token is after current time", - expiration: time.Now().Add(2 * time.Hour), - expectedHasExpired: false, - }, - { - name: "User.HasExpired returns true when expiration of token is before current time", - expiration: time.Now().Add(-2 * time.Hour), - expectedHasExpired: true, - }, - } - for _, testCase := range hasExpiredTestCases { - t.Run(testCase.name, func(t *testing.T) { - goKeyring.MockInit() + t.Run("HasExpired", func(t *testing.T) { + type testCase struct { + name string + expiration time.Time + expectedHasExpired bool + } + + for _, testCase := range []testCase{ + { + name: "User.HasExpired returns false when expiration of token is after current time", + expiration: time.Now().Add(2 * time.Hour), + expectedHasExpired: false, + }, + { + name: "User.HasExpired returns true when expiration of token is before current time", + expiration: time.Now().Add(-2 * time.Hour), + expectedHasExpired: true, + }, + } { + t.Run(testCase.name, func(t *testing.T) { + goKeyring.MockInit() + + accessToken := test.GenerateJWT(t, testIssuer, testCase.expiration) + user := User{ + AccessToken: accessToken, + RefreshToken: "refresh-token", + } + + assert.Equal(t, testCase.expectedHasExpired, user.HasExpiredToken()) + }) + } + }) + + t.Run("TestRefreshAccessToken", func(t *testing.T) { + t.Run("Refreshes access token if it has expired", func(t *testing.T) { + refreshToken := "refresh-token" + accessToken := test.GenerateJWT(t, testIssuer, time.Now().Add(-time.Hour)) + expectedNewAccessToken := test.GenerateJWT(t, testIssuer, time.Now().Add(time.Hour)) + + user := &User{ + AccessToken: accessToken, + RefreshToken: refreshToken, + Tenant: testTenant, + } + + client := &http.Client{} + m := new(MockAuthenticator) + m.On("RegenerateAccessToken", client, refreshToken).Return(expectedNewAccessToken, nil) + m.On("StoreAccessToken", expectedNewAccessToken).Return(nil) + + err := user.RefreshAccessToken(client, m) + require.NoError(t, err) + + assert.Equal(t, expectedNewAccessToken, user.AccessToken) + m.AssertExpectations(t) + }) + + t.Run("Does not refresh if user not logged in", func(t *testing.T) { + var user *User + m := new(MockAuthenticator) + client := &http.Client{} + err := user.RefreshAccessToken(client, m) + + assert.ErrorIs(t, err, ErrUserNotLoggedIn) + m.AssertExpectations(t) + }) - accessToken := test.GenerateJWT(t, testIssuer, testCase.expiration) - user := User{ + t.Run("Does not refresh if access token has not expired", func(t *testing.T) { + // Create test tokens + refreshToken := "refresh-token" + accessToken := test.GenerateJWT(t, testIssuer, time.Now().Add(time.Hour)) + + user := &User{ AccessToken: accessToken, - RefreshToken: "refresh-token", + RefreshToken: refreshToken, + Tenant: testTenant, } - assert.Equal(t, testCase.expectedHasExpired, user.HasExpiredToken()) + // Mock function from authenticator + client := &http.Client{} + m := new(MockAuthenticator) + + // Execute function we test + err := user.RefreshAccessToken(client, m) + require.NoError(t, err) + + assert.Equal(t, user.AccessToken, accessToken) + m.AssertExpectations(t) }) - } + }) } func storeTestTokens(t *testing.T, tenant, accessToken, refreshToken string) {