Skip to content

Commit f3b961b

Browse files
committed
test(e2e): fix flaky namespace-watch specs; guard creator-grant rollback
The "two users" namespace-watch specs flake in CI (e.g. "doesn't show users namespaces the other has created"): the watch occasionally returns a stale or another user's namespace instead of the one the spec just created. Root cause: WatchNamespaces did a cluster-scoped watch with no ResourceVersion (so the apiserver first replays all existing namespaces) and returned the *first* event, asserting it was this spec's namespace. Namespaces accumulate across specs in envtest (there is no namespace GC controller), so the replay delivers a stale namespace first and the helper returns before the freshly-created one arrives. Fix: wait for the specific expected namespace (keyed on name, not a count), and have callers also assert the other user's namespace is absent, so a real cross-user leak still fails. Also add an assertion to "recovers when there are kube write failures": after paul's failed create of chani's namespace, no `namespace:<ns>#creator@user:paul` relationship may remain (with `view = viewer + creator`, a dangling grant would let paul see chani's namespace). It passes in both lock modes -- the dual-write rollback is correct -- so it stands as a regression guard, not a bug fix. Unrelated to the REST mapper race; this is pre-existing e2e test-harness flakiness, exposed once the embedded-SpiceDB metrics fix let the e2e suite run far enough to reach these specs. Signed-off-by: Víctor Roldán Betancort <vroldanbet@authzed.com>
1 parent 61a81f9 commit f3b961b

1 file changed

Lines changed: 28 additions & 5 deletions

File tree

e2e/proxy_test.go

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -132,11 +132,19 @@ var _ = Describe("Proxy", func() {
132132
return item.Name
133133
})
134134
}
135-
WatchNamespaces := func(ctx context.Context, client kubernetes.Interface, expected int) []string {
135+
// WatchNamespaces watches namespaces as the given client until it observes the
136+
// expected namespace, returning every name it saw along the way. It waits for the
137+
// specific name rather than returning the first event: the namespace watch is
138+
// cluster-scoped and starts with a replay of pre-existing namespaces, which
139+
// accumulate across specs in envtest (there is no namespace GC controller). Keying
140+
// on a count would otherwise make this return an arbitrary stale namespace and miss
141+
// the one the test just created. Callers assert both that the expected name appears
142+
// and that the other user's namespace does not, so this does not mask leaks.
143+
WatchNamespaces := func(ctx context.Context, client kubernetes.Interface, expected string) []string {
136144
ctx, cancel := context.WithTimeout(ctx, 10*time.Second)
137145
defer cancel()
138146

139-
got := make([]string, 0, expected)
147+
got := make([]string, 0)
140148
watcher, err := client.CoreV1().Namespaces().Watch(ctx, metav1.ListOptions{})
141149
Expect(err).To(Succeed())
142150
defer watcher.Stop()
@@ -147,7 +155,7 @@ var _ = Describe("Proxy", func() {
147155
return got
148156
}
149157
got = append(got, ns.Name)
150-
if len(got) == expected {
158+
if ns.Name == expected {
151159
return got
152160
}
153161
}
@@ -618,12 +626,16 @@ var _ = Describe("Proxy", func() {
618626
defer wg.Wait()
619627
wg.Go(func() error {
620628
defer GinkgoRecover()
621-
Expect(WatchNamespaces(ctx, paulClient, 1)).To(ContainElement(paulNamespace))
629+
seen := WatchNamespaces(ctx, paulClient, paulNamespace)
630+
Expect(seen).To(ContainElement(paulNamespace))
631+
Expect(seen).ToNot(ContainElement(chaniNamespace))
622632
return nil
623633
})
624634
wg.Go(func() error {
625635
defer GinkgoRecover()
626-
Expect(WatchNamespaces(ctx, chaniClient, 1)).To(ContainElement(chaniNamespace))
636+
seen := WatchNamespaces(ctx, chaniClient, chaniNamespace)
637+
Expect(seen).To(ContainElement(chaniNamespace))
638+
Expect(seen).ToNot(ContainElement(paulNamespace))
627639
return nil
628640
})
629641

@@ -666,6 +678,17 @@ var _ = Describe("Proxy", func() {
666678
// paul isn't able to create chanis namespace
667679
Expect(CreateNamespace(ctx, paulClient, chaniNamespace)).ToNot(BeNil())
668680

681+
// paul's failed create must not leave a dangling `creator` grant behind: with
682+
// `permission view = viewer + creator`, that would let paul get/list/watch
683+
// chani's namespace. This surfaces incomplete rollback of the optimistically
684+
// written relationship when the kube write fails.
685+
Expect(GetAllTuples(ctx, &v1.RelationshipFilter{
686+
ResourceType: "namespace",
687+
OptionalResourceId: chaniNamespace,
688+
OptionalRelation: "creator",
689+
OptionalSubjectFilter: &v1.SubjectFilter{SubjectType: "user", OptionalSubjectId: "paul"},
690+
})).To(BeEmpty())
691+
669692
// paul can only get his namespace
670693
Expect(GetNamespace(ctx, paulClient, paulNamespace)).To(Succeed())
671694
Expect(GetNamespace(ctx, paulClient, chaniNamespace)).ToNot(BeNil())

0 commit comments

Comments
 (0)