Skip to content

Issue/278#279

Merged
NLipatov merged 6 commits into
mainfrom
issue/278
Apr 5, 2026
Merged

Issue/278#279
NLipatov merged 6 commits into
mainfrom
issue/278

Conversation

@NLipatov

@NLipatov NLipatov commented Mar 31, 2026

Copy link
Copy Markdown
Owner

See: #278

Summary by CodeRabbit

Release Notes

  • New Features

    • Implemented protocol-specific tunnel naming with automatic client-server differentiation
    • Added outbound IP address resolution capability for IPv4 and IPv6
  • Bug Fixes

    • Enhanced error reporting for unsupported network protocols
  • Refactor

    • Improved configuration generation architecture through dependency management enhancements
    • Streamlined server configuration defaults and naming convention handling

@NLipatov NLipatov self-assigned this Mar 31, 2026
@NLipatov NLipatov linked an issue Mar 31, 2026 that may be closed by this pull request
@coderabbitai

coderabbitai Bot commented Mar 31, 2026

Copy link
Copy Markdown

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: fdeaf2a2-98ff-45fd-8f20-2290a2cf767d

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • ✅ Review completed - (🔄 Check again to review again)
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch issue/278

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov

codecov Bot commented Mar 31, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 86.20690% with 8 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/application/confgen/generator.go 79.31% 3 Missing and 3 partials ⚠️
...rastructure/network/host_resolver/dial_resolver.go 88.88% 2 Missing ⚠️
Flag Coverage Δ
unittests 95.27% <86.20%> (+0.04%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...tructure/PAL/configuration/server/configuration.go 100.00% <100.00%> (ø)
...infrastructure/PAL/configuration/server/manager.go 100.00% <100.00%> (ø)
.../infrastructure/PAL/configuration/server/reader.go 100.00% <100.00%> (ø)
...ui/tui/internal/bubble_tea/configurator_session.go 96.65% <100.00%> (ø)
src/presentation/ui/tui/server_configurator.go 98.64% <100.00%> (ø)
...rastructure/network/host_resolver/dial_resolver.go 88.88% <88.88%> (ø)
src/application/confgen/generator.go 92.38% <79.31%> (+7.11%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 `@src/application/confgen/generator.go`:
- Around line 194-203: deriveClientTunName currently treats settings.WSS as
unsupported which breaks deriveClientSettings for WSS; add a case for
settings.WSS to return clientWSTunName (same as settings.WS) and keep
ErrUnsupportedProtocol only for truly unknown protocols (e.g., settings.UNKNOWN)
so deriveClientSettings(...) succeeds for WSS. Update the switch in
deriveClientTunName to include settings.WSS alongside settings.WS returning
clientWSTunName.
🪄 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: 6fd0d489-a59a-4121-ace6-bb2d738a3596

📥 Commits

Reviewing files that changed from the base of the PR and between fc643a8 and 9f93bc5.

📒 Files selected for processing (10)
  • src/application/confgen/const.go
  • src/application/confgen/errors.go
  • src/application/confgen/generator.go
  • src/application/confgen/generator_test.go
  • src/infrastructure/PAL/configuration/server/configuration.go
  • src/infrastructure/PAL/configuration/server/configuration_test.go
  • src/infrastructure/PAL/configuration/server/const.go
  • src/infrastructure/PAL/configuration/server/manager.go
  • src/infrastructure/PAL/configuration/server/manager_test.go
  • src/infrastructure/PAL/configuration/server/reader.go

Comment thread src/application/confgen/generator.go

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/application/confgen/generator.go (1)

24-33: Guard against nil resolver injection.

Line 27 accepts resolver as-is; if a caller passes nil, Generate() will fail later when invoking g.resolver.ResolveIPv4(). Add an early guard so failures are deterministic and non-panicking.

♻️ Proposed fix
 import (
+	"errors"
 	"fmt"
 	"net/netip"
 	"tungo/infrastructure/PAL/configuration/client"
 	serverConfiguration "tungo/infrastructure/PAL/configuration/server"
 	"tungo/infrastructure/cryptography/primitives"
 	"tungo/infrastructure/settings"
 )
+
+var errHostResolverNotConfigured = errors.New("host resolver is not configured")
@@
 func (g *Generator) Generate() (*client.Configuration, error) {
+	if g.resolver == nil {
+		return nil, errHostResolverNotConfigured
+	}
+
 	serverConf, err := g.serverConfigurationManager.Configuration()
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/application/confgen/generator.go` around lines 24 - 33, NewGenerator
currently assigns the passed resolver directly, which allows a nil resolver to
be injected and causes a panic later in Generate when calling
g.resolver.ResolveIPv4(); add an early guard in NewGenerator that checks if
resolver == nil and returns nil (or an explicit error-producing Generator/stub)
so construction fails deterministically instead of panicking—update NewGenerator
to validate resolver and fail fast, referencing the NewGenerator function, the
resolver parameter, the Generator struct, and the Generate method
(g.resolver.ResolveIPv4) so callers cannot create a Generator with a nil
resolver.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/application/confgen/generator.go`:
- Around line 24-33: NewGenerator currently assigns the passed resolver
directly, which allows a nil resolver to be injected and causes a panic later in
Generate when calling g.resolver.ResolveIPv4(); add an early guard in
NewGenerator that checks if resolver == nil and returns nil (or an explicit
error-producing Generator/stub) so construction fails deterministically instead
of panicking—update NewGenerator to validate resolver and fail fast, referencing
the NewGenerator function, the resolver parameter, the Generator struct, and the
Generate method (g.resolver.ResolveIPv4) so callers cannot create a Generator
with a nil resolver.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: f09c59ec-4d01-4157-b1bf-4298a0c666fe

📥 Commits

Reviewing files that changed from the base of the PR and between 9f93bc5 and 487e699.

📒 Files selected for processing (7)
  • src/application/confgen/generator.go
  • src/application/confgen/generator_test.go
  • src/infrastructure/network/host_resolver/dial_resolver.go
  • src/infrastructure/network/host_resolver/dial_resolver_test.go
  • src/main.go
  • src/presentation/ui/tui/internal/bubble_tea/configurator_session.go
  • src/presentation/ui/tui/server_configurator.go
✅ Files skipped from review due to trivial changes (1)
  • src/main.go
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/application/confgen/generator_test.go

@NLipatov NLipatov marked this pull request as ready for review April 5, 2026 11:27
@NLipatov NLipatov merged commit 6c92c41 into main Apr 5, 2026
9 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unify TUN interface naming with role-based client/server prefixes

1 participant