Skip to content

Webhook Middleware Phase 4: CLI configuration support#4410

Draft
Sanskarzz wants to merge 3 commits intostacklok:mainfrom
Sanskarzz:dynamicwebhook4
Draft

Webhook Middleware Phase 4: CLI configuration support#4410
Sanskarzz wants to merge 3 commits intostacklok:mainfrom
Sanskarzz:dynamicwebhook4

Conversation

@Sanskarzz
Copy link
Copy Markdown
Contributor

[WIP] Depends on the merge of Phase 3 PR

Summary

This PR implements Phase 4 of the Dynamic Webhook Middleware RFC (THV-0017), introducing CLI configuration support for passing validating and mutating webhooks to thv run.

Users can now define their webhook pipelines in YAML or JSON files and inject them into the ToolHive runner at startup using the --webhook-config flag.

Fixes #3400

Key Changes

pkg/webhook/config.go

  • Parsing: Introduced the FileConfig struct to unmarshal both YAML and JSON webhook configuration files (auto-detected via file extension).
  • Merging: Implemented MergeConfigs to support combining multiple configuration files. Files passed later in the arguments take precedence for webhooks with the same name (last-writer-wins deduplication).
  • Validation: Implemented ValidateConfig to aggregate wh.Validate() errors across all provided mutating and validating webhook definitions to fail fast at CLI startup.

cmd/thv/app/run_flags.go

  • Added WebhookConfigs []string property to RunFlags.
  • Registered the --webhook-config flag which can be specified multiple times.
  • Added the loadAndMergeWebhookConfigs helper to parse and process the files into a single webhook.FileConfig.

pkg/runner/config_builder.go

  • Added WithValidatingWebhooks and WithMutatingWebhooks builder functions to inject parsed webhooks into RunConfig.
  • Fixed a gap in CLI runner initialization by updating WithMiddlewareFromFlags to actively instantiate and append addMutatingWebhookMiddleware and addValidatingWebhookMiddleware directly to the thv runner chain (matching the existing Operator behavior).

Testing

  • Unit Tests: Full coverage for file loading (yaml and json), parsing errors, file aggregation, deduplication sorting, and granular validation constraints inside pkg/webhook/config_test.go.
  • Linting & Coverage Checks: Clean task lint and passing task test-coverage requirements.
  • Manual Verification: Successfully mapped a valid local configuration file configuring using thv CLI binary ./thv run fetch --webhook-config webhook-config/webhooks.yaml --debug targets, confirming auth -> mcp-parser -> mutating-webhook -> validating-webhook proxy application inside .thv logs.

webhook-config/webhooks.yaml

validating:
  - name: test-validator
    url: https://example.com/validate
    timeout: 5s
    failure_policy: ignore
    tls_config:
      insecure_skip_verify: true
mutating:
  - name: test-mutator
    url: https://example.com/mutate
    timeout: 3s
    failure_policy: ignore
    tls_config:
      insecure_skip_verify: true
image

Type of change

  • Bug fix
  • New feature
  • Refactoring (no behavior change)
  • Dependency update
  • Documentation
  • Other (describe):

Test plan

  • Unit tests (task test)
  • E2E tests (task test-e2e)
  • Linting (task lint-fix)
  • Manual testing (describe below)
Signed-off-by: Sanskarzz <sanskar.gur@gmail.com>
Signed-off-by: Sanskarzz <sanskar.gur@gmail.com>
Signed-off-by: Sanskarzz <sanskar.gur@gmail.com>
@github-actions github-actions bot added the size/XL Extra large PR: 1000+ lines changed label Mar 27, 2026
Copy link
Copy Markdown
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Large PR Detected

This PR exceeds 1000 lines of changes and requires justification before it can be reviewed.

How to unblock this PR:

Add a section to your PR description with the following format:

## Large PR Justification

[Explain why this PR must be large, such as:]
- Generated code that cannot be split
- Large refactoring that must be atomic
- Multiple related changes that would break if separated
- Migration or data transformation

Alternative:

Consider splitting this PR into smaller, focused changes (< 1000 lines each) for easier review and reduced risk.

See our Contributing Guidelines for more details.


This review will be automatically dismissed once you add the justification section.

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 27, 2026

Codecov Report

❌ Patch coverage is 82.60870% with 48 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.58%. Comparing base (887b39c) to head (5c7949c).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
pkg/webhook/mutating/middleware.go 82.87% 19 Missing and 6 partials ⚠️
pkg/runner/config_builder.go 20.00% 10 Missing and 2 partials ⚠️
pkg/runner/middleware.go 76.47% 2 Missing and 2 partials ⚠️
pkg/webhook/mutating/patch.go 85.71% 2 Missing and 2 partials ⚠️
pkg/webhook/config.go 93.47% 1 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4410      +/-   ##
==========================================
+ Coverage   69.53%   69.58%   +0.04%     
==========================================
  Files         485      490       +5     
  Lines       49879    50212     +333     
==========================================
+ Hits        34684    34940     +256     
- Misses      12516    12575      +59     
- Partials     2679     2697      +18     

☔ 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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/XL Extra large PR: 1000+ lines changed

1 participant