Phase 3: Startup validation (V-01..V-07) — upstream_inject types and validateAuthServerIntegration

[tgrunnagle]

Description

Add the upstream_inject auth strategy types to pkg/vmcp/auth/types/types.go and implement the validateAuthServerIntegration function in pkg/vmcp/config/validator.go, covering all seven validation rules (V-01 through V-07) that guard the embedded auth server configuration. Write comprehensive table-driven unit tests for all rules and extend the existing validateAuthServerConfig unit tests in the operator CRD types package.

Context

RFC-0053 adds an optional embedded OAuth/OIDC authorization server to vMCP. Phase 1 (#4140) established the foundational config model types — AuthServerConfig, the AuthServer field on Config, and the AuthServerConfigRef CRD field. Phase 3 builds on that skeleton to add the validation layer that prevents misconfigured auth wiring from reaching runtime. Without these startup checks, a user who mistakenly configures upstream_inject without an auth server, or who sets a mismatched issuer, would only discover the problem at token validation time rather than at startup.

The seven validation rules (V-01 through V-07) enforce the contracts between the embedded AS, the incoming OIDC config, and the outgoing backend auth strategies. V-01/V-02/V-06 guard the upstream_inject strategy against invalid provider references; V-03 warns about a common misconfiguration of token_exchange when the AS is the incoming auth provider; V-04/V-05/V-07 guard issuer and audience consistency between the AS and the incoming OIDC config.

Parent epic: #4120 — vMCP: add embedded authorization server
RFC document: docs/proposals/THV-0053-vmcp-embedded-authserver.md
Dependencies: #4140 (Phase 1 — foundation types must exist before validation can reference them)
Blocks: Phase 4 (operator reconciler), which requires the validation rules to be available for the Kubernetes reconciliation loop

Acceptance Criteria

• pkg/vmcp/auth/types/types.go exports StrategyTypeUpstreamInject = "upstream_inject" constant alongside the existing strategy type constants
• pkg/vmcp/auth/types/types.go exports UpstreamInjectConfig struct with a ProviderName string field and +kubebuilder:object:generate=true annotation, with both json:"providerName" and yaml:"providerName" tags
• pkg/vmcp/auth/types/types.go BackendAuthStrategy struct has a new UpstreamInject *UpstreamInjectConfig field with json:"upstreamInject,omitempty" yaml:"upstreamInject,omitempty" tags
• pkg/vmcp/config/validator.go calls validateAuthServerIntegration(cfg) as the last step in DefaultValidator.Validate() (before the final error join)
• V-01: validateAuthServerIntegration returns an error when any backend strategy has Type == "upstream_inject" and cfg.AuthServer == nil
• V-02: validateAuthServerIntegration returns an error when upstream_inject ProviderName is non-empty but does not match any Name in cfg.AuthServer.RunConfig.Upstreams
• V-03: validateAuthServerIntegration emits slog.Warn (not an error) when any backend strategy has Type == "token_exchange" and the incoming auth type is "oidc" with the issuer equal to the AS issuer
• V-04: validateAuthServerIntegration returns an error when cfg.AuthServer.RunConfig.Issuer does not exactly match cfg.IncomingAuth.OIDC.Issuer (when both are non-nil/non-empty)
• V-05: validateAuthServerIntegration returns an error when runner.NewEmbeddedAuthServer internal config validation fails (i.e., the AS RunConfig itself is invalid)
• V-06: validateAuthServerIntegration returns an error when any backend strategy has Type == "upstream_inject" and UpstreamInject.ProviderName is empty
• V-07: validateAuthServerIntegration returns an error when cfg.IncomingAuth.OIDC.Audience is non-empty and is not present in cfg.AuthServer.RunConfig.AllowedAudiences (YAML path check only)
• validateBackendAuthStrategy in validator.go accepts "upstream_inject" as a valid strategy type (added to validTypes)
• validateBackendAuthStrategy returns an error when Type == "upstream_inject" and UpstreamInject config is nil (V-06 pre-check)
• Helper functions collectAllBackendStrategies, hasUpstreamProvider, and containsString are added to validator.go
• pkg/vmcp/config/validator_test.go contains a table-driven test function TestValidateAuthServerIntegration with one test case per rule: V-01 through V-07, plus a Mode A pass-through case (nil AuthServer) and a valid Mode B config case
• cmd/thv-operator/api/v1alpha1/virtualmcpserver_types_test.go contains unit tests for validateAuthServerConfig covering empty Name rejection and nil-pointer safety (nil AuthServerConfigRef must not panic or error)
• All existing tests continue to pass (Mode A configs with nil AuthServer must not be affected)
• All new Go files and new code additions include the SPDX license header
• task test passes
• task lint passes (or task lint-fix resolves all issues)

Technical Approach

Recommended Implementation

Work in three discrete steps: (1) add the upstream_inject types to pkg/vmcp/auth/types/types.go; (2) extend validateBackendAuthStrategy and add validateAuthServerIntegration with helpers to validator.go; (3) write the unit tests. Step 1 is a prerequisite for Step 2 because the validator references authtypes.StrategyTypeUpstreamInject and authtypes.UpstreamInjectConfig.

For V-05, use runner.NewEmbeddedAuthServer to validate the AS config internally. Because this function requires a context.Context, pass context.Background(). NewEmbeddedAuthServer validates the RunConfig synchronously during construction; the returned server can be discarded (or closed immediately with defer as.Close()) — the validation side-effect is what matters. This approach avoids duplicating the AS's own validation logic.

For V-03, use slog.Warn with a structured message; do not return an error. The warning should include the backend name and the matching issuer so operators can identify the misconfiguration.

The containsString helper is functionally identical to the existing private contains(slice []string, item string) bool in validator.go. Either alias it or add a new export — the architecture note in research.md says the existing helper "can be aliased as containsString". Since Go does not support type aliases for functions, add containsString as a new named function calling the existing contains helper, or simply inline the logic.

For the Ginkgo-vs-testing question: the architecture.md constraint says "if Phase 3 adds Ginkgo DescribeTable tests, a Ginkgo suite bootstrap must also be added to the config package. Alternatively, use the existing testing table-driven style to avoid the bootstrap." Use the existing testing package table-driven style to stay consistent with the current validator_test.go and avoid a Ginkgo bootstrap dependency. The table structure []struct{name, cfg, wantErr, wantWarn, errContains string} cleanly covers all nine cases.

Patterns and Frameworks

• Table-driven tests (testing package): Match the pattern in pkg/vmcp/config/validator_test.go — []struct{name, cfg, wantErr bool, errMsg string} with t.Run + t.Parallel(). Do not introduce Ginkgo to this package.
• slog.Warn for non-fatal validation: Follow the logging guideline in CLAUDE.md — slog.Warn for non-fatal issues (deprecations, fallback behavior). Use structured fields for context.
• Helper composition: collectAllBackendStrategies walks cfg.OutgoingAuth.Default and all entries in cfg.OutgoingAuth.Backends, returning []*authtypes.BackendAuthStrategy. This is a simple slice-building function — keep it pure and testable.
• SPDX headers: Every Go file must start with // SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. and // SPDX-License-Identifier: Apache-2.0. Use task license-fix to add missing headers automatically.
• require.NoError(t, err) for test assertions: Use require from github.com/stretchr/testify/require rather than t.Fatal for error checking in tests.
• +kubebuilder:object:generate=true on UpstreamInjectConfig: Required so controller-gen generates DeepCopyInto/DeepCopy for the new struct. After adding this annotation, run task gen from repo root to regenerate pkg/vmcp/auth/types/zz_generated.deepcopy.go (if it exists) or verify the generator picks up the new struct.

Code Pointers

• pkg/vmcp/auth/types/types.go — Add StrategyTypeUpstreamInject constant here (alongside StrategyTypeUnauthenticated, StrategyTypeHeaderInjection, StrategyTypeTokenExchange). Add UpstreamInjectConfig struct and UpstreamInject *UpstreamInjectConfig field to BackendAuthStrategy. This is the primary type file for Phase 3 additions.
• pkg/vmcp/config/validator.go (lines 182–223) — validateBackendAuthStrategy: add authtypes.StrategyTypeUpstreamInject to validTypes slice (line 187–193) and add a case authtypes.StrategyTypeUpstreamInject: block in the switch statement handling V-01 (nil AS check) and V-06 (empty provider name check).
• pkg/vmcp/config/validator.go (lines 31–81) — DefaultValidator.Validate(): add validateAuthServerIntegration(cfg) call at the end, before the final error aggregation.
• pkg/vmcp/config/validator.go (lines 441–457) — Existing contains and containsStrategy helpers at the bottom of the file. Add the new helper functions collectAllBackendStrategies, hasUpstreamProvider, and containsString in this section.
• pkg/vmcp/config/validator_test.go — Add TestValidateAuthServerIntegration using the existing table-driven testing style. Import authserver "github.com/stacklok/toolhive/pkg/authserver" to construct test RunConfig values.
• pkg/authserver/config.go (lines 33–74) — authserver.RunConfig: the type wrapped by AuthServerConfig.RunConfig. Key validation-relevant fields: Issuer string, AllowedAudiences []string, Upstreams []UpstreamRunConfig (each with Name string). Use these to construct test fixtures.
• pkg/authserver/runner/embeddedauthserver.go — NewEmbeddedAuthServer(ctx, *authserver.RunConfig) (*EmbeddedAuthServer, error) — called by V-05 to perform internal AS config validation.
• cmd/thv-operator/api/v1alpha1/virtualmcpserver_types_test.go — Add TestValidateAuthServerConfig function here, following the existing TestValidateEmbeddingServer pattern (table-driven, parallel). Tests should cover: nil AuthServerConfigRef (no error), non-nil ref with non-empty Name (no error), non-nil ref with empty Name (error containing "spec.authServerConfigRef.name is required").
• cmd/thv-operator/api/v1alpha1/virtualmcpserver_types.go (lines 362–398) — VirtualMCPServer.Validate(): the Phase 1 task (Phase 1: Foundation — add AuthServerConfig model, CRD field, and structural validation #4140) adds a call to r.validateAuthServerConfig() here. The Phase 3 tests exercise this path; verify the call is present before writing tests.

Component Interfaces

New constant and types in pkg/vmcp/auth/types/types.go:

// StrategyTypeUpstreamInject identifies the upstream inject strategy.
// This strategy injects an upstream provider token (acquired by the embedded AS)
// as a Bearer token when authenticating to the backend.
// The actual outgoing strategy implementation is deferred to a follow-up RFC;
// only the constant, config struct, and validation are added here.
const StrategyTypeUpstreamInject = "upstream_inject"

// UpstreamInjectConfig configures the upstream inject auth strategy.
// Used when Type = "upstream_inject".
// +kubebuilder:object:generate=true
type UpstreamInjectConfig struct {
    // ProviderName is the upstream provider whose token is injected as Bearer.
    // Must match a provider name in authServer.runConfig.upstreams.
    ProviderName string `json:"providerName" yaml:"providerName"`
}

Addition to BackendAuthStrategy in pkg/vmcp/auth/types/types.go:

// UpstreamInject contains configuration for upstream inject auth strategy.
// Used when Type = "upstream_inject".
// +optional
UpstreamInject *UpstreamInjectConfig `json:"upstreamInject,omitempty" yaml:"upstreamInject,omitempty"`

New function signatures in pkg/vmcp/config/validator.go:

// validateAuthServerIntegration validates the relationship between AuthServer config,
// IncomingAuth config, and backend auth strategies.
// Called from DefaultValidator.Validate() when cfg.AuthServer is non-nil (Mode B checks)
// or when upstream_inject strategies are present (V-01 requires AS).
// Rules V-01 through V-07 are checked here.
func (v *DefaultValidator) validateAuthServerIntegration(cfg *Config) error

// collectAllBackendStrategies returns a flat slice of all configured backend auth strategies,
// including the default strategy and all per-backend strategies.
func collectAllBackendStrategies(auth *OutgoingAuthConfig) []*authtypes.BackendAuthStrategy

// hasUpstreamProvider reports whether any upstream in cfg.AuthServer.RunConfig.Upstreams
// has Name equal to providerName.
func hasUpstreamProvider(cfg *Config, providerName string) bool

// containsString reports whether slice contains item.
func containsString(slice []string, item string) bool

Validation rule summary (for implementation reference):

Rule	Condition	Severity	Short description
V-01	upstream_inject used + AuthServer == nil	error	AS required for upstream_inject
V-02	upstream_inject ProviderName not in AS upstreams	error	Unknown provider name
V-03	token_exchange + AS is incoming auth issuer	slog.Warn	Likely misconfiguration
V-04	AuthServer.RunConfig.Issuer != IncomingAuth.OIDC.Issuer	error	Issuer mismatch
V-05	NewEmbeddedAuthServer internal validation fails	error	AS config invalid
V-06	upstream_inject ProviderName is empty	error	Missing provider name
V-07	IncomingAuth.OIDC.Audience not in AllowedAudiences	error	Audience not permitted

Testing Strategy

Unit Tests — TestValidateAuthServerIntegration in pkg/vmcp/config/validator_test.go

Use the existing table-driven testing style. Each test case sets up a *Config and checks v.Validate(cfg) != nil (or == nil for passing cases). Construct test authserver.RunConfig values inline in the table — they do not need to be fully valid for all rules (e.g., V-01 just needs AuthServer == nil).

• V-01 case: OutgoingAuth.Backends["test"] has Type = "upstream_inject", AuthServer = nil → error containing "upstream_inject" and "authServer"
• V-02 case: AuthServer non-nil with upstream named "github", backend has upstream_inject with ProviderName = "unknown" → error containing "unknown" and "not found"
• V-03 case: AuthServer.RunConfig.Issuer = "https://as.example.com", IncomingAuth.OIDC.Issuer = "https://as.example.com", backend has token_exchange → no error (warning only, captured via slog test hook or verified by absence of error)
• V-04 case: AuthServer.RunConfig.Issuer = "https://as.example.com", IncomingAuth.OIDC.Issuer = "https://external-idp.example.com" → error containing "issuer" and "mismatch"
• V-05 case: AuthServer.RunConfig is structurally invalid (e.g., empty Issuer) → error surfacing the AS internal validation failure
• V-06 case: backend has Type = "upstream_inject", UpstreamInject.ProviderName = "" → error containing "providerName" and "required"
• V-07 case: AuthServer.RunConfig.AllowedAudiences = ["https://vmcp.example.com"], IncomingAuth.OIDC.Audience = "https://other.example.com" → error containing "audience" and "allowedAudiences"
• Mode A pass-through: AuthServer = nil, no upstream_inject backends → no error
• Valid Mode B config: all fields consistent (matching issuer, audience in AllowedAudiences, valid upstream name) → no error

Unit Tests — TestValidateAuthServerConfig in cmd/thv-operator/api/v1alpha1/virtualmcpserver_types_test.go

Follow the TestValidateEmbeddingServer pattern (table-driven, t.Parallel(), require.NoError/require.Error assertions):

• Nil AuthServerConfigRef: VirtualMCPServer.Validate() returns nil (no regression, Mode A)
• Non-nil AuthServerConfigRef with non-empty Name: Validate() returns nil
• Non-nil AuthServerConfigRef with empty Name: Validate() returns error containing "spec.authServerConfigRef.name is required"

Edge Cases

• OutgoingAuth.Default strategy is upstream_inject (not just per-backend) — collectAllBackendStrategies must include the default strategy in its output
• OutgoingAuth is nil — collectAllBackendStrategies must return empty slice without panicking
• cfg.IncomingAuth is not OIDC type (e.g., anonymous) — V-04 and V-07 must skip gracefully when cfg.IncomingAuth.OIDC == nil
• Multiple backends with mixed strategies — only upstream_inject backends trigger V-01/V-02/V-06
• Exact string match for V-04 and V-07 (no URL normalization — trailing slash differences cause failures)

Out of Scope

• upstream_inject outgoing strategy implementation (deferred to a follow-up RFC — the constant and config struct are added, but the actual middleware that injects the token is not implemented here)
• Operator reconciler cross-resource validation (AuthServerConfigValid condition surfacing) — Phase 4
• CRD-to-config converter changes in converter.go — Phase 4
• E2E tests for Mode A/Mode B behavior — Phase 4
• HTTP handler registration and AS route mounting — Phase 2
• Documentation updates to docs/arch/ — Phase 4
• Hot-reload of AS configuration
• Multi-upstream IDP support beyond what UpstreamRunConfig.Name already provides in authserver.RunConfig

References

• RFC-0053 design document: docs/proposals/THV-0053-vmcp-embedded-authserver.md
• Parent epic: vMCP: add embedded authorization server #4120
• Phase 1 (foundation, upstream dependency): Phase 1: Foundation — add AuthServerConfig model, CRD field, and structural validation #4140
• RFC-0052 (multi-upstream IDP, required for full E2E): Auth Server: multi-upstream provider support #3924
• Auth strategy types (primary edit target): pkg/vmcp/auth/types/types.go
• Config validator (primary edit target): pkg/vmcp/config/validator.go
• Auth server RunConfig (test fixture type): pkg/authserver/config.go
• EmbeddedAuthServer (used for V-05 validation): pkg/authserver/runner/embeddedauthserver.go
• Validator tests (pattern to follow): pkg/vmcp/config/validator_test.go
• Operator CRD types tests (pattern to follow): cmd/thv-operator/api/v1alpha1/virtualmcpserver_types_test.go
• validateEmbeddingServer (pattern for validateAuthServerConfig tests): cmd/thv-operator/api/v1alpha1/virtualmcpserver_types.go