refactor: inject k8s.Runner for testable orchestration (#25)

cmd/creds.go

@@ -12,9 +12,8 @@ import (
 	"github.com/robbycochran/harness-openshell/internal/status"
 )
 
-func ensureCreds(namespace string, force bool) error {
+func ensureCreds(kc k8s.Runner, namespace string, force bool) error {
 	ctx := context.Background()
-	kc := k8s.New("", namespace)
 
 	if !kc.NamespaceExists(ctx, namespace) {
 		return fmt.Errorf("namespace '%s' not found — run: harness deploy --remote", namespace)
@@ -63,7 +62,7 @@ func ensureCreds(namespace string, force bool) error {
 	return nil
 }
 
-func createGWSSecret(ctx context.Context, kc *k8s.Client) error {
+func createGWSSecret(ctx context.Context, kc k8s.Runner) error {
 	gwsPath, err := exec.LookPath("gws")
 	if err != nil {
 		status.Info("GWS: not installed (skipping)")

cmd/creds_test.go

@@ -0,0 +1,77 @@
+package cmd
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/robbycochran/harness-openshell/internal/k8s"
+)
+
+func TestEnsureCreds_NamespaceNotFound(t *testing.T) {
+	nsRunner := k8s.NewMockRunner()
+	nsRunner.Errors["namespace-exists"] = fmt.Errorf("not found")
+
+	err := ensureCreds(nsRunner, "openshell", false)
+	if err == nil {
+		t.Fatal("expected error for missing namespace")
+	}
+}
+
+func TestEnsureCreds_SecretsExist(t *testing.T) {
+	nsRunner := k8s.NewMockRunner()
+	// namespace exists (no error), secrets exist (no error)
+
+	err := ensureCreds(nsRunner, "openshell", false)
+	if err != nil {
+		t.Fatalf("ensureCreds: %v", err)
+	}
+	// Should NOT attempt to create secrets
+	if nsRunner.HasCall("create secret") {
+		t.Error("should not create secrets when they already exist")
+	}
+}
+
+func TestEnsureCreds_ForceDeletesExisting(t *testing.T) {
+	nsRunner := k8s.NewMockRunner()
+
+	err := ensureCreds(nsRunner, "openshell", true)
+	if err != nil {
+		t.Fatalf("ensureCreds: %v", err)
+	}
+	// Should attempt to delete both secrets
+	if nsRunner.CallCount("delete secret") != 2 {
+		t.Errorf("expected 2 secret deletes, got %d: %v",
+			nsRunner.CallCount("delete secret"), nsRunner.Calls)
+	}
+}
+
+func TestEnsureCreds_AtlassianCreated(t *testing.T) {
+	nsRunner := k8s.NewMockRunner()
+	nsRunner.Errors["secret-exists openshell-atlassian"] = fmt.Errorf("not found")
+
+	t.Setenv("JIRA_URL", "https://example.atlassian.net")
+	t.Setenv("JIRA_USERNAME", "user@example.com")
+
+	err := ensureCreds(nsRunner, "openshell", false)
+	if err != nil {
+		t.Fatalf("ensureCreds: %v", err)
+	}
+	if !nsRunner.HasCall("create secret generic openshell-atlassian") {
+		t.Errorf("expected atlassian secret creation, calls: %v", nsRunner.Calls)
+	}
+}
+
+func TestEnsureCreds_AtlassianSkipped(t *testing.T) {
+	nsRunner := k8s.NewMockRunner()
+	nsRunner.Errors["secret-exists openshell-atlassian"] = fmt.Errorf("not found")
+	t.Setenv("JIRA_URL", "")
+	t.Setenv("JIRA_USERNAME", "")
+
+	err := ensureCreds(nsRunner, "openshell", false)
+	if err != nil {
+		t.Fatalf("ensureCreds: %v", err)
+	}
+	if nsRunner.HasCall("create secret generic openshell-atlassian") {
+		t.Error("should not create atlassian secret without JIRA_URL")
+	}
+}

cmd/deploy.go

@@ -29,7 +29,9 @@ func NewDeployCmd(harnessDir, cli string) *cobra.Command {
 		RunE: func(cmd *cobra.Command, args []string) error {
 			if remote {
 				gw := gateway.NewCLI(cli)
-				return deployRemote(harnessDir, gw, kubeconfig)
+				kc := k8s.New(kubeconfig, k8s.DefaultNamespace())
+				clusterRunner := k8s.New(kubeconfig, "")
+				return deployRemote(harnessDir, gw, kc, clusterRunner)
 			}
 			if local {
 				gw := gateway.NewCLI(cli)
@@ -46,7 +48,7 @@ func NewDeployCmd(harnessDir, cli string) *cobra.Command {
 	return cmd
 }
 
-func deployRemote(harnessDir string, gw gateway.Gateway, kubeconfig string) error {
+func deployRemote(harnessDir string, gw gateway.Gateway, kc, clusterRunner k8s.Runner) error {
 	ctx := context.Background()
 	namespace := k8s.DefaultNamespace()
 
@@ -63,18 +65,15 @@ func deployRemote(harnessDir string, gw gateway.Gateway, kubeconfig string) erro
 	chartOCI := "oci://ghcr.io/nvidia/openshell/helm-chart"
 
 	fmt.Printf("OpenShell chart: %s\n", chartVersion)
-	if kubeconfig != "" {
-		fmt.Printf("KUBECONFIG: %s\n", kubeconfig)
+	if kbcfg := os.Getenv("KUBECONFIG"); kbcfg != "" {
+		fmt.Printf("KUBECONFIG: %s\n", kbcfg)
 	}
 	fmt.Println()
 
-	kc := k8s.New(kubeconfig, namespace)
-	kcNoNS := k8s.New(kubeconfig, "")
-
 	// Step 1: Namespace (idempotent — ignore AlreadyExists)
 	status.Step(1, "Creating namespace")
-	kcNoNS.RunKubectl(ctx, "create", "ns", namespace)
-	if _, err := kcNoNS.RunKubectl(ctx, "label", "ns", namespace,
+	clusterRunner.RunKubectl(ctx, "create", "ns", namespace)
+	if _, err := clusterRunner.RunKubectl(ctx, "label", "ns", namespace,
 		"pod-security.kubernetes.io/enforce=privileged",
 		"pod-security.kubernetes.io/warn=privileged",
 		"--overwrite"); err != nil {
@@ -83,7 +82,7 @@ func deployRemote(harnessDir string, gw gateway.Gateway, kubeconfig string) erro
 
 	// Step 2: Sandbox CRD
 	status.Step(2, "Installing Sandbox CRD")
-	if err := kcNoNS.RunKubectlPassthrough(ctx, "apply", "-f",
+	if err := clusterRunner.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)
 	}
@@ -94,7 +93,7 @@ func deployRemote(harnessDir string, gw gateway.Gateway, kubeconfig string) erro
 		kc.RunOC(ctx, "adm", "policy", "add-scc-to-user", "privileged", "-z", sa, "-n", namespace)
 	}
 	kc.RunOC(ctx, "adm", "policy", "add-scc-to-user", "anyuid", "-z", "openshell", "-n", namespace)
-	kcNoNS.RunKubectl(ctx, "create", "clusterrolebinding", "agent-sandbox-admin",
+	clusterRunner.RunKubectl(ctx, "create", "clusterrolebinding", "agent-sandbox-admin",
 		"--clusterrole=cluster-admin",
 		"--serviceaccount=agent-sandbox-system:agent-sandbox-controller")
 
@@ -141,7 +140,7 @@ func deployRemote(harnessDir string, gw gateway.Gateway, kubeconfig string) erro
 		sandboxImage = "quay.io/rcochran/openshell:sandbox"
 	}
 
-	appsDomain, err := kcNoNS.GetJSONPath(ctx, "ingresses.config.openshift.io/cluster", "{.spec.domain}")
+	appsDomain, err := clusterRunner.GetJSONPath(ctx, "ingresses.config.openshift.io/cluster", "{.spec.domain}")
 	if err != nil || appsDomain == "" {
 		return fmt.Errorf("could not determine OpenShift apps domain — is this an OpenShift cluster? (kubectl get ingresses.config.openshift.io cluster)")
 	}

cmd/deploy_test.go

@@ -0,0 +1,151 @@
+package cmd
+
+import (
+	"fmt"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/robbycochran/harness-openshell/internal/gateway"
+	"github.com/robbycochran/harness-openshell/internal/k8s"
+)
+
+func setupDeployHarnessDir(t *testing.T) string {
+	t.Helper()
+	dir := t.TempDir()
+	os.WriteFile(filepath.Join(dir, "values-ocp.yaml"), []byte("image:\n  pullPolicy: Always\n"), 0o644)
+	os.MkdirAll(filepath.Join(dir, "profiles"), 0o755)
+	return dir
+}
+
+func TestDeployRemote_Success(t *testing.T) {
+	dir := setupDeployHarnessDir(t)
+	t.Setenv("OPENSHELL_CHART_VERSION", "0.0.55")
+	t.Setenv("OPENSHELL_NAMESPACE", "openshell")
+
+	nsRunner := k8s.NewMockRunner()
+	clusterRunner := k8s.NewMockRunner()
+	clusterRunner.Responses["get-jsonpath"] = "apps.example.com"
+	nsRunner.Responses["get-secret-field"] = "dGVzdA==" // base64 "test"
+	nsRunner.Errors["get route"] = fmt.Errorf("not found") // trigger Route creation
+
+	gw := &mockGW{}
+
+	err := deployRemote(dir, gw, nsRunner, clusterRunner)
+	if err != nil {
+		t.Fatalf("deployRemote: %v", err)
+	}
+
+	// Verify namespace created
+	if !clusterRunner.HasCall("create ns openshell") {
+		t.Errorf("missing create ns, calls: %v", clusterRunner.Calls)
+	}
+
+	// Verify namespace labeled
+	if !clusterRunner.HasCall("label ns openshell") {
+		t.Errorf("missing label ns, calls: %v", clusterRunner.Calls)
+	}
+
+	// Verify CRD installed
+	if !clusterRunner.HasCall("apply -f") {
+		t.Errorf("missing CRD apply, calls: %v", clusterRunner.Calls)
+	}
+
+	// Verify RBAC applied
+	if !nsRunner.HasCall("apply-yaml ServiceAccount") {
+		t.Errorf("missing RBAC ServiceAccount, calls: %v", nsRunner.Calls)
+	}
+	if !nsRunner.HasCall("apply-yaml Role") {
+		t.Errorf("missing RBAC Role, calls: %v", nsRunner.Calls)
+	}
+
+	// Verify Helm install
+	if !nsRunner.HasCall("helm upgrade --install") {
+		t.Errorf("missing helm install, calls: %v", nsRunner.Calls)
+	}
+
+	// Verify rollout status
+	if !nsRunner.HasCall("rollout status statefulset/openshell") {
+		t.Errorf("missing rollout status, calls: %v", nsRunner.Calls)
+	}
+
+	// Verify Route applied
+	if !nsRunner.HasCall("apply-yaml Route") {
+		t.Errorf("missing Route apply, calls: %v", nsRunner.Calls)
+	}
+}
+
+func TestDeployRemote_HelmFailure(t *testing.T) {
+	dir := setupDeployHarnessDir(t)
+	t.Setenv("OPENSHELL_CHART_VERSION", "0.0.55")
+	t.Setenv("OPENSHELL_NAMESPACE", "openshell")
+
+	nsRunner := k8s.NewMockRunner()
+	clusterRunner := k8s.NewMockRunner()
+	clusterRunner.Responses["get-jsonpath"] = "apps.example.com"
+	nsRunner.Errors["helm upgrade"] = fmt.Errorf("chart not found")
+
+	gw := &mockGW{}
+
+	err := deployRemote(dir, gw, nsRunner, clusterRunner)
+	if err == nil {
+		t.Fatal("expected error from helm failure")
+	}
+	if !nsRunner.HasCall("helm upgrade") {
+		t.Errorf("helm should have been called, calls: %v", nsRunner.Calls)
+	}
+	// Should NOT have attempted rollout after helm failure
+	if nsRunner.HasCall("rollout status") {
+		t.Error("should not attempt rollout after helm failure")
+	}
+}
+
+func TestDeployRemote_CRDFailure(t *testing.T) {
+	dir := setupDeployHarnessDir(t)
+	t.Setenv("OPENSHELL_CHART_VERSION", "0.0.55")
+	t.Setenv("OPENSHELL_NAMESPACE", "openshell")
+
+	nsRunner := k8s.NewMockRunner()
+	clusterRunner := k8s.NewMockRunner()
+	clusterRunner.Errors["apply -f"] = fmt.Errorf("network error")
+
+	gw := &mockGW{}
+
+	err := deployRemote(dir, gw, nsRunner, clusterRunner)
+	if err == nil {
+		t.Fatal("expected error from CRD install failure")
+	}
+	// Should NOT have attempted helm after CRD failure
+	if nsRunner.HasCall("helm") {
+		t.Error("should not attempt helm after CRD failure")
+	}
+}
+
+func TestDeployRemote_NoAppsDomain(t *testing.T) {
+	dir := setupDeployHarnessDir(t)
+	t.Setenv("OPENSHELL_CHART_VERSION", "0.0.55")
+	t.Setenv("OPENSHELL_NAMESPACE", "openshell")
+
+	nsRunner := k8s.NewMockRunner()
+	clusterRunner := k8s.NewMockRunner()
+	clusterRunner.Responses["get-jsonpath"] = "" // empty domain
+
+	gw := &mockGW{}
+
+	err := deployRemote(dir, gw, nsRunner, clusterRunner)
+	if err == nil {
+		t.Fatal("expected error for missing apps domain")
+	}
+}
+
+func TestDeployLocal_NoGateway(t *testing.T) {
+	gw := &mockGW{inferenceErr: fmt.Errorf("not reachable")}
+	gw.gatewayListResult = []gateway.GatewayInfo{
+		{Name: "local", Endpoint: "https://127.0.0.1:17670"},
+	}
+
+	err := deployLocal(gw)
+	if err == nil {
+		t.Fatal("expected error for unreachable gateway")
+	}
+}

cmd/new.go

@@ -78,11 +78,13 @@ type newLocalOpts struct {
 func newRemote(harnessDir string, gw gateway.Gateway, profileName, sandboxName string) error {
 	ctx := context.Background()
 	namespace := k8s.DefaultNamespace()
+	kc := k8s.New("", namespace)
+	clusterRunner := k8s.New("", "")
 
 	// 1. Ensure gateway
 	if err := gw.InferenceGet(); err != nil {
 		status.Section("Deploying gateway")
-		if err := deployRemote(harnessDir, gw, ""); err != nil {
+		if err := deployRemote(harnessDir, gw, kc, clusterRunner); err != nil {
 			return fmt.Errorf("deploy failed: %w", err)
 		}
 	}
@@ -97,7 +99,7 @@ func newRemote(harnessDir string, gw gateway.Gateway, profileName, sandboxName s
 	}
 
 	// 3. Ensure credentials
-	if err := ensureCreds(namespace, false); err != nil {
+	if err := ensureCreds(kc, namespace, false); err != nil {
 		return fmt.Errorf("credentials setup failed: %w", err)
 	}
 
@@ -110,7 +112,6 @@ func newRemote(harnessDir string, gw gateway.Gateway, profileName, sandboxName s
 		cfg.Name = sandboxName
 	}
 
-	kc := k8s.New("", namespace)
 	profilePath := filepath.Join(harnessDir, "profiles", profileName+".toml")
 
 	// 1. ConfigMap from profile

cmd/new_test.go

@@ -11,14 +11,16 @@ import (
 )
 
 type mockGW struct {
-	inferenceErr  error
-	providers     map[string]bool
-	providerList  []string
-	providerErr   error
-	createErr     error
-	createCalls   int
-	createOpts    []gateway.SandboxCreateOpts
-	deletedNames  []string
+	inferenceErr      error
+	providers         map[string]bool
+	providerList      []string
+	providerErr       error
+	createErr         error
+	createCalls       int
+	createOpts        []gateway.SandboxCreateOpts
+	deletedNames      []string
+	gatewayListResult []gateway.GatewayInfo
+	onGatewayRemove   func(string)
 }
 
 func (m *mockGW) InferenceGet() error { return m.inferenceErr }
@@ -56,10 +58,17 @@ func (m *mockGW) SandboxList() ([]string, error)
 func (m *mockGW) SandboxConnect(string) error                                   { return nil }
 func (m *mockGW) SandboxUpload(string, string, string) error                    { return nil }
 func (m *mockGW) SandboxExec(string, ...string) error                           { return nil }
-func (m *mockGW) GatewayAdd(string, string, bool) error                         { return nil }
-func (m *mockGW) GatewayRemove(string) error                                    { return nil }
-func (m *mockGW) GatewayList() ([]gateway.GatewayInfo, error)                   { return nil, nil }
-func (m *mockGW) GatewaySelect(string) error                                    { return nil }
+func (m *mockGW) GatewayAdd(string, string, bool) error { return nil }
+func (m *mockGW) GatewayRemove(name string) error {
+	if m.onGatewayRemove != nil {
+		m.onGatewayRemove(name)
+	}
+	return nil
+}
+func (m *mockGW) GatewayList() ([]gateway.GatewayInfo, error) {
+	return m.gatewayListResult, nil
+}
+func (m *mockGW) GatewaySelect(string) error { return nil }
 
 func setupTestProfile(t *testing.T) string {
 	t.Helper()