diff --git a/cmd/delete.go b/cmd/delete.go index a9513cb..c8ee19e 100644 --- a/cmd/delete.go +++ b/cmd/delete.go @@ -1,21 +1,26 @@ package cmd import ( + "context" "fmt" + "os" + "time" + "github.com/spf13/cobra" "github.com/stackrox/harness-openshell/internal/gateway" "github.com/stackrox/harness-openshell/internal/k8s" + "github.com/stackrox/harness-openshell/internal/openshell" "github.com/stackrox/harness-openshell/internal/status" - "github.com/spf13/cobra" ) -func NewDeleteCmd(harnessDir, cli string) *cobra.Command { +func NewDeleteCmd(harnessDir, cli string, newClient openshell.Factory) *cobra.Command { var ( all bool sandboxes bool providers bool k8sFlag bool ) + var gatewayName, workspace *string cmd := &cobra.Command{ Use: "delete [NAME...] [--all] [--providers] [--k8s]", @@ -33,12 +38,27 @@ Examples: return fmt.Errorf("specify sandbox name(s) or use --all, --sandboxes, --providers, --k8s") } - gw := gateway.New(cli) + ctx := cmd.Context() + target := openshell.ResolveTarget(*gatewayName, *workspace, "", "", os.Getenv) + + // The --k8s branch is CLI/kubectl-backed and needs no SDK client; + // open (and dial) one only when a sandbox/provider path will use it, + // so `delete --k8s` still works when the OpenShell API is down. + needsSDK := len(args) > 0 || all || sandboxes || providers + var client openshell.Client + if needsSDK { + var err error + client, err = newClient(ctx, target) + if err != nil { + return fmt.Errorf("create OpenShell client: %w", err) + } + defer client.Close() + } // Targeted sandbox deletion if len(args) > 0 { for _, name := range args { - if err := gw.SandboxDelete(name); err != nil { + if err := client.DeleteSandbox(ctx, name); err != nil { status.Failf("%s: %v", name, err) } else { status.OKf("Deleted sandbox %s", name) @@ -49,23 +69,26 @@ Examples: } } - activeGW := gw.ActiveGateway() - if activeGW != "" { - status.Infof("Active gateway: %s", activeGW) + if target.Gateway != "" { + status.Infof("Active gateway: %s", target.Gateway) } else { status.Info("Active gateway: none") } fmt.Println() if all || sandboxes { - teardownSandboxes(gw, activeGW) + deleteSandboxesSDK(ctx, client, target.Gateway) } if all || providers { - if err := teardownProviders(gw, activeGW); err != nil { + if err := deleteProvidersSDK(ctx, client, target.Gateway); err != nil { return err } } if all || k8sFlag { + // internal/gateway residual: the --k8s path stays CLI-backed + // until PR7b retires the legacy bridge. This is the only + // sanctioned use of internal/gateway and internal/k8s in delete. + gw := gateway.New(cli) ns := k8s.DefaultNamespace() gwCfg := resolveFirstRemoteGateway(harnessDir) teardownK8s(gw, gwCfg, k8s.New("", ns), k8s.New("", "")) @@ -80,6 +103,87 @@ Examples: cmd.Flags().BoolVar(&sandboxes, "sandboxes", false, "Delete all sandboxes") cmd.Flags().BoolVar(&providers, "providers", false, "Delete all providers") cmd.Flags().BoolVar(&k8sFlag, "k8s", false, "Delete k8s resources") + gatewayName, workspace = registerTargetFlags(cmd) return cmd } + +// deleteSandboxesSDK sweeps every sandbox in the target workspace over the +// OpenShell SDK. It is delete's own SDK-backed sweep, intentionally mirroring +// teardownSandboxes (cmd/teardown.go) on a different backing; the duplication is +// a short-lived seam removed in PR7b when the CLI helper and teardown command +// are retired, leaving this the single owner of the sweep. +func deleteSandboxesSDK(ctx context.Context, client openshell.Client, activeGW string) { + status.Section("Sandboxes") + if activeGW == "" { + status.Info("No active gateway, skipping") + fmt.Println() + return + } + + sandboxes, err := client.Sandboxes(ctx) + if err != nil { + status.Fail(fmt.Sprintf("could not list sandboxes: %v", err)) + fmt.Println() + return + } + if len(sandboxes) == 0 { + status.Info("None running") + } else { + for _, s := range sandboxes { + status.Infof("Deleting %s", s.Name) + if err := client.DeleteSandbox(ctx, s.Name); err != nil { + status.Failf("failed to delete %s: %v", s.Name, err) + } + } + } + fmt.Println() +} + +// deleteProvidersSDK sweeps every provider over the SDK, preserving the +// running-sandbox guard from teardownProviders (cmd/teardown.go): providers are +// refused while any sandbox is still up, with one brief retry to absorb a +// mid-deletion race. Like deleteSandboxesSDK this is a short-lived duplicate of +// the CLI helper, collapsed to the single owner in PR7b. +func deleteProvidersSDK(ctx context.Context, client openshell.Client, activeGW string) error { + status.Section("Providers") + if activeGW == "" { + status.Info("No active gateway, skipping") + fmt.Println() + return nil + } + + remaining, err := client.Sandboxes(ctx) + if err != nil { + return fmt.Errorf("could not check for running sandboxes: %w", err) + } + if len(remaining) > 0 { + // Sandbox may be mid-deletion — wait briefly and retry. + time.Sleep(2 * time.Second) + remaining, err = client.Sandboxes(ctx) + if err != nil { + return fmt.Errorf("rechecking sandboxes: %w", err) + } + if len(remaining) > 0 { + return fmt.Errorf("cannot delete providers with running sandboxes — run: harness delete --sandboxes") + } + } + + providers, err := client.Providers(ctx) + if err != nil { + return fmt.Errorf("could not list providers: %w", err) + } + if len(providers) == 0 { + status.Info("None registered") + } else { + for _, p := range providers { + status.Infof("Deleting %s", p.Name) + if err := client.DeleteProvider(ctx, p.Name); err != nil { + status.Failf("failed to delete %s: %v", p.Name, err) + } + } + } + + fmt.Println() + return nil +} diff --git a/cmd/delete_test.go b/cmd/delete_test.go new file mode 100644 index 0000000..bbaf44d --- /dev/null +++ b/cmd/delete_test.go @@ -0,0 +1,116 @@ +package cmd + +import ( + "context" + "testing" + + "github.com/NVIDIA/OpenShell/sdk/go/openshell/v1/types" + + "github.com/stackrox/harness-openshell/internal/openshell" + "github.com/stackrox/harness-openshell/internal/testutil" +) + +// The delete tests use keepOpenFactory (executor_inference_test.go) so the +// command's deferred Close doesn't shut the shared fake before the test can +// assert the resources were actually removed, not merely that a log line printed. + +func sandboxNames(t *testing.T, c openshell.Client) []string { + t.Helper() + sandboxes, err := c.Sandboxes(context.Background()) + if err != nil { + t.Fatalf("list sandboxes: %v", err) + } + names := make([]string, len(sandboxes)) + for i, s := range sandboxes { + names[i] = s.Name + } + return names +} + +func providerNames(t *testing.T, c openshell.Client) []string { + t.Helper() + providers, err := c.Providers(context.Background()) + if err != nil { + t.Fatalf("list providers: %v", err) + } + names := make([]string, len(providers)) + for i, p := range providers { + names[i] = p.Name + } + return names +} + +func TestDeleteTargeted(t *testing.T) { + client, fc := testutil.NewFakeClient("default") + fc.AddSandbox("default", &types.Sandbox{Name: "agent-a", Status: types.SandboxStatus{Phase: types.SandboxReady}}) + fc.AddSandbox("default", &types.Sandbox{Name: "agent-b", Status: types.SandboxStatus{Phase: types.SandboxReady}}) + + cmd := NewDeleteCmd("", "", keepOpenFactory(client)) + cmd.SetArgs([]string{"agent-a"}) + if _, err := captureStdout(t, cmd.Execute); err != nil { + t.Fatalf("delete agent-a: %v", err) + } + + remaining := sandboxNames(t, client) + if len(remaining) != 1 || remaining[0] != "agent-b" { + t.Errorf("targeted delete should remove only agent-a, got %v", remaining) + } +} + +func TestDeleteSandboxesSweep(t *testing.T) { + client, fc := testutil.NewFakeClient("default") + fc.AddSandbox("default", &types.Sandbox{Name: "agent-a", Status: types.SandboxStatus{Phase: types.SandboxReady}}) + fc.AddSandbox("default", &types.Sandbox{Name: "agent-b", Status: types.SandboxStatus{Phase: types.SandboxReady}}) + + cmd := NewDeleteCmd("", "", keepOpenFactory(client)) + cmd.SetArgs([]string{"--sandboxes", "--gateway", "prod"}) + if _, err := captureStdout(t, cmd.Execute); err != nil { + t.Fatalf("delete --sandboxes: %v", err) + } + + if remaining := sandboxNames(t, client); len(remaining) != 0 { + t.Errorf("--sandboxes should sweep every sandbox, got %v", remaining) + } +} + +func TestDeleteProvidersGuard(t *testing.T) { + client, fc := testutil.NewFakeClient("default") + fc.AddSandbox("default", &types.Sandbox{Name: "agent-a", Status: types.SandboxStatus{Phase: types.SandboxReady}}) + fc.AddProvider("default", &types.Provider{Name: "github", Type: "github"}) + + cmd := NewDeleteCmd("", "", keepOpenFactory(client)) + cmd.SetArgs([]string{"--providers", "--gateway", "prod"}) + _, err := captureStdout(t, cmd.Execute) + if err == nil { + t.Fatal("deleting providers with a running sandbox should be refused") + } + if !contains(err.Error(), "running sandboxes") { + t.Errorf("unexpected guard error: %v", err) + } + + // The guard must prevent deletion, not delete-then-error: the provider survives. + if names := providerNames(t, client); len(names) != 1 || names[0] != "github" { + t.Errorf("guard should leave the provider untouched, got %v", names) + } +} + +// Note: `delete --k8s`-only skipping the SDK client (CodeRabbit finding) is not +// unit-tested — the --k8s path invokes the real, non-injectable teardownK8s, +// which shells out to the ambient kubeconfig and would destructively act on a +// live cluster. The gating (`needsSDK`) is a simple guard in delete.go. + +func TestDeleteProvidersSweep(t *testing.T) { + client, fc := testutil.NewFakeClient("default") + fc.AddProvider("default", &types.Provider{Name: "github", Type: "github"}) + fc.AddProvider("default", &types.Provider{Name: "vertex", Type: "google-vertex-ai"}) + + cmd := NewDeleteCmd("", "", keepOpenFactory(client)) + cmd.SetArgs([]string{"--providers", "--gateway", "prod"}) + if _, err := captureStdout(t, cmd.Execute); err != nil { + t.Fatalf("delete --providers: %v", err) + } + + if names := providerNames(t, client); len(names) != 0 { + t.Errorf("--providers should sweep every provider, got %v", names) + } +} diff --git a/cmd/describe.go b/cmd/describe.go index 4c0fd4f..6a1533f 100644 --- a/cmd/describe.go +++ b/cmd/describe.go @@ -1,15 +1,17 @@ package cmd import ( + "errors" "fmt" - "github.com/stackrox/harness-openshell/internal/gateway" - "github.com/stackrox/harness-openshell/internal/status" "github.com/spf13/cobra" + "github.com/stackrox/harness-openshell/internal/openshell" + "github.com/stackrox/harness-openshell/internal/status" ) -func NewDescribeCmd(harnessDir, cli string) *cobra.Command { +func NewDescribeCmd(newClient openshell.Factory) *cobra.Command { var output string + var gatewayName, workspace *string cmd := &cobra.Command{ Use: "describe [NAME]", @@ -17,42 +19,41 @@ func NewDescribeCmd(harnessDir, cli string) *cobra.Command { Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { name := args[0] - gw := gateway.New(cli) - sandboxes, err := gw.SandboxStatus() + format, err := parseOutputFormat(output) if err != nil { - return fmt.Errorf("listing sandboxes: %w", err) + return err } - var found *gateway.SandboxInfo - for i := range sandboxes { - if sandboxes[i].Name == name { - found = &sandboxes[i] - break - } - } - if found == nil { - return fmt.Errorf("sandbox %q not found", name) + client, err := openClient(cmd.Context(), newClient, gatewayName, workspace) + if err != nil { + return fmt.Errorf("create OpenShell client: %w", err) } + defer client.Close() - // Find active gateway - var activeGW *gateway.GatewayInfo - gateways, err := gw.GatewayList() - if err == nil { - for i := range gateways { - if gateways[i].Active { - activeGW = &gateways[i] - break - } + sandbox, err := client.GetSandbox(cmd.Context(), name) + if err != nil { + if errors.Is(err, openshell.ErrNotFound) { + return fmt.Errorf("sandbox %q not found", name) } + return fmt.Errorf("reading sandbox: %w", err) } - // Find providers - providers, _ := gw.ProviderList() + // Gateway context and providers are best-effort: a describe still + // shows the sandbox even if gateway introspection or the provider + // list fails (behavior-preserving with the former CLI path). + var gwName, gwEndpoint string + if info, err := client.GatewayInfo(cmd.Context()); err == nil { + gwName = info.Name + gwEndpoint = info.Endpoint + } - format, err := parseOutputFormat(output) - if err != nil { - return err + var providerNames []string + if providers, err := client.Providers(cmd.Context()); err == nil { + providerNames = make([]string, len(providers)) + for i, p := range providers { + providerNames[i] = p.Name + } } if format != formatTable { @@ -63,28 +64,25 @@ func NewDescribeCmd(harnessDir, cli string) *cobra.Command { Endpoint string `json:"endpoint,omitempty" yaml:"endpoint,omitempty"` Providers []string `json:"providers,omitempty" yaml:"providers,omitempty"` } - out := describeOut{ - Name: found.Name, - Phase: found.Phase, - Providers: providers, - } - if activeGW != nil { - out.Gateway = activeGW.Name - out.Endpoint = activeGW.Endpoint - } - return printStructured(format, out) + return printStructured(format, describeOut{ + Name: sandbox.Name, + Phase: sandbox.Phase, + Gateway: gwName, + Endpoint: gwEndpoint, + Providers: providerNames, + }) } - status.Header(found.Name) - status.Infof("Phase: %s", found.Phase) + status.Header(sandbox.Name) + status.Infof("Phase: %s", sandbox.Phase) - if activeGW != nil { - status.Infof("Gateway: %s (%s)", activeGW.Name, activeGW.Endpoint) + if gwName != "" { + status.Infof("Gateway: %s (%s)", gwName, gwEndpoint) } - if len(providers) > 0 { - status.Infof("Providers: %d registered", len(providers)) - for _, p := range providers { + if len(providerNames) > 0 { + status.Infof("Providers: %d registered", len(providerNames)) + for _, p := range providerNames { fmt.Printf(" - %s\n", p) } } @@ -94,5 +92,6 @@ func NewDescribeCmd(harnessDir, cli string) *cobra.Command { } cmd.Flags().StringVarP(&output, "output", "o", "", "Output format: table, json, or yaml") + gatewayName, workspace = registerTargetFlags(cmd) return cmd } diff --git a/cmd/describe_test.go b/cmd/describe_test.go new file mode 100644 index 0000000..dccc04a --- /dev/null +++ b/cmd/describe_test.go @@ -0,0 +1,59 @@ +package cmd + +import ( + "testing" + + fake "github.com/NVIDIA/OpenShell/sdk/go/openshell/v1/fake" + "github.com/NVIDIA/OpenShell/sdk/go/openshell/v1/types" + + "github.com/stackrox/harness-openshell/internal/testutil" +) + +func TestDescribeSandbox(t *testing.T) { + client, fc := testutil.NewFakeClient("default", fake.WithGatewayInfo(&types.GatewayInfo{ + Status: types.ServiceStatusHealthy, + Version: "0.0.110", + })) + fc.AddSandbox("default", &types.Sandbox{Name: "agent-a", Status: types.SandboxStatus{Phase: types.SandboxReady}}) + fc.AddProvider("default", &types.Provider{Name: "github", Type: "github"}) + + cmd := NewDescribeCmd(testutil.FakeFactory(client)) + cmd.SetArgs([]string{"agent-a"}) + out, err := captureStdout(t, cmd.Execute) + if err != nil { + t.Fatalf("describe: %v", err) + } + for _, want := range []string{"agent-a", "Ready", "github"} { + if !contains(out, want) { + t.Errorf("describe output missing %q:\n%s", want, out) + } + } +} + +func TestDescribeSandboxNotFound(t *testing.T) { + client, _ := testutil.NewFakeClient("default") + cmd := NewDescribeCmd(testutil.FakeFactory(client)) + cmd.SetArgs([]string{"nope"}) + _, err := captureStdout(t, cmd.Execute) + if err == nil { + t.Fatal("describe of a missing sandbox should error") + } + if !contains(err.Error(), `sandbox "nope" not found`) { + t.Errorf("unexpected error: %v", err) + } +} + +func TestDescribeSandboxJSON(t *testing.T) { + client, fc := testutil.NewFakeClient("default") + fc.AddSandbox("default", &types.Sandbox{Name: "agent-a", Status: types.SandboxStatus{Phase: types.SandboxReady}}) + + cmd := NewDescribeCmd(testutil.FakeFactory(client)) + cmd.SetArgs([]string{"agent-a", "-o", "json"}) + out, err := captureStdout(t, cmd.Execute) + if err != nil { + t.Fatalf("describe -o json: %v", err) + } + if !contains(out, `"name": "agent-a"`) || !contains(out, `"phase": "Ready"`) { + t.Errorf("json output missing fields:\n%s", out) + } +} diff --git a/cmd/get.go b/cmd/get.go index 4a815cc..2185d8a 100644 --- a/cmd/get.go +++ b/cmd/get.go @@ -3,11 +3,11 @@ package cmd import ( "fmt" - "github.com/stackrox/harness-openshell/internal/gateway" "github.com/spf13/cobra" + "github.com/stackrox/harness-openshell/internal/openshell" ) -func NewGetCmd(harnessDir, cli string) *cobra.Command { +func NewGetCmd(newClient openshell.Factory) *cobra.Command { cmd := &cobra.Command{ Use: "get", Short: "Display resources", @@ -15,16 +15,17 @@ func NewGetCmd(harnessDir, cli string) *cobra.Command { } cmd.AddCommand( - newGetAgentsCmd(cli), - newGetProvidersCmd(cli), - newGetGatewaysCmd(cli), + newGetAgentsCmd(newClient), + newGetProvidersCmd(newClient), + newGetGatewaysCmd(newClient), ) return cmd } -func newGetAgentsCmd(cli string) *cobra.Command { +func newGetAgentsCmd(newClient openshell.Factory) *cobra.Command { var output string + var gatewayName, workspace *string cmd := &cobra.Command{ Use: "agents", @@ -36,8 +37,13 @@ func newGetAgentsCmd(cli string) *cobra.Command { return err } - gw := gateway.New(cli) - sandboxes, err := gw.SandboxStatus() + client, err := openClient(cmd.Context(), newClient, gatewayName, workspace) + if err != nil { + return fmt.Errorf("create OpenShell client: %w", err) + } + defer client.Close() + + sandboxes, err := client.Sandboxes(cmd.Context()) if err != nil { return fmt.Errorf("listing sandboxes: %w", err) } @@ -73,11 +79,13 @@ func newGetAgentsCmd(cli string) *cobra.Command { } cmd.Flags().StringVarP(&output, "output", "o", "", "Output format: table, json, or yaml") + gatewayName, workspace = registerTargetFlags(cmd) return cmd } -func newGetProvidersCmd(cli string) *cobra.Command { +func newGetProvidersCmd(newClient openshell.Factory) *cobra.Command { var output string + var gatewayName, workspace *string cmd := &cobra.Command{ Use: "providers", @@ -89,8 +97,13 @@ func newGetProvidersCmd(cli string) *cobra.Command { return err } - gw := gateway.New(cli) - providers, err := gw.ProviderList() + client, err := openClient(cmd.Context(), newClient, gatewayName, workspace) + if err != nil { + return fmt.Errorf("create OpenShell client: %w", err) + } + defer client.Close() + + providers, err := client.Providers(cmd.Context()) if err != nil { return fmt.Errorf("listing providers: %w", err) } @@ -110,14 +123,14 @@ func newGetProvidersCmd(cli string) *cobra.Command { } out := make([]providerOut, len(providers)) for i, p := range providers { - out[i] = providerOut{Name: p} + out[i] = providerOut{Name: p.Name} } return printStructured(format, out) } rows := make([][]string, len(providers)) for i, p := range providers { - rows[i] = []string{p} + rows[i] = []string{p.Name} } printTable([]string{"Name"}, rows) return nil @@ -125,63 +138,64 @@ func newGetProvidersCmd(cli string) *cobra.Command { } cmd.Flags().StringVarP(&output, "output", "o", "", "Output format: table, json, or yaml") + gatewayName, workspace = registerTargetFlags(cmd) return cmd } -func newGetGatewaysCmd(cli string) *cobra.Command { +func newGetGatewaysCmd(newClient openshell.Factory) *cobra.Command { var output string + var gatewayName, workspace *string cmd := &cobra.Command{ Use: "gateways", Aliases: []string{"gateway", "gw"}, - Short: "List gateways", + Short: "Show the active gateway", + Long: `Show the active OpenShell gateway (name, endpoint, status, version). + +The OpenShell SDK has no gateway-list RPC, so this reports the single gateway the +client is bound to (via --gateway or $OPENSHELL_GATEWAY), not every configured +registration.`, RunE: func(cmd *cobra.Command, args []string) error { format, err := parseOutputFormat(output) if err != nil { return err } - gw := gateway.New(cli) - gateways, err := gw.GatewayList() + client, err := openClient(cmd.Context(), newClient, gatewayName, workspace) if err != nil { - return fmt.Errorf("listing gateways: %w", err) + return fmt.Errorf("create OpenShell client: %w", err) } + defer client.Close() - if len(gateways) == 0 { - if format == formatTable { - fmt.Println("No gateways registered.") - } else { - return printStructured(format, []any{}) - } - return nil + info, err := client.GatewayInfo(cmd.Context()) + if err != nil { + return fmt.Errorf("reading gateway info: %w", err) } if format != formatTable { type gwOut struct { Name string `json:"name" yaml:"name"` Endpoint string `json:"endpoint" yaml:"endpoint"` - Active bool `json:"active" yaml:"active"` + Status string `json:"status" yaml:"status"` + Version string `json:"version" yaml:"version"` } - out := make([]gwOut, len(gateways)) - for i, g := range gateways { - out[i] = gwOut{Name: g.Name, Endpoint: g.Endpoint, Active: g.Active} - } - return printStructured(format, out) + return printStructured(format, gwOut{ + Name: info.Name, + Endpoint: info.Endpoint, + Status: info.Status, + Version: info.Version, + }) } - rows := make([][]string, len(gateways)) - for i, g := range gateways { - active := "" - if g.Active { - active = "*" - } - rows[i] = []string{g.Name, g.Endpoint, active} - } - printTable([]string{"Name", "Endpoint", "Active"}, rows) + printTable( + []string{"Name", "Endpoint", "Status", "Version"}, + [][]string{{info.Name, info.Endpoint, info.Status, info.Version}}, + ) return nil }, } cmd.Flags().StringVarP(&output, "output", "o", "", "Output format: table, json, or yaml") + gatewayName, workspace = registerTargetFlags(cmd) return cmd } diff --git a/cmd/get_test.go b/cmd/get_test.go new file mode 100644 index 0000000..222d099 --- /dev/null +++ b/cmd/get_test.go @@ -0,0 +1,134 @@ +package cmd + +import ( + "testing" + + fake "github.com/NVIDIA/OpenShell/sdk/go/openshell/v1/fake" + "github.com/NVIDIA/OpenShell/sdk/go/openshell/v1/types" + + "github.com/stackrox/harness-openshell/internal/testutil" +) + +func TestGetAgents(t *testing.T) { + client, fc := testutil.NewFakeClient("default") + fc.AddSandbox("default", &types.Sandbox{Name: "agent-a", Status: types.SandboxStatus{Phase: types.SandboxReady}}) + fc.AddSandbox("default", &types.Sandbox{Name: "agent-b", Status: types.SandboxStatus{Phase: types.SandboxProvisioning}}) + + cmd := NewGetCmd(testutil.FakeFactory(client)) + cmd.SetArgs([]string{"agents"}) + out, err := captureStdout(t, cmd.Execute) + if err != nil { + t.Fatalf("get agents: %v", err) + } + for _, want := range []string{"NAME", "PHASE", "agent-a", "Ready", "agent-b", "Provisioning"} { + if !contains(out, want) { + t.Errorf("get agents table missing %q:\n%s", want, out) + } + } +} + +func TestGetAgentsEmpty(t *testing.T) { + client, _ := testutil.NewFakeClient("default") + cmd := NewGetCmd(testutil.FakeFactory(client)) + cmd.SetArgs([]string{"agents"}) + out, err := captureStdout(t, cmd.Execute) + if err != nil { + t.Fatalf("get agents: %v", err) + } + if !contains(out, "No sandboxes running.") { + t.Errorf("empty get agents should print the friendly message:\n%s", out) + } +} + +func TestGetAgentsJSON(t *testing.T) { + client, fc := testutil.NewFakeClient("default") + fc.AddSandbox("default", &types.Sandbox{Name: "agent-a", Status: types.SandboxStatus{Phase: types.SandboxReady}}) + + cmd := NewGetCmd(testutil.FakeFactory(client)) + cmd.SetArgs([]string{"agents", "-o", "json"}) + out, err := captureStdout(t, cmd.Execute) + if err != nil { + t.Fatalf("get agents -o json: %v", err) + } + if !contains(out, `"name": "agent-a"`) || !contains(out, `"phase": "Ready"`) { + t.Errorf("json output missing fields:\n%s", out) + } + if !contains(out, "[") { + t.Errorf("json output should be an array:\n%s", out) + } +} + +func TestGetProviders(t *testing.T) { + client, fc := testutil.NewFakeClient("default") + fc.AddProvider("default", &types.Provider{Name: "github", Type: "github"}) + fc.AddProvider("default", &types.Provider{Name: "vertex", Type: "google-vertex-ai"}) + + cmd := NewGetCmd(testutil.FakeFactory(client)) + cmd.SetArgs([]string{"providers"}) + out, err := captureStdout(t, cmd.Execute) + if err != nil { + t.Fatalf("get providers: %v", err) + } + for _, want := range []string{"NAME", "github", "vertex"} { + if !contains(out, want) { + t.Errorf("get providers table missing %q:\n%s", want, out) + } + } +} + +func TestGetProvidersEmpty(t *testing.T) { + client, _ := testutil.NewFakeClient("default") + cmd := NewGetCmd(testutil.FakeFactory(client)) + cmd.SetArgs([]string{"providers"}) + out, err := captureStdout(t, cmd.Execute) + if err != nil { + t.Fatalf("get providers: %v", err) + } + if !contains(out, "No providers registered.") { + t.Errorf("empty get providers should print the friendly message:\n%s", out) + } +} + +// TestGetGateways pins the decision-4 reframe: a single active-gateway record +// with Status+Version (from the health RPC) and NO Active column. +func TestGetGateways(t *testing.T) { + client, _ := testutil.NewFakeClient("default", fake.WithGatewayInfo(&types.GatewayInfo{ + Status: types.ServiceStatusHealthy, + Version: "0.0.110", + })) + cmd := NewGetCmd(testutil.FakeFactory(client)) + cmd.SetArgs([]string{"gateways"}) + out, err := captureStdout(t, cmd.Execute) + if err != nil { + t.Fatalf("get gateways: %v", err) + } + for _, want := range []string{"NAME", "ENDPOINT", "STATUS", "VERSION", "Healthy", "0.0.110"} { + if !contains(out, want) { + t.Errorf("get gateways missing %q:\n%s", want, out) + } + } + if contains(out, "ACTIVE") { + t.Errorf("get gateways should no longer show an Active column:\n%s", out) + } +} + +// TestGetGatewaysJSON checks the structured form is a single object, not an array. +func TestGetGatewaysJSON(t *testing.T) { + client, _ := testutil.NewFakeClient("default", fake.WithGatewayInfo(&types.GatewayInfo{ + Status: types.ServiceStatusDegraded, + Version: "0.0.110", + })) + cmd := NewGetCmd(testutil.FakeFactory(client)) + cmd.SetArgs([]string{"gateways", "-o", "json"}) + out, err := captureStdout(t, cmd.Execute) + if err != nil { + t.Fatalf("get gateways -o json: %v", err) + } + if !contains(out, `"status": "Degraded"`) || !contains(out, `"version": "0.0.110"`) { + t.Errorf("json output missing fields:\n%s", out) + } + // A single object starts with "{", not a "[" array. + if contains(out, "[") { + t.Errorf("get gateways json should be a single object, not an array:\n%s", out) + } +} diff --git a/cmd/target.go b/cmd/target.go index 190d27c..524c6f1 100644 --- a/cmd/target.go +++ b/cmd/target.go @@ -1,7 +1,9 @@ package cmd import ( + "context" "fmt" + "os" "github.com/spf13/cobra" "github.com/stackrox/harness-openshell/internal/gateway" @@ -26,6 +28,17 @@ func registerTargetFlags(cmd *cobra.Command) (gateway, workspace *string) { return gateway, workspace } +// openClient resolves the standard --gateway/--workspace target (flag > env > +// empty, via openshell.ResolveTarget) and constructs an SDK client through the +// Factory seam. It is the shared construction site for get and describe, so +// target resolution stays identical across them. delete resolves the target +// itself (it needs the resolved gateway name for its banner) but uses the same +// ResolveTarget rule. Callers own the returned client's Close. +func openClient(ctx context.Context, newClient openshell.Factory, gatewayName, workspace *string) (openshell.Client, error) { + target := openshell.ResolveTarget(*gatewayName, *workspace, "", "", os.Getenv) + return newClient(ctx, target) +} + // resolveApplyTarget builds the SDK openshell.Target for the apply command from // the CLI's currently-active gateway registration. // diff --git a/internal/openshell/client.go b/internal/openshell/client.go index 327de3d..381440e 100644 --- a/internal/openshell/client.go +++ b/internal/openshell/client.go @@ -18,6 +18,19 @@ type Client interface { Health(ctx context.Context) (Health, error) // Providers lists the providers registered in the bound workspace. Providers(ctx context.Context) ([]Provider, error) + // Sandboxes lists the sandboxes in the bound workspace (read UX: get agents). + Sandboxes(ctx context.Context) ([]Sandbox, error) + // GetSandbox reads the named sandbox in the bound workspace. Returns + // ErrNotFound when no such sandbox exists (read UX: describe). + GetSandbox(ctx context.Context, name string) (Sandbox, error) + // DeleteSandbox removes the named sandbox in the bound workspace. + DeleteSandbox(ctx context.Context, name string) error + // DeleteProvider removes the named provider in the bound workspace. + DeleteProvider(ctx context.Context, name string) error + // GatewayInfo introspects the active gateway (name, endpoint, status, + // version). The SDK offers no gateway list; this reports the single gateway + // the client is bound to. + GatewayInfo(ctx context.Context) (GatewayInfo, error) // GetProvider reads the named provider in the bound workspace. Returns // ErrNotFound when no such provider exists (requires the "provider:read" // role). diff --git a/internal/openshell/sdkclient/client.go b/internal/openshell/sdkclient/client.go index 16f3651..decbf3f 100644 --- a/internal/openshell/sdkclient/client.go +++ b/internal/openshell/sdkclient/client.go @@ -34,9 +34,16 @@ var ( // client wraps the SDK client interface, binding it to one workspace. It holds // the interface (not *v1.Client) so tests can inject the SDK fake. +// +// gatewayName and gatewayEndpoint are connection facts the SDK does not report +// over the wire (GatewayInfo carries neither): New captures them from the +// CLI-managed gateway config so GatewayInfo can merge them with the health RPC's +// status/version. The NewFromClient injection path leaves both empty. type client struct { - raw v1.ClientInterface - workspace string + raw v1.ClientInterface + workspace string + gatewayName string + gatewayEndpoint string } // New constructs an openshell.Client for the given target: it loads the @@ -60,15 +67,28 @@ func New(ctx context.Context, t openshell.Target) (openshell.Client, error) { return nil, err } - // NewFromClient is the single owner of the "" -> defaultWorkspace default; - // pass t.Workspace straight through. - return NewFromClient(raw, t.Workspace), nil + // newClient is the single owner of the "" -> defaultWorkspace default; pass + // t.Workspace straight through. Capture the connection facts the SDK never + // reports (gateway name and endpoint) so GatewayInfo can merge them. + c := newClient(raw, t.Workspace) + c.gatewayName = t.Gateway + c.gatewayEndpoint = cfg.Endpoint + return c, nil } // NewFromClient wraps an existing SDK client (or the SDK fake) bound to a // workspace. It is the injection seam used by white-box tests and by -// internal/testutil. Empty workspace defaults to defaultWorkspace. +// internal/testutil. Empty workspace defaults to defaultWorkspace. It leaves the +// gateway name and endpoint empty — those are connection facts only New can +// capture from the CLI-managed config. func NewFromClient(raw v1.ClientInterface, workspace string) openshell.Client { + return newClient(raw, workspace) +} + +// newClient builds the concrete client with the workspace default applied. It +// returns the concrete type so New can set the connection facts before returning +// the interface. +func newClient(raw v1.ClientInterface, workspace string) *client { if workspace == "" { workspace = defaultWorkspace } diff --git a/internal/openshell/sdkclient/gateway.go b/internal/openshell/sdkclient/gateway.go new file mode 100644 index 0000000..f96cd4e --- /dev/null +++ b/internal/openshell/sdkclient/gateway.go @@ -0,0 +1,34 @@ +package sdkclient + +import ( + "context" + + v1 "github.com/NVIDIA/OpenShell/sdk/go/openshell/v1" + + "github.com/stackrox/harness-openshell/internal/openshell" +) + +// fromSDKGatewayInfo maps the SDK gateway health view to the harness +// GatewayInfo, merging the two sources of truth: name and endpoint are +// connection facts captured at construction (the SDK reports neither over the +// wire), while status and version come from the health RPC. Status is carried +// through as a string (Healthy|Degraded|Unhealthy|Unknown). +func fromSDKGatewayInfo(info *v1.GatewayInfo, name, endpoint string) openshell.GatewayInfo { + return openshell.GatewayInfo{ + Name: name, + Endpoint: endpoint, + Status: string(info.Status), + Version: info.Version, + } +} + +// GatewayInfo introspects the active gateway. It reports the single gateway the +// client is bound to (the SDK has no gateway list), merging the health RPC's +// status/version with the name/endpoint captured at construction. +func (c *client) GatewayInfo(ctx context.Context) (openshell.GatewayInfo, error) { + info, err := c.raw.Health().GetGatewayInfo(ctx) + if err != nil { + return openshell.GatewayInfo{}, translate(err) + } + return fromSDKGatewayInfo(info, c.gatewayName, c.gatewayEndpoint), nil +} diff --git a/internal/openshell/sdkclient/gateway_test.go b/internal/openshell/sdkclient/gateway_test.go new file mode 100644 index 0000000..9bf5bd1 --- /dev/null +++ b/internal/openshell/sdkclient/gateway_test.go @@ -0,0 +1,57 @@ +package sdkclient + +import ( + "context" + "testing" + + fake "github.com/NVIDIA/OpenShell/sdk/go/openshell/v1/fake" + "github.com/NVIDIA/OpenShell/sdk/go/openshell/v1/types" + + "github.com/stackrox/harness-openshell/internal/openshell" +) + +// TestGatewayInfoMergesConnectionFactsAndHealth pins invariant 37: Status and +// Version come from the SDK health RPC, while Name and Endpoint come from the +// connection facts captured at construction. A client built here with those +// fields set proves both sources merge into one record. +func TestGatewayInfoMergesConnectionFactsAndHealth(t *testing.T) { + ctx := context.Background() + fc := fake.NewClient(fake.WithGatewayInfo(&types.GatewayInfo{ + Status: types.ServiceStatusHealthy, + Version: "0.0.110", + })) + c := &client{raw: fc, workspace: "default", gatewayName: "prod", gatewayEndpoint: "gw.example:443"} + + got, err := c.GatewayInfo(ctx) + if err != nil { + t.Fatalf("GatewayInfo: %v", err) + } + want := openshell.GatewayInfo{Name: "prod", Endpoint: "gw.example:443", Status: "Healthy", Version: "0.0.110"} + if got != want { + t.Errorf("GatewayInfo: got %+v, want %+v", got, want) + } +} + +// TestGatewayInfoInjectionPathLeavesNameEndpointEmpty pins the other half of +// invariant 37: NewFromClient (the injection/test seam) loads no config, so Name +// and Endpoint are empty while Status and Version still come from the RPC. This +// is why command tests backed by the fake assert Status/Version, not Endpoint. +func TestGatewayInfoInjectionPathLeavesNameEndpointEmpty(t *testing.T) { + ctx := context.Background() + fc := fake.NewClient(fake.WithGatewayInfo(&types.GatewayInfo{ + Status: types.ServiceStatusDegraded, + Version: "0.0.110", + })) + c := NewFromClient(fc, "default") + + got, err := c.GatewayInfo(ctx) + if err != nil { + t.Fatalf("GatewayInfo: %v", err) + } + if got.Name != "" || got.Endpoint != "" { + t.Errorf("injection path should leave Name/Endpoint empty, got %+v", got) + } + if got.Status != "Degraded" || got.Version != "0.0.110" { + t.Errorf("Status/Version not mapped from RPC: %+v", got) + } +} diff --git a/internal/openshell/sdkclient/provider.go b/internal/openshell/sdkclient/provider.go index 3b82741..6631042 100644 --- a/internal/openshell/sdkclient/provider.go +++ b/internal/openshell/sdkclient/provider.go @@ -45,6 +45,11 @@ func (c *client) GetProvider(ctx context.Context, name string) (openshell.Provid return fromSDKProvider(p), nil } +// DeleteProvider removes the named provider in the bound workspace. +func (c *client) DeleteProvider(ctx context.Context, name string) error { + return translate(c.raw.Providers().Delete(ctx, c.workspace, name)) +} + // UpdateProvider writes the desired non-secret Config/Labels of an existing // provider while preserving everything else the gateway holds — this is the // single credential-preserving-update site (spec §8.5). diff --git a/internal/openshell/sdkclient/sandbox.go b/internal/openshell/sdkclient/sandbox.go new file mode 100644 index 0000000..1e57e99 --- /dev/null +++ b/internal/openshell/sdkclient/sandbox.go @@ -0,0 +1,50 @@ +package sdkclient + +import ( + "context" + + v1 "github.com/NVIDIA/OpenShell/sdk/go/openshell/v1" + + "github.com/stackrox/harness-openshell/internal/openshell" +) + +// fromSDKSandbox maps the SDK sandbox view to the harness Sandbox. It reads the +// top-level Name (the resource name, always populated) rather than +// Status.SandboxName (a status echo that can be empty before Ready), and carries +// the lifecycle phase through as a string. Everything else the SDK holds (Spec, +// Labels, Conditions, ResourceVersion, ...) is dropped at this boundary +// (least-exposure firewall — see openshell.Sandbox). +func fromSDKSandbox(s *v1.Sandbox) openshell.Sandbox { + return openshell.Sandbox{ + Name: s.Name, + Phase: string(s.Status.Phase), + } +} + +// Sandboxes lists the sandboxes in the bound workspace. +func (c *client) Sandboxes(ctx context.Context) ([]openshell.Sandbox, error) { + raw, err := c.raw.Sandboxes().List(ctx, c.workspace) + if err != nil { + return nil, translate(err) + } + out := make([]openshell.Sandbox, 0, len(raw)) + for _, s := range raw { + out = append(out, fromSDKSandbox(s)) + } + return out, nil +} + +// GetSandbox reads the named sandbox in the bound workspace, mapping a missing +// sandbox to openshell.ErrNotFound (via translate). +func (c *client) GetSandbox(ctx context.Context, name string) (openshell.Sandbox, error) { + s, err := c.raw.Sandboxes().Get(ctx, c.workspace, name) + if err != nil { + return openshell.Sandbox{}, translate(err) + } + return fromSDKSandbox(s), nil +} + +// DeleteSandbox removes the named sandbox in the bound workspace. +func (c *client) DeleteSandbox(ctx context.Context, name string) error { + return translate(c.raw.Sandboxes().Delete(ctx, c.workspace, name)) +} diff --git a/internal/openshell/sdkclient/sandbox_test.go b/internal/openshell/sdkclient/sandbox_test.go new file mode 100644 index 0000000..2703104 --- /dev/null +++ b/internal/openshell/sdkclient/sandbox_test.go @@ -0,0 +1,100 @@ +package sdkclient + +import ( + "context" + "errors" + "testing" + + fake "github.com/NVIDIA/OpenShell/sdk/go/openshell/v1/fake" + "github.com/NVIDIA/OpenShell/sdk/go/openshell/v1/types" + + "github.com/stackrox/harness-openshell/internal/openshell" +) + +// TestFromSDKSandboxMapsNameAndPhase pins the read-widening: the harness Sandbox +// carries the top-level Name and the lifecycle phase as a string, and nothing +// else the SDK holds. +func TestFromSDKSandboxMapsNameAndPhase(t *testing.T) { + got := fromSDKSandbox(&types.Sandbox{ + Name: "agent-1", + Status: types.SandboxStatus{SandboxName: "echo-should-be-ignored", Phase: types.SandboxReady}, + }) + if got.Name != "agent-1" { + t.Errorf("Name: got %q, want agent-1 (top-level Name, not Status.SandboxName)", got.Name) + } + if got.Phase != "Ready" { + t.Errorf("Phase: got %q, want Ready", got.Phase) + } +} + +// TestSandboxes lists mapped sandboxes; an empty store yields an empty slice. +func TestSandboxes(t *testing.T) { + ctx := context.Background() + fc := fake.NewClient() + fc.AddSandbox("default", &types.Sandbox{Name: "a", Status: types.SandboxStatus{Phase: types.SandboxReady}}) + fc.AddSandbox("default", &types.Sandbox{Name: "b", Status: types.SandboxStatus{Phase: types.SandboxProvisioning}}) + c := NewFromClient(fc, "default") + + got, err := c.Sandboxes(ctx) + if err != nil { + t.Fatalf("Sandboxes: %v", err) + } + if len(got) != 2 { + t.Fatalf("want 2 sandboxes, got %d: %+v", len(got), got) + } + byName := map[string]string{} + for _, s := range got { + byName[s.Name] = s.Phase + } + if byName["a"] != "Ready" || byName["b"] != "Provisioning" { + t.Errorf("unexpected phases: %v", byName) + } + + empty, err := NewFromClient(fake.NewClient(), "default").Sandboxes(ctx) + if err != nil { + t.Fatalf("Sandboxes(empty): %v", err) + } + if len(empty) != 0 { + t.Errorf("want empty slice, got %+v", empty) + } +} + +// TestGetSandbox covers the by-name read: fields map through and a missing +// sandbox surfaces as openshell.ErrNotFound. +func TestGetSandbox(t *testing.T) { + ctx := context.Background() + fc := fake.NewClient() + fc.AddSandbox("default", &types.Sandbox{Name: "agent-1", Status: types.SandboxStatus{Phase: types.SandboxReady}}) + c := NewFromClient(fc, "default") + + got, err := c.GetSandbox(ctx, "agent-1") + if err != nil { + t.Fatalf("GetSandbox: %v", err) + } + if got.Name != "agent-1" || got.Phase != "Ready" { + t.Errorf("unexpected sandbox: %+v", got) + } + + if _, err := c.GetSandbox(ctx, "absent"); !errors.Is(err, openshell.ErrNotFound) { + t.Errorf("GetSandbox(absent): want ErrNotFound, got %v", err) + } +} + +// TestDeleteSandbox removes the named sandbox; a follow-up list confirms it. +func TestDeleteSandbox(t *testing.T) { + ctx := context.Background() + fc := fake.NewClient() + fc.AddSandbox("default", &types.Sandbox{Name: "agent-1", Status: types.SandboxStatus{Phase: types.SandboxReady}}) + c := NewFromClient(fc, "default") + + if err := c.DeleteSandbox(ctx, "agent-1"); err != nil { + t.Fatalf("DeleteSandbox: %v", err) + } + got, err := c.Sandboxes(ctx) + if err != nil { + t.Fatalf("Sandboxes: %v", err) + } + if len(got) != 0 { + t.Errorf("sandbox not deleted: %+v", got) + } +} diff --git a/internal/openshell/types.go b/internal/openshell/types.go index c9c4d3f..a39a2af 100644 --- a/internal/openshell/types.go +++ b/internal/openshell/types.go @@ -34,6 +34,31 @@ type Provider struct { Labels map[string]string // ownership + metadata (see plan.OwnerLabelKey) } +// Sandbox is the harness view of a sandbox for the read UX (get/describe). +// +// Deliberately narrow (least-exposure firewall): only the fields the read +// commands render. Phase is the SDK SandboxPhase carried through as a string +// (Provisioning|Ready|Error|Deleting|Unknown|Stopping). Widen only when a +// consumer genuinely needs more, changing this and fromSDKSandbox together. +type Sandbox struct { + Name string + Phase string +} + +// GatewayInfo is the harness view of the active gateway. +// +// Name and Endpoint are connection facts captured at construction (New, from the +// CLI-managed gateway config); the injection path (NewFromClient) leaves them +// empty, so Endpoint in particular is production-only and untested via the SDK +// fake. Status and Version come from the gateway's health RPC. Status is the SDK +// ServiceStatus as a string (Healthy|Degraded|Unhealthy|Unknown). +type GatewayInfo struct { + Name string + Endpoint string + Status string + Version string +} + // InferenceRoute is the harness view of an inference route read from a gateway. // // Deliberately minimal (least-exposure firewall): only the fields the harness diff --git a/internal/plan/state_test.go b/internal/plan/state_test.go index 553a8e3..934afa8 100644 --- a/internal/plan/state_test.go +++ b/internal/plan/state_test.go @@ -314,6 +314,26 @@ func (r *recordingClient) GetProvider(ctx context.Context, name string) (openshe return r.wrapped.GetProvider(ctx, name) } +func (r *recordingClient) Sandboxes(ctx context.Context) ([]openshell.Sandbox, error) { + return r.wrapped.Sandboxes(ctx) +} + +func (r *recordingClient) GetSandbox(ctx context.Context, name string) (openshell.Sandbox, error) { + return r.wrapped.GetSandbox(ctx, name) +} + +func (r *recordingClient) DeleteSandbox(ctx context.Context, name string) error { + return r.wrapped.DeleteSandbox(ctx, name) +} + +func (r *recordingClient) DeleteProvider(ctx context.Context, name string) error { + return r.wrapped.DeleteProvider(ctx, name) +} + +func (r *recordingClient) GatewayInfo(ctx context.Context) (openshell.GatewayInfo, error) { + return r.wrapped.GatewayInfo(ctx) +} + func (r *recordingClient) UpdateProvider(ctx context.Context, p openshell.Provider) (openshell.Provider, error) { return r.wrapped.UpdateProvider(ctx, p) } @@ -352,6 +372,26 @@ func (e *errorClient) GetProvider(ctx context.Context, name string) (openshell.P return openshell.Provider{}, e.err } +func (e *errorClient) Sandboxes(ctx context.Context) ([]openshell.Sandbox, error) { + return nil, e.err +} + +func (e *errorClient) GetSandbox(ctx context.Context, name string) (openshell.Sandbox, error) { + return openshell.Sandbox{}, e.err +} + +func (e *errorClient) DeleteSandbox(ctx context.Context, name string) error { + return e.err +} + +func (e *errorClient) DeleteProvider(ctx context.Context, name string) error { + return e.err +} + +func (e *errorClient) GatewayInfo(ctx context.Context) (openshell.GatewayInfo, error) { + return openshell.GatewayInfo{}, e.err +} + func (e *errorClient) UpdateProvider(ctx context.Context, p openshell.Provider) (openshell.Provider, error) { return openshell.Provider{}, e.err } diff --git a/main.go b/main.go index ed48fc6..9f868a7 100644 --- a/main.go +++ b/main.go @@ -63,9 +63,9 @@ func main() { root.AddCommand( cmd.NewApplyCmd(harnessDir, cli, sdkclient.New), - cmd.NewGetCmd(harnessDir, cli), - cmd.NewDescribeCmd(harnessDir, cli), - cmd.NewDeleteCmd(harnessDir, cli), + cmd.NewGetCmd(sdkclient.New), + cmd.NewDescribeCmd(sdkclient.New), + cmd.NewDeleteCmd(harnessDir, cli, sdkclient.New), cmd.NewDeployCmd(harnessDir, cli), cmd.NewDoctorCmd(harnessDir, cli, sdkclient.New), cmd.NewInitCmd(harnessDir),