Skip to content

Implement upstream_inject strategy and SubjectProviderName#4390

Merged
jhrozek merged 3 commits intomainfrom
vmcp-add-as-scaffolding-6a
Mar 28, 2026
Merged

Implement upstream_inject strategy and SubjectProviderName#4390
jhrozek merged 3 commits intomainfrom
vmcp-add-as-scaffolding-6a

Conversation

@jhrozek
Copy link
Copy Markdown
Contributor

@jhrozek jhrozek commented Mar 26, 2026

Summary

  • Phase 2 of the embedded auth server work (RFC-0054): backends configured with
    upstream_inject need to receive per-provider upstream IDP tokens. This adds
    the runtime strategy and extends token_exchange to source its subject token
    from upstream tokens when SubjectProviderName is configured.
  • Adds UpstreamInjectStrategy (stateless, follows the header_injection pattern)
    that reads from Identity.UpstreamTokens and sets the Authorization header.
  • Extends TokenExchangeStrategy to resolve the subject token from upstream tokens
    when SubjectProviderName is set, falling back to Identity.Token.
  • Fixes a bug in handleSpecValidationError that swallowed the status-apply error
    and proceeded to ensureDeployment even when the ConfigMap was not created.

Fixes #4145

Type of change

  • New feature

Test plan

  • Unit tests (task test)
  • Linting (task lint-fix)

Changes

File Change
pkg/vmcp/auth/strategies/upstream_inject.go New UpstreamInjectStrategy — looks up provider token from identity, sets Bearer header
pkg/vmcp/auth/strategies/upstream_inject_test.go Comprehensive tests: valid injection, missing identity, nil map, health check bypass, validation
pkg/vmcp/auth/strategies/tokenexchange.go Resolve subject token from UpstreamTokens[SubjectProviderName] when configured
pkg/vmcp/auth/strategies/tokenexchange_test.go Tests for upstream token selection and ErrUpstreamTokenNotFound sentinel
pkg/vmcp/auth/types/types.go Add ErrUpstreamTokenNotFound, StrategyTypeUpstreamInject, SubjectProviderName field
pkg/vmcp/auth/factory/outgoing.go Register upstream_inject in factory
pkg/vmcp/auth/factory/outgoing_test.go Add strategy to registry assertions
cmd/thv-operator/controllers/virtualmcpserver_controller.go Fix handleSpecValidationError to propagate status-apply errors and stop reconciliation

Special notes for reviewers

🤖 Generated with Claude Code

@jhrozek jhrozek force-pushed the vmcp-add-as-scaffolding-5 branch from 8050a55 to 0810bda Compare March 26, 2026 15:55
@jhrozek jhrozek force-pushed the vmcp-add-as-scaffolding-6a branch from 76cd569 to fc12658 Compare March 26, 2026 15:55
@github-actions github-actions bot added the size/M Medium PR: 300-599 lines changed label Mar 26, 2026
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 26, 2026

Codecov Report

❌ Patch coverage is 84.93151% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.49%. Comparing base (7cff3cd) to head (99c8e33).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...perator/controllers/virtualmcpserver_controller.go 67.85% 7 Missing and 2 partials ⚠️
pkg/vmcp/auth/factory/outgoing.go 60.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4390      +/-   ##
==========================================
+ Coverage   69.47%   69.49%   +0.02%     
==========================================
  Files         485      486       +1     
  Lines       49969    50017      +48     
==========================================
+ Hits        34715    34760      +45     
- Misses      12570    12574       +4     
+ Partials     2684     2683       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jhrozek jhrozek requested a review from tgrunnagle March 26, 2026 16:01
jerm-dro
jerm-dro previously approved these changes Mar 26, 2026
@jhrozek jhrozek force-pushed the vmcp-add-as-scaffolding-6a branch from fc12658 to 9f25687 Compare March 26, 2026 22:33
@github-actions github-actions bot added size/M Medium PR: 300-599 lines changed and removed size/M Medium PR: 300-599 lines changed labels Mar 26, 2026
@jhrozek jhrozek force-pushed the vmcp-add-as-scaffolding-5 branch from d1d62a3 to 21a42ec Compare March 27, 2026 10:08
@jhrozek jhrozek force-pushed the vmcp-add-as-scaffolding-6a branch from 9f25687 to c904405 Compare March 27, 2026 10:21
@github-actions github-actions bot added size/M Medium PR: 300-599 lines changed size/L Large PR: 600-999 lines changed and removed size/M Medium PR: 300-599 lines changed labels Mar 27, 2026
tgrunnagle
tgrunnagle previously approved these changes Mar 27, 2026
@jhrozek jhrozek force-pushed the vmcp-add-as-scaffolding-5 branch from 21a42ec to 62337a2 Compare March 27, 2026 15:36
@jhrozek jhrozek force-pushed the vmcp-add-as-scaffolding-6a branch from 8156719 to dacf7fb Compare March 27, 2026 15:36
@github-actions github-actions bot added size/L Large PR: 600-999 lines changed and removed size/L Large PR: 600-999 lines changed labels Mar 27, 2026
Base automatically changed from vmcp-add-as-scaffolding-5 to main March 27, 2026 19:49
@jhrozek jhrozek dismissed stale reviews from tgrunnagle and jerm-dro March 27, 2026 19:49

The base branch was changed.

jhrozek and others added 3 commits March 27, 2026 19:55
Phase 2 of RFC-0054: add the upstream_inject outgoing auth strategy
that injects per-provider upstream IDP tokens into backend requests,
and extend token_exchange to source its subject token from
identity.UpstreamTokens when SubjectProviderName is configured.

- Add UpstreamTokens stub field to Identity with MarshalJSON redaction
- Add UpstreamInjectStrategy (stateless, follows header_injection pattern)
- Extend TokenExchangeStrategy to resolve subject token from upstream
  tokens when SubjectProviderName is set, falling back to identity.Token
- Register upstream_inject in the outgoing auth factory

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The anonymous function returning (string, error) obscured control flow
without benefit — error returns exit the closure, not the method. Plain
if/else is idiomatic and matches the rest of the codebase.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@jhrozek jhrozek force-pushed the vmcp-add-as-scaffolding-6a branch from dacf7fb to 99c8e33 Compare March 27, 2026 19:56
@github-actions github-actions bot added size/L Large PR: 600-999 lines changed and removed size/L Large PR: 600-999 lines changed labels Mar 27, 2026
@jhrozek jhrozek merged commit e3c605f into main Mar 28, 2026
96 of 109 checks passed
@jhrozek jhrozek deleted the vmcp-add-as-scaffolding-6a branch March 28, 2026 07:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/L Large PR: 600-999 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Phase 2: Implement upstream_inject strategy and SubjectProviderName for token_exchange (RFC-0054)

3 participants