Design: umbrella operator integration for mcp-gateway#1198
Design: umbrella operator integration for mcp-gateway#1198Patryk-Stefanski wants to merge 4 commits into
Conversation
Covers subchart readiness, multi-tenancy analysis confirming Authorino and Limitador provide strong tenant isolation with shared singletons, migration path from standalone OLM, and implementation tasks for chart versioning, publishing, and dependency removal. Ref: Kuadrant/architecture#179
- Clarify standalone OLM bundle is maintained alongside subchart - Add CRD lifecycle section covering Helm, OLM, and conflict scenarios - Make migration steps specific: list exact resources for ownerReference stripping, document control plane gap during migration window - Add rollback section to upgrade ordering - Replace fragile line-number references with function names and version annotations for external repo code - Fix tasks: delete dependencies.yaml (not empty it), reference existing helm-release.yaml workflow, note umbrella-operator external dependency with no ETA, add guide index update to migration task
This reverts commit be641d2.
📝 WalkthroughWalkthroughAdds design documentation and a task plan for integrating mcp-gateway into the umbrella operator as an optional Helm subchart, covering deployment model, migration flow, OLM ownership, and implementation sequencing. ChangesUmbrella operator integration
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~4 minutes Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
coderabbitai
Bot
added
the
review-effort/small
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
docs/design/umbrella-operator-integration/tasks/tasks.md (1)
98-104: 🗄️ Data Integrity & Integration | 🔵 TrivialResolve the CRD installation mechanism.
The criterion currently presents an "either/or" choice for CRD loading (OLM vs. Helm
crds/). While the umbrella chart is correctly identified as the owner, the implementation plan must specify a single mechanism to ensure deterministic build and verification flows. Replace the "either/or" phrasing with the chosen implementation strategy (e.g., Helmcrds/directory).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/design/umbrella-operator-integration/tasks/tasks.md` around lines 98 - 104, Update the acceptance criteria in tasks.md to remove the CRD installation “either/or” ambiguity and specify one deterministic mechanism for the umbrella chart, such as installing CRDs via the Helm crds/ directory; keep the umbrella chart ownership clear and align the wording with the chosen strategy so the plan and verification path are unambiguous.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/design/umbrella-operator-integration/tasks/tasks.md`:
- Around line 134-141: The dependency diagram block in the tasks document is an
unlabeled fenced block, which triggers markdownlint. Update the fence around the
Task 1–Task 6 dependency diagram to include an appropriate language tag, or
convert it to an indented code block, so the markdown stays lint-clean.
- Around line 39-44: The verification step is too broad because `make check` can
succeed even when bundle manifests are out of sync; update the task’s
verification instructions to call the exact bundle sync target required by the
acceptance criteria, alongside the existing `test ! -f
bundle/metadata/dependencies.yaml && echo "deleted"` and `make bundle` checks.
Keep the fix in the `Verification` section so it clearly points to the bundle
sync target rather than the generic `make check` command.
- Around line 19-27: The verification step in tasks.md is too weak because it
only checks that version fields exist, not that they match the release tag or
avoid "latest". Update the verification guidance around the Chart.yaml checks to
assert that Chart.yaml’s version and appVersion both equal the intended
mcp-gateway release version, using the release/version-related acceptance
criteria as the source of truth. Refer to the version/appVersion checks in the
acceptance criteria and the verification block so the instructions clearly
require exact matching rather than presence-only grepping.
- Around line 53-64: The verification block currently checks repository access
and templating but does not validate the signed chart artifact, so update the
verification steps in the tasks entry to include provenance verification for the
mcp-gateway chart. Keep the existing helm repo/chart checks, and add a step
using Helm’s provenance verification against kuadrant/mcp-gateway so the
.tgz.prov requirement is explicitly exercised alongside the chart install and
template checks.
In
`@docs/design/umbrella-operator-integration/umbrella-operator-integration-design.md`:
- Around line 84-100: Label the fenced architecture block in the design doc by
adding a language hint to the plain-text fence so markdownlint MD040 no longer
fires; update the fence around the Umbrella Operator / mcp-gateway-controller
diagram to use an explicit text label, and keep the surrounding content
unchanged.
- Around line 31-52: Add the missing job stories in the Job Stories section to
cover MCP client users, MCP Developers, non-interactive agents, and at least one
edge case, using the required “When <situation>, <actor> wants <action/outcome>
so that <benefit>” format. Update the existing stories in the
umbrella-operator-integration-design document so the set reflects all required
personas and includes the missing scenarios, keeping the wording consistent with
the current style.
---
Nitpick comments:
In `@docs/design/umbrella-operator-integration/tasks/tasks.md`:
- Around line 98-104: Update the acceptance criteria in tasks.md to remove the
CRD installation “either/or” ambiguity and specify one deterministic mechanism
for the umbrella chart, such as installing CRDs via the Helm crds/ directory;
keep the umbrella chart ownership clear and align the wording with the chosen
strategy so the plan and verification path are unambiguous.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 49f3b5e4-aa27-44d9-b110-8e5144fca841
📒 Files selected for processing (2)
docs/design/umbrella-operator-integration/tasks/tasks.mddocs/design/umbrella-operator-integration/umbrella-operator-integration-design.md
| **Acceptance criteria:** | ||
| - [ ] `version` field tracks the mcp-gateway release version (e.g., `0.7.1`) | ||
| - [ ] `appVersion` field matches the release version (not `"latest"`) | ||
| - [ ] `make generate-all` and release automation update these fields | ||
|
|
||
| **Verification:** | ||
| ```bash | ||
| grep -E 'version:|appVersion:' charts/mcp-gateway/Chart.yaml | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Tighten the version check.
This grep only proves the keys exist; it won't catch version drifting from the release tag or appVersion staying on "latest".
Suggested tweak
- grep -E 'version:|appVersion:' charts/mcp-gateway/Chart.yaml
+ grep -Fx "version: ${CHART_VERSION}" charts/mcp-gateway/Chart.yaml
+ grep -Fx "appVersion: \"${APP_VERSION}\"" charts/mcp-gateway/Chart.yaml📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| **Acceptance criteria:** | |
| - [ ] `version` field tracks the mcp-gateway release version (e.g., `0.7.1`) | |
| - [ ] `appVersion` field matches the release version (not `"latest"`) | |
| - [ ] `make generate-all` and release automation update these fields | |
| **Verification:** | |
| ```bash | |
| grep -E 'version:|appVersion:' charts/mcp-gateway/Chart.yaml | |
| ``` | |
| **Acceptance criteria:** | |
| - [ ] `version` field tracks the mcp-gateway release version (e.g., `0.7.1`) | |
| - [ ] `appVersion` field matches the release version (not `"latest"`) | |
| - [ ] `make generate-all` and release automation update these fields | |
| **Verification:** |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/design/umbrella-operator-integration/tasks/tasks.md` around lines 19 -
27, The verification step in tasks.md is too weak because it only checks that
version fields exist, not that they match the release tag or avoid "latest".
Update the verification guidance around the Chart.yaml checks to assert that
Chart.yaml’s version and appVersion both equal the intended mcp-gateway release
version, using the release/version-related acceptance criteria as the source of
truth. Refer to the version/appVersion checks in the acceptance criteria and the
verification block so the instructions clearly require exact matching rather
than presence-only grepping.
| **Verification:** | ||
| ```bash | ||
| test ! -f bundle/metadata/dependencies.yaml && echo "deleted" | ||
| make bundle | ||
| make check | ||
| ``` |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Call the bundle sync target directly.
make check is broader and can pass while bundle manifests still drift. This task should verify the exact target named in the acceptance criteria.
Suggested tweak
- make bundle
- make check
+ make bundle
+ make check-bundle-crd-sync
+ make check📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| **Verification:** | |
| ```bash | |
| test ! -f bundle/metadata/dependencies.yaml && echo "deleted" | |
| make bundle | |
| make check | |
| ``` | |
| **Verification:** |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/design/umbrella-operator-integration/tasks/tasks.md` around lines 39 -
44, The verification step is too broad because `make check` can succeed even
when bundle manifests are out of sync; update the task’s verification
instructions to call the exact bundle sync target required by the acceptance
criteria, alongside the existing `test ! -f bundle/metadata/dependencies.yaml &&
echo "deleted"` and `make bundle` checks. Keep the fix in the `Verification`
section so it clearly points to the bundle sync target rather than the generic
`make check` command.
| **Acceptance criteria:** | ||
| - [ ] mcp-gateway chart `.tgz` is published to `https://kuadrant.io/helm-charts/` | ||
| - [ ] `index.yaml` in the helm-charts repo includes the mcp-gateway entry | ||
| - [ ] Chart is installable via `helm install mcp-gateway kuadrant/mcp-gateway` | ||
| - [ ] Chart is signed (`.tgz.prov` file, matching other Kuadrant charts) | ||
|
|
||
| **Verification:** | ||
| ```bash | ||
| helm repo add kuadrant https://kuadrant.io/helm-charts/ | ||
| helm search repo kuadrant/mcp-gateway | ||
| helm template mcp-gateway kuadrant/mcp-gateway --set controller.enabled=true | ||
| ``` |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor
🧩 Analysis chain
🏁 Script executed:
helm pull --help 2>&1 | grep -A 5 "verify" || echo "Checking flag availability"
helm verify --help 2>&1 | head -20Repository: Kuadrant/mcp-gateway
Length of output: 223
🏁 Script executed:
helm repository add kuadrant https://kuadrant.io/helm-charts/ --debug 2>&1 || \
curl -s https://kuadrant.io/helm-charts/index.yaml | head -50Repository: Kuadrant/mcp-gateway
Length of output: 1802
🏁 Script executed:
curl -s https://kuadrant.io/helm-charts/index.yaml | python3 -c "
import sys, yaml, hashlib
data = yaml.safe_load(sys.stdin)
entries = data.get('entries', {})
for name, versions in entries.items():
if versions:
latest = versions[0]
print(f'Chart: {name}')
print(f' Version: {latest.get(\"version\")}')
print(f' URL: {latest.get(\"url\")}')
print(f' Provenance check: {latest.get(\"provenance\") or \"Checking .prov file manually...\"}')
# Try to find .prov file
url = latest.get('url')
if url:
prov_url = url.rsplit('.', 1)[0] + '.tgz.prov'
print(f' Expected .prov URL: {prov_url}')
print()
"Repository: Kuadrant/mcp-gateway
Length of output: 592
🏁 Script executed:
curl -s https://kuadrant.io/helm-charts/index.yaml | python3 -c "
import sys, yaml
data = yaml.safe_load(sys.stdin)
entries = data.get('entries', {})
for name, versions in entries.items():
if versions:
for v in versions:
url = v.get('url')
if url:
# Get the basename and derive .prov path
if url.endswith('.tgz'):
prov = url[:-4] + '.prov'
else:
prov = url + '.prov'
print(f'{name} (v{v.get(\"version\")}): URL={url}, PROV_URL={prov}')
break
"Repository: Kuadrant/mcp-gateway
Length of output: 158
🏁 Script executed:
curl -s https://kuadrant.io/helm-charts/index.yaml | python3 -c "
import sys, yaml, json
data = yaml.safe_load(sys.stdin)
if not data:
print('Empty or invalid YAML')
sys.exit(1)
entries = data.get('entries', {})
# Print the full 'entries' section raw to debug structure
print('Entries keys:', list(entries.keys()))
print('Sample entry (authorino-operator):')
import pprint
pprint.pprint(entries.get('authorino-operator', [])[:1])
"Repository: Kuadrant/mcp-gateway
Length of output: 5974
🏁 Script executed:
curl -sL "https://github.com/Kuadrant/helm-charts/raw/refs/heads/main/charts/authorino-operator-0.23.1.tgz.prov" -o /tmp/authorino-operator-0.23.1.tgz.prov --write-out "%{http_code}" -o /dev/nullRepository: Kuadrant/mcp-gateway
Length of output: 161
🏁 Script executed:
cat -n docs/design/umbrella-operator-integration/tasks/tasks.md | sed -n '50,70p'Repository: Kuadrant/mcp-gateway
Length of output: 1331
Add a provenance validation step to the verification block.
The current verification commands load the repo and inspect templates but never validate the chart's .tgz.prov signature. To prevent regression of the signing requirement, replace the steps with:
- helm repo add kuadrant https://kuadrant.io/helm-charts/
- helm search repo kuadrant/mcp-gateway
- helm template mcp-gateway kuadrant/mcp-gateway --set controller.enabled=true
+ helm repo add kuadrant https://kuadrant.io/helm-charts/
+ helm search repo kuadrant/mcp-gateway --versions
+ helm pull kuadrant/mcp-gateway --version "$CHART_VERSION" --verify
+ helm template mcp-gateway kuadrant/mcp-gateway --version "$CHART_VERSION" --set controller.enabled=true📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| **Acceptance criteria:** | |
| - [ ] mcp-gateway chart `.tgz` is published to `https://kuadrant.io/helm-charts/` | |
| - [ ] `index.yaml` in the helm-charts repo includes the mcp-gateway entry | |
| - [ ] Chart is installable via `helm install mcp-gateway kuadrant/mcp-gateway` | |
| - [ ] Chart is signed (`.tgz.prov` file, matching other Kuadrant charts) | |
| **Verification:** | |
| ```bash | |
| helm repo add kuadrant https://kuadrant.io/helm-charts/ | |
| helm search repo kuadrant/mcp-gateway | |
| helm template mcp-gateway kuadrant/mcp-gateway --set controller.enabled=true | |
| ``` | |
| **Acceptance criteria:** | |
| - [ ] mcp-gateway chart `.tgz` is published to `https://kuadrant.io/helm-charts/` | |
| - [ ] `index.yaml` in the helm-charts repo includes the mcp-gateway entry | |
| - [ ] Chart is installable via `helm install mcp-gateway kuadrant/mcp-gateway` | |
| - [ ] Chart is signed (`.tgz.prov` file, matching other Kuadrant charts) | |
| **Verification:** |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/design/umbrella-operator-integration/tasks/tasks.md` around lines 53 -
64, The verification block currently checks repository access and templating but
does not validate the signed chart artifact, so update the verification steps in
the tasks entry to include provenance verification for the mcp-gateway chart.
Keep the existing helm repo/chart checks, and add a step using Helm’s provenance
verification against kuadrant/mcp-gateway so the .tgz.prov requirement is
explicitly exercised alongside the chart install and template checks.
| ``` | ||
| Task 1 (Chart versioning) | ||
| └──> Task 3 (Publish chart) ──> Task 4 (Release automation) | ||
| └──> Task 5 (Subchart integration) | ||
| Task 2 (Remove OLM deps) — independent, can run in parallel | ||
|
|
||
| Task 6 (Migration guide) — depends on Task 5 | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Label the dependency diagram fence.
The unlabeled fenced block at Line 134 trips the markdownlint warning. Add a language tag or switch to an indented block so docs CI stays clean.
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 134-134: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/design/umbrella-operator-integration/tasks/tasks.md` around lines 134 -
141, The dependency diagram block in the tasks document is an unlabeled fenced
block, which triggers markdownlint. Update the fence around the Task 1–Task 6
dependency diagram to include an appropriate language tag, or convert it to an
indented code block, so the markdown stays lint-clean.
Source: Linters/SAST tools
| ## Job Stories | ||
|
|
||
| ### When I install Kuadrant with MCP Gateway support | ||
|
|
||
| When a platform engineer installs Kuadrant via the umbrella operator or direct Helm, they want to enable mcp-gateway as an optional component so that MCP server aggregation and routing is available alongside auth and rate limiting without a separate installation step. | ||
|
|
||
| ### When I upgrade Kuadrant and mcp-gateway together | ||
|
|
||
| When a platform engineer upgrades Kuadrant via OLM or Helm, they want the umbrella operator to upgrade the mcp-gateway controller with version-pinned images so that all components stay compatible and the upgrade is coordinated. | ||
|
|
||
| ### When I migrate from standalone mcp-gateway to the umbrella | ||
|
|
||
| When a platform engineer has mcp-gateway installed via its own OLM subscription, they want a documented migration path to the umbrella operator so that existing MCPGatewayExtensions, broker-router deployments, and MCP server registrations continue working without disruption. | ||
|
|
||
| ### When multiple teams share a Kuadrant-managed gateway | ||
|
|
||
| When multiple teams each have their own MCPGatewayExtension in their namespace, they want their MCP server catalogs, auth policies, and rate limits to be isolated from other teams so that one team's configuration cannot affect another team's traffic, even though Authorino and Limitador are shared singletons. | ||
|
|
||
| ### When I want Kuadrant without mcp-gateway | ||
|
|
||
| When a platform engineer installs Kuadrant but does not need MCP Gateway functionality, they want to omit the mcp-gateway subchart so that no unnecessary CRDs, RBAC, or controller deployments are created. | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add the missing persona coverage to the job stories.
This section only covers platform engineer and tenant-isolation cases. The doc guideline expects stories for MCP client users, MCP Developers, and non-interactive agents too, plus at least one edge case. As per coding guidelines, job stories must cover those personas and follow the When <situation>, <actor> <context> wants <action/outcome> so that <benefit> format.
🧰 Tools
🪛 LanguageTool
[style] ~37-~37: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...hout a separate installation step. ### When I upgrade Kuadrant and mcp-gateway toge...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~41-~41: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...le and the upgrade is coordinated. ### When I migrate from standalone mcp-gateway t...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~45-~45: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ntinue working without disruption. ### When multiple teams share a Kuadrant-managed...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~49-~49: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...d Limitador are shared singletons. ### When I want Kuadrant without mcp-gateway Wh...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@docs/design/umbrella-operator-integration/umbrella-operator-integration-design.md`
around lines 31 - 52, Add the missing job stories in the Job Stories section to
cover MCP client users, MCP Developers, non-interactive agents, and at least one
edge case, using the required “When <situation>, <actor> wants <action/outcome>
so that <benefit>” format. Update the existing stories in the
umbrella-operator-integration-design document so the set reflects all required
personas and includes the missing scenarios, keeping the wording consistent with
the current style.
Source: Coding guidelines
| ``` | ||
| Umbrella Operator (renders Helm charts, deploys singletons): | ||
| ├── kuadrant-operator Deployment (policy controller) | ||
| ├── dns-operator Deployment | ||
| ├── Authorino Deployment (workload, cluster-wide) | ||
| ├── Limitador Deployment (workload, cluster-wide) | ||
| └── mcp-gateway-controller Deployment (NEW) | ||
|
|
||
| mcp-gateway-controller (reconciles MCPGatewayExtension CRs): | ||
| Per namespace: | ||
| ├── broker-router Deployment | ||
| ├── Service | ||
| ├── ServiceAccount | ||
| ├── HTTPRoute(s) | ||
| ├── Config Secret (mcp-gateway-config) | ||
| └── EnvoyFilter (in Gateway namespace) | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Label the architecture block.
This fenced block is plain text, which triggers the markdownlint MD040 warning at Line 84. Add a language hint such as text so the doc passes lint.
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 84-84: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@docs/design/umbrella-operator-integration/umbrella-operator-integration-design.md`
around lines 84 - 100, Label the fenced architecture block in the design doc by
adding a language hint to the plain-text fence so markdownlint MD040 no longer
fires; update the fence around the Umbrella Operator / mcp-gateway-controller
diagram to use an explicit text label, and keep the surrounding content
unchanged.
Source: Linters/SAST tools
1 participant
Summary
Context
Ref: Kuadrant/architecture#179 (RFC 0019: OLMv1 Operator Consolidation)
The umbrella operator RFC identifies mcp-gateway as a future subchart candidate. This design doc covers what mcp-gateway needs to become subchart-ready and confirms the multi-tenant deployment model (per-namespace broker-router) is compatible with Kuadrant's singleton Authorino/Limitador.
Test plan
docs/design/CLAUDE.mdSummary by CodeRabbit
mcp-gatewayas an optional subchart in the umbrella operator.