feat: support Slack dual-token credentials with per-role proxy routes#215
Conversation
Add `EnvVarSuffix` and `AllowedPaths` fields to `channelSecretRole` so multi-secret channels (like Slack) generate one proxy route per role with distinct env vars (`CRED_<NAME>_APP`, `CRED_<NAME>_BOT`). The app-token route restricts to `/api/apps.connections.open` while the bot-token route is a catch-all. Single-role channels are unaffected. Also add body token replacement for Slack routes: the proxy rewrites the `token` field in JSON and form-encoded request bodies, replacing the placeholder with the real credential from the env var. Additional changes: - add `--verbose` flag to the proxy - add `SetVerbose()` to `Server` for goproxy request/response logging - move user namespace to `test-e2e` in e2e tests and add TLS echo server helpers (`deployTLSEchoServer`, `patchProxyForEchoServer`, `curlThroughProxy`) for end-to-end proxy verification Assisted-by: Claude Opus 4.6 (1M context) Signed-off-by: Xavier Coulon <xcoulon@redhat.com>
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
🚧 Files skipped from review as they are similar to previous changes (1)
📜 Recent review details🧰 Additional context used🔀 Multi-repo context codeready-toolchain/host-operator, codeready-toolchain/toolchain-e2e, codeready-toolchain/api, codeready-toolchain/toolchain-commonLinked repositories findingscodeready-toolchain/host-operator
codeready-toolchain/toolchain-e2e
codeready-toolchain/api
codeready-toolchain/toolchain-common
codeready-toolchain/toolchain-e2e
WalkthroughThe PR adds Slack dual-token routing metadata, per-role proxy route and env-var generation, Slack request-body rewriting in the proxy, verbose proxy logging, and matching unit and e2e coverage. ChangesSlack dual-token routing
Sequence Diagram(s)sequenceDiagram
participant ClawController
participant ProxyServer
participant SlackBodyTokenReplacer
participant SlackAPI
ClawController->>ProxyServer: configure routes and env vars
ProxyServer->>SlackBodyTokenReplacer: Rewrite(request body)
SlackBodyTokenReplacer-->>ProxyServer: rewritten body
ProxyServer->>SlackAPI: forward request
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested labels
Suggested reviewers
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
coderabbitai
Bot
added
documentation
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 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 @.gitignore:
- Line 44: The ignore rule for proxy is too broad and can match any path
component named proxy, including source under cmd/proxy/. Update the .gitignore
entry to anchor it to the repository root or make it more specific to the
intended build artifact so it no longer hides legitimate tracked files; keep the
change focused on the proxy pattern in .gitignore.
In `@internal/assets/manifests/claw-proxy/proxy-deployment.yaml`:
- Around line 31-34: The base proxy deployment is starting every pod with the
verbose debug flag enabled, which should not ship in the default manifest.
Remove the --verbose entry from the main claw proxy deployment in
proxy-deployment.yaml and place any debug-only flags alongside the existing
commented dump-domains guidance in a separate overlay or temporary patch,
keeping the proxy deployment defaulted to non-debug behavior.
In `@internal/controller/claw_proxy_test.go`:
- Line 1282: The test in claw_proxy_test should assert against the correct env
var name returned by credEnvVarName("slack"), since using CRED_SL leaves the
check too weak. Update the NotContains assertion in the relevant test case to
look for CRED_SLACK instead, so the test fails if the old unsuffixed variable is
still emitted alongside the role-specific ones.
In `@internal/proxy/config.go`:
- Around line 48-49: The config is referencing body-rewriter wiring before the
package defines the needed symbols, causing compile failures; add the missing
BodyRewriter type and ensure NewSlackBodyTokenReplacer is defined or imported in
the proxy package before using them in Route/config wiring. Update the relevant
proxy types and constructor plumbing so injector/bodyRewriter fields and any
Route setup compile cleanly with the new body-rewriter symbols.
In `@internal/proxy/server.go`:
- Around line 60-61: The Slack route setup in server.go is referencing a missing
body rewriter constructor, causing the undefined symbol failure in CI. Add the
missing constructor to internal/proxy or update the call in the route
configuration loop to use the correct exported symbol that creates the Slack
body token rewriter. Make sure the symbol name matches what is available to
cfg.Routes[i].bodyRewriter assignment.
In `@test/e2e/e2e_test.go`:
- Around line 2121-2127: After scaling claw-operator-controller-manager to zero
in the e2e test, wait until the controller pod/process is actually gone before
patching any operator-managed resources. Update the test flow around the
existing kubectl scale call and the subsequent managed ConfigMap/Deployment
mutations so it blocks until the reconciler is fully stopped, preventing the
still-running controller from overwriting the test patches.
- Around line 112-115: The cleanup in e2e_test.go only removes
operatorNamespace, so userNamespace (the persistent test-e2e namespace created
in the setup flow) must also be deleted there. Update the top-level t.Cleanup in
the test setup to clean up both namespaces, using the same namespace variables
referenced around the kubectl create ns userNamespace step, so reruns do not hit
AlreadyExists or reuse stale resources.
🪄 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: Enterprise
Run ID: 322fbb9c-ea6a-4868-b5b2-231d921a4895
📒 Files selected for processing (17)
.gitignorecmd/proxy/main.gointernal/assets/manifests/claw-proxy/proxy-deployment.yamlinternal/controller/claw_channels.gointernal/controller/claw_channels_test.gointernal/controller/claw_proxy.gointernal/controller/claw_proxy_test.gointernal/proxy/config.gointernal/proxy/server.gointernal/proxy/server_test.goopenspec/changes/archive/2026-06-25-slack-dual-token/.openspec.yamlopenspec/changes/archive/2026-06-25-slack-dual-token/design.mdopenspec/changes/archive/2026-06-25-slack-dual-token/proposal.mdopenspec/changes/archive/2026-06-25-slack-dual-token/specs/channel-multi-secret-routing/spec.mdopenspec/changes/archive/2026-06-25-slack-dual-token/tasks.mdopenspec/specs/channel-multi-secret-routing/spec.mdtest/e2e/e2e_test.go
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
codeready-toolchain/api(manual)codeready-toolchain/toolchain-common(manual)codeready-toolchain/host-operator(manual)codeready-toolchain/toolchain-e2e(manual)
📜 Review details
🧰 Additional context used
📓 Path-based instructions (5)
internal/assets/manifests/**/*.{yaml,yml}
📄 CodeRabbit inference engine (CLAUDE.md)
internal/assets/manifests/**/*.{yaml,yml}: Configure pod security with non-root user (uid 65532), restricted seccomp profile, and drop all Linux capabilities
SetreadOnlyRootFilesystem: trueon proxy andwait-for-proxycontainers, but not on init-config or gateway containers which require writable paths for Node.js and AI tools
Files:
internal/assets/manifests/claw-proxy/proxy-deployment.yaml
**
⚙️ CodeRabbit configuration file
-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
Files:
internal/assets/manifests/claw-proxy/proxy-deployment.yamlcmd/proxy/main.goopenspec/changes/archive/2026-06-25-slack-dual-token/tasks.mdopenspec/specs/channel-multi-secret-routing/spec.mdopenspec/changes/archive/2026-06-25-slack-dual-token/specs/channel-multi-secret-routing/spec.mdopenspec/changes/archive/2026-06-25-slack-dual-token/proposal.mdopenspec/changes/archive/2026-06-25-slack-dual-token/design.mdinternal/proxy/server_test.gointernal/proxy/config.gointernal/proxy/server.gointernal/controller/claw_channels_test.gointernal/controller/claw_channels.gointernal/controller/claw_proxy_test.gointernal/controller/claw_proxy.gotest/e2e/e2e_test.go
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: Include Go license header fromhack/boilerplate.go.txttemplate in all Go source files
Run golangci-lint viamake lintand usemake lint-fixfor auto-fixing, with.golangci.ymlconfiguration includinglllanddupllinters enabled
Usemake fmt(go fmt) andmake vet(go vet) for code formatting and vetting before committing
Files:
cmd/proxy/main.gointernal/proxy/server_test.gointernal/proxy/config.gointernal/proxy/server.gointernal/controller/claw_channels_test.gointernal/controller/claw_channels.gointernal/controller/claw_proxy_test.gointernal/controller/claw_proxy.gotest/e2e/e2e_test.go
**/*_test.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*_test.go: Userequireassertion from testify for fatal setup errors in tests, andassertfor value comparisons
Structure tests withTest*function names, uset.Run()for subtests,t.Cleanup()for cleanup, and implement table-driven test patterns
UsewaitFor(t, timeout, interval, condition, message)helper for async test assertions with 10s timeout and 250ms poll interval
Create separate test files per resource type (e.g.,claw_configmap_test.go,claw_credentials_test.go)
Files:
internal/proxy/server_test.gointernal/controller/claw_channels_test.gointernal/controller/claw_proxy_test.gotest/e2e/e2e_test.go
internal/controller/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
Set owner references on all created resources via
controllerutil.SetControllerReference
Files:
internal/controller/claw_channels_test.gointernal/controller/claw_channels.gointernal/controller/claw_proxy_test.gointernal/controller/claw_proxy.go
🪛 GitHub Actions: E2E Tests / 0_E2E Tests.txt
internal/proxy/config.go
[error] 49-49: go vet failed: internal/proxy/config.go:49:15: undefined: BodyRewriter
🪛 GitHub Actions: E2E Tests / E2E Tests
internal/proxy/config.go
[error] 49-49: go vet failed: undefined: BodyRewriter
[error] 1-1: Command failed at step: go vet ./...
🪛 GitHub Actions: Lint / Lint
internal/proxy/config.go
[error] 49-49: golangci-lint failed: undefined: BodyRewriter
🪛 GitHub Actions: Tests / 0_Unit Tests.txt
internal/proxy/config.go
[error] 49-49: go vet failed: internal/proxy/config.go:49:15: undefined: BodyRewriter
🪛 GitHub Actions: Tests / Unit Tests
internal/proxy/config.go
[error] 49-49: go vet failed: undefined: BodyRewriter
🪛 GitHub Check: E2E Tests
internal/proxy/config.go
[failure] 49-49:
undefined: BodyRewriter
internal/proxy/server.go
[failure] 61-61:
undefined: NewSlackBodyTokenReplacer
🪛 GitHub Check: Lint
internal/proxy/server_test.go
[failure] 597-597:
undefined: NewSlackBodyTokenReplacer (typecheck)
internal/proxy/config.go
[failure] 49-49:
undefined: BodyRewriter
internal/proxy/server.go
[failure] 61-61:
undefined: NewSlackBodyTokenReplacer
🪛 GitHub Check: Unit Tests
internal/proxy/config.go
[failure] 49-49:
undefined: BodyRewriter
internal/proxy/server.go
[failure] 61-61:
undefined: NewSlackBodyTokenReplacer
🔇 Additional comments (1)
internal/controller/claw_proxy.go (1)
224-248: 🎯 Functional CorrectnessVerify partial Slack role sets are rejected before generating routes.
credentialRoutesalways materializes both Slack role routes fromknownChannels, butconfigureProxyForCredentialsonly injects env vars for roles that actually exist inSecretRef. If validation allows a credential with onlyappTokenor onlybotToken, the specific route still wins with an unset env var and fails at runtime. Please confirm the CRD/webhook rejects incomplete Slack role sets.
Signed-off-by: Xavier Coulon <xcoulon@redhat.com>
Signed-off-by: Xavier Coulon <xcoulon@redhat.com>
Signed-off-by: Xavier Coulon <xcoulon@redhat.com>
Signed-off-by: Xavier Coulon <xcoulon@redhat.com>
Signed-off-by: Xavier Coulon <xcoulon@redhat.com>
coderabbitai
Bot
removed
the
documentation
Signed-off-by: Xavier Coulon <xcoulon@redhat.com>
Signed-off-by: Xavier Coulon <xcoulon@redhat.com>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #215 +/- ##
==========================================
- Coverage 78.97% 78.80% -0.18%
==========================================
Files 32 33 +1
Lines 4176 4284 +108
==========================================
+ Hits 3298 3376 +78
- Misses 562 581 +19
- Partials 316 327 +11
🚀 New features to boost your workflow:
|
2 participants
Add
EnvVarSuffixandAllowedPathsfields tochannelSecretRolesomulti-secret channels (like Slack) generate one proxy route per role
with distinct env vars (
CRED_<NAME>_APP,CRED_<NAME>_BOT). Theapp-token route restricts to
/api/apps.connections.openwhile thebot-token route is a catch-all. Single-role channels are unaffected.
Also add body token replacement for Slack routes: the proxy rewrites the
tokenfield in JSON and form-encoded request bodies, replacing theplaceholder with the real credential from the env var.
Additional changes:
--verboseflag to the proxySetVerbose()toServerfor goproxy request/response loggingtest-e2ein e2e tests and add TLS echoserver helpers (
deployTLSEchoServer,patchProxyForEchoServer,curlThroughProxy) for end-to-end proxy verificationAssisted-by: Claude Opus 4.6 (1M context)
Signed-off-by: Xavier Coulon xcoulon@redhat.com
Summary by CodeRabbit
-verboseoption to enable more detailed proxy logging.tokenin both JSON andapplication/x-www-form-urlencodedbodies..slack.com).