[management] temporary peer access for resources #4889
Conversation
WalkthroughAdds a temporary-access endpoint for network resources, injects an accountManager into the resource handler, implements CreateTemporaryAccess HTTP handler to create a temporary peer and associated temporary access policies, and updates OpenAPI and generated API types for the new route. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant HTTP as CreateTemporaryAccess Handler
participant Auth as Auth Validator
participant ResMgr as Resource Manager
participant AcctMgr as Account Manager
participant PeerMgr as Peer Creator
participant PolicyMgr as Policy Manager
Client->>HTTP: POST /networks/{networkId}/resources/{resourceId}/temporary-access
HTTP->>Auth: validate authentication/authorization
Auth-->>HTTP: auth result
HTTP->>ResMgr: validate networkId and resourceId, fetch resource
ResMgr-->>HTTP: resource found
HTTP->>HTTP: decode PeerTemporaryAccessRequest
HTTP->>PeerMgr: create temporary peer object
PeerMgr-->>HTTP: peer created
HTTP->>AcctMgr: attach peer to account
AcctMgr-->>HTTP: attachment confirmed
HTTP->>PolicyMgr: create & persist temporary access policies per rule
PolicyMgr-->>HTTP: policies saved
HTTP->>Client: 200 PeerTemporaryAccessResponse
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (23)
🔇 Additional comments (1)
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]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
shared/management/http/api/openapi.yml (1)
4111-4152: Resource temporary-access path is consistent with existing temporary-access APIThe new
/api/networks/{networkId}/resources/{resourceId}/temporary-accessPOST operation looks structurally correct (params, security, request/response match the existing peer-scoped temporary-access endpoint).One minor docs consideration: all other
/api/networks/...endpoints are tagged[ Networks ], while this one is tagged[ Peers ]. If you want all network-related operations grouped together in generated docs, consider switching to[ Networks ]or adding both tags.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
management/server/http/handlers/networks/resources_handler.go(2 hunks)shared/management/http/api/openapi.yml(1 hunks)shared/management/http/api/types.gen.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
management/server/http/handlers/networks/resources_handler.go (5)
management/server/context/auth.go (1)
GetUserAuthFromContext(25-30)shared/management/http/util/util.go (3)
WriteError(84-120)WriteErrorResponse(70-80)WriteJSONObject(27-35)shared/management/http/api/types.gen.go (9)
PeerTemporaryAccessRequest(1232-1241)Peer(1021-1105)Policy(1294-1312)PolicyRule(1345-1380)Resource(1528-1532)ResourceTypePeer(171-171)ResourceType(1535-1535)RulePortRange(1628-1634)PeerTemporaryAccessResponse(1244-1253)management/server/types/policy.go (2)
ParseRuleString(145-180)PolicyTrafficActionAccept(12-12)management/server/types/resource.go (3)
Resource(16-19)ResourceTypePeer(10-10)ResourceType(7-7)
⏰ 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). (20)
- GitHub Check: Management / Unit (amd64, postgres)
- GitHub Check: Management / Unit (amd64, mysql)
- GitHub Check: Management / Benchmark (amd64, sqlite)
- GitHub Check: Management / Benchmark (amd64, postgres)
- GitHub Check: Management / Unit (amd64, sqlite)
- GitHub Check: Relay / Unit (amd64, -race)
- GitHub Check: Relay / Unit (386)
- GitHub Check: Client / Unit (amd64)
- GitHub Check: Client / Unit (386)
- GitHub Check: Client (Docker) / Unit
- GitHub Check: Android / Build
- GitHub Check: Linux
- GitHub Check: release
- GitHub Check: Client / Unit
- GitHub Check: release_ui_darwin
- GitHub Check: release_ui
- GitHub Check: Windows
- GitHub Check: Darwin
- GitHub Check: JS / Lint
- GitHub Check: Client / Unit
🔇 Additional comments (3)
shared/management/http/api/types.gen.go (1)
1974-1976: New request body alias matches existing API typing patternThe alias cleanly reuses
PeerTemporaryAccessRequestfor the new resource temporary-access endpoint and is consistent with the otherPostApi*JSONRequestBodyaliases.management/server/http/handlers/networks/resources_handler.go (2)
21-43: InjectingaccountManagerintoresourceHandlerand wiring the new route looks consistentAdding
accountManagertoresourceHandler, updatingaddResourceEndpoints/newResourceHandler, and wiring/networks/{networkId}/resources/{resourceId}/temporary-accessthrough the same handler instance matches the existing DI pattern used forresourceManager/groupsManagerand ensures the handler has everything it needs to create peers and policies.
229-314: Non-atomic behavior is the main concern; type usage and naming are correctThe handler flow is functionally sound overall. However, there is one notable design consideration:
- Non-atomic behavior / potential stray objects:
OnceAddPeersucceeds, any subsequent failure inParseRuleStringorSavePolicywill leave a temporary peer (and possibly some policies) created even though the client receives an error. If feasible, consider:
- Parsing all
req.Rulesup-front before callingAddPeer, so invalid rule strings never cause a partial create after the peer exists.- Optionally grouping all rules into a single
Policyrather than one policy per rule, reducingSavePolicycalls and making it easier to reason about rollbacks.
|
Describe your changes
Issue ticket number and link
Stack
Checklist
Documentation
Select exactly one:
Docs PR URL (required if "docs added" is checked)
Paste the PR link from https://github.com/netbirdio/docs here:
https://github.com/netbirdio/docs/pull/__
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.