From ac45d9d7360ad9dd5203706a13809520aae6ce9f Mon Sep 17 00:00:00 2001 From: EvaTheSalmon <18466855+EvaTheSalmon@users.noreply.github.com> Date: Wed, 14 Oct 2026 22:07:50 +0300 Subject: [PATCH] docs(repo): trim comments to the irreducible why Co-Authored-By: Claude Opus 5.5 (1M context) --- cmd/usher/env.go | 3 -- cmd/usher/main.go | 4 +- deploy/compose.e2e-xray.yml | 4 +- deploy/compose.example.yml | 8 +--- internal/atomicfile/atomicfile.go | 5 +-- internal/awg/enroll.go | 12 ++---- internal/awg/fake_test.go | 1 - internal/awg/render.go | 3 +- internal/awg/session.go | 13 +----- internal/clients/mirror.go | 14 ++----- internal/config/config.go | 6 +-- internal/pass/backend.go | 7 +--- internal/pass/pass.go | 22 ++--------- internal/pass/pass_test.go | 2 - internal/pass/settings.go | 1 - internal/state/state.go | 16 ++------ internal/watch/watch.go | 7 +--- internal/watch/watch_test.go | 1 - internal/xray/config.go | 66 +++++++++---------------------- internal/xray/config_test.go | 4 +- internal/xray/drift.go | 29 ++------------ internal/xray/drift_test.go | 17 +++----- internal/xray/enroll.go | 5 +-- internal/xray/mirror.go | 25 +++--------- internal/xray/mirror_test.go | 6 +-- internal/xray/protocol.go | 17 ++------ internal/xray/render.go | 11 +----- internal/xray/render_test.go | 7 +--- internal/xray/session.go | 44 +++++---------------- internal/xray/session_test.go | 33 +++++++--------- internal/xray/socket.go | 14 ++----- test/e2e/e2e_test.go | 36 ++++++----------- test/e2e/xray/client_test.go | 24 ++--------- test/e2e/xray/pki_test.go | 6 +-- test/e2e/xray/server_test.go | 14 ++----- test/e2e/xray/xray_test.go | 55 +++++++------------------- 36 files changed, 134 insertions(+), 408 deletions(-) diff --git a/cmd/usher/env.go b/cmd/usher/env.go index 4ccc2bd..fc8928d 100644 --- a/cmd/usher/env.go +++ b/cmd/usher/env.go @@ -16,7 +16,6 @@ const ( statePath = "/srv/usher/state/users.json" ) -// environment holds the server facts from the process environment and the fixed paths. type environment struct { pass.Settings Host netip.Addr // USHER_HOST @@ -70,7 +69,6 @@ func environmentFrom(getenv func(string) string) (environment, error) { }, nil } -// parseDNS parses the comma-separated USHER_DNS list that the awg backend puts into client configs. func parseDNS(value string) ([]netip.Addr, error) { var dns []netip.Addr for _, field := range strings.Split(value, ",") { @@ -94,7 +92,6 @@ func parseIPv4(name, value string) (netip.Addr, error) { return addr, nil } -// parseAbsPath requires a non-empty absolute path. func parseAbsPath(name, value string) (string, error) { if value == "" { return "", fmt.Errorf("%s is required", name) diff --git a/cmd/usher/main.go b/cmd/usher/main.go index a4c948e..1ae77b5 100644 --- a/cmd/usher/main.go +++ b/cmd/usher/main.go @@ -84,8 +84,7 @@ func xrayBackend(env environment, log *slog.Logger) pass.Backend { } } -// asSession converts a backend session to pass.Session. On error it returns a nil interface, not -// an interface that holds a nil pointer. +// asSession returns a nil interface on error, not an interface holding a nil pointer. func asSession[S pass.Session](session S, err error) (pass.Session, error) { if err != nil { return nil, err @@ -93,7 +92,6 @@ func asSession[S pass.Session](session S, err error) (pass.Session, error) { return session, nil } -// dialUnix returns a dial function that opens a gRPC client over the unix socket at path. func dialUnix[C any](path string, newClient func(grpc.ClientConnInterface) C) func() (C, func(), error) { target := "unix://" + path return func() (C, func(), error) { diff --git a/deploy/compose.e2e-xray.yml b/deploy/compose.e2e-xray.yml index e8892ea..c3e2039 100644 --- a/deploy/compose.e2e-xray.yml +++ b/deploy/compose.e2e-xray.yml @@ -58,9 +58,7 @@ services: USHER_HOST: 172.30.98.2 USHER_XRAY_SOCKET: /run/xray/api.sock USHER_XRAY_CONFIG: /etc/xray/config.json - # Client of one user; the test writes its config to ../.e2e-xray/client//config.json. The - # config has one HTTP proxy inbound per tested link, and the test reaches them at the fixed - # address. SSL_CERT_FILE stands in for the public CA that a real certificate chains to. + # SSL_CERT_FILE stands in for the public CA that a real certificate chains to. client-alice: profiles: [client] image: ghcr.io/xtls/xray-core:26.10.10 diff --git a/deploy/compose.example.yml b/deploy/compose.example.yml index 7caf681..da2cadb 100644 --- a/deploy/compose.example.yml +++ b/deploy/compose.example.yml @@ -12,7 +12,6 @@ services: ulimits: nofile: { soft: 65536, hard: 65536 } ports: - # Publish the UDP port of every interface 1:1. - "51820:51820/udp" volumes: # One .conf per interface, [Interface] only. @@ -22,8 +21,7 @@ services: # Must equal usher's group. AWG_GRPC_SOCKET_GID: "1000" xray: - # The tag follows the xray-core version in usher's go.mod. A different version can change how - # Xray builds an inbound, and usher then reports every inbound as drifted. + # Match the xray-core version in usher's go.mod, or usher may report every inbound as drifted. image: ghcr.io/xtls/xray-core:26.10.10 restart: unless-stopped # Must equal usher's group, so that usher can open the API socket. @@ -38,9 +36,7 @@ services: # The certificates and keys that the config names. Only Xray mounts them: usher needs no # access to a key file, and Xray reloads the files itself. - ./xray-certs:/etc/xray-certs:ro - # A host directory that uid 1000 owns (create it before the first start), not a named volume, - # which belongs to root: after a crash usher unlinks the stale API socket, and that needs - # write access to the directory. + # Owned by uid 1000, not a named volume: usher unlinks a stale API socket after a crash. - ./xray-sock:/run/xray usher: image: ghcr.io/mrcsin/usher:v0.1.0 diff --git a/internal/atomicfile/atomicfile.go b/internal/atomicfile/atomicfile.go index d1bb5a1..1b4cd1e 100644 --- a/internal/atomicfile/atomicfile.go +++ b/internal/atomicfile/atomicfile.go @@ -7,9 +7,8 @@ import ( "path/filepath" ) -// Write puts data into path with the given mode: it writes a temp file in the same directory, -// syncs it, renames it over path and syncs the directory so the rename survives power loss. The -// directory of path must exist. +// Write replaces path atomically and syncs the directory so the rename survives power loss. +// The directory of path must exist. func Write(path string, data []byte, mode os.FileMode) error { dir := filepath.Dir(path) tmp, err := os.CreateTemp(dir, ".atomic-*.tmp") diff --git a/internal/awg/enroll.go b/internal/awg/enroll.go index 03173bc..41626a4 100644 --- a/internal/awg/enroll.go +++ b/internal/awg/enroll.go @@ -15,11 +15,8 @@ import ( "github.com/mrcsin/usher/internal/state" ) -// enroll makes st hold a valid entry for every user in users on the interface. It creates missing -// entries (X25519 key pair, random PSK, lowest free address) and reassigns stored addresses that -// break the allocation rules. users must be sorted: of two entries with the same address the -// first keeps it. It reports whether any entry was created or changed. On error st is left as it -// was. +// enroll reports whether st changed; on error st is unchanged. users must be sorted: of two +// entries with the same address the first keeps it. func enroll(log *slog.Logger, st *state.State, iface *awgv1.InterfaceStatus, users []string) (bool, error) { if len(users) == 0 { return false, nil @@ -43,8 +40,7 @@ func enroll(log *slog.Logger, st *state.State, iface *awgv1.InterfaceStatus, use } } - // Every valid stored address is claimed before any new one is allocated, so a new user never - // takes the address of an existing one. + // Claim every stored address first, so a new user never takes an existing one's. var needAddress []string for _, user := range users { entry := st.AWG[state.EntryName(ifaceName, user)] @@ -100,7 +96,6 @@ func interfaceNetwork(iface *awgv1.InterfaceStatus) (netip.Prefix, map[netip.Add return network, reserved, nil } -// usable reports whether addr is a host address of prefix that nothing else holds. func usable(prefix netip.Prefix, reserved, taken map[netip.Addr]bool, addr netip.Addr) bool { if !prefix.Contains(addr) || addr == prefix.Addr() || addr == broadcast(prefix) { return false @@ -119,7 +114,6 @@ func lowestFree(prefix netip.Prefix, reserved, taken map[netip.Addr]bool) (netip return netip.Addr{}, fmt.Errorf("no free address in %s", prefix) } -// broadcast returns the highest address of an IPv4 prefix. func broadcast(prefix netip.Prefix) netip.Addr { network := prefix.Masked().Addr().As4() hostMask := uint32(1)<<(32-prefix.Bits()) - 1 diff --git a/internal/awg/fake_test.go b/internal/awg/fake_test.go index e53eedc..5c46f15 100644 --- a/internal/awg/fake_test.go +++ b/internal/awg/fake_test.go @@ -8,7 +8,6 @@ import ( "google.golang.org/grpc" ) -// fakeClient implements awgv1.ManagementServiceClient with func fields and records requests. type fakeClient struct { status func() (*awgv1.GetStatusResponse, error) apply func(*awgv1.ApplyPeersRequest) (*awgv1.ApplyPeersResponse, error) diff --git a/internal/awg/render.go b/internal/awg/render.go index f9b72e6..dfbea82 100644 --- a/internal/awg/render.go +++ b/internal/awg/render.go @@ -12,8 +12,7 @@ import ( "github.com/mrcsin/usher/internal/state" ) -// render returns the client config of one user on one interface. The error never quotes a -// client_params value. +// render never quotes a client_params value in its errors. func render(entry state.Entry, iface *awgv1.InterfaceStatus, host netip.Addr, dns []netip.Addr) ([]byte, error) { port := iface.GetListenPort() if port > math.MaxUint16 { diff --git a/internal/awg/session.go b/internal/awg/session.go index 9a28234..c165308 100644 --- a/internal/awg/session.go +++ b/internal/awg/session.go @@ -18,17 +18,12 @@ import ( "github.com/mrcsin/usher/internal/state" ) -// Suffix is the file suffix of an AmneziaWG client config. const Suffix = ".conf" -// callTimeout bounds each call to awg-grpc. const callTimeout = 10 * time.Second -// Dial opens a connection to awg-grpc. It returns the client and the function that closes the -// connection. type Dial func() (awgv1.ManagementServiceClient, func(), error) -// Session is one pass's view of awg-grpc: the interfaces it reported when the session opened. type Session struct { client awgv1.ManagementServiceClient closeConn func() @@ -67,14 +62,12 @@ func Open(ctx context.Context, dial Dial, host netip.Addr, dns []netip.Addr, log return s, nil } -// Interfaces returns the names of the interfaces awg-grpc reported, in the order it reported them. func (s *Session) Interfaces() []string { return s.names } // Prepare enrolls the users on the interface and renders their client configs, keyed by -// clients.Path. It reports whether it changed st. It fails an interface that is not present in -// the kernel. +// clients.Path. It reports whether it changed st and fails an interface absent from the kernel. func (s *Session) Prepare(st *state.State, name string, users []string) (map[string][]byte, bool, error) { iface := s.ifaces[name] if !iface.GetPresent() { @@ -95,7 +88,6 @@ func (s *Session) Prepare(st *state.State, name string, users []string) (map[str return files, changed, nil } -// Apply sends the peers of the users to awg-grpc and logs what changed. func (s *Session) Apply(ctx context.Context, st *state.State, name string, users []string) error { peers := make([]*awgv1.Peer, 0, len(users)) for _, user := range users { @@ -126,13 +118,10 @@ func (s *Session) Apply(ctx context.Context, st *state.State, name string, users return nil } -// Close closes the connection to awg-grpc. func (s *Session) Close() { s.closeConn() } -// publicKeyNames maps the public key of every entry of the interface, switched off or not, to -// its user name. func publicKeyNames(st *state.State, ifaceName string) map[string]string { names := make(map[string]string) for user, e := range st.InterfaceEntries(ifaceName) { diff --git a/internal/clients/mirror.go b/internal/clients/mirror.go index 846ee08..1875967 100644 --- a/internal/clients/mirror.go +++ b/internal/clients/mirror.go @@ -24,9 +24,8 @@ func Path(user, ifaceName, suffix string) string { return user + "/" + ifaceName + suffix } -// Existing returns the current content of the files in dir that end in one of suffixes and belong -// to an interface for which keep returns true, keyed by Path. It reads as much as it can and returns -// the errors of the rest joined. +// Existing returns the files in dir with one of suffixes on an interface that keep accepts, keyed +// by Path. It reads what it can and joins the errors of the rest. func Existing(dir string, suffixes []string, keep func(ifaceName string) bool) (map[string][]byte, error) { files := make(map[string][]byte) userDirs, err := os.ReadDir(dir) @@ -62,7 +61,6 @@ func Existing(dir string, suffixes []string, keep func(ifaceName string) bool) ( return files, errors.Join(errs...) } -// cutSuffixes strips the first of suffixes that name ends in and returns it. func cutSuffixes(name string, suffixes []string) (base, suffix string, ok bool) { for _, suffix := range suffixes { if base, ok := strings.CutSuffix(name, suffix); ok { @@ -72,10 +70,8 @@ func cutSuffixes(name string, suffixes []string) (base, suffix string, ok bool) return "", "", false } -// Mirror makes dir hold exactly the desired files. Keys of desired are slash-separated paths -// relative to dir, such as "phone/awg0.conf". Files are written atomically with mode 0600 and -// only when the content differs. Every other file and every empty directory is -// removed. +// Mirror makes dir hold exactly desired, keyed by slash-separated path relative to dir. Files are +// written atomically with mode 0600 and only when the content differs. func Mirror(dir string, desired map[string][]byte) error { wanted := make(map[string]bool, len(desired)) for rel := range desired { @@ -94,8 +90,6 @@ func Mirror(dir string, desired map[string][]byte) error { return nil } -// removeUnwanted deletes every non-directory entry under dir that is not wanted, then every -// directory left empty. dir itself stays. func removeUnwanted(dir string, wanted map[string]bool) error { var dirs []string err := filepath.WalkDir(dir, func(path string, d fs.DirEntry, err error) error { diff --git a/internal/config/config.go b/internal/config/config.go index a0d9d2d..6f2fd19 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -14,14 +14,12 @@ var ( interfaceNamePattern = regexp.MustCompile(`^[A-Za-z0-9_][A-Za-z0-9_=+.-]{0,31}$`) ) -// ValidInterfaceName reports whether name is acceptable as an interface name in usher.yml. func ValidInterfaceName(name string) bool { return interfaceNamePattern.MatchString(name) } -// Load reads path and returns the interface names of every user. A user with an empty list is -// valid, but a file without any user/interface pair is not. Every error names the file, and the -// line when the parser or a rule violation has one. +// Load returns the interface names of every user; a file without any user/interface pair is an +// error. Every error names the file, and the line when there is one. func Load(path string) (map[string][]string, error) { data, err := os.ReadFile(path) if err != nil { diff --git a/internal/pass/backend.go b/internal/pass/backend.go index 483e20b..37e9145 100644 --- a/internal/pass/backend.go +++ b/internal/pass/backend.go @@ -6,21 +6,16 @@ import ( "github.com/mrcsin/usher/internal/state" ) -// Session is one pass's connection to a backend. Open lists the backend's interfaces; the -// session prepares and applies one interface at a time. +// Session is one pass's connection to a backend, working one interface at a time. type Session interface { - // Interfaces returns the names of the interfaces the backend reported. Interfaces() []string // Prepare enrolls the users on the interface in st and returns their client files, keyed by // clients.Path, and whether it changed st. The pass saves st before any Apply. Prepare(st *state.State, name string, users []string) (files map[string][]byte, changed bool, err error) - // Apply makes the interface hold exactly the users. Apply(ctx context.Context, st *state.State, name string, users []string) error - // Close releases the connection. Close() } -// Backend is a VPN server usher moves users into. type Backend struct { Name string // used in logs, such as "awg-grpc" Suffixes []string // suffixes of the client files the backend renders, such as ".conf" diff --git a/internal/pass/pass.go b/internal/pass/pass.go index 7488154..00bae71 100644 --- a/internal/pass/pass.go +++ b/internal/pass/pass.go @@ -13,7 +13,7 @@ import ( "github.com/mrcsin/usher/internal/state" ) -// Pass runs reconcile passes. It keeps the last valid usher.yml it read. +// Pass keeps the last valid usher.yml it read. type Pass struct { settings Settings backends []Backend @@ -21,26 +21,22 @@ type Pass struct { last map[string][]string } -// New returns a Pass that works with the paths in settings and moves users into backends. func New(settings Settings, backends []Backend, log *slog.Logger) *Pass { return &Pass{settings: settings, backends: backends, log: log} } -// openBackend is a backend with its session for this pass. type openBackend struct { Backend session Session } -// interfaceKey identifies an interface of one backend, by the backend's index in the open list. type interfaceKey struct { backend int name string } -// Run executes one pass. It logs every failure and never panics on one; a failure of one -// interface leaves the others to apply, and a backend that is down leaves the other backends to -// apply. +// Run executes one pass. It logs every failure; one failed interface or down backend never +// blocks the others. func (p *Pass) Run(ctx context.Context) { cfg, ok := p.loadConfig() if !ok { @@ -80,9 +76,6 @@ func (p *Pass) Run(ctx context.Context) { p.mirrorClients(open, down, rendered, failed) } -// prepareAll prepares every interface that exactly one backend reports. It marks a failed -// interface in failed, and returns the rendered client files by interface and whether any -// backend changed the state. func (p *Pass) prepareAll(st *state.State, open []openBackend, byInterface map[string][]string, owners map[string][]string, failed map[interfaceKey]bool) (map[interfaceKey]map[string][]byte, bool) { rendered := make(map[interfaceKey]map[string][]byte) @@ -109,7 +102,6 @@ func (p *Pass) prepareAll(st *state.State, open []openBackend, byInterface map[s return rendered, changed } -// applyAll applies every interface that Prepare did not fail and marks the ones that fail now. func (p *Pass) applyAll(ctx context.Context, st *state.State, open []openBackend, byInterface map[string][]string, failed map[interfaceKey]bool) { for i, b := range open { @@ -126,8 +118,7 @@ func (p *Pass) applyAll(ctx context.Context, st *state.State, open []openBackend } } -// mirrorClients writes the client files of the interfaces that did not fail and keeps the current -// files of the failed interfaces and of the backends that are down. +// mirrorClients keeps the current files of failed interfaces and of backends that are down. func (p *Pass) mirrorClients(open []openBackend, down []Backend, rendered map[interfaceKey]map[string][]byte, failed map[interfaceKey]bool) { desired := make(map[string][]byte) @@ -147,7 +138,6 @@ func (p *Pass) mirrorClients(open []openBackend, down []Backend, } } -// openBackends opens a session for every backend and returns the backends that failed to open. func (p *Pass) openBackends(ctx context.Context) ([]openBackend, []Backend) { var open []openBackend var down []Backend @@ -163,8 +153,6 @@ func (p *Pass) openBackends(ctx context.Context) ([]openBackend, []Backend) { return open, down } -// keepFiles copies the current client files of the backend's interfaces that keep selects into -// desired. func (p *Pass) keepFiles(desired map[string][]byte, b Backend, keep func(ifaceName string) bool) { kept, err := clients.Existing(p.settings.ClientsDir, b.Suffixes, keep) if err != nil { @@ -187,8 +175,6 @@ func (p *Pass) loadConfig() (map[string][]string, bool) { return p.last, true } -// interfaceOwners maps every reported interface name to the names of the backends that -// reported it. func interfaceOwners(open []openBackend) map[string][]string { owners := make(map[string][]string) for _, b := range open { diff --git a/internal/pass/pass_test.go b/internal/pass/pass_test.go index 94a74bd..db30a6d 100644 --- a/internal/pass/pass_test.go +++ b/internal/pass/pass_test.go @@ -18,8 +18,6 @@ import ( "github.com/mrcsin/usher/internal/state" ) -// fakeSession is a Session that renders "new " into one file per user and suffix and -// enrolls a state entry the first time an interface has users. type fakeSession struct { names []string suffixes []string diff --git a/internal/pass/settings.go b/internal/pass/settings.go index e593676..cf3c2c0 100644 --- a/internal/pass/settings.go +++ b/internal/pass/settings.go @@ -2,7 +2,6 @@ // requests to each backend and the clients directory. package pass -// Settings holds the paths one pass works with. type Settings struct { ConfigPath string ClientsDir string diff --git a/internal/state/state.go b/internal/state/state.go index 999126c..73fb1bb 100644 --- a/internal/state/state.go +++ b/internal/state/state.go @@ -23,26 +23,22 @@ const version = 2 const firstVersion = 1 -// backupSuffix names the copy of a version 1 file that Save makes before it writes version 2. +// backupSuffix names the copy of a version 1 file that Save makes. const backupSuffix = ".v1" const fileMode = 0o600 -// KeyLength is the size in bytes of a private key and of a preshared key. const KeyLength = 32 -// hostBits is the prefix length of the one-address route a peer gets. const hostBits = 32 -// Key is a private or a preshared key. It is base64 in users.json. type Key [KeyLength]byte -// MarshalText encodes the key as standard base64. func (k Key) MarshalText() ([]byte, error) { return []byte(k.String()), nil } -// UnmarshalText decodes standard base64 of exactly KeyLength bytes. The error never quotes text. +// UnmarshalText decodes standard base64 of exactly KeyLength bytes. Its error never quotes text. func (k *Key) UnmarshalText(text []byte) error { raw, err := base64.StdEncoding.DecodeString(string(text)) if err != nil || len(raw) != KeyLength { @@ -52,26 +48,22 @@ func (k *Key) UnmarshalText(text []byte) error { return nil } -// String returns the standard base64 form of the key. func (k Key) String() string { return base64.StdEncoding.EncodeToString(k[:]) } -// Entry holds the secrets and the address of one user on one AmneziaWG interface. type Entry struct { Address netip.Addr `json:"address"` PrivateKey Key `json:"private_key"` PresharedKey Key `json:"preshared_key"` } -// PublicKey returns the X25519 public key of the private key. func (e Entry) PublicKey() []byte { // NewPrivateKey fails only for a key that is not 32 bytes, and a Key always is. key, _ := ecdh.X25519().NewPrivateKey(e.PrivateKey[:]) return key.PublicKey().Bytes() } -// Route returns the one-address prefix of the entry. func (e Entry) Route() netip.Prefix { return netip.PrefixFrom(e.Address, hostBits) } @@ -119,7 +111,6 @@ type State struct { Xray map[string]XrayEntry `json:"xray"` } -// InterfaceEntries yields the user name and the entry of every entry on the interface. func (s *State) InterfaceEntries(iface string) iter.Seq2[string, Entry] { return func(yield func(string, Entry) bool) { for name, e := range s.AWG { @@ -166,8 +157,7 @@ func Load(path string) (*State, error) { return &s, nil } -// Save writes s to path atomically with mode 0600. When path holds a version 1 file, Save first -// copies it to path+".v1" unless that copy exists, so a rollback is a file rename. +// Save writes s to path atomically with mode 0600, first backing up a version 1 file to path+".v1". func Save(path string, s *State) error { if err := backupFirstVersion(path); err != nil { return err diff --git a/internal/watch/watch.go b/internal/watch/watch.go index fb63f80..4e85da0 100644 --- a/internal/watch/watch.go +++ b/internal/watch/watch.go @@ -23,15 +23,12 @@ type Intervals struct { Refill time.Duration } -// New returns a Watcher of the file at path. func New(path string, intervals Intervals, pass func(context.Context)) *Watcher { return &Watcher{path: path, intervals: intervals, pass: pass} } -// Run blocks until ctx is done. A missing file is not an error: the pass reports it, and the -// file triggers a pass once it appears. One goroutine does everything. A tick that fires during a -// pass waits in its ticker, which buffers one tick, so each source yields at most one following -// pass. +// Run blocks until ctx is done. A missing file is not an error, and a tick that fires during a +// pass yields at most one following pass. func (w *Watcher) Run(ctx context.Context) { last, _ := w.modTime() w.pass(ctx) diff --git a/internal/watch/watch_test.go b/internal/watch/watch_test.go index 7404ecc..bbcd6bd 100644 --- a/internal/watch/watch_test.go +++ b/internal/watch/watch_test.go @@ -49,7 +49,6 @@ func start(t *testing.T, create bool, refill time.Duration, pass func(h *harness return h } -// write creates or touches the file and gives it a distinct modification time. func (h *harness) write(t *testing.T, n int64) { t.Helper() if err := os.WriteFile(h.path, []byte("a: [b]\n"), 0o600); err != nil { diff --git a/internal/xray/config.go b/internal/xray/config.go index dfc03b3..799c18a 100644 --- a/internal/xray/config.go +++ b/internal/xray/config.go @@ -21,29 +21,21 @@ import ( "github.com/mrcsin/usher/internal/config" ) -// inbound is one managed inbound of the Xray server config. Raw is the inbound object with the -// config aliases normalized to their canonical names. A non-nil Err means the inbound failed -// validation. Unless hintsOnly is set, it must not be used; with hintsOnly the drift check and -// the revoke of foreign users still use it. +// inbound with Err set failed validation; only a hintsOnly one still serves checkDrift and revoke. type inbound struct { - Tag string - Protocol string // lower case: vless, hysteria, vmess or trojan - Port int - Network string // raw, ws, grpc, httpupgrade, xhttp or hysteria - Security string // none, tls or reality - // ServerName is the name the client sends: x-usher.sni, else the first usable Reality - // serverNames entry or the tlsSettings serverName. It is empty when security is none. + Tag string + Protocol string // lower case: vless, hysteria, vmess or trojan + Port int + Network string // raw, ws, grpc, httpupgrade, xhttp or hysteria + Security string // none, tls or reality ServerName string Hints hints Raw map[string]any Err error - // hintsOnly is true when Err comes from the x-usher block alone: the rest of the inbound - // passed validation, so checkDrift still compares it with the running inbound. - hintsOnly bool + hintsOnly bool } -// hints is the x-usher block of an inbound with the defaults that do not depend on usher's -// environment applied. An empty Address means USHER_HOST. +// hints is the x-usher block; an empty Address means USHER_HOST. type hints struct { Address string Port int @@ -54,9 +46,7 @@ type hints struct { } const ( - // trustCA makes the client verify the server certificate against its CA roots. - trustCA = "ca" - // trustPin makes the client also pin the leaf certificate. + trustCA = "ca" trustPin = "pin" defaultFingerprint = "chrome" @@ -72,8 +62,7 @@ const ( ) var ( - // realityFingerprints are the uTLS fingerprints that carry X25519MLKEM768, which the Reality - // server requires. + // The Reality server requires X25519MLKEM768, which only these uTLS fingerprints carry. realityFingerprints = []string{"chrome", "firefox", "safari"} networkAliases = map[string]string{"tcp": "raw", "splithttp": "xhttp", "websocket": "ws"} @@ -84,8 +73,7 @@ var ( sha256Pattern = regexp.MustCompile(`^[0-9a-fA-F]{64}$`) ) -// discardLog drops Xray's global log output. Build prints deprecation warnings, and usher runs no -// Xray instance whose log they belong to. +// discardLog drops Xray's global log; Build prints deprecation warnings into it. type discardLog struct{} func (discardLog) Handle(log.Message) {} @@ -96,10 +84,7 @@ func silenceXrayLog() { silenceOnce.Do(func() { log.RegisterHandler(discardLog{}) }) } -// readConfig reads the Xray server config at path and returns its managed inbounds in file order. -// Inbounds of other protocols and inbounds without a tag are skipped. An inbound that fails -// validation is returned with Err set, so the caller can keep its files. An error means the file -// as a whole is unusable. +// readConfig returns invalid inbounds with Err set; an error means the file is unusable. func readConfig(path string) ([]inbound, error) { silenceXrayLog() f, err := os.Open(path) @@ -143,7 +128,6 @@ func readConfig(path string) ([]inbound, error) { return inbounds, nil } -// decodeInbound decodes an inbound object as Xray's config reader does. func decodeInbound(raw map[string]any) (conf.InboundDetourConfig, error) { var detour conf.InboundDetourConfig encoded, err := json.Marshal(raw) @@ -167,8 +151,7 @@ func decodeObject(data []byte) (map[string]any, error) { return obj, nil } -// validate normalizes in.Raw, checks it against what the backend supports and fills the derived -// fields. Every error names the field and never quotes a value. +// Every validate error names the field and never quotes a value. func (in *inbound) validate() error { fail := func(field, reason string) error { return fmt.Errorf("inbound %q: %s: %s", in.Tag, field, reason) @@ -215,8 +198,7 @@ func (in *inbound) validate() error { return fail("streamSettings.realitySettings."+field, reason) } } - // The decode catches field type errors that the checks above do not look at. Xray's decoders - // can quote the value they reject, so the error names the Go field of a type mismatch at most. + // Xray's decoders can quote the rejected value, so the error names at most the Go field. if _, err := decodeInbound(in.Raw); err != nil { var typeErr *json.UnmarshalTypeError if errors.As(err, &typeErr) { @@ -225,7 +207,7 @@ func (in *inbound) validate() error { return fail("inbound", "does not decode as an Xray inbound") } - // Last, so that a hint error is the only reason left and the drift check can still run. + // Last, so a hint error alone leaves the inbound usable for the drift check. if field, reason := in.readHints(stream); field != "" { in.hintsOnly = true return fail(field, reason) @@ -259,8 +241,7 @@ func (in *inbound) settings() (map[string]any, error) { return settings, nil } -// checkUsersEmpty folds the Hysteria alias users into clients and requires the list to be empty, -// because usher owns every user of the inbound. +// checkUsersEmpty requires an empty user list: usher owns every user. func (in *inbound) checkUsersEmpty(settings map[string]any) error { if in.Protocol == protocolHysteria { if users, ok := settings["users"]; ok { @@ -282,9 +263,7 @@ func isEmptyList(v any) bool { return v == nil || (ok && len(list) == 0) } -// normalizeStream rewrites the aliases of streamSettings to their canonical names and fills in -// Network and Security. It returns the streamSettings object, a detached empty one when the inbound -// has none. +// normalizeStream returns streamSettings, a detached empty object when the inbound has none. func (in *inbound) normalizeStream() (map[string]any, error) { v, present := in.Raw["streamSettings"] stream, ok := v.(map[string]any) @@ -292,8 +271,7 @@ func (in *inbound) normalizeStream() (map[string]any, error) { return nil, errors.New("want an object") } if stream == nil { - // Not stored in Raw: an inbound without streamSettings must build the way Xray builds - // the original file. + // Not stored in Raw, so the inbound builds as Xray builds the original file. stream = map[string]any{} } @@ -392,7 +370,6 @@ func isVersion2(v any) bool { return ok && n.String() == "2" } -// checkMasks allows only salamander without packetSize in the udp list, and quicParams. func (in *inbound) checkMasks(stream map[string]any) (field, reason string) { mask, present := stream["finalmask"] if !present { @@ -442,7 +419,6 @@ func (in *inbound) checkXHTTP(stream map[string]any) (field, reason string) { return "", "" } -// xUsher mirrors the x-usher block. Pointers tell an absent field from a zero one. type xUsher struct { Address string `json:"address"` Port *int `json:"port"` @@ -453,7 +429,6 @@ type xUsher struct { HopPorts string `json:"hop_ports"` } -// readHints decodes x-usher, checks it against the inbound and fills Hints and ServerName. func (in *inbound) readHints(stream map[string]any) (field, reason string) { var hint xUsher if block, ok := in.Raw["x-usher"]; ok { @@ -532,9 +507,7 @@ func (in *inbound) readHints(stream map[string]any) (field, reason string) { return "", "" } -// serverName resolves the name the client sends. An explicit x-usher.sni wins over the server -// config on TLS. Reality looks the name up in serverNames by exact match, so there the override -// must be one of the entries; it is how an operator picks one of several names. +// serverName: on Reality the x-usher.sni override must be one of serverNames, matched exactly. func (in *inbound) serverName(stream map[string]any, override string) (name, field, reason string) { switch in.Security { case securityTLS: @@ -583,7 +556,6 @@ func validHopPorts(s string) bool { return from >= 1 && from < to && to <= 65535 } -// object returns v as a JSON object, or nil when it is not one. func object(v any) map[string]any { m, _ := v.(map[string]any) return m diff --git a/internal/xray/config_test.go b/internal/xray/config_test.go index 6f381b5..84360e8 100644 --- a/internal/xray/config_test.go +++ b/internal/xray/config_test.go @@ -147,7 +147,6 @@ func baseInbound(t *testing.T, kind string) map[string]any { return m } -// setPath sets the dotted path in m; a nil value deletes the key. func setPath(m map[string]any, path string, value any) { parts := strings.Split(path, ".") for _, p := range parts[:len(parts)-1] { @@ -191,7 +190,7 @@ func TestReadConfigRejects(t *testing.T) { name string kind string set map[string]any - want string // error substring, a field name + want string }{ {"port range", "vless-tls", map[string]any{"port": "443-445"}, "port: want a single integer"}, {"port list", "vless-tls", map[string]any{"port": "443,444"}, "port:"}, @@ -467,7 +466,6 @@ func TestDeployExampleConfig(t *testing.T) { t.Errorf("shipped Reality inbound error = %v, want one about privateKey", inbounds[0].Err) } - // With the placeholder key replaced, every inbound is valid. data, err := os.ReadFile(path) if err != nil { t.Fatal(err) diff --git a/internal/xray/drift.go b/internal/xray/drift.go index 73d2373..055d850 100644 --- a/internal/xray/drift.go +++ b/internal/xray/drift.go @@ -17,9 +17,7 @@ import ( "google.golang.org/protobuf/proto" ) -// driftError is the reason a running inbound differs from the file. unusable is true when the -// running inbound may not take the file's users: it is missing, runs another protocol or could not -// be compared. +// driftError is unusable when the running inbound is missing, of another protocol or not comparable. type driftError struct { reason string unusable bool @@ -27,9 +25,7 @@ type driftError struct { func (e *driftError) Error() string { return e.reason } -// buildInbound builds the inbound object the way Xray does, without the certificate entries that -// name a file, which Xray reads from its own filesystem. Xray quotes the keys and short ids it -// rejects, so the error of its decode or Build never reaches a reason. +// buildInbound drops file certificates; its decode errors quote keys, so they never reach a reason. func buildInbound(raw map[string]any) (*core.InboundHandlerConfig, error) { detour, err := decodeInbound(withoutFileCertificates(raw)) if err != nil { @@ -42,8 +38,6 @@ func buildInbound(raw map[string]any) (*core.InboundHandlerConfig, error) { return built, nil } -// isManagedProxyType reports whether proxyType is the proxy settings type of a protocol the -// backend manages. func isManagedProxyType(proxyType string) bool { for _, spec := range protocols { if spec.proxyType == proxyType { @@ -53,15 +47,8 @@ func isManagedProxyType(proxyType string) bool { return false } -// checkDrift compares every inbound of the file that passed structural validation, hint errors -// included, with the inbound of the same tag in the running Xray. The result maps a tag to the -// reason its running inbound differs from the file or is missing; a tag without an entry matches. -// A mismatch means Xray runs an older file, so the client files for the tag are not trustworthy -// until Xray restarts. -// -// Certificate entries that name a file are left out on both sides: Xray reloads those files -// itself, so a changed file is no drift. A reason names the differing parts and never a -// value. Running inbounds of a managed protocol that the file does not define are logged. +// checkDrift maps a tag to why its running inbound differs from the file; a mismatch means the +// client files are untrustworthy until Xray restarts. A reason never quotes a value. func checkDrift(ctx context.Context, client command.HandlerServiceClient, log *slog.Logger, inbounds []inbound) (map[string]error, error) { resp, err := client.ListInbounds(ctx, &command.ListInboundsRequest{}) @@ -122,9 +109,6 @@ func checkDrift(ctx context.Context, client command.HandlerServiceClient, log *s const partProxyType = "proxy type" -// differingParts returns the names of the parts of the file inbound that differ from the running -// one: "proxy settings", "port", "stream settings" and "receiver settings". The caller has -// checked that both run the same proxy type. func differingParts(in inbound, live *core.InboundHandlerConfig) ([]string, error) { built, err := buildInbound(in.Raw) if err != nil { @@ -179,7 +163,6 @@ func equalMessages(a, b *serial.TypedMessage) (bool, error) { return proto.Equal(left, right), nil } -// receiver unpacks the receiver settings into a fresh message the caller may modify. func receiver(handler *core.InboundHandlerConfig) (*proxyman.ReceiverConfig, error) { instance, err := handler.GetReceiverSettings().GetInstance() if err != nil { @@ -194,8 +177,6 @@ func receiver(handler *core.InboundHandlerConfig) (*proxyman.ReceiverConfig, err var tlsConfigType = string((&tls.Config{}).ProtoReflect().Descriptor().FullName()) -// dropPathCertificates removes the certificates that Xray loads from a file from the TLS settings -// of stream. func dropPathCertificates(stream *internet.StreamConfig) error { for i, settings := range stream.GetSecuritySettings() { if settings.GetType() != tlsConfigType { @@ -214,8 +195,6 @@ func dropPathCertificates(stream *internet.StreamConfig) error { return nil } -// withoutFileCertificates returns a copy of an inbound object without the tlsSettings certificate -// entries that name a file. func withoutFileCertificates(raw map[string]any) map[string]any { clone := deepCopy(raw).(map[string]any) tlsSettings := object(object(clone["streamSettings"])["tlsSettings"]) diff --git a/internal/xray/drift_test.go b/internal/xray/drift_test.go index da90f60..1bfca0f 100644 --- a/internal/xray/drift_test.go +++ b/internal/xray/drift_test.go @@ -18,7 +18,6 @@ import ( xjson "github.com/xtls/xray-core/infra/conf/json" ) -// driftConfig holds the parts of the test server config that the cases change. type driftConfig struct { dir string realityPort int @@ -27,8 +26,8 @@ type driftConfig struct { realityProto string certContent string hysteriaCerts string - realityExtra string // extra members of the reality inbound, each followed by a comma - extra string // extra inbound objects, each followed by a comma + realityExtra string + extra string } func newDriftConfig(dir string) driftConfig { @@ -43,7 +42,6 @@ func newDriftConfig(dir string) driftConfig { } } -// testKey returns a valid Reality private key: 32 bytes of fill in base64url. func testKey(fill byte) string { return base64.RawURLEncoding.EncodeToString(bytes.Repeat([]byte{fill}, 32)) } @@ -85,8 +83,7 @@ func (c driftConfig) write(t *testing.T) string { return path } -// runningFrom builds every inbound of the file at path the way Xray does at start: the original -// file goes through Xray's own reader and config types, with none of usher's normalization. +// runningFrom builds the inbounds through Xray's own reader, without usher's normalization. func runningFrom(t *testing.T, path string) []*core.InboundHandlerConfig { t.Helper() f, err := os.Open(path) @@ -114,7 +111,7 @@ func TestCheckDrift(t *testing.T) { name string change func(c *driftConfig) running func(running []*core.InboundHandlerConfig) []*core.InboundHandlerConfig - want map[string][]string // tag to the substrings of its reason + want map[string][]string }{ {name: "match"}, {name: "sniffing change", change: func(c *driftConfig) { c.realityExtra = `"sniffing": {"enabled": true},` }, @@ -254,8 +251,7 @@ func TestCheckDriftListError(t *testing.T) { } } -// TestCheckDriftMatchesXrayBuild reads the original file with usher and with Xray's own types. A -// file without streamSettings and a file written in aliases must show no drift. +// TestCheckDriftMatchesXrayBuild: a file without streamSettings and a file in aliases show no drift. func TestCheckDriftMatchesXrayBuild(t *testing.T) { plain := newDriftConfig(t.TempDir()) plain.extra = `{"tag": "plain", "protocol": "vless", "port": 9001, @@ -290,8 +286,7 @@ func TestCheckDriftMatchesXrayBuild(t *testing.T) { } } -// TestCheckDriftDoesNotQuoteBuildErrors gives the file a short id that Xray rejects by quoting it. -// The reason must not carry the value. +// TestCheckDriftDoesNotQuoteBuildErrors: Xray quotes a bad short id; the reason must not. func TestCheckDriftDoesNotQuoteBuildErrors(t *testing.T) { const bad = "zzSHORTID-SECRET" base := newDriftConfig(t.TempDir()) diff --git a/internal/xray/enroll.go b/internal/xray/enroll.go index 809d4c8..a268a77 100644 --- a/internal/xray/enroll.go +++ b/internal/xray/enroll.go @@ -11,12 +11,9 @@ import ( "github.com/mrcsin/usher/internal/state" ) -// passwordBytes is the entropy of a generated Trojan or Hysteria2 password. const passwordBytes = 32 -// enroll makes st hold a credential of the kind the inbound's protocol uses for every user in -// users. It creates the missing ones and never changes an existing entry; an existing entry of the -// wrong kind is an error. It reports whether it created any. On error st is left as it was. +// enroll never changes an existing entry; one of the wrong kind is an error and leaves st as it was. func enroll(st *state.State, in inbound, users []string) (bool, error) { spec := protocols[in.Protocol] created := make(map[string]state.XrayEntry) diff --git a/internal/xray/mirror.go b/internal/xray/mirror.go index 0aa154c..ded3f1f 100644 --- a/internal/xray/mirror.go +++ b/internal/xray/mirror.go @@ -16,14 +16,12 @@ import ( ) const ( - // hopInterval is the udphop interval in seconds; libxray omits it from a link at this value. + // libxray omits the udphop interval from a link at this value. hopInterval = 30 hopMode = "intervalLocal,intervalRemote" ) -// clientOutbound returns the Xray client outbound for one user on the inbound. It copies only what -// the client needs from the server config and rebuilds the TLS and Reality blocks, so nothing -// secret or server-only reaches the result. host is the address clients dial unless x-usher sets one. +// clientOutbound rebuilds the TLS and Reality blocks, so nothing secret or server-only reaches it. func clientOutbound(in inbound, host string, entry state.XrayEntry) (map[string]any, error) { stream, err := clientStream(in, entry) if err != nil { @@ -79,7 +77,6 @@ func clientStream(in inbound, entry state.XrayEntry) (map[string]any, error) { return stream, nil } -// pick returns a copy of the listed keys of m that are present. func pick(m map[string]any, keys ...string) map[string]any { out := map[string]any{} for _, k := range keys { @@ -90,8 +87,7 @@ func pick(m map[string]any, keys ...string) map[string]any { return out } -// xhttpSettings keeps path, host and mode at the top. A server extra replaces every other field on -// the server, so it is mirrored verbatim; without one the other fields move into extra. +// xhttpSettings: a server extra replaces every other field there, so it is mirrored verbatim. func xhttpSettings(server map[string]any) map[string]any { out := pick(server, "path", "host", "mode") if extra := object(server["extra"]); len(extra) > 0 { @@ -110,8 +106,7 @@ func xhttpSettings(server map[string]any) map[string]any { return out } -// hysteriaMask returns the client finalmask: the salamander password of the server, then a udphop -// entry when x-usher.hop_ports is set. libxray reads a link only in that order. +// hysteriaMask orders salamander before udphop: libxray reads a link only in that order. func hysteriaMask(in inbound, server map[string]any) map[string]any { var udp []any serverUDP, _ := object(server["finalmask"])["udp"].([]any) @@ -140,8 +135,6 @@ func hysteriaMask(in inbound, server map[string]any) map[string]any { return map[string]any{"udp": udp} } -// clientTLS rebuilds the tlsSettings block: the name, the ALPN list, the fingerprint and, for -// trust "pin", the pinned leaf hash. func clientTLS(in inbound, server map[string]any) (map[string]any, error) { tls := map[string]any{"serverName": in.ServerName, "fingerprint": in.Hints.Fingerprint} if alpn, ok := server["alpn"]; ok { @@ -161,8 +154,6 @@ func clientTLS(in inbound, server map[string]any) (map[string]any, error) { return tls, nil } -// inlineCertificatePin returns the lower case hex SHA-256 of the first certificate that an inline -// certificates entry carries. func inlineCertificatePin(tls map[string]any) (string, error) { certs, _ := tls["certificates"].([]any) for _, c := range certs { @@ -184,9 +175,7 @@ func inlineCertificatePin(tls map[string]any) (string, error) { return "", errors.New("no inline certificate") } -// realityKeys decodes the privateKey and the optional mldsa65Seed of a realitySettings object. A -// problem is returned as the field and a reason that never quotes a value, because Xray's own -// errors do. seed is nil without an mldsa65Seed. +// realityKeys returns a reason that never quotes a value, unlike Xray's own errors. func realityKeys(server map[string]any) (key *ecdh.PrivateKey, seed []byte, field, reason string) { private, _ := server["privateKey"].(string) raw, err := base64.RawURLEncoding.DecodeString(private) @@ -211,8 +200,6 @@ func realityKeys(server map[string]any) (key *ecdh.PrivateKey, seed []byte, fiel return key, seed, "", "" } -// clientReality rebuilds the realitySettings block from the server's: the public key of its -// privateKey, its first short id and, with an mldsa65Seed, the matching verify key. func clientReality(in inbound, server map[string]any) (map[string]any, error) { key, seed, field, reason := realityKeys(server) if field != "" { @@ -237,7 +224,7 @@ func clientReality(in inbound, server map[string]any) (map[string]any, error) { return reality, nil } -// deepCopy copies the maps and lists of a decoded JSON value, so a result never aliases inbound.Raw. +// deepCopy keeps a result from aliasing inbound.Raw. func deepCopy(v any) any { switch v := v.(type) { case map[string]any: diff --git a/internal/xray/mirror_test.go b/internal/xray/mirror_test.go index bbc8829..d4eb9dc 100644 --- a/internal/xray/mirror_test.go +++ b/internal/xray/mirror_test.go @@ -158,7 +158,6 @@ func TestClientOutboundShapes(t *testing.T) { } } -// assertBuilds checks that Xray accepts the outbound as a client config. func assertBuilds(t *testing.T, outbound map[string]any) { t.Helper() encoded, err := json.Marshal(outbound) @@ -229,7 +228,7 @@ func TestClientOutboundTrust(t *testing.T) { name string kind string set map[string]any - want string // expected pinnedPeerCertSha256, empty for none + want string fails string }{ {"ca by default", "vless-tls", nil, "", ""}, @@ -521,8 +520,7 @@ func TestClientOutboundOverrides(t *testing.T) { }) } -// TestClientOutboundLeaksNothing feeds an inbound full of server-only material and checks that -// none of it reaches the outbound, the client files or the log. +// TestClientOutboundLeaksNothing: server-only material never reaches the outbound, files or log. func TestClientOutboundLeaksNothing(t *testing.T) { cert, _ := selfSignedPEM(t) var seed [mldsa65.SeedSize]byte diff --git a/internal/xray/protocol.go b/internal/xray/protocol.go index 2cad5b3..0e79a4a 100644 --- a/internal/xray/protocol.go +++ b/internal/xray/protocol.go @@ -10,26 +10,17 @@ import ( "github.com/mrcsin/usher/internal/state" ) -// vlessVisionFlow is the VLESS flow of a raw inbound behind TLS or Reality. const vlessVisionFlow = "xtls-rprx-vision" -// protocolSpec describes how one inbound protocol holds its users. type protocolSpec struct { - // proxyType is the full name of the proxy settings message that Xray stores for an inbound of - // the protocol. - proxyType string - // usesID is true when the credential is a UUID in XrayEntry.ID, false when it is a password in - // XrayEntry.Password. - usesID bool - // clientSettings adds the credential and the protocol fields of the client outbound settings. + proxyType string + usesID bool clientSettings func(in inbound, e state.XrayEntry, settings map[string]any) account func(in inbound, e state.XrayEntry) proto.Message - // credential returns the part of an account that identifies what the client must present, or - // "" for an account of another type. Two accounts with the same credential are the same user. + // credential is "" for an account of another type; equal credentials are the same user. credential func(account proto.Message) string } -// protocols maps the lower case protocol of an inbound to its spec. var protocols = map[string]protocolSpec{ protocolVLESS: { proxyType: "xray.proxy.vless.inbound.Config", @@ -96,8 +87,6 @@ var protocols = map[string]protocolSpec{ }, } -// vlessFlow returns the flow of the VLESS users of the inbound: vision for raw behind TLS or -// Reality, none otherwise. func vlessFlow(in inbound) string { if in.Network == "raw" && (in.Security == securityTLS || in.Security == securityReality) { return vlessVisionFlow diff --git a/internal/xray/render.go b/internal/xray/render.go index 4cc4ddc..f2da5d9 100644 --- a/internal/xray/render.go +++ b/internal/xray/render.go @@ -13,18 +13,14 @@ import ( ) const ( - // JSONSuffix is the suffix of the client config usher writes for an inbound. JSONSuffix = ".json" - // LinkSuffix is the suffix of the share link usher writes for an inbound. LinkSuffix = ".txt" localSocksPort = 10808 localHTTPPort = 10809 ) -// renderJSON returns the complete client config for the outbound: the outbound plus a local socks -// and http proxy. The outbound must build as an Xray outbound, so a shape Xray itself refuses never -// reaches a client. +// renderJSON fails on an outbound Xray itself refuses to build, so no client gets one. func renderJSON(outbound map[string]any) ([]byte, error) { config := map[string]any{ "log": map[string]any{"loglevel": "warning"}, @@ -58,7 +54,6 @@ func renderJSON(outbound map[string]any) ([]byte, error) { return append(out, '\n'), nil } -// renderLink returns the share link of the client config, one line with a trailing newline. func renderLink(clientJSON []byte) ([]byte, error) { link, err := share.ConvertXrayJsonToShareLinks(clientJSON) if err != nil { @@ -67,9 +62,7 @@ func renderLink(clientJSON []byte) ([]byte, error) { return []byte(link + "\n"), nil } -// renderFiles returns the client files of one user on the inbound, keyed by clients.Path: always -// the .json, and the .txt unless no client reads its link. A link libxray refuses is a -// defect of usher, which builds only shapes libxray knows; it is logged and the .json stays. +// renderFiles keeps the .json when libxray refuses the link: that is a usher defect, logged. func renderFiles(log *slog.Logger, in inbound, host, user string, entry state.XrayEntry) (map[string][]byte, error) { outbound, err := clientOutbound(in, host, entry) if err != nil { diff --git a/internal/xray/render_test.go b/internal/xray/render_test.go index 92d6171..6c435b1 100644 --- a/internal/xray/render_test.go +++ b/internal/xray/render_test.go @@ -71,7 +71,6 @@ func TestGolden(t *testing.T) { t.Fatalf("link parsed back to %d outbounds, err %v, want 1", len(round.OutboundConfigs), err) } if in.Hints.HopPorts != "" { - // A hop link names the range in its authority and parses back to its first port. first, _, _ := strings.Cut(in.Hints.HopPorts, "-") parsed = withPort(t, clientJSON, first) } @@ -104,9 +103,7 @@ func withPort(t *testing.T, clientJSON []byte, port string) conf.Config { return cfg } -// assertSameOutbound builds both outbounds and requires the same proxy settings (address, port, id -// or password, flow) and the same stream settings (transport, TLS or Reality block, xhttp extra, -// salamander, hop). The link carries no tag. +// assertSameOutbound requires the link and the JSON to build the same proxy and stream settings. func assertSameOutbound(t *testing.T, fromJSON, fromLink conf.OutboundDetourConfig) { t.Helper() want, err := fromJSON.Build() @@ -129,8 +126,6 @@ func assertSameOutbound(t *testing.T, fromJSON, fromLink conf.OutboundDetourConf } } -// checkGolden compares the file for suffix with testdata/golden/, or checks that it -// is absent when want is false. func checkGolden(t *testing.T, files map[string][]byte, tag, suffix string, want bool) { t.Helper() got, present := files[clients.Path("alice", tag, suffix)] diff --git a/internal/xray/session.go b/internal/xray/session.go index 80075ff..03bee41 100644 --- a/internal/xray/session.go +++ b/internal/xray/session.go @@ -17,32 +17,23 @@ import ( "github.com/mrcsin/usher/internal/state" ) -// callTimeout bounds each call to Xray. const callTimeout = 10 * time.Second -// Dial opens a connection to the Xray API. It returns the client and the function that closes the -// connection. type Dial func() (command.HandlerServiceClient, func(), error) -// Session is one pass's view of Xray: the managed inbounds of the config file and the drift of -// each against the running Xray. type Session struct { client command.HandlerServiceClient closeConn func() inbounds []inbound byTag map[string]inbound drift map[string]error - // broken maps a tag whose accounts and client files cannot be built this pass to the reason. - // Apply only revokes users on such a tag. + // broken maps a tag whose accounts or client files cannot be built to the reason. broken map[string]error host string log *slog.Logger } -// Open reads the Xray config at configPath, connects to the API and checks every inbound of the -// file against the running Xray. host is the address clients dial unless an inbound sets one. When -// socketPath is a stale socket of a killed Xray, Open removes it and fails, so the pass reports Xray -// down and the restart policy can start Xray again. +// Open removes a stale socket left by a killed Xray and fails the pass. func Open(ctx context.Context, dial Dial, socketPath, configPath, host string, log *slog.Logger) (*Session, error) { inbounds, err := readConfig(configPath) if err != nil { @@ -83,9 +74,7 @@ func Open(ctx context.Context, dial Dial, socketPath, configPath, host string, l }, nil } -// Interfaces returns the tag of every managed inbound of the file, valid or not, so a tag that -// fails keeps its client files. A tag the file defines twice appears once; its entry carries the -// error. +// Interfaces includes invalid tags, so their client files are kept. func (s *Session) Interfaces() []string { tags := make([]string, 0, len(s.byTag)) for _, in := range s.inbounds { @@ -96,12 +85,8 @@ func (s *Session) Interfaces() []string { return tags } -// Prepare enrolls the users on the inbound in st and renders their client files. When the inbound -// failed structural validation, Prepare returns the error and the pass skips Apply, so usher -// touches no user of that tag. When only the x-usher block failed, an enrolled credential does not -// fit the inbound or a file cannot be rendered, Prepare keeps the reason and returns no error, and -// Apply then still revokes the users who left and returns the reason, which keeps the old client -// files. +// Prepare returns structural errors; for a hint, enroll or render error it returns nil and +// Apply only revokes. func (s *Session) Prepare(st *state.State, name string, users []string) (map[string][]byte, bool, error) { in := s.byTag[name] if in.Err != nil { @@ -128,11 +113,8 @@ func (s *Session) Prepare(st *state.State, name string, users []string) (map[str return files, changed, nil } -// Apply makes the inbound hold exactly the users, then returns the error of the tag, if any: the -// reason Prepare kept, else the drift. Users are reconciled even under most drift, so a revoked -// user loses access. When the running inbound is missing or runs another protocol, Apply changes -// nothing, because an account of the wrong type crashes Xray. It also changes nothing when the -// inbound failed structural validation. When Prepare kept a reason, Apply only removes users. +// Apply never touches a tag whose running inbound is missing or runs another protocol: an +// account of the wrong type crashes Xray. func (s *Session) Apply(ctx context.Context, st *state.State, name string, users []string) error { if in := s.byTag[name]; in.Err != nil && !in.hintsOnly { return in.Err @@ -148,19 +130,15 @@ func (s *Session) Apply(ctx context.Context, st *state.State, name string, users return errors.Join(s.reconcile(ctx, st, s.byTag[name], users), drift) } -// Close closes the connection to Xray. func (s *Session) Close() { s.closeConn() } -// userChanges is what reconcile did to an inbound, by email. type userChanges struct { added, removed, updated []string } -// reconcile lists the users of the inbound, removes foreign and changed ones, adds missing and -// changed ones, and logs the difference. It tries every operation and joins the errors, so one -// failure does not hold back a removal; the next pass converges from the listed state. +// reconcile joins the errors, so one failure never blocks a removal. func (s *Session) reconcile(ctx context.Context, st *state.State, in inbound, users []string) error { desired := make(map[string]*serial.TypedMessage, len(users)) credentials := make(map[string]string, len(users)) @@ -214,7 +192,7 @@ func (s *Session) reconcile(ctx context.Context, st *state.State, in inbound, us changes.added = append(changes.added, email) } } - // A changed credential whose new account was refused left the user without access. + // A changed credential whose new account was refused leaves the user without access. changes.removed = append(changes.removed, slices.Sorted(maps.Keys(replaced))...) s.logChanges(in.Tag, changes) if err := errors.Join(errs...); err != nil { @@ -223,8 +201,7 @@ func (s *Session) reconcile(ctx context.Context, st *state.State, in inbound, us return nil } -// revoke removes every user of the inbound whose email is not one of users, and touches no other -// account. It serves an inbound whose accounts cannot be built. +// revoke only removes users, so it is safe on an inbound whose accounts cannot be built. func (s *Session) revoke(ctx context.Context, tag string, users []string) error { keep := make(map[string]bool, len(users)) for _, user := range users { @@ -282,7 +259,6 @@ func (s *Session) listUsers(ctx context.Context, tag string) ([]*protocol.User, return resp.GetUsers(), nil } -// liveCredentials maps the email of every user of the inbound to the credential of its account. func (s *Session) liveCredentials(ctx context.Context, in inbound) (map[string]string, error) { users, err := s.listUsers(ctx, in.Tag) if err != nil { diff --git a/internal/xray/session_test.go b/internal/xray/session_test.go index 22b46b7..f36e90b 100644 --- a/internal/xray/session_test.go +++ b/internal/xray/session_test.go @@ -24,15 +24,14 @@ import ( "github.com/mrcsin/usher/internal/state" ) -// userService is a fake Xray HandlerService that keeps the users of each inbound in memory. type userService struct { command.HandlerServiceClient running []*core.InboundHandlerConfig - users map[string][]*protocol.User // tag to users - failAdd map[string]bool // emails whose AddUser fails - failRemove map[string]bool // emails whose RemoveUser fails - listErr error // answer of GetInboundUsers - inboundErr error // answer of ListInbounds + users map[string][]*protocol.User + failAdd map[string]bool + failRemove map[string]bool + listErr error + inboundErr error alters int } @@ -146,7 +145,7 @@ func newState() *state.State { func idEntry() state.XrayEntry { return state.XrayEntry{ID: uuid.New()} } -// badInbound is an inbound that fails validation (clients are not empty) and still builds in Xray. +// badInbound fails validation (clients are not empty) and still builds in Xray. const badInbound = `{"tag": "bad", "protocol": "vless", "port": 9000, "settings": {"clients": [{"id": "5783a3e7-e373-51cd-8642-c83782b807c5"}], "decryption": "none"}},` @@ -222,9 +221,8 @@ func TestSessionPrepareHysteriaPinHasNoLink(t *testing.T) { } } -// TestSessionBrokenInboundStillRevokes covers the three ways Prepare keeps a reason instead of -// failing: an x-usher hint error, a credential of the wrong kind and a file that cannot be -// rendered. The drift check finds the running inbound usable in each, so Apply removes the users who left. +// TestSessionBrokenInboundStillRevokes: Prepare keeps a reason for a hint, enroll or render error, +// and Apply still removes the users who left. func TestSessionBrokenInboundStillRevokes(t *testing.T) { notPEM := `{"tag": "pinned", "protocol": "vless", "port": 9003, "x-usher": {"trust": "pin"}, @@ -278,9 +276,8 @@ func TestSessionBrokenInboundStillRevokes(t *testing.T) { } } -// TestSessionApplyGuards: Apply changes no account when the inbound failed structural validation -// or when nothing proves the running inbound takes the file's accounts. A hint error must not -// skip the drift check. The running Xray holds the base config; the file holds the change. +// TestSessionApplyGuards: Apply changes no account unless the running inbound provably takes the +// file's accounts; a hint error must not skip the drift check. func TestSessionApplyGuards(t *testing.T) { badTag := `{"tag": "bad tag", "protocol": "vless", "port": 9005, "settings": {"clients": [], "decryption": "none"}},` @@ -291,8 +288,8 @@ func TestSessionApplyGuards(t *testing.T) { name string change func(c *driftConfig) tag string - prepareFails bool // Prepare returns the validation error - wantApplyError string // "" accepts any error + prepareFails bool + wantApplyError string }{ {"static clients", func(c *driftConfig) { c.extra = badInbound }, "bad", true, ""}, {"invalid tag", func(c *driftConfig) { c.extra = badTag }, "bad tag", true, ""}, @@ -364,8 +361,8 @@ func TestSessionApply(t *testing.T) { tag string setup func(f sessionFixture, st *state.State) users []string - want func(st *state.State) map[string]string // email to credential - alter int // AlterInbound calls expected + want func(st *state.State) map[string]string + alter int logs []string }{ { @@ -470,7 +467,6 @@ func TestSessionApply(t *testing.T) { func TestSessionApplyUnderDrift(t *testing.T) { f := newSessionFixture(t, nil) - // The file now says a different port than the running Xray. f.cfg.realityPort = 8444 path := f.cfg.write(t) dial := func() (command.HandlerServiceClient, func(), error) { return f.service, func() {}, nil } @@ -495,7 +491,6 @@ func TestSessionApplyUnderDrift(t *testing.T) { } } -// openWithConfig opens a session on the service for the file the config writes. func openWithConfig(t *testing.T, service *userService, cfg driftConfig) *Session { t.Helper() dial := func() (command.HandlerServiceClient, func(), error) { return service, func() {}, nil } diff --git a/internal/xray/socket.go b/internal/xray/socket.go index 993a306..c37b902 100644 --- a/internal/xray/socket.go +++ b/internal/xray/socket.go @@ -10,14 +10,10 @@ import ( "time" ) -// dialTimeout bounds the probe of a socket that has no lock file. const dialTimeout = time.Second -// removeStaleSocket unlinks the API socket when no Xray listens on it. Xray holds an exclusive -// flock on .lock while it listens, so a lock that is free means a killed Xray left the -// socket behind and the next start would fail on it. A failed start removes the lock file, so a -// socket without one is probed instead: a refused connection means nobody listens. A live Xray -// accepts the probe, and its socket stays. It reports whether it removed the socket. +// removeStaleSocket removes the socket only when the flock on .lock is free or, without +// the lock file, the probe gets ECONNREFUSED; a live Xray keeps its socket. func removeStaleSocket(socketPath string) (bool, error) { before, err := os.Lstat(socketPath) if errors.Is(err, fs.ErrNotExist) { @@ -30,8 +26,7 @@ func removeStaleSocket(socketPath string) (bool, error) { return false, fmt.Errorf("xray socket path %s is not a socket", socketPath) } - // Xray creates the lock as root with mode 0644, so usher opens it read-only; flock needs no - // write access. + // Xray creates the lock as root, mode 0644: open it read-only. lock, err := os.Open(socketPath + ".lock") switch { case errors.Is(err, fs.ErrNotExist): @@ -61,8 +56,7 @@ func removeStaleSocket(socketPath string) (bool, error) { return true, nil } -// connectionRefused reports whether a unix socket answers a connection with ECONNREFUSED, which is -// what a socket file without a listener does. Any other outcome counts as live. +// connectionRefused treats any outcome other than ECONNREFUSED as live. func connectionRefused(socketPath string) bool { conn, err := net.DialTimeout("unix", socketPath, dialTimeout) if err == nil { diff --git a/test/e2e/e2e_test.go b/test/e2e/e2e_test.go index 60b97b0..ce4198c 100644 --- a/test/e2e/e2e_test.go +++ b/test/e2e/e2e_test.go @@ -1,8 +1,7 @@ //go:build e2e -// Package e2e drives usher against the published awg-grpc image through deploy/compose.e2e.yml -// and the host amneziawg module. It needs Docker with compose and the module loaded; AGENTS.md, -// Test, runs it. +// Package e2e drives usher against the awg-grpc image through deploy/compose.e2e.yml. It needs +// Docker with compose and the host amneziawg module; AGENTS.md, Test, runs it. package e2e import ( @@ -25,7 +24,6 @@ import ( ) const ( - // repoDir is the repository root relative to this package; go test runs in the package dir. repoDir = "../.." composeFile = "deploy/compose.e2e.yml" iface = "awge2e0" @@ -38,7 +36,6 @@ const ( refillTimeout = 60 * time.Second ) -// The bind-mount directories of the compose project, under scratchRoot. const ( scratchRoot = ".e2e" configDir = "config" @@ -63,16 +60,14 @@ func usherYAML(users ...string) string { return b.String() } -// peerSet maps the public key of a peer, base64, to its allowed IPs. type peerSet map[string]string func TestMain(m *testing.M) { os.Exit(runMain(m)) } -// runMain creates the bind-mount directories as the test user, so dockerd does not create them -// as root and usher, which runs as the test user, can write them. A run that was killed leaves -// the project up, so runMain takes it down before it starts. +// runMain creates the bind mounts as the test user, not root, and first takes down a project +// that a killed run left up. func runMain(m *testing.M) int { flag.Parse() if count := flag.Lookup("test.count").Value.String(); count != "1" { @@ -118,8 +113,7 @@ func printLogs() { fmt.Fprintf(os.Stderr, "docker compose logs (err %v):\n%s\n", err, out) } -// composeCmd builds docker compose on the e2e project, run from the repository root with the -// uid and gid of the test user, so usher can write the bind mounts and open the socket. +// composeCmd runs with the uid and gid of the test user, so usher can write the bind mounts. func composeCmd(args ...string) *exec.Cmd { cmd := exec.Command("docker", append([]string{"compose", "-f", composeFile}, args...)...) cmd.Dir = repoDir @@ -146,7 +140,7 @@ func compose(t *testing.T, args ...string) string { return stdout } -// TestE2E runs its subtests in order; each one starts from the state the previous one left. +// TestE2E runs its subtests in order; each starts from the state the previous one left. func TestE2E(t *testing.T) { steps := []struct { name string @@ -193,10 +187,8 @@ func testEnroll(t *testing.T) { waitPeers(t, deadline, want) } -// testHandshake proves that a copy of the phone's client config completes a handshake with the -// interface from a second container. The config differs from the original in two lines, as in -// awg-grpc's smoke test: the image has no resolvconf for DNS, and a full-tunnel AllowedIPs needs -// sysctl writes a non-privileged container cannot make. +// testHandshake drops DNS (the image has no resolvconf) and narrows AllowedIPs (full tunnel needs +// sysctl writes), as awg-grpc's smoke test does. func testHandshake(t *testing.T) { original, err := os.ReadFile(e2ePath(clientsDir, "phone", iface+".conf")) if err != nil { @@ -293,10 +285,8 @@ func testSwitchOff(t *testing.T) { }) } -// testBrokenFileAndRefill writes a file with a YAML syntax error and, once usher has logged the -// error, restarts the awg service, which empties its interfaces. usher must refill them from the -// last valid file on its refill ticker and keep clients/ as it was. The log line of the refill -// pass proves the restart emptied the kernel: its added list names both users. +// testBrokenFileAndRefill checks that usher refills a restarted awg from the last valid file. +// The added list of the refill log line names both users, which proves the kernel was empty. func testBrokenFileAndRefill(t *testing.T) { defer writeUsherConfig(t, bothUsers) @@ -367,7 +357,6 @@ func writeUsherConfig(t *testing.T, content string) { } } -// checkClientFile reports why clients//.conf is not a mode 0600 file yet. func checkClientFile(user string) error { info, err := os.Stat(e2ePath(clientsDir, user, iface+".conf")) if err != nil { @@ -414,7 +403,6 @@ func publicKey(e state.Entry) string { return base64.StdEncoding.EncodeToString(e.PublicKey()) } -// dumpPeer is one peer line of awg show dump. type dumpPeer struct { allowedIPs string handshake int64 @@ -430,8 +418,7 @@ func (d dumpSet) allowed() peerSet { return set } -// dump reads the peers of the interface from the wrapper container. The preshared key column is -// never kept. +// dump never keeps the preshared key column. func dump() (dumpSet, error) { stdout, stderr, err := run(composeCmd("exec", "-T", "awg", "awg", "show", iface, "dump")) if err != nil { @@ -439,7 +426,6 @@ func dump() (dumpSet, error) { } lines := strings.Split(strings.TrimSpace(stdout), "\n") peers := dumpSet{} - // The first line is the interface itself. for _, line := range lines[1:] { fields := strings.Split(line, "\t") if len(fields) < 5 { diff --git a/test/e2e/xray/client_test.go b/test/e2e/xray/client_test.go index d3daee3..012d31a 100644 --- a/test/e2e/xray/client_test.go +++ b/test/e2e/xray/client_test.go @@ -28,14 +28,12 @@ const ( requestTimeout = 5 * time.Second ) -// proxyHosts maps each user to the address of that user's client container. var proxyHosts = map[string]string{ "alice": "172.30.98.11", "bob": "172.30.98.12", } -// route is one way to reach the target: the client file of a user for an inbound, either the -// rendered .json config or the .txt share link. Each route has its own proxy port on the client. +// route is the .json config or .txt share link of a user for an inbound, with its own proxy port. type route struct { tag string form string @@ -46,7 +44,6 @@ func (r route) String() string { return r.tag + r.formSuffix() } func (r route) formSuffix() string { return "." + r.form } -// routes lists a route for every inbound and form, in the port order of the client config. func routes() []route { var all []route for _, tag := range tags { @@ -67,9 +64,7 @@ func routesOf(tag string) []route { return among } -// writeClientConfig builds the Xray client config of a user from the rendered files of every -// inbound: one outbound and one HTTP proxy inbound per route. The test only adds useSystemCA to -// the TLS settings, so the client trusts SSL_CERT_FILE. +// writeClientConfig only adds useSystemCA, so the client trusts SSL_CERT_FILE. func writeClientConfig(t *testing.T, user string) { t.Helper() var inbounds, outbounds, rules []any @@ -108,7 +103,6 @@ func writeClientConfig(t *testing.T, user string) { } } -// routeOutbound returns the outbound that the rendered file of the route defines. func routeOutbound(user string, r route) (map[string]any, error) { data, err := os.ReadFile(project.path(clientsDir, user, r.tag+r.formSuffix())) if err != nil { @@ -132,7 +126,6 @@ func routeOutbound(user string, r route) (map[string]any, error) { return config.Outbounds[0], nil } -// nested returns the object at the path of keys, or nil when any step is missing. func nested(object map[string]any, keys ...string) map[string]any { for _, key := range keys { next, ok := object[key].(map[string]any) @@ -144,8 +137,6 @@ func nested(object map[string]any, keys ...string) map[string]any { return object } -// waitForClientFiles waits until usher has written the .json and .txt of the user for every -// inbound. func waitForClientFiles(t *testing.T, user string, timeout time.Duration) { t.Helper() waitFor(t, timeout, "client files of "+user, func() error { @@ -163,8 +154,6 @@ func waitForClientFiles(t *testing.T, user string, timeout time.Duration) { }) } -// fetch requests the target page through the proxy of the route and reports why the request did -// not return the page with status 200. func fetch(user string, r route) error { proxy, err := url.Parse("http://" + net.JoinHostPort(proxyHosts[user], strconv.Itoa(r.port))) if err != nil { @@ -192,7 +181,6 @@ func fetch(user string, r route) error { return nil } -// fetchAll fetches through the given routes of a user at once and returns the error of each. func fetchAll(user string, among []route) map[route]error { var mu sync.Mutex var wg sync.WaitGroup @@ -211,7 +199,6 @@ func fetchAll(user string, among []route) map[route]error { return results } -// allWork returns nil when every route of the list returns the page. func allWork(user string, among []route) error { results := fetchAll(user, among) var errs []error @@ -223,7 +210,6 @@ func allWork(user string, among []route) error { return errors.Join(errs...) } -// proxyReachable reports why the proxy port of the route on the user's client is not open. func proxyReachable(user string, r route) error { conn, err := net.DialTimeout("tcp", net.JoinHostPort(proxyHosts[user], strconv.Itoa(r.port)), requestTimeout) if err != nil { @@ -232,7 +218,6 @@ func proxyReachable(user string, r route) error { return conn.Close() } -// proxiesReachable returns nil when the proxy port of every route of the list is open. func proxiesReachable(user string, among []route) error { var errs []error for _, r := range among { @@ -241,9 +226,8 @@ func proxiesReachable(user string, among []route) error { return errors.Join(errs...) } -// noneWorks returns nil when the proxy of every route of the list is reachable and the request -// through it fails. A refused connection to the proxy of a client that is still starting is an -// error, not a failed route. +// noneWorks treats a refused connection to the proxy of a still-starting client as an error, not +// a failed route. func noneWorks(user string, among []route) error { if err := proxiesReachable(user, among); err != nil { return err diff --git a/test/e2e/xray/pki_test.go b/test/e2e/xray/pki_test.go index 053f691..f815f3b 100644 --- a/test/e2e/xray/pki_test.go +++ b/test/e2e/xray/pki_test.go @@ -14,8 +14,7 @@ import ( "time" ) -// authority is a throwaway CA that signs the server leaves. The client containers trust it through -// SSL_CERT_FILE, as they would trust the public CA of a real certificate. +// authority is a throwaway CA that signs the server leaves; clients trust it through SSL_CERT_FILE. type authority struct { cert *x509.Certificate key *ecdsa.PrivateKey @@ -47,13 +46,10 @@ func newAuthority() (*authority, error) { return &authority{cert: cert, key: key, pem: pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: der})}, nil } -// leaf is a certificate and its key, both PEM encoded. type leaf struct { cert, key []byte } -// issue signs a new leaf for the DNS name. Every call yields a different certificate and key from -// the same CA. func (a *authority) issue(name string) (leaf, error) { key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) if err != nil { diff --git a/test/e2e/xray/server_test.go b/test/e2e/xray/server_test.go index 0db157f..9c0943b 100644 --- a/test/e2e/xray/server_test.go +++ b/test/e2e/xray/server_test.go @@ -13,7 +13,6 @@ import ( "strings" ) -// The inbounds of the server config; usher.yml names them as interfaces. const ( tagHysteria = "hy2" tagReality = "vless-reality" @@ -21,7 +20,7 @@ const ( tagWS = "vless-ws" ) -// tags lists the inbounds. The order fixes the proxy port of each route. +// The order of tags fixes the proxy port of each route. var tags = []string{tagHysteria, tagReality, tagXHTTP, tagWS} const ( @@ -33,15 +32,11 @@ const ( driftShortID = "cd34" ) -// Where the files the server config names sit, relative to the config directory that -// Xray mounts at /etc/xray. const ( wsCertFile = "certs/ws.pem" wsKeyFile = "certs/ws.key" ) -// server holds what the generated server config needs: the Reality key and the leaves of the -// inline certificates. type server struct { realityPrivateKey string hysteria, xhttp leaf @@ -62,7 +57,7 @@ func newServer(ca *authority) (*server, error) { return s, nil } -// pemLines splits a PEM block into the line list that Xray takes for an inline certificate. +// pemLines returns the line list Xray takes for an inline certificate. func pemLines(data []byte) []string { return strings.Split(strings.TrimSpace(string(data)), "\n") } @@ -75,7 +70,6 @@ func inlineCertificate(l leaf, serverName string, alpn ...string) map[string]any } } -// config renders the server config with the four inbounds and the given Reality shortId. func (s *server) config(shortID string) ([]byte, error) { usherBlock := map[string]any{"address": serverAddress} inbound := func(tag string, port int, protocol string, settings, stream map[string]any) map[string]any { @@ -154,7 +148,6 @@ func (s *server) config(shortID string) ([]byte, error) { return json.MarshalIndent(config, "", " ") } -// writeConfig replaces config.json in the mounted config directory. func (s *server) writeConfig(dir, shortID string) error { data, err := s.config(shortID) if err != nil { @@ -163,7 +156,6 @@ func (s *server) writeConfig(dir, shortID string) error { return replaceFile(filepath.Join(dir, "config.json"), data) } -// writeWSCertificate replaces the certificate and key files of the ws inbound. func (s *server) writeWSCertificate(dir string) error { if err := os.MkdirAll(filepath.Join(dir, filepath.Dir(wsCertFile)), 0o755); err != nil { return err @@ -174,7 +166,7 @@ func (s *server) writeWSCertificate(dir string) error { return replaceFile(filepath.Join(dir, wsCertFile), s.ws.cert) } -// replaceFile writes data next to path and renames it over path, so no reader sees half a file. +// replaceFile renames a temporary file over path, so no reader sees half a file. func replaceFile(path string, data []byte) error { tmp := path + ".tmp" if err := os.WriteFile(tmp, data, 0o644); err != nil { diff --git a/test/e2e/xray/xray_test.go b/test/e2e/xray/xray_test.go index c144bf7..fe08de6 100644 --- a/test/e2e/xray/xray_test.go +++ b/test/e2e/xray/xray_test.go @@ -19,11 +19,10 @@ import ( ) const ( - // repoDir is the repository root relative to this package; go test runs in the package dir. repoDir = "../../.." composeFile = "deploy/compose.e2e-xray.yml" clientProfile = "client" - // helperImage is the image of the compose file that has a shell, kill and pidof. + // helperImage has a shell, kill and pidof. helperImage = "nginx:1.27-alpine" // The deadlines follow the production constants: 3 s file poll, 500 ms settle, 30 s refill. @@ -33,13 +32,11 @@ const ( restartTimeout = 45 * time.Second killTimeout = 90 * time.Second // refillTick is a little longer than the 30 s refill interval. - refillTick = 35 * time.Second - // failedRounds is the number of consecutive failed rounds that prove a user is off. + refillTick = 35 * time.Second failedRounds = 3 pollInterval = 500 * time.Millisecond ) -// The bind-mount directories of the compose project, under scratchRoot. const ( scratchRoot = ".e2e-xray" configDir = "config" @@ -54,17 +51,15 @@ type composeProject struct{} var ( project composeProject - // ca and srv are the fixtures that TestMain generates before the containers start. - ca *authority - srv *server + ca *authority + srv *server ) func (composeProject) path(elem ...string) string { return filepath.Join(append([]string{repoDir, scratchRoot}, elem...)...) } -// cmd builds docker compose on the project, run from the repository root with the uid and gid of -// the test user, so usher can write the bind mounts and open the socket. +// cmd runs with the uid and gid of the test user, so usher can write the bind mounts. func (composeProject) cmd(args ...string) *exec.Cmd { cmd := exec.Command("docker", append([]string{"compose", "-f", composeFile}, args...)...) cmd.Dir = repoDir @@ -74,7 +69,6 @@ func (composeProject) cmd(args ...string) *exec.Cmd { return cmd } -// compose runs docker compose on the project and returns its stdout; a failure ends the test. func (p composeProject) compose(t *testing.T, args ...string) string { t.Helper() var stdout, stderr bytes.Buffer @@ -104,10 +98,8 @@ func TestMain(m *testing.M) { os.Exit(runSuite(m)) } -// runSuite creates the bind-mount directories as the test user, so dockerd does not create them as -// root and usher, which runs as the test user, can write them. It writes the server config and the -// CA before the containers start. A run that was killed leaves the project up, so it takes the -// project down first. +// runSuite creates the bind mounts as the test user, not root, and first takes down a project +// that a killed run left up. func runSuite(m *testing.M) int { flag.Parse() if count := flag.Lookup("test.count").Value.String(); count != "1" { @@ -164,8 +156,6 @@ func writeFixtures() error { return srv.writeConfig(project.path(xrayDir), realityShortID) } -// waitFor runs check until it returns nil or the timeout passes; then it ends the test with the -// last error. func waitFor(t *testing.T, timeout time.Duration, what string, check func() error) { t.Helper() deadline := time.Now().Add(timeout) @@ -205,7 +195,6 @@ func logsSince(t *testing.T, mark logMark) string { return logs[mark:] } -// waitForLog waits until a line of the usher log after the mark matches the pattern. func waitForLog(t *testing.T, mark logMark, timeout time.Duration, pattern string) { t.Helper() re := regexp.MustCompile(pattern) @@ -225,7 +214,6 @@ func usherYAML(users ...string) string { return b.String() } -// writeUsherConfig replaces config/usher.yml atomically. func writeUsherConfig(t *testing.T, users ...string) { t.Helper() if err := replaceFile(project.path(configDir, "usher.yml"), []byte(usherYAML(users...))); err != nil { @@ -233,8 +221,6 @@ func writeUsherConfig(t *testing.T, users ...string) { } } -// startClient writes the client config of the user from the rendered files and starts or restarts -// the user's container, so no earlier connection survives. func startClient(t *testing.T, user string) { t.Helper() waitForClientFiles(t, user, changeTimeout) @@ -242,8 +228,7 @@ func startClient(t *testing.T, user string) { project.compose(t, "--profile", clientProfile, "up", "-d", "--force-recreate", "client-"+user) } -// assertOffRepeatedly requires several consecutive rounds in which no route of the user works, so -// one transient error cannot stand for a removed user. +// assertOffRepeatedly requires several rounds with no working route, so one error is not a removal. func assertOffRepeatedly(t *testing.T, user string, among []route) { t.Helper() waitFor(t, connectTimeout, "the proxies of the client of "+user, func() error { @@ -257,17 +242,15 @@ func assertOffRepeatedly(t *testing.T, user string, among []route) { } } -// removedLine matches the usher log line that reports the removal of a user from an inbound. func removedLine(tag, user string) string { return fmt.Sprintf(`users changed" interface=%s .*removed="?\[[^\]]*%s/%s[ \]]`, tag, tag, user) } -// addedLine matches the usher log line that reports the addition of a user to an inbound. func addedLine(tag, user string) string { return fmt.Sprintf(`users changed" interface=%s added="?\[[^\]]*%s/%s[ \]]`, tag, tag, user) } -// TestXray runs its subtests in order; each one starts from the state the previous one left. +// TestXray runs its subtests in order; each starts from the state the previous one left. func TestXray(t *testing.T) { steps := []struct { name string @@ -287,8 +270,6 @@ func TestXray(t *testing.T) { } } -// testConnect writes a config of two users, builds a client from the rendered files of each and -// fetches the target page through every .json and .txt. func testConnect(t *testing.T) { waitFor(t, changeTimeout, "the API socket of xray", func() error { _, err := os.Stat(project.path(sockDir, "api.sock")) @@ -332,8 +313,7 @@ func testRestart(t *testing.T) { assertOffRepeatedly(t, "bob", routes()) } -// testHardKill kills the xray process with SIGKILL, which leaves its socket behind. The restart policy starts -// xray again, it cannot bind the stale socket, and usher unlinks the socket at its next tick. +// testHardKill leaves a stale socket; the restarted xray cannot bind it until usher unlinks it. func testHardKill(t *testing.T) { mark := markLogs(t) killXray(t) @@ -346,9 +326,8 @@ func testHardKill(t *testing.T) { assertOffRepeatedly(t, "bob", routes()) } -// killXray sends SIGKILL to the Xray process from a helper container in the PID namespace of the -// xray container. The container exits with status 137, as after a crash or an OOM kill, so the -// restart policy applies. +// killXray sends SIGKILL from a helper container in the PID namespace of xray; exit status 137 +// triggers the restart policy. func killXray(t *testing.T) { t.Helper() id := strings.TrimSpace(project.compose(t, "ps", "-q", "xray")) @@ -362,9 +341,8 @@ func killXray(t *testing.T) { } } -// testDrift changes shortIds in the mounted config without restarting xray. The reality inbound -// must fail with a drift reason at the next tick, the other inbounds must not, and a user removed -// from usher.yml while the drift is present must still be removed from xray. +// testDrift changes shortIds without restarting xray: only the reality inbound reports drift, and +// a removal during the drift still reaches xray. func testDrift(t *testing.T) { mark := markLogs(t) writeUsherConfig(t, "alice", "bob") @@ -412,9 +390,7 @@ func testDrift(t *testing.T) { assertNoFailureSince(t, mark, "the drift was undone") } -// assertNoFailureSince fails the test when usher logged a failed interface, a backend that is down -// or any error after the mark. The caller first makes usher log a change, which proves that a -// pass ran: a pass that succeeds logs nothing else. +// assertNoFailureSince needs a logged change first: a successful pass logs nothing else. func assertNoFailureSince(t *testing.T, mark logMark, what string) { t.Helper() time.Sleep(2 * time.Second) // the pass that logged the change finishes its other inbounds @@ -426,7 +402,6 @@ func assertNoFailureSince(t *testing.T, mark logMark, what string) { } } -// failedInterfaces returns the interface of every "interface failed" line of the log. func failedInterfaces(logs string) []string { var failed []string for _, match := range regexp.MustCompile(`interface failed" backend=xray interface=(\S+)`).FindAllStringSubmatch(logs, -1) {