diff --git a/.goreleaser.yaml b/.goreleaser.yaml index ad727dc..f71bde4 100644 --- a/.goreleaser.yaml +++ b/.goreleaser.yaml @@ -54,6 +54,7 @@ release: owner: siliconwitchery name: superstack-cli mode: keep-existing + replace_existing_artifacts: true prerelease: auto homebrew_casks: diff --git a/CLAUDE.md b/CLAUDE.md index a83f1e0..9e58b0b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -13,8 +13,8 @@ the entire git history ships. `~/projects/superstack-server` (github.com/siliconwitchery/superstack-server) and owns the JSON API this binary speaks. A change on either side of the wire usually implies one on the other, so read its CLAUDE.md before changing -anything that crosses it. The two files share the coding principles below -verbatim; a change to one belongs in both. +anything that crosses it. The two files share the coding principles and the +development cycle below verbatim; a change to one belongs in both. ## Coding principles @@ -79,11 +79,45 @@ General and meant to be reused verbatim across projects. too, so neither reaches for a C toolchain that need not exist. - Keep the dependency set small: nothing a distribution's packager would balk at. -- Development happens on the `dev` branch of both repositories; check the - current branch before the first edit, and never resume a branch that has - been merged and deleted. -- During development the CLI version is the next 0.0.x above the latest - release tag, and the server's minimum-version gate matches it exactly. +- Development happens on `dev` in both repositories, created fresh from `main` + at the start of every cycle and never resumed: merging squashes the branch + and deletes it, so any local copy left behind is permanently diverged. +- The CLI version changes only in a release pull request. The server's minimum + version is an independent compatibility floor, not the current CLI version; + raise it only for an incompatible API change, after the required CLI release + is available through its package channels. + +## Development cycle + +One request, one branch, one pull request, and the branch never outlives it. +Raj owns steps 1, 5 and 7; the rest happen here. + +1. **Raj asks for something.** A cycle starts from a request, never from + picking up where the last one stopped. +2. **Create `dev` fresh from `main`:** `git fetch origin`, then + `git switch -C dev origin/main`. Never resume an existing `dev`. Merging + squashes the branch and deletes it, so a local copy left behind is + permanently diverged, and its next pull request conflicts on every line it + touches. +3. **Work, then iterate on the feedback.** +4. **Open the pull request.** Product work does not change the CLI version or + the server's minimum-version gate. If an incompatible API change needs a + higher floor, keep the server compatible until the release in step 8 is + installable, then raise the gate in a server pull request. +5. **Raj reviews and merges on GitHub.** Squash only. +6. **Return to `main`:** `git switch main`, `git pull`, `git branch -D dev`, + so nothing stale is left to resume. +7. **Raj may ask for a release.** A merged cycle is not automatically one. +8. **Cut releases independently.** For the CLI, open and merge a release pull + request that changes its version, then tag that commit. A server-only + release needs no CLI release. For an incompatible API change, publish the + compatible CLI first, let its package channels catch up, and only then + merge and deploy the higher server gate. + +The production deploy refuses a gate ahead of the newest published CLI. The +branch invariant rests on step 2's `-C`, which resets a leftover `dev` onto +`origin/main` rather than resuming it, and on both repositories being set to +squash-only merges with Automatically delete head branches enabled. ## Architecture @@ -117,10 +151,11 @@ fetch the fleet-reading commands share. ## Releases -The version lives in one place, `version` in `main.go`. Bump it, then tag the -commit that bumped it. Nothing injects the version at build time, because -`-ldflags -X` cannot write to a Go const; the release workflow instead refuses -to build when the tag and the const disagree. +The version lives in one place, `version` in `main.go`. Change it in a release +pull request, then tag the commit that merges it. Product pull requests leave +it alone. Nothing injects the version at build time, because `-ldflags -X` +cannot write to a Go const; the release workflow refuses to build when the tag +and the const disagree. The workflow also refuses to build a tag that does not sit on `main`. GitHub rulesets cannot express that, because a tag rule can restrict who creates a diff --git a/README.md b/README.md index 235e98a..ac57944 100644 --- a/README.md +++ b/README.md @@ -95,6 +95,7 @@ Do everything below once. 1. Register at [aur.archlinux.org](https://aur.archlinux.org/register), then: ```sh + cd "$(mktemp -d)" ssh-keygen -t ed25519 -N "" -f aur_key cat aur_key.pub # paste into SSH Public Key in your AUR account settings cat aur_key # add as the Actions secret AUR_KEY @@ -103,26 +104,21 @@ Do everything below once. 1. Add one ruleset (Settings > Rules > Rulesets) targeting the default branch: require a pull request with 0 approvals, require the `build` status check, - block force pushes, restrict deletions. Add a second targeting `v*` tags: - block force pushes, restrict deletions. + allowed merge method squash only, block force pushes, restrict deletions. + Add a second targeting `v*` tags: block force pushes, restrict deletions. -## Releasing +1. Enable **Automatically delete head branches** (Settings > General). -1. Bump `version` in `main.go`, then open and merge a pull request: +## Releasing - ```sh - git checkout -b version-0.1.0 - git commit -am "Version 0.1.0" - git push -u origin version-0.1.0 - ``` +1. Change `version` in `main.go`, then open and merge a release pull request. -1. Tag the new commit that merging created: +1. Tag the commit that merging created: ```sh - git checkout main && git pull - grep '^const version' main.go - git tag v0.1.0 - git push origin v0.1.0 + git switch main && git pull + tag="v$(sed -n 's/^const version = "\(.*\)"$/\1/p' main.go)" + git tag "$tag" && git push origin "$tag" ``` 1. Write the release notes into the empty release body on GitHub, following the diff --git a/internal/commands/account_delete.go b/internal/commands/account_delete.go new file mode 100644 index 0000000..fba83bf --- /dev/null +++ b/internal/commands/account_delete.go @@ -0,0 +1,68 @@ +package commands + +import ( + "bufio" + "errors" + "fmt" + "io" + "io/fs" + "net/http" + "os" + "strings" +) + +func AccountDelete(arguments []string) error { + + if len(arguments) != 0 { + return errors.New("account delete takes no arguments") + } + + fmt.Print("Delete your account, its logins, and your access to every fleet? This cannot be undone. [y/N] ") + + answer, _ := bufio.NewReader(os.Stdin).ReadString('\n') + + answer = strings.ToLower(strings.TrimSpace(answer)) + + if answer != "y" && answer != "yes" { + fmt.Println("Nothing deleted.") + return nil + } + + request, err := authenticatedRequest(http.MethodDelete, "/account", nil) + + if err != nil { + return err + } + + response, err := apiClient.Do(request) + + if err != nil { + return fmt.Errorf("the server could not be reached: %w", err) + } + + defer response.Body.Close() + + if response.StatusCode != http.StatusNoContent { + message, _ := io.ReadAll(io.LimitReader(response.Body, 4096)) + + return fmt.Errorf("the server said: %s", strings.TrimSpace(string(message))) + } + + // The stored login died with the account, so it goes whether or not the + // file is still there + path, err := keyPath() + + if err != nil { + return err + } + + err = os.Remove(path) + + if err != nil && !errors.Is(err, fs.ErrNotExist) { + return err + } + + fmt.Println("Account deleted.") + + return nil +} diff --git a/internal/commands/account_delete_test.go b/internal/commands/account_delete_test.go new file mode 100644 index 0000000..734dad5 --- /dev/null +++ b/internal/commands/account_delete_test.go @@ -0,0 +1,122 @@ +package commands + +import ( + "net/http" + "os" + "strings" + "testing" +) + +func TestAccountDelete(t *testing.T) { + tests := []struct { + name string + arguments []string + answer string + refusal string + refusalCode int + wantDeleted bool + wantShown string + wantError string + }{ + { + name: "confirmed with y", + answer: "y\n", + wantDeleted: true, + wantShown: "Account deleted", + }, + { + name: "confirmed with yes", + answer: "YES\n", + wantDeleted: true, + wantShown: "Account deleted", + }, + { + name: "declined by default", + answer: "\n", + wantShown: "Nothing deleted", + }, + { + name: "declined with n", + answer: "n\n", + wantShown: "Nothing deleted", + }, + { + name: "closed input", + wantShown: "Nothing deleted", + }, + { + name: "the server refuses while a fleet is owned", + answer: "y\n", + refusal: "you still own fleets, hand each one over or delete it first", + refusalCode: http.StatusConflict, + wantDeleted: true, + wantError: "you still own fleets, hand each one over or delete it first", + }, + { + name: "arguments are refused", + arguments: []string{"everything"}, + wantError: "takes no arguments", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + deleted := false + + mux := http.NewServeMux() + + mux.HandleFunc("DELETE /account", func(w http.ResponseWriter, r *http.Request) { + deleted = true + + if test.refusal != "" { + http.Error(w, test.refusal, test.refusalCode) + return + } + + w.WriteHeader(http.StatusNoContent) + }) + + loggedInTestServer(t, mux) + + path, err := keyPath() + + if err != nil { + t.Fatal(err) + } + + answerOnStdin(t, test.answer) + + printed, err := captureStdout(t, func() error { + return AccountDelete(test.arguments) + }) + + if test.wantError != "" { + if err == nil || !strings.Contains(err.Error(), test.wantError) { + t.Fatalf("error = %v, want it to mention %q", err, test.wantError) + } + } else if err != nil { + t.Fatal(err) + } + + if deleted != test.wantDeleted { + t.Errorf("the server saw the account deleted = %v, want %v", deleted, test.wantDeleted) + } + + if test.wantShown != "" && !strings.Contains(printed, test.wantShown) { + t.Errorf("the output %q does not show %q", printed, test.wantShown) + } + + // The stored login is worthless once the account is gone, and must + // survive anything short of a completed delete + _, statErr := os.Stat(path) + + switch { + case test.wantDeleted && test.wantError == "" && statErr == nil: + t.Error("the login is still stored although the account was deleted") + + case (!test.wantDeleted || test.wantError != "") && statErr != nil: + t.Errorf("the login was removed although the account was not deleted: %v", statErr) + } + }) + } +} diff --git a/internal/commands/client_test.go b/internal/commands/client_test.go index d9744fc..68de4af 100644 --- a/internal/commands/client_test.go +++ b/internal/commands/client_test.go @@ -74,7 +74,18 @@ func loggedInTestServer(t *testing.T, handler http.Handler) { t.Fatal(err) } - server := httptest.NewServer(handler) + // Every command reaching a logged-in server must carry the stored key, so + // the fixture proves it once rather than each command remembering to + authorized := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Header.Get("Authorization") != "Bearer ssk_test" { + t.Errorf("%s %s carried authorization %q, want the stored key", + r.Method, r.URL.Path, r.Header.Get("Authorization")) + } + + handler.ServeHTTP(w, r) + }) + + server := httptest.NewServer(authorized) t.Cleanup(server.Close) @@ -219,6 +230,12 @@ func TestTakeServerFlag(t *testing.T) { } func TestApiRequestBase(t *testing.T) { + previousVersion := CliVersion + + CliVersion = "1.2.3" + + t.Cleanup(func() { CliVersion = previousVersion }) + tests := []struct { name string chosenBase string @@ -251,8 +268,10 @@ func TestApiRequestBase(t *testing.T) { t.Errorf("url = %q, want %q", request.URL.String(), test.wantUrl) } - if request.Header.Get("User-Agent") != "superstack/"+CliVersion { - t.Errorf("User-Agent = %q, want the CLI version", request.Header.Get("User-Agent")) + // Pinned to a literal, not to CliVersion: the server's gate parses + // this exact shape, so deriving it here would agree with any value + if request.Header.Get("User-Agent") != "superstack/1.2.3" { + t.Errorf("User-Agent = %q, want superstack/1.2.3", request.Header.Get("User-Agent")) } }) } diff --git a/internal/commands/fleet_delete_test.go b/internal/commands/fleet_delete_test.go index a951c39..63c58c5 100644 --- a/internal/commands/fleet_delete_test.go +++ b/internal/commands/fleet_delete_test.go @@ -38,13 +38,22 @@ func TestFleetDelete(t *testing.T) { tests := []struct { name string answer string + refusal string wantDeleted bool + wantError string }{ - {"confirmed with y", "y\n", true}, - {"confirmed with yes", "YES\n", true}, - {"declined with n", "n\n", false}, - {"declined by default", "\n", false}, - {"closed input", "", false}, + {name: "confirmed with y", answer: "y\n", wantDeleted: true}, + {name: "confirmed with yes", answer: "YES\n", wantDeleted: true}, + {name: "declined with n", answer: "n\n"}, + {name: "declined by default", answer: "\n"}, + {name: "closed input", answer: ""}, + { + name: "the server refuses after the confirmation", + answer: "y\n", + refusal: "only the fleet's owner can delete it", + wantDeleted: true, + wantError: "only the fleet's owner can delete it", + }, } for _, test := range tests { @@ -64,6 +73,11 @@ func TestFleetDelete(t *testing.T) { mux.HandleFunc("DELETE /fleets/{id}", func(w http.ResponseWriter, r *http.Request) { deletedPath = r.URL.Path + if test.refusal != "" { + http.Error(w, test.refusal, http.StatusForbidden) + return + } + w.WriteHeader(http.StatusNoContent) }) @@ -71,9 +85,22 @@ func TestFleetDelete(t *testing.T) { answerOnStdin(t, test.answer) - err := FleetDelete([]string{"3"}) + printed, err := captureStdout(t, func() error { + return FleetDelete([]string{"3"}) + }) - if err != nil { + switch { + case test.wantError != "": + if err == nil || !strings.Contains(err.Error(), test.wantError) { + t.Fatalf("error = %v, want it to mention %q", err, test.wantError) + } + + // A refused delete must never claim the fleet is gone + if strings.Contains(printed, "Deleted") { + t.Errorf("the output %q says the fleet was deleted although the server refused", printed) + } + + case err != nil: t.Fatal(err) } diff --git a/internal/commands/fleet_list_test.go b/internal/commands/fleet_list_test.go index 95105de..2091f2f 100644 --- a/internal/commands/fleet_list_test.go +++ b/internal/commands/fleet_list_test.go @@ -9,23 +9,30 @@ import ( func TestFleetList(t *testing.T) { tests := []struct { - name string - arguments []string - fleets string - wantError string + name string + arguments []string + fleets string + wantShown []string + wantAbsent []string + wantExact string + wantError string }{ { - name: "some fleets", - fleets: `[{"id":1,"name":"field trial","owner":true},{"id":2,"name":"rooftop","owner":false}]`, + name: "some fleets", + fleets: `[{"id":1,"name":"field trial","owner":true},{"id":2,"name":"rooftop","owner":false}]`, + wantShown: []string{"ID", "NAME", "ROLE", "field trial", "owner", "rooftop", "member"}, }, { - name: "no fleets", - fleets: `[]`, + name: "no fleets", + fleets: `[]`, + wantShown: []string{"No fleets yet"}, + wantAbsent: []string{"ID", "NAME"}, }, { name: "machine-readable output", arguments: []string{"--json"}, fleets: `[{"id":1,"name":"field trial","owner":true}]`, + wantExact: `[{"id":1,"name":"field trial","owner":true}]` + "\n", }, { name: "an unknown argument", @@ -44,7 +51,9 @@ func TestFleetList(t *testing.T) { loggedInTestServer(t, mux) - err := FleetList(test.arguments) + printed, err := captureStdout(t, func() error { + return FleetList(test.arguments) + }) if test.wantError != "" { if err == nil || !strings.Contains(err.Error(), test.wantError) { @@ -57,6 +66,22 @@ func TestFleetList(t *testing.T) { if err != nil { t.Fatal(err) } + + if test.wantExact != "" && printed != test.wantExact { + t.Errorf("the output is %q, want exactly %q", printed, test.wantExact) + } + + for _, want := range test.wantShown { + if !strings.Contains(printed, want) { + t.Errorf("the output %q does not show %q", printed, want) + } + } + + for _, absent := range test.wantAbsent { + if strings.Contains(printed, absent) { + t.Errorf("the output %q shows %q although there is nothing to list", printed, absent) + } + } }) } } diff --git a/internal/commands/key_list.go b/internal/commands/key_list.go index 4cd6cfa..295dd30 100644 --- a/internal/commands/key_list.go +++ b/internal/commands/key_list.go @@ -4,20 +4,10 @@ import ( "encoding/json" "errors" "fmt" - "io" - "net/http" "os" "strconv" - "strings" ) -type keyEntry struct { - Id int64 `json:"id"` - Fleet int64 `json:"fleet"` - Label string `json:"label"` - Suffix string `json:"suffix"` -} - func KeyList(arguments []string) error { jsonOutput := false @@ -67,29 +57,7 @@ func KeyList(arguments []string) error { } } - request, err := authenticatedRequest(http.MethodGet, "/keys", nil) - - if err != nil { - return err - } - - response, err := apiClient.Do(request) - - if err != nil { - return fmt.Errorf("the server could not be reached: %w", err) - } - - defer response.Body.Close() - - if response.StatusCode != http.StatusOK { - message, _ := io.ReadAll(io.LimitReader(response.Body, 4096)) - - return fmt.Errorf("the server said: %s", strings.TrimSpace(string(message))) - } - - fetched := []keyEntry{} - - err = json.NewDecoder(response.Body).Decode(&fetched) + fetched, err := fetchKeys() if err != nil { return err diff --git a/internal/commands/key_revoke.go b/internal/commands/key_revoke.go index 0d14fcc..edf4364 100644 --- a/internal/commands/key_revoke.go +++ b/internal/commands/key_revoke.go @@ -1,10 +1,12 @@ package commands import ( + "bufio" "errors" "fmt" "io" "net/http" + "os" "strconv" "strings" ) @@ -21,6 +23,37 @@ func KeyRevoke(arguments []string) error { return errors.New("the key id is the number shown by key list") } + keys, err := fetchKeys() + + if err != nil { + return err + } + + label := "" + found := false + + for _, key := range keys { + if key.Id == keyId { + label = key.Label + found = true + } + } + + if !found { + return errors.New("no such key") + } + + fmt.Printf("Revoke %q? Anything still using it stops reaching the fleet. [y/N] ", label) + + answer, _ := bufio.NewReader(os.Stdin).ReadString('\n') + + answer = strings.ToLower(strings.TrimSpace(answer)) + + if answer != "y" && answer != "yes" { + fmt.Println("Nothing revoked.") + return nil + } + request, err := authenticatedRequest(http.MethodDelete, "/keys/"+strconv.FormatInt(keyId, 10), nil) diff --git a/internal/commands/key_revoke_test.go b/internal/commands/key_revoke_test.go index 0ec5e63..37ac2f3 100644 --- a/internal/commands/key_revoke_test.go +++ b/internal/commands/key_revoke_test.go @@ -1,6 +1,7 @@ package commands import ( + "fmt" "net/http" "strings" "testing" @@ -8,23 +9,51 @@ import ( func TestKeyRevoke(t *testing.T) { tests := []struct { - name string - arguments []string - wantPath string - refusal string - wantError string + name string + arguments []string + answer string + refusal string + wantRevoked string + wantShown string + wantError string }{ { - name: "revoke a key", + name: "revoke a key", + arguments: []string{"3"}, + answer: "y\n", + wantRevoked: "/keys/3", + wantShown: "production", + }, + { + name: "declined by default", + arguments: []string{"3"}, + answer: "\n", + wantShown: "Nothing revoked", + }, + { + name: "declined with n", arguments: []string{"3"}, - wantPath: "/keys/3", + answer: "n\n", + wantShown: "Nothing revoked", + }, + { + name: "closed input", + arguments: []string{"3"}, + wantShown: "Nothing revoked", + }, + { + name: "the server refuses after the confirmation", + arguments: []string{"3"}, + answer: "y\n", + refusal: "no such key", + wantRevoked: "/keys/3", + wantError: "the server said: no such key", }, { - name: "a key out of reach", + name: "a key that is not yours", arguments: []string{"9"}, - wantPath: "/keys/9", - refusal: "no such key", - wantError: "the server said: no such key", + answer: "y\n", + wantError: "no such key", }, { name: "no key id", @@ -45,12 +74,16 @@ func TestKeyRevoke(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { + revokedPath := "" + mux := http.NewServeMux() + mux.HandleFunc("GET /keys", func(w http.ResponseWriter, r *http.Request) { + fmt.Fprint(w, `[{"id":3,"fleet":1,"label":"production","suffix":"a1b2c"}]`) + }) + mux.HandleFunc("DELETE /keys/{id}", func(w http.ResponseWriter, r *http.Request) { - if r.URL.Path != test.wantPath { - t.Errorf("the request went to %s, want %s", r.URL.Path, test.wantPath) - } + revokedPath = r.URL.Path if test.refusal != "" { http.Error(w, test.refusal, http.StatusNotFound) @@ -62,18 +95,26 @@ func TestKeyRevoke(t *testing.T) { loggedInTestServer(t, mux) - err := KeyRevoke(test.arguments) + answerOnStdin(t, test.answer) + + printed, err := captureStdout(t, func() error { + return KeyRevoke(test.arguments) + }) if test.wantError != "" { if err == nil || !strings.Contains(err.Error(), test.wantError) { t.Fatalf("error = %v, want it to mention %q", err, test.wantError) } + } else if err != nil { + t.Fatal(err) + } - return + if revokedPath != test.wantRevoked { + t.Errorf("the server saw %q revoked, want %q", revokedPath, test.wantRevoked) } - if err != nil { - t.Fatal(err) + if test.wantShown != "" && !strings.Contains(printed, test.wantShown) { + t.Errorf("the output %q does not show %q", printed, test.wantShown) } }) } diff --git a/internal/commands/keys.go b/internal/commands/keys.go new file mode 100644 index 0000000..6b64800 --- /dev/null +++ b/internal/commands/keys.go @@ -0,0 +1,48 @@ +package commands + +import ( + "encoding/json" + "fmt" + "io" + "net/http" + "strings" +) + +type keyEntry struct { + Id int64 `json:"id"` + Fleet int64 `json:"fleet"` + Label string `json:"label"` + Suffix string `json:"suffix"` +} + +func fetchKeys() ([]keyEntry, error) { + request, err := authenticatedRequest(http.MethodGet, "/keys", nil) + + if err != nil { + return nil, err + } + + response, err := apiClient.Do(request) + + if err != nil { + return nil, fmt.Errorf("the server could not be reached: %w", err) + } + + defer response.Body.Close() + + if response.StatusCode != http.StatusOK { + message, _ := io.ReadAll(io.LimitReader(response.Body, 4096)) + + return nil, fmt.Errorf("the server said: %s", strings.TrimSpace(string(message))) + } + + keys := []keyEntry{} + + err = json.NewDecoder(response.Body).Decode(&keys) + + if err != nil { + return nil, err + } + + return keys, nil +} diff --git a/internal/commands/login.go b/internal/commands/login.go index f0582e1..e206872 100644 --- a/internal/commands/login.go +++ b/internal/commands/login.go @@ -138,9 +138,13 @@ func Login(arguments []string) error { fmt.Println("Press enter to open the browser.") // The read sits in a goroutine so an unpressed key never stalls the - // poll: the code may just as well be entered on another device. + // poll: the code may just as well be entered on another device. The + // stream is captured here, because the goroutine outlives the read and + // must not touch os.Stdin once something else may have replaced it. + prompt := os.Stdin + go func() { - _, err := bufio.NewReader(os.Stdin).ReadString('\n') + _, err := bufio.NewReader(prompt).ReadString('\n') if err == nil { openBrowser(enterAt) diff --git a/internal/commands/member_list.go b/internal/commands/member_list.go index 90d4a30..bc79cdd 100644 --- a/internal/commands/member_list.go +++ b/internal/commands/member_list.go @@ -6,6 +6,7 @@ import ( "fmt" "io" "net/http" + "os" "strconv" "strings" ) @@ -60,11 +61,6 @@ func MemberList(arguments []string) error { return fmt.Errorf("the server said: %s", strings.TrimSpace(string(body))) } - if jsonOutput { - fmt.Print(string(body)) - return nil - } - people := struct { Owner string `json:"owner"` Members []string `json:"members"` @@ -76,6 +72,10 @@ func MemberList(arguments []string) error { return err } + if jsonOutput { + return json.NewEncoder(os.Stdout).Encode(people) + } + emailWidth := max(len("EMAIL"), len(people.Owner)) for _, email := range people.Members { diff --git a/internal/commands/member_list_test.go b/internal/commands/member_list_test.go index 7edce6f..9867d1b 100644 --- a/internal/commands/member_list_test.go +++ b/internal/commands/member_list_test.go @@ -12,27 +12,38 @@ func TestMemberList(t *testing.T) { name string arguments []string people string + wantFleet string + wantShown []string + wantExact string wantError string }{ { name: "the people table", arguments: []string{"3"}, people: `{"owner":"owner@example.com","members":["member@example.com"]}`, + wantFleet: "3", + wantShown: []string{"EMAIL", "ROLE", "owner@example.com", "owner", "member@example.com", "member"}, }, { name: "nobody but the owner", - arguments: []string{"3"}, + arguments: []string{"7"}, people: `{"owner":"owner@example.com","members":[]}`, + wantFleet: "7", + wantShown: []string{"owner@example.com", "owner"}, }, { name: "machine-readable output", arguments: []string{"3", "--json"}, people: `{"owner":"owner@example.com","members":["member@example.com"]}`, + wantFleet: "3", + wantExact: `{"owner":"owner@example.com","members":["member@example.com"]}` + "\n", }, { name: "the flag before the id", - arguments: []string{"--json", "3"}, + arguments: []string{"--json", "5"}, people: `{"owner":"owner@example.com","members":[]}`, + wantFleet: "5", + wantExact: `{"owner":"owner@example.com","members":[]}` + "\n", }, { name: "no fleet id", @@ -55,13 +66,19 @@ func TestMemberList(t *testing.T) { t.Run(test.name, func(t *testing.T) { mux := http.NewServeMux() + askedFleet := "" + mux.HandleFunc("GET /fleets/{id}/members", func(w http.ResponseWriter, r *http.Request) { + askedFleet = r.PathValue("id") + fmt.Fprint(w, test.people) }) loggedInTestServer(t, mux) - err := MemberList(test.arguments) + printed, err := captureStdout(t, func() error { + return MemberList(test.arguments) + }) if test.wantError != "" { if err == nil || !strings.Contains(err.Error(), test.wantError) { @@ -74,6 +91,20 @@ func TestMemberList(t *testing.T) { if err != nil { t.Fatal(err) } + + if askedFleet != test.wantFleet { + t.Errorf("the people of fleet %q were listed, want fleet %q", askedFleet, test.wantFleet) + } + + if test.wantExact != "" && printed != test.wantExact { + t.Errorf("the output is %q, want exactly %q", printed, test.wantExact) + } + + for _, want := range test.wantShown { + if !strings.Contains(printed, want) { + t.Errorf("the output %q does not show %q", printed, want) + } + } }) } } diff --git a/internal/commands/member_remove.go b/internal/commands/member_remove.go index dd3b803..1b81a37 100644 --- a/internal/commands/member_remove.go +++ b/internal/commands/member_remove.go @@ -1,11 +1,13 @@ package commands import ( + "bufio" "errors" "fmt" "io" "net/http" "net/url" + "os" "strconv" "strings" ) @@ -24,6 +26,37 @@ func MemberRemove(arguments []string) error { return errors.New("the fleet id is the number shown by fleet list") } + fleets, err := fetchFleets() + + if err != nil { + return err + } + + name := "" + found := false + + for _, fleet := range fleets { + if fleet.Id == fleetId { + name = fleet.Name + found = true + } + } + + if !found { + return errors.New("no such fleet") + } + + fmt.Printf("Take away %s's access to %q? [y/N] ", email, name) + + answer, _ := bufio.NewReader(os.Stdin).ReadString('\n') + + answer = strings.ToLower(strings.TrimSpace(answer)) + + if answer != "y" && answer != "yes" { + fmt.Println("Nothing changed.") + return nil + } + request, err := authenticatedRequest(http.MethodDelete, "/fleets/"+strconv.FormatInt(fleetId, 10)+"/members/"+url.PathEscape(email), nil) diff --git a/internal/commands/member_remove_test.go b/internal/commands/member_remove_test.go index f21bc82..1d4f34b 100644 --- a/internal/commands/member_remove_test.go +++ b/internal/commands/member_remove_test.go @@ -1,6 +1,7 @@ package commands import ( + "fmt" "net/http" "strings" "testing" @@ -8,11 +9,18 @@ import ( func TestMemberRemove(t *testing.T) { tests := []struct { - name string - email string + name string + email string + answer string + wantRemoved bool + wantShown string }{ - {"a plain address", "member@example.com"}, - {"an address with a hash", "a#b@example.com"}, + {name: "a plain address", email: "member@example.com", answer: "y\n", wantRemoved: true}, + {name: "an address with a hash", email: "a#b@example.com", answer: "yes\n", wantRemoved: true}, + {name: "the prompt names the fleet", email: "member@example.com", answer: "y\n", wantRemoved: true, wantShown: `access to "pilot"`}, + {name: "declined by default", email: "member@example.com", answer: "\n", wantShown: "Nothing changed"}, + {name: "declined with n", email: "member@example.com", answer: "n\n", wantShown: "Nothing changed"}, + {name: "closed input", email: "member@example.com", wantShown: "Nothing changed"}, } for _, test := range tests { @@ -22,6 +30,10 @@ func TestMemberRemove(t *testing.T) { mux := http.NewServeMux() + mux.HandleFunc("GET /fleets", func(w http.ResponseWriter, r *http.Request) { + fmt.Fprint(w, `[{"id":3,"name":"pilot","owner":true}]`) + }) + mux.HandleFunc("DELETE /fleets/{id}/members/{email}", func(w http.ResponseWriter, r *http.Request) { removedFleet = r.PathValue("id") removedEmail = r.PathValue("email") @@ -31,15 +43,27 @@ func TestMemberRemove(t *testing.T) { loggedInTestServer(t, mux) - err := MemberRemove([]string{test.email, "3"}) + answerOnStdin(t, test.answer) + + printed, err := captureStdout(t, func() error { + return MemberRemove([]string{test.email, "3"}) + }) if err != nil { t.Fatal(err) } - if removedFleet != "3" || removedEmail != test.email { + switch { + case test.wantRemoved && (removedFleet != "3" || removedEmail != test.email): t.Errorf("the server saw %q removed from fleet %q, want %q from fleet %q", removedEmail, removedFleet, test.email, "3") + + case !test.wantRemoved && removedEmail != "": + t.Errorf("the server saw %q removed although the confirmation was declined", removedEmail) + } + + if test.wantShown != "" && !strings.Contains(printed, test.wantShown) { + t.Errorf("the output %q does not show %q", printed, test.wantShown) } }) } diff --git a/main.go b/main.go index 250e528..2acd1b3 100644 --- a/main.go +++ b/main.go @@ -9,7 +9,7 @@ import ( "github.com/siliconwitchery/superstack-cli/internal/commands" ) -const version = "0.0.4" +const version = "0.0.2" type command struct { name string @@ -35,7 +35,7 @@ var sections = []section{ title: "Fleets", commands: []command{ {name: "fleet create", arguments: "", summary: "Create a fleet", run: commands.FleetCreate}, - {name: "fleet list", summary: "List the fleets you can reach", run: commands.FleetList}, + {name: "fleet list", arguments: "[--json]", summary: "List the fleets you can reach", run: commands.FleetList}, {name: "fleet rename", arguments: " ", summary: "Rename a fleet", run: commands.FleetRename}, {name: "fleet transfer", arguments: " ", summary: "Hand a fleet to a new owner", run: commands.FleetTransfer}, {name: "fleet delete", arguments: "", summary: "Delete a fleet and release its devices", run: commands.FleetDelete}, @@ -71,7 +71,7 @@ var sections = []section{ title: "People", commands: []command{ {name: "member add", arguments: " ", summary: "Give someone access to a fleet", run: commands.MemberAdd}, - {name: "member list", arguments: "", summary: "List the people who can reach a fleet", run: commands.MemberList}, + {name: "member list", arguments: " [--json]", summary: "List the people who can reach a fleet", run: commands.MemberList}, {name: "member remove", arguments: " ", summary: "Take away someone's access", run: commands.MemberRemove}, }, }, @@ -79,16 +79,16 @@ var sections = []section{ title: "Keys", commands: []command{ {name: "key create", arguments: "