Skip to content

Conversation

@jhrozek
Copy link
Contributor

@jhrozek jhrozek commented Feb 11, 2026

Register the AWS STS middleware in the runner so it can be activated via RunConfig.

Add AWSStsConfig to RunConfig, a WithAWSStsConfig builder option, and factory registration in GetSupportedMiddlewareFactories. Place the middleware in PopulateMiddlewareConfigs after audit/authz but before header forwarding — only authorized requests trigger credential exchange and SigV4 signing happens as late as possible.

Regenerate Swagger docs to include the new awssts.Config and awssts.RoleMapping schemas.

Consolidate the three GetSupportedMiddlewareFactories tests into one table-driven test and extract the duplicated createMinimalAuthServerConfig helper to package level.

Related: #3569

@github-actions github-actions bot added the size/M Medium PR: 300-599 lines changed label Feb 11, 2026
@codecov
Copy link

codecov bot commented Feb 11, 2026

Codecov Report

❌ Patch coverage is 70.00000% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 66.70%. Comparing base (c8f2254) to head (d1414c0).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
pkg/runner/config_builder.go 0.00% 4 Missing ⚠️
pkg/runner/middleware.go 87.50% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3770      +/-   ##
==========================================
- Coverage   66.78%   66.70%   -0.08%     
==========================================
  Files         437      437              
  Lines       43002    43025      +23     
==========================================
- Hits        28718    28701      -17     
- Misses      12078    12118      +40     
  Partials     2206     2206              

☔ 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 force-pushed the aws_sts-pr-5b-runner-wiring branch from 7e7727d to 1764a75 Compare February 11, 2026 12:38
@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 Feb 11, 2026
UpstreamSwapConfig *upstreamswap.Config `json:"upstream_swap_config,omitempty" yaml:"upstream_swap_config,omitempty"`

// AWSStsConfig contains AWS STS token exchange configuration for accessing AWS services
AWSStsConfig *awssts.Config `json:"aws_sts_config,omitempty" yaml:"aws_sts_config,omitempty"`
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this meant to be middleware configuration? should it go with other middleware? cc @blkt

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fwiw I modeled this after the token exchange and header injection code. If there's other pattern I should be using, I'll be happy to do that.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know how valuable is it to enforce that distinction anymore. The main problem is that RunConfig represents both the MCP server's config and the Proxy's config, and doubts regarding where to put any bit of configuration stem out of that.

I think it's sensible to have each configuration parameter as a top level field of a struct, it's much easier to know where to find things, and have two separate structs for MCP server and Proxy configurations.
Finally, I would leave to the code under pkg/runner/runner.go to determine the correct order of middlewares.

This is easier said than done because this requires a data migration. This is one of the reasons why I believe having run configs on database would be a boon, because data migrations are an established practice.

I would not bang the head to hard on this and leave it where it's easy to understand. Having middleware configurations in a list rather than in top level fields gives little advantage.

@jhrozek jhrozek force-pushed the aws_sts-pr-5b-runner-wiring branch from 1764a75 to ac1c550 Compare February 11, 2026 23:03
@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 Feb 11, 2026
Register the AWS STS middleware in the runner so it can be activated via
RunConfig.

Add AWSStsConfig to RunConfig, a WithAWSStsConfig builder option, and
factory registration in GetSupportedMiddlewareFactories. Place the
middleware in PopulateMiddlewareConfigs after audit/authz but before
header forwarding — only authorized requests trigger credential exchange
and SigV4 signing happens as late as possible.

Regenerate Swagger docs to include the new awssts.Config and
awssts.RoleMapping schemas.

Consolidate the three GetSupportedMiddlewareFactories tests into one
table-driven test and extract the duplicated createMinimalAuthServerConfig
helper to package level.

Related: #3569
@jhrozek jhrozek force-pushed the aws_sts-pr-5b-runner-wiring branch from ac1c550 to d1414c0 Compare February 11, 2026 23:03
@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 Feb 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/M Medium PR: 300-599 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants