Skip to content
51 changes: 51 additions & 0 deletions cmd/plan_test.go
Original file line numberDiff line numberDiff line change
Expand Up@@ -11,6 +11,7 @@ import (
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"
"github.com/stackrox/harness-openshell/internal/plan"
"github.com/stackrox/harness-openshell/internal/testutil"
)

Expand DownExpand Up@@ -132,6 +133,56 @@ spec:
}
}

// TestPlanCmd_InferenceRealDiff proves the inference row reflects the gateway's
// actual route: a matching seeded route renders noop, not the old flat validate.
func TestPlanCmd_InferenceRealDiff(t *testing.T) {
tmpDir := t.TempDir()

configPath := filepath.Join(tmpDir, "plan-test.yaml")
configContent := `apiVersion: harness.openshell.dev/v1alpha1
kind: Harness
metadata:
name: plan-test
spec:
target:
gateway: test-gateway
inference:
provider: test-provider
model: claude-haiku-4-5
`
if err := os.WriteFile(configPath, []byte(configContent), 0o644); err != nil {
t.Fatalf("write config: %v", err)
}

fakeClient := testutil.NewFake("default", fake.WithHealthResult(&types.HealthResult{
Healthy: true, Version: "1.0.0",
}))
// Seed a route matching the desired config, under the resolved default name.
if _, err := fakeClient.SetInferenceRoute(context.Background(), openshell.InferenceRouteConfig{
Provider: "test-provider", Model: "claude-haiku-4-5", Route: plan.DefaultInferenceRoute, NoVerify: true,
}); err != nil {
t.Fatalf("seed route: %v", err)
}

cmd := NewPlanCmd(tmpDir, testutil.FakeFactory(fakeClient))
cmd.SetArgs([]string{"-f", configPath, "-o", "table"})

output, err := captureStdout(t, func() error { return cmd.Execute() })
if err != nil {
t.Fatalf("cmd.Execute: %v", err)
}

if !contains(output, "INFERENCE") {
t.Fatalf("output missing INFERENCE section:\n%s", output)
}
if !contains(output, "noop") {
t.Errorf("expected inference noop for a matching route:\n%s", output)
}
if contains(output, "does not report inference state") {
t.Errorf("capable gateway should not render the config-only caveat:\n%s", output)
}
}

// TestPlanCmd_JSONOutput tests JSON output format.
func TestPlanCmd_JSONOutput(t *testing.T) {
tmpDir := t.TempDir()
Expand Down
23 changes: 23 additions & 0 deletions internal/config/env.go
Original file line numberDiff line numberDiff line change
Expand Up@@ -112,6 +112,29 @@ func Resolve(h *Harness, getenv func(string) string) (*Harness, error) {
s.Inference.Provider = exp("spec.inference.provider", h.Spec.Inference.Provider)
s.Inference.Model = exp("spec.inference.model", h.Spec.Inference.Model)
s.Inference.Timeout = exp("spec.inference.timeout", h.Spec.Inference.Timeout)
// Validate the (now expanded) timeout once, here at resolve time, so the plan
// diff and reconcile write can parse it without handling an error.
if _, err := s.Inference.TimeoutSecs(); err != nil {
errs = append(errs, fmt.Sprintf("spec.inference.timeout: %v", err))
}
// A configured inference block must name both a provider and a model: the
// gateway rejects a route write that lacks either, and reconcile has nothing
// to write without them. Catch it here at resolve time with a clear message
// instead of surfacing a late ErrInvalidArgument on apply. Route/timeout alone
// (or verify alone) don't identify a route to reconcile. Kept in step with
// plan.isInferenceConfigured.
if s.Inference.Route != "" || s.Inference.Provider != "" || s.Inference.Model != "" || s.Inference.Timeout != "" {
if s.Inference.Provider == "" {
errs = append(errs, "spec.inference.provider: required when inference is configured")
}
if s.Inference.Model == "" {
errs = append(errs, "spec.inference.model: required when inference is configured")
}
}
if v := h.Spec.Inference.Verify; v != nil {
b := *v // copy so the resolved struct never aliases the input's *bool
s.Inference.Verify = &b
}

s.Sandbox.Image = exp("spec.sandbox.image", h.Spec.Sandbox.Image)
if p := h.Spec.Sandbox.Policy; p != nil {
Expand Down
67 changes: 67 additions & 0 deletions internal/config/env_test.go
Original file line numberDiff line numberDiff line change
Expand Up@@ -123,6 +123,73 @@ func TestResolveEmptyString(t *testing.T) {
}
}

func TestResolveInvalidTimeout(t *testing.T) {
h := &Harness{
APIVersion: "harness.openshell.dev/v1alpha1",
Kind: "Harness",
Metadata: Metadata{Name: "test"},
Spec: Spec{Inference: Inference{Timeout: "60"}}, // bare integer, no unit
}

if _, err := Resolve(h, func(string) string { return "" }); err == nil {
t.Fatal("expected Resolve to reject a unitless inference timeout")
}
}

func TestResolveValidTimeout(t *testing.T) {
h := &Harness{
APIVersion: "harness.openshell.dev/v1alpha1",
Kind: "Harness",
Metadata: Metadata{Name: "test"},
// Provider+model are required whenever the inference block is configured;
// this test only exercises timeout expansion, so supply them as fixtures.
Spec: Spec{Inference: Inference{Provider: "gcp", Model: "claude-opus-4-8", Timeout: "${INF_TIMEOUT}"}},
}

resolved, err := Resolve(h, func(name string) string {
if name == "INF_TIMEOUT" {
return "90s"
}
return ""
})
if err != nil {
t.Fatalf("Resolve failed: %v", err)
}
secs, err := resolved.Spec.Inference.TimeoutSecs()
if err != nil || secs != 90 {
t.Errorf("resolved+parsed timeout = %d (err %v), want 90", secs, err)
}
}

func TestResolveVerifyRoundTrips(t *testing.T) {
// verify:false must survive YAML parse + Resolve as an explicit false, not
// collapse to the nil→true default, and must not alias the input pointer.
src := `apiVersion: harness.openshell.dev/v1alpha1
kind: Harness
metadata:
name: test
spec:
inference:
provider: gcp
model: claude-opus-4-8
verify: false
`
h, err := Parse([]byte(src))
if err != nil {
t.Fatalf("Parse: %v", err)
}
resolved, err := Resolve(h, func(string) string { return "" })
if err != nil {
t.Fatalf("Resolve: %v", err)
}
if resolved.Spec.Inference.VerifyEnabled() {
t.Error("verify:false should resolve to VerifyEnabled()==false")
}
if resolved.Spec.Inference.Verify == h.Spec.Inference.Verify {
t.Error("resolved Verify aliases the input's *bool pointer")
}
}

func TestResolveNonSecretField(t *testing.T) {
// Build Harness with ${SECRET_ISH} in non-secret field
h := &Harness{
Expand Down
40 changes: 38 additions & 2 deletions internal/config/types.go
Original file line numberDiff line numberDiff line change
Expand Up@@ -5,7 +5,11 @@
// the source (e.g. "gcloud-adc").
package config

import "strings"
import (
"fmt"
"strings"
"time"
)

// Harness is the root v1alpha1 configuration document.
type Harness struct {
Expand DownExpand Up@@ -88,7 +92,39 @@ type Inference struct {
Provider string `yaml:"provider,omitempty"`
Model string `yaml:"model,omitempty"`
Timeout string `yaml:"timeout,omitempty"`
Verify bool `yaml:"verify,omitempty"`
// Verify controls the gateway's synchronous endpoint validation on write.
// It is a *bool so "unset" is distinct from "false": unset (nil) means
// verify (the safe default), so only an explicit `verify: false` skips it.
// Verify is not part of the reconcile diff, so changing only this field does
// not by itself trigger a re-write; it takes effect on the next write caused
// by a provider/model/timeout change.
Verify *bool `yaml:"verify,omitempty"`
}

// VerifyEnabled reports whether endpoint verification should run on write.
// Unset (nil) defaults to true; this is the single owner of the nil→verify rule.
func (inf Inference) VerifyEnabled() bool {
return inf.Verify == nil || *inf.Verify
}

// TimeoutSecs parses Timeout (a Go duration string like "60s" or "2m") into
// whole seconds. Empty → 0, meaning "let the gateway apply its default". A bare
// number without a unit (e.g. "60") is an error — the unit is required so the
// meaning is unambiguous. This is the single owner of the Timeout → seconds
// conversion; the plan diff and the reconcile write both call it. Validated at
// Resolve time, so by plan/reconcile time it cannot fail.
func (inf Inference) TimeoutSecs() (uint64, error) {
if inf.Timeout == "" {
return 0, nil
}
d, err := time.ParseDuration(inf.Timeout)
if err != nil {
return 0, fmt.Errorf("invalid timeout %q: want a duration string like \"60s\" or \"2m\"", inf.Timeout)
}
if d < 0 {
return 0, fmt.Errorf("invalid timeout %q: must not be negative", inf.Timeout)
}
return uint64(d.Round(time.Second) / time.Second), nil
}

// Sandbox describes the execution sandbox for this run.
Expand Down
39 changes: 39 additions & 0 deletions internal/config/types_test.go
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
package config

import "testing"

func TestInferenceTimeoutSecs(t *testing.T) {
tests := []struct {
name string
timeout string
want uint64
wantErr bool
}{
{name: "empty is zero", timeout: "", want: 0},
{name: "seconds", timeout: "60s", want: 60},
{name: "minutes", timeout: "2m", want: 120},
{name: "mixed", timeout: "1m30s", want: 90},
{name: "sub-second rounds", timeout: "1500ms", want: 2},
{name: "bare integer is an error", timeout: "60", wantErr: true},
{name: "garbage is an error", timeout: "soon", wantErr: true},
{name: "negative is an error", timeout: "-5s", wantErr: true},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, err := Inference{Timeout: tt.timeout}.TimeoutSecs()
if tt.wantErr {
if err == nil {
t.Fatalf("expected error for %q, got %d", tt.timeout, got)
}
return
}
if err != nil {
t.Fatalf("unexpected error for %q: %v", tt.timeout, err)
}
if got != tt.want {
t.Errorf("TimeoutSecs(%q) = %d, want %d", tt.timeout, got, tt.want)
}
})
}
}
12 changes: 12 additions & 0 deletions internal/openshell/client.go
Original file line numberDiff line numberDiff line change
Expand Up@@ -18,6 +18,18 @@ type Client interface {
Health(ctx context.Context) (Health, error)
// Providers lists the providers registered in the bound workspace.
Providers(ctx context.Context) ([]Provider, error)
// GetInferenceRoute reads the named inference route in the bound workspace.
// An empty route targets the gateway default route. Returns ErrNotFound when
// no such route exists (requires the workspace "user" role).
GetInferenceRoute(ctx context.Context, route string) (InferenceRoute, error)
// SetInferenceRoute creates or updates an inference route in the bound
// workspace (upsert). Returns the resulting route. Requires the workspace
// "admin" role; a caller lacking it gets ErrPermission.
SetInferenceRoute(ctx context.Context, cfg InferenceRouteConfig) (InferenceRoute, error)
// DeleteInferenceRoute removes the named inference route in the bound
// workspace. Idempotent: deleting a missing route is not an error. Requires
// the workspace "admin" role.
DeleteInferenceRoute(ctx context.Context, route string) error
// Close releases any resources held by the client.
Close() error
}
Expand Down
4 changes: 4 additions & 0 deletions internal/openshell/errors.go
Original file line numberDiff line numberDiff line change
Expand Up@@ -19,6 +19,10 @@ var (
// ErrUnsupported is returned when the gateway does not implement the
// requested RPC (gRPC Unimplemented).
ErrUnsupported = errors.New("openshell: not supported by gateway")
// ErrInvalidArgument is returned when the caller supplied invalid input
// (e.g. a required field left empty), rejected by the gateway before any
// state change (gRPC InvalidArgument).
ErrInvalidArgument = errors.New("openshell: invalid argument")
// ErrConfig is returned when the gateway config cannot be loaded, parsed, or
// its auth mode cannot be satisfied.
ErrConfig = errors.New("openshell: gateway config error")
Expand Down
8 changes: 8 additions & 0 deletions internal/openshell/sdkclient/client_test.go
Original file line numberDiff line numberDiff line change
Expand Up@@ -214,6 +214,14 @@ func TestTranslate(t *testing.T) {
},
expectSent: openshell.ErrPermission,
},
{
name: "invalid argument",
err: &types.StatusError{
Code: types.ErrorInvalidArgument,
Message: "invalid argument",
},
expectSent: openshell.ErrInvalidArgument,
},
}

for _, tt := range tests {
Expand Down
2 changes: 2 additions & 0 deletions internal/openshell/sdkclient/errors.go
Original file line numberDiff line numberDiff line change
Expand Up@@ -25,6 +25,8 @@ func translate(err error) error {
return fmt.Errorf("%w: %v", openshell.ErrUnauthenticated, err)
case v1.IsPermissionDenied(err):
return fmt.Errorf("%w: %v", openshell.ErrPermission, err)
case v1.IsInvalidArgument(err):
return fmt.Errorf("%w: %v", openshell.ErrInvalidArgument, err)
default:
return err
}
Expand Down
53 changes: 53 additions & 0 deletions internal/openshell/sdkclient/inference.go
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
package sdkclient

import (
"context"

v1 "github.com/NVIDIA/OpenShell/sdk/go/openshell/v1"

"github.com/stackrox/harness-openshell/internal/openshell"
)

// fromSDKInferenceRoute maps the SDK route view to the minimal harness view.
// Deliberately narrow (least-exposure firewall); widen only when a consumer
// genuinely needs more fields, changing this and openshell.InferenceRoute
// together.
func fromSDKInferenceRoute(r *v1.InferenceRoute) openshell.InferenceRoute {
return openshell.InferenceRoute{
Provider: r.ProviderName,
Model: r.ModelID,
Route: r.RouteName,
TimeoutSecs: r.TimeoutSecs,
Version: r.Version,
}
}

// GetInferenceRoute reads the named inference route in the bound workspace.
func (c *client) GetInferenceRoute(ctx context.Context, route string) (openshell.InferenceRoute, error) {
r, err := c.raw.Inference().GetRoute(ctx, c.workspace, route)
if err != nil {
return openshell.InferenceRoute{}, translate(err)
}
return fromSDKInferenceRoute(r), nil
}

// SetInferenceRoute creates or updates (upserts) an inference route in the bound
// workspace.
func (c *client) SetInferenceRoute(ctx context.Context, cfg openshell.InferenceRouteConfig) (openshell.InferenceRoute, error) {
r, err := c.raw.Inference().SetRoute(ctx, c.workspace, &v1.InferenceRouteConfig{
ProviderName: cfg.Provider,
ModelID: cfg.Model,
RouteName: cfg.Route,
NoVerify: cfg.NoVerify,
TimeoutSecs: cfg.TimeoutSecs,
})
if err != nil {
return openshell.InferenceRoute{}, translate(err)
}
return fromSDKInferenceRoute(r), nil
}

// DeleteInferenceRoute removes the named inference route in the bound workspace.
func (c *client) DeleteInferenceRoute(ctx context.Context, route string) error {
return translate(c.raw.Inference().DeleteRoute(ctx, c.workspace, route))
}
Loading
Loading