refactor: removes legacy format for auth in provider-proxy#2283
Conversation
WalkthroughReplaces legacy top-level MTLS fields with a nested Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2283 +/- ##
==========================================
- Coverage 47.62% 47.27% -0.35%
==========================================
Files 1036 1026 -10
Lines 29361 29007 -354
Branches 7625 7580 -45
==========================================
- Hits 13982 13714 -268
+ Misses 14988 14900 -88
- Partials 391 393 +2
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/provider-proxy/src/services/WebsocketServer.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{ts,tsx,js}: Never use typeanyor cast to typeany. Always define the proper TypeScript types.
Never use deprecated methods from libraries.
Don't add unnecessary comments to the code.
Files:
apps/provider-proxy/src/services/WebsocketServer.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: test-build
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
apps/provider-proxy/src/services/WebsocketServer.ts (1)
136-138: LGTM! Good improvement for type safety.Wrapping the authentication check in
Boolean()ensures the attribute value is always a proper boolean instead of a truthy/falsy value, which improves consistency for OpenTelemetry attributes.
stalniy
force-pushed
the
fix/auth-type
branch
from
45bb656 to
f29f369
Compare
stalniy
changed the title
stalniy
force-pushed
the
fix/auth-type
branch
from
f29f369 to
66811d5
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/provider-proxy/src/services/WebsocketServer.ts (1)
127-136: Guard against undefinedauthenticationTypein OpenTelemetry attributes.
message.authis optional, somessage.auth?.typecan beundefined. OpenTelemetry attributes do not supportundefinedvalues; passing one intospan.setAttributes(attributes)is invalid and was already flagged in a previous review.Set
authenticationTypeonly when it’s defined:const message = parsedMessage.data as WsMessage; const attributes: Attributes = { type: message.type }; if (message.type === "websocket") { attributes.providerUrl = message.url; attributes.providerAddress = message.providerAddress; attributes.function = getWebSocketUsage(message); - attributes.authenticationType = message.auth?.type; + if (message.auth?.type) { + attributes.authenticationType = message.auth.type; + } }
🧹 Nitpick comments (1)
apps/provider-proxy/src/utils/schema.ts (1)
33-36: Consider removing the now no-oppreprocesswrapper.
z.preprocess(data => data, schema)is effectively a no-op now that legacy flatcertPem/keyPemmapping is gone; you could simplify this to just attachsuperRefinetoschema:export function addProviderAuthValidation<T extends z.ZodTypeAny>(schema: T): z.ZodEffects<T> { return schema .superRefine((data, ctx) => { // existing mtls/jwt validation }) as unknown as z.ZodEffects<T>; }This keeps behavior the same while shaving off an unnecessary preprocessing step.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/provider-proxy/src/app.ts(2 hunks)apps/provider-proxy/src/config/env.config.ts(1 hunks)apps/provider-proxy/src/routes/proxyProviderRequest.ts(1 hunks)apps/provider-proxy/src/services/WebsocketServer.ts(1 hunks)apps/provider-proxy/src/utils/schema.ts(1 hunks)apps/provider-proxy/test/functional/provider-proxy-http.spec.ts(4 hunks)apps/provider-proxy/test/setup/proxyServer.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{ts,tsx,js}: Never use typeanyor cast to typeany. Always define the proper TypeScript types.
Never use deprecated methods from libraries.
Don't add unnecessary comments to the code.
Files:
apps/provider-proxy/src/app.tsapps/provider-proxy/test/functional/provider-proxy-http.spec.tsapps/provider-proxy/test/setup/proxyServer.tsapps/provider-proxy/src/config/env.config.tsapps/provider-proxy/src/routes/proxyProviderRequest.tsapps/provider-proxy/src/utils/schema.tsapps/provider-proxy/src/services/WebsocketServer.ts
**/*.spec.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/no-jest-mock.mdc)
Don't use
jest.mock()in test files. Instead, usejest-mock-extendedto create mocks and pass mocks as dependencies to the service under testUse
setupfunction instead ofbeforeEachin test files. Thesetupfunction must be at the bottom of the rootdescribeblock, should create an object under test and return it, accept a single parameter with inline type definition, avoid shared state, and not have a specified return type.
**/*.spec.{ts,tsx}: Use<Subject>.namein the root describe suite description instead of hardcoded class/service name strings to enable automated refactoring tools to find all references
Use either a method name or a condition starting with 'when' for nested suite descriptions in tests
Use present simple, 3rd person singular for test descriptions without prepending 'should'
Files:
apps/provider-proxy/test/functional/provider-proxy-http.spec.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: baktun14
Repo: akash-network/console PR: 1725
File: apps/api/src/utils/constants.ts:5-5
Timestamp: 2025-07-24T17:00:52.361Z
Learning: In the Akash Network Console project, when cross-cutting concerns or broader refactoring issues are identified during PR review, the preferred approach is to create a separate GitHub issue to track the work rather than expanding the scope of the current PR. This maintains focus and allows for proper planning of architectural improvements.
📚 Learning: 2025-09-04T04:27:40.858Z
Learnt from: stalniy
Repo: akash-network/console PR: 1868
File: apps/api/src/billing/services/managed-signer/managed-signer.service.ts:98-107
Timestamp: 2025-09-04T04:27:40.858Z
Learning: In the Akash protocol, when processing MsgCreateLease messages, the bidId.dseq field is always required and guaranteed to be present at runtime, even though TypeScript types may suggest it could be undefined. The non-null assertion operator (!) is appropriate to use in this context.
Applied to files:
apps/provider-proxy/src/services/WebsocketServer.ts
🧬 Code graph analysis (3)
apps/provider-proxy/src/app.ts (1)
apps/provider-proxy/src/config/env.config.ts (1)
AppConfigInput(9-9)
apps/provider-proxy/test/setup/proxyServer.ts (2)
apps/provider-proxy/src/app.ts (1)
AppServer(88-92)apps/provider-proxy/src/config/env.config.ts (1)
AppConfigInput(9-9)
apps/provider-proxy/src/routes/proxyProviderRequest.ts (1)
apps/api/src/auth/services/auth.interceptor.ts (1)
auth(88-96)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: codecov/project/provider-proxy
- GitHub Check: test-build
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (5)
apps/provider-proxy/src/routes/proxyProviderRequest.ts (1)
58-65: LoggingtimeoutandauthenticationTypelooks good and avoids leaking secrets.Including
timeoutandauth?.typeinPROXY_REQUESTlogs improves observability while keeping credential material out of logs.apps/provider-proxy/src/config/env.config.ts (1)
8-9: AppConfigInput alias is consistent with schema usage.Exposing
AppConfigInput = z.input<typeof appConfigSchema>matches how you’re now typing untrusted config and keeps the input/output types explicit.apps/provider-proxy/src/app.ts (1)
10-53: startAppServer now correctly advertises the config input type.Switching to
AppConfigInputin the import andstartAppServerparameter makes the expected config shape explicit while keeping compatibility with the genericRecord<string, unknown>path.apps/provider-proxy/test/setup/proxyServer.ts (1)
3-9: Test server setup now matches the new config input type.Updating
startServerto takeAppConfigInputand passing it through tostartAppServerwithPORT: 0keeps test setup aligned with the runtime API and avoids relying on the inferredAppConfigoutput type.apps/provider-proxy/test/functional/provider-proxy-http.spec.ts (1)
201-205: Tests correctly exercise the new nestedauthformat.Updating the HTTP proxy tests to send
auth: { type: "mtls" | "jwt", ... }keeps them aligned withproviderRequestSchemaand the new validation paths (["auth","certPem"],["auth","token"]), while continuing to cover invalid/expired cert and JWT error scenarios.Also applies to: 497-501, 560-564, 609-613
2 participants
Why
cleanup
Summary by CodeRabbit
Bug Fixes
Chores
Tests
✏️ Tip: You can customize this high-level summary in your review settings.