Skip to content

Commit 0dc015b

Browse files
josephschorrjosephschorr
committed
Setup defined (and configurable) behavior if a ZedToken from
an older datastore is used All ZedTokens are now minted with the datastore's unique ID included in the ZedToken and that ID is checked when the ZedToken is decoded. In scenarios where the datastore ID does not match, either an error is raised (watch, at_exact_snapshot) or configurable behavior is used (at_least_as_fresh) Fixes authzed#1541
1 parent a04dd54 commit 0dc015b

38 files changed

+769
-165
lines changed

‎e2e/newenemy/newenemy_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -376,8 +376,8 @@ func checkDataNoNewEnemy(ctx context.Context, t testing.TB, slowNodeID int, crdb
376376
require.NoError(t, err)
377377
t.Log("r2 token: ", r2.WrittenAt.Token)
378378

379-
z1, _ := zedtoken.DecodeRevision(r1.WrittenAt, revisions.CommonDecoder{Kind: revisions.HybridLogicalClock})
380-
z2, _ := zedtoken.DecodeRevision(r2.WrittenAt, revisions.CommonDecoder{Kind: revisions.HybridLogicalClock})
379+
z1, _, _ := zedtoken.DecodeRevision(r1.WrittenAt, revisions.CommonDecoder{Kind: revisions.HybridLogicalClock})
380+
z2, _, _ := zedtoken.DecodeRevision(r2.WrittenAt, revisions.CommonDecoder{Kind: revisions.HybridLogicalClock})
381381

382382
t.Log("z1 revision: ", z1)
383383
t.Log("z2 revision: ", z2)

‎internal/datastore/proxy/proxy_test/mock.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,16 @@ import (
1515

1616
type MockDatastore struct {
1717
mock.Mock
18+
19+
CurrentUniqueID string
1820
}
1921

2022
func (dm *MockDatastore) UniqueID(_ context.Context) (string, error) {
21-
return "mockds", nil
23+
if dm.CurrentUniqueID == "" {
24+
return "mockds", nil
25+
}
26+
27+
return dm.CurrentUniqueID, nil
2228
}
2329

2430
func (dm *MockDatastore) SnapshotReader(rev datastore.Revision) datastore.Reader {

‎internal/datastore/proxy/relationshipintegrity.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,10 @@ func (r *relationshipIntegrityProxy) OptimizedRevision(ctx context.Context) (dat
197197
return r.ds.OptimizedRevision(ctx)
198198
}
199199

200+
func (r *relationshipIntegrityProxy) UniqueID(ctx context.Context) (string, error) {
201+
return r.ds.UniqueID(ctx)
202+
}
203+
200204
func (r *relationshipIntegrityProxy) ReadyState(ctx context.Context) (datastore.ReadyState, error) {
201205
return r.ds.ReadyState(ctx)
202206
}

‎internal/datastore/revisions/commonrevision.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package revisions
22

33
import (
4+
"context"
5+
46
"github.com/authzed/spicedb/pkg/datastore"
57
"github.com/authzed/spicedb/pkg/spiceerrors"
68
)
@@ -43,7 +45,12 @@ func RevisionParser(kind RevisionKind) ParsingFunc {
4345

4446
// CommonDecoder is a revision decoder that can decode revisions of a given kind.
4547
type CommonDecoder struct {
46-
Kind RevisionKind
48+
Kind RevisionKind
49+
DatastoreUniqueID string
50+
}
51+
52+
func (cd CommonDecoder) UniqueID(_ context.Context) (string, error) {
53+
return cd.DatastoreUniqueID, nil
4754
}
4855

4956
func (cd CommonDecoder) RevisionFromString(s string) (datastore.Revision, error) {

‎internal/services/integrationtesting/cert_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ func TestCertRotation(t *testing.T) {
148148
},
149149
{
150150
Name: "consistency",
151-
Middleware: consistency.UnaryServerInterceptor("testing"),
151+
Middleware: consistency.UnaryServerInterceptor("testing", consistency.TreatMismatchingTokensAsError),
152152
},
153153
{
154154
Name: "servicespecific",
@@ -167,7 +167,7 @@ func TestCertRotation(t *testing.T) {
167167
},
168168
{
169169
Name: "consistency",
170-
Middleware: consistency.StreamServerInterceptor("testing"),
170+
Middleware: consistency.StreamServerInterceptor("testing", consistency.TreatMismatchingTokensAsError),
171171
},
172172
{
173173
Name: "servicespecific",
@@ -211,7 +211,7 @@ func TestCertRotation(t *testing.T) {
211211
_, err = client.CheckPermission(ctx, &v1.CheckPermissionRequest{
212212
Consistency: &v1.Consistency{
213213
Requirement: &v1.Consistency_AtLeastAsFresh{
214-
AtLeastAsFresh: zedtoken.MustNewFromRevision(revision),
214+
AtLeastAsFresh: zedtoken.MustNewFromRevisionForTesting(revision),
215215
},
216216
},
217217
Resource: rel.Resource,
@@ -264,7 +264,7 @@ func TestCertRotation(t *testing.T) {
264264
_, err = client.CheckPermission(ctx, &v1.CheckPermissionRequest{
265265
Consistency: &v1.Consistency{
266266
Requirement: &v1.Consistency_AtLeastAsFresh{
267-
AtLeastAsFresh: zedtoken.MustNewFromRevision(revision),
267+
AtLeastAsFresh: zedtoken.MustNewFromRevisionForTesting(revision),
268268
},
269269
},
270270
Resource: rel.Resource,

‎internal/services/integrationtesting/consistencytestutil/servicetester.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ func (v1st v1ServiceTester) Check(ctx context.Context, resource tuple.ObjectAndR
7878
},
7979
Consistency: &v1.Consistency{
8080
Requirement: &v1.Consistency_AtLeastAsFresh{
81-
AtLeastAsFresh: zedtoken.MustNewFromRevision(atRevision),
81+
AtLeastAsFresh: zedtoken.MustNewFromRevisionForTesting(atRevision),
8282
},
8383
},
8484
Context: context,
@@ -98,7 +98,7 @@ func (v1st v1ServiceTester) Expand(ctx context.Context, resource tuple.ObjectAnd
9898
Permission: resource.Relation,
9999
Consistency: &v1.Consistency{
100100
Requirement: &v1.Consistency_AtLeastAsFresh{
101-
AtLeastAsFresh: zedtoken.MustNewFromRevision(atRevision),
101+
AtLeastAsFresh: zedtoken.MustNewFromRevisionForTesting(atRevision),
102102
},
103103
},
104104
})
@@ -134,7 +134,7 @@ func (v1st v1ServiceTester) Read(_ context.Context, namespaceName string, atRevi
134134
},
135135
Consistency: &v1.Consistency{
136136
Requirement: &v1.Consistency_AtLeastAsFresh{
137-
AtLeastAsFresh: zedtoken.MustNewFromRevision(atRevision),
137+
AtLeastAsFresh: zedtoken.MustNewFromRevisionForTesting(atRevision),
138138
},
139139
},
140140
})
@@ -181,7 +181,7 @@ func (v1st v1ServiceTester) LookupResources(_ context.Context, resourceRelation
181181
},
182182
Consistency: &v1.Consistency{
183183
Requirement: &v1.Consistency_AtLeastAsFresh{
184-
AtLeastAsFresh: zedtoken.MustNewFromRevision(atRevision),
184+
AtLeastAsFresh: zedtoken.MustNewFromRevisionForTesting(atRevision),
185185
},
186186
},
187187
OptionalLimit: limit,
@@ -230,7 +230,7 @@ func (v1st v1ServiceTester) LookupSubjects(_ context.Context, resource tuple.Obj
230230
OptionalSubjectRelation: optionalizeRelation(subjectRelation.Relation),
231231
Consistency: &v1.Consistency{
232232
Requirement: &v1.Consistency_AtLeastAsFresh{
233-
AtLeastAsFresh: zedtoken.MustNewFromRevision(atRevision),
233+
AtLeastAsFresh: zedtoken.MustNewFromRevisionForTesting(atRevision),
234234
},
235235
},
236236
Context: builtContext,
@@ -260,7 +260,7 @@ func (v1st v1ServiceTester) BulkCheck(ctx context.Context, items []*v1.BulkCheck
260260
Items: items,
261261
Consistency: &v1.Consistency{
262262
Requirement: &v1.Consistency_AtLeastAsFresh{
263-
AtLeastAsFresh: zedtoken.MustNewFromRevision(atRevision),
263+
AtLeastAsFresh: zedtoken.MustNewFromRevisionForTesting(atRevision),
264264
},
265265
},
266266
})
@@ -276,7 +276,7 @@ func (v1st v1ServiceTester) CheckBulk(ctx context.Context, items []*v1.CheckBulk
276276
Items: items,
277277
Consistency: &v1.Consistency{
278278
Requirement: &v1.Consistency_AtLeastAsFresh{
279-
AtLeastAsFresh: zedtoken.MustNewFromRevision(atRevision),
279+
AtLeastAsFresh: zedtoken.MustNewFromRevisionForTesting(atRevision),
280280
},
281281
},
282282
})

‎internal/services/integrationtesting/perf_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ func TestBurst(t *testing.T) {
5858
_, err := client.CheckPermission(context.Background(), &v1.CheckPermissionRequest{
5959
Consistency: &v1.Consistency{
6060
Requirement: &v1.Consistency_AtLeastAsFresh{
61-
AtLeastAsFresh: zedtoken.MustNewFromRevision(revision),
61+
AtLeastAsFresh: zedtoken.MustNewFromRevisionForTesting(revision),
6262
},
6363
},
6464
Resource: rel.Resource,

‎internal/services/v1/debug_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -508,7 +508,7 @@ func TestCheckPermissionWithDebug(t *testing.T) {
508508
checkResp, err := client.CheckPermission(ctx, &v1.CheckPermissionRequest{
509509
Consistency: &v1.Consistency{
510510
Requirement: &v1.Consistency_AtLeastAsFresh{
511-
AtLeastAsFresh: zedtoken.MustNewFromRevision(revision),
511+
AtLeastAsFresh: zedtoken.MustNewFromRevisionForTesting(revision),
512512
},
513513
},
514514
Resource: stc.checkRequest.resource,

‎internal/services/v1/experimental.go

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -525,10 +525,16 @@ func (es *experimentalServer) ExperimentalReflectSchema(ctx context.Context, req
525525
}
526526
}
527527

528+
ds := datastoremw.MustFromContext(ctx)
529+
readAt, err := zedtoken.NewFromRevision(ctx, atRevision, ds)
530+
if err != nil {
531+
return nil, shared.RewriteErrorWithoutConfig(ctx, err)
532+
}
533+
528534
return &v1.ExperimentalReflectSchemaResponse{
529535
Definitions: definitions,
530536
Caveats: caveats,
531-
ReadAt: zedtoken.MustNewFromRevision(atRevision),
537+
ReadAt: readAt,
532538
}, nil
533539
}
534540

@@ -543,12 +549,21 @@ func (es *experimentalServer) ExperimentalDiffSchema(ctx context.Context, req *v
543549
return nil, shared.RewriteErrorWithoutConfig(ctx, err)
544550
}
545551

546-
resp, err := convertDiff(diff, existingSchema, comparisonSchema, atRevision)
552+
ds := datastoremw.MustFromContext(ctx)
553+
diffs, err := convertDiff(diff, existingSchema, comparisonSchema)
547554
if err != nil {
548555
return nil, shared.RewriteErrorWithoutConfig(ctx, err)
549556
}
550557

551-
return resp, nil
558+
readAt, err := zedtoken.NewFromRevision(ctx, atRevision, ds)
559+
if err != nil {
560+
return nil, shared.RewriteErrorWithoutConfig(ctx, err)
561+
}
562+
563+
return &v1.ExperimentalDiffSchemaResponse{
564+
Diffs: diffs,
565+
ReadAt: readAt,
566+
}, nil
552567
}
553568

554569
func (es *experimentalServer) ExperimentalComputablePermissions(ctx context.Context, req *v1.ExperimentalComputablePermissionsRequest) (*v1.ExperimentalComputablePermissionsResponse, error) {
@@ -749,11 +764,16 @@ func (es *experimentalServer) ExperimentalCountRelationships(ctx context.Context
749764
return nil, spiceerrors.MustBugf("count should not be negative")
750765
}
751766

767+
readAt, err := zedtoken.NewFromRevision(ctx, headRev, ds)
768+
if err != nil {
769+
return nil, shared.RewriteErrorWithoutConfig(ctx, err)
770+
}
771+
752772
return &v1.ExperimentalCountRelationshipsResponse{
753773
CounterResult: &v1.ExperimentalCountRelationshipsResponse_ReadCounterValue{
754774
ReadCounterValue: &v1.ReadCounterValue{
755775
RelationshipCount: uintCount,
756-
ReadAt: zedtoken.MustNewFromRevision(headRev),
776+
ReadAt: readAt,
757777
},
758778
},
759779
}, nil

‎internal/services/v1/expreflection.go

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import (
99

1010
"github.com/authzed/spicedb/pkg/caveats"
1111
caveattypes "github.com/authzed/spicedb/pkg/caveats/types"
12-
"github.com/authzed/spicedb/pkg/datastore"
1312
"github.com/authzed/spicedb/pkg/diff"
1413
caveatdiff "github.com/authzed/spicedb/pkg/diff/caveats"
1514
nsdiff "github.com/authzed/spicedb/pkg/diff/namespace"
@@ -18,7 +17,6 @@ import (
1817
iv1 "github.com/authzed/spicedb/pkg/proto/impl/v1"
1918
"github.com/authzed/spicedb/pkg/spiceerrors"
2019
"github.com/authzed/spicedb/pkg/tuple"
21-
"github.com/authzed/spicedb/pkg/zedtoken"
2220
)
2321

2422
type schemaFilters struct {
@@ -180,8 +178,7 @@ func convertDiff(
180178
diff *diff.SchemaDiff,
181179
existingSchema *diff.DiffableSchema,
182180
comparisonSchema *diff.DiffableSchema,
183-
atRevision datastore.Revision,
184-
) (*v1.ExperimentalDiffSchemaResponse, error) {
181+
) ([]*v1.ExpSchemaDiff, error) {
185182
size := len(diff.AddedNamespaces) + len(diff.RemovedNamespaces) + len(diff.AddedCaveats) + len(diff.RemovedCaveats) + len(diff.ChangedNamespaces) + len(diff.ChangedCaveats)
186183
diffs := make([]*v1.ExpSchemaDiff, 0, size)
187184

@@ -513,10 +510,7 @@ func convertDiff(
513510
}
514511
}
515512

516-
return &v1.ExperimentalDiffSchemaResponse{
517-
Diffs: diffs,
518-
ReadAt: zedtoken.MustNewFromRevision(atRevision),
519-
}, nil
513+
return diffs, nil
520514
}
521515

522516
// namespaceAPIReprForName builds an API representation of a namespace.

‎internal/services/v1/expreflection_test.go

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import (
99
"github.com/ettle/strcase"
1010
"github.com/stretchr/testify/require"
1111

12-
"github.com/authzed/spicedb/pkg/datastore/revisionparsing"
1312
"github.com/authzed/spicedb/pkg/diff"
1413
"github.com/authzed/spicedb/pkg/genutil/mapz"
1514
"github.com/authzed/spicedb/pkg/schemadsl/compiler"
@@ -544,21 +543,18 @@ func TestConvertDiff(t *testing.T) {
544543
diff, err := diff.DiffSchemas(es, cs)
545544
require.NoError(t, err)
546545

547-
resp, err := convertDiff(
546+
diffs, err := convertDiff(
548547
diff,
549548
&es,
550549
&cs,
551-
revisionparsing.MustParseRevisionForTest("1"),
552550
)
553551
if err != nil {
554552
t.Fatalf("unexpected error: %v", err)
555553
}
556-
require.NotNil(t, resp.ReadAt)
557-
resp.ReadAt = nil
558554

559-
testutil.RequireProtoEqual(t, tc.expectedResponse, resp, "got mismatch")
555+
testutil.RequireProtoEqual(t, tc.expectedResponse, &v1.ExperimentalDiffSchemaResponse{Diffs: diffs}, "got mismatch")
560556

561-
for _, diff := range resp.Diffs {
557+
for _, diff := range diffs {
562558
name := reflect.TypeOf(diff.GetDiff()).String()
563559
encounteredDiffTypes.Add(strings.ToLower(strings.Split(name, "_")[1]))
564560
}

‎internal/services/v1/metadata_test.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ func TestAllMethodsReturnMetadata(t *testing.T) {
3838
_, err := client.CheckPermission(ctx, &v1.CheckPermissionRequest{
3939
Consistency: &v1.Consistency{
4040
Requirement: &v1.Consistency_AtLeastAsFresh{
41-
AtLeastAsFresh: zedtoken.MustNewFromRevision(revision),
41+
AtLeastAsFresh: zedtoken.MustNewFromRevisionForTesting(revision),
4242
},
4343
},
4444
Resource: obj("document", "masterplan"),
@@ -53,7 +53,7 @@ func TestAllMethodsReturnMetadata(t *testing.T) {
5353
_, err := client.CheckBulkPermissions(ctx, &v1.CheckBulkPermissionsRequest{
5454
Consistency: &v1.Consistency{
5555
Requirement: &v1.Consistency_AtLeastAsFresh{
56-
AtLeastAsFresh: zedtoken.MustNewFromRevision(revision),
56+
AtLeastAsFresh: zedtoken.MustNewFromRevisionForTesting(revision),
5757
},
5858
},
5959
Items: []*v1.CheckBulkPermissionsRequestItem{
@@ -96,7 +96,7 @@ func TestAllMethodsReturnMetadata(t *testing.T) {
9696
_, err := client.ExpandPermissionTree(ctx, &v1.ExpandPermissionTreeRequest{
9797
Consistency: &v1.Consistency{
9898
Requirement: &v1.Consistency_AtLeastAsFresh{
99-
AtLeastAsFresh: zedtoken.MustNewFromRevision(revision),
99+
AtLeastAsFresh: zedtoken.MustNewFromRevisionForTesting(revision),
100100
},
101101
},
102102
Resource: obj("document", "masterplan"),
@@ -130,7 +130,7 @@ func TestAllMethodsReturnMetadata(t *testing.T) {
130130
stream, err := client.LookupResources(ctx, &v1.LookupResourcesRequest{
131131
Consistency: &v1.Consistency{
132132
Requirement: &v1.Consistency_AtLeastAsFresh{
133-
AtLeastAsFresh: zedtoken.MustNewFromRevision(revision),
133+
AtLeastAsFresh: zedtoken.MustNewFromRevisionForTesting(revision),
134134
},
135135
},
136136
ResourceObjectType: "document",
@@ -155,7 +155,7 @@ func TestAllMethodsReturnMetadata(t *testing.T) {
155155
stream, err := client.LookupSubjects(ctx, &v1.LookupSubjectsRequest{
156156
Consistency: &v1.Consistency{
157157
Requirement: &v1.Consistency_AtLeastAsFresh{
158-
AtLeastAsFresh: zedtoken.MustNewFromRevision(revision),
158+
AtLeastAsFresh: zedtoken.MustNewFromRevisionForTesting(revision),
159159
},
160160
},
161161
Resource: obj("document", "masterplan"),
@@ -188,7 +188,7 @@ func TestAllMethodsReturnMetadata(t *testing.T) {
188188
stream, err := client.ExportBulkRelationships(ctx, &v1.ExportBulkRelationshipsRequest{
189189
Consistency: &v1.Consistency{
190190
Requirement: &v1.Consistency_AtLeastAsFresh{
191-
AtLeastAsFresh: zedtoken.MustNewFromRevision(revision),
191+
AtLeastAsFresh: zedtoken.MustNewFromRevisionForTesting(revision),
192192
},
193193
},
194194
}, grpc.Trailer(&trailer))

0 commit comments

Comments
 (0)