Skip to content

Improvement of the NetBird integration#97

Merged
lucasdillmann merged 2 commits into
mainfrom
netbird-improvements
Mar 2, 2026
Merged

Improvement of the NetBird integration#97
lucasdillmann merged 2 commits into
mainfrom
netbird-improvements

Conversation

@lucasdillmann

Copy link
Copy Markdown
Owner

No description provided.

@lucasdillmann lucasdillmann self-assigned this Mar 2, 2026
@lucasdillmann lucasdillmann marked this pull request as ready for review March 2, 2026 01:30
@greptile-apps

greptile-apps Bot commented Mar 2, 2026

Copy link
Copy Markdown

Greptile Summary

This PR improves the NetBird VPN integration by adding proper HTTPS/certificate support end-to-end: the backend gains a new validation rule, the endpoint hash is extended to detect HTTPS/certificate changes, and the frontend is updated to load, display, and persist certificate data correctly.

Key changes:

  • core/host/validator.go: New guard rejects a VPN entry that carries a certificateId while enableHTTPS is false, preventing an inconsistent state from reaching the backend.
  • core/nginx/vpn_manager.go: endpointAdapter.Hash() now incorporates enableHTTPS and certDetails.ID, so toggling HTTPS or swapping a certificate correctly triggers an endpoint restart on the next nginx reload.
  • frontend/src/domain/host/HostConverter.ts: vpnToFormValues now fetches the associated certificate object and formValuesToVpn serialises enableHttps + certificateId, completing the round-trip for saved VPN configurations.
  • frontend/src/domain/host/HostFormPage.tsx / HostVpns.tsx: The onChange signature is widened to pass the full updated array, and a new handleVpnsChange handler keeps the component state in sync. The mutation pattern in the handler functions uses a shallow array spread, leaving the original element objects mutated in place — worth replacing with a proper per-element spread (see inline comment).
  • vpn/netbird/logger.go: Adds a noOpLogger to silence verbose NetBird client output.
  • i18n: All eleven locale files receive the new core/host/vpn-certificate-cannot-be-informed-if-disabled message key.

Confidence Score: 4/5

  • This PR is safe to merge; the only identified issue is a shallow-copy mutation pattern that is a best-practice concern rather than an observable runtime defect.
  • All backend changes (validation, hash, certificate loading) are correct and well-tested. The frontend changes fix real bugs (stale state, missing certificate round-trip). A minor React immutability violation in HostVpns.tsx is the sole imperfection — it is unlikely to cause a user-visible bug with the current class-component rendering path but should be addressed.
  • frontend/src/domain/host/components/HostVpns.tsx — shallow-copy mutation of VPN objects in handleVpnChange and handleCertificateChange

Important Files Changed

Filename Overview
core/host/validator.go Adds an early validation guard: if a certificate is provided but HTTPS is disabled, a validation error is emitted. The fieldPath variable is hoisted out of the guard for reuse. Logic is correct and well-tested.
core/nginx/vpn_manager.go Hash now includes enableHTTPS flag and certificate ID, ensuring configuration changes to HTTPS or the selected certificate correctly trigger endpoint restart/stop during reload. Improvement is safe and well-reasoned.
frontend/src/domain/host/HostConverter.ts Now fetches the associated certificate when converting a saved VPN entry to form values, and correctly serializes enableHttps and certificateId back into the request payload. Both additions are required for round-trip correctness.
frontend/src/domain/host/HostFormPage.tsx Adds handleVpnsChange that keeps component state in sync with the VPN list; the call to setFieldsValue in the callback correctly synchronises Ant Design's internal form state after the React state update.
frontend/src/domain/host/components/HostVpns.tsx Refactored to pass the updated VPN array back through onChange; however, both handleVpnChange and handleCertificateChange use a shallow array spread ([...vpns]) which still mutates the original element objects, violating React immutability expectations.
frontend/src/domain/host/model/HostRequest.ts Adds enableHttps: boolean (required) and certificateId?: string (optional) to the HostVpn payload interface, correctly aligning the frontend model with the backend's updated VPN structure.
vpn/netbird/endpoint.go No functional changes; this file reads the target.HTTPS data already constructed by vpn_manager.go, and the TLS listener / proxy transport logic is unchanged.
vpn/netbird/logger.go Introduces noOpLogger, a minimal io.Writer that discards NetBird client log output; clean, focused change that avoids polluting application logs with library internals.

Sequence Diagram

sequenceDiagram
    participant User
    participant HostVpns
    participant HostFormPage
    participant HostConverter
    participant ValidatorGo as validator.go
    participant VpnManagerGo as vpn_manager.go
    participant NetbirdEndpoint as netbird/endpoint.go

    User->>HostVpns: Select VPN / Certificate
    HostVpns->>HostVpns: handleVpnChange / handleCertificateChange
    HostVpns->>HostFormPage: onChange(updatedVpns)
    HostFormPage->>HostFormPage: handleVpnsChange → setState + setFieldsValue

    User->>HostFormPage: Submit form
    HostFormPage->>HostConverter: formValuesToRequest(formValues)
    HostConverter-->>HostFormPage: HostRequest (vpns[].enableHttps, vpns[].certificateId)

    HostFormPage->>ValidatorGo: POST /hosts (HTTP request)
    ValidatorGo->>ValidatorGo: validateVPNCertificate
    alt certificateId set but enableHTTPS=false
        ValidatorGo-->>HostFormPage: 422 Validation error
    else valid
        ValidatorGo-->>VpnManagerGo: proceed
    end

    VpnManagerGo->>VpnManagerGo: buildEndpoints (loads certDetails)
    VpnManagerGo->>VpnManagerGo: endpointAdapter.Hash() includes enableHTTPS + certID
    VpnManagerGo->>VpnManagerGo: stopObsoleteEndpoints / startNewEndpoints
    VpnManagerGo->>NetbirdEndpoint: Start(ctx, endpoint)
    NetbirdEndpoint->>NetbirdEndpoint: startListener (TLS if HTTPS enabled)
Loading

Last reviewed commit: 424b1f9

@greptile-apps greptile-apps 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.

22 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

Comment thread frontend/src/domain/host/components/HostVpns.tsx Outdated
@sonarqubecloud

sonarqubecloud Bot commented Mar 2, 2026

Copy link
Copy Markdown

@lucasdillmann lucasdillmann merged commit a0f43dc into main Mar 2, 2026
4 checks passed
@lucasdillmann lucasdillmann deleted the netbird-improvements branch March 2, 2026 01:57
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.

1 participant