Refcount shared intercept hostnames so iptables rules are installed for every service (#3868)

* Refcount shared intercept hostnames so iptables rules are installed for every service. Fixes #3868

- modifies getDnsIp to invoke addrCB and register a cleanup action when the hostname already has an allocated IP, so each service sharing the hostname gets its per-service iptables rule installed
- adds a reference count on hostname allocations so the resolver entry is removed and the CGNAT IP recycled only when the last service using the hostname is cleaned up
- canonicalizes the refcount key by lowercasing so case variants of the same hostname share one allocation, matching the resolver's case-insensitive view
- adds tests covering shared-hostname rule installation, refcounted cleanup, out-of-order cleanup, and case-insensitive sharing

* Address review comments

* Refcount wildcard-allocated intercept hostnames. Fixes #3957

- moves hostname registration into allocateDnsIp, under the allocation mutex, so every allocation (fresh or reused) takes one reference through the refcounting resolver and closes the lookup/registration race between wildcard DNS queries and service updates
- removes the direct AddHostname call in getAddress, which bypassed the refcounting layer and let one service's cleanup remove a hostname still used by an overlapping wildcard or literal intercept
- documents the AddDomain callback contract: the callback registers the hostname mapping itself
- updates tests to match the production wiring and adds coverage for overlapping wildcard/literal intercepts in both removal orders
- adds a dns package test pinning the getAddress callback contract
- adds local ai tooling files to .gitignore

.gitignore

@@ -19,6 +19,12 @@ release/
 # Dependency directories (remove the comment below to include it)
 # vendor/
 
+# ai agents
+.claude
+.mercurius/
+mercurius.yaml
+.mcp.json
+
 # goland
 .idea
 github_deploy_key

tunnel/dns/refcount.go

@@ -1,19 +1,28 @@
 package dns
 
 import (
-	cmap "github.com/orcaman/concurrent-map/v2"
 	"net"
+	"strings"
+	"sync"
 )
 
+// NewRefCountingResolver wraps resolver so the underlying hostname is only
+// removed once the last caller releases it. Successive AddHostname calls for
+// the same hostname bump a reference count; RemoveHostname decrements it and
+// only forwards to the wrapped resolver when the count reaches zero.
 func NewRefCountingResolver(resolver Resolver) Resolver {
 	return &RefCountingResolver{
-		names:   cmap.New[int](),
+		names:   map[string]int{},
 		wrapped: resolver,
 	}
 }
 
+// RefCountingResolver reference counts AddHostname/RemoveHostname calls per
+// hostname. The lock is held across the wrapped resolver calls so the count
+// and the wrapped resolver's state can't diverge under concurrent use.
 type RefCountingResolver struct {
-	names   cmap.ConcurrentMap[string, int]
+	lock    sync.Mutex
+	names   map[string]int
 	wrapped Resolver
 }
 
@@ -34,31 +43,33 @@ func (self *RefCountingResolver) RemoveDomain(name string) {
 }
 
 func (self *RefCountingResolver) AddHostname(s string, ip net.IP) error {
+	self.lock.Lock()
+	defer self.lock.Unlock()
+
 	err := self.wrapped.AddHostname(s, ip)
-	if err != nil {
-		self.names.Upsert(s, 1, func(exist bool, valueInMap int, newValue int) int {
-			if exist {
-				return valueInMap + 1
-			}
-			return 1
-		})
+	if err == nil {
+		// canonicalize so different-case spellings of the same hostname share
+		// one count, matching the wrapped resolver's case-insensitive view
+		self.names[strings.ToLower(s)]++
 	}
 	return err
 }
 
 func (self *RefCountingResolver) RemoveHostname(s string) net.IP {
-	val := self.names.Upsert(s, 1, func(exist bool, valueInMap int, newValue int) int {
-		if exist {
-			return valueInMap - 1
-		}
-		return 0
-	})
+	self.lock.Lock()
+	defer self.lock.Unlock()
 
-	if val == 0 {
-		self.names.Remove(s)
-		return self.wrapped.RemoveHostname(s)
+	key := strings.ToLower(s)
+	if count := self.names[key]; count > 1 {
+		self.names[key] = count - 1
+		return nil
 	}
-	return nil
+
+	// count <= 1 covers both the last reference and, defensively, hostnames
+	// never added through this layer (e.g. when the wrapped AddHostname
+	// failed, so the count was never incremented)
+	delete(self.names, key)
+	return self.wrapped.RemoveHostname(s)
 }
 
 func (self *RefCountingResolver) Cleanup() error {

tunnel/dns/resolver.go

@@ -18,8 +18,13 @@ package dns
 
 import "net"
 
+// Resolver maps hostnames to intercept IPs for the tunneler's internal DNS.
 type Resolver interface {
 	AddHostname(string, net.IP) error
+	// AddDomain registers a wildcard domain (e.g. *.ziti). The callback is
+	// invoked to produce an IP for each previously unknown hostname matching
+	// the domain, and must register the resulting hostname -> IP mapping via
+	// AddHostname; the resolver does not register it implicitly.
 	AddDomain(string, func(string) (net.IP, error)) error
 	Lookup(net.IP) (string, error)
 	LookupIP(string) (net.IP, bool)

tunnel/dns/server.go

@@ -121,7 +121,11 @@ func (r *resolver) getAddress(name string) (net.IP, error) {
 				return nil, err
 			}
 			log.Debugf("assigned %v => %v", name, ip)
-			_ = r.AddHostname(name, ip) // this resolver impl never returns an error
+			// the getIP callback registers the hostname -> IP mapping (see
+			// Resolver.AddDomain), so later queries short-circuit via the
+			// LookupIP check above. Registering it here directly would bypass
+			// the refcounting layer wrapping this resolver and let a cleanup
+			// of one service remove a hostname other services still use.
 			return ip, err
 		}
 	}

tunnel/dns/server_test.go

@@ -0,0 +1,70 @@
+/*
+	Copyright NetFoundry Inc.
+
+	Licensed under the Apache License, Version 2.0 (the "License");
+	you may not use this file except in compliance with the License.
+	You may obtain a copy of the License at
+
+	https://www.apache.org/licenses/LICENSE-2.0
+
+	Unless required by applicable law or agreed to in writing, software
+	distributed under the License is distributed on an "AS IS" BASIS,
+	WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+	See the License for the specific language governing permissions and
+	limitations under the License.
+*/
+
+package dns
+
+import (
+	"net"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+// newTestResolver builds a resolver with no listening DNS server, suitable
+// for exercising getAddress directly.
+func newTestResolver() *resolver {
+	return &resolver{
+		names:   map[string]net.IP{},
+		ips:     map[string]string{},
+		domains: map[string]*domainEntry{},
+	}
+}
+
+// Test_GetAddress_DomainCallbackRegistersHostname pins the AddDomain callback
+// contract: the callback registers the hostname -> IP mapping itself (through
+// the refcounting layer in production), and getAddress does not add it to the
+// wrapped resolver directly. A direct add would bypass the refcount and let
+// one service's cleanup remove a hostname other services still use.
+func Test_GetAddress_DomainCallbackRegistersHostname(t *testing.T) {
+	req := require.New(t)
+
+	wrapped := newTestResolver()
+	wrapper := NewRefCountingResolver(wrapped)
+
+	ip := net.IP{100, 64, 0, 1}
+	calls := 0
+	req.NoError(wrapper.AddDomain("*.example.com", func(host string) (net.IP, error) {
+		calls++
+		req.NoError(wrapper.AddHostname(host, ip))
+		return ip, nil
+	}))
+
+	got, err := wrapped.getAddress("test.example.com.")
+	req.NoError(err)
+	req.True(ip.Equal(got))
+	req.Equal(1, calls, "first query must invoke the domain callback")
+
+	got, err = wrapped.getAddress("test.example.com.")
+	req.NoError(err)
+	req.True(ip.Equal(got))
+	req.Equal(1, calls, "registered hostname must short-circuit; callback must not run again")
+
+	// the callback registered through the refcounting layer, so a paired
+	// release removes the mapping from the wrapped resolver
+	req.NotNil(wrapper.RemoveHostname("test.example.com"))
+	_, found := wrapped.LookupIP("test.example.com.")
+	req.False(found, "hostname must be removed once its only reference is released")
+}

tunnel/intercept/iputils.go

@@ -64,8 +64,12 @@ func GetDnsInterceptIpRange() *net.IPNet {
 	}
 }
 
+// cleanUpFunc returns the per-service cleanup action registered when a hostname
+// is intercepted. The wrapping RefCountingResolver only forwards
+// RemoveHostname to the underlying resolver on the last release, so this
+// cleanup recycles the CGNAT IP only when the returned IP is non-nil.
 func cleanUpFunc(hostname string, resolver dns.Resolver) func() {
-	f := func() {
+	return func() {
 		ip := resolver.RemoveHostname(hostname)
 		if ip != nil {
 			dnsCurrentIpMtx.Lock()
@@ -74,19 +78,51 @@ func cleanUpFunc(hostname string, resolver dns.Resolver) func() {
 			dnsRecycledIps.PushBack(addr)
 		}
 	}
-	return f
+}
+
+// hostMask returns a host-route mask (/32 or /128) for the given IP.
+func hostMask(ip net.IP) net.IPMask {
+	bits := len(ip) * 8
+	return net.CIDRMask(bits, bits)
 }
 
 func getDnsIp(host string, addrCB func(*net.IPNet, bool), svc *entities.Service, resolver dns.Resolver) (net.IP, error) {
+	addr, cleanup, err := allocateDnsIp(host, resolver)
+	if err != nil {
+		return nil, err
+	}
+	// addrCB and AddCleanupAction must run outside dnsCurrentIpMtx: addrCB can
+	// touch interceptor state, and AddCleanupAction takes service.lock which
+	// is acquired in the reverse order by RunCleanupActions -> cleanUpFunc.
+	addrCB(addr, false) // no route is needed because the dns cidr was added to "lo" at startup
+	svc.AddCleanupAction(cleanup)
+	return addr.IP, nil
+}
+
+// allocateDnsIp returns the IP mapped to host, allocating one from the
+// intercept range if the hostname is unknown, and registers the hostname -> IP
+// mapping with the resolver. Lookup and registration both happen under
+// dnsCurrentIpMtx so concurrent allocations of the same hostname (e.g. a
+// wildcard-domain DNS query racing a service update) can't both miss the
+// lookup and allocate distinct IPs. Every call takes one reference on the
+// hostname (see dns.NewRefCountingResolver); the returned cleanup releases it.
+func allocateDnsIp(host string, resolver dns.Resolver) (*net.IPNet, func(), error) {
 	dnsCurrentIpMtx.Lock()
 	defer dnsCurrentIpMtx.Unlock()
-	var ip netip.Addr
 
-	foundIP, found := resolver.LookupIP(host + ".")
-	if found {
-		return foundIP, nil
+	// If the hostname already has an allocated IP, reuse it, taking an
+	// additional reference. Even though no new IP is allocated, the caller
+	// still must invoke addrCB so the interceptor installs its per-service
+	// iptables rule; otherwise the tproxy listener exists with no kernel rule
+	// pointing at it and the service is silently unreachable.
+	if foundIP, found := resolver.LookupIP(host + "."); found {
+		if err := resolver.AddHostname(host, foundIP); err != nil {
+			pfxlog.Logger().WithError(err).Errorf("failed to add host/ip mapping to resolver: %v -> %v", host, foundIP)
+		}
+		return &net.IPNet{IP: foundIP, Mask: hostMask(foundIP)}, cleanUpFunc(host, resolver), nil
 	}
 
+	var ip netip.Addr
 	// look for returned IPs first
 	if dnsRecycledIps.Len() > 0 {
 		e := dnsRecycledIps.Front()
@@ -98,19 +134,18 @@ func getDnsIp(host string, addrCB func(*net.IPNet, bool), svc *entities.Service,
 		if ip.IsValid() && dnsPrefix.Contains(ip) {
 			dnsCurrentIp = ip
 		} else {
-			return nil, fmt.Errorf("cannot allocate ip address: ip range exhausted")
+			return nil, nil, fmt.Errorf("cannot allocate ip address: ip range exhausted")
 		}
 	}
 
-	addr := &net.IPNet{IP: ip.AsSlice(), Mask: net.CIDRMask(ip.BitLen(), ip.BitLen())}
-	addrCB(addr, false) // no route is needed because the dns cidr was added to "lo" at startup
-	svc.AddCleanupAction(cleanUpFunc(host, resolver))
-	return ip.AsSlice(), nil
+	ipBytes := ip.AsSlice()
+	if err := resolver.AddHostname(host, ipBytes); err != nil {
+		pfxlog.Logger().WithError(err).Errorf("failed to add host/ip mapping to resolver: %v -> %v", host, ip)
+	}
+	return &net.IPNet{IP: ipBytes, Mask: hostMask(ipBytes)}, cleanUpFunc(host, resolver), nil
 }
 
 func getInterceptIP(svc *entities.Service, hostname string, resolver dns.Resolver, addrCB func(*net.IPNet, bool)) error {
-	logger := pfxlog.Logger()
-
 	// handle wildcard domain - IPs will be allocated when matching hostnames are queried
 	if hostname[0] == '*' {
 		err := resolver.AddDomain(hostname, func(host string) (net.IP, error) {
@@ -129,14 +164,11 @@ func getInterceptIP(svc *entities.Service, hostname string, resolver dns.Resolve
 		return err
 	}
 
-	// handle hostnames
-	ip, err := getDnsIp(hostname, addrCB, svc, resolver)
-	if err != nil {
+	// handle hostnames. getDnsIp registers the hostname -> IP mapping with
+	// the resolver as part of allocation.
+	if _, err = getDnsIp(hostname, addrCB, svc, resolver); err != nil {
 		return fmt.Errorf("invalid IP address or unresolvable hostname: %s", hostname)
 	}
-	if err = resolver.AddHostname(hostname, ip); err != nil {
-		logger.WithError(err).Errorf("failed to add host/ip mapping to resolver: %v -> %v", hostname, ip)
-	}
 
 	return nil
 }

tunnel/intercept/iputils_test.go

@@ -0,0 +1,368 @@
+/*
+	Copyright NetFoundry Inc.
+
+	Licensed under the Apache License, Version 2.0 (the "License");
+	you may not use this file except in compliance with the License.
+	You may obtain a copy of the License at
+
+	https://www.apache.org/licenses/LICENSE-2.0
+
+	Unless required by applicable law or agreed to in writing, software
+	distributed under the License is distributed on an "AS IS" BASIS,
+	WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+	See the License for the specific language governing permissions and
+	limitations under the License.
+*/
+
+package intercept
+
+import (
+	"net"
+	"strings"
+	"testing"
+
+	"github.com/openziti/edge-api/rest_model"
+	"github.com/openziti/ziti/tunnel/dns"
+	"github.com/openziti/ziti/tunnel/entities"
+	"github.com/stretchr/testify/require"
+)
+
+// fakeResolver is a minimal in-memory dns.Resolver. Only the methods the
+// tests exercise are implemented; the rest panic so an accidental dependency
+// on more behavior fails loudly. In tests this is wrapped with
+// dns.NewRefCountingResolver so the refcounting layer is exercised end to end.
+type fakeResolver struct {
+	names map[string]net.IP
+}
+
+func newFakeResolver() *fakeResolver {
+	return &fakeResolver{names: map[string]net.IP{}}
+}
+
+func (r *fakeResolver) LookupIP(name string) (net.IP, bool) {
+	ip, ok := r.names[strings.ToLower(name)]
+	return ip, ok
+}
+
+func (r *fakeResolver) AddHostname(hostname string, ip net.IP) error {
+	r.names[strings.ToLower(hostname)+"."] = ip
+	return nil
+}
+
+func (r *fakeResolver) RemoveHostname(hostname string) net.IP {
+	key := strings.ToLower(hostname) + "."
+	ip, ok := r.names[key]
+	if !ok {
+		return nil
+	}
+	delete(r.names, key)
+	return ip
+}
+
+func (r *fakeResolver) AddDomain(string, func(string) (net.IP, error)) error {
+	panic("unused")
+}
+func (r *fakeResolver) Lookup(net.IP) (string, error) { panic("unused") }
+func (r *fakeResolver) RemoveDomain(string)           { panic("unused") }
+func (r *fakeResolver) Cleanup() error                { panic("unused") }
+
+// resetDnsState resets the package-level allocation state so each test starts
+// from a known baseline. The intercept package keeps allocator state in
+// globals, and other tests in the package may run first.
+func resetDnsState(t *testing.T) {
+	t.Helper()
+	require.NoError(t, SetDnsInterceptIpRange("100.64.0.1/10"))
+}
+
+// addInterceptHostname simulates getInterceptIP's handling of a direct
+// hostname by calling getDnsIp, which allocates (or reuses) the IP and
+// registers the hostname -> IP mapping with the resolver as part of
+// allocation.
+func addInterceptHostname(t *testing.T, host string, addrCB func(*net.IPNet, bool), svc *entities.Service, resolver dns.Resolver) net.IP {
+	t.Helper()
+	ip, err := getDnsIp(host, addrCB, svc, resolver)
+	require.NoError(t, err)
+	return ip
+}
+
+// Test_GetDnsIp_SharedHostnameInstallsRule asserts that when two services share
+// an intercept hostname, getDnsIp invokes addrCB for both services so each one
+// gets its iptables rule installed (the regression in #3867). Without the fix
+// the second service silently has no rule programmed and is unreachable.
+func Test_GetDnsIp_SharedHostnameInstallsRule(t *testing.T) {
+	req := require.New(t)
+	resetDnsState(t)
+
+	resolver := dns.NewRefCountingResolver(newFakeResolver())
+	svcA := &entities.Service{}
+	svcB := &entities.Service{}
+
+	var callsA, callsB []*net.IPNet
+	cbA := func(ipNet *net.IPNet, _ bool) { callsA = append(callsA, ipNet) }
+	cbB := func(ipNet *net.IPNet, _ bool) { callsB = append(callsB, ipNet) }
+
+	ipA := addInterceptHostname(t, "shared.example", cbA, svcA, resolver)
+	req.Len(callsA, 1, "addrCB must fire for the first service")
+	req.True(ipA.Equal(callsA[0].IP), "addrCB receives the allocated IP")
+
+	ipB := addInterceptHostname(t, "shared.example", cbB, svcB, resolver)
+	req.True(ipA.Equal(ipB), "second service must reuse the first service's IP")
+	req.Len(callsB, 1, "addrCB must fire for the second service so its iptables rule is installed")
+	req.True(ipB.Equal(callsB[0].IP), "addrCB for second service receives the shared IP")
+}
+
+// Test_GetDnsIp_RefCountedCleanup asserts that the hostname/IP allocation is
+// reference counted so a service cleanup does not pull the rug out from under
+// other services still using the same hostname.
+func Test_GetDnsIp_RefCountedCleanup(t *testing.T) {
+	req := require.New(t)
+	resetDnsState(t)
+
+	resolver := dns.NewRefCountingResolver(newFakeResolver())
+	svcA := &entities.Service{}
+	svcB := &entities.Service{}
+	noopCB := func(*net.IPNet, bool) {}
+
+	addInterceptHostname(t, "shared.example", noopCB, svcA, resolver)
+	addInterceptHostname(t, "shared.example", noopCB, svcB, resolver)
+
+	// Cleaning up the second service must not remove the hostname or recycle
+	// the IP -- the first service still depends on both.
+	svcB.RunCleanupActions()
+	_, found := resolver.LookupIP("shared.example.")
+	req.True(found, "hostname must remain while another service still uses it")
+	req.Equal(0, dnsRecycledIps.Len(), "IP must not be recycled while another service uses it")
+
+	// Cleaning up the last service must remove the hostname and recycle the IP.
+	svcA.RunCleanupActions()
+	_, found = resolver.LookupIP("shared.example.")
+	req.False(found, "hostname must be removed when the last service cleans up")
+	req.Equal(1, dnsRecycledIps.Len(), "IP must be recycled when the last service cleans up")
+}
+
+// Test_GetDnsIp_SharedHostnameDifferentCase asserts that two services whose
+// hostnames differ only in case share a single allocation, matching the
+// resolver's case-insensitive view.
+func Test_GetDnsIp_SharedHostnameDifferentCase(t *testing.T) {
+	req := require.New(t)
+	resetDnsState(t)
+
+	resolver := dns.NewRefCountingResolver(newFakeResolver())
+	svcA := &entities.Service{}
+	svcB := &entities.Service{}
+	noopCB := func(*net.IPNet, bool) {}
+
+	ipA := addInterceptHostname(t, "Shared.Example", noopCB, svcA, resolver)
+	ipB := addInterceptHostname(t, "shared.example", noopCB, svcB, resolver)
+	req.True(ipA.Equal(ipB), "case variants must resolve to the same IP")
+
+	// First cleanup must not recycle: both services share the allocation.
+	svcA.RunCleanupActions()
+	req.Equal(0, dnsRecycledIps.Len(), "IP must not be recycled while a case variant still holds it")
+
+	// Final cleanup recycles.
+	svcB.RunCleanupActions()
+	req.Equal(1, dnsRecycledIps.Len(), "IP must be recycled once the last case variant releases it")
+}
+
+// Test_GetDnsIp_CleanupOutOfOrder asserts that cleaning up the
+// first-registered service does not strand other services still using the
+// same hostname. Before the fix, the first service's cleanUpFunc was the
+// only one registered for a shared hostname and would unconditionally remove
+// the resolver entry and recycle the IP.
+func Test_GetDnsIp_CleanupOutOfOrder(t *testing.T) {
+	req := require.New(t)
+	resetDnsState(t)
+
+	resolver := dns.NewRefCountingResolver(newFakeResolver())
+	svcA := &entities.Service{}
+	svcB := &entities.Service{}
+	noopCB := func(*net.IPNet, bool) {}
+
+	addInterceptHostname(t, "shared.example", noopCB, svcA, resolver)
+	addInterceptHostname(t, "shared.example", noopCB, svcB, resolver)
+
+	svcA.RunCleanupActions()
+	_, found := resolver.LookupIP("shared.example.")
+	req.True(found, "hostname must remain when an earlier-registered service cleans up while others still use it")
+	req.Equal(0, dnsRecycledIps.Len(), "IP must not be recycled while another service uses it")
+}
+
+// Test_GetDnsIp_FromWildcardLambda asserts that getDnsIp works the same way
+// when invoked from the closure registered via resolver.AddDomain for a
+// wildcard intercept -- the lambda path getInterceptIP uses for hostnames
+// starting with '*'. The closure must allocate an IP, register the hostname
+// through the refcounting layer, install the per-service iptables rule via
+// addrCB, and register a refcount-aware cleanup so reusing the same name from
+// another service shares the allocation. This also covers the
+// wildcard-then-literal overlap with the wildcard service removed first:
+// before allocation took a reference, the wildcard cleanup stole the literal
+// service's reference, removing the hostname while it was still intercepted.
+func Test_GetDnsIp_FromWildcardLambda(t *testing.T) {
+	req := require.New(t)
+	resetDnsState(t)
+
+	resolver := dns.NewRefCountingResolver(newFakeResolver())
+	svcA := &entities.Service{}
+	svcB := &entities.Service{}
+
+	var callsA, callsB []*net.IPNet
+	cbA := func(ipNet *net.IPNet, _ bool) { callsA = append(callsA, ipNet) }
+	cbB := func(ipNet *net.IPNet, _ bool) { callsB = append(callsB, ipNet) }
+
+	// Mimic the closure getInterceptIP registers via resolver.AddDomain.
+	wildcardLambdaA := func(host string) (net.IP, error) {
+		return getDnsIp(host, cbA, svcA, resolver)
+	}
+
+	ipA, err := wildcardLambdaA("host.example")
+	req.NoError(err)
+	req.Len(callsA, 1, "wildcard lambda must invoke addrCB for the first service")
+
+	// A second service joining the same hostname directly must reuse the
+	// allocation and still get its own addrCB call.
+	ipB := addInterceptHostname(t, "host.example", cbB, svcB, resolver)
+	req.True(ipA.Equal(ipB), "second service must reuse the allocation from the wildcard lambda")
+	req.Len(callsB, 1, "addrCB must fire so the joining service gets its iptables rule")
+
+	// First cleanup must not recycle while another service still holds the hostname.
+	svcA.RunCleanupActions()
+	req.Equal(0, dnsRecycledIps.Len(), "IP must not be recycled while another service uses it")
+	_, found := resolver.LookupIP("host.example.")
+	req.True(found, "hostname must remain while another service still uses it")
+
+	// Last cleanup releases the IP.
+	svcB.RunCleanupActions()
+	req.Equal(1, dnsRecycledIps.Len(), "IP must be recycled when the last service cleans up")
+}
+
+// Test_GetDnsIp_WildcardThenLiteral_RemoveLiteralFirst covers the other
+// removal order for overlapping wildcard and literal intercepts: a hostname
+// (test.example.com) is first allocated through a wildcard domain's callback
+// (*.example.com), then a service intercepting the literal hostname joins.
+// Removing the literal service must not remove the resolver entry or recycle
+// the IP while the wildcard service's iptables rule still points at it.
+// Before the fix the wildcard allocation held no reference, so the literal
+// service's cleanup tore down the shared entry and recycled the IP.
+func Test_GetDnsIp_WildcardThenLiteral_RemoveLiteralFirst(t *testing.T) {
+	req := require.New(t)
+	resetDnsState(t)
+
+	resolver := dns.NewRefCountingResolver(newFakeResolver())
+	wildcardSvc := &entities.Service{}
+	literalSvc := &entities.Service{}
+	noopCB := func(*net.IPNet, bool) {}
+
+	// wildcard path: a DNS query for test.example.com matches *.example.com
+	// and the domain callback allocates and registers the hostname
+	wildcardIP, err := getDnsIp("test.example.com", noopCB, wildcardSvc, resolver)
+	req.NoError(err)
+
+	// literal path: a service intercepting test.example.com directly joins
+	literalIP := addInterceptHostname(t, "test.example.com", noopCB, literalSvc, resolver)
+	req.True(wildcardIP.Equal(literalIP), "literal service must reuse the wildcard allocation")
+
+	literalSvc.RunCleanupActions()
+	_, found := resolver.LookupIP("test.example.com.")
+	req.True(found, "hostname must remain while the wildcard service still uses it")
+	req.Equal(0, dnsRecycledIps.Len(), "IP must not be recycled while the wildcard service uses it")
+
+	wildcardSvc.RunCleanupActions()
+	_, found = resolver.LookupIP("test.example.com.")
+	req.False(found, "hostname must be removed when the last user cleans up")
+	req.Equal(1, dnsRecycledIps.Len(), "IP must be recycled when the last user cleans up")
+}
+
+// recordingAddrCB collects the InterceptAddress values produced by
+// GetInterceptAddresses so tests can assert per-service expansion.
+type recordingAddrCB struct {
+	calls []*InterceptAddress
+}
+
+func (r *recordingAddrCB) Apply(a *InterceptAddress) { r.calls = append(r.calls, a) }
+
+// newSharedHostnameService builds a minimally populated entities.Service that
+// intercepts the given hostname on a single TCP port. Only the fields
+// GetInterceptAddresses actually reads are set.
+func newSharedHostnameService(name, hostname string, port uint16) *entities.Service {
+	svc := &entities.Service{}
+	svc.Name = &name
+	svc.InterceptV1Config = &entities.InterceptV1Config{
+		Addresses:  []string{hostname},
+		PortRanges: []*entities.PortRange{{Low: port, High: port}},
+	}
+	return svc
+}
+
+// Test_GetInterceptAddresses_SharedHostnameProducesRulePerService is the
+// integration-level guard for #3867. GetInterceptAddresses is the actual entry
+// point tproxy_linux.go calls per service; each Apply call eventually becomes
+// an iptables rule. Before the fix the second service's callback never fired,
+// so this test would observe zero InterceptAddress instances for svcB. It also
+// guards against future drift in getInterceptIP's wiring that bypasses our
+// finer-grained getDnsIp tests.
+func Test_GetInterceptAddresses_SharedHostnameProducesRulePerService(t *testing.T) {
+	req := require.New(t)
+	resetDnsState(t)
+
+	resolver := dns.NewRefCountingResolver(newFakeResolver())
+	svcA := newSharedHostnameService("svcA", "shared.example", 80)
+	svcB := newSharedHostnameService("svcB", "shared.example", 80)
+
+	cbA := &recordingAddrCB{}
+	cbB := &recordingAddrCB{}
+
+	req.NoError(GetInterceptAddresses(svcA, []string{"tcp"}, resolver, cbA))
+	req.NoError(GetInterceptAddresses(svcB, []string{"tcp"}, resolver, cbB))
+
+	req.Len(cbA.calls, 1, "svcA must receive its InterceptAddress")
+	req.Len(cbB.calls, 1, "svcB must receive its InterceptAddress (the #3867 regression)")
+
+	req.True(cbA.calls[0].IpNet().IP.Equal(cbB.calls[0].IpNet().IP),
+		"both services must see the same allocated IP")
+	req.Equal(uint16(80), cbA.calls[0].LowPort())
+	req.Equal(uint16(80), cbB.calls[0].LowPort())
+	req.Equal("tcp", cbA.calls[0].Proto())
+	req.Equal("tcp", cbB.calls[0].Proto())
+}
+
+// Test_GetInterceptAddresses_SharedHostnameMultiPortProtocol checks that the
+// per-service expansion of protocols x port ranges still fires correctly for
+// the reuse path. With N services, M protocols, and K port ranges we expect
+// each service to produce M*K InterceptAddress instances, all sharing the
+// allocated IP.
+func Test_GetInterceptAddresses_SharedHostnameMultiPortProtocol(t *testing.T) {
+	req := require.New(t)
+	resetDnsState(t)
+
+	resolver := dns.NewRefCountingResolver(newFakeResolver())
+
+	makeSvc := func(name string) *entities.Service {
+		n := name
+		return &entities.Service{
+			ServiceDetail: rest_model.ServiceDetail{
+				BaseEntity: rest_model.BaseEntity{},
+				Name:       &n,
+			},
+			InterceptV1Config: &entities.InterceptV1Config{
+				Addresses:  []string{"multi.example"},
+				PortRanges: []*entities.PortRange{{Low: 80, High: 80}, {Low: 443, High: 443}},
+			},
+		}
+	}
+
+	svcA, svcB := makeSvc("svcA"), makeSvc("svcB")
+	cbA, cbB := &recordingAddrCB{}, &recordingAddrCB{}
+	protocols := []string{"tcp", "udp"}
+
+	req.NoError(GetInterceptAddresses(svcA, protocols, resolver, cbA))
+	req.NoError(GetInterceptAddresses(svcB, protocols, resolver, cbB))
+
+	// 2 protocols x 2 port ranges = 4 InterceptAddress per service.
+	req.Len(cbA.calls, 4)
+	req.Len(cbB.calls, 4, "reuse path must still expand protocols x port ranges for the joining service")
+
+	for _, c := range cbB.calls {
+		req.True(cbA.calls[0].IpNet().IP.Equal(c.IpNet().IP), "svcB calls must all use the shared IP")
+	}
+}