From 32b24c6833e4266880fe1386f0d6f2a6139f115a Mon Sep 17 00:00:00 2001 From: lizrabuya <115472349+lizrabuya@users.noreply.github.com> Date: Tue, 16 Dec 2025 17:46:29 +1100 Subject: [PATCH 1/7] Move configure cmd to Kong --- cmd/configure/add.go | 113 ++++++++++++++++++++++++++ cmd/configure/add_test.go | 157 +++++++++++++++++++++++++++++++++++++ cmd/configure/configure.go | 52 ++++++++++++ 3 files changed, 322 insertions(+) create mode 100644 cmd/configure/add.go create mode 100644 cmd/configure/add_test.go create mode 100644 cmd/configure/configure.go diff --git a/cmd/configure/add.go b/cmd/configure/add.go new file mode 100644 index 00000000..d538fa64 --- /dev/null +++ b/cmd/configure/add.go @@ -0,0 +1,113 @@ +package configure + +import ( + "bufio" + "context" + "errors" + "fmt" + "os" + "strings" + "syscall" + + "github.com/alecthomas/kong" + "github.com/buildkite/cli/v3/internal/cli" + "github.com/buildkite/cli/v3/pkg/cmd/factory" + "golang.org/x/term" +) + +type AddCmd struct{} + +func (c *AddCmd) Help() string { + return ` +Examples: + # Add configuration for a new organization + $ bk configure add +` +} + +func (c *AddCmd) Run(kongCtx *kong.Context, globals cli.GlobalFlags) error { + f, err := factory.New() + if err != nil { + return err + } + + f.SkipConfirm = globals.SkipConfirmation() + f.NoInput = globals.DisableInput() + f.Quiet = globals.IsQuiet() + + ctx := context.Background() + + return ConfigureRun(ctx, f) +} + +func ConfigureWithCredentials(ctx context.Context, f *factory.Factory, org, token string) error { + if err := f.Config.SelectOrganization(org, f.GitRepository != nil); err != nil { + return err + } + + return f.Config.SetTokenForOrg(org, token) +} + +func ConfigureRun(ctx context.Context, f *factory.Factory) error { + // Check if we're in a Git repository + if f.GitRepository == nil { + return errors.New("not in a Git repository - bk should be configured at the root of a Git repository") + } + + // Get organization slug + org, err := promptForInput("Organization slug: ", false) + if err != nil { + return err + } + if org == "" { + return errors.New("organization slug cannot be empty") + } + + // Check if token already exists for this organization + existingToken := getTokenForOrg(f, org) + if existingToken != "" { + fmt.Printf("Using existing API token for organization: %s\n", org) + return f.Config.SelectOrganization(org, f.GitRepository != nil) + } + + // Get API token with password input (no echo) + token, err := promptForInput("API Token: ", true) + if err != nil { + return err + } + if token == "" { + return errors.New("API token cannot be empty") + } + + fmt.Println("API token set for organization:", org) + return ConfigureWithCredentials(ctx, f, org, token) +} + +// getTokenForOrg retrieves the token for a specific organization from the user config +func getTokenForOrg(f *factory.Factory, org string) string { + return f.Config.GetTokenForOrg(org) +} + +// promptForInput handles terminal input with optional password masking +func promptForInput(prompt string, isPassword bool) (string, error) { + fmt.Print(prompt) + + if isPassword { + // Use term.ReadPassword for secure password input + passwordBytes, err := term.ReadPassword(int(syscall.Stdin)) + fmt.Println() // Add a newline after password input + if err != nil { + return "", err + } + return string(passwordBytes), nil + } else { + // Use standard input for regular text + reader := bufio.NewReader(os.Stdin) + input, err := reader.ReadString('\n') + if err != nil { + return "", err + } + // Trim whitespace and newlines + return strings.TrimSpace(input), nil + } +} diff --git a/cmd/configure/add_test.go b/cmd/configure/add_test.go new file mode 100644 index 00000000..c55144e7 --- /dev/null +++ b/cmd/configure/add_test.go @@ -0,0 +1,157 @@ +package configure + +import ( + "testing" + + "github.com/buildkite/cli/v3/internal/config" + "github.com/buildkite/cli/v3/pkg/cmd/factory" + "github.com/spf13/afero" +) + +func TestGetTokenForOrg(t *testing.T) { + t.Parallel() + + t.Run("returns empty string when no token exists", func(t *testing.T) { + t.Parallel() + fs := afero.NewMemMapFs() + conf := config.New(fs, nil) + f := &factory.Factory{Config: conf} + + token := getTokenForOrg(f, "nonexistent") + if token != "" { + t.Errorf("expected empty string, got %s", token) + } + }) + + t.Run("returns token when it exists for organization", func(t *testing.T) { + t.Parallel() + fs := afero.NewMemMapFs() + conf := config.New(fs, nil) + f := &factory.Factory{Config: conf} + + // Set up a token for an organization + expectedToken := "bk_test_token_12345" + conf.SetTokenForOrg("test-org", expectedToken) + + token := getTokenForOrg(f, "test-org") + if token != expectedToken { + t.Errorf("expected %s, got %s", expectedToken, token) + } + }) + + t.Run("returns different tokens for different organizations", func(t *testing.T) { + t.Parallel() + fs := afero.NewMemMapFs() + conf := config.New(fs, nil) + f := &factory.Factory{Config: conf} + + // Set up tokens for different organizations + token1 := "bk_test_token_org1" + token2 := "bk_test_token_org2" + conf.SetTokenForOrg("org1", token1) + conf.SetTokenForOrg("org2", token2) + + if getTokenForOrg(f, "org1") != token1 { + t.Errorf("expected %s for org1", token1) + } + if getTokenForOrg(f, "org2") != token2 { + t.Errorf("expected %s for org2", token2) + } + }) +} + +func TestConfigureWithCredentials(t *testing.T) { + t.Parallel() + + t.Run("configures organization and token", func(t *testing.T) { + t.Parallel() + fs := afero.NewMemMapFs() + conf := config.New(fs, nil) + f := &factory.Factory{Config: conf} + + org := "test-org" + token := "bk_test_token_12345" + + err := ConfigureWithCredentials(f, org, token) + if err != nil { + t.Errorf("expected no error, got %s", err) + } + + if conf.OrganizationSlug() != org { + t.Errorf("expected organization to be %s, got %s", org, conf.OrganizationSlug()) + } + + if conf.GetTokenForOrg(org) != token { + t.Errorf("expected token to be %s, got %s", token, conf.GetTokenForOrg(org)) + } + }) +} + +func TestConfigureTokenReuse(t *testing.T) { + t.Parallel() + + t.Run("reuses existing token when available", func(t *testing.T) { + t.Parallel() + fs := afero.NewMemMapFs() + conf := config.New(fs, nil) + f := &factory.Factory{Config: conf} + + org := "test-org" + existingToken := "bk_existing_token_12345" + + // Pre-configure a token for the organization + conf.SetTokenForOrg(org, existingToken) + + // Verify the token can be retrieved + retrievedToken := getTokenForOrg(f, org) + if retrievedToken != existingToken { + t.Errorf("expected to retrieve existing token %s, got %s", existingToken, retrievedToken) + } + + // Configure with the existing token (simulating the logic in ConfigureRun) + err := ConfigureWithCredentials(f, org, retrievedToken) + if err != nil { + t.Errorf("expected no error, got %s", err) + } + + // Verify the configuration still works + if conf.OrganizationSlug() != org { + t.Errorf("expected organization to be %s, got %s", org, conf.OrganizationSlug()) + } + + if conf.GetTokenForOrg(org) != existingToken { + t.Errorf("expected token to be %s, got %s", existingToken, conf.GetTokenForOrg(org)) + } + }) +} + +func TestConfigureRequiresGitRepository(t *testing.T) { + t.Parallel() + + t.Run("fails when not in a git repository", func(t *testing.T) { + t.Parallel() + fs := afero.NewMemMapFs() + conf := config.New(fs, nil) + + // Create a factory with nil GitRepository (simulating not being in a git repo) + f := &factory.Factory{Config: conf, GitRepository: nil} + + err := ConfigureRun(f) + + if err == nil { + t.Error("expected error when not in a git repository, got nil") + } + + expectedErr := "not in a Git repository - bk should be configured at the root of a Git repository" + if err.Error() != expectedErr { + t.Errorf("expected error message %q, got %q", expectedErr, err.Error()) + } + }) + + t.Run("succeeds when in a git repository", func(t *testing.T) { + // Skip this test because we can't easily mock the interactive prompts + // In a real implementation, we would need to mock the promptForInput function + // or restructure the code to allow for testing without interactive input + t.Skip("skipping test that requires interactive input") + }) +} diff --git a/cmd/configure/configure.go b/cmd/configure/configure.go new file mode 100644 index 00000000..b38a2884 --- /dev/null +++ b/cmd/configure/configure.go @@ -0,0 +1,52 @@ +package configure + +import ( + "context" + "errors" + + "github.com/alecthomas/kong" + "github.com/buildkite/cli/v3/internal/cli" + "github.com/buildkite/cli/v3/pkg/cmd/factory" +) + +type ConfigureCmd struct { + Force bool `help:"Force setting a new token"` + Org string `help:"Organization slug"` + Token string `help:"API token"` + + Add AddCmd `cmd:"" help:"Add config for new organization" subcmd:"add"` +} + +func (c *ConfigureCmd) Help() string { + return ` +Examples: + # Configure Buildkite API token + $ bk configure --org my-org --token my-token + + # Force setting a new token + $ bk configure --force --org my-org --token my-token +` +} + +func (c *ConfigureCmd) Run(kongCtx *kong.Context, globals cli.GlobalFlags) error { + f, err := factory.New() + + if err != nil { + return err + } + + f.SkipConfirm = globals.SkipConfirmation() + f.NoInput = globals.DisableInput() + f.Quiet = globals.IsQuiet() + + if !c.Force && f.Config.APIToken() != "" { + return errors.New("API token already configured. You must use --force") + } + + ctx := context.Background() + if c.Org != "" && c.Token != "" { + return ConfigureWithCredentials(ctx, f, c.Org, c.Token) + + } + return ConfigureRun(ctx, f) +} From 6fdbc88ef49dfa7bd2c2a940e388d4887a92fcfe Mon Sep 17 00:00:00 2001 From: lizrabuya <115472349+lizrabuya@users.noreply.github.com> Date: Tue, 16 Dec 2025 17:50:36 +1100 Subject: [PATCH 2/7] Removed delegate functions to cobra and clean up of cobra implementation of configure --- cmd/configure/add_test.go | 194 +++++++++++++++--------------- main.go | 83 +++---------- pkg/cmd/configure/add/add.go | 100 --------------- pkg/cmd/configure/add/add_test.go | 157 ------------------------ pkg/cmd/configure/configure.go | 46 ------- pkg/cmd/root/root.go | 2 - 6 files changed, 118 insertions(+), 464 deletions(-) delete mode 100644 pkg/cmd/configure/add/add.go delete mode 100644 pkg/cmd/configure/add/add_test.go delete mode 100644 pkg/cmd/configure/configure.go diff --git a/cmd/configure/add_test.go b/cmd/configure/add_test.go index c55144e7..af8e1b98 100644 --- a/cmd/configure/add_test.go +++ b/cmd/configure/add_test.go @@ -10,6 +10,7 @@ import ( func TestGetTokenForOrg(t *testing.T) { t.Parallel() + t.Skip("Skipping test due to recent code changes") // Remove this line when re-enabling the test t.Run("returns empty string when no token exists", func(t *testing.T) { t.Parallel() @@ -60,98 +61,101 @@ func TestGetTokenForOrg(t *testing.T) { }) } -func TestConfigureWithCredentials(t *testing.T) { - t.Parallel() - - t.Run("configures organization and token", func(t *testing.T) { - t.Parallel() - fs := afero.NewMemMapFs() - conf := config.New(fs, nil) - f := &factory.Factory{Config: conf} - - org := "test-org" - token := "bk_test_token_12345" - - err := ConfigureWithCredentials(f, org, token) - if err != nil { - t.Errorf("expected no error, got %s", err) - } - - if conf.OrganizationSlug() != org { - t.Errorf("expected organization to be %s, got %s", org, conf.OrganizationSlug()) - } - - if conf.GetTokenForOrg(org) != token { - t.Errorf("expected token to be %s, got %s", token, conf.GetTokenForOrg(org)) - } - }) -} - -func TestConfigureTokenReuse(t *testing.T) { - t.Parallel() - - t.Run("reuses existing token when available", func(t *testing.T) { - t.Parallel() - fs := afero.NewMemMapFs() - conf := config.New(fs, nil) - f := &factory.Factory{Config: conf} - - org := "test-org" - existingToken := "bk_existing_token_12345" - - // Pre-configure a token for the organization - conf.SetTokenForOrg(org, existingToken) - - // Verify the token can be retrieved - retrievedToken := getTokenForOrg(f, org) - if retrievedToken != existingToken { - t.Errorf("expected to retrieve existing token %s, got %s", existingToken, retrievedToken) - } - - // Configure with the existing token (simulating the logic in ConfigureRun) - err := ConfigureWithCredentials(f, org, retrievedToken) - if err != nil { - t.Errorf("expected no error, got %s", err) - } - - // Verify the configuration still works - if conf.OrganizationSlug() != org { - t.Errorf("expected organization to be %s, got %s", org, conf.OrganizationSlug()) - } - - if conf.GetTokenForOrg(org) != existingToken { - t.Errorf("expected token to be %s, got %s", existingToken, conf.GetTokenForOrg(org)) - } - }) -} - -func TestConfigureRequiresGitRepository(t *testing.T) { - t.Parallel() - - t.Run("fails when not in a git repository", func(t *testing.T) { - t.Parallel() - fs := afero.NewMemMapFs() - conf := config.New(fs, nil) - - // Create a factory with nil GitRepository (simulating not being in a git repo) - f := &factory.Factory{Config: conf, GitRepository: nil} - - err := ConfigureRun(f) - - if err == nil { - t.Error("expected error when not in a git repository, got nil") - } - - expectedErr := "not in a Git repository - bk should be configured at the root of a Git repository" - if err.Error() != expectedErr { - t.Errorf("expected error message %q, got %q", expectedErr, err.Error()) - } - }) - - t.Run("succeeds when in a git repository", func(t *testing.T) { - // Skip this test because we can't easily mock the interactive prompts - // In a real implementation, we would need to mock the promptForInput function - // or restructure the code to allow for testing without interactive input - t.Skip("skipping test that requires interactive input") - }) -} +// func TestConfigureWithCredentials(t *testing.T) { +// t.Parallel() +// t.Skip("Skipping test due to recent code changes") // Remove this line when re-enabling the test + +// t.Run("configures organization and token", func(t *testing.T) { +// t.Parallel() +// fs := afero.NewMemMapFs() +// conf := config.New(fs, nil) +// f := &factory.Factory{Config: conf} + +// org := "test-org" +// token := "bk_test_token_12345" + +// err := ConfigureWithCredentials(f, org, token) +// if err != nil { +// t.Errorf("expected no error, got %s", err) +// } + +// if conf.OrganizationSlug() != org { +// t.Errorf("expected organization to be %s, got %s", org, conf.OrganizationSlug()) +// } + +// if conf.GetTokenForOrg(org) != token { +// t.Errorf("expected token to be %s, got %s", token, conf.GetTokenForOrg(org)) +// } +// }) +// } + +// func TestConfigureTokenReuse(t *testing.T) { +// t.Parallel() +// t.Skip("Skipping test due to recent code changes") // Remove this line when re-enabling the test + +// t.Run("reuses existing token when available", func(t *testing.T) { +// t.Parallel() +// fs := afero.NewMemMapFs() +// conf := config.New(fs, nil) +// f := &factory.Factory{Config: conf} + +// org := "test-org" +// existingToken := "bk_existing_token_12345" + +// // Pre-configure a token for the organization +// conf.SetTokenForOrg(org, existingToken) + +// // Verify the token can be retrieved +// retrievedToken := getTokenForOrg(f, org) +// if retrievedToken != existingToken { +// t.Errorf("expected to retrieve existing token %s, got %s", existingToken, retrievedToken) +// } + +// // Configure with the existing token (simulating the logic in ConfigureRun) +// err := ConfigureWithCredentials(f, org, retrievedToken) +// if err != nil { +// t.Errorf("expected no error, got %s", err) +// } + +// // Verify the configuration still works +// if conf.OrganizationSlug() != org { +// t.Errorf("expected organization to be %s, got %s", org, conf.OrganizationSlug()) +// } + +// if conf.GetTokenForOrg(org) != existingToken { +// t.Errorf("expected token to be %s, got %s", existingToken, conf.GetTokenForOrg(org)) +// } +// }) +// } + +// func TestConfigureRequiresGitRepository(t *testing.T) { +// t.Parallel() +// t.Skip("Skipping test due to recent code changes") // Remove this line when re-enabling the test + +// t.Run("fails when not in a git repository", func(t *testing.T) { +// t.Parallel() +// fs := afero.NewMemMapFs() +// conf := config.New(fs, nil) + +// // Create a factory with nil GitRepository (simulating not being in a git repo) +// f := &factory.Factory{Config: conf, GitRepository: nil} + +// err := ConfigureRun(f) + +// if err == nil { +// t.Error("expected error when not in a git repository, got nil") +// } + +// expectedErr := "not in a Git repository - bk should be configured at the root of a Git repository" +// if err.Error() != expectedErr { +// t.Errorf("expected error message %q, got %q", expectedErr, err.Error()) +// } +// }) + +// t.Run("succeeds when in a git repository", func(t *testing.T) { +// // Skip this test because we can't easily mock the interactive prompts +// // In a real implementation, we would need to mock the promptForInput function +// // or restructure the code to allow for testing without interactive input +// t.Skip("skipping test that requires interactive input") +// }) +// } diff --git a/main.go b/main.go index 1b0ac840..786654ef 100644 --- a/main.go +++ b/main.go @@ -11,6 +11,7 @@ import ( "github.com/buildkite/cli/v3/cmd/artifacts" "github.com/buildkite/cli/v3/cmd/build" "github.com/buildkite/cli/v3/cmd/cluster" + "github.com/buildkite/cli/v3/cmd/configure" bkInit "github.com/buildkite/cli/v3/cmd/init" "github.com/buildkite/cli/v3/cmd/job" "github.com/buildkite/cli/v3/cmd/organization" @@ -35,21 +36,21 @@ type CLI struct { Quiet bool `help:"Suppress progress output" short:"q"` // Verbose bool `help:"Enable verbose error output" short:"V"` // TODO: Implement this, atm this is just a skeleton flag - Agent AgentCmd `cmd:"" help:"Manage agents"` - Api ApiCmd `cmd:"" help:"Interact with the Buildkite API"` - Artifacts ArtifactsCmd `cmd:"" help:"Manage pipeline build artifacts"` - Build BuildCmd `cmd:"" help:"Manage pipeline builds"` - Cluster ClusterCmd `cmd:"" help:"Manage organization clusters"` - Configure ConfigureCmd `cmd:"" help:"Configure Buildkite API token"` - Init bkInit.InitCmd `cmd:"" help:"Initialize a pipeline.yaml file"` - Job JobCmd `cmd:"" help:"Manage jobs within a build"` - Organization OrganizationCmd `cmd:"" help:"Manage organizations" aliases:"org"` - Pipeline PipelineCmd `cmd:"" help:"Manage pipelines"` - Package PackageCmd `cmd:"" help:"Manage packages"` - Use use.UseCmd `cmd:"" help:"Select an organization"` - User UserCmd `cmd:"" help:"Invite users to the organization"` - Version VersionCmd `cmd:"" help:"Print the version of the CLI being used"` - Whoami whoami.WhoAmICmd `cmd:"" help:"Print the current user and organization"` + Agent AgentCmd `cmd:"" help:"Manage agents"` + Api ApiCmd `cmd:"" help:"Interact with the Buildkite API"` + Artifacts ArtifactsCmd `cmd:"" help:"Manage pipeline build artifacts"` + Build BuildCmd `cmd:"" help:"Manage pipeline builds"` + Cluster ClusterCmd `cmd:"" help:"Manage organization clusters"` + Configure configure.ConfigureCmd `cmd:"" help:"Configure Buildkite API token"` + Init bkInit.InitCmd `cmd:"" help:"Initialize a pipeline.yaml file"` + Job JobCmd `cmd:"" help:"Manage jobs within a build"` + Organization OrganizationCmd `cmd:"" help:"Manage organizations" aliases:"org"` + Pipeline PipelineCmd `cmd:"" help:"Manage pipelines"` + Package PackageCmd `cmd:"" help:"Manage packages"` + Use use.UseCmd `cmd:"" help:"Select an organization"` + User UserCmd `cmd:"" help:"Invite users to the organization"` + Version VersionCmd `cmd:"" help:"Print the version of the CLI being used"` + Whoami whoami.WhoAmICmd `cmd:"" help:"Print the current user and organization"` } // Hybrid delegation commands, we should delete from these when native Kong implementations ready @@ -106,57 +107,8 @@ type ( ApiCmd struct { api.ApiCmd `cmd:"" help:"Interact with the Buildkite API"` } - ConfigureCmd struct { - Args []string `arg:"" optional:"" passthrough:"all"` - } ) -// Delegation methods, we should delete when native Kong implementations ready -func (c *ConfigureCmd) Run(cli *CLI) error { return cli.delegateToCobraSystem("configure", c.Args) } - -// delegateToCobraSystem delegates execution to the legacy Cobra command system. -// This is a temporary bridge during the Kong migration that ensures backwards compatibility -// by reconstructing global flags that Kong has already parsed. -func (cli *CLI) delegateToCobraSystem(command string, args []string) error { - // Preserve and restore original args for safety - originalArgs := os.Args - defer func() { os.Args = originalArgs }() - - // Reconstruct command args with global flags for Cobra compatibility - reconstructedArgs := cli.buildCobraArgs(command, args) - os.Args = reconstructedArgs - - if code := runCobraSystem(); code != 0 { - os.Exit(code) - } - return nil -} - -// buildCobraArgs constructs the argument slice for Cobra, including global flags. -// Kong parses and consumes global flags before delegation, so we need to reconstruct -// them to maintain backwards compatibility with Cobra commands. -func (cli *CLI) buildCobraArgs(command string, passthroughArgs []string) []string { - args := []string{os.Args[0], command} - - if cli.Yes { - args = append(args, "--yes") - } - if cli.NoInput { - args = append(args, "--no-input") - } - if cli.Quiet { - args = append(args, "--quiet") - } - // TODO: Add verbose flag reconstruction when implemented - // if cli.Verbose { - // args = append(args, "--verbose") - // } - - args = append(args, passthroughArgs...) - - return args -} - func runCobraSystem() int { f, err := factory.New() if err != nil { @@ -312,6 +264,9 @@ func isHelpRequest() bool { if len(os.Args) >= 2 && os.Args[1] == "user" { return false } + if len(os.Args) >= 2 && os.Args[1] == "configure" { + return false + } if len(os.Args) == 3 && (os.Args[2] == "-h" || os.Args[2] == "--help") { return true diff --git a/pkg/cmd/configure/add/add.go b/pkg/cmd/configure/add/add.go deleted file mode 100644 index a84000b9..00000000 --- a/pkg/cmd/configure/add/add.go +++ /dev/null @@ -1,100 +0,0 @@ -package add - -import ( - "bufio" - "errors" - "fmt" - "os" - "strings" - "syscall" - - "github.com/buildkite/cli/v3/pkg/cmd/factory" - "github.com/spf13/cobra" - "golang.org/x/term" -) - -func NewCmdAdd(f *factory.Factory) *cobra.Command { - cmd := &cobra.Command{ - Use: "add", - Args: cobra.NoArgs, - Short: "Add config for new organization", - Long: "Add configuration for a new organization.", - RunE: func(cmd *cobra.Command, args []string) error { - return ConfigureRun(f) - }, - } - - return cmd -} - -func ConfigureWithCredentials(f *factory.Factory, org, token string) error { - if err := f.Config.SelectOrganization(org, f.GitRepository != nil); err != nil { - return err - } - - return f.Config.SetTokenForOrg(org, token) -} - -func ConfigureRun(f *factory.Factory) error { - // Check if we're in a Git repository - if f.GitRepository == nil { - return errors.New("not in a Git repository - bk should be configured at the root of a Git repository") - } - - // Get organization slug - org, err := promptForInput("Organization slug: ", false) - if err != nil { - return err - } - if org == "" { - return errors.New("organization slug cannot be empty") - } - - // Check if token already exists for this organization - existingToken := getTokenForOrg(f, org) - if existingToken != "" { - fmt.Printf("Using existing API token for organization: %s\n", org) - return f.Config.SelectOrganization(org, f.GitRepository != nil) - } - - // Get API token with password input (no echo) - token, err := promptForInput("API Token: ", true) - if err != nil { - return err - } - if token == "" { - return errors.New("API token cannot be empty") - } - - fmt.Println("API token set for organization:", org) - return ConfigureWithCredentials(f, org, token) -} - -// getTokenForOrg retrieves the token for a specific organization from the user config -func getTokenForOrg(f *factory.Factory, org string) string { - return f.Config.GetTokenForOrg(org) -} - -// promptForInput handles terminal input with optional password masking -func promptForInput(prompt string, isPassword bool) (string, error) { - fmt.Print(prompt) - - if isPassword { - // Use term.ReadPassword for secure password input - passwordBytes, err := term.ReadPassword(int(syscall.Stdin)) - fmt.Println() // Add a newline after password input - if err != nil { - return "", err - } - return string(passwordBytes), nil - } else { - // Use standard input for regular text - reader := bufio.NewReader(os.Stdin) - input, err := reader.ReadString('\n') - if err != nil { - return "", err - } - // Trim whitespace and newlines - return strings.TrimSpace(input), nil - } -} diff --git a/pkg/cmd/configure/add/add_test.go b/pkg/cmd/configure/add/add_test.go deleted file mode 100644 index 26e37eb1..00000000 --- a/pkg/cmd/configure/add/add_test.go +++ /dev/null @@ -1,157 +0,0 @@ -package add - -import ( - "testing" - - "github.com/buildkite/cli/v3/internal/config" - "github.com/buildkite/cli/v3/pkg/cmd/factory" - "github.com/spf13/afero" -) - -func TestGetTokenForOrg(t *testing.T) { - t.Parallel() - - t.Run("returns empty string when no token exists", func(t *testing.T) { - t.Parallel() - fs := afero.NewMemMapFs() - conf := config.New(fs, nil) - f := &factory.Factory{Config: conf} - - token := getTokenForOrg(f, "nonexistent") - if token != "" { - t.Errorf("expected empty string, got %s", token) - } - }) - - t.Run("returns token when it exists for organization", func(t *testing.T) { - t.Parallel() - fs := afero.NewMemMapFs() - conf := config.New(fs, nil) - f := &factory.Factory{Config: conf} - - // Set up a token for an organization - expectedToken := "bk_test_token_12345" - conf.SetTokenForOrg("test-org", expectedToken) - - token := getTokenForOrg(f, "test-org") - if token != expectedToken { - t.Errorf("expected %s, got %s", expectedToken, token) - } - }) - - t.Run("returns different tokens for different organizations", func(t *testing.T) { - t.Parallel() - fs := afero.NewMemMapFs() - conf := config.New(fs, nil) - f := &factory.Factory{Config: conf} - - // Set up tokens for different organizations - token1 := "bk_test_token_org1" - token2 := "bk_test_token_org2" - conf.SetTokenForOrg("org1", token1) - conf.SetTokenForOrg("org2", token2) - - if getTokenForOrg(f, "org1") != token1 { - t.Errorf("expected %s for org1", token1) - } - if getTokenForOrg(f, "org2") != token2 { - t.Errorf("expected %s for org2", token2) - } - }) -} - -func TestConfigureWithCredentials(t *testing.T) { - t.Parallel() - - t.Run("configures organization and token", func(t *testing.T) { - t.Parallel() - fs := afero.NewMemMapFs() - conf := config.New(fs, nil) - f := &factory.Factory{Config: conf} - - org := "test-org" - token := "bk_test_token_12345" - - err := ConfigureWithCredentials(f, org, token) - if err != nil { - t.Errorf("expected no error, got %s", err) - } - - if conf.OrganizationSlug() != org { - t.Errorf("expected organization to be %s, got %s", org, conf.OrganizationSlug()) - } - - if conf.GetTokenForOrg(org) != token { - t.Errorf("expected token to be %s, got %s", token, conf.GetTokenForOrg(org)) - } - }) -} - -func TestConfigureTokenReuse(t *testing.T) { - t.Parallel() - - t.Run("reuses existing token when available", func(t *testing.T) { - t.Parallel() - fs := afero.NewMemMapFs() - conf := config.New(fs, nil) - f := &factory.Factory{Config: conf} - - org := "test-org" - existingToken := "bk_existing_token_12345" - - // Pre-configure a token for the organization - conf.SetTokenForOrg(org, existingToken) - - // Verify the token can be retrieved - retrievedToken := getTokenForOrg(f, org) - if retrievedToken != existingToken { - t.Errorf("expected to retrieve existing token %s, got %s", existingToken, retrievedToken) - } - - // Configure with the existing token (simulating the logic in ConfigureRun) - err := ConfigureWithCredentials(f, org, retrievedToken) - if err != nil { - t.Errorf("expected no error, got %s", err) - } - - // Verify the configuration still works - if conf.OrganizationSlug() != org { - t.Errorf("expected organization to be %s, got %s", org, conf.OrganizationSlug()) - } - - if conf.GetTokenForOrg(org) != existingToken { - t.Errorf("expected token to be %s, got %s", existingToken, conf.GetTokenForOrg(org)) - } - }) -} - -func TestConfigureRequiresGitRepository(t *testing.T) { - t.Parallel() - - t.Run("fails when not in a git repository", func(t *testing.T) { - t.Parallel() - fs := afero.NewMemMapFs() - conf := config.New(fs, nil) - - // Create a factory with nil GitRepository (simulating not being in a git repo) - f := &factory.Factory{Config: conf, GitRepository: nil} - - err := ConfigureRun(f) - - if err == nil { - t.Error("expected error when not in a git repository, got nil") - } - - expectedErr := "not in a Git repository - bk should be configured at the root of a Git repository" - if err.Error() != expectedErr { - t.Errorf("expected error message %q, got %q", expectedErr, err.Error()) - } - }) - - t.Run("succeeds when in a git repository", func(t *testing.T) { - // Skip this test because we can't easily mock the interactive prompts - // In a real implementation, we would need to mock the promptForInput function - // or restructure the code to allow for testing without interactive input - t.Skip("skipping test that requires interactive input") - }) -} diff --git a/pkg/cmd/configure/configure.go b/pkg/cmd/configure/configure.go deleted file mode 100644 index 6d1c1564..00000000 --- a/pkg/cmd/configure/configure.go +++ /dev/null @@ -1,46 +0,0 @@ -package configure - -import ( - "errors" - - addCmd "github.com/buildkite/cli/v3/pkg/cmd/configure/add" - "github.com/buildkite/cli/v3/pkg/cmd/factory" - "github.com/spf13/cobra" -) - -func NewCmdConfigure(f *factory.Factory) *cobra.Command { - var ( - force bool - org string - token string - ) - - cmd := &cobra.Command{ - Use: "configure", - Aliases: []string{"config"}, - Args: cobra.NoArgs, - Short: "Configure Buildkite API token", - RunE: func(cmd *cobra.Command, args []string) error { - // if the token already exists and --force is not used - if !force && f.Config.APIToken() != "" { - return errors.New("API token already configured. You must use --force") - } - - // If flags are provided, use them directly - if org != "" && token != "" { - return addCmd.ConfigureWithCredentials(f, org, token) - } - - // Otherwise fall back to interactive mode - return addCmd.ConfigureRun(f) - }, - } - - cmd.Flags().BoolVar(&force, "force", false, "Force setting a new token") - cmd.Flags().StringVar(&org, "org", "", "Organization slug") - cmd.Flags().StringVar(&token, "token", "", "API token") - - cmd.AddCommand(addCmd.NewCmdAdd(f)) - - return cmd -} diff --git a/pkg/cmd/root/root.go b/pkg/cmd/root/root.go index e7921da1..defe0d54 100644 --- a/pkg/cmd/root/root.go +++ b/pkg/cmd/root/root.go @@ -4,7 +4,6 @@ import ( "fmt" "github.com/MakeNowJust/heredoc" - configureCmd "github.com/buildkite/cli/v3/pkg/cmd/configure" "github.com/buildkite/cli/v3/pkg/cmd/factory" initCmd "github.com/buildkite/cli/v3/pkg/cmd/init" promptCmd "github.com/buildkite/cli/v3/pkg/cmd/prompt" @@ -41,7 +40,6 @@ func NewCmdRoot(f *factory.Factory) (*cobra.Command, error) { }, } - cmd.AddCommand(configureCmd.NewCmdConfigure(f)) cmd.AddCommand(initCmd.NewCmdInit(f)) cmd.AddCommand(promptCmd.NewCmdPrompt(f)) From 433be6d1d4072487c4b6c97ea0f7a644104edce0 Mon Sep 17 00:00:00 2001 From: lizrabuya <115472349+lizrabuya@users.noreply.github.com> Date: Fri, 19 Dec 2025 02:21:45 +1100 Subject: [PATCH 3/7] fix configure logic --- cmd/configure/add.go | 40 +++----- cmd/configure/add_test.go | 196 ++++++++++++++++++------------------- cmd/configure/configure.go | 23 +++-- 3 files changed, 124 insertions(+), 135 deletions(-) diff --git a/cmd/configure/add.go b/cmd/configure/add.go index d538fa64..2ab24149 100644 --- a/cmd/configure/add.go +++ b/cmd/configure/add.go @@ -9,8 +9,6 @@ import ( "strings" "syscall" - "github.com/alecthomas/kong" - "github.com/buildkite/cli/v3/internal/cli" "github.com/buildkite/cli/v3/pkg/cmd/factory" "golang.org/x/term" ) @@ -18,51 +16,37 @@ import ( type AddCmd struct{} func (c *AddCmd) Help() string { - return ` + return ` Examples: # Add configuration for a new organization $ bk configure add ` } -func (c *AddCmd) Run(kongCtx *kong.Context, globals cli.GlobalFlags) error { - f, err := factory.New() - if err != nil { +func ConfigureWithCredentials(f *factory.Factory, org, token string) error { + if err := f.Config.SelectOrganization(org, f.GitRepository != nil); err != nil { return err } - f.SkipConfirm = globals.SkipConfirmation() - f.NoInput = globals.DisableInput() - f.Quiet = globals.IsQuiet() - - ctx := context.Background() - - return ConfigureRun(ctx, f) -} - -func ConfigureWithCredentials(ctx context.Context, f *factory.Factory, org, token string) error { - if err := f.Config.SelectOrganization(org, f.GitRepository != nil); err != nil { - return err + if token == "" { + // Check if token already exists for this organization + existingToken := getTokenForOrg(f, org) + if existingToken != "" { + fmt.Printf("Using existing API token for organization: %s\n", org) + return f.Config.SelectOrganization(org, f.GitRepository != nil) + } + return errors.New("API token cannot be empty") } return f.Config.SetTokenForOrg(org, token) } -func ConfigureRun(ctx context.Context, f *factory.Factory) error { +func ConfigureRun(ctx context.Context, f *factory.Factory, org string) error { // Check if we're in a Git repository if f.GitRepository == nil { return errors.New("not in a Git repository - bk should be configured at the root of a Git repository") } - // Get organization slug - org, err := promptForInput("Organization slug: ", false) - if err != nil { - return err - } - if org == "" { - return errors.New("organization slug cannot be empty") - } - // Check if token already exists for this organization existingToken := getTokenForOrg(f, org) if existingToken != "" { diff --git a/cmd/configure/add_test.go b/cmd/configure/add_test.go index af8e1b98..a85e874b 100644 --- a/cmd/configure/add_test.go +++ b/cmd/configure/add_test.go @@ -61,101 +61,101 @@ func TestGetTokenForOrg(t *testing.T) { }) } -// func TestConfigureWithCredentials(t *testing.T) { -// t.Parallel() -// t.Skip("Skipping test due to recent code changes") // Remove this line when re-enabling the test - -// t.Run("configures organization and token", func(t *testing.T) { -// t.Parallel() -// fs := afero.NewMemMapFs() -// conf := config.New(fs, nil) -// f := &factory.Factory{Config: conf} - -// org := "test-org" -// token := "bk_test_token_12345" - -// err := ConfigureWithCredentials(f, org, token) -// if err != nil { -// t.Errorf("expected no error, got %s", err) -// } - -// if conf.OrganizationSlug() != org { -// t.Errorf("expected organization to be %s, got %s", org, conf.OrganizationSlug()) -// } - -// if conf.GetTokenForOrg(org) != token { -// t.Errorf("expected token to be %s, got %s", token, conf.GetTokenForOrg(org)) -// } -// }) -// } - -// func TestConfigureTokenReuse(t *testing.T) { -// t.Parallel() -// t.Skip("Skipping test due to recent code changes") // Remove this line when re-enabling the test - -// t.Run("reuses existing token when available", func(t *testing.T) { -// t.Parallel() -// fs := afero.NewMemMapFs() -// conf := config.New(fs, nil) -// f := &factory.Factory{Config: conf} - -// org := "test-org" -// existingToken := "bk_existing_token_12345" - -// // Pre-configure a token for the organization -// conf.SetTokenForOrg(org, existingToken) - -// // Verify the token can be retrieved -// retrievedToken := getTokenForOrg(f, org) -// if retrievedToken != existingToken { -// t.Errorf("expected to retrieve existing token %s, got %s", existingToken, retrievedToken) -// } - -// // Configure with the existing token (simulating the logic in ConfigureRun) -// err := ConfigureWithCredentials(f, org, retrievedToken) -// if err != nil { -// t.Errorf("expected no error, got %s", err) -// } - -// // Verify the configuration still works -// if conf.OrganizationSlug() != org { -// t.Errorf("expected organization to be %s, got %s", org, conf.OrganizationSlug()) -// } - -// if conf.GetTokenForOrg(org) != existingToken { -// t.Errorf("expected token to be %s, got %s", existingToken, conf.GetTokenForOrg(org)) -// } -// }) -// } - -// func TestConfigureRequiresGitRepository(t *testing.T) { -// t.Parallel() -// t.Skip("Skipping test due to recent code changes") // Remove this line when re-enabling the test - -// t.Run("fails when not in a git repository", func(t *testing.T) { -// t.Parallel() -// fs := afero.NewMemMapFs() -// conf := config.New(fs, nil) - -// // Create a factory with nil GitRepository (simulating not being in a git repo) -// f := &factory.Factory{Config: conf, GitRepository: nil} - -// err := ConfigureRun(f) - -// if err == nil { -// t.Error("expected error when not in a git repository, got nil") -// } - -// expectedErr := "not in a Git repository - bk should be configured at the root of a Git repository" -// if err.Error() != expectedErr { -// t.Errorf("expected error message %q, got %q", expectedErr, err.Error()) -// } -// }) - -// t.Run("succeeds when in a git repository", func(t *testing.T) { -// // Skip this test because we can't easily mock the interactive prompts -// // In a real implementation, we would need to mock the promptForInput function -// // or restructure the code to allow for testing without interactive input -// t.Skip("skipping test that requires interactive input") -// }) -// } +func TestConfigureWithCredentials(t *testing.T) { + t.Parallel() + t.Skip("Skipping test due to recent code changes") // Remove this line when re-enabling the test + + t.Run("configures organization and token", func(t *testing.T) { + t.Parallel() + fs := afero.NewMemMapFs() + conf := config.New(fs, nil) + f := &factory.Factory{Config: conf} + + org := "test-org" + token := "bk_test_token_12345" + + err := ConfigureWithCredentials(f, org, token) + if err != nil { + t.Errorf("expected no error, got %s", err) + } + + if conf.OrganizationSlug() != org { + t.Errorf("expected organization to be %s, got %s", org, conf.OrganizationSlug()) + } + + if conf.GetTokenForOrg(org) != token { + t.Errorf("expected token to be %s, got %s", token, conf.GetTokenForOrg(org)) + } + }) +} + +func TestConfigureTokenReuse(t *testing.T) { + t.Parallel() + t.Skip("Skipping test due to recent code changes") // Remove this line when re-enabling the test + + t.Run("reuses existing token when available", func(t *testing.T) { + t.Parallel() + fs := afero.NewMemMapFs() + conf := config.New(fs, nil) + f := &factory.Factory{Config: conf} + + org := "test-org" + existingToken := "bk_existing_token_12345" + + // Pre-configure a token for the organization + conf.SetTokenForOrg(org, existingToken) + + // Verify the token can be retrieved + retrievedToken := getTokenForOrg(f, org) + if retrievedToken != existingToken { + t.Errorf("expected to retrieve existing token %s, got %s", existingToken, retrievedToken) + } + + // Configure with the existing token (simulating the logic in ConfigureRun) + err := ConfigureWithCredentials(f, org, retrievedToken) + if err != nil { + t.Errorf("expected no error, got %s", err) + } + + // Verify the configuration still works + if conf.OrganizationSlug() != org { + t.Errorf("expected organization to be %s, got %s", org, conf.OrganizationSlug()) + } + + if conf.GetTokenForOrg(org) != existingToken { + t.Errorf("expected token to be %s, got %s", existingToken, conf.GetTokenForOrg(org)) + } + }) +} + +func TestConfigureRequiresGitRepository(t *testing.T) { + t.Parallel() + t.Skip("Skipping test due to recent code changes") // Remove this line when re-enabling the test + + t.Run("fails when not in a git repository", func(t *testing.T) { + t.Parallel() + fs := afero.NewMemMapFs() + conf := config.New(fs, nil) + + // Create a factory with nil GitRepository (simulating not being in a git repo) + f := &factory.Factory{Config: conf, GitRepository: nil} + + err := ConfigureRun(f) + + if err == nil { + t.Error("expected error when not in a git repository, got nil") + } + + expectedErr := "not in a Git repository - bk should be configured at the root of a Git repository" + if err.Error() != expectedErr { + t.Errorf("expected error message %q, got %q", expectedErr, err.Error()) + } + }) + + t.Run("succeeds when in a git repository", func(t *testing.T) { + // Skip this test because we can't easily mock the interactive prompts + // In a real implementation, we would need to mock the promptForInput function + // or restructure the code to allow for testing without interactive input + t.Skip("skipping test that requires interactive input") + }) +} diff --git a/cmd/configure/configure.go b/cmd/configure/configure.go index b38a2884..4a0293d7 100644 --- a/cmd/configure/configure.go +++ b/cmd/configure/configure.go @@ -7,24 +7,24 @@ import ( "github.com/alecthomas/kong" "github.com/buildkite/cli/v3/internal/cli" "github.com/buildkite/cli/v3/pkg/cmd/factory" + "github.com/buildkite/cli/v3/pkg/cmd/validation" ) type ConfigureCmd struct { Force bool `help:"Force setting a new token"` Org string `help:"Organization slug"` - Token string `help:"API token"` - Add AddCmd `cmd:"" help:"Add config for new organization" subcmd:"add"` + Add AddCmd `cmd:"" optional:"" help:"Add configuration for a new organization" default:"1"` } func (c *ConfigureCmd) Help() string { return ` Examples: # Configure Buildkite API token - $ bk configure --org my-org --token my-token + $ bk configure --org my-org # Force setting a new token - $ bk configure --force --org my-org --token my-token + $ bk configure --force --org my-org ` } @@ -39,14 +39,19 @@ func (c *ConfigureCmd) Run(kongCtx *kong.Context, globals cli.GlobalFlags) error f.NoInput = globals.DisableInput() f.Quiet = globals.IsQuiet() - if !c.Force && f.Config.APIToken() != "" { - return errors.New("API token already configured. You must use --force") + if err := validation.ValidateConfiguration(f.Config, kongCtx.Command()); err != nil { + return err } ctx := context.Background() - if c.Org != "" && c.Token != "" { - return ConfigureWithCredentials(ctx, f, c.Org, c.Token) + if c.Org == "" { + return errors.New("organization slug cannot be empty") + } + + if !c.Force && f.Config.APIToken() != "" { + return errors.New("API token already configured. You must use --force") } - return ConfigureRun(ctx, f) + + return ConfigureRun(ctx, f, c.Org) } From 015b991565aad911e3dbef306d02894d01d48420 Mon Sep 17 00:00:00 2001 From: lizrabuya <115472349+lizrabuya@users.noreply.github.com> Date: Fri, 19 Dec 2025 11:28:25 +1100 Subject: [PATCH 4/7] fix lint errors --- cmd/configure/{ => add}/add.go | 4 ++-- cmd/configure/{ => add}/add_test.go | 6 +++--- cmd/configure/configure.go | 5 +++-- 3 files changed, 8 insertions(+), 7 deletions(-) rename cmd/configure/{ => add}/add.go (97%) rename cmd/configure/{ => add}/add_test.go (97%) diff --git a/cmd/configure/add.go b/cmd/configure/add/add.go similarity index 97% rename from cmd/configure/add.go rename to cmd/configure/add/add.go index 2ab24149..accd8391 100644 --- a/cmd/configure/add.go +++ b/cmd/configure/add/add.go @@ -1,4 +1,4 @@ -package configure +package add import ( "bufio" @@ -64,7 +64,7 @@ func ConfigureRun(ctx context.Context, f *factory.Factory, org string) error { } fmt.Println("API token set for organization:", org) - return ConfigureWithCredentials(ctx, f, org, token) + return ConfigureWithCredentials(f, org, token) } // getTokenForOrg retrieves the token for a specific organization from the user config diff --git a/cmd/configure/add_test.go b/cmd/configure/add/add_test.go similarity index 97% rename from cmd/configure/add_test.go rename to cmd/configure/add/add_test.go index a85e874b..989f7ea9 100644 --- a/cmd/configure/add_test.go +++ b/cmd/configure/add/add_test.go @@ -1,6 +1,7 @@ -package configure +package add import ( + "context" "testing" "github.com/buildkite/cli/v3/internal/config" @@ -130,7 +131,6 @@ func TestConfigureTokenReuse(t *testing.T) { func TestConfigureRequiresGitRepository(t *testing.T) { t.Parallel() - t.Skip("Skipping test due to recent code changes") // Remove this line when re-enabling the test t.Run("fails when not in a git repository", func(t *testing.T) { t.Parallel() @@ -140,7 +140,7 @@ func TestConfigureRequiresGitRepository(t *testing.T) { // Create a factory with nil GitRepository (simulating not being in a git repo) f := &factory.Factory{Config: conf, GitRepository: nil} - err := ConfigureRun(f) + err := ConfigureRun(context.Background(), f, "test-org") if err == nil { t.Error("expected error when not in a git repository, got nil") diff --git a/cmd/configure/configure.go b/cmd/configure/configure.go index 4a0293d7..25c08e74 100644 --- a/cmd/configure/configure.go +++ b/cmd/configure/configure.go @@ -5,6 +5,7 @@ import ( "errors" "github.com/alecthomas/kong" + addCmd "github.com/buildkite/cli/v3/cmd/configure/add" "github.com/buildkite/cli/v3/internal/cli" "github.com/buildkite/cli/v3/pkg/cmd/factory" "github.com/buildkite/cli/v3/pkg/cmd/validation" @@ -14,7 +15,7 @@ type ConfigureCmd struct { Force bool `help:"Force setting a new token"` Org string `help:"Organization slug"` - Add AddCmd `cmd:"" optional:"" help:"Add configuration for a new organization" default:"1"` + Add addCmd.AddCmd `cmd:"" optional:"" help:"Add configuration for a new organization" default:"1"` } func (c *ConfigureCmd) Help() string { @@ -53,5 +54,5 @@ func (c *ConfigureCmd) Run(kongCtx *kong.Context, globals cli.GlobalFlags) error return errors.New("API token already configured. You must use --force") } - return ConfigureRun(ctx, f, c.Org) + return addCmd.ConfigureRun(ctx, f, c.Org) } From c2b38afed827a1ee35316c66fc46115724b08c2a Mon Sep 17 00:00:00 2001 From: lizrabuya <115472349+lizrabuya@users.noreply.github.com> Date: Fri, 19 Dec 2025 11:56:31 +1100 Subject: [PATCH 5/7] Add back asking for token as input prompt --- cmd/configure/add/add.go | 17 ++++++++++------- cmd/configure/add/add_test.go | 2 +- cmd/configure/configure.go | 9 +++++---- 3 files changed, 16 insertions(+), 12 deletions(-) diff --git a/cmd/configure/add/add.go b/cmd/configure/add/add.go index accd8391..508c0689 100644 --- a/cmd/configure/add/add.go +++ b/cmd/configure/add/add.go @@ -41,7 +41,7 @@ func ConfigureWithCredentials(f *factory.Factory, org, token string) error { return f.Config.SetTokenForOrg(org, token) } -func ConfigureRun(ctx context.Context, f *factory.Factory, org string) error { +func ConfigureRun(ctx context.Context, f *factory.Factory, org, token string) error { // Check if we're in a Git repository if f.GitRepository == nil { return errors.New("not in a Git repository - bk should be configured at the root of a Git repository") @@ -54,13 +54,16 @@ func ConfigureRun(ctx context.Context, f *factory.Factory, org string) error { return f.Config.SelectOrganization(org, f.GitRepository != nil) } - // Get API token with password input (no echo) - token, err := promptForInput("API Token: ", true) - if err != nil { - return err - } if token == "" { - return errors.New("API token cannot be empty") + // Get API token with password input (no echo) + inputToken, err := promptForInput("API Token: ", true) + if err != nil { + return err + } + if inputToken == "" { + return errors.New("API token cannot be empty") + } + token = inputToken } fmt.Println("API token set for organization:", org) diff --git a/cmd/configure/add/add_test.go b/cmd/configure/add/add_test.go index 989f7ea9..0fe02f08 100644 --- a/cmd/configure/add/add_test.go +++ b/cmd/configure/add/add_test.go @@ -140,7 +140,7 @@ func TestConfigureRequiresGitRepository(t *testing.T) { // Create a factory with nil GitRepository (simulating not being in a git repo) f := &factory.Factory{Config: conf, GitRepository: nil} - err := ConfigureRun(context.Background(), f, "test-org") + err := ConfigureRun(context.Background(), f, "test-org", "some-token") if err == nil { t.Error("expected error when not in a git repository, got nil") diff --git a/cmd/configure/configure.go b/cmd/configure/configure.go index 25c08e74..42154abc 100644 --- a/cmd/configure/configure.go +++ b/cmd/configure/configure.go @@ -14,18 +14,19 @@ import ( type ConfigureCmd struct { Force bool `help:"Force setting a new token"` Org string `help:"Organization slug"` + Token string `help:"API token"` - Add addCmd.AddCmd `cmd:"" optional:"" help:"Add configuration for a new organization" default:"1"` + Add addCmd.AddCmd `cmd:"" help:"Add configuration for a new organization" default:"1"` } func (c *ConfigureCmd) Help() string { return ` Examples: # Configure Buildkite API token - $ bk configure --org my-org + $ bk configure --org my-org --token my-token # Force setting a new token - $ bk configure --force --org my-org + $ bk configure --force --org my-org --token my-token ` } @@ -54,5 +55,5 @@ func (c *ConfigureCmd) Run(kongCtx *kong.Context, globals cli.GlobalFlags) error return errors.New("API token already configured. You must use --force") } - return addCmd.ConfigureRun(ctx, f, c.Org) + return addCmd.ConfigureRun(ctx, f, c.Org, c.Token) } From 13bd4bbc39463dda0fee6d41433830cca2c50328 Mon Sep 17 00:00:00 2001 From: lizrabuya <115472349+lizrabuya@users.noreply.github.com> Date: Fri, 19 Dec 2025 17:48:06 +1100 Subject: [PATCH 6/7] add back the prompts for org and token --- cmd/configure/add/add.go | 70 ++++++++++++++++++++++++----------- cmd/configure/add/add_test.go | 3 +- cmd/configure/configure.go | 42 ++++++++++++--------- main.go | 34 +++++++++-------- 4 files changed, 92 insertions(+), 57 deletions(-) diff --git a/cmd/configure/add/add.go b/cmd/configure/add/add.go index 508c0689..0703efab 100644 --- a/cmd/configure/add/add.go +++ b/cmd/configure/add/add.go @@ -2,18 +2,23 @@ package add import ( "bufio" - "context" "errors" "fmt" "os" "strings" "syscall" + "github.com/alecthomas/kong" + "github.com/buildkite/cli/v3/internal/cli" "github.com/buildkite/cli/v3/pkg/cmd/factory" + "github.com/buildkite/cli/v3/pkg/cmd/validation" "golang.org/x/term" ) -type AddCmd struct{} +type AddCmd struct { + Org string `help:"Organization slug"` + Token string `help:"API token"` +} func (c *AddCmd) Help() string { return ` @@ -23,30 +28,54 @@ Examples: ` } -func ConfigureWithCredentials(f *factory.Factory, org, token string) error { - if err := f.Config.SelectOrganization(org, f.GitRepository != nil); err != nil { +func (c *AddCmd) Run(kongCtx *kong.Context, globals cli.GlobalFlags) error { + f, err := factory.New() + + if err != nil { return err } - if token == "" { - // Check if token already exists for this organization - existingToken := getTokenForOrg(f, org) - if existingToken != "" { - fmt.Printf("Using existing API token for organization: %s\n", org) - return f.Config.SelectOrganization(org, f.GitRepository != nil) - } - return errors.New("API token cannot be empty") + f.SkipConfirm = globals.SkipConfirmation() + f.NoInput = globals.DisableInput() + f.Quiet = globals.IsQuiet() + + if err := validation.ValidateConfiguration(f.Config, kongCtx.Command()); err != nil { + return err + } + + // If flags are provided, use them directly + if c.Org != "" && c.Token != "" { + return ConfigureWithCredentials(f, c.Org, c.Token) } + return ConfigureRun(f, c.Org) +} + +func ConfigureWithCredentials(f *factory.Factory, org, token string) error { + if err := f.Config.SelectOrganization(org, f.GitRepository != nil); err != nil { + return err + } return f.Config.SetTokenForOrg(org, token) } -func ConfigureRun(ctx context.Context, f *factory.Factory, org, token string) error { +func ConfigureRun(f *factory.Factory, org string) error { // Check if we're in a Git repository if f.GitRepository == nil { return errors.New("not in a Git repository - bk should be configured at the root of a Git repository") } + if org == "" { + // Get organization slug + inputOrg, err := promptForInput("Organization slug: ", false) + + if err != nil { + return err + } + if inputOrg == "" { + return errors.New("organization slug cannot be empty") + } + org = inputOrg + } // Check if token already exists for this organization existingToken := getTokenForOrg(f, org) if existingToken != "" { @@ -54,16 +83,13 @@ func ConfigureRun(ctx context.Context, f *factory.Factory, org, token string) er return f.Config.SelectOrganization(org, f.GitRepository != nil) } + // Get API token with password input (no echo) + token, err := promptForInput("API Token: ", true) + if err != nil { + return err + } if token == "" { - // Get API token with password input (no echo) - inputToken, err := promptForInput("API Token: ", true) - if err != nil { - return err - } - if inputToken == "" { - return errors.New("API token cannot be empty") - } - token = inputToken + return errors.New("API token cannot be empty") } fmt.Println("API token set for organization:", org) diff --git a/cmd/configure/add/add_test.go b/cmd/configure/add/add_test.go index 0fe02f08..b7626615 100644 --- a/cmd/configure/add/add_test.go +++ b/cmd/configure/add/add_test.go @@ -1,7 +1,6 @@ package add import ( - "context" "testing" "github.com/buildkite/cli/v3/internal/config" @@ -140,7 +139,7 @@ func TestConfigureRequiresGitRepository(t *testing.T) { // Create a factory with nil GitRepository (simulating not being in a git repo) f := &factory.Factory{Config: conf, GitRepository: nil} - err := ConfigureRun(context.Background(), f, "test-org", "some-token") + err := ConfigureRun(f, "test-org") if err == nil { t.Error("expected error when not in a git repository, got nil") diff --git a/cmd/configure/configure.go b/cmd/configure/configure.go index 42154abc..f39fa368 100644 --- a/cmd/configure/configure.go +++ b/cmd/configure/configure.go @@ -1,36 +1,43 @@ package configure import ( - "context" "errors" + "strings" "github.com/alecthomas/kong" - addCmd "github.com/buildkite/cli/v3/cmd/configure/add" + "github.com/buildkite/cli/v3/cmd/configure/add" "github.com/buildkite/cli/v3/internal/cli" "github.com/buildkite/cli/v3/pkg/cmd/factory" "github.com/buildkite/cli/v3/pkg/cmd/validation" ) type ConfigureCmd struct { - Force bool `help:"Force setting a new token"` - Org string `help:"Organization slug"` - Token string `help:"API token"` + Show ConfigureShowCmd `cmd:"" help:"Configure Buildkite API token" default:"1" hidden:"" kong:"-"` + Add add.AddCmd `cmd:"" optional:"" help:"Add configuration for a new organization"` +} - Add addCmd.AddCmd `cmd:"" help:"Add configuration for a new organization" default:"1"` +type ConfigureShowCmd struct { + Default bool `cmd:"" help:"Configure Buildkite API token with provided arguments" default:"true" hidden:"" kong:"-"` + Org string `help:"Organization slug"` + Token string `help:"API token"` + Force bool `help:"Force setting a new token"` } -func (c *ConfigureCmd) Help() string { - return ` +func (c *ConfigureShowCmd) Help() string { + var help strings.Builder + help.WriteString(` Examples: # Configure Buildkite API token $ bk configure --org my-org --token my-token # Force setting a new token - $ bk configure --force --org my-org --token my-token -` + $ bk configure --force --org my-org --token my-token +`) + + return help.String() } -func (c *ConfigureCmd) Run(kongCtx *kong.Context, globals cli.GlobalFlags) error { +func (c *ConfigureShowCmd) Run(kongCtx *kong.Context, globals cli.GlobalFlags) error { f, err := factory.New() if err != nil { @@ -45,15 +52,14 @@ func (c *ConfigureCmd) Run(kongCtx *kong.Context, globals cli.GlobalFlags) error return err } - ctx := context.Background() - - if c.Org == "" { - return errors.New("organization slug cannot be empty") - } - if !c.Force && f.Config.APIToken() != "" { return errors.New("API token already configured. You must use --force") } - return addCmd.ConfigureRun(ctx, f, c.Org, c.Token) + // If flags are provided, use them directly + if c.Org != "" && c.Token != "" { + return add.ConfigureWithCredentials(f, c.Org, c.Token) + } + + return add.ConfigureRun(f, c.Org) } diff --git a/main.go b/main.go index 00e218ee..a496940a 100644 --- a/main.go +++ b/main.go @@ -36,21 +36,21 @@ type CLI struct { Quiet bool `help:"Suppress progress output" short:"q"` // Verbose bool `help:"Enable verbose error output" short:"V"` // TODO: Implement this, atm this is just a skeleton flag - Agent AgentCmd `cmd:"" help:"Manage agents"` - Api ApiCmd `cmd:"" help:"Interact with the Buildkite API"` - Artifacts ArtifactsCmd `cmd:"" help:"Manage pipeline build artifacts"` - Build BuildCmd `cmd:"" help:"Manage pipeline builds"` - Cluster ClusterCmd `cmd:"" help:"Manage organization clusters"` - Configure configure.ConfigureCmd `cmd:"" help:"Configure Buildkite API token"` - Init bkInit.InitCmd `cmd:"" help:"Initialize a pipeline.yaml file"` - Job JobCmd `cmd:"" help:"Manage jobs within a build"` - Organization OrganizationCmd `cmd:"" help:"Manage organizations" aliases:"org"` - Pipeline PipelineCmd `cmd:"" help:"Manage pipelines"` - Package PackageCmd `cmd:"" help:"Manage packages"` - Use use.UseCmd `cmd:"" help:"Select an organization"` - User UserCmd `cmd:"" help:"Invite users to the organization"` - Version VersionCmd `cmd:"" help:"Print the version of the CLI being used"` - Whoami whoami.WhoAmICmd `cmd:"" help:"Print the current user and organization"` + Agent AgentCmd `cmd:"" help:"Manage agents"` + Api ApiCmd `cmd:"" help:"Interact with the Buildkite API"` + Artifacts ArtifactsCmd `cmd:"" help:"Manage pipeline build artifacts"` + Build BuildCmd `cmd:"" help:"Manage pipeline builds"` + Cluster ClusterCmd `cmd:"" help:"Manage organization clusters"` + Configure ConfigureCmd `cmd:"" help:"Configure Buildkite API token"` + Init bkInit.InitCmd `cmd:"" help:"Initialize a pipeline.yaml file"` + Job JobCmd `cmd:"" help:"Manage jobs within a build"` + Organization OrganizationCmd `cmd:"" help:"Manage organizations" aliases:"org"` + Pipeline PipelineCmd `cmd:"" help:"Manage pipelines"` + Package PackageCmd `cmd:"" help:"Manage packages"` + Use use.UseCmd `cmd:"" help:"Select an organization"` + User UserCmd `cmd:"" help:"Invite users to the organization"` + Version VersionCmd `cmd:"" help:"Print the version of the CLI being used"` + Whoami whoami.WhoAmICmd `cmd:"" help:"Print the current user and organization"` } // Hybrid delegation commands, we should delete from these when native Kong implementations ready @@ -108,6 +108,10 @@ type ( ApiCmd struct { api.ApiCmd `cmd:"" help:"Interact with the Buildkite API"` } + ConfigureCmd struct { + configure.ConfigureCmd `cmd:"" help:"Configure Buildkite API token"` + //Add add.AddCmd `cmd:"" optional:"" hidden:"" help:"Add configuration for a new organization"` + } ) func runCobraSystem() int { From ee1d8755ed76e51953d2dd1c3ffe39e0aa667fca Mon Sep 17 00:00:00 2001 From: lizrabuya <115472349+lizrabuya@users.noreply.github.com> Date: Mon, 5 Jan 2026 12:15:26 +1100 Subject: [PATCH 7/7] Move implementation of bk configure to one package --- cmd/configure/add/add.go | 126 ------------------ cmd/configure/configure.go | 122 ++++++++++++++--- .../{add/add_test.go => configure_test.go} | 5 +- main.go | 1 - 4 files changed, 104 insertions(+), 150 deletions(-) delete mode 100644 cmd/configure/add/add.go rename cmd/configure/{add/add_test.go => configure_test.go} (93%) diff --git a/cmd/configure/add/add.go b/cmd/configure/add/add.go deleted file mode 100644 index 0703efab..00000000 --- a/cmd/configure/add/add.go +++ /dev/null @@ -1,126 +0,0 @@ -package add - -import ( - "bufio" - "errors" - "fmt" - "os" - "strings" - "syscall" - - "github.com/alecthomas/kong" - "github.com/buildkite/cli/v3/internal/cli" - "github.com/buildkite/cli/v3/pkg/cmd/factory" - "github.com/buildkite/cli/v3/pkg/cmd/validation" - "golang.org/x/term" -) - -type AddCmd struct { - Org string `help:"Organization slug"` - Token string `help:"API token"` -} - -func (c *AddCmd) Help() string { - return ` -Examples: - # Add configuration for a new organization - $ bk configure add -` -} - -func (c *AddCmd) Run(kongCtx *kong.Context, globals cli.GlobalFlags) error { - f, err := factory.New() - - if err != nil { - return err - } - - f.SkipConfirm = globals.SkipConfirmation() - f.NoInput = globals.DisableInput() - f.Quiet = globals.IsQuiet() - - if err := validation.ValidateConfiguration(f.Config, kongCtx.Command()); err != nil { - return err - } - - // If flags are provided, use them directly - if c.Org != "" && c.Token != "" { - return ConfigureWithCredentials(f, c.Org, c.Token) - } - - return ConfigureRun(f, c.Org) -} - -func ConfigureWithCredentials(f *factory.Factory, org, token string) error { - if err := f.Config.SelectOrganization(org, f.GitRepository != nil); err != nil { - return err - } - return f.Config.SetTokenForOrg(org, token) -} - -func ConfigureRun(f *factory.Factory, org string) error { - // Check if we're in a Git repository - if f.GitRepository == nil { - return errors.New("not in a Git repository - bk should be configured at the root of a Git repository") - } - - if org == "" { - // Get organization slug - inputOrg, err := promptForInput("Organization slug: ", false) - - if err != nil { - return err - } - if inputOrg == "" { - return errors.New("organization slug cannot be empty") - } - org = inputOrg - } - // Check if token already exists for this organization - existingToken := getTokenForOrg(f, org) - if existingToken != "" { - fmt.Printf("Using existing API token for organization: %s\n", org) - return f.Config.SelectOrganization(org, f.GitRepository != nil) - } - - // Get API token with password input (no echo) - token, err := promptForInput("API Token: ", true) - if err != nil { - return err - } - if token == "" { - return errors.New("API token cannot be empty") - } - - fmt.Println("API token set for organization:", org) - return ConfigureWithCredentials(f, org, token) -} - -// getTokenForOrg retrieves the token for a specific organization from the user config -func getTokenForOrg(f *factory.Factory, org string) string { - return f.Config.GetTokenForOrg(org) -} - -// promptForInput handles terminal input with optional password masking -func promptForInput(prompt string, isPassword bool) (string, error) { - fmt.Print(prompt) - - if isPassword { - // Use term.ReadPassword for secure password input - passwordBytes, err := term.ReadPassword(int(syscall.Stdin)) - fmt.Println() // Add a newline after password input - if err != nil { - return "", err - } - return string(passwordBytes), nil - } else { - // Use standard input for regular text - reader := bufio.NewReader(os.Stdin) - input, err := reader.ReadString('\n') - if err != nil { - return "", err - } - // Trim whitespace and newlines - return strings.TrimSpace(input), nil - } -} diff --git a/cmd/configure/configure.go b/cmd/configure/configure.go index f39fa368..248020c1 100644 --- a/cmd/configure/configure.go +++ b/cmd/configure/configure.go @@ -1,43 +1,56 @@ package configure import ( + "bufio" "errors" + "fmt" + "os" "strings" "github.com/alecthomas/kong" - "github.com/buildkite/cli/v3/cmd/configure/add" "github.com/buildkite/cli/v3/internal/cli" + "github.com/buildkite/cli/v3/internal/io" "github.com/buildkite/cli/v3/pkg/cmd/factory" "github.com/buildkite/cli/v3/pkg/cmd/validation" ) type ConfigureCmd struct { - Show ConfigureShowCmd `cmd:"" help:"Configure Buildkite API token" default:"1" hidden:"" kong:"-"` - Add add.AddCmd `cmd:"" optional:"" help:"Add configuration for a new organization"` + Org string `help:"Organization slug" optional:""` + Token string `help:"API token" optional:""` + Force bool `help:"Force setting a new token" optional:""` + Default ConfigureDefaultCmd `cmd:"" optional:"" help:"Configure Buildkite API token" hidden:"" default:"1"` + Add ConfigureAddCmd `cmd:"" optional:"" help:"Add configuration for a new organization"` } -type ConfigureShowCmd struct { - Default bool `cmd:"" help:"Configure Buildkite API token with provided arguments" default:"true" hidden:"" kong:"-"` - Org string `help:"Organization slug"` - Token string `help:"API token"` - Force bool `help:"Force setting a new token"` +type ConfigureDefaultCmd struct { } -func (c *ConfigureShowCmd) Help() string { - var help strings.Builder - help.WriteString(` +type ConfigureAddCmd struct { +} + +func (c *ConfigureAddCmd) Help() string { + return ` +Examples: + # Prompt configuration to add for a new organization + $ bk configure add + + # Add configure Buildkite API token + $ bk configure add --org my-org --token my-token +` +} + +func (c *ConfigureCmd) Help() string { + return ` Examples: # Configure Buildkite API token $ bk configure --org my-org --token my-token # Force setting a new token $ bk configure --force --org my-org --token my-token -`) - - return help.String() +` } -func (c *ConfigureShowCmd) Run(kongCtx *kong.Context, globals cli.GlobalFlags) error { +func (c *ConfigureCmd) Run(kongCtx *kong.Context, globals cli.GlobalFlags) error { f, err := factory.New() if err != nil { @@ -52,14 +65,85 @@ func (c *ConfigureShowCmd) Run(kongCtx *kong.Context, globals cli.GlobalFlags) e return err } - if !c.Force && f.Config.APIToken() != "" { - return errors.New("API token already configured. You must use --force") + if kongCtx.Command() == "configure default" { + if !c.Force && f.Config.APIToken() != "" { + return errors.New("API token already configured. You must use --force") + } + } // If flags are provided, use them directly if c.Org != "" && c.Token != "" { - return add.ConfigureWithCredentials(f, c.Org, c.Token) + return ConfigureWithCredentials(f, c.Org, c.Token) + } + + return ConfigureRun(f, c.Org) +} + +func ConfigureWithCredentials(f *factory.Factory, org, token string) error { + if err := f.Config.SelectOrganization(org, f.GitRepository != nil); err != nil { + return err + } + return f.Config.SetTokenForOrg(org, token) +} + +func ConfigureRun(f *factory.Factory, org string) error { + // Check if we're in a Git repository + if f.GitRepository == nil { + return errors.New("not in a Git repository - bk should be configured at the root of a Git repository") } - return add.ConfigureRun(f, c.Org) + if org == "" { + // Get organization slug + inputOrg, err := promptForInput("Organization slug: ", false) + + if err != nil { + return err + } + if inputOrg == "" { + return errors.New("organization slug cannot be empty") + } + org = inputOrg + } + // Check if token already exists for this organization + existingToken := getTokenForOrg(f, org) + if existingToken != "" { + fmt.Printf("Using existing API token for organization: %s\n", org) + return f.Config.SelectOrganization(org, f.GitRepository != nil) + } + + // Get API token with password input (no echo) + token, err := promptForInput("API Token: ", true) + if err != nil { + return err + } + if token == "" { + return errors.New("API token cannot be empty") + } + + fmt.Println("API token set for organization:", org) + return ConfigureWithCredentials(f, org, token) +} + +// getTokenForOrg retrieves the token for a specific organization from the user config +func getTokenForOrg(f *factory.Factory, org string) string { + return f.Config.GetTokenForOrg(org) +} + +// promptForInput handles terminal input with optional password masking +func promptForInput(prompt string, isPassword bool) (string, error) { + fmt.Print(prompt) + + if isPassword { + return io.ReadPassword() + } else { + // Use standard input for regular text + reader := bufio.NewReader(os.Stdin) + input, err := reader.ReadString('\n') + if err != nil { + return "", err + } + // Trim whitespace and newlines + return strings.TrimSpace(input), nil + } } diff --git a/cmd/configure/add/add_test.go b/cmd/configure/configure_test.go similarity index 93% rename from cmd/configure/add/add_test.go rename to cmd/configure/configure_test.go index b7626615..1d65a10e 100644 --- a/cmd/configure/add/add_test.go +++ b/cmd/configure/configure_test.go @@ -1,4 +1,4 @@ -package add +package configure import ( "testing" @@ -10,7 +10,6 @@ import ( func TestGetTokenForOrg(t *testing.T) { t.Parallel() - t.Skip("Skipping test due to recent code changes") // Remove this line when re-enabling the test t.Run("returns empty string when no token exists", func(t *testing.T) { t.Parallel() @@ -63,7 +62,6 @@ func TestGetTokenForOrg(t *testing.T) { func TestConfigureWithCredentials(t *testing.T) { t.Parallel() - t.Skip("Skipping test due to recent code changes") // Remove this line when re-enabling the test t.Run("configures organization and token", func(t *testing.T) { t.Parallel() @@ -91,7 +89,6 @@ func TestConfigureWithCredentials(t *testing.T) { func TestConfigureTokenReuse(t *testing.T) { t.Parallel() - t.Skip("Skipping test due to recent code changes") // Remove this line when re-enabling the test t.Run("reuses existing token when available", func(t *testing.T) { t.Parallel() diff --git a/main.go b/main.go index a496940a..8fba0b71 100644 --- a/main.go +++ b/main.go @@ -110,7 +110,6 @@ type ( } ConfigureCmd struct { configure.ConfigureCmd `cmd:"" help:"Configure Buildkite API token"` - //Add add.AddCmd `cmd:"" optional:"" hidden:"" help:"Add configuration for a new organization"` } )