diff --git a/cmd/doctor.go b/cmd/doctor.go index 4560c3a..437847d 100644 --- a/cmd/doctor.go +++ b/cmd/doctor.go @@ -1,15 +1,17 @@ package cmd import ( + "context" + "errors" "fmt" "os" "os/exec" "path/filepath" "strings" - "github.com/stackrox/harness-openshell/internal/agent" - "github.com/stackrox/harness-openshell/internal/gateway" "github.com/spf13/cobra" + "github.com/stackrox/harness-openshell/internal/agent" + "github.com/stackrox/harness-openshell/internal/openshell" "gopkg.in/yaml.v3" ) @@ -22,11 +24,13 @@ type CheckResult struct { type CheckFunc func(cfg *agent.AgentConfig, cli, harnessDir string) []CheckResult -func NewDoctorCmd(harnessDir, cli string) *cobra.Command { +func NewDoctorCmd(harnessDir, cli string, newClient openshell.Factory) *cobra.Command { var ( - agentFile string - agentName string - output string + agentFile string + agentName string + output string + gatewayName string + workspace string ) cmd := &cobra.Command{ @@ -60,7 +64,16 @@ Phase 2 (online): if the gateway is reachable, checks provider registration.`, results = append(results, fn(h.Agent, cli, harnessDir)...) } - results = append(results, checkOnline(h.Agent, cli)...) + providerProfiles := make([]string, 0, len(h.Agent.Providers)) + for _, p := range h.Agent.Providers { + providerProfiles = append(providerProfiles, p.Profile) + } + // Flag resolution order (AGENTS.md): explicit flag > OPENSHELL_* env + // var > default. Empty flag defaults let the env fallback apply; an + // unset gateway (flag and env both empty) skips Phase 2. + gw := resolveOnlineFlag(gatewayName, "OPENSHELL_GATEWAY", "") + ws := resolveOnlineFlag(workspace, "OPENSHELL_WORKSPACE", defaultDoctorWorkspace) + results = append(results, runOnlineChecks(cmd.Context(), newClient, gw, ws, providerProfiles)...) if format != formatTable { return printStructured(format, results) @@ -80,6 +93,8 @@ Phase 2 (online): if the gateway is reachable, checks provider registration.`, cmd.Flags().StringVarP(&agentFile, "file", "f", "", "Path to harness YAML") cmd.Flags().StringVar(&agentName, "agent", "default", "Agent config name") cmd.Flags().StringVarP(&output, "output", "o", "", "Output format (table, json, yaml)") + cmd.Flags().StringVar(&gatewayName, "gateway", "", "OpenShell registration name for online checks (Phase 2). Defaults to $OPENSHELL_GATEWAY; empty skips online checks.") + cmd.Flags().StringVar(&workspace, "workspace", "", "Workspace for provider registration checks (default \"default\"; overridable via $OPENSHELL_WORKSPACE)") return cmd } @@ -179,8 +194,8 @@ func checkRemoteDeps() []CheckResult { } type providerProfile struct { - ID string `yaml:"id"` - DisplayName string `yaml:"display_name"` + ID string `yaml:"id"` + DisplayName string `yaml:"display_name"` Credentials []providerCredential `yaml:"credentials"` } @@ -328,47 +343,126 @@ func loadProfileFromDisk(name, harnessDir string) *providerProfile { return nil } -func checkOnline(cfg *agent.AgentConfig, cli string) []CheckResult { - gw := gateway.New(cli) - if gw.ActiveGateway() == "" { +// defaultDoctorWorkspace is the workspace used when neither --workspace nor +// $OPENSHELL_WORKSPACE is set. sdkclient also defaults "" -> "default"; this +// keeps the resolved value explicit for logging and table output. +const defaultDoctorWorkspace = "default" + +// resolveOnlineFlag applies the repo's standard flag-resolution order +// (AGENTS.md: explicit flag > OPENSHELL_* env var > default) for doctor's +// online flags. An empty flag value is treated as "unset" so the env var can +// take effect. +func resolveOnlineFlag(flagVal, envKey, def string) string { + if flagVal != "" { + return flagVal + } + if v := os.Getenv(envKey); v != "" { + return v + } + return def +} + +// runOnlineChecks performs Phase 2 (online) checks via the SDK. It is +// non-fatal by construction: a missing --gateway or any client-construction +// failure yields a single warn (Phase 2 skipped), never a fail, preserving +// doctor's long-standing "online failures don't break the build" contract. +func runOnlineChecks(ctx context.Context, newClient openshell.Factory, gatewayName, workspace string, providers []string) []CheckResult { + if gatewayName == "" { return []CheckResult{{ Group: "gateway", Name: "status", Status: "warn", - Message: "no active gateway (Phase 2 checks skipped)", + Message: "Phase 2 (online) checks skipped: no --gateway specified", }} } - _, err := gw.ProviderList() + client, err := newClient(ctx, openshell.Target{Gateway: gatewayName, Workspace: workspace}) if err != nil { + return []CheckResult{{ + Group: "gateway", + Name: "status", + Status: "warn", + Message: fmt.Sprintf("Phase 2 (online) checks skipped: %v", err), + }} + } + defer client.Close() + + return checkOnlineSDK(ctx, client, providers) +} + +// checkOnlineSDK reads gateway health and provider registration through the +// SDK client. Health maps: healthy -> pass; ErrUnauthenticated -> fail (the +// one actionable online failure); ErrUnavailable and any other error -> warn +// (non-fatal). Provider rows are only produced when the gateway is healthy. +func checkOnlineSDK(ctx context.Context, client openshell.Client, providers []string) []CheckResult { + h, err := client.Health(ctx) + switch { + case err == nil && h.Healthy: + // fall through to provider checks below + case errors.Is(err, openshell.ErrUnauthenticated): + return []CheckResult{{ + Group: "gateway", + Name: "status", + Status: "fail", + Message: "authentication failed — check gateway credentials", + }} + case errors.Is(err, openshell.ErrUnavailable): return []CheckResult{{ Group: "gateway", Name: "status", Status: "warn", Message: "gateway not reachable (Phase 2 checks skipped)", }} + case err != nil: + return []CheckResult{{ + Group: "gateway", + Name: "status", + Status: "warn", + Message: fmt.Sprintf("gateway health check failed: %v (Phase 2 checks skipped)", err), + }} + default: // err == nil but not healthy + return []CheckResult{{ + Group: "gateway", + Name: "status", + Status: "warn", + Message: "gateway reports unhealthy (Phase 2 checks skipped)", + }} } - var results []CheckResult - results = append(results, CheckResult{ + results := []CheckResult{{ Group: "gateway", Name: "status", Status: "pass", Message: "connected", - }) + }} - for _, p := range cfg.Providers { - if gw.ProviderGet(p.Profile) == nil { + provs, err := client.Providers(ctx) + if err != nil { + results = append(results, CheckResult{ + Group: "gateway", + Name: "providers", + Status: "warn", + Message: fmt.Sprintf("could not list providers: %v", err), + }) + return results + } + + registered := make(map[string]bool, len(provs)) + for _, p := range provs { + registered[p.Name] = true + } + for _, name := range providers { + if registered[name] { results = append(results, CheckResult{ Group: "gateway", - Name: p.Profile, + Name: name, Status: "pass", Message: "registered", }) } else { results = append(results, CheckResult{ Group: "gateway", - Name: p.Profile, + Name: name, Status: "warn", Message: "not registered (will be registered on apply)", }) diff --git a/cmd/doctor_test.go b/cmd/doctor_test.go index 517d42e..6c20365 100644 --- a/cmd/doctor_test.go +++ b/cmd/doctor_test.go @@ -1,11 +1,16 @@ package cmd import ( + "context" "os" "path/filepath" "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/agent" + "github.com/stackrox/harness-openshell/internal/openshell" + "github.com/stackrox/harness-openshell/internal/testutil" ) func TestCheckOpenShell_Found(t *testing.T) { @@ -251,3 +256,84 @@ func writeProviderProfile(t *testing.T, harnessDir, name, content string) { t.Fatal(err) } } + +func TestCheckOnlineSDK_Healthy(t *testing.T) { + c := testutil.NewFake("default", fake.WithHealthResult(&types.HealthResult{Healthy: true, Version: "1.0.0"})) + results := checkOnlineSDK(context.Background(), c, nil) + if len(results) != 1 { + t.Fatalf("expected 1 result, got %d", len(results)) + } + if results[0].Group != "gateway" || results[0].Name != "status" || results[0].Status != "pass" { + t.Errorf("unexpected status result: %+v", results[0]) + } +} + +func TestCheckOnlineSDK_ProviderRegistered(t *testing.T) { + c, raw := testutil.NewFakeClient("default", fake.WithHealthResult(&types.HealthResult{Healthy: true})) + raw.AddProvider("default", &types.Provider{Name: "github", Type: "git"}) + results := checkOnlineSDK(context.Background(), c, []string{"github"}) + // results[0] is gateway/status pass; results[1] is the provider row. + if len(results) != 2 { + t.Fatalf("expected 2 results, got %d: %+v", len(results), results) + } + if results[1].Name != "github" || results[1].Status != "pass" || results[1].Message != "registered" { + t.Errorf("expected github registered pass, got %+v", results[1]) + } +} + +func TestCheckOnlineSDK_ProviderNotRegistered(t *testing.T) { + c := testutil.NewFake("default", fake.WithHealthResult(&types.HealthResult{Healthy: true})) + results := checkOnlineSDK(context.Background(), c, []string{"github"}) + if len(results) != 2 { + t.Fatalf("expected 2 results, got %d: %+v", len(results), results) + } + if results[1].Name != "github" || results[1].Status != "warn" { + t.Errorf("expected github not-registered warn, got %+v", results[1]) + } +} + +func TestRunOnlineChecks_NoGateway(t *testing.T) { + // No --gateway: single non-fatal warn, factory never called. + called := false + f := func(ctx context.Context, tgt openshell.Target) (openshell.Client, error) { + called = true + return nil, nil + } + results := runOnlineChecks(context.Background(), f, "", "default", nil) + if len(results) != 1 || results[0].Status != "warn" { + t.Fatalf("expected 1 warn result, got %+v", results) + } + if called { + t.Error("factory should not be called when --gateway is empty") + } +} + +func TestRunOnlineChecks_HealthyViaFactory(t *testing.T) { + c := testutil.NewFake("default", fake.WithHealthResult(&types.HealthResult{Healthy: true, Version: "1.0.0"})) + f := testutil.FakeFactory(c) + results := runOnlineChecks(context.Background(), f, "some-gateway", "default", nil) + if len(results) != 1 || results[0].Status != "pass" { + t.Fatalf("expected 1 pass result, got %+v", results) + } +} + +func TestResolveOnlineFlag(t *testing.T) { + const envKey = "OPENSHELL_TEST_RESOLVE" + + // Explicit flag wins over env var and default. + t.Setenv(envKey, "from-env") + if got := resolveOnlineFlag("from-flag", envKey, "from-default"); got != "from-flag" { + t.Errorf("flag precedence: got %q, want from-flag", got) + } + + // Empty flag falls back to the env var over the default. + if got := resolveOnlineFlag("", envKey, "from-default"); got != "from-env" { + t.Errorf("env precedence: got %q, want from-env", got) + } + + // Empty flag and unset env fall back to the default. + t.Setenv(envKey, "") + if got := resolveOnlineFlag("", envKey, "from-default"); got != "from-default" { + t.Errorf("default fallback: got %q, want from-default", got) + } +} diff --git a/main.go b/main.go index b8c6efb..b778bed 100644 --- a/main.go +++ b/main.go @@ -6,9 +6,10 @@ import ( "os" "path/filepath" + "github.com/spf13/cobra" "github.com/stackrox/harness-openshell/cmd" + "github.com/stackrox/harness-openshell/internal/openshell/sdkclient" "github.com/stackrox/harness-openshell/internal/status" - "github.com/spf13/cobra" ) var version = "dev" @@ -54,9 +55,9 @@ func main() { cmd.Version = version cmd.DefaultAgentConfig = defaultAgentConfig cmd.EmbeddedGatewayProfiles = map[string][]byte{ - "local-container": localContainerGatewayProfile, - "helm": helmNodeportGatewayProfile, - "openshift": helmOpenshiftRouteGatewayProfile, + "local-container": localContainerGatewayProfile, + "helm": helmNodeportGatewayProfile, + "openshift": helmOpenshiftRouteGatewayProfile, } root.CompletionOptions.HiddenDefaultCmd = true @@ -66,7 +67,7 @@ func main() { cmd.NewDescribeCmd(harnessDir, cli), cmd.NewDeleteCmd(harnessDir, cli), cmd.NewDeployCmd(harnessDir, cli), - cmd.NewDoctorCmd(harnessDir, cli), + cmd.NewDoctorCmd(harnessDir, cli, sdkclient.New), cmd.NewInitCmd(harnessDir), )