Skip to content

Commit 3ed6e2b

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 fcd03d1 commit 3ed6e2b

39 files changed

+807
-160
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/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: 2 additions & 2 deletions
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,
@@ -906,7 +906,7 @@ func TestBulkCheckPermissionWithDebug(t *testing.T) {
906906
checkResp, err := client.CheckBulkPermissions(ctx, &v1.CheckBulkPermissionsRequest{
907907
Consistency: &v1.Consistency{
908908
Requirement: &v1.Consistency_AtLeastAsFresh{
909-
AtLeastAsFresh: zedtoken.MustNewFromRevision(revision),
909+
AtLeastAsFresh: zedtoken.MustNewFromRevisionForTesting(revision),
910910
},
911911
},
912912
WithTracing: true,

‎internal/services/v1/experimental.go

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -535,10 +535,16 @@ func (es *experimentalServer) ExperimentalReflectSchema(ctx context.Context, req
535535
}
536536
}
537537

538+
ds := datastoremw.MustFromContext(ctx)
539+
readAt, err := zedtoken.NewFromRevision(ctx, atRevision, ds)
540+
if err != nil {
541+
return nil, shared.RewriteErrorWithoutConfig(ctx, err)
542+
}
543+
538544
return &v1.ExperimentalReflectSchemaResponse{
539545
Definitions: definitions,
540546
Caveats: caveats,
541-
ReadAt: zedtoken.MustNewFromRevision(atRevision),
547+
ReadAt: readAt,
542548
}, nil
543549
}
544550

@@ -553,7 +559,7 @@ func (es *experimentalServer) ExperimentalDiffSchema(ctx context.Context, req *v
553559
return nil, shared.RewriteErrorWithoutConfig(ctx, err)
554560
}
555561

556-
resp, err := expConvertDiff(diff, existingSchema, comparisonSchema, atRevision, es.caveatTypeSet)
562+
resp, err := expConvertDiff(ctx, diff, existingSchema, comparisonSchema, atRevision, es.caveatTypeSet)
557563
if err != nil {
558564
return nil, shared.RewriteErrorWithoutConfig(ctx, err)
559565
}
@@ -761,11 +767,16 @@ func (es *experimentalServer) ExperimentalCountRelationships(ctx context.Context
761767
return nil, spiceerrors.MustBugf("count should not be negative")
762768
}
763769

770+
readAt, err := zedtoken.NewFromRevision(ctx, headRev, ds)
771+
if err != nil {
772+
return nil, shared.RewriteErrorWithoutConfig(ctx, err)
773+
}
774+
764775
return &v1.ExperimentalCountRelationshipsResponse{
765776
CounterResult: &v1.ExperimentalCountRelationshipsResponse_ReadCounterValue{
766777
ReadCounterValue: &v1.ReadCounterValue{
767778
RelationshipCount: uintCount,
768-
ReadAt: zedtoken.MustNewFromRevision(headRev),
779+
ReadAt: readAt,
769780
},
770781
},
771782
}, nil

‎internal/services/v1/expreflection.go

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,15 @@
11
package v1
22

33
import (
4+
"context"
5+
"fmt"
46
"sort"
57
"strings"
68

79
v1 "github.com/authzed/authzed-go/proto/authzed/api/v1"
810
"golang.org/x/exp/maps"
911

12+
datastoremw "github.com/authzed/spicedb/internal/middleware/datastore"
1013
"github.com/authzed/spicedb/pkg/caveats"
1114
caveattypes "github.com/authzed/spicedb/pkg/caveats/types"
1215
"github.com/authzed/spicedb/pkg/datastore"
@@ -177,6 +180,7 @@ func (sf *expSchemaFilters) HasPermission(namespaceName, permissionName string)
177180

178181
// expConvertDiff converts a schema diff into an API response.
179182
func expConvertDiff(
183+
ctx context.Context,
180184
diff *diff.SchemaDiff,
181185
existingSchema *diff.DiffableSchema,
182186
comparisonSchema *diff.DiffableSchema,
@@ -514,9 +518,15 @@ func expConvertDiff(
514518
}
515519
}
516520

521+
ds := datastoremw.MustFromContext(ctx)
522+
zedToken, err := zedtoken.NewFromRevision(ctx, atRevision, ds)
523+
if err != nil {
524+
return nil, fmt.Errorf("failed to create zed token: %w", err)
525+
}
526+
517527
return &v1.ExperimentalDiffSchemaResponse{
518528
Diffs: diffs,
519-
ReadAt: zedtoken.MustNewFromRevision(atRevision),
529+
ReadAt: zedToken,
520530
}, nil
521531
}
522532

‎internal/services/v1/expreflection_test.go

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,18 @@
11
package v1
22

33
import (
4+
"context"
45
"reflect"
56
"strings"
67
"testing"
8+
"time"
79

810
v1 "github.com/authzed/authzed-go/proto/authzed/api/v1"
911
"github.com/ettle/strcase"
1012
"github.com/stretchr/testify/require"
1113

14+
"github.com/authzed/spicedb/internal/datastore/dsfortesting"
15+
datastoremw "github.com/authzed/spicedb/internal/middleware/datastore"
1216
caveattypes "github.com/authzed/spicedb/pkg/caveats/types"
1317
"github.com/authzed/spicedb/pkg/datastore/revisionparsing"
1418
"github.com/authzed/spicedb/pkg/diff"
@@ -545,7 +549,14 @@ func TestExpConvertDiff(t *testing.T) {
545549
diff, err := diff.DiffSchemas(es, cs, caveattypes.Default.TypeSet)
546550
require.NoError(t, err)
547551

552+
ds, err := dsfortesting.NewMemDBDatastoreForTesting(100, 1*time.Second, 100*time.Minute)
553+
require.NoError(t, err)
554+
555+
ctx := context.Background()
556+
ctx = datastoremw.ContextWithDatastore(ctx, ds)
557+
548558
resp, err := expConvertDiff(
559+
ctx,
549560
diff,
550561
&es,
551562
&cs,
@@ -555,10 +566,8 @@ func TestExpConvertDiff(t *testing.T) {
555566
if err != nil {
556567
t.Fatalf("unexpected error: %v", err)
557568
}
558-
require.NotNil(t, resp.ReadAt)
559-
resp.ReadAt = nil
560569

561-
testutil.RequireProtoEqual(t, tc.expectedResponse, resp, "got mismatch")
570+
testutil.RequireProtoEqual(t, tc.expectedResponse, &v1.ExperimentalDiffSchemaResponse{Diffs: resp.Diffs}, "got mismatch")
562571

563572
for _, diff := range resp.Diffs {
564573
name := reflect.TypeOf(diff.GetDiff()).String()

‎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)