import tokenbroker#460
Conversation
Signed-off-by: David Hadas <david.hadas@gmail.com>
📝 WalkthroughWalkthroughThree authbridge binary entry points now import the ChangesToken Broker Plugin Registration
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Possibly related issues
Poem
🚥 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)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@authbridge/cmd/authbridge-lite/main.go`:
- Around line 41-44: The inline comment claiming "Only two plugins: drop the
parsers and token-broker." is stale because main.go imports jwtvalidation,
tokenbroker and tokenexchange; update that comment and the package header (lines
1-3) to accurately list the compiled plugins (e.g., "jwtvalidation, tokenbroker,
tokenexchange" or the intended subset) so the package doc and the comment above
the blank imports reflect the actual set; search for the blank-imports _
"github.com/kagenti/.../plugins/jwtvalidation", _
"github.com/kagenti/.../plugins/tokenbroker", and _
"github.com/kagenti/.../plugins/tokenexchange" to place the corrected wording.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 50bae63f-f162-40aa-aa6c-52009b9c8f23
📒 Files selected for processing (3)
authbridge/cmd/authbridge-envoy/main.goauthbridge/cmd/authbridge-lite/main.goauthbridge/cmd/authbridge-proxy/main.go
| // Only two plugins: drop the parsers and token-broker. | ||
| _ "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins/jwtvalidation" | ||
| _ "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins/tokenbroker" | ||
| _ "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins/tokenexchange" |
There was a problem hiding this comment.
Comment now contradicts the code: token-broker is no longer dropped.
Line 41 still says "drop the parsers and token-broker," but Line 43 compiles tokenbroker in. This is misleading for anyone auditing the lite binary's surface. Update both this comment and the package doc (Lines 1-3, "jwt-validation + token-exchange as the only plugins compiled in").
📝 Suggested fix
- // Only two plugins: drop the parsers and token-broker.
+ // Drop the parsers; keep the auth/token plugins.
_ "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins/jwtvalidation"
_ "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins/tokenbroker"
_ "github.com/kagenti/kagenti-extensions/authbridge/authlib/plugins/tokenexchange"And the package header:
-// Package main is the "lite" authbridge binary: proxy-sidecar mode
-// only (no Envoy), with jwt-validation + token-exchange as the only
-// plugins compiled in. Optimizes for binary size by dropping the
+// Package main is the "lite" authbridge binary: proxy-sidecar mode
+// only (no Envoy), with jwt-validation, token-exchange, and
+// token-broker as the only plugins compiled in. Optimizes for binary
+// size by dropping the🤖 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 `@authbridge/cmd/authbridge-lite/main.go` around lines 41 - 44, The inline
comment claiming "Only two plugins: drop the parsers and token-broker." is stale
because main.go imports jwtvalidation, tokenbroker and tokenexchange; update
that comment and the package header (lines 1-3) to accurately list the compiled
plugins (e.g., "jwtvalidation, tokenbroker, tokenexchange" or the intended
subset) so the package doc and the comment above the blank imports reflect the
actual set; search for the blank-imports _
"github.com/kagenti/.../plugins/jwtvalidation", _
"github.com/kagenti/.../plugins/tokenbroker", and _
"github.com/kagenti/.../plugins/tokenexchange" to place the corrected wording.
huang195
left a comment
There was a problem hiding this comment.
Minimal, correct change — one blank _ import added to each of the three authbridge entrypoints using a consistent, alphabetically-ordered import path. Builds and all CI checks pass.
Areas reviewed: Go
Commits: 1 commit, signed-off ✓
CI status: All passing
PR title note: "import tokenbroker" doesn't follow the required prefix format (Feat: / Chore:) from the repo's enforced ruleset — worth fixing to avoid future enforcement issues.
| @@ -40,6 +40,7 @@ import ( | |||
|
|
|||
| // Only two plugins: drop the parsers and token-broker. | |||
There was a problem hiding this comment.
This comment now directly contradicts the code — it says token-broker is dropped, but this PR adds it. CodeRabbit flagged the same. Suggest updating to reflect the actual plugin set, e.g.:
// Three plugins: jwtvalidation, tokenbroker, tokenexchange (parsers dropped).
Summary
add import tokenbroker to authbridge
Related issue(s)
(Optional) Testing Instructions
Fixes # #459
Summary by CodeRabbit