[client] Add Expose support to embed library#5695
Merged
Conversation
Add ability to expose local services via the NetBird reverse proxy from embedded client code. Introduce ExposeSession with a blocking Wait method that keeps the session alive until the context is cancelled. Extract ProtocolType with ParseProtocolType into the expose package and use it across CLI and embed layers.
Contributor
|
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:
📝 WalkthroughWalkthroughAdds a typed ProtocolType enum and parser; updates internal expose request/manager to use the enum; introduces an embed Expose API and ExposeSession with Wait; updates CLI to parse protocols via the new enum and switch on enum values; updates tests and minor reorderings. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User/CLI
participant CLI as CLI (toExposeProtocol)
participant Client as Client.Expose
participant Engine as Engine
participant Manager as ExposeManager
participant Session as ExposeSession
User->>CLI: provide protocol string
CLI->>CLI: ParseProtocolType(str) -> ProtocolType
User->>Client: Expose(ctx, ExposeRequest)
Client->>Engine: getEngine()
Engine-->>Client: engine
Client->>Manager: engine.GetExposeManager()
Manager-->>Client: manager
Client->>Manager: mgr.Expose(ctx, req)
Manager-->>Client: expose response
Client->>Session: create ExposeSession{Domain,ServiceName,ServiceURL,mgr}
Client-->>User: return *ExposeSession
User->>Session: Wait(ctx)
Session->>Manager: mgr.KeepAlive(ctx, Domain)
Manager-->>Session: blocks until ctx cancelled or error
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
client/internal/expose/protocol.go (1)
8-22: Consider adding aString()method for debugging and logging.Adding a
String()method toProtocolTypewould improve debugging and logging output. Currently, logging aProtocolTypewill show the numeric value rather than a human-readable name.💡 Suggested implementation
// String returns the string representation of the protocol type. func (p ProtocolType) String() string { switch p { case ProtocolHTTP: return "http" case ProtocolHTTPS: return "https" case ProtocolTCP: return "tcp" case ProtocolUDP: return "udp" case ProtocolTLS: return "tls" default: return fmt.Sprintf("unknown(%d)", p) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/internal/expose/protocol.go` around lines 8 - 22, Add a String() method on ProtocolType to return human-readable names for logging/debugging: implement func (p ProtocolType) String() string { ... } that switches on ProtocolHTTP, ProtocolHTTPS, ProtocolTCP, ProtocolUDP, ProtocolTLS and returns "http", "https", "tcp", "udp", "tls" respectively, and returns a formatted fallback like "unknown(%d)" for defaults; ensure you import fmt if needed and place the method near the ProtocolType definition so logs calling ProtocolType.String() or fmt.Sprintf("%v", p) produce readable output.client/cmd/expose.go (1)
136-143: Consider consolidating protocol validation logic.
isProtocolValidduplicates the validation already performed byexpose.ParseProtocolType. Similarly,isClusterProtocolandisPortBasedProtocoluse string-based checks whiletoExposeProtocolnow uses the typed enum. Consider adding helper methods toProtocolType(e.g.,IsCluster(),IsPortBased()) to centralize this logic and prevent drift.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/cmd/expose.go` around lines 136 - 143, isProtocolValid duplicates expose.ParseProtocolType and string checks in isClusterProtocol/isPortBased drift from the typed enum; add helper methods on ProtocolType (e.g., func (p ProtocolType) IsCluster() bool and func (p ProtocolType) IsPortBased() bool and optionally IsValid()) in the expose package, implement their logic against the enum values, then replace string-based checks in isProtocolValid, isClusterProtocol, isPortBased and update toExposeProtocol to use these ProtocolType helpers (or call ParseProtocolType once and use the resulting ProtocolType.IsCluster/IsPortBased) so all protocol logic is centralized on the ProtocolType methods.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@client/cmd/expose.go`:
- Around line 136-143: isProtocolValid duplicates expose.ParseProtocolType and
string checks in isClusterProtocol/isPortBased drift from the typed enum; add
helper methods on ProtocolType (e.g., func (p ProtocolType) IsCluster() bool and
func (p ProtocolType) IsPortBased() bool and optionally IsValid()) in the expose
package, implement their logic against the enum values, then replace
string-based checks in isProtocolValid, isClusterProtocol, isPortBased and
update toExposeProtocol to use these ProtocolType helpers (or call
ParseProtocolType once and use the resulting ProtocolType.IsCluster/IsPortBased)
so all protocol logic is centralized on the ProtocolType methods.
In `@client/internal/expose/protocol.go`:
- Around line 8-22: Add a String() method on ProtocolType to return
human-readable names for logging/debugging: implement func (p ProtocolType)
String() string { ... } that switches on ProtocolHTTP, ProtocolHTTPS,
ProtocolTCP, ProtocolUDP, ProtocolTLS and returns "http", "https", "tcp", "udp",
"tls" respectively, and returns a formatted fallback like "unknown(%d)" for
defaults; ensure you import fmt if needed and place the method near the
ProtocolType definition so logs calling ProtocolType.String() or
fmt.Sprintf("%v", p) produce readable output.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a7a1a300-fcf9-40d2-b9f3-d0b37db0f3a8
📒 Files selected for processing (6)
client/cmd/expose.goclient/embed/embed.goclient/embed/expose.goclient/internal/expose/manager.goclient/internal/expose/protocol.goclient/internal/expose/request.go
lixmal
requested changes
Mar 26, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
client/embed/expose.go (1)
23-27: Consider decoupling embed API from internal types.Using type aliases (
ExposeRequest/ExposeProtocolType) directly toclient/internal/exposemakes internal refactors potentially API-breaking for embed consumers. Defining explicit embed-facing types plus conversion at the boundary would improve long-term API stability.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/embed/expose.go` around lines 23 - 27, The public embed API currently aliases internal types (ExposeRequest and ExposeProtocolType) to client/internal/expose, which couples the public surface to internal refactors; change these aliases to explicit embed-facing types (e.g., define a new struct/type ExposeRequest and new type ExposeProtocolType in this package) and implement conversion functions (e.g., ToInternalExposeRequest and FromInternalExposeRequest) that translate between the embed types and client/internal/expose types at the boundary (use these converters wherever the package currently passes alias types to internal code such as in any functions referencing ExposeRequest or ExposeProtocolType).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@client/embed/expose.go`:
- Around line 39-40: The Wait method on ExposeSession must guard against a nil
receiver or missing manager to avoid panics: in the ExposeSession.Wait function
check if s == nil or s.mgr == nil and return a clear error (e.g., "nil
ExposeSession" or "missing manager") instead of calling s.mgr.KeepAlive; keep
the call to s.mgr.KeepAlive(ctx, s.Domain) only after those checks. Ensure
references to the ExposeSession type and the Wait method and the s.mgr.KeepAlive
call are used to locate and update the code.
---
Nitpick comments:
In `@client/embed/expose.go`:
- Around line 23-27: The public embed API currently aliases internal types
(ExposeRequest and ExposeProtocolType) to client/internal/expose, which couples
the public surface to internal refactors; change these aliases to explicit
embed-facing types (e.g., define a new struct/type ExposeRequest and new type
ExposeProtocolType in this package) and implement conversion functions (e.g.,
ToInternalExposeRequest and FromInternalExposeRequest) that translate between
the embed types and client/internal/expose types at the boundary (use these
converters wherever the package currently passes alias types to internal code
such as in any functions referencing ExposeRequest or ExposeProtocolType).
🪄 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
Run ID: f7c87a9a-473a-44a0-bb66-2fa44537e9d0
📒 Files selected for processing (4)
client/cmd/expose.goclient/embed/embed.goclient/embed/expose.goclient/internal/expose/manager.go
✅ Files skipped from review due to trivial changes (1)
- client/internal/expose/manager.go
🚧 Files skipped from review as they are similar to previous changes (2)
- client/embed/embed.go
- client/cmd/expose.go
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
client/embed/expose.go (1)
3-7:⚠️ Potential issue | 🔴 CriticalImport
errorsand remove the trailing brace.Line 41 uses
errors.New()without importing theerrorspackage, and line 45 contains an extra closing brace. The file will not compile as-is.Proposed fix
import ( "context" + "errors" "github.com/netbirdio/netbird/client/internal/expose" ) @@ func (s *ExposeSession) Wait(ctx context.Context) error { if s == nil || s.mgr == nil { return errors.New("expose session is not initialized") } return s.mgr.KeepAlive(ctx, s.Domain) } -}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/embed/expose.go` around lines 3 - 7, The import block in client/embed/expose.go is missing the "errors" package needed by the call to errors.New(), and there is an extra closing brace at the end of the file that breaks compilation; update the import list to include "errors" alongside "context" and "github.com/netbirdio/netbird/client/internal/expose", and remove the stray trailing brace after the function/variable that calls errors.New() so the file parses and compiles correctly.
🧹 Nitpick comments (1)
client/embed/expose.go (1)
10-19: Clarify that these constants describe the exposed/frontend mode.The current comments can be read as “the local service must speak HTTPS/TLS.” For a new embed-facing API, it would be clearer to say these values select how the public endpoint is exposed, because backend transport is handled separately internally.
Based on learnings,
Service.Modecontrols the frontend listener type, whileTarget.Protocolis a separate backend concern.Also applies to: 25-26
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/embed/expose.go` around lines 10 - 19, Update the comments for the ExposeProtocol* constants (ExposeProtocolHTTP, ExposeProtocolHTTPS, ExposeProtocolTCP, ExposeProtocolUDP, ExposeProtocolTLS) to make clear these values select how the public/frontend endpoint is exposed (the frontend/listener mode), not the backend transport; mention that backend transport is controlled separately by Target.Protocol and that Service.Mode controls the frontend listener type—apply the same clarification to the other related comments at the 25-26 area.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@client/embed/expose.go`:
- Around line 3-7: The import block in client/embed/expose.go is missing the
"errors" package needed by the call to errors.New(), and there is an extra
closing brace at the end of the file that breaks compilation; update the import
list to include "errors" alongside "context" and
"github.com/netbirdio/netbird/client/internal/expose", and remove the stray
trailing brace after the function/variable that calls errors.New() so the file
parses and compiles correctly.
---
Nitpick comments:
In `@client/embed/expose.go`:
- Around line 10-19: Update the comments for the ExposeProtocol* constants
(ExposeProtocolHTTP, ExposeProtocolHTTPS, ExposeProtocolTCP, ExposeProtocolUDP,
ExposeProtocolTLS) to make clear these values select how the public/frontend
endpoint is exposed (the frontend/listener mode), not the backend transport;
mention that backend transport is controlled separately by Target.Protocol and
that Service.Mode controls the frontend listener type—apply the same
clarification to the other related comments at the 25-26 area.
lixmal
reviewed
Mar 27, 2026
lixmal
previously approved these changes
Mar 27, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@client/embed/expose.go`:
- Around line 3-7: The build fails because errors.New is used but the "errors"
package is not imported; update the import block in this file to include the
standard "errors" package so the call to errors.New() compiles (i.e., add
"errors" to the import list alongside context and the internal/expose import).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
lixmal
approved these changes
Mar 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Describe your changes
Add Expose API to the embed library, allowing programmatic creation and
management of service exposures (HTTP, HTTPS, TCP, UDP, TLS) from embedded
client code.
Introduces ExposeSession with a blocking Wait method that keeps the exposure
alive until the context is cancelled. Extracts ProtocolType into a shared
expose package for reuse across CLI and embed layers.
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
New Features
Bug Fixes / Improvements