Skip to content

feat: implement organization context in the cli #12259

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 15 commits into from
Feb 26, 2024
8 changes: 8 additions & 0 deletions cli/config/file.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,14 @@ func (r Root) PostgresPort() File {
// File provides convenience methods for interacting with *os.File.
type File string

func (f File) Exists() bool {
if f == "" {
return false
}
_, err := os.Stat(string(f))
return err == nil
}

// Delete deletes the file.
func (f File) Delete() error {
if f == "" {
Expand Down
2 changes: 1 addition & 1 deletion cli/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func (r *RootCmd) create() *clibase.Cmd {
),
Middleware: clibase.Chain(r.InitClient(client)),
Handler: func(inv *clibase.Invocation) error {
organization, err := CurrentOrganization(inv, client)
organization, err := CurrentOrganization(r, inv, client)
if err != nil {
return err
}
Expand Down
131 changes: 131 additions & 0 deletions cli/organization.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
package cli

import (
"fmt"
"strings"

"github.com/coder/coder/v2/cli/clibase"
"github.com/coder/coder/v2/cli/cliui"
"github.com/coder/coder/v2/codersdk"
)

func (r *RootCmd) organizations() *clibase.Cmd {
cmd := &clibase.Cmd{
Annotations: workspaceCommand,
Use: "organizations [subcommand]",
Short: "Organization related commands",
Aliases: []string{"organization", "org", "orgs"},
Hidden: true, // Hidden until these commands are complete.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I marked this as hidden, I think we can defer the actual command name discussion and get the code in. Don't want to bikeshed and prevent code progress.

Handler: func(inv *clibase.Invocation) error {
return inv.Command.HelpHandler(inv)
},
Children: []*clibase.Cmd{
r.currentOrganization(),
},
}

cmd.Options = clibase.OptionSet{}
return cmd
}

func (r *RootCmd) currentOrganization() *clibase.Cmd {
var (
stringFormat func(orgs []codersdk.Organization) (string, error)
client = new(codersdk.Client)
formatter = cliui.NewOutputFormatter(
cliui.ChangeFormatterData(cliui.TextFormat(), func(data any) (any, error) {
typed, ok := data.([]codersdk.Organization)
if !ok {
// This should never happen
return "", fmt.Errorf("expected []Organization, got %T", data)
}
return stringFormat(typed)
}),
cliui.TableFormat([]codersdk.Organization{}, []string{"id", "name", "default"}),
cliui.JSONFormat(),
)
onlyID = false
)
cmd := &clibase.Cmd{
Use: "show [current|me|uuid]",
Short: "Show the organization, if no argument is given, the organization currently in use will be shown.",
Middleware: clibase.Chain(
r.InitClient(client),
clibase.RequireRangeArgs(0, 1),
),
Options: clibase.OptionSet{
{
Name: "only-id",
Description: "Only print the organization ID.",
Required: false,
Flag: "only-id",
Value: clibase.BoolOf(&onlyID),
},
},
Handler: func(inv *clibase.Invocation) error {
orgArg := "current"
if len(inv.Args) >= 1 {
orgArg = inv.Args[0]
}

var orgs []codersdk.Organization
var err error
switch strings.ToLower(orgArg) {
case "current":
stringFormat = func(orgs []codersdk.Organization) (string, error) {
if len(orgs) != 1 {
return "", fmt.Errorf("expected 1 organization, got %d", len(orgs))
}
return fmt.Sprintf("Current CLI Organization: %s (%s)\n", orgs[0].Name, orgs[0].ID.String()), nil
}
org, err := CurrentOrganization(r, inv, client)
if err != nil {
return err
}
orgs = []codersdk.Organization{org}
case "me":
stringFormat = func(orgs []codersdk.Organization) (string, error) {
var str strings.Builder
_, _ = fmt.Fprint(&str, "Organizations you are a member of:\n")
for _, org := range orgs {
_, _ = fmt.Fprintf(&str, "\t%s (%s)\n", org.Name, org.ID.String())
}
return str.String(), nil
}
orgs, err = client.OrganizationsByUser(inv.Context(), codersdk.Me)
if err != nil {
return err
}
default:
stringFormat = func(orgs []codersdk.Organization) (string, error) {
if len(orgs) != 1 {
return "", fmt.Errorf("expected 1 organization, got %d", len(orgs))
}
return fmt.Sprintf("Organization: %s (%s)\n", orgs[0].Name, orgs[0].ID.String()), nil
}
// This works for a uuid or a name
org, err := client.OrganizationByName(inv.Context(), orgArg)
if err != nil {
return err
}
orgs = []codersdk.Organization{org}
}

if onlyID {
for _, org := range orgs {
_, _ = fmt.Fprintf(inv.Stdout, "%s\n", org.ID)
}
} else {
out, err := formatter.Format(inv.Context(), orgs)
if err != nil {
return err
}
_, _ = fmt.Fprint(inv.Stdout, out)
}
return nil
},
}
formatter.AttachOptions(&cmd.Options)

return cmd
}
45 changes: 45 additions & 0 deletions cli/organization_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
package cli_test

import (
"testing"

"github.com/stretchr/testify/require"

"github.com/coder/coder/v2/cli/clitest"
"github.com/coder/coder/v2/coderd/coderdtest"
"github.com/coder/coder/v2/coderd/rbac"
"github.com/coder/coder/v2/codersdk"
"github.com/coder/coder/v2/pty/ptytest"
"github.com/coder/coder/v2/testutil"
)

func TestCurrentOrganization(t *testing.T) {
t.Parallel()

t.Run("OnlyID", func(t *testing.T) {
t.Parallel()
ownerClient := coderdtest.New(t, nil)
first := coderdtest.CreateFirstUser(t, ownerClient)
// Owner is required to make orgs
client, _ := coderdtest.CreateAnotherUser(t, ownerClient, first.OrganizationID, rbac.RoleOwner())

ctx := testutil.Context(t, testutil.WaitMedium)
orgs := []string{"foo", "bar"}
for _, orgName := range orgs {
_, err := client.CreateOrganization(ctx, codersdk.CreateOrganizationRequest{
Name: orgName,
})
require.NoError(t, err)
}

inv, root := clitest.New(t, "organizations", "show", "--only-id")
clitest.SetupConfig(t, client, root)
pty := ptytest.New(t).Attach(inv)
errC := make(chan error)
go func() {
errC <- inv.Run()
}()
require.NoError(t, <-errC)
pty.ExpectMatch(first.OrganizationID.String())
})
}
41 changes: 36 additions & 5 deletions cli/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ func (r *RootCmd) Core() []*clibase.Cmd {
r.tokens(),
r.users(),
r.version(defaultVersionInfo),
r.organizations(),

// Workspace Commands
r.autoupdate(),
Expand Down Expand Up @@ -698,14 +699,44 @@ func (r *RootCmd) createAgentClient() (*agentsdk.Client, error) {
}

// CurrentOrganization returns the currently active organization for the authenticated user.
func CurrentOrganization(inv *clibase.Invocation, client *codersdk.Client) (codersdk.Organization, error) {
func CurrentOrganization(r *RootCmd, inv *clibase.Invocation, client *codersdk.Client) (codersdk.Organization, error) {
conf := r.createConfig()
selected := ""
if conf.Organization().Exists() {
org, err := conf.Organization().Read()
if err != nil {
return codersdk.Organization{}, fmt.Errorf("read selected organization from config file %q: %w", conf.Organization(), err)
}
selected = org
}

// Verify the org exists and the user is a member
orgs, err := client.OrganizationsByUser(inv.Context(), codersdk.Me)
if err != nil {
return codersdk.Organization{}, nil
return codersdk.Organization{}, err
Comment on lines -704 to +716
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mafredri The unit test expecting context cancelled had to be changed because of this change. (#12259 (comment))

Before, if you failed to fetch the orgs, we'd return a nil uuid. I made this api failure fail the command now. So the unit tests fails differently now.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't expect TestTemplateCreate/WithVariablesFileWithoutRequiredValue to be testing for org failure though 🤔

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't write the test, but there is a comment indicating this cli error is expected. We cancel the context before running the command.

// We expect the cli to return an error, so we have to handle it
// ourselves.
go func() {
cancel()
err := inv.Run()
assert.Error(t, err)
}()

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we're testing against a cancelled context, that test is useless. I think it should either be removed or fixed. 😅 It's definitely not testing what the test name is indicating.

}
// For now, we won't use the config to set this.
// Eventually, we will support changing using "coder switch <org>"
return orgs[0], nil

// User manually selected an organization
if selected != "" {
index := slices.IndexFunc(orgs, func(org codersdk.Organization) bool {
return org.Name == selected || org.ID.String() == selected
})

if index < 0 {
return codersdk.Organization{}, xerrors.Errorf("organization %q not found, are you sure you are a member of this organization?", selected)
}
return orgs[index], nil
}

// User did not select an organization, so use the default.
index := slices.IndexFunc(orgs, func(org codersdk.Organization) bool {
return org.IsDefault
})
if index < 0 {
return codersdk.Organization{}, xerrors.Errorf("unable to determine current organization. Use 'coder organizations switch <org>' to select an organization to use")
}

return orgs[index], nil
}

func splitNamedWorkspace(identifier string) (owner string, workspaceName string, err error) {
Expand Down
2 changes: 1 addition & 1 deletion cli/templatecreate.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ func (r *RootCmd) templateCreate() *clibase.Cmd {
}
}

organization, err := CurrentOrganization(inv, client)
organization, err := CurrentOrganization(r, inv, client)
if err != nil {
return err
}
Expand Down
14 changes: 1 addition & 13 deletions cli/templatecreate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -243,19 +243,7 @@ func TestTemplateCreate(t *testing.T) {
assert.Error(t, err)
}()

matches := []struct {
match string
write string
}{
{match: "Upload", write: "yes"},
}
for _, m := range matches {
pty.ExpectMatch(m.match)
if len(m.write) > 0 {
pty.WriteLine(m.write)
}
}

pty.ExpectMatch("context canceled")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this change?

<-ctx.Done()
})

Expand Down
2 changes: 1 addition & 1 deletion cli/templatedelete.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func (r *RootCmd) templateDelete() *clibase.Cmd {
templates = []codersdk.Template{}
)

organization, err := CurrentOrganization(inv, client)
organization, err := CurrentOrganization(r, inv, client)
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion cli/templateedit.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ func (r *RootCmd) templateEdit() *clibase.Cmd {
}
}

organization, err := CurrentOrganization(inv, client)
organization, err := CurrentOrganization(r, inv, client)
if err != nil {
return xerrors.Errorf("get current organization: %w", err)
}
Expand Down
2 changes: 1 addition & 1 deletion cli/templatelist.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ func (r *RootCmd) templateList() *clibase.Cmd {
r.InitClient(client),
),
Handler: func(inv *clibase.Invocation) error {
organization, err := CurrentOrganization(inv, client)
organization, err := CurrentOrganization(r, inv, client)
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion cli/templatepull.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func (r *RootCmd) templatePull() *clibase.Cmd {
return xerrors.Errorf("either tar or zip can be selected")
}

organization, err := CurrentOrganization(inv, client)
organization, err := CurrentOrganization(r, inv, client)
if err != nil {
return xerrors.Errorf("get current organization: %w", err)
}
Expand Down
2 changes: 1 addition & 1 deletion cli/templatepush.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func (r *RootCmd) templatePush() *clibase.Cmd {
Handler: func(inv *clibase.Invocation) error {
uploadFlags.setWorkdir(workdir)

organization, err := CurrentOrganization(inv, client)
organization, err := CurrentOrganization(r, inv, client)
if err != nil {
return err
}
Expand Down
4 changes: 2 additions & 2 deletions cli/templateversionarchive.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func (r *RootCmd) setArchiveTemplateVersion(archive bool) *clibase.Cmd {
versions []codersdk.TemplateVersion
)

organization, err := CurrentOrganization(inv, client)
organization, err := CurrentOrganization(r, inv, client)
if err != nil {
return err
}
Expand Down Expand Up @@ -121,7 +121,7 @@ func (r *RootCmd) archiveTemplateVersions() *clibase.Cmd {
templates = []codersdk.Template{}
)

organization, err := CurrentOrganization(inv, client)
organization, err := CurrentOrganization(r, inv, client)
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion cli/templateversions.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ func (r *RootCmd) templateVersionsList() *clibase.Cmd {
},
},
Handler: func(inv *clibase.Invocation) error {
organization, err := CurrentOrganization(inv, client)
organization, err := CurrentOrganization(r, inv, client)
if err != nil {
return xerrors.Errorf("get current organization: %w", err)
}
Expand Down
2 changes: 1 addition & 1 deletion cli/usercreate.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ func (r *RootCmd) userCreate() *clibase.Cmd {
r.InitClient(client),
),
Handler: func(inv *clibase.Invocation) error {
organization, err := CurrentOrganization(inv, client)
organization, err := CurrentOrganization(r, inv, client)
if err != nil {
return err
}
Expand Down
Loading