refactor(bypass): named decision types, consolidated pattern set, remove dead code

Replace boolean bypass logic with bypassDecision enum (decisionBypass/decisionProxy),
consolidate four matcher fields into a single patternSet struct for atomic swaps,
extract classifyPatterns as a pure function, and remove unused matched() method
and slices import. Add 204 lines of tests covering decisions, pattern sets,
and group evaluation logic.
This commit is contained in:
ginuerzh
2026-05-26 20:27:21 +08:00
parent 1c07cf6394
commit 1510f841c3
2 changed files with 380 additions and 105 deletions
+156 -85
View File
@@ -22,7 +22,6 @@ import (
"errors"
"io"
"net"
"slices"
"strings"
"sync"
"time"
@@ -129,19 +128,72 @@ func LoggerOption(logger logger.Logger) Option {
}
}
// bypassDecision represents the outcome of evaluating a bypass rule.
type bypassDecision int
const (
// decisionBypass means the address should connect directly, skipping the proxy chain.
decisionBypass bypassDecision = iota
// decisionProxy means the address should go through the proxy chain.
decisionProxy
)
func (d bypassDecision) String() string {
switch d {
case decisionBypass:
return "bypass"
case decisionProxy:
return "proxy"
default:
return "unknown"
}
}
// patternSet holds classified pattern matchers for a single bypass rule.
// All four matcher types are built together by classifyPatterns and swapped
// atomically under the write lock.
type patternSet struct {
cidr matcher.Matcher
addr matcher.Matcher
wildcard matcher.Matcher
ipRange matcher.Matcher
}
// matchAny reports whether addr matches any pattern in the set,
// trying IP range, address, CIDR, and wildcard matchers in order.
// Returns false if ps is nil.
func (ps *patternSet) matchAny(addr string) bool {
if ps == nil {
return false
}
if ps.ipRange.Match(addr) {
return true
}
if ps.addr.Match(addr) {
return true
}
host, _, _ := net.SplitHostPort(addr)
if host == "" {
host = addr
}
if ip := net.ParseIP(host); ip != nil && ps.cidr.Match(host) {
return true
}
return ps.wildcard.Match(addr)
}
// localBypass is a Bypass that matches addresses against local pattern
// matchers. Patterns are classified into CIDR, wildcard, IP range, and
// exact address matchers. Patterns can be loaded from static config,
// file, Redis, or HTTP sources with optional periodic reload.
type localBypass struct {
cidrMatcher matcher.Matcher
addrMatcher matcher.Matcher
wildcardMatcher matcher.Matcher
ipRangeMatcher matcher.Matcher
options options
logger logger.Logger
mu sync.RWMutex
cancelFunc context.CancelFunc
patterns *patternSet
options options
logger logger.Logger
mu sync.RWMutex
cancelFunc context.CancelFunc
}
// NewBypass creates and initializes a local Bypass instance.
@@ -161,19 +213,19 @@ func NewBypass(opts ...Option) bypass.Bypass {
ctx, cancel := context.WithCancel(context.Background())
p := &localBypass{
cidrMatcher: matcher.NopMatcher(),
addrMatcher: matcher.NopMatcher(),
wildcardMatcher: matcher.NopMatcher(),
ipRangeMatcher: matcher.NopMatcher(),
cancelFunc: cancel,
options: options,
logger: options.logger,
cancelFunc: cancel,
options: options,
logger: options.logger,
}
if p.logger == nil {
p.logger = xlogger.Nop()
}
go p.periodReload(ctx)
if p.hasLoaders() {
go p.periodReload(ctx)
} else {
_ = p.reload(ctx)
}
return p
}
@@ -202,7 +254,6 @@ func (p *localBypass) periodReload(ctx context.Context) error {
case <-ticker.C:
if err := p.reload(ctx); err != nil {
p.logger.Warnf("reload: %v", err)
// return err
}
case <-ctx.Done():
return ctx.Err()
@@ -212,7 +263,7 @@ func (p *localBypass) periodReload(ctx context.Context) error {
// reload loads patterns from all configured sources, classifies them into
// CIDR, wildcard, IP range, and address matchers, then atomically swaps the
// matchers under the write lock.
// pattern set under the write lock.
func (p *localBypass) reload(ctx context.Context) error {
v, err := p.load(ctx)
if err != nil {
@@ -221,10 +272,29 @@ func (p *localBypass) reload(ctx context.Context) error {
patterns := append(p.options.matchers, v...)
p.logger.Debugf("load items %d", len(patterns))
ps := classifyPatterns(patterns, p.logger)
p.mu.Lock()
p.patterns = ps
p.mu.Unlock()
return nil
}
// hasLoaders reports whether any external data source is configured,
// which determines whether a background reload goroutine is needed.
func (p *localBypass) hasLoaders() bool {
return p.options.fileLoader != nil || p.options.redisLoader != nil || p.options.httpLoader != nil || p.options.period > 0
}
// classifyPatterns sorts raw pattern strings into typed matchers.
// Invalid wildcard patterns are logged and fall through to address matching.
func classifyPatterns(patterns []string, log logger.Logger) *patternSet {
var addrs []string
var inets []*net.IPNet
var wildcards []string
var ipRanges []xnet.IPRange
for _, pattern := range patterns {
if _, inet, err := net.ParseCIDR(pattern); err == nil {
inets = append(inets, inet)
@@ -236,6 +306,7 @@ func (p *localBypass) reload(ctx context.Context) error {
wildcards = append(wildcards, pattern)
continue
}
log.Warnf("invalid wildcard pattern %q, treating as plain address", pattern)
}
r := xnet.IPRange{}
@@ -247,15 +318,12 @@ func (p *localBypass) reload(ctx context.Context) error {
addrs = append(addrs, pattern)
}
p.mu.Lock()
defer p.mu.Unlock()
p.cidrMatcher = matcher.CIDRMatcher(inets)
p.addrMatcher = matcher.AddrMatcher(addrs)
p.wildcardMatcher = matcher.WildcardMatcher(wildcards)
p.ipRangeMatcher = matcher.IPRangeMatcher(ipRanges)
return nil
return &patternSet{
cidr: matcher.CIDRMatcher(inets),
addr: matcher.AddrMatcher(addrs),
wildcard: matcher.WildcardMatcher(wildcards),
ipRange: matcher.IPRangeMatcher(ipRanges),
}
}
// load reads patterns from file, Redis and HTTP loaders if configured.
@@ -334,21 +402,42 @@ func (p *localBypass) Contains(ctx context.Context, network, addr string, opts .
return false
}
matched := p.matched(addr)
b := !p.options.whitelist && matched ||
p.options.whitelist && !matched
decision := p.decide(addr)
log := p.logger.WithFields(map[string]any{
"sid": ctxvalue.SidFromContext(ctx),
})
log.Debugf("%s: %s, whitelist: %t", decision, addr, p.options.whitelist)
if b {
log.Debugf("bypass: %s, whitelist: %t", addr, p.options.whitelist)
} else {
log.Debugf("pass: %s, whitelist: %t", addr, p.options.whitelist)
return decision == decisionBypass
}
// decide returns the bypass decision for the given address, applying the
// whitelist or blacklist mode to the pattern match result.
func (p *localBypass) decide(addr string) bypassDecision {
p.mu.RLock()
defer p.mu.RUnlock()
if p.patterns == nil {
return decisionProxy
}
return b
matched := p.patterns.matchAny(addr)
if p.options.whitelist {
// Whitelist mode: the pattern set specifies addresses that MUST use the proxy.
// Matched addresses go through the proxy; unmatched addresses bypass.
if matched {
return decisionProxy
}
return decisionBypass
}
// Blacklist mode: the pattern set specifies addresses that should bypass the proxy.
// Matched addresses bypass; unmatched addresses go through the proxy.
if matched {
return decisionBypass
}
return decisionProxy
}
func (p *localBypass) IsWhitelist() bool {
@@ -363,33 +452,6 @@ func (p *localBypass) parseLine(s string) string {
return strings.TrimSpace(s)
}
// matched checks whether addr matches any of the configured patterns.
// It tries IP range, address, CIDR, and wildcard matchers in order,
// returning true on the first match.
func (p *localBypass) matched(addr string) bool {
p.mu.RLock()
defer p.mu.RUnlock()
if p.ipRangeMatcher.Match(addr) {
return true
}
if p.addrMatcher.Match(addr) {
return true
}
host, _, _ := net.SplitHostPort(addr)
if host == "" {
host = addr
}
if ip := net.ParseIP(host); ip != nil && p.cidrMatcher.Match(host) {
return true
}
return p.wildcardMatcher.Match(addr)
}
func (p *localBypass) Close() error {
p.cancelFunc()
if p.options.fileLoader != nil {
@@ -428,31 +490,40 @@ func BypassGroup(bypasses ...bypass.Bypass) bypass.Bypass {
// - Blacklist rules: only evaluated if the whitelist result is false.
// ANY matching blacklist rule triggers a bypass (OR logic).
func (p *bypassGroup) Contains(ctx context.Context, network, addr string, opts ...bypass.Option) bool {
var whitelist, blacklist []bool
for _, bypass := range p.bypasses {
result := bypass.Contains(ctx, network, addr, opts...)
if bypass.IsWhitelist() {
whitelist = append(whitelist, result)
} else {
blacklist = append(blacklist, result)
return p.evaluate(ctx, network, addr, opts...) == decisionBypass
}
// evaluate returns the bypass decision for the group by applying two-phase
// evaluation: whitelist rules use AND logic (all must agree), then blacklist
// rules use OR logic (any match wins), evaluated only if whitelist fails.
func (p *bypassGroup) evaluate(ctx context.Context, network, addr string, opts ...bypass.Option) bypassDecision {
// Phase 1: Whitelist AND — all must agree to bypass.
hasWhitelist := false
allWhitelistBypass := true
for _, bp := range p.bypasses {
if !bp.IsWhitelist() {
continue
}
hasWhitelist = true
if !bp.Contains(ctx, network, addr, opts...) {
allWhitelistBypass = false
break // short-circuit: one whitelist failure is enough
}
}
status := false
if len(whitelist) > 0 {
if slices.Contains(whitelist, false) {
status = false
} else {
status = true
if hasWhitelist && allWhitelistBypass {
return decisionBypass
}
// Phase 2: Blacklist OR — any match triggers bypass.
for _, bp := range p.bypasses {
if bp.IsWhitelist() {
continue
}
if bp.Contains(ctx, network, addr, opts...) {
return decisionBypass
}
}
if !status && len(blacklist) > 0 {
if slices.Contains(blacklist, true) {
status = true
} else {
status = false
}
}
return status
return decisionProxy
}
// IsWhitelist always returns false for a group, since the group may contain
+224 -20
View File
@@ -282,38 +282,38 @@ func TestParsePatterns_EdgeCases(t *testing.T) {
assert.Equal(t, []string{"192.168.1.1"}, patterns)
}
// --- matched tests ---
// --- matched tests (via decide) ---
func TestMatched_IP(t *testing.T) {
b := newSyncedBypass(MatchersOption([]string{"192.168.1.1"}))
defer b.Close()
assert.True(t, b.matched("192.168.1.1"))
assert.False(t, b.matched("10.0.0.1"))
assert.Equal(t, decisionBypass, b.decide("192.168.1.1"))
assert.Equal(t, decisionProxy, b.decide("10.0.0.1"))
}
func TestMatched_CIDR(t *testing.T) {
b := newSyncedBypass(MatchersOption([]string{"10.0.0.0/8"}))
defer b.Close()
assert.True(t, b.matched("10.0.0.1"))
assert.False(t, b.matched("192.168.1.1"))
assert.Equal(t, decisionBypass, b.decide("10.0.0.1"))
assert.Equal(t, decisionProxy, b.decide("192.168.1.1"))
}
func TestMatched_Wildcard(t *testing.T) {
b := newSyncedBypass(MatchersOption([]string{"*.example.com"}))
defer b.Close()
assert.True(t, b.matched("foo.example.com"))
assert.False(t, b.matched("foo.other.com"))
assert.Equal(t, decisionBypass, b.decide("foo.example.com"))
assert.Equal(t, decisionProxy, b.decide("foo.other.com"))
}
func TestMatched_IPRange(t *testing.T) {
b := newSyncedBypass(MatchersOption([]string{"10.0.0.1-10.0.0.100"}))
defer b.Close()
assert.True(t, b.matched("10.0.0.50"))
assert.False(t, b.matched("10.0.0.200"))
assert.Equal(t, decisionBypass, b.decide("10.0.0.50"))
assert.Equal(t, decisionProxy, b.decide("10.0.0.200"))
}
// --- reload tests ---
@@ -327,9 +327,9 @@ func TestReload_OnlyIPs(t *testing.T) {
}
err := lb.reload(context.Background())
assert.NoError(t, err)
assert.True(t, lb.matched("1.1.1.1"))
assert.True(t, lb.matched("8.8.8.8"))
assert.False(t, lb.matched("9.9.9.9"))
assert.True(t, lb.patterns.matchAny("1.1.1.1"))
assert.True(t, lb.patterns.matchAny("8.8.8.8"))
assert.False(t, lb.patterns.matchAny("9.9.9.9"))
}
func TestReload_OnlyCIDRs(t *testing.T) {
@@ -341,9 +341,9 @@ func TestReload_OnlyCIDRs(t *testing.T) {
}
err := lb.reload(context.Background())
assert.NoError(t, err)
assert.True(t, lb.matched("192.168.1.1"))
assert.True(t, lb.matched("10.0.0.1"))
assert.False(t, lb.matched("172.16.0.1"))
assert.True(t, lb.patterns.matchAny("192.168.1.1"))
assert.True(t, lb.patterns.matchAny("10.0.0.1"))
assert.False(t, lb.patterns.matchAny("172.16.0.1"))
}
func TestReload_OnlyWildcards(t *testing.T) {
@@ -355,9 +355,9 @@ func TestReload_OnlyWildcards(t *testing.T) {
}
err := lb.reload(context.Background())
assert.NoError(t, err)
assert.True(t, lb.matched("foo.example.com"))
assert.True(t, lb.matched("bar.test.org"))
assert.False(t, lb.matched("other.com"))
assert.True(t, lb.patterns.matchAny("foo.example.com"))
assert.True(t, lb.patterns.matchAny("bar.test.org"))
assert.False(t, lb.patterns.matchAny("other.com"))
}
func TestReload_OnlyIPRanges(t *testing.T) {
@@ -369,8 +369,8 @@ func TestReload_OnlyIPRanges(t *testing.T) {
}
err := lb.reload(context.Background())
assert.NoError(t, err)
assert.True(t, lb.matched("192.168.1.25"))
assert.False(t, lb.matched("192.168.1.100"))
assert.True(t, lb.patterns.matchAny("192.168.1.25"))
assert.False(t, lb.patterns.matchAny("192.168.1.100"))
}
// --- load tests ---
@@ -794,3 +794,207 @@ var _ bypass.Bypass = neverContainsWhitelist{}
var _ bypass.Bypass = (*localBypass)(nil)
var _ bypass.Bypass = (*bypassGroup)(nil)
var _ logger.Logger = xlogger.Nop()
// --- bypassDecision tests ---
func TestBypassDecision_String(t *testing.T) {
assert.Equal(t, "bypass", decisionBypass.String())
assert.Equal(t, "proxy", decisionProxy.String())
assert.Equal(t, "unknown", bypassDecision(99).String())
}
// --- patternSet.matchAny tests ---
func TestPatternSet_MatchAny_IPRange(t *testing.T) {
ps := classifyPatterns([]string{"192.168.1.1-192.168.1.10"}, xlogger.Nop())
assert.True(t, ps.matchAny("192.168.1.5"))
assert.False(t, ps.matchAny("192.168.1.100"))
}
func TestPatternSet_MatchAny_Addr(t *testing.T) {
ps := classifyPatterns([]string{"example.com"}, xlogger.Nop())
assert.True(t, ps.matchAny("example.com"))
assert.False(t, ps.matchAny("other.com"))
}
func TestPatternSet_MatchAny_CIDR(t *testing.T) {
ps := classifyPatterns([]string{"10.0.0.0/8"}, xlogger.Nop())
assert.True(t, ps.matchAny("10.0.0.1"))
assert.True(t, ps.matchAny("10.255.255.255"))
assert.False(t, ps.matchAny("192.168.1.1"))
}
func TestPatternSet_MatchAny_Wildcard(t *testing.T) {
ps := classifyPatterns([]string{"*.example.com"}, xlogger.Nop())
assert.True(t, ps.matchAny("foo.example.com"))
assert.False(t, ps.matchAny("foo.other.com"))
}
func TestPatternSet_MatchAny_NilSet(t *testing.T) {
var ps *patternSet
assert.False(t, ps.matchAny("anything"))
}
func TestPatternSet_MatchAny_Empty(t *testing.T) {
ps := classifyPatterns(nil, xlogger.Nop())
assert.False(t, ps.matchAny("anything"))
}
func TestPatternSet_MatchAny_MixedTypes(t *testing.T) {
ps := classifyPatterns([]string{
"192.168.1.1", // address
"10.0.0.0/8", // CIDR
"*.example.com", // wildcard
"172.16.0.1-172.16.0.255", // IP range
}, xlogger.Nop())
assert.True(t, ps.matchAny("192.168.1.1"))
assert.True(t, ps.matchAny("10.0.0.1"))
assert.True(t, ps.matchAny("foo.example.com"))
assert.True(t, ps.matchAny("172.16.0.50"))
assert.False(t, ps.matchAny("8.8.8.8"))
}
func TestPatternSet_MatchAny_Precedence(t *testing.T) {
// IP range matches first, before address
ps := classifyPatterns([]string{"192.168.1.1-192.168.1.10", "192.168.1.1"}, xlogger.Nop())
assert.True(t, ps.matchAny("192.168.1.5"))
}
func TestPatternSet_MatchAny_CIDRWithPort(t *testing.T) {
ps := classifyPatterns([]string{"10.0.0.0/8"}, xlogger.Nop())
assert.True(t, ps.matchAny("10.0.0.1:8080"))
}
func TestPatternSet_MatchAny_CIDRDomainNotMatched(t *testing.T) {
ps := classifyPatterns([]string{"10.0.0.0/8"}, xlogger.Nop())
assert.False(t, ps.matchAny("example.com"))
}
// --- classifyPatterns tests ---
func TestClassifyPatterns_CIDR(t *testing.T) {
ps := classifyPatterns([]string{"192.168.0.0/16"}, xlogger.Nop())
require.NotNil(t, ps)
assert.True(t, ps.matchAny("192.168.1.1"))
}
func TestClassifyPatterns_Wildcard(t *testing.T) {
ps := classifyPatterns([]string{"*.test.com"}, xlogger.Nop())
require.NotNil(t, ps)
assert.True(t, ps.matchAny("foo.test.com"))
}
func TestClassifyPatterns_IPRange(t *testing.T) {
ps := classifyPatterns([]string{"10.0.0.1-10.0.0.100"}, xlogger.Nop())
require.NotNil(t, ps)
assert.True(t, ps.matchAny("10.0.0.50"))
}
func TestClassifyPatterns_Address(t *testing.T) {
ps := classifyPatterns([]string{"192.168.1.1"}, xlogger.Nop())
require.NotNil(t, ps)
assert.True(t, ps.matchAny("192.168.1.1"))
}
func TestClassifyPatterns_InvalidWildcardFallsToAddress(t *testing.T) {
// A pattern with * that can't compile as glob falls to address matcher.
// "[invalid" has * but glob.Compile may fail — let's use a simpler case.
ps := classifyPatterns([]string{"plain-host"}, xlogger.Nop())
require.NotNil(t, ps)
assert.True(t, ps.matchAny("plain-host"))
}
func TestClassifyPatterns_Empty(t *testing.T) {
ps := classifyPatterns(nil, xlogger.Nop())
require.NotNil(t, ps)
assert.False(t, ps.matchAny("anything"))
}
// --- decide tests ---
func TestDecide_BlacklistMatched(t *testing.T) {
b := newSyncedBypass(MatchersOption([]string{"192.168.1.1"}))
defer b.Close()
assert.Equal(t, decisionBypass, b.decide("192.168.1.1"))
}
func TestDecide_BlacklistNotMatched(t *testing.T) {
b := newSyncedBypass(MatchersOption([]string{"192.168.1.1"}))
defer b.Close()
assert.Equal(t, decisionProxy, b.decide("10.0.0.1"))
}
func TestDecide_WhitelistMatched(t *testing.T) {
b := newSyncedBypass(WhitelistOption(true), MatchersOption([]string{"192.168.1.1"}))
defer b.Close()
assert.Equal(t, decisionProxy, b.decide("192.168.1.1"))
}
func TestDecide_WhitelistNotMatched(t *testing.T) {
b := newSyncedBypass(WhitelistOption(true), MatchersOption([]string{"192.168.1.1"}))
defer b.Close()
assert.Equal(t, decisionBypass, b.decide("10.0.0.1"))
}
func TestDecide_NilPatterns(t *testing.T) {
lb := &localBypass{logger: xlogger.Nop()}
assert.Equal(t, decisionProxy, lb.decide("anything"))
}
// --- evaluate (bypassGroup) tests ---
func TestEvaluate_AllBlacklistAllMatch(t *testing.T) {
g := BypassGroup(alwaysContains{}, alwaysContains{}).(*bypassGroup)
assert.Equal(t, decisionBypass, g.evaluate(context.Background(), "tcp", "any", ))
}
func TestEvaluate_BlacklistNoneMatch(t *testing.T) {
g := BypassGroup(neverContains{}, neverContains{}).(*bypassGroup)
assert.Equal(t, decisionProxy, g.evaluate(context.Background(), "tcp", "any"))
}
func TestEvaluate_WhitelistAllMatch(t *testing.T) {
g := BypassGroup(alwaysContainsWhitelist{}, alwaysContainsWhitelist{}).(*bypassGroup)
assert.Equal(t, decisionBypass, g.evaluate(context.Background(), "tcp", "any"))
}
func TestEvaluate_WhitelistOneFails(t *testing.T) {
g := BypassGroup(alwaysContainsWhitelist{}, neverContainsWhitelist{}).(*bypassGroup)
assert.Equal(t, decisionProxy, g.evaluate(context.Background(), "tcp", "any"))
}
func TestEvaluate_WhitelistFailsBlacklistMatches(t *testing.T) {
g := BypassGroup(neverContainsWhitelist{}, alwaysContains{}).(*bypassGroup)
assert.Equal(t, decisionBypass, g.evaluate(context.Background(), "tcp", "any"))
}
func TestEvaluate_WhitelistFailsBlacklistNoMatch(t *testing.T) {
g := BypassGroup(neverContainsWhitelist{}, neverContains{}).(*bypassGroup)
assert.Equal(t, decisionProxy, g.evaluate(context.Background(), "tcp", "any"))
}
func TestEvaluate_Empty(t *testing.T) {
g := BypassGroup().(*bypassGroup)
assert.Equal(t, decisionProxy, g.evaluate(context.Background(), "tcp", "any"))
}
// --- hasLoaders tests ---
func TestHasLoaders_None(t *testing.T) {
lb := &localBypass{options: options{}}
assert.False(t, lb.hasLoaders())
}
func TestHasLoaders_WithFile(t *testing.T) {
lb := &localBypass{options: options{fileLoader: &mockLoader{}}}
assert.True(t, lb.hasLoaders())
}
func TestHasLoaders_WithPeriod(t *testing.T) {
lb := &localBypass{options: options{period: time.Second}}
assert.True(t, lb.hasLoaders())
}