From 5f811d504096c5f1d01c111a4ea3300c0aa2fca5 Mon Sep 17 00:00:00 2001 From: Robby Cochran Date: Fri, 5 Jun 2026 00:32:04 -0700 Subject: [PATCH 1/3] X-Smart-Branch-Parent: main From d23c8dfcb8779c94a860cfbceec327394111a96d Mon Sep 17 00:00:00 2001 From: Robby Cochran Date: Fri, 5 Jun 2026 00:48:21 -0700 Subject: [PATCH 2/3] refactor: Gateway interface split, output standardization, dedup (#23,#24,#27) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Gateway interface (#27): Split into focused sub-interfaces aligned with OpenShell proto domains: ProviderManager, SandboxManager, InferenceConfig, GatewayAdmin. Gateway composes all four. profile.ValidateProviders now accepts ProviderChecker (single method) instead of full Gateway — mock dropped from 24 stubs to 1. Code deduplication (#24): - Extract k8s.DefaultNamespace() — was in 3 cmd files - Extract k8s.FileExists() — was in cmd/providers.go + launcher - Profile mock simplified (24 → 1 stub method) Output standardization (#23): - All cmd/ files now use status.* helpers instead of raw fmt.Print - Added status.Warn, Warnf, Infof, Detailf, Step, Done - creds.go: 15 raw lines → all status helpers - deploy.go: numbered steps via status.Step - teardown.go: all sections via status.Section - providers.go: per-provider status via status.OK/Info - new.go: WARNING → status.Warn, sections → status.Section - teardownProviders: brief retry on sandbox-mid-deletion race TODO.md: migration status updated to 11/12 native Go. Validated: unit 47+7, bats 29+29, podman 19+19, Go+OCP 17/17. Co-Authored-By: Claude Opus 4.6 (1M context) --- TODO.md | 95 ++++++++++++++++++-------------- cmd/creds.go | 33 ++++++----- cmd/deploy.go | 27 ++++----- cmd/new.go | 17 +++--- cmd/providers.go | 37 ++++++------- cmd/teardown.go | 44 ++++++++------- internal/gateway/gateway.go | 71 ++++++++++++------------ internal/k8s/kubectl.go | 12 ++++ internal/profile/profile.go | 9 ++- internal/profile/profile_test.go | 38 ++----------- internal/status/status.go | 9 +++ 11 files changed, 199 insertions(+), 193 deletions(-) diff --git a/TODO.md b/TODO.md index 2073e09..409dfed 100644 --- a/TODO.md +++ b/TODO.md @@ -2,46 +2,22 @@ ## Migration Status -| Command | Go Status | Bash stays? | Notes | -|---------|-----------|-------------|-------| -| `new --local` | Native | Reference only | Profile parsing, provider validation, sandbox create with retry | -| `new --remote` | Bash wrapper | Yes | K8s Job YAML generation, kubectl apply | -| `connect` | Native | Reference only | exec into `openshell sandbox connect` | -| `deploy --local` | Native | Reference only | Podman check, gateway find/select/verify | -| `deploy --remote` | Bash wrapper | Yes | Helm install, route, mTLS, RBAC, SCCs | -| `teardown --sandboxes` | Native | Reference only | SandboxList + SandboxDelete via Gateway | -| `teardown --providers` | Native | Reference only | ProviderList + ProviderDelete + InferenceRemove | -| `teardown --k8s` | Bash wrapper | Yes | Helm uninstall, CRDs, SCCs, secrets, namespace | -| `preflight` | Native | Reference only | All 29 bats tests pass against Go | -| `providers` | Native | Reference only | Eliminates jq dependency | -| `test` | Bash wrapper | Yes | test-flow.sh orchestration | -| **Launcher** | Native | Reference only | In-cluster Go binary, UBI9 + openssh | - -**Score: 8/12 paths native Go.** Remaining 4 are kubectl/helm-heavy operations. - -## Remaining Bash Wrappers - -### `new --remote` — Medium lift -- Generates K8s Job YAML inline (ConfigMap, volumes, env vars) -- Applies via `kubectl apply`, watches Job, tails logs -- Could use Go `text/template` for YAML + `k8s.io/client-go` or `os/exec kubectl` -- Prerequisite: decide whether to depend on client-go or keep shelling out to kubectl - -### `deploy --remote` — Largest lift -- ~160 lines: namespace, CRDs, SCCs (oc adm), Helm install, route, mTLS cert copy -- Uses: kubectl, helm, oc (OpenShift-specific) -- Consider: keep as bash or accept client-go + helm SDK dependency -- Low priority — rarely changes, works reliably - -### `teardown --k8s` — Medium lift -- Helm uninstall, delete CRDs/SCCs/secrets/namespace, gateway config cleanup -- Mirror of deploy --remote in reverse -- Port together with deploy --remote or not at all - -### `test` — Low priority -- test-flow.sh is a bash test harness, not a harness subcommand -- Porting to Go would mean rewriting the test framework -- No value — bash is the right tool for test orchestration +| Command | Go Status | Notes | +|---------|-----------|-------| +| `new --local` | Native | Profile parsing, provider validation, sandbox create with retry | +| `new --remote` | Native | K8s Job YAML via internal/k8s, prerequisite chain (deploy+providers+creds) | +| `connect` | Native | exec into `openshell sandbox connect` | +| `deploy --local` | Native | Podman check, gateway find/select/verify | +| `deploy --remote` | Native | Helm install, Route, mTLS, RBAC, SCCs via internal/k8s | +| `teardown --sandboxes` | Native | SandboxList + SandboxDelete via Gateway | +| `teardown --providers` | Native | ProviderList + ProviderDelete + InferenceRemove | +| `teardown --k8s` | Native | Helm uninstall, CRDs, SCCs, secrets, namespace via internal/k8s | +| `preflight` | Native | All 29 bats tests pass against Go | +| `providers` | Native | Eliminates jq dependency | +| `test` | Bash | test-flow.sh orchestration (intentionally stays bash) | +| **Launcher** | Native | In-cluster Go binary, UBI9 + openssh | + +**Score: 11/12 paths native Go.** Only `test` stays bash (test orchestration, not a user command). ## Architecture Improvements @@ -64,6 +40,45 @@ - Can't share code (different execution context, separate binary) - Consider: extract shared types to a third module, or accept duplication +## Profile Schema — Cross-Project Alignment + +Analysis of field naming and structure against OpenShell provider profiles (#896) +and Kaiden projects (#1272). See `profile.md` for full comparison. + +### Current schema (Config struct) + +``` +name, image, command, keep, providers, [env] +``` + +### Changes + +- [ ] Add `description` field — one line of human-readable context per profile. + Makes multi-profile use cases (`harness new --profile research`) self-documenting. + Both OpenShell and Kaiden include this. + +- [ ] Split `[env]` by purpose — the current `[env]` map mixes inference config + (`ANTHROPIC_*`), agent config (`CLAUDE_CODE_*`), provider metadata + (`JIRA_URL`, `JIRA_USERNAME`), and custom provider workaround paths + (`GOOGLE_WORKSPACE_*`). At minimum, add grouping comments in `default.toml`. + Longer term, consider a `[provider-config]` section for non-secret provider + metadata that belongs with the provider, not the sandbox. The `ANTHROPIC_*` + vars should eventually drop entirely when OpenShell inference automation ships. + +### No changes needed + +- **`name`** — correct term, maps to `openshell sandbox create --name` +- **`image`** — harness-openshell differentiator (Kaiden is folder-first, we're image-first) +- **`command`** — maps to `openshell sandbox create --command` +- **`providers`** — correct term, matches OpenShell's provider naming throughout +- **`keep`** — unique to harness-openshell lifecycle, neither upstream has it, that's fine +- **TOML format** — trivially convertible to YAML (OpenShell) or JSON (Kaiden), no reason to change + +### Future fields (not now) + +- `repo` — git URL to clone into the sandbox at start (Kaiden supports this via `folder`) +- `secrets` — non-provider secrets to inject, cleaner than stuffing credentials into `[env]` + ## Low-Priority Cleanup (from audit) - [ ] `copyFile` in launcher: check Close error (`defer out.Close()` discards it) diff --git a/cmd/creds.go b/cmd/creds.go index cf06664..93805e3 100644 --- a/cmd/creds.go +++ b/cmd/creds.go @@ -9,6 +9,7 @@ import ( "path/filepath" "github.com/robbycochran/harness-openshell/internal/k8s" + "github.com/robbycochran/harness-openshell/internal/status" ) func ensureCreds(namespace string, force bool) error { @@ -19,35 +20,33 @@ func ensureCreds(namespace string, force bool) error { return fmt.Errorf("namespace '%s' not found — run: harness deploy --remote", namespace) } - fmt.Println("=== Setting up cluster credentials ===") - fmt.Printf(" Namespace: %s\n", namespace) - fmt.Println() + status.Section("Setting up cluster credentials") + status.Detailf("Namespace: %s", namespace) // GWS credentials - fmt.Println("=== GWS ===") + status.Section("GWS") if force && kc.SecretExists(ctx, "openshell-gws") { kc.RunKubectl(ctx, "delete", "secret", "openshell-gws") - fmt.Println(" Deleted existing secret.") + status.Info("Deleted existing secret") } if kc.SecretExists(ctx, "openshell-gws") { - fmt.Println(" openshell-gws: exists (use --force to recreate)") + status.Info("openshell-gws: exists (use --force to recreate)") } else { if err := createGWSSecret(ctx, kc); err != nil { - fmt.Printf(" GWS: %v\n", err) + status.Failf("GWS: %v", err) } } // Atlassian credentials - fmt.Println() - fmt.Println("=== Atlassian ===") + status.Section("Atlassian") if force && kc.SecretExists(ctx, "openshell-atlassian") { kc.RunKubectl(ctx, "delete", "secret", "openshell-atlassian") - fmt.Println(" Deleted existing secret.") + status.Info("Deleted existing secret") } if kc.SecretExists(ctx, "openshell-atlassian") { - fmt.Println(" openshell-atlassian: exists (use --force to recreate)") + status.Info("openshell-atlassian: exists (use --force to recreate)") } else if jiraURL := os.Getenv("JIRA_URL"); jiraURL != "" { jiraUser := os.Getenv("JIRA_USERNAME") _, err := kc.RunKubectl(ctx, "create", "secret", "generic", "openshell-atlassian", @@ -56,9 +55,9 @@ func ensureCreds(namespace string, force bool) error { if err != nil { return fmt.Errorf("creating atlassian secret: %w", err) } - fmt.Printf(" openshell-atlassian: created (%s)\n", jiraURL) + status.OKf("openshell-atlassian: created (%s)", jiraURL) } else { - fmt.Println(" Atlassian: JIRA_URL not set (skipping)") + status.Info("Atlassian: JIRA_URL not set (skipping)") } return nil @@ -67,7 +66,7 @@ func ensureCreds(namespace string, force bool) error { func createGWSSecret(ctx context.Context, kc *k8s.Client) error { gwsPath, err := exec.LookPath("gws") if err != nil { - fmt.Println(" GWS: not installed (skipping)") + status.Info("GWS: not installed (skipping)") return nil } @@ -75,7 +74,7 @@ func createGWSSecret(ctx context.Context, kc *k8s.Client) error { check.Stdout = io.Discard check.Stderr = io.Discard if check.Run() != nil { - fmt.Println(" GWS: not authenticated (run 'gws auth login')") + status.Info("GWS: not authenticated (run 'gws auth login')") return nil } @@ -88,7 +87,7 @@ func createGWSSecret(ctx context.Context, kc *k8s.Client) error { credFile := filepath.Join(tmpDir, "credentials.json") out, err := exec.Command(gwsPath, "auth", "export", "--unmasked").Output() if err != nil { - fmt.Println(" GWS: export failed (skipping)") + status.Info("GWS: export failed (skipping)") return nil } if err := os.WriteFile(credFile, out, 0o600); err != nil { @@ -111,6 +110,6 @@ func createGWSSecret(ctx context.Context, kc *k8s.Client) error { if _, err := kc.RunKubectl(ctx, args...); err != nil { return fmt.Errorf("creating gws secret: %w", err) } - fmt.Println(" openshell-gws: created") + status.OK("openshell-gws: created") return nil } diff --git a/cmd/deploy.go b/cmd/deploy.go index 79b2c3d..ba67d0a 100644 --- a/cmd/deploy.go +++ b/cmd/deploy.go @@ -48,10 +48,7 @@ func NewDeployCmd(harnessDir, cli string) *cobra.Command { func deployRemote(harnessDir string, gw gateway.Gateway, kubeconfig string) error { ctx := context.Background() - namespace := os.Getenv("OPENSHELL_NAMESPACE") - if namespace == "" { - namespace = "openshell" - } + namespace := k8s.DefaultNamespace() chartVersion := os.Getenv("OPENSHELL_CHART_VERSION") if chartVersion == "" { @@ -75,7 +72,7 @@ func deployRemote(harnessDir string, gw gateway.Gateway, kubeconfig string) erro kcNoNS := k8s.New(kubeconfig, "") // Step 1: Namespace (idempotent — ignore AlreadyExists) - fmt.Println("=== Step 1: Creating namespace ===") + status.Step(1, "Creating namespace") kcNoNS.RunKubectl(ctx, "create", "ns", namespace) if _, err := kcNoNS.RunKubectl(ctx, "label", "ns", namespace, "pod-security.kubernetes.io/enforce=privileged", @@ -85,14 +82,14 @@ func deployRemote(harnessDir string, gw gateway.Gateway, kubeconfig string) erro } // Step 2: Sandbox CRD - fmt.Println("=== Step 2: Installing Sandbox CRD ===") + status.Step(2, "Installing Sandbox CRD") if err := kcNoNS.RunKubectlPassthrough(ctx, "apply", "-f", "https://github.com/kubernetes-sigs/agent-sandbox/releases/latest/download/manifest.yaml"); err != nil { return fmt.Errorf("installing sandbox CRD: %w", err) } // Step 3: OpenShift SCCs (best-effort — oc may not exist on non-OpenShift) - fmt.Println("=== Step 3: Granting OpenShift SCCs ===") + status.Step(3, "Granting OpenShift SCCs") for _, sa := range []string{"openshell", "openshell-sandbox", "default"} { kc.RunOC(ctx, "adm", "policy", "add-scc-to-user", "privileged", "-z", sa, "-n", namespace) } @@ -138,7 +135,7 @@ func deployRemote(harnessDir string, gw gateway.Gateway, kubeconfig string) erro } // Step 4: Helm install - fmt.Println("=== Step 4: Deploying gateway via Helm ===") + status.Step(4, "Deploying gateway via Helm") sandboxImage := os.Getenv("SANDBOX_IMAGE") if sandboxImage == "" { sandboxImage = "quay.io/rcochran/openshell:sandbox" @@ -168,13 +165,13 @@ func deployRemote(harnessDir string, gw gateway.Gateway, kubeconfig string) erro } // Wait for gateway - fmt.Println("=== Waiting for gateway ===") + status.Section("Waiting for gateway") if err := kc.RunKubectlPassthrough(ctx, "rollout", "status", "statefulset/openshell", "--timeout=300s"); err != nil { return fmt.Errorf("gateway rollout failed: %w", err) } // Step 5: Route - fmt.Println("=== Step 5: Creating OpenShift route ===") + status.Step(5, "Creating OpenShift route") if err := kc.RunKubectlQuiet(ctx, "get", "route", "gateway"); err != nil { kc.ApplyYAML(ctx, map[string]any{ "apiVersion": "route.openshift.io/v1", @@ -190,7 +187,7 @@ func deployRemote(harnessDir string, gw gateway.Gateway, kubeconfig string) erro fmt.Printf(" Route: %s\n", routeHost) // Step 6: CLI gateway config - fmt.Println("=== Step 6: Configuring CLI gateway ===") + status.Step(6, "Configuring CLI gateway") gatewayName := os.Getenv("GATEWAY_NAME") if gatewayName == "" { gatewayName = "openshell-remote-ocp" @@ -229,18 +226,18 @@ func deployRemote(harnessDir string, gw gateway.Gateway, kubeconfig string) erro fmt.Print(" Waiting for gateway...") for i := range 30 { if gw.InferenceGet() == nil { - fmt.Println(" ✓ reachable") + status.OK("reachable") break } time.Sleep(2 * time.Second) fmt.Print(".") if i == 29 { - fmt.Println(" ✗ timed out (try: openshell inference get)") + status.Fail("timed out (try: openshell inference get)") } } fmt.Println() - fmt.Println("Done.") + status.Done("Done.") return nil } @@ -298,6 +295,6 @@ func deployLocal(gw gateway.Gateway) error { } fmt.Println() - fmt.Println("Done.") + status.Done("Done.") return nil } diff --git a/cmd/new.go b/cmd/new.go index e721ab6..a10dee0 100644 --- a/cmd/new.go +++ b/cmd/new.go @@ -77,14 +77,11 @@ type newLocalOpts struct { func newRemote(harnessDir string, gw gateway.Gateway, profileName, sandboxName string) error { ctx := context.Background() - namespace := os.Getenv("OPENSHELL_NAMESPACE") - if namespace == "" { - namespace = "openshell" - } + namespace := k8s.DefaultNamespace() // 1. Ensure gateway if err := gw.InferenceGet(); err != nil { - fmt.Println("=== Deploying gateway ===") + status.Section("Deploying gateway") if err := deployRemote(harnessDir, gw, ""); err != nil { return fmt.Errorf("deploy failed: %w", err) } @@ -93,7 +90,7 @@ func newRemote(harnessDir string, gw gateway.Gateway, profileName, sandboxName s // 2. Ensure providers providers, _ := gw.ProviderList() if len(providers) == 0 { - fmt.Println("\n=== Registering providers ===") + status.Section("Registering providers") if err := registerProviders(harnessDir, gw, false); err != nil { return fmt.Errorf("provider registration failed: %w", err) } @@ -193,7 +190,7 @@ func newRemote(harnessDir string, gw gateway.Gateway, profileName, sandboxName s // 5. Wait for launcher pod fmt.Println() - fmt.Println("Waiting for launcher...") + status.Info("Waiting for launcher...") kc.RunKubectl(ctx, "wait", "--for=condition=ready", "pod", "-l", "job-name="+jobName, "--timeout=120s") @@ -223,7 +220,7 @@ func newRemote(harnessDir string, gw gateway.Gateway, profileName, sandboxName s fmt.Println() if jobStatus == "Complete" || jobStatus == "SuccessCriteriaMet" { - fmt.Printf("Sandbox ready. Connect with: harness connect %s\n", cfg.Name) + status.OKf("Sandbox ready. Connect with: harness connect %s", cfg.Name) return nil } if jobStatus == "" { @@ -250,7 +247,7 @@ func newLocal(opts newLocalOpts) error { // 2. Ensure providers providers, _ := gw.ProviderList() if len(providers) == 0 { - fmt.Println("\n=== Registering providers ===") + status.Section("Registering providers") if err := opts.runScript("providers.sh"); err != nil { return fmt.Errorf("provider registration failed: %w", err) } @@ -281,7 +278,7 @@ func newLocal(opts newLocalOpts) error { } if len(missing) > 0 && len(registered) == 0 { fmt.Println() - fmt.Println("WARNING: no providers available. Run: harness providers") + status.Warn("no providers available — run: harness providers") } // 5. Stage files diff --git a/cmd/providers.go b/cmd/providers.go index 704cc72..c553dd2 100644 --- a/cmd/providers.go +++ b/cmd/providers.go @@ -9,6 +9,7 @@ import ( "strings" "github.com/robbycochran/harness-openshell/internal/gateway" + "github.com/robbycochran/harness-openshell/internal/k8s" "github.com/robbycochran/harness-openshell/internal/status" "github.com/spf13/cobra" ) @@ -46,7 +47,7 @@ func registerProviders(harnessDir string, gw gateway.Gateway, force bool) error gw.ProviderDelete(name) } deleteCustomProfiles(harnessDir, gw) - fmt.Println("Deleted existing providers.") + status.Info("Deleted existing providers") } // Enable providers v2 @@ -59,7 +60,7 @@ func registerProviders(harnessDir string, gw gateway.Gateway, force bool) error status.Section("Importing custom profiles") profilesDir := filepath.Join(harnessDir, "sandbox", "profiles") if err := gw.ProviderProfileImport(profilesDir); err != nil { - fmt.Println(" (already imported)") + status.Info("already imported") } // Register providers @@ -73,12 +74,12 @@ func registerProviders(harnessDir string, gw gateway.Gateway, force bool) error }); err != nil { return fmt.Errorf("creating github provider: %w", err) } - fmt.Println(" github — registered") + status.OK("github: registered") } else { - fmt.Println(" github — exists (use --force to recreate)") + status.Info("github: exists (use --force to recreate)") } } else { - fmt.Println(" github — skipped (GITHUB_TOKEN not set)") + status.Info("github: skipped (GITHUB_TOKEN not set)") } // Vertex AI @@ -97,7 +98,7 @@ func registerProviders(harnessDir string, gw gateway.Gateway, force bool) error project = readADCProject(adcPath) } - if fileExists(adcPath) && project != "" { + if k8s.FileExists(adcPath) && project != "" { if gw.ProviderGet("vertex-local") != nil { if err := gw.ProviderCreate("vertex-local", "google-vertex-ai", gateway.ProviderCreateOpts{ FromADC: true, @@ -108,18 +109,18 @@ func registerProviders(harnessDir string, gw gateway.Gateway, force bool) error }); err != nil { return fmt.Errorf("creating vertex-local provider: %w", err) } - fmt.Printf(" vertex-local — registered (project: %s, region: %s)\n", project, region) + status.OKf("vertex-local: registered (project: %s, region: %s)", project, region) } else { - fmt.Println(" vertex-local — exists (use --force to recreate)") + status.Info("vertex-local: exists (use --force to recreate)") } if err := gw.InferenceSet("vertex-local", model); err != nil { return fmt.Errorf("setting inference: %w", err) } - fmt.Printf(" inference — model: %s\n", model) - } else if !fileExists(adcPath) { - fmt.Printf(" vertex-local — skipped (no ADC file at %s)\n", adcPath) + status.OKf("inference: model %s", model) + } else if !k8s.FileExists(adcPath) { + status.Infof("vertex-local: skipped (no ADC file at %s)", adcPath) } else { - fmt.Println(" vertex-local — skipped (no project ID — set ANTHROPIC_VERTEX_PROJECT_ID or run gcloud auth application-default login)") + status.Info("vertex-local: skipped (no project ID — set ANTHROPIC_VERTEX_PROJECT_ID)") } // Atlassian @@ -130,12 +131,12 @@ func registerProviders(harnessDir string, gw gateway.Gateway, force bool) error }); err != nil { return fmt.Errorf("creating atlassian provider: %w", err) } - fmt.Println(" atlassian — registered") + status.OK("atlassian: registered") } else { - fmt.Println(" atlassian — exists (use --force to recreate)") + status.Info("atlassian: exists (use --force to recreate)") } } else { - fmt.Println(" atlassian — skipped (JIRA_API_TOKEN not set)") + status.Info("atlassian: skipped (JIRA_API_TOKEN not set)") } // Show results @@ -152,7 +153,7 @@ func registerProviders(harnessDir string, gw gateway.Gateway, force bool) error } fmt.Println() - fmt.Println("Done. Launch a sandbox with: harness new --local") + status.Done("Done. Launch a sandbox with: harness new --local") return nil } @@ -203,7 +204,3 @@ func readADCProject(path string) string { return adc.QuotaProjectID } -func fileExists(path string) bool { - _, err := os.Stat(path) - return err == nil -} diff --git a/cmd/teardown.go b/cmd/teardown.go index 8504967..8d88506 100644 --- a/cmd/teardown.go +++ b/cmd/teardown.go @@ -3,8 +3,8 @@ package cmd import ( "context" "fmt" - "os" "strings" + "time" "github.com/robbycochran/harness-openshell/internal/gateway" "github.com/robbycochran/harness-openshell/internal/k8s" @@ -51,7 +51,7 @@ func NewTeardownCmd(harnessDir, cli string) *cobra.Command { teardownK8s(gw) } - fmt.Println("Done.") + status.Done("Done.") return nil }, } @@ -81,7 +81,7 @@ func teardownSandboxes(gw gateway.Gateway, activeGW string) { status.Info("None running") } else { for _, name := range names { - fmt.Printf(" Deleting %s\n", name) + status.Infof("Deleting %s", name) if err := gw.SandboxDelete(name); err != nil { status.Failf("failed to delete %s: %v", name, err) } @@ -103,7 +103,12 @@ func teardownProviders(gw gateway.Gateway, activeGW string) error { return fmt.Errorf("could not check for running sandboxes: %w", err) } if len(remaining) > 0 { - return fmt.Errorf("cannot delete providers with running sandboxes — run: harness teardown --sandboxes") + // Sandbox may be mid-deletion — wait briefly and retry + time.Sleep(2 * time.Second) + remaining, _ = gw.SandboxList() + if len(remaining) > 0 { + return fmt.Errorf("cannot delete providers with running sandboxes — run: harness teardown --sandboxes") + } } names, err := gw.ProviderList() @@ -114,7 +119,7 @@ func teardownProviders(gw gateway.Gateway, activeGW string) error { status.Info("None registered") } else { for _, name := range names { - fmt.Printf(" Deleting %s\n", name) + status.Infof("Deleting %s", name) if err := gw.ProviderDelete(name); err != nil { status.Failf("failed to delete %s: %v", name, err) } @@ -133,22 +138,19 @@ func teardownProviders(gw gateway.Gateway, activeGW string) error { func teardownK8s(gw gateway.Gateway) { ctx := context.Background() - namespace := os.Getenv("OPENSHELL_NAMESPACE") - if namespace == "" { - namespace = "openshell" - } + namespace := k8s.DefaultNamespace() kc := k8s.New("", namespace) kcNoNS := k8s.New("", "") if !kcNoNS.NamespaceExists(ctx, namespace) { - fmt.Println("=== K8s ===") + status.Section("K8s") status.Info("No openshell namespace found, skipping") return } // Helm release - fmt.Println("=== Helm release ===") + status.Section("Helm release") if _, err := kc.RunHelm(ctx, "uninstall", "openshell"); err == nil { status.Info("Uninstalled") } else { @@ -157,7 +159,7 @@ func teardownK8s(gw gateway.Gateway) { // Sandbox CRD namespace fmt.Println() - fmt.Println("=== Sandbox CRD ===") + status.Section("Sandbox CRD") if _, err := kcNoNS.RunKubectl(ctx, "delete", "ns", "agent-sandbox-system"); err == nil { status.Info("Deleted agent-sandbox-system") } else { @@ -166,7 +168,7 @@ func teardownK8s(gw gateway.Gateway) { // OpenShift SCCs fmt.Println() - fmt.Println("=== OpenShift SCCs ===") + status.Section("OpenShift SCCs") for _, sa := range []string{"openshell", "openshell-sandbox", "default"} { kc.RunOC(ctx, "adm", "policy", "remove-scc-from-user", "privileged", "-z", sa, "-n", namespace) } @@ -176,32 +178,32 @@ func teardownK8s(gw gateway.Gateway) { // Secrets fmt.Println() - fmt.Println("=== K8s secrets ===") + status.Section("K8s secrets") for _, secret := range []string{"openshell-gws", "openshell-atlassian"} { if _, err := kc.RunKubectl(ctx, "delete", "secret", secret); err == nil { - fmt.Printf(" Deleted %s\n", secret) + status.OKf("Deleted %s", secret) } else { - fmt.Printf(" %s: not found\n", secret) + status.Infof("%s: not found", secret) } } // Namespace fmt.Println() - fmt.Println("=== Namespace ===") + status.Section("Namespace") if _, err := kcNoNS.RunKubectl(ctx, "delete", "ns", namespace); err == nil { - fmt.Printf(" Deleted %s\n", namespace) + status.OKf("Deleted %s", namespace) } else { - fmt.Printf(" %s: not found\n", namespace) + status.Infof("%s: not found", namespace) } // Gateway config cleanup fmt.Println() - fmt.Println("=== Gateway config ===") + status.Section("Gateway config") gateways, _ := gw.GatewayList() for _, g := range gateways { if !strings.Contains(g.Endpoint, "127.0.0.1") { if err := gw.GatewayRemove(g.Name); err == nil { - fmt.Printf(" Removed gateway '%s'\n", g.Name) + status.OKf("Removed gateway '%s'", g.Name) } } } diff --git a/internal/gateway/gateway.go b/internal/gateway/gateway.go index 34f16b4..b698f1c 100644 --- a/internal/gateway/gateway.go +++ b/internal/gateway/gateway.go @@ -1,62 +1,61 @@ package gateway -// Gateway abstracts all OpenShell gateway operations. The CLI implementation -// shells out to the openshell binary; a future gRPC implementation will call -// the gateway API directly. -type Gateway interface { - // CLIVersion returns the openshell CLI version string, or empty if not found. - CLIVersion() string +// Focused sub-interfaces aligned with OpenShell proto service domains. +// Each consumer imports only the interface it needs, simplifying mocks. - // CLIPath returns the path to the openshell CLI binary, or empty if not found. - CLIPath() string - - // InferenceGet checks if the gateway is active and reachable. - InferenceGet() error - - // InferenceModel returns the configured inference model name, or empty. - InferenceModel() string - - // ActiveGateway returns the name of the currently selected gateway, or empty. - ActiveGateway() string - - // ProviderGet checks if a provider is registered. Returns nil if it exists. +// ProviderManager handles provider CRUD and profile operations. +type ProviderManager interface { ProviderGet(name string) error - - // ProviderList returns the names of all registered providers. - ProviderList() ([]string, error) - - // Provider lifecycle ProviderCreate(name, providerType string, opts ProviderCreateOpts) error ProviderDelete(name string) error + ProviderList() ([]string, error) ProviderProfileImport(dir string) error ProviderProfileDelete(id string) error +} - // Inference config - InferenceSet(provider, model string) error - InferenceRemove() error - - // Settings - SettingsSet(key, value string) error - - // Sandbox lifecycle +// SandboxManager handles sandbox lifecycle operations. +type SandboxManager interface { SandboxList() ([]string, error) SandboxCreate(opts SandboxCreateOpts) error SandboxDelete(name string) error SandboxConnect(name string) error SandboxUpload(name, localDir, remotePath string) error SandboxExec(name string, command ...string) error +} - // Gateway management +// InferenceConfig handles inference routing configuration. +type InferenceConfig interface { + InferenceGet() error + InferenceModel() string + InferenceSet(provider, model string) error + InferenceRemove() error +} + +// GatewayAdmin handles gateway management, CLI detection, and settings. +type GatewayAdmin interface { + CLIVersion() string + CLIPath() string + ActiveGateway() string GatewayAdd(endpoint, name string, local bool) error GatewayRemove(name string) error GatewayList() ([]GatewayInfo, error) GatewaySelect(name string) error + SettingsSet(key, value string) error +} + +// Gateway composes all sub-interfaces. Use this when a function needs +// multiple domains. Prefer the narrower interfaces when possible. +type Gateway interface { + ProviderManager + SandboxManager + InferenceConfig + GatewayAdmin } type ProviderCreateOpts struct { - Credentials []string // "KEY=VALUE" pairs - Configs []string // "KEY=VALUE" pairs - FromADC bool // --from-gcloud-adc + Credentials []string + Configs []string + FromADC bool } type GatewayInfo struct { diff --git a/internal/k8s/kubectl.go b/internal/k8s/kubectl.go index f0d237d..4e08a24 100644 --- a/internal/k8s/kubectl.go +++ b/internal/k8s/kubectl.go @@ -197,3 +197,15 @@ func containsFlag(args []string, flags ...string) bool { func decodeBase64(s string) ([]byte, error) { return base64.StdEncoding.DecodeString(s) } + +func DefaultNamespace() string { + if ns := os.Getenv("OPENSHELL_NAMESPACE"); ns != "" { + return ns + } + return "openshell" +} + +func FileExists(path string) bool { + _, err := os.Stat(path) + return err == nil +} diff --git a/internal/profile/profile.go b/internal/profile/profile.go index b8b9bfb..494a5ac 100644 --- a/internal/profile/profile.go +++ b/internal/profile/profile.go @@ -9,9 +9,14 @@ import ( "sort" "strings" - "github.com/robbycochran/harness-openshell/internal/gateway" ) +// ProviderChecker checks if a provider is registered. Accepts gateway.ProviderManager +// or any type with a ProviderGet method — decouples profile from the full Gateway interface. +type ProviderChecker interface { + ProviderGet(name string) error +} + type Config struct { Name string `toml:"name"` Image string `toml:"image"` @@ -46,7 +51,7 @@ func (c *Config) BuildSandboxEnv() string { // ValidateProviders checks which profile providers are registered on the // gateway. Returns the list of registered providers and a list of missing ones. -func ValidateProviders(providers []string, gw gateway.Gateway) (registered, missing []string) { +func ValidateProviders(providers []string, gw ProviderChecker) (registered, missing []string) { for _, name := range providers { if gw.ProviderGet(name) == nil { registered = append(registered, name) diff --git a/internal/profile/profile_test.go b/internal/profile/profile_test.go index e2a4292..1a249e9 100644 --- a/internal/profile/profile_test.go +++ b/internal/profile/profile_test.go @@ -6,46 +6,20 @@ import ( "path/filepath" "strings" "testing" - - "github.com/robbycochran/harness-openshell/internal/gateway" ) -// mockGateway implements gateway.Gateway for testing provider validation. -type mockGateway struct { +// mockProviderChecker implements profile.ProviderChecker for testing. +type mockProviderChecker struct { providers map[string]bool } -func (m *mockGateway) ProviderGet(name string) error { +func (m *mockProviderChecker) ProviderGet(name string) error { if m.providers[name] { return nil } return fmt.Errorf("not found") } -func (m *mockGateway) CLIVersion() string { return "" } -func (m *mockGateway) CLIPath() string { return "" } -func (m *mockGateway) InferenceGet() error { return nil } -func (m *mockGateway) InferenceModel() string { return "" } -func (m *mockGateway) InferenceSet(string, string) error { return nil } -func (m *mockGateway) InferenceRemove() error { return nil } -func (m *mockGateway) ActiveGateway() string { return "" } -func (m *mockGateway) ProviderCreate(string, string, gateway.ProviderCreateOpts) error { return nil } -func (m *mockGateway) ProviderDelete(string) error { return nil } -func (m *mockGateway) ProviderProfileImport(string) error { return nil } -func (m *mockGateway) ProviderProfileDelete(string) error { return nil } -func (m *mockGateway) ProviderList() ([]string, error) { return nil, nil } -func (m *mockGateway) SettingsSet(string, string) error { return nil } -func (m *mockGateway) SandboxList() ([]string, error) { return nil, nil } -func (m *mockGateway) SandboxCreate(gateway.SandboxCreateOpts) error { return nil } -func (m *mockGateway) SandboxDelete(string) error { return nil } -func (m *mockGateway) SandboxConnect(string) error { return nil } -func (m *mockGateway) SandboxUpload(string, string, string) error { return nil } -func (m *mockGateway) SandboxExec(string, ...string) error { return nil } -func (m *mockGateway) GatewayAdd(string, string, bool) error { return nil } -func (m *mockGateway) GatewayRemove(string) error { return nil } -func (m *mockGateway) GatewayList() ([]gateway.GatewayInfo, error) { return nil, nil } -func (m *mockGateway) GatewaySelect(string) error { return nil } - func TestParseFile_Full(t *testing.T) { dir := t.TempDir() path := filepath.Join(dir, "config.toml") @@ -177,7 +151,7 @@ func TestStageHarnessDir(t *testing.T) { } func TestValidateProviders_AllRegistered(t *testing.T) { - gw := &mockGateway{providers: map[string]bool{"github": true, "vertex-local": true}} + gw := &mockProviderChecker{providers: map[string]bool{"github": true, "vertex-local": true}} reg, missing := ValidateProviders([]string{"github", "vertex-local"}, gw) if len(reg) != 2 { t.Errorf("registered = %v, want 2", reg) @@ -188,7 +162,7 @@ func TestValidateProviders_AllRegistered(t *testing.T) { } func TestValidateProviders_SomeMissing(t *testing.T) { - gw := &mockGateway{providers: map[string]bool{"github": true}} + gw := &mockProviderChecker{providers: map[string]bool{"github": true}} reg, missing := ValidateProviders([]string{"github", "vertex-local", "atlassian"}, gw) if len(reg) != 1 || reg[0] != "github" { t.Errorf("registered = %v", reg) @@ -199,7 +173,7 @@ func TestValidateProviders_SomeMissing(t *testing.T) { } func TestValidateProviders_NoneRegistered(t *testing.T) { - gw := &mockGateway{providers: map[string]bool{}} + gw := &mockProviderChecker{providers: map[string]bool{}} reg, missing := ValidateProviders([]string{"github", "vertex-local"}, gw) if len(reg) != 0 { t.Errorf("registered = %v, want empty", reg) diff --git a/internal/status/status.go b/internal/status/status.go index e81cea8..3f715b0 100644 --- a/internal/status/status.go +++ b/internal/status/status.go @@ -6,9 +6,14 @@ func OK(msg string) { fmt.Println(" ✓ " + msg) } func OKf(format string, a ...any) { fmt.Printf(" ✓ "+format+"\n", a...) } func Fail(msg string) { fmt.Println(" ✗ " + msg) } func Failf(format string, a ...any) { fmt.Printf(" ✗ "+format+"\n", a...) } +func Warn(msg string) { fmt.Println(" ! " + msg) } +func Warnf(format string, a ...any) { fmt.Printf(" ! "+format+"\n", a...) } func Info(msg string) { fmt.Println(" - " + msg) } +func Infof(format string, a ...any) { fmt.Printf(" - "+format+"\n", a...) } func Detail(msg string) { fmt.Println(" " + msg) } +func Detailf(format string, a ...any){ fmt.Printf(" "+format+"\n", a...) } func Sub(msg string) { fmt.Println(" " + msg) } +func Step(n int, msg string) { fmt.Printf("\n=== Step %d: %s ===\n", n, msg) } func Section(title string) { fmt.Printf("\n=== %s ===\n", title) } func Summary(ok bool) { if ok { @@ -17,3 +22,7 @@ func Summary(ok bool) { fmt.Println("✗ Not ready — fix issues above") } } +func Done(msg string) { + fmt.Println() + fmt.Println(msg) +} From 818547ae6600f66e473ff8f67aef9ee2f404e826 Mon Sep 17 00:00:00 2001 From: Robby Cochran Date: Fri, 5 Jun 2026 00:54:30 -0700 Subject: [PATCH 3/3] refactor: move FileExists to internal/util (not k8s-specific) Co-Authored-By: Claude Opus 4.6 (1M context) --- cmd/providers.go | 6 +++--- internal/k8s/kubectl.go | 4 ---- internal/util/util.go | 8 ++++++++ 3 files changed, 11 insertions(+), 7 deletions(-) create mode 100644 internal/util/util.go diff --git a/cmd/providers.go b/cmd/providers.go index c553dd2..1062f2e 100644 --- a/cmd/providers.go +++ b/cmd/providers.go @@ -9,8 +9,8 @@ import ( "strings" "github.com/robbycochran/harness-openshell/internal/gateway" - "github.com/robbycochran/harness-openshell/internal/k8s" "github.com/robbycochran/harness-openshell/internal/status" + "github.com/robbycochran/harness-openshell/internal/util" "github.com/spf13/cobra" ) @@ -98,7 +98,7 @@ func registerProviders(harnessDir string, gw gateway.Gateway, force bool) error project = readADCProject(adcPath) } - if k8s.FileExists(adcPath) && project != "" { + if util.FileExists(adcPath) && project != "" { if gw.ProviderGet("vertex-local") != nil { if err := gw.ProviderCreate("vertex-local", "google-vertex-ai", gateway.ProviderCreateOpts{ FromADC: true, @@ -117,7 +117,7 @@ func registerProviders(harnessDir string, gw gateway.Gateway, force bool) error return fmt.Errorf("setting inference: %w", err) } status.OKf("inference: model %s", model) - } else if !k8s.FileExists(adcPath) { + } else if !util.FileExists(adcPath) { status.Infof("vertex-local: skipped (no ADC file at %s)", adcPath) } else { status.Info("vertex-local: skipped (no project ID — set ANTHROPIC_VERTEX_PROJECT_ID)") diff --git a/internal/k8s/kubectl.go b/internal/k8s/kubectl.go index 4e08a24..b1f7fae 100644 --- a/internal/k8s/kubectl.go +++ b/internal/k8s/kubectl.go @@ -205,7 +205,3 @@ func DefaultNamespace() string { return "openshell" } -func FileExists(path string) bool { - _, err := os.Stat(path) - return err == nil -} diff --git a/internal/util/util.go b/internal/util/util.go new file mode 100644 index 0000000..473a8b1 --- /dev/null +++ b/internal/util/util.go @@ -0,0 +1,8 @@ +package util + +import "os" + +func FileExists(path string) bool { + _, err := os.Stat(path) + return err == nil +}