Skip to content

Commit 93ec997

Browse files
ipcache: add tests reproducing host-IP-as-world misclassification on restart
Add two test files that reproduce and document a bug where node/host IPs inside a local-DC CiliumCIDRGroup are misclassified as world identity during rolling Cilium agent restarts. Root cause (two cooperating code paths): 1. local_identity_restorer.go:128 - dumpOldIPCache() only restores identities with IdentityScopeLocal or ReservedIdentityIngress. ReservedIdentityHost (id=1, scope=global) is excluded. After ipcachemap.Recreate() wipes the BPF map, host IP entries are gone until syncHostIPs runs. 2. daemon.go startup ordering - K8sWatcher.InitK8sSubsystem() (line 202) starts CiliumCIDRGroup processing before syncHostIPs.StartAndWaitFirst() (line 249). During this window a host IP in a local-DC CiliumCIDRGroup receives only a cidrgroup label. 3. metadata.go:798 resolveLabels() - without reserved:host, HasHostLabel()=false, isInCluster=false, AddWorldLabel() fires. The IP is assigned world identity. Impact: traffic from node IPs to cluster-dns is denied because the CNP uses fromEntities:cluster (which excludes world). Confirmed in production at us1.fed.dog (plain1, us-gov-west-1a) on 2026-03-20 with 10,551 drops over 48h. pkg/ipcache/metadata_restart_test.go: - TestHostIPWorldFallbackDuringRestartWindow: asserts the buggy behaviour where a host IP with only a cidrgroup label gets world identity. Expected to fail once the root cause is fixed. - TestWorldFallbackDoesNotOccurWhenHostLabelPresentFirst: asserts the correct steady-state behaviour where reserved:host arrives before cidrgroup labels. pkg/ipcache/restore/local_identity_restorer_test.go: - TestHostIdentityExcludedFromIPCacheRestoration: directly tests the filter condition at local_identity_restorer.go:128 for all relevant identity types. - TestHostIdentityScopeIsGlobal: verifies that ReservedIdentityHost.Scope() returns IdentityScopeGlobal, which is why it is excluded from restoration. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
1 parent cf9dfdf commit 93ec997

2 files changed

Lines changed: 322 additions & 0 deletions

File tree

Lines changed: 187 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,187 @@
1+
// SPDX-License-Identifier: Apache-2.0
2+
// Copyright Authors of Cilium
3+
4+
package ipcache
5+
6+
// TestHostIPWorldFallbackDuringRestartWindow and TestHostIdentityRestorationGap
7+
// reproduce a bug observed in production (us1.fed.dog, 2026-03-20) where node/host
8+
// IPs in a local-DC CIDR were misclassified as "world" identity during rolling
9+
// Cilium agent restarts, causing policy_denied drops against cluster-dns.
10+
//
11+
// Root cause (two code paths, both required):
12+
//
13+
// 1. pkg/ipcache/restore/local_identity_restorer.go:128
14+
// dumpOldIPCache() filters restored identities to IdentityScopeLocal and
15+
// ReservedIdentityIngress only. ReservedIdentityHost (scope=global, id=1) is
16+
// explicitly excluded. After ipcachemap.Recreate(), the new BPF map has no
17+
// entry for host IPs.
18+
//
19+
// 2. daemon/cmd/daemon.go startup ordering
20+
// K8sWatcher.InitK8sSubsystem() starts at line 202 (begins processing
21+
// CiliumCIDRGroups managed by fabric-k8s-controller). syncHostIPs.StartAndWaitFirst()
22+
// is not called until line 249. During this window, a host IP in the local-DC
23+
// CiliumCIDRGroup (e.g. 10.160.0.0/14) receives only a cidrgroup label.
24+
//
25+
// 3. pkg/ipcache/metadata.go:798 (resolveLabels)
26+
// Any IP without reserved:host, reserved:remote-node, reserved:health, or
27+
// reserved:ingress label has AddWorldLabel() called on it. A host IP with only
28+
// a cidrgroup label therefore becomes world — which is not covered by the
29+
// cluster-dns CNP's "fromEntities: cluster" ingress rule, causing drops.
30+
31+
import (
32+
"net/netip"
33+
"testing"
34+
35+
"github.com/stretchr/testify/assert"
36+
"github.com/stretchr/testify/require"
37+
38+
cmtypes "github.com/cilium/cilium/pkg/clustermesh/types"
39+
"github.com/cilium/cilium/pkg/identity"
40+
"github.com/cilium/cilium/pkg/labels"
41+
"github.com/cilium/cilium/pkg/option"
42+
"github.com/cilium/cilium/pkg/source"
43+
)
44+
45+
// cidrGroupLabels returns a Labels set simulating what the CiliumCIDRGroup reconciler
46+
// (fabric-k8s-controller) injects via UpsertMetadata for an IP that matches a
47+
// CiliumCIDRGroup (e.g. the "local-dc" group covering 10.160.0.0/14).
48+
func cidrGroupLabels(groupName string) labels.Labels {
49+
return labels.Labels{
50+
groupName: labels.NewLabel(groupName, "", labels.LabelSourceCIDRGroup),
51+
}
52+
}
53+
54+
// TestHostIPWorldFallbackDuringRestartWindow reproduces the bug where a host IP
55+
// is assigned world identity because resolveLabels() runs with only cidrgroup labels
56+
// — before syncHostIPs has inserted the reserved:host label.
57+
//
58+
// This test asserts the CURRENT BUGGY BEHAVIOR. It is expected to fail once the
59+
// bug is fixed (e.g. by ensuring host IPs are seeded into ipcache metadata before
60+
// CiliumCIDRGroup processing can trigger resolveLabels for those prefixes).
61+
func TestHostIPWorldFallbackDuringRestartWindow(t *testing.T) {
62+
s := setupIPCacheTestSuite(t)
63+
ctx := t.Context()
64+
65+
// Disable PolicyCIDRMatchMode to avoid interference from node-CIDR matching.
66+
oldVal := option.Config.PolicyCIDRMatchMode
67+
t.Cleanup(func() { option.Config.PolicyCIDRMatchMode = oldVal })
68+
option.Config.PolicyCIDRMatchMode = []string{}
69+
70+
// The host IP observed in production: 10.161.39.126 (in 10.160.0.0/14, localDc CIDR).
71+
// 8,258 drops were recorded against cluster-dns over 48h.
72+
hostIPPrefix := cmtypes.NewLocalPrefixCluster(netip.MustParsePrefix("10.161.39.126/32"))
73+
74+
// ── Stage 1: Restart window ──────────────────────────────────────────────
75+
// K8sWatcher has processed the "local-dc" CiliumCIDRGroup. The ipcache BPF
76+
// map has been recreated empty (RestoreLocalIdentities skipped this IP since
77+
// ReservedIdentityHost is not locally-scoped). syncHostIPs has NOT run yet.
78+
//
79+
// Only the cidrgroup label is present — no reserved:host.
80+
s.IPIdentityCache.metadata.upsertLocked(
81+
hostIPPrefix,
82+
source.Generated,
83+
"cidrgroup-resource-uid",
84+
cidrGroupLabels("local-dc"),
85+
)
86+
87+
_, err := s.IPIdentityCache.doInjectLabels(ctx, []cmtypes.PrefixCluster{hostIPPrefix})
88+
require.NoError(t, err)
89+
90+
entry, ok := s.IPIdentityCache.ipToIdentityCache["10.161.39.126/32"]
91+
require.True(t, ok, "expected an identity entry for 10.161.39.126/32")
92+
93+
assignedID := entry.ID
94+
95+
// Verify the assigned identity is NOT reserved:host (id=1).
96+
// This demonstrates the bug: the IP should be host but is not.
97+
assert.NotEqual(t, identity.ReservedIdentityHost, assignedID,
98+
"BUG REPRODUCED: host IP 10.161.39.126 was not assigned ReservedIdentityHost (id=1). "+
99+
"Got id=%d. This occurs because resolveLabels() ran with only cidrgroup labels "+
100+
"(no reserved:host) during the restart window before syncHostIPs executed.",
101+
assignedID)
102+
103+
// Verify the assigned identity has a world label — the world fallback fired.
104+
resolvedIdentity := s.Allocator.LookupIdentityByID(ctx, assignedID)
105+
require.NotNil(t, resolvedIdentity, "identity %d should be resolvable", assignedID)
106+
assert.True(t,
107+
resolvedIdentity.Labels.HasWorldLabel() || resolvedIdentity.Labels.HasWorldIPv4Label(),
108+
"BUG: host IP 10.161.39.126/32 was assigned world identity (id=%d, labels=%v). "+
109+
"resolveLabels() called AddWorldLabel() because HasHostLabel()=false. "+
110+
"This causes policy_denied drops: the cluster-dns CNP allows 'fromEntities: cluster' "+
111+
"but world (id=2) is not in the cluster entity.",
112+
assignedID, resolvedIdentity.Labels)
113+
114+
// ── Stage 2: syncHostIPs runs ────────────────────────────────────────────
115+
// After daemon initialization completes (daemon.go:249), syncHostIPs inserts
116+
// the reserved:host label for this IP. resolveLabels() now sees HasHostLabel()=true,
117+
// sets isInCluster=true, removes the cidrgroup label, and does NOT add world.
118+
s.IPIdentityCache.metadata.upsertLocked(
119+
hostIPPrefix,
120+
source.Local,
121+
"daemon-reserved",
122+
labels.LabelHost,
123+
)
124+
125+
_, err = s.IPIdentityCache.doInjectLabels(ctx, []cmtypes.PrefixCluster{hostIPPrefix})
126+
require.NoError(t, err)
127+
128+
correctedEntry, ok := s.IPIdentityCache.ipToIdentityCache["10.161.39.126/32"]
129+
require.True(t, ok)
130+
131+
// After syncHostIPs runs, the identity must be corrected to reserved:host.
132+
assert.Equal(t, identity.ReservedIdentityHost, correctedEntry.ID,
133+
"After syncHostIPs inserts reserved:host, identity should be corrected to "+
134+
"ReservedIdentityHost (id=1). Got id=%d.", correctedEntry.ID)
135+
}
136+
137+
// TestWorldFallbackDoesNotOccurWhenHostLabelPresentFirst verifies the CORRECT
138+
// behaviour: when reserved:host is present before CIDRGroup labels are processed,
139+
// resolveLabels() correctly identifies the IP as in-cluster and does not add
140+
// the world label.
141+
//
142+
// This is the inverse of TestHostIPWorldFallbackDuringRestartWindow and
143+
// documents the expected steady-state behaviour (no restart window).
144+
func TestWorldFallbackDoesNotOccurWhenHostLabelPresentFirst(t *testing.T) {
145+
s := setupIPCacheTestSuite(t)
146+
ctx := t.Context()
147+
148+
oldVal := option.Config.PolicyCIDRMatchMode
149+
t.Cleanup(func() { option.Config.PolicyCIDRMatchMode = oldVal })
150+
option.Config.PolicyCIDRMatchMode = []string{}
151+
152+
hostIPPrefix := cmtypes.NewLocalPrefixCluster(netip.MustParsePrefix("10.161.39.126/32"))
153+
154+
// syncHostIPs runs FIRST (correct startup order / no restart window).
155+
s.IPIdentityCache.metadata.upsertLocked(
156+
hostIPPrefix,
157+
source.Local,
158+
"daemon-reserved",
159+
labels.LabelHost,
160+
)
161+
162+
// CiliumCIDRGroup label arrives afterwards (normal steady-state order).
163+
s.IPIdentityCache.metadata.upsertLocked(
164+
hostIPPrefix,
165+
source.Generated,
166+
"cidrgroup-resource-uid",
167+
cidrGroupLabels("local-dc"),
168+
)
169+
170+
_, err := s.IPIdentityCache.doInjectLabels(ctx, []cmtypes.PrefixCluster{hostIPPrefix})
171+
require.NoError(t, err)
172+
173+
entry, ok := s.IPIdentityCache.ipToIdentityCache["10.161.39.126/32"]
174+
require.True(t, ok)
175+
176+
// When reserved:host is present, the identity must be ReservedIdentityHost.
177+
assert.Equal(t, identity.ReservedIdentityHost, entry.ID,
178+
"When reserved:host is already in ipcache metadata before CIDRGroup labels "+
179+
"arrive, the identity must be ReservedIdentityHost (id=1). Got id=%d.", entry.ID)
180+
181+
resolvedIdentity := s.Allocator.LookupIdentityByID(ctx, entry.ID)
182+
require.NotNil(t, resolvedIdentity)
183+
assert.False(t,
184+
resolvedIdentity.Labels.HasWorldLabel() || resolvedIdentity.Labels.HasWorldIPv4Label(),
185+
"Identity must not have world label when reserved:host is present. Labels: %v",
186+
resolvedIdentity.Labels)
187+
}
Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
// SPDX-License-Identifier: Apache-2.0
2+
// Copyright Authors of Cilium
3+
4+
package restoration
5+
6+
// TestHostIdentityExcludedFromIPCacheRestoration tests that the ipcache
7+
// restoration logic (dumpOldIPCache) explicitly excludes ReservedIdentityHost
8+
// entries from the set of identities that survive an agent restart.
9+
//
10+
// This is one of two root causes for the host-IP-as-world misclassification bug:
11+
//
12+
// dumpOldIPCache() at local_identity_restorer.go:128:
13+
//
14+
// if nid.Scope() == identity.IdentityScopeLocal ||
15+
// nid == identity.ReservedIdentityIngress {
16+
// localPrefixes[k.Prefix()] = nid // host identity NEVER matches
17+
// }
18+
//
19+
// Because ReservedIdentityHost (id=1) has IdentityScopeGlobal (scope bits = 0),
20+
// it does not pass the IdentityScopeLocal check. It is also not
21+
// ReservedIdentityIngress. The host IP entry from the OLD ipcache BPF map is
22+
// therefore NEVER written into localPrefixes and is NEVER restored into the new
23+
// ipcache metadata.
24+
//
25+
// Consequence: after ipcachemap.Recreate() wipes the BPF map (cell.go:118),
26+
// there is a window before syncHostIPs runs (daemon.go:249) during which host
27+
// IPs have no ipcache metadata entry. If a CiliumCIDRGroup covering the host IP
28+
// is processed during this window, resolveLabels() sees only the cidrgroup label,
29+
// calls AddWorldLabel(), and assigns world identity — causing policy_denied drops.
30+
31+
import (
32+
"testing"
33+
34+
"github.com/stretchr/testify/assert"
35+
36+
"github.com/cilium/cilium/pkg/identity"
37+
)
38+
39+
// TestHostIdentityExcludedFromIPCacheRestoration documents the restoration
40+
// filter condition that causes host IPs to lose their identity on restart.
41+
//
42+
// The filter in dumpOldIPCache (local_identity_restorer.go:128) is:
43+
//
44+
// nid.Scope() == identity.IdentityScopeLocal || nid == identity.ReservedIdentityIngress
45+
//
46+
// This test verifies the scope/identity values used by the filter and shows
47+
// which identities are retained vs dropped during restoration.
48+
func TestHostIdentityExcludedFromIPCacheRestoration(t *testing.T) {
49+
type testCase struct {
50+
name string
51+
id identity.NumericIdentity
52+
wantRestored bool
53+
explanation string
54+
}
55+
56+
// These cases mirror the exact filter condition at local_identity_restorer.go:128.
57+
cases := []testCase{
58+
{
59+
name: "ReservedIdentityHost is NOT restored",
60+
id: identity.ReservedIdentityHost, // id=1, scope=global
61+
wantRestored: false,
62+
explanation: "ReservedIdentityHost (id=1) has IdentityScopeGlobal (scope bits = 0). " +
63+
"It does not satisfy Scope()==IdentityScopeLocal and is not ReservedIdentityIngress. " +
64+
"BUG: after ipcachemap.Recreate(), host IP entries are missing from the new ipcache " +
65+
"until syncHostIPs runs. If CiliumCIDRGroup processing happens first, " +
66+
"resolveLabels() assigns world identity to the host IP.",
67+
},
68+
{
69+
name: "ReservedIdentityWorld is NOT restored",
70+
id: identity.ReservedIdentityWorld, // id=2, scope=global
71+
wantRestored: false,
72+
explanation: "World identity is re-added as a /0 catch-all by syncHostIPs, not via restoration.",
73+
},
74+
{
75+
name: "ReservedIdentityIngress IS restored",
76+
id: identity.ReservedIdentityIngress,
77+
wantRestored: true,
78+
explanation: "Ingress is explicitly included via the nid==ReservedIdentityIngress check.",
79+
},
80+
{
81+
name: "A local-scope CIDR identity IS restored",
82+
id: identity.NumericIdentity(1<<24 + 42), // IdentityScopeLocal | 42
83+
wantRestored: true,
84+
explanation: "Local-scope CIDR identities are restored to preserve numeric identity " +
85+
"stability across restarts. This is the primary use case for dumpOldIPCache.",
86+
},
87+
{
88+
name: "A remote-node-scope identity is NOT restored",
89+
id: identity.NumericIdentity(2<<24 + 1), // IdentityScopeRemoteNode | 1
90+
wantRestored: false,
91+
explanation: "Remote-node-scope identities are re-derived from the node manager, not restored.",
92+
},
93+
}
94+
95+
for _, tc := range cases {
96+
t.Run(tc.name, func(t *testing.T) {
97+
// This is the exact filter predicate from dumpOldIPCache:
98+
// local_identity_restorer.go:128
99+
wouldBeRestored := tc.id.Scope() == identity.IdentityScopeLocal ||
100+
tc.id == identity.ReservedIdentityIngress
101+
102+
assert.Equal(t, tc.wantRestored, wouldBeRestored,
103+
"%s\n"+
104+
" identity id: %d\n"+
105+
" identity scope: %d (IdentityScopeLocal=%d, IdentityScopeGlobal=%d)\n"+
106+
" filter result: restored=%v",
107+
tc.explanation,
108+
tc.id,
109+
tc.id.Scope(),
110+
identity.IdentityScopeLocal,
111+
identity.IdentityScopeGlobal,
112+
wouldBeRestored,
113+
)
114+
})
115+
}
116+
}
117+
118+
// TestHostIdentityScopeIsGlobal explicitly verifies that ReservedIdentityHost
119+
// has global scope, which is why it is excluded from the restoration filter.
120+
// This is the direct mechanical reason for the misclassification bug.
121+
func TestHostIdentityScopeIsGlobal(t *testing.T) {
122+
hostScope := identity.ReservedIdentityHost.Scope()
123+
124+
assert.Equal(t, identity.IdentityScopeGlobal, hostScope,
125+
"ReservedIdentityHost must have IdentityScopeGlobal (scope=0). "+
126+
"This means it is excluded by the dumpOldIPCache filter "+
127+
"(local_identity_restorer.go:128) which only retains IdentityScopeLocal "+
128+
"and ReservedIdentityIngress. As a result, host IP entries are lost from "+
129+
"the ipcache BPF map after ipcachemap.Recreate() and are not re-inserted "+
130+
"until syncHostIPs.StartAndWaitFirst() completes (daemon.go:249).")
131+
132+
assert.NotEqual(t, identity.IdentityScopeLocal, hostScope,
133+
"If this assertion fails, the bug would be self-healing: "+
134+
"host identity would be restored and the world fallback would not occur.")
135+
}

0 commit comments

Comments
 (0)