diff --git a/module/services/accounts/TESTING.md b/module/services/accounts/TESTING.md index 3456af2ab..20f3ee118 100644 --- a/module/services/accounts/TESTING.md +++ b/module/services/accounts/TESTING.md @@ -26,7 +26,10 @@ execution and teardown through the existing package lock. Business tests use Vault for hashing and encrypted credentials. None of them uses telemetry, and only the Redis target starts the `cache` service: it proves the org-membership cache stack across two instances and the Redis-held revocation, nonce and -rate-limit state against the real server. Dependency exclusions prevent every +rate-limit state against the real server, and runs the codefly.dev/cache +conformance suite (`cachetest.Run` and `RunStack`: every mode the driver's +declaration supports, the named pitfalls, the simulation's fixed seeds) against +the service-redis driver the membership stack shares. Dependency exclusions prevent every other service from building or starting. With `go test -v`, database harnesses emit JSON timing records to stderr for diff --git a/module/services/accounts/code/go.mod b/module/services/accounts/code/go.mod index eb56fd445..123d7c2bb 100644 --- a/module/services/accounts/code/go.mod +++ b/module/services/accounts/code/go.mod @@ -9,12 +9,12 @@ require ( connectrpc.com/otelconnect v0.9.0 github.com/codefly-dev/cli/contracts/deployment v0.0.0-20261006151622-5824cf5342fc github.com/codefly-dev/core v0.19.6-0.20261018171306-0a8d16c5e32d - github.com/codefly-dev/interface-cache/go/cache v0.2.2-0.20260926205419-327a6e1d9aba + github.com/codefly-dev/interface-cache/go/cache v0.4.1 github.com/codefly-dev/module-saas-starter/module/frontendpayload v0.0.0-20261018173826-0d423cb8bd17 github.com/codefly-dev/sdk-go v0.4.1 github.com/codefly-dev/sdk-go/workcontext v0.1.1-0.20261018052022-70dc7a546458 github.com/codefly-dev/service-postgres/libs/go v0.0.0-20260929200903-bae6c31c7211 - github.com/codefly-dev/service-redis/cache v0.2.0 + github.com/codefly-dev/service-redis/cache v0.3.0 github.com/go-webauthn/webauthn v0.18.0 github.com/golang-jwt/jwt/v5 v5.3.1 github.com/google/uuid v1.6.0 diff --git a/module/services/accounts/code/go.sum b/module/services/accounts/code/go.sum index 00576cbaf..9e7aee730 100644 --- a/module/services/accounts/code/go.sum +++ b/module/services/accounts/code/go.sum @@ -64,8 +64,8 @@ github.com/codefly-dev/core v0.19.6-0.20261018171306-0a8d16c5e32d h1:vL8Beqd5fsK github.com/codefly-dev/core v0.19.6-0.20261018171306-0a8d16c5e32d/go.mod h1:0uX/7lTBVaGV0RI2Mb3mpEr/PPdx/jZqXHym/U8FVDI= github.com/codefly-dev/gortk v0.2.0 h1:7bOlS5valYz2zil+fZctQNcPCYBcPj86abcw9N8h1hQ= github.com/codefly-dev/gortk v0.2.0/go.mod h1:dDWUMFgAP063OCGhTEpV7FFUj9+zTcxxO/nV80VKEwg= -github.com/codefly-dev/interface-cache/go/cache v0.2.2-0.20260926205419-327a6e1d9aba h1:6nFaDYsLyA5x3WiGAk6pMI3c0naqxiPeaTHMqRPvbVs= -github.com/codefly-dev/interface-cache/go/cache v0.2.2-0.20260926205419-327a6e1d9aba/go.mod h1:OQDNwz4rpEGfq7VC/FjvmQGlo9H8gH8fIDcZhKIe8iQ= +github.com/codefly-dev/interface-cache/go/cache v0.4.1 h1:S7a20cVs3ilW8/HSpzmsyfJx/GJHgJ1xW9Dm9MYXYPI= +github.com/codefly-dev/interface-cache/go/cache v0.4.1/go.mod h1:ki57cBwXnpFLx1uO29KiDY06lzyG/gXXBkqgi5ZNKWQ= github.com/codefly-dev/module-saas-starter/module/frontendpayload v0.0.0-20261018173826-0d423cb8bd17 h1:ppOzy5ops7mEO9PApDfIcAiUBNy4z1MHOf3UJ8xXNNw= github.com/codefly-dev/module-saas-starter/module/frontendpayload v0.0.0-20261018173826-0d423cb8bd17/go.mod h1:OxvwxMdo3HD190xFiMOPHtUsBC7/7Jt1evsEts/2HRs= github.com/codefly-dev/sdk-go v0.4.1 h1:WUKYWxXfojTr88SkLtq0T8aqpV0o5pR/muUUcsUUlzk= @@ -74,8 +74,8 @@ github.com/codefly-dev/sdk-go/workcontext v0.1.1-0.20261018052022-70dc7a546458 h github.com/codefly-dev/sdk-go/workcontext v0.1.1-0.20261018052022-70dc7a546458/go.mod h1:hwtrdGtgh8i2xzjgihyd4wY2u+UK7nHQz30OGKL0n+M= github.com/codefly-dev/service-postgres/libs/go v0.0.0-20260929200903-bae6c31c7211 h1:4KAPx8haCA/h4Kx5mB7D0rMriwLLKiOW9G9mMWuiQDw= github.com/codefly-dev/service-postgres/libs/go v0.0.0-20260929200903-bae6c31c7211/go.mod h1:5mwz8oM8INcukJLc6C84zdRnmDQ/gkMNI8GxWMwgk+8= -github.com/codefly-dev/service-redis/cache v0.2.0 h1:MKeyXFTIiZMnz5bdndb6YTsahws8DjmLTldtz/nH6EQ= -github.com/codefly-dev/service-redis/cache v0.2.0/go.mod h1:bP1kDx4E6UWWIHNkzekfj2NbhSPPDdfRHAdfLC24fCA= +github.com/codefly-dev/service-redis/cache v0.3.0 h1:F0mb7T1xHJBJDgEmI3kPriyb2EmPxKgtmOxSIUylX7s= +github.com/codefly-dev/service-redis/cache v0.3.0/go.mod h1:cs9MIjYuIQpjqjaq1Os/JA/z7kLy093BTUDz4yjs4BE= github.com/containerd/errdefs v1.0.0 h1:tg5yIfIlQIrxYtu9ajqY42W3lpS19XqdxRQeEwYG8PI= github.com/containerd/errdefs v1.0.0/go.mod h1:+YBYIdtsnF4Iw6nWZhJcqGSg/dwvV7tyJ/kCkyJ2k+M= github.com/containerd/errdefs/pkg v0.3.0 h1:9IKJ06FvyNlexW690DXuQNx2KA2cUJXx151Xdx3ZPPE= diff --git a/module/services/accounts/code/pkg/membership/membership.go b/module/services/accounts/code/pkg/membership/membership.go index aac194801..e5127209b 100644 --- a/module/services/accounts/code/pkg/membership/membership.go +++ b/module/services/accounts/code/pkg/membership/membership.go @@ -16,14 +16,26 @@ // through to the store behind a breaker; and "not a member" is cached as a // negative entry. // +// The stack runs in the cache's default mode — TTLBounded by the longest tier +// TTL (sharedTTL), WriteInvalidate, Degrade — except for its fills, which are +// FillLease: the stack only leases a miss on the shared layer when asked to, +// and fill-once across replicas is the reason this lookup is cached at all. +// // # How stale a copy can be after an invalidation // -// Up to memoryTTL (5s) on another replica, not zero. The stack can store a -// generation it read from the shared layer into the in-process tier AFTER the -// change notice for that key arrived: the replica then serves its pre-change -// copy until the in-process entry expires. That fill path is being rewritten in -// codefly-dev/interface-cache#7. Until it lands, memoryTTL is the bound, which -// is why it stays short. +// On another replica, until the shared layer's change notice reaches it: a +// round trip while its tracking connection is up. A fill that read the old +// generation or copy from the shared layer before that notice, and stores it +// in the in-process tier after, undoes its store (go/cache v0.4, the +// StaleRefillAfterInvalidation pitfall of its cachetest suite, which the +// redis-db suite runs against this driver). When the tracking connection drops +// silently, the gap is detected within the driver's NoticeBound and the +// in-process tier flushed; until then memoryTTL (5s) bounds the copy, which is +// why it stays short. +// +// The origin sets neither Entry.Expires nor Entry.NoStore: a role carries no +// validity of its own, and every answer the store gives is one to keep. No +// observer is registered. // // # Store-only mode // @@ -128,6 +140,9 @@ func New(ctx context.Context, shared cache.Layer, store Store, opts ...Option) ( // "Not a member" is cached as long as a role is: every membership // mutation invalidates the key, negative or not. cache.WithNegativeTTL(sharedTTL), + // A miss reaches the store once across replicas, not once per + // replica: the lease is taken on the deepest tier that grants one. + cache.WithMode(cache.FillLease()), ) if err != nil { return nil, fmt.Errorf("membership: build cache stack: %w", err) diff --git a/module/services/accounts/code/pkg/membership/membership_test.go b/module/services/accounts/code/pkg/membership/membership_test.go index 9f0a109fb..404e02404 100644 --- a/module/services/accounts/code/pkg/membership/membership_test.go +++ b/module/services/accounts/code/pkg/membership/membership_test.go @@ -11,6 +11,7 @@ import ( cache "github.com/codefly-dev/interface-cache/go/cache" "github.com/google/uuid" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "accounts/pkg/auth" @@ -27,6 +28,8 @@ type countingStore struct { roles map[string]gen.OrgRole err error loads atomic.Int64 + // delay widens the window in which concurrent misses overlap. + delay time.Duration } func newCountingStore() *countingStore { @@ -45,6 +48,7 @@ func (s *countingStore) set(orgID, userID string, role gen.OrgRole) { func (s *countingStore) GetOrgMembership(_ context.Context, orgID, userID string) (*gen.OrgMembership, error) { s.loads.Add(1) + time.Sleep(s.delay) s.mu.Lock() defer s.mu.Unlock() if s.err != nil { @@ -167,6 +171,35 @@ func TestTwoInstancesShareFillsAndInvalidations(t *testing.T) { }, 2*time.Second, 10*time.Millisecond, "b's in-process copy must be evicted by a's invalidation") } +// Concurrent misses of one key on two instances reach the store once: the +// stack's FillLease mode. Under the cache's default fill (singleflight) each +// instance would load it once. The same over a real Redis is the redis-db +// suite's TestTwoInstancesLoadAMissingMembershipFromTheStoreOnce. +func TestConcurrentMissesOnTwoInstancesLoadTheStoreOnce(t *testing.T) { + store := newCountingStore() + store.delay = 100 * time.Millisecond + org, user := uuid.NewString(), uuid.NewString() + store.set(org, user, gen.OrgRole_ORG_ROLE_MEMBER) + shared := cache.NewMemory() + a := newCache(t, shared, store) + b := newCache(t, shared.Share(), store) + ctx := verified(org, user) + + start := make(chan struct{}) + var wg sync.WaitGroup + for _, c := range []*membership.Cache{a, b, a, b} { + wg.Go(func() { + <-start + role, err := c.Role(ctx, org, user) + assert.NoError(t, err) + assert.Equal(t, gen.OrgRole_ORG_ROLE_MEMBER.String(), role) + }) + } + close(start) + wg.Wait() + require.EqualValues(t, 1, store.loads.Load(), "one load from the store across both instances") +} + func TestInvalidateRefusesAnIDThatIsNotAUUID(t *testing.T) { c := newCache(t, cache.NewMemory(), newCountingStore()) require.Error(t, c.Invalidate(context.Background(), "", uuid.NewString())) diff --git a/module/services/accounts/code/pkg/redisintegration/cachetest_test.go b/module/services/accounts/code/pkg/redisintegration/cachetest_test.go new file mode 100644 index 000000000..e0da28808 --- /dev/null +++ b/module/services/accounts/code/pkg/redisintegration/cachetest_test.go @@ -0,0 +1,95 @@ +//go:build !pure + +package redisintegration_test + +import ( + "context" + "strings" + "testing" + "time" + + cache "github.com/codefly-dev/interface-cache/go/cache" + "github.com/codefly-dev/interface-cache/go/cache/cachetest" + rediscache "github.com/codefly-dev/service-redis/cache" + goredis "github.com/redis/go-redis/v9" + "github.com/stretchr/testify/require" + + "accounts/pkg/infra" +) + +// The codefly.dev/cache conformance suite against the shared layer the +// membership cache runs on: the service-redis driver, over a client opened by +// infra.NewRedisClient, on the server the pinned codefly.dev/redis agent runs. +// Run holds the layer to what it declares (Leases, Notifies within its +// NoticeBound, Resyncs, Fences); RunStack holds stacks over it — each with its +// own Memory tier, as each replica has — to every mode the declaration +// supports, FillLease and the default TTLBounded among them, every named +// pitfall, and the simulation's fixed seeds. +func TestMembershipCacheLayerConformsToTheCacheInterface(t *testing.T) { + h := membershipCacheHarness(t) + + // The layer the service builds is the one the harness builds, but for its + // fixed key prefix, which the suite replaces with a namespace per test. + client := cacheClient(t) + require.IsType(t, &rediscache.Layer{}, infra.MembershipCacheLayer(client)) + require.Equal(t, + cache.CapabilitiesOf(infra.MembershipCacheLayer(client)), + cache.CapabilitiesOf(h.New(t, cachetest.Namespace(t)))) + + t.Run("Conformance", func(t *testing.T) { cachetest.Run(t, h) }) + t.Run("Stack", func(t *testing.T) { cachetest.RunStack(t, h) }) +} + +// membershipCacheHarness gives each namespace its own client, as separate +// replicas would have, and reaches the server directly for the writes and the +// connection drops a driver cannot make itself. +func membershipCacheHarness(t *testing.T) cachetest.Harness { + server := cacheClient(t) + return cachetest.Harness{ + New: func(t *testing.T, namespace string) cache.Layer { + return rediscache.New(cacheClient(t), rediscache.WithPrefix(namespace)) + }, + // A plain SET of the driver's value key, as another program would. + ExternalWrite: func(t *testing.T, namespace, key string) { + require.NoError(t, server.Set(context.Background(), namespace+":v:{"+key+"}", "written externally", time.Minute).Err()) + }, + // The server drops the layer's tracking connection, as a network + // failure or a restart would. + Interrupt: func(t *testing.T, l cache.Layer) { + name := l.(*rediscache.Layer).TrackingClientName() + require.NotZero(t, killConnectionsNamed(t, server, name), "no tracking connection %q to interrupt", name) + }, + } +} + +func cacheClient(t *testing.T) *goredis.Client { + t.Helper() + client, err := infra.NewRedisClient(testCtx) + require.NoError(t, err) + t.Cleanup(func() { _ = client.Close() }) + return client +} + +// killConnectionsNamed CLIENT KILLs every connection named name and returns how +// many there were. +func killConnectionsNamed(t *testing.T, server *goredis.Client, name string) int { + t.Helper() + ctx := context.Background() + list, err := server.ClientList(ctx).Result() + require.NoError(t, err) + killed := 0 + for _, line := range strings.Split(list, "\n") { + fields := map[string]string{} + for _, field := range strings.Fields(line) { + if k, v, ok := strings.Cut(field, "="); ok { + fields[k] = v + } + } + if fields["name"] != name { + continue + } + require.NoError(t, server.ClientKillByFilter(ctx, "ID", fields["id"]).Err()) + killed++ + } + return killed +}