diff --git a/pkg/cmd/clipboard/clipboard_listener.go b/pkg/cmd/clipboard/clipboard_listener.go index 38faadb2..32d6ae5c 100644 --- a/pkg/cmd/clipboard/clipboard_listener.go +++ b/pkg/cmd/clipboard/clipboard_listener.go @@ -3,7 +3,7 @@ package clipboard import ( "context" "fmt" - "io/ioutil" + "io" "net/http" "strings" @@ -56,7 +56,7 @@ func (server *Server) Run() { r := gin.New() r.GET("/", func(c *gin.Context) { - jsonData, err := ioutil.ReadAll(c.Request.Body) + jsonData, err := io.ReadAll(c.Request.Body) if err != nil { // Handle error c.String(http.StatusBadRequest, "Can't parse body") diff --git a/pkg/cmd/cmd.go b/pkg/cmd/cmd.go index 96de08d5..f7735c2f 100644 --- a/pkg/cmd/cmd.go +++ b/pkg/cmd/cmd.go @@ -272,7 +272,6 @@ func createCmdTree(cmd *cobra.Command, t *terminal.Terminal, loginCmdStore *stor cmd.AddCommand(secret.NewCmdSecret(loginCmdStore, t)) cmd.AddCommand(sshkeys.NewCmdSSHKeys(t, loginCmdStore)) cmd.AddCommand(start.NewCmdStart(t, loginCmdStore, noLoginCmdStore)) - cmd.AddCommand(start.NewCmdStart(t, loginCmdStore, noLoginCmdStore)) cmd.AddCommand(create.NewCmdCreate(t, loginCmdStore)) cmd.AddCommand(stop.NewCmdStop(t, loginCmdStore, noLoginCmdStore)) cmd.AddCommand(delete.NewCmdDelete(t, loginCmdStore, noLoginCmdStore)) diff --git a/pkg/cmd/importideconfig/importideconfig.go b/pkg/cmd/importideconfig/importideconfig.go index 1a891781..c2dd0a73 100644 --- a/pkg/cmd/importideconfig/importideconfig.go +++ b/pkg/cmd/importideconfig/importideconfig.go @@ -3,7 +3,6 @@ package importideconfig import ( "errors" "fmt" - "io/ioutil" "os" "os/exec" "path" @@ -190,7 +189,7 @@ func getExtensions(homedir string) ([]entity.VscodeExtensionMetadata, error) { func recursivelyFindFile(filenames []string, path string) ([]string, error) { var paths []string - files, err := ioutil.ReadDir(path) + files, err := os.ReadDir(path) if err != nil { return nil, breverrors.WrapAndTrace(err) } diff --git a/pkg/cmd/paths/paths.go b/pkg/cmd/paths/paths.go index e1e69f4c..1657c26a 100644 --- a/pkg/cmd/paths/paths.go +++ b/pkg/cmd/paths/paths.go @@ -2,12 +2,12 @@ package paths import ( "fmt" - "io/ioutil" + "os" // breverrors "github.com/brevdev/brev-cli/pkg/errors" ) func GetVsCodePaths() []string { - fi, err := ioutil.ReadDir("/home/brev/.vscode-server/bin") + fi, err := os.ReadDir("/home/brev/.vscode-server/bin") if err != nil { return []string{} } diff --git a/pkg/files/files.go b/pkg/files/files.go index fbf0ad56..056c2ad4 100644 --- a/pkg/files/files.go +++ b/pkg/files/files.go @@ -3,7 +3,7 @@ package files import ( "encoding/json" "fmt" - "io/ioutil" + "io" "os" "os/exec" "path/filepath" @@ -143,7 +143,7 @@ func ReadString(fs afero.Fs, unsafeFilePathString string) (string, error) { return "", breverrors.WrapAndTrace(err) } - dataBytes, err := ioutil.ReadAll(f) + dataBytes, err := io.ReadAll(f) if err != nil { return "", breverrors.WrapAndTrace(err) } @@ -186,7 +186,7 @@ func OverwriteJSON(fs afero.Fs, filepath string, v interface{}) error { if err != nil { return breverrors.WrapAndTrace(err) } - err = ioutil.WriteFile(filepath, dataBytes, os.ModePerm) + err = os.WriteFile(filepath, dataBytes, os.ModePerm) if err != nil { return breverrors.WrapAndTrace(err) } diff --git a/pkg/huproxyclient/huproxyclient.go b/pkg/huproxyclient/huproxyclient.go index eabfb5c6..66ac9daa 100644 --- a/pkg/huproxyclient/huproxyclient.go +++ b/pkg/huproxyclient/huproxyclient.go @@ -7,7 +7,6 @@ import ( "crypto/tls" "fmt" "io" - "io/ioutil" "net/http" "os" "time" @@ -30,7 +29,7 @@ type HubProxyStore interface { func dialError(url string, resp *http.Response, err error) { if resp != nil { extra := "" - b, err1 := ioutil.ReadAll(resp.Body) + b, err1 := io.ReadAll(resp.Body) if err1 != nil { log.Warningf("Failed to read HTTP body: %v", err1) } diff --git a/pkg/integration/cli_output_compatibility_test.go b/pkg/integration/cli_output_compatibility_test.go index 95dc0d50..93be78a9 100644 --- a/pkg/integration/cli_output_compatibility_test.go +++ b/pkg/integration/cli_output_compatibility_test.go @@ -14,6 +14,45 @@ import ( // These tests verify CLI output formats that external integrations depend on. // Breaking these tests indicates a breaking change that could affect external tools // like NVIDIA Workbench that parse brev CLI output. +// +// NVIDIA AI Workbench Dependencies (CRITICAL - DO NOT BREAK): +// ============================================================ +// Workbench uses the Brev CLI for managing compute instances and depends on: +// +// 1. Commands that MUST exist and maintain their behavior: +// - brev -h / brev --help (to check CLI exists) +// - brev --version (returns X.Y.Z format, with minimum version 0.6.306) +// - brev refresh (refreshes SSH configuration) +// - brev org set (sets active organization) +// - brev org ls (lists organizations with NAME and ID columns) +// - brev ls (lists instances with NAME, STATUS, ID columns) +// - brev ls --org (lists instances for specific org) +// - brev start [url] --org (starts/creates workspace) +// - brev stop (stops workspace) +// +// 2. Output formats that MUST remain stable: +// - brev ls: Table format with columns (in order): NAME, STATUS, ID, MACHINE +// * Workbench has a custom parser that uses column headers to identify cells +// * New columns can be ADDED but existing columns cannot be REMOVED or RENAMED +// * Column order should be maintained for reliable parsing +// - brev --version: Must contain "Current Version:" or "Current version:" +// followed by semantic version X.Y.Z that can be parsed with regex +// - brev org ls: Table format with NAME and ID columns +// * Current org marked with "*" prefix +// +// 3. API endpoints that Workbench depends on: +// - https://api.brev.dev/v1/instance/types +// * Required fields: "type" (string), "stoppable" (boolean) +// * DO NOT remove or rename these fields +// +// 4. Authentication integration: +// - Workbench implements custom KAS login flow that integrates with Brev credentials +// - Credential management must remain compatible with Brev's token storage +// +// References: +// - Workbench CLI integration: https://gitlab-master.nvidia.com/workbench/workbench-cli/-/blob/main/pkg/brev/cli.go +// - Credential manager: https://gitlab-master.nvidia.com/workbench/credential-manager/-/blob/main/pkg/integrations/brev.go +// - Documentation: /Users/kejones/Git/brevdev/notes/brev-cli/workbench-dependencies.md const ( // Regular expressions @@ -198,16 +237,19 @@ func Test_OrgSetCommandExists(t *testing.T) { } // Test_StartCommandFormat verifies that 'brev start' command accepts --org flag +// CRITICAL: Workbench requires the --org flag to specify organization for workspace creation func Test_StartCommandFormat(t *testing.T) { cmd := exec.Command("go", "run", brevCLIPath, "start", "--help") - output, _ := cmd.CombinedOutput() + output, err := cmd.CombinedOutput() + require.NoError(t, err, "brev start --help should succeed") outputStr := string(output) - // Should mention org flag or organization, or at least not be unknown command - hasOrgSupport := strings.Contains(outputStr, "--org") || - strings.Contains(outputStr, "organization") || - (strings.Contains(outputStr, "start") && !strings.Contains(outputStr, "unknown command")) - assert.True(t, hasOrgSupport, "start command should exist and potentially support org specification") + // Verify the command exists + assert.Contains(t, outputStr, "start", "start command should exist") + assert.NotContains(t, outputStr, "unknown command", "start should be a valid command") + + // CRITICAL: Verify --org flag is documented and available + assert.Contains(t, outputStr, "--org", "start command MUST support --org flag for Workbench compatibility") } // Test_StopCommandExists verifies that 'brev stop' command exists @@ -316,3 +358,175 @@ func compareVersions(t *testing.T, installedVersion, minVersion string) bool { } return false } + +// Test_ListWithOrgFlag verifies that 'brev ls --org' flag is recognized +// CRITICAL: Workbench uses this to list instances for specific organizations +func Test_ListWithOrgFlag(t *testing.T) { + cmd := exec.Command("go", "run", brevCLIPath, "ls", "--help") + output, err := cmd.CombinedOutput() + require.NoError(t, err, "brev ls --help should succeed") + + outputStr := string(output) + + // CRITICAL: Verify --org flag exists for ls command + assert.Contains(t, outputStr, "--org", "ls command MUST support --org flag for Workbench compatibility") + + // Verify the flag description mentions organization + lines := strings.Split(outputStr, "\n") + foundOrgLine := false + for _, line := range lines { + if strings.Contains(line, "--org") { + foundOrgLine = true + // The line should mention organization or org + assert.True(t, + strings.Contains(strings.ToLower(line), "org") || + strings.Contains(strings.ToLower(line), "organization"), + "--org flag should have documentation mentioning organization") + break + } + } + assert.True(t, foundOrgLine, "--org flag should be documented in help output") +} + +// Test_ShortHelpFlag verifies that 'brev -h' works +// CRITICAL: Workbench uses 'brev -h' to check if CLI exists +func Test_ShortHelpFlag(t *testing.T) { + cmd := exec.Command("go", "run", brevCLIPath, "-h") + output, err := cmd.CombinedOutput() + require.NoError(t, err, "brev -h should succeed") + + outputStr := string(output) + + // Should show help text + assert.Contains(t, outputStr, "brev", "Help should mention brev") + assert.Contains(t, outputStr, "Usage", "Help should show usage information") + + // Should list essential commands + essentialCommands := []string{"ls", "start", "stop", "org"} + for _, cmd := range essentialCommands { + assert.Contains(t, outputStr, cmd, "Help should list essential command: %s", cmd) + } +} + +// Test_VersionWithNoCheckLatestFlag verifies version flag variations +// CRITICAL: Ensures version command works with various flags Workbench might use +func Test_VersionWithNoCheckLatestFlag(t *testing.T) { + cmd := exec.Command("go", "run", brevCLIPath, "--version", "--no-check-latest") + output, err := cmd.CombinedOutput() + require.NoError(t, err, "brev --version --no-check-latest should succeed") + + outputStr := string(output) + + // Should still show version information (even if it's dev-XXXXXXXX format) + // The important thing is the command doesn't crash or fail + assert.NotEmpty(t, outputStr, "Version command should produce output") + + // For production builds, should contain version information + versionRegexp := regexp.MustCompile(versionPattern) + matches := versionRegexp.FindAllString(outputStr, -1) + + // If we find a semver version, validate it + if len(matches) > 0 { + versionStr := matches[0] + versionParts := strings.Split(versionStr, ".") + assert.Len(t, versionParts, 3, "Version should have exactly 3 components") + } else { + // Dev builds may have "dev-XXXXXXXX" format, which is acceptable + t.Log("Version is in dev format (not semver), which is acceptable for development builds") + } +} + +// Test_InstanceListColumnHeadersStability verifies ALL required columns exist +// CRITICAL: Workbench parser depends on these exact column headers +func Test_InstanceListColumnHeadersStability(t *testing.T) { + cmd := exec.Command("go", "run", brevCLIPath, "ls", "--help") + output, _ := cmd.CombinedOutput() + + // Verify ls command exists + outputStr := string(output) + assert.NotContains(t, outputStr, "unknown command", "ls command must exist") + + // Run actual ls command (may skip if auth fails) + cmd = exec.Command("go", "run", brevCLIPath, "ls") + output, err := cmd.CombinedOutput() + if err != nil { + t.Skip("ls command requires authentication") + return + } + + outputStr = string(output) + + // If there are no instances, the output won't have column headers (which is correct behavior) + if strings.Contains(outputStr, "No instances") { + t.Log("✅ No instances present, skipping column header validation (headers only shown when data exists)") + return + } + + // CRITICAL: These columns MUST exist when instances are present - Workbench parser depends on them + requiredColumns := []string{"NAME", "STATUS", "ID"} + for _, col := range requiredColumns { + assert.Contains(t, outputStr, col, + "CRITICAL: '%s' column MUST exist for Workbench compatibility. DO NOT REMOVE OR RENAME.", col) + } + + // Note: Additional columns can be added, but these core columns must remain + t.Log("✅ All required column headers present. New columns can be added, but existing ones MUST NOT be removed or renamed.") +} + +// Test_OrgListColumnHeadersStability verifies org list column headers +// CRITICAL: Ensures org list output format remains stable for Workbench +func Test_OrgListColumnHeadersStability(t *testing.T) { + cmd := exec.Command("go", "run", brevCLIPath, "org", "ls") + output, err := cmd.CombinedOutput() + if err != nil { + t.Skip("org ls requires authentication") + return + } + + outputStr := string(output) + + // CRITICAL: These columns MUST exist for Workbench + requiredColumns := []string{"NAME", "ID"} + for _, col := range requiredColumns { + assert.Contains(t, outputStr, col, + "CRITICAL: '%s' column MUST exist in org ls for Workbench compatibility.", col) + } +} + +// Test_CommandExistenceForWorkbench verifies all Workbench-critical commands exist +// CRITICAL: Comprehensive check that all commands Workbench depends on are available +func Test_CommandExistenceForWorkbench(t *testing.T) { + criticalCommands := []struct { + name string + args []string + description string + }{ + {"help_short", []string{"-h"}, "check CLI exists"}, + {"help_long", []string{"--help"}, "show full help"}, + {"version", []string{"--version"}, "get version"}, + {"ls", []string{"ls", "--help"}, "list instances"}, + {"org_ls", []string{"org", "ls", "--help"}, "list organizations"}, + {"org_set", []string{"org", "set", "--help"}, "set active org"}, + {"start", []string{"start", "--help"}, "start/create workspace"}, + {"stop", []string{"stop", "--help"}, "stop workspace"}, + {"refresh", []string{"refresh", "--help"}, "refresh SSH config"}, + } + + for _, cmd := range criticalCommands { + t.Run(cmd.name, func(t *testing.T) { + // nolint:gosec // G204: false positive - cmd.args contains hardcoded test values, not user input + execCmd := exec.Command("go", append([]string{"run", brevCLIPath}, cmd.args...)...) + output, err := execCmd.CombinedOutput() + + // Command should execute (may fail with auth, but shouldn't be "unknown command") + outputStr := string(output) + assert.NotContains(t, outputStr, "unknown command", + "CRITICAL: Command '%s' MUST exist for Workbench (%s)", cmd.name, cmd.description) + + // For help commands, should succeed + if strings.Contains(cmd.name, "help") || strings.HasSuffix(cmd.args[len(cmd.args)-1], "--help") { + assert.NoError(t, err, "Help command should succeed: %s", cmd.name) + } + }) + } +} diff --git a/pkg/mergeshells/mergeshells.go b/pkg/mergeshells/mergeshells.go index 6de97c7c..c1ec2774 100644 --- a/pkg/mergeshells/mergeshells.go +++ b/pkg/mergeshells/mergeshells.go @@ -4,7 +4,7 @@ import ( "embed" "errors" "fmt" - "io/ioutil" + "io" "os" "os/exec" "path/filepath" @@ -336,7 +336,7 @@ func importFile(nameVersion string) ([]ShellFragment, error) { return []ShellFragment{}, errors.New(strings.Join([]string{"Path does not exist:", path}, " ")) } } - out, err := ioutil.ReadAll(script) + out, err := io.ReadAll(script) stringScript := string(out) if !noversion { stringScript = strings.ReplaceAll(stringScript, "${version}", subPaths[1]) @@ -556,7 +556,7 @@ func appendPath(a string, b string) string { func recursivelyFindFile(filenames []string, path string) []string { var paths []string - files, err := ioutil.ReadDir(path) + files, err := os.ReadDir(path) if err != nil { fmt.Println(err) } diff --git a/pkg/setupworkspace/setupworkspace.go b/pkg/setupworkspace/setupworkspace.go index a06340dc..dc94d050 100644 --- a/pkg/setupworkspace/setupworkspace.go +++ b/pkg/setupworkspace/setupworkspace.go @@ -5,7 +5,6 @@ import ( "fmt" "io" "io/fs" - "io/ioutil" "log" "os" "os/exec" @@ -50,7 +49,7 @@ func SetupWorkspace(params *store.SetupParamsV0) error { if err != nil { fmt.Println("------ Failure ------") time.Sleep(time.Millisecond * 100) // wait for buffer to be written - logFile, errF := ioutil.ReadFile(logFilePath) + logFile, errF := os.ReadFile(logFilePath) if errF != nil { return multierror.Append(err, errF) } @@ -884,7 +883,7 @@ func (w WorkspaceIniter) SetupCodeServer(password string, bindAddr string, works return breverrors.WrapAndTrace(err) } - configFile, err := ioutil.ReadFile(codeServerConfigPath) //nolint:gosec // secure sandbox + configFile, err := os.ReadFile(codeServerConfigPath) //nolint:gosec // secure sandbox if err != nil { return breverrors.WrapAndTrace(err) } @@ -1025,7 +1024,7 @@ mkdir -vp ~/.vscode-server/bin/"${commit_sha}" # Extract the tarball to the right location. tar --no-same-owner -xzv --strip-components=1 -C ~/.vscode-server/bin/"${commit_sha}" -f "/tmp/${archive}" ` - err := ioutil.WriteFile("/tmp/vscode-install.sh", []byte(script), fs.ModePerm) //nolint:gosec // safe env + err := os.WriteFile("/tmp/vscode-install.sh", []byte(script), fs.ModePerm) //nolint:gosec // safe env if err != nil { return breverrors.WrapAndTrace(err) } diff --git a/pkg/store/authtoken.go b/pkg/store/authtoken.go index 584dd4c8..e40b70c7 100644 --- a/pkg/store/authtoken.go +++ b/pkg/store/authtoken.go @@ -2,7 +2,7 @@ package store import ( "fmt" - "io/ioutil" + "io" "path" "github.com/brevdev/brev-cli/pkg/entity" @@ -92,7 +92,7 @@ func (f FileStore) GetCurrentWorkspaceServiceToken() (string, error) { if err != nil { return "", breverrors.WrapAndTrace(err) } - token, err := ioutil.ReadAll(saTokenFile) + token, err := io.ReadAll(saTokenFile) if err != nil { return "", breverrors.WrapAndTrace(err) } diff --git a/pkg/store/file.go b/pkg/store/file.go index 55ea44ff..d5bdccaf 100644 --- a/pkg/store/file.go +++ b/pkg/store/file.go @@ -8,7 +8,6 @@ import ( "errors" "fmt" "io" - "io/ioutil" "os" "os/exec" "path/filepath" @@ -206,7 +205,7 @@ func (f FileStore) GetCurrentWorkspaceMeta() (*WorkspaceMeta, error) { return nil, breverrors.WrapAndTrace(err) } - fileB, err := ioutil.ReadAll(file) + fileB, err := io.ReadAll(file) if err != nil { return nil, breverrors.WrapAndTrace(err) } diff --git a/pkg/store/workspace.go b/pkg/store/workspace.go index 5190d313..6d9659d1 100644 --- a/pkg/store/workspace.go +++ b/pkg/store/workspace.go @@ -3,7 +3,7 @@ package store import ( "encoding/json" "fmt" - "io/ioutil" + "io" "strings" "github.com/brevdev/brev-cli/pkg/config" @@ -582,7 +582,7 @@ func (f FileStore) GetSetupParams() (*SetupParamsV0, error) { return nil, breverrors.WrapAndTrace(err) } - fileB, err := ioutil.ReadAll(file) + fileB, err := io.ReadAll(file) if err != nil { return nil, breverrors.WrapAndTrace(err) } diff --git a/pkg/workspacemanagerv2/containermanager_test.go b/pkg/workspacemanagerv2/containermanager_test.go index 01363154..9af20299 100644 --- a/pkg/workspacemanagerv2/containermanager_test.go +++ b/pkg/workspacemanagerv2/containermanager_test.go @@ -3,7 +3,6 @@ package workspacemanagerv2 import ( "context" "fmt" - "io/ioutil" "os" "os/exec" "path/filepath" @@ -134,7 +133,7 @@ func Test_Volumes(t *testing.T) { err = cm.StartContainer(ctx, containerID) assert.Nil(t, err) - info, err := ioutil.ReadDir(localPath) + info, err := os.ReadDir(localPath) assert.Nil(t, err) assert.Len(t, info, 2) }