Move multi-upstream restriction from authserver library to consumer layers (RFC-0052 Phase 3)

[tgrunnagle]

Description

Move the multi-upstream restriction from the auth server library layer to the consumer layers: lift the len > 1 guard from pkg/authserver/config.go, add explicit-name and uniqueness requirements for multi-upstream configs, add a len > 1 guard to the proxy runner, remove the guard from MCPExternalAuthConfig.validateEmbeddedAuthServer(), and add multi-upstream rejection to the MCPServer and MCPRemoteProxy controller reconcile loops (while leaving VirtualMCPServer intentionally unrestricted). This is the config and operator plumbing that lets VirtualMCPServer reference multi-upstream auth configs once Phase 2 (handler layer) lands.

Context

RFC-0052 introduces a sequential multi-upstream authorization chain in the embedded auth server. Phase 1 (#4136) restructured the storage layer so multiple providers' tokens can coexist per session. Phase 3 updates the validation boundaries: the library (pkg/authserver/) should no longer enforce single-upstream — that check belongs at the consumer layer, where the proxy runner and the K8s workload types MCPServer/MCPRemoteProxy remain single-upstream, while VirtualMCPServer is the intended multi-upstream consumer. Phase 3 can be developed in parallel with Phase 2 after Phase 1 merges.

Dependencies: #4136 (Phase 1 — Storage Layer; the new UpstreamTokenStorage signatures must exist before this phase is merged so that the library layer can accept multi-upstream configs end-to-end)
Blocks: None (TASK-004 depends on TASK-001 only)

Acceptance Criteria

• pkg/authserver/config.go validateUpstreams() no longer contains the if len(c.Upstreams) > 1 rejection block
• validateUpstreams() adds a new explicit-name requirement: when len(c.Upstreams) > 1, each upstream's Name field must be explicitly set (non-empty and not "default") before defaulting; a missing or "default" name in multi-upstream context returns a clear error
• validateUpstreams() name-uniqueness check is retained (already present; verify it runs even after removing the len > 1 early return)
• Config.GetUpstream() method is removed from pkg/authserver/config.go; the sole call site pkg/authserver/server_impl.go is updated to access cfg.Upstreams[0] (or equivalent) directly instead
• TestConfigGetUpstream test function is removed from pkg/authserver/config_test.go; the test case "multiple upstreams" in TestConfigValidate is updated to expect success (no error) instead of the old "multiple upstreams not yet supported" error; the "duplicate upstream names" test case is updated to verify the correct new error message
• A new test case is added in TestConfigValidate for multi-upstream with explicit names (valid) and multi-upstream where one upstream has an empty or "default" name (invalid)
• pkg/runner/ has a new validation path that rejects EmbeddedAuthServerConfig with len(Upstreams) > 1; this validation is invoked before the runner starts the embedded auth server
• New tests in pkg/runner/ (or pkg/runner/config_test.go) cover: embedded auth server config with 1 upstream passes; embedded auth server config with 2 upstreams fails with a clear error
• MCPExternalAuthConfig.validateEmbeddedAuthServer() in cmd/thv-operator/api/v1alpha1/mcpexternalauthconfig_types.go no longer contains the if len(cfg.UpstreamProviders) > 1 rejection block and its comment
• MCPExternalAuthConfig.Validate() in cmd/thv-operator/api/v1alpha1/mcpexternalauthconfig_types_test.go no longer has a test case that expects validateEmbeddedAuthServer to reject len > 1 upstreams; a new test case verifies that a multi-upstream MCPExternalAuthConfig passes Validate() successfully
• MCPServer controller reconcile logic in cmd/thv-operator/controllers/mcpserver_controller.go rejects an MCPExternalAuthConfig reference that has embeddedAuthServer type with more than one upstream provider, using the ConditionTypeExternalAuthConfigValidated status condition pattern already present in handleExternalAuthConfig; the condition message clearly identifies the cause
• MCPRemoteProxy controller reconcile logic in cmd/thv-operator/controllers/mcpremoteproxy_controller.go applies the same multi-upstream rejection for its ExternalAuthConfigRef using the ConditionTypeMCPRemoteProxyExternalAuthConfigValidated status condition pattern
• VirtualMCPServer controller reconcile logic does NOT enforce a single-upstream restriction on its ExternalAuthConfigRef (the resource is intentionally exempt)
• Tests are added or updated in cmd/thv-operator/controllers/mcpserver_controller_test.go to cover: MCPServer referencing a single-upstream config passes; MCPServer referencing a multi-upstream config fails with ConditionTypeExternalAuthConfigValidated = False
• Tests are added or updated in cmd/thv-operator/controllers/mcpremoteproxy_controller_test.go for the equivalent MCPRemoteProxy cases
• task license-check passes (all new/modified .go files have SPDX headers)
• task lint-fix passes with no remaining lint errors
• task test passes (unit tests)

Technical Approach

Recommended Implementation

Start with pkg/authserver/config.go since it is the most self-contained change: remove the two-line len > 1 guard, then add the multi-upstream explicit-name rule (when len > 1, an empty name or the literal "default" must be rejected with a descriptive error). Remove GetUpstream() from config.go and update the single call site in server_impl.go to use cfg.Upstreams[0] directly. Update config_test.go to match.

Next, add the proxy runner validation. The RunConfig struct in pkg/runner/config.go already embeds *authserver.RunConfig as EmbeddedAuthServerConfig. The natural place for the guard is either a new Validate() method on RunConfig or inline in the runner startup path in pkg/runner/runner.go where the embedded auth server is initialized. Check where EmbeddedAuthServerConfig is first used and add the guard there; a standalone validation function is preferred so it can be independently unit-tested.

Then update the operator. In cmd/thv-operator/api/v1alpha1/mcpexternalauthconfig_types.go, delete lines 786-789 (the len > 1 guard and its comment). In the MCPServer and MCPRemoteProxy controllers, extend the existing handleExternalAuthConfig functions: after fetching the referenced MCPExternalAuthConfig, inspect the UpstreamProviders slice length. If len > 1 and the auth type is embeddedAuthServer, set the ExternalAuthConfigValidated condition to False with a clear reason and message, and return an error to prevent reconciliation from proceeding.

Patterns and Frameworks

• MCPExternalAuthConfig.validateEmbeddedAuthServer() pattern: The existing validation helper in cmd/thv-operator/api/v1alpha1/mcpexternalauthconfig_types.go (lines 772–798) is the reference for how CRD-level Go Validate() methods are structured. After Phase 3, the len > 1 block (lines 786–789) is removed; the pattern for len == 0 check and per-provider loop is retained unchanged.
• handleExternalAuthConfig controller pattern: MCPServerReconciler.handleExternalAuthConfig (lines 1805–1849 in mcpserver_controller.go) and MCPRemoteProxyReconciler.handleExternalAuthConfig (lines 477–522 in mcpremoteproxy_controller.go) already fetch the referenced config, set conditions, and return errors. The multi-upstream check should be inserted after the fetch succeeds, before the hash check, following the same meta.SetStatusCondition + return error pattern.
• ConditionTypeExternalAuthConfigValidated / ConditionTypeMCPRemoteProxyExternalAuthConfigValidated: Use the existing constants and reason constants (e.g., ConditionReasonMCPRemoteProxyExternalAuthConfigValid) from mcpserver_types.go and mcpremoteproxy_types.go. Add new reason constants for the multi-upstream rejection case following the naming pattern in those files.
• Table-driven tests with require.NoError: Follow the pattern in pkg/authserver/config_test.go and cmd/thv-operator/controllers/mcpremoteproxy_controller_test.go for table-driven test organization.
• fmt.Errorf with %w: Use for all error wrapping; errors.Is() for inspection.
• SPDX license headers: All new or modified .go files must have the standard header. Run task license-fix to add them automatically.

Code Pointers

• pkg/authserver/config.go — Primary file: remove lines 392–394 (if len(c.Upstreams) > 1 block) from validateUpstreams(); remove GetUpstream() method (lines 342–350); add explicit-name validation for multi-upstream in the existing for i := range c.Upstreams loop (the upstream name defaulting to "default" when empty must only apply to the single-upstream case)
• pkg/authserver/server_impl.go line 128 — Update cfg.GetUpstream() call to &cfg.Upstreams[0]; confirm no other files call GetUpstream() by searching the repository
• pkg/authserver/config_test.go lines 55–109 — TestConfigValidate: update the "multiple upstreams" case (line 84) from wantErr: true to wantErr: false; update the "duplicate upstream names" case (line 85) to expect the new error message from the updated uniqueness check; add new test cases for multi-upstream with valid explicit names and multi-upstream with a "default" name; lines 111–149 TestConfigGetUpstream: remove the entire test function
• pkg/runner/config.go — Add a validateEmbeddedAuthServerConfig helper function (or add to an existing validate path); reject EmbeddedAuthServerConfig when len(Upstreams) > 1 with an error like "proxy runner does not support multiple upstream providers (found %d); use VirtualMCPServer for multi-upstream deployments"
• pkg/runner/config_test.go — Add table-driven test cases for the new proxy runner embedded auth server validation
• cmd/thv-operator/api/v1alpha1/mcpexternalauthconfig_types.go lines 786–789 — Remove the len > 1 block; update the comment on line 786 to remove the "max items = 1" note; update mcpexternalauthconfig_types_test.go to add a test case for multi-upstream MCPExternalAuthConfig that passes validation
• cmd/thv-operator/controllers/mcpserver_controller.go lines 1820–1848 — In handleExternalAuthConfig, after line 1828 (nil check), add a len(externalAuthConfig.Spec.EmbeddedAuthServer.UpstreamProviders) > 1 check; set ConditionTypeExternalAuthConfigValidated condition to False with a new reason constant; return an error
• cmd/thv-operator/controllers/mcpremoteproxy_controller.go lines 491–522 — Apply the same multi-upstream check in handleExternalAuthConfig after the config is fetched
• cmd/thv-operator/api/v1alpha1/mcpserver_types.go — Add a new condition reason constant for the multi-upstream rejection (following the ConditionReasonExternalAuthConfig* naming pattern)
• cmd/thv-operator/api/v1alpha1/mcpremoteproxy_types.go — Add a new condition reason constant for MCPRemoteProxy multi-upstream rejection
• cmd/thv-operator/controllers/mcpserver_controller_test.go — Add test cases for handleExternalAuthConfig with multi-upstream embedded auth server config
• cmd/thv-operator/controllers/mcpremoteproxy_controller_test.go — Add equivalent test cases

Component Interfaces

// pkg/authserver/config.go — updated validateUpstreams() signature (unchanged)
// Remove: the len > 1 early-return block
// Add: explicit-name enforcement for multi-upstream

func (c *Config) validateUpstreams() error {
    if len(c.Upstreams) == 0 {
        return fmt.Errorf("at least one upstream is required")
    }
    // len > 1 guard removed here

    seenNames := make(map[string]bool)

    for i := range c.Upstreams {
        up := &c.Upstreams[i]

        // For single-upstream, allow empty name to default to "default".
        // For multi-upstream, each upstream must have an explicit, non-"default" name.
        if len(c.Upstreams) == 1 {
            if up.Name == "" {
                up.Name = "default"
            }
        } else {
            // Multi-upstream: explicit names required
            if up.Name == "" || up.Name == "default" {
                return fmt.Errorf(
                    "upstream[%d]: name must be explicitly set and must not be %q in multi-upstream configs",
                    i, "default",
                )
            }
        }

        if seenNames[up.Name] {
            return fmt.Errorf("duplicate upstream name: %q", up.Name)
        }
        seenNames[up.Name] = true

        // ... existing per-upstream type validation unchanged ...
    }
    return nil
}

// GetUpstream() is removed entirely — no replacement in this file.
// Call sites use cfg.Upstreams[0] directly.

// pkg/runner/ — new validation for proxy runner
// Location: validate function or runner startup, called before embedded auth server start.

// validateEmbeddedAuthServerConfig enforces proxy runner constraints on the embedded auth
// server configuration. The proxy runner supports exactly one upstream provider.
// VirtualMCPServer deployments must be used for multi-upstream configurations.
func validateEmbeddedAuthServerConfig(cfg *authserver.RunConfig) error {
    if cfg == nil {
        return nil
    }
    if len(cfg.Upstreams) > 1 {
        return fmt.Errorf(
            "proxy runner does not support multiple upstream providers (found %d); "+
                "use VirtualMCPServer for multi-upstream deployments",
            len(cfg.Upstreams),
        )
    }
    return nil
}

// cmd/thv-operator/api/v1alpha1/mcpserver_types.go — new condition reason constant
// Add alongside existing ConditionReasonExternalAuthConfig* constants:

// ConditionReasonExternalAuthConfigMultiUpstreamNotSupported indicates that the
// referenced MCPExternalAuthConfig has multiple upstream providers, which MCPServer
// does not support. Use VirtualMCPServer for multi-upstream configurations.
ConditionReasonExternalAuthConfigMultiUpstreamNotSupported = "MultiUpstreamNotSupported"

// cmd/thv-operator/api/v1alpha1/mcpremoteproxy_types.go — new condition reason constant

// ConditionReasonMCPRemoteProxyExternalAuthConfigMultiUpstreamNotSupported indicates
// that the referenced MCPExternalAuthConfig has multiple upstream providers, which
// MCPRemoteProxy does not support. Use VirtualMCPServer for multi-upstream configurations.
ConditionReasonMCPRemoteProxyExternalAuthConfigMultiUpstreamNotSupported = "MultiUpstreamNotSupported"

// cmd/thv-operator/controllers/mcpserver_controller.go — handleExternalAuthConfig
// Insert after fetching the config and the nil check, before the hash check:

embeddedCfg := externalAuthConfig.Spec.EmbeddedAuthServer
if embeddedCfg != nil && len(embeddedCfg.UpstreamProviders) > 1 {
    meta.SetStatusCondition(&m.Status.Conditions, metav1.Condition{
        Type:   mcpv1alpha1.ConditionTypeExternalAuthConfigValidated,
        Status: metav1.ConditionFalse,
        Reason: mcpv1alpha1.ConditionReasonExternalAuthConfigMultiUpstreamNotSupported,
        Message: fmt.Sprintf(
            "MCPExternalAuthConfig %q has %d upstream providers; MCPServer supports only one. "+
                "Use VirtualMCPServer for multi-upstream deployments.",
            externalAuthConfig.Name, len(embeddedCfg.UpstreamProviders),
        ),
        ObservedGeneration: m.Generation,
    })
    return fmt.Errorf(
        "MCPExternalAuthConfig %q has %d upstream providers; MCPServer supports only one",
        externalAuthConfig.Name, len(embeddedCfg.UpstreamProviders),
    )
}

Testing Strategy

Unit Tests — pkg/authserver/config_test.go

• Update "multiple upstreams" test case in TestConfigValidate to wantErr: false (two upstreams with explicit non-"default" names should now pass)
• Update "duplicate upstream names" test case to use a two-upstream config with non-"default" names that collide; verify the duplicate-name error message is correct
• Add "multi-upstream with empty name" test case: two upstreams where one Name is empty; expect error containing "must be explicitly set"
• Add "multi-upstream with default name" test case: two upstreams where one Name is "default"; expect error containing "must not be"
• Add "multi-upstream valid explicit names" test case: two upstreams with distinct non-"default" names; expect no error
• Remove TestConfigGetUpstream test function entirely

Unit Tests — pkg/runner/config_test.go

• "embedded auth server config nil": nil EmbeddedAuthServerConfig returns no error
• "embedded auth server config single upstream": config with 1 upstream returns no error
• "embedded auth server config two upstreams": config with 2 upstreams returns error containing "proxy runner does not support multiple upstream providers"

Unit Tests — cmd/thv-operator/api/v1alpha1/mcpexternalauthconfig_types_test.go

• Update or remove the existing test case that expects validateEmbeddedAuthServer to reject len > 1 upstreams; replace with a test case that expects multi-upstream MCPExternalAuthConfig to pass Validate() successfully (two providers with valid configs)

Unit Tests — cmd/thv-operator/controllers/mcpserver_controller_test.go

• "handleExternalAuthConfig single upstream passes": MCPExternalAuthConfig with 1 upstream provider; expect no error, condition set to True
• "handleExternalAuthConfig multi upstream rejected": MCPExternalAuthConfig with 2 upstream providers; expect error; condition set to False with reason MultiUpstreamNotSupported

Unit Tests — cmd/thv-operator/controllers/mcpremoteproxy_controller_test.go

• Same two test cases as above for MCPRemoteProxy.handleExternalAuthConfig

Edge Cases

• MCPExternalAuthConfig with embeddedAuthServer: nil (different auth type) must not be rejected when MCPServer or MCPRemoteProxy references it — the multi-upstream guard only applies when embeddedAuthServer is non-nil
• Single-upstream config with empty Name field continues to default to "default" (backward compatibility for existing single-upstream deployments)
• validateUpstreams() called on a config with exactly 2 upstreams: both with distinct non-"default" names passes; one with "default" name fails

Out of Scope

• Handler layer changes for multi-upstream chain logic (sequential authorize/callback, nextMissingUpstream): covered in TASK-002
• Identity.UpstreamTokens enrichment and upstreamswap middleware refactor: covered in TASK-004
• Multi-upstream support in the CLI proxy runner or MCPServer/MCPRemoteProxy K8s workload types — these workload types intentionally remain single-upstream
• VirtualMCPServer validation changes — it is explicitly exempt from the single-upstream restriction
• cmd/thv-operator/pkg/controllerutil/authserver.go multi-upstream support (line 435 currently takes only UpstreamProviders[0]): this is a consumer-layer wiring concern that may be needed for Phase 2 or a follow-on task, but is not required to complete the Phase 3 validation boundary changes
• pkg/authserver/server_impl.go multi-upstream handler wiring (building the upstreams map and upstreamOrder slice): covered in TASK-002
• CRD kubebuilder marker changes (+kubebuilder:validation:MaxItems=1) on MCPExternalAuthConfig.EmbeddedAuthServerConfig.upstreamProviders — assess whether the admission webhook marker also needs updating; if it blocks multi-upstream at API admission time, it must be relaxed; coordinate with the operator CRD regeneration (task operator-generate, task operator-manifests, task crdref-gen)

References

• RFC: docs/proposals/THV-0052-multi-upstream-idp-authserver.md
• Architecture doc: docs/arch/11-auth-server-storage.md
• Parent epic: Auth Server: multi-upstream provider support #3924
• Phase 1 (dependency): Implement multi-provider upstream token storage layer (RFC-0052 Phase 1) #4136
• Related Stacklok epic: https://github.com/stacklok/stacklok-epics/issues/251