feat: introduce proxy for webhook requests#2671
Merged
wilsonrivera merged 9 commits intomainfrom Mar 30, 2026
Merged
Conversation
wilsonrivera
requested review from
a team,
JivusAyrus,
Noroth,
StarpTech,
pepol and
thisisnithin
as code owners
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds optional HTTPS proxy support for outgoing webhooks: new dependency, env var and Helm values, config/routing wiring, webhook services accept a proxy URL (create HttpsProxyAgent), and many handlers now forward the proxy URL into OrganizationWebhookService. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2671 +/- ##
===========================================
- Coverage 63.11% 46.92% -16.19%
===========================================
Files 249 1052 +803
Lines 26643 143322 +116679
Branches 0 9612 +9612
===========================================
+ Hits 16816 67257 +50441
- Misses 8451 74319 +65868
- Partials 1376 1746 +370
🚀 New features to boost your workflow:
|
Router-nonroot image scan passed✅ No security vulnerabilities found in image: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
controlplane/src/core/bufservices/federated-graph/createFederatedGraph.ts (1)
38-44: Consider centralizing webhook service construction to reduce callsite drift.This change is correct. Since the same constructor arguments are repeated in many handlers, a small factory/helper in one place would make future signature changes safer.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controlplane/src/core/bufservices/federated-graph/createFederatedGraph.ts` around lines 38 - 44, Multiple callsites construct OrganizationWebhookService with the same argument list (opts.db, authContext.organizationId, opts.logger, opts.billingDefaultPlanId, opts.webhookProxyUrl), which will drift if the constructor changes; factor this into a single factory function (e.g., makeOrganizationWebhookService or OrganizationWebhookServiceFactory) that accepts the minimal inputs (opts and authContext) and returns new OrganizationWebhookService(...). Replace direct new OrganizationWebhookService(...) usages (including the one in createFederatedGraph and other handlers) with calls to the factory so future constructor signature changes only need one update.helm/cosmo/charts/controlplane/values.yaml (1)
177-179: Consider adding a documentation comment for consistency.Other configuration fields in this file have descriptive comments (e.g., lines 164, 188-189, 200). Adding a comment for
webhookProxyUrlwould improve clarity for operators deploying this chart.📝 Suggested documentation comment
webhookUrl: '' webhookSecret: '' + # -- The proxy URL to use for outgoing webhook requests (optional) webhookProxyUrl: ''🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@helm/cosmo/charts/controlplane/values.yaml` around lines 177 - 179, Add a descriptive documentation comment above the webhookProxyUrl entry in values.yaml (next to existing comments for webhookUrl and webhookSecret) that explains its purpose (e.g., an optional HTTP(S) proxy URL used when sending webhook requests), expected format (scheme://host:port), and that it defaults to empty when not used; update the comment to match the style and tone of surrounding comments so operators understand when and how to set webhookProxyUrl.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@helm/cosmo/charts/controlplane/templates/deployment.yaml`:
- Around line 191-195: The WEBHOOK_PROXY_URL environment variable is
unconditionally referenced which can cause missing-secret or empty-string
issues; wrap the WEBHOOK_PROXY_URL block in the same conditional pattern used
for SLACK_APP_CLIENT_SECRET/S3_STORAGE_URL by guarding it with {{- if
.Values.configuration.webhookProxyUrl }} so the secretKeyRef (name: {{ include
"controlplane.secretName" . }}, key: webhookProxyUrl) is only added when
.Values.configuration.webhookProxyUrl is set; remove the block when the value is
not provided to avoid pod startup and Zod .url() validation failures.
---
Nitpick comments:
In `@controlplane/src/core/bufservices/federated-graph/createFederatedGraph.ts`:
- Around line 38-44: Multiple callsites construct OrganizationWebhookService
with the same argument list (opts.db, authContext.organizationId, opts.logger,
opts.billingDefaultPlanId, opts.webhookProxyUrl), which will drift if the
constructor changes; factor this into a single factory function (e.g.,
makeOrganizationWebhookService or OrganizationWebhookServiceFactory) that
accepts the minimal inputs (opts and authContext) and returns new
OrganizationWebhookService(...). Replace direct new
OrganizationWebhookService(...) usages (including the one in
createFederatedGraph and other handlers) with calls to the factory so future
constructor signature changes only need one update.
In `@helm/cosmo/charts/controlplane/values.yaml`:
- Around line 177-179: Add a descriptive documentation comment above the
webhookProxyUrl entry in values.yaml (next to existing comments for webhookUrl
and webhookSecret) that explains its purpose (e.g., an optional HTTP(S) proxy
URL used when sending webhook requests), expected format (scheme://host:port),
and that it defaults to empty when not used; update the comment to match the
style and tone of surrounding comments so operators understand when and how to
set webhookProxyUrl.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e9f6b498-7efc-467c-beab-9c5ffef8645d
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (30)
controlplane/package.jsoncontrolplane/src/core/bufservices/contract/updateContract.tscontrolplane/src/core/bufservices/feature-flag/createFeatureFlag.tscontrolplane/src/core/bufservices/feature-flag/deleteFeatureFlag.tscontrolplane/src/core/bufservices/feature-flag/enableFeatureFlag.tscontrolplane/src/core/bufservices/feature-flag/updateFeatureFlag.tscontrolplane/src/core/bufservices/federated-graph/createFederatedGraph.tscontrolplane/src/core/bufservices/federated-graph/migrateFromApollo.tscontrolplane/src/core/bufservices/federated-graph/moveFederatedGraph.tscontrolplane/src/core/bufservices/federated-graph/updateFederatedGraph.tscontrolplane/src/core/bufservices/graph/recomposeGraph.tscontrolplane/src/core/bufservices/monograph/publishMonograph.tscontrolplane/src/core/bufservices/monograph/updateMonograph.tscontrolplane/src/core/bufservices/proposal/createProposal.tscontrolplane/src/core/bufservices/proposal/updateProposal.tscontrolplane/src/core/bufservices/subgraph/checkSubgraphSchema.tscontrolplane/src/core/bufservices/subgraph/deleteFederatedSubgraph.tscontrolplane/src/core/bufservices/subgraph/moveSubgraph.tscontrolplane/src/core/bufservices/subgraph/publishFederatedSubgraph.tscontrolplane/src/core/bufservices/subgraph/updateSubgraph.tscontrolplane/src/core/build-server.tscontrolplane/src/core/env.schema.tscontrolplane/src/core/routes.tscontrolplane/src/core/webhooks/OrganizationWebhookService.tscontrolplane/src/core/webhooks/PlatformWebhookService.tscontrolplane/src/core/webhooks/utils.tscontrolplane/src/index.tshelm/cosmo/charts/controlplane/templates/deployment.yamlhelm/cosmo/charts/controlplane/templates/secret.yamlhelm/cosmo/charts/controlplane/values.yaml
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@helm/cosmo/charts/controlplane/README.md`:
- Line 44: Update the README description for configuration.prometheus.host to
correct the spelling error: replace "defautls" with "defaults" in the
user-facing sentence so the line reads that the host "defaults to 127.0.0.1 to
avoid opening the metrics endpoint by default" (locate the text under
configuration.prometheus.host in the README).
- Line 55: Update the README table cell for configuration.s3ForcePathStyle to
use the standard technical casing "path-style URLs" (hyphenated and with "URLs"
uppercase). Locate the table row referencing configuration.s3ForcePathStyle in
helm/cosmo/charts/controlplane/README.md and replace the phrase "path style
urls" (or similar) with "path-style URLs" while preserving the rest of the
description and formatting.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 75e8ab6b-33d0-4e43-973f-4626e21fe12f
📒 Files selected for processing (2)
helm/cosmo/charts/controlplane/README.mdhelm/cosmo/charts/controlplane/templates/deployment.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- helm/cosmo/charts/controlplane/templates/deployment.yaml
pepol
reviewed
Mar 19, 2026
pepol
approved these changes
Mar 19, 2026
…he-proxy-when-sending-webhooks
…he-proxy-when-sending-webhooks
…he-proxy-when-sending-webhooks
JivusAyrus
approved these changes
Mar 27, 2026
…he-proxy-when-sending-webhooks
…he-proxy-when-sending-webhooks
…he-proxy-when-sending-webhooks
wilsonrivera
deleted the
wilson/eng-9247-update-controlplane-to-use-the-proxy-when-sending-webhooks
branch
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Chores
Checklist
Open Source AI Manifesto
This project follows the principles of the Open Source AI Manifesto. Please ensure your contribution aligns with its principles.