From 9b9b291c8b7128984bb76b4cea1988e291cacfbe Mon Sep 17 00:00:00 2001 From: Tanner Stirrat Date: Thu, 12 Sep 2024 12:40:12 -0600 Subject: [PATCH] Lots of lint fixes --- internal/datastore/crdb/crdb.go | 12 +++++-- internal/datastore/crdb/pool/pool.go | 9 ++++-- internal/datastore/crdb/readwrite.go | 7 +++- internal/datastore/crdb/stats.go | 8 ++++- internal/datastore/mysql/gc.go | 9 +++++- internal/datastore/mysql/readwrite.go | 8 ++++- internal/datastore/mysql/revisions.go | 10 +++++- internal/datastore/mysql/stats.go | 9 +++++- internal/datastore/postgres/common/bulk.go | 8 ++++- .../postgres/postgres_shared_test.go | 1 + internal/datastore/postgres/revisions.go | 7 +++- internal/datastore/postgres/revisions_test.go | 8 +++-- internal/datastore/postgres/snapshot.go | 5 ++- internal/datastore/postgres/watch.go | 9 +++++- internal/datastore/proxy/proxy_test/mock.go | 5 ++- .../proxy/schemacaching/estimatedsize_test.go | 22 ++++++++----- .../datastore/revisions/optimized_test.go | 5 ++- internal/datastore/spanner/readwrite.go | 8 ++++- internal/datastore/spanner/stats.go | 15 +++++++-- internal/developmentmembership/onrset.go | 6 +++- .../integrationtesting/benchmark_test.go | 1 + internal/services/v1/experimental.go | 9 +++++- internal/services/v1/experimental_test.go | 29 +++++++++-------- .../v1/options/zz_generated.query_options.go | 3 +- internal/services/v1/relationships.go | 10 +++--- pkg/cache/cache.go | 4 ++- pkg/datastore/test/bulk.go | 9 ++++-- pkg/datastore/test/pagination.go | 7 ++-- pkg/datastore/test/tuples.go | 4 ++- pkg/schemadsl/compiler/translator.go | 9 ++++-- pkg/validationfile/blocks/assertions.go | 22 +++++++++---- .../blocks/expectedrelations.go | 32 +++++++++++++++---- pkg/validationfile/blocks/relationships.go | 19 ++++++++--- pkg/validationfile/blocks/schema.go | 15 +++++++-- 34 files changed, 266 insertions(+), 78 deletions(-) diff --git a/internal/datastore/crdb/crdb.go b/internal/datastore/crdb/crdb.go index dc255b61d9..6a6855f9d4 100644 --- a/internal/datastore/crdb/crdb.go +++ b/internal/datastore/crdb/crdb.go @@ -10,6 +10,7 @@ import ( "github.com/IBM/pgxpoolprometheus" sq "github.com/Masterminds/squirrel" + "github.com/ccoveille/go-safecast" "github.com/jackc/pgx/v5" "github.com/jackc/pgx/v5/pgconn" "github.com/jackc/pgx/v5/pgxpool" @@ -28,6 +29,7 @@ import ( log "github.com/authzed/spicedb/internal/logging" "github.com/authzed/spicedb/pkg/datastore" "github.com/authzed/spicedb/pkg/datastore/options" + "github.com/authzed/spicedb/pkg/spiceerrors" ) func init() { @@ -420,8 +422,14 @@ func (cds *crdbDatastore) ReadyState(ctx context.Context) (datastore.ReadyState, if writeMin > 0 { writeMin-- } - writeTotal := uint32(cds.writePool.Stat().TotalConns()) - readTotal := uint32(cds.readPool.Stat().TotalConns()) + writeTotal, err := safecast.ToUint32(cds.writePool.Stat().TotalConns()) + if err != nil { + return datastore.ReadyState{}, spiceerrors.MustBugf("could not cast writeTotal to uint32") + } + readTotal, err := safecast.ToUint32(cds.readPool.Stat().TotalConns()) + if err != nil { + return datastore.ReadyState{}, spiceerrors.MustBugf("could not cast readTotal to uint32") + } if writeTotal < writeMin || readTotal < readMin { return datastore.ReadyState{ Message: fmt.Sprintf( diff --git a/internal/datastore/crdb/pool/pool.go b/internal/datastore/crdb/pool/pool.go index 3c674935da..5c4ab2dab5 100644 --- a/internal/datastore/crdb/pool/pool.go +++ b/internal/datastore/crdb/pool/pool.go @@ -8,6 +8,7 @@ import ( "sync" "time" + "github.com/ccoveille/go-safecast" "github.com/jackc/pgx/v5" "github.com/jackc/pgx/v5/pgconn" "github.com/jackc/pgx/v5/pgxpool" @@ -141,12 +142,16 @@ func (p *RetryPool) ID() string { // MaxConns returns the MaxConns configured on the underlying pool func (p *RetryPool) MaxConns() uint32 { - return uint32(p.pool.Config().MaxConns) + // This should be non-negative + maxConns, _ := safecast.ToUint32(p.pool.Config().MaxConns) + return maxConns } // MinConns returns the MinConns configured on the underlying pool func (p *RetryPool) MinConns() uint32 { - return uint32(p.pool.Config().MinConns) + // This should be non-negative + minConns, _ := safecast.ToUint32(p.pool.Config().MinConns) + return minConns } // ExecFunc is a replacement for pgxpool.Pool.Exec that allows resetting the diff --git a/internal/datastore/crdb/readwrite.go b/internal/datastore/crdb/readwrite.go index c7c25c0de4..410e4e9ae1 100644 --- a/internal/datastore/crdb/readwrite.go +++ b/internal/datastore/crdb/readwrite.go @@ -8,6 +8,7 @@ import ( sq "github.com/Masterminds/squirrel" v1 "github.com/authzed/authzed-go/proto/authzed/api/v1" + "github.com/ccoveille/go-safecast" "github.com/jackc/pgx/v5" "github.com/jzelinskie/stringz" @@ -430,7 +431,11 @@ func (rwt *crdbReadWriteTXN) DeleteRelationships(ctx context.Context, filter *v1 } rwt.relCountChange -= modified.RowsAffected() - if delLimit > 0 && uint64(modified.RowsAffected()) == delLimit { + rowsAffected, err := safecast.ToUint64(modified.RowsAffected()) + if err != nil { + return false, spiceerrors.MustBugf("could not cast RowsAffected to uint64") + } + if delLimit > 0 && rowsAffected == delLimit { return true, nil } diff --git a/internal/datastore/crdb/stats.go b/internal/datastore/crdb/stats.go index ea3d3e6706..dd28426ae4 100644 --- a/internal/datastore/crdb/stats.go +++ b/internal/datastore/crdb/stats.go @@ -6,11 +6,13 @@ import ( "slices" "github.com/Masterminds/squirrel" + "github.com/ccoveille/go-safecast" "github.com/jackc/pgx/v5" "github.com/rs/zerolog/log" pgxcommon "github.com/authzed/spicedb/internal/datastore/postgres/common" "github.com/authzed/spicedb/pkg/datastore" + "github.com/authzed/spicedb/pkg/spiceerrors" ) const ( @@ -118,7 +120,11 @@ func (cds *crdbDatastore) Statistics(ctx context.Context) (datastore.Stats, erro return nil } - estimatedRelCount = uint64(rowCount) + uintRowCount, err := safecast.ToUint64(rowCount) + if err != nil { + return spiceerrors.MustBugf("row count was negative") + } + estimatedRelCount = uintRowCount return nil } } diff --git a/internal/datastore/mysql/gc.go b/internal/datastore/mysql/gc.go index 255a791a36..e890e41490 100644 --- a/internal/datastore/mysql/gc.go +++ b/internal/datastore/mysql/gc.go @@ -6,11 +6,13 @@ import ( "time" sq "github.com/Masterminds/squirrel" + "github.com/ccoveille/go-safecast" "github.com/authzed/spicedb/internal/datastore/common" "github.com/authzed/spicedb/internal/datastore/revisions" log "github.com/authzed/spicedb/internal/logging" "github.com/authzed/spicedb/pkg/datastore" + "github.com/authzed/spicedb/pkg/spiceerrors" ) var _ common.GarbageCollector = (*Datastore)(nil) @@ -64,7 +66,12 @@ func (mds *Datastore) TxIDBefore(ctx context.Context, before time.Time) (datasto return datastore.NoRevision, nil } - return revisions.NewForTransactionID(uint64(value.Int64)), nil + uintValue, err := safecast.ToUint64(value.Int64) + if err != nil { + return datastore.NoRevision, spiceerrors.MustBugf("value could not be cast to uint64") + } + + return revisions.NewForTransactionID(uintValue), nil } // - implementation misses metrics diff --git a/internal/datastore/mysql/readwrite.go b/internal/datastore/mysql/readwrite.go index 26c7f4a017..f351764e2a 100644 --- a/internal/datastore/mysql/readwrite.go +++ b/internal/datastore/mysql/readwrite.go @@ -13,6 +13,7 @@ import ( sq "github.com/Masterminds/squirrel" v1 "github.com/authzed/authzed-go/proto/authzed/api/v1" + "github.com/ccoveille/go-safecast" "github.com/go-sql-driver/mysql" "github.com/jzelinskie/stringz" @@ -374,7 +375,12 @@ func (rwt *mysqlReadWriteTXN) DeleteRelationships(ctx context.Context, filter *v return false, fmt.Errorf(errUnableToDeleteRelationships, err) } - if delLimit > 0 && uint64(rowsAffected) == delLimit { + uintRowsAffected, err := safecast.ToUint64(rowsAffected) + if err != nil { + return false, spiceerrors.MustBugf("rowsAffected was negative") + } + + if delLimit > 0 && uintRowsAffected == delLimit { return true, nil } diff --git a/internal/datastore/mysql/revisions.go b/internal/datastore/mysql/revisions.go index 5570e670a9..aad60cc432 100644 --- a/internal/datastore/mysql/revisions.go +++ b/internal/datastore/mysql/revisions.go @@ -7,8 +7,11 @@ import ( "fmt" "time" + "github.com/ccoveille/go-safecast" + "github.com/authzed/spicedb/internal/datastore/revisions" "github.com/authzed/spicedb/pkg/datastore" + "github.com/authzed/spicedb/pkg/spiceerrors" ) var ParseRevisionString = revisions.RevisionParser(revisions.TransactionID) @@ -175,5 +178,10 @@ func (mds *Datastore) createNewTransaction(ctx context.Context, tx *sql.Tx) (new return 0, fmt.Errorf("createNewTransaction: failed to get last inserted id: %w", err) } - return uint64(lastInsertID), nil + uintLastInsertID, err := safecast.ToUint64(lastInsertID) + if err != nil { + return 0, spiceerrors.MustBugf("lastInsertID was negative") + } + + return uintLastInsertID, nil } diff --git a/internal/datastore/mysql/stats.go b/internal/datastore/mysql/stats.go index b4bf0d2954..d4efcc4320 100644 --- a/internal/datastore/mysql/stats.go +++ b/internal/datastore/mysql/stats.go @@ -6,9 +6,11 @@ import ( "fmt" "github.com/Masterminds/squirrel" + "github.com/ccoveille/go-safecast" "github.com/authzed/spicedb/internal/datastore/common" "github.com/authzed/spicedb/pkg/datastore" + "github.com/authzed/spicedb/pkg/spiceerrors" ) const ( @@ -74,10 +76,15 @@ func (mds *Datastore) Statistics(ctx context.Context) (datastore.Stats, error) { return datastore.Stats{}, fmt.Errorf("unable to load namespaces: %w", err) } + uintCount, err := safecast.ToUint64(count.Int64) + if err != nil { + return datastore.Stats{}, spiceerrors.MustBugf("could not cast count to uint64") + } + return datastore.Stats{ UniqueID: uniqueID, ObjectTypeStatistics: datastore.ComputeObjectTypeStats(nsDefs), - EstimatedRelationshipCount: uint64(count.Int64), + EstimatedRelationshipCount: uintCount, }, nil } diff --git a/internal/datastore/postgres/common/bulk.go b/internal/datastore/postgres/common/bulk.go index d7c8e8d8fd..613d8cefbd 100644 --- a/internal/datastore/postgres/common/bulk.go +++ b/internal/datastore/postgres/common/bulk.go @@ -3,10 +3,12 @@ package common import ( "context" + "github.com/ccoveille/go-safecast" "github.com/jackc/pgx/v5" "github.com/authzed/spicedb/pkg/datastore" core "github.com/authzed/spicedb/pkg/proto/core/v1" + "github.com/authzed/spicedb/pkg/spiceerrors" ) type tupleSourceAdapter struct { @@ -74,5 +76,9 @@ func BulkLoad( colNames: colNames, } copied, err := tx.CopyFrom(ctx, pgx.Identifier{tupleTableName}, colNames, adapter) - return uint64(copied), err + uintCopied, castErr := safecast.ToUint64(copied) + if castErr != nil { + return 0, spiceerrors.MustBugf("number copied was negative") + } + return uintCopied, err } diff --git a/internal/datastore/postgres/postgres_shared_test.go b/internal/datastore/postgres/postgres_shared_test.go index b1933b9613..d89b379506 100644 --- a/internal/datastore/postgres/postgres_shared_test.go +++ b/internal/datastore/postgres/postgres_shared_test.go @@ -7,6 +7,7 @@ import ( "context" "errors" "fmt" + "math" "math/rand" "strings" "sync" diff --git a/internal/datastore/postgres/revisions.go b/internal/datastore/postgres/revisions.go index e923f8f4ae..d25291778b 100644 --- a/internal/datastore/postgres/revisions.go +++ b/internal/datastore/postgres/revisions.go @@ -173,10 +173,15 @@ func parseRevisionProto(revisionStr string) (datastore.Revision, error) { } } + xmax, err := safecast.ToUint64(xminInt + decoded.RelativeXmax) + if err != nil { + return datastore.NoRevision, spiceerrors.MustBugf("could not cast xmax to int64") + } + return postgresRevision{ snapshot: pgSnapshot{ xmin: decoded.Xmin, - xmax: uint64(xminInt + decoded.RelativeXmax), + xmax: xmax, xipList: xips, }, optionalTxID: xid8{Uint64: decoded.OptionalTxid, Valid: decoded.OptionalTxid != 0}, diff --git a/internal/datastore/postgres/revisions_test.go b/internal/datastore/postgres/revisions_test.go index 126830ce2c..c8ebfe0d57 100644 --- a/internal/datastore/postgres/revisions_test.go +++ b/internal/datastore/postgres/revisions_test.go @@ -6,6 +6,7 @@ import ( "testing" "time" + "github.com/ccoveille/go-safecast" "github.com/stretchr/testify/require" ) @@ -50,7 +51,9 @@ func TestRevisionOrdering(t *testing.T) { func TestRevisionSerDe(t *testing.T) { maxSizeList := make([]uint64, 20) for i := range maxSizeList { - maxSizeList[i] = maxInt - uint64(len(maxSizeList)) + uint64(i) + // i should be nonnegative + index, _ := safecast.ToUint64(i) + maxSizeList[i] = maxInt - uint64(len(maxSizeList)) + index } testCases := []struct { @@ -85,7 +88,8 @@ func TestRevisionSerDe(t *testing.T) { } func TestTxIDTimestampAvailable(t *testing.T) { - testTimestamp := uint64(time.Now().Unix()) + // Timestamps should be non-negative + testTimestamp, _ := safecast.ToUint64(time.Now().Unix()) snapshot := snap(0, 5, 1) pgr := postgresRevision{snapshot: snapshot, optionalTxID: newXid8(1), optionalNanosTimestamp: testTimestamp} receivedTimestamp, ok := pgr.OptionalNanosTimestamp() diff --git a/internal/datastore/postgres/snapshot.go b/internal/datastore/postgres/snapshot.go index f20380cff2..163bd1f63b 100644 --- a/internal/datastore/postgres/snapshot.go +++ b/internal/datastore/postgres/snapshot.go @@ -6,6 +6,7 @@ import ( "strconv" "strings" + "github.com/ccoveille/go-safecast" "github.com/jackc/pgx/v5/pgtype" ) @@ -258,7 +259,9 @@ func (s pgSnapshot) markInProgress(txid uint64) pgSnapshot { var numToDrop int startingXipLen := len(newSnapshot.xipList) for numToDrop = 0; numToDrop < startingXipLen; numToDrop++ { - if newSnapshot.xipList[startingXipLen-1-numToDrop] != newSnapshot.xmax-uint64(numToDrop)-1 { + // numToDrop should be nonnegative + uintNumToDrop, _ := safecast.ToUint64(numToDrop) + if newSnapshot.xipList[startingXipLen-1-numToDrop] != newSnapshot.xmax-uintNumToDrop-1 { break } } diff --git a/internal/datastore/postgres/watch.go b/internal/datastore/postgres/watch.go index a562c4adfe..ab1a353c2c 100644 --- a/internal/datastore/postgres/watch.go +++ b/internal/datastore/postgres/watch.go @@ -7,6 +7,7 @@ import ( "time" sq "github.com/Masterminds/squirrel" + "github.com/ccoveille/go-safecast" "github.com/jackc/pgx/v5" "google.golang.org/protobuf/types/known/structpb" @@ -14,6 +15,7 @@ import ( pgxcommon "github.com/authzed/spicedb/internal/datastore/postgres/common" "github.com/authzed/spicedb/pkg/datastore" core "github.com/authzed/spicedb/pkg/proto/core/v1" + "github.com/authzed/spicedb/pkg/spiceerrors" ) const ( @@ -204,10 +206,15 @@ func (pgd *pgDatastore) getNewRevisions(ctx context.Context, afterTX postgresRev return fmt.Errorf("unable to decode new revision: %w", err) } + nanosTimestamp, err := safecast.ToUint64(timestamp.UnixNano()) + if err != nil { + return spiceerrors.MustBugf("could not cast timestamp to uint64") + } + ids = append(ids, postgresRevision{ snapshot: nextSnapshot.markComplete(nextXID.Uint64), optionalTxID: nextXID, - optionalNanosTimestamp: uint64(timestamp.UnixNano()), + optionalNanosTimestamp: nanosTimestamp, }) } if rows.Err() != nil { diff --git a/internal/datastore/proxy/proxy_test/mock.go b/internal/datastore/proxy/proxy_test/mock.go index 0b5af2254e..3b109c39de 100644 --- a/internal/datastore/proxy/proxy_test/mock.go +++ b/internal/datastore/proxy/proxy_test/mock.go @@ -4,6 +4,7 @@ import ( "context" v1 "github.com/authzed/authzed-go/proto/authzed/api/v1" + "github.com/ccoveille/go-safecast" "github.com/stretchr/testify/mock" "github.com/authzed/spicedb/pkg/datastore" @@ -289,7 +290,9 @@ func (dm *MockReadWriteTransaction) DeleteNamespaces(_ context.Context, nsNames func (dm *MockReadWriteTransaction) BulkLoad(_ context.Context, iter datastore.BulkWriteRelationshipSource) (uint64, error) { args := dm.Called(iter) - return uint64(args.Int(0)), args.Error(1) + // We're assuming this is non-negative. + uintArg, _ := safecast.ToUint64(args.Int(0)) + return uintArg, args.Error(1) } func (dm *MockReadWriteTransaction) ReadCaveatByName(_ context.Context, name string) (*core.CaveatDefinition, datastore.Revision, error) { diff --git a/internal/datastore/proxy/schemacaching/estimatedsize_test.go b/internal/datastore/proxy/schemacaching/estimatedsize_test.go index 134079f87d..7848d1843f 100644 --- a/internal/datastore/proxy/schemacaching/estimatedsize_test.go +++ b/internal/datastore/proxy/schemacaching/estimatedsize_test.go @@ -11,6 +11,7 @@ import ( "testing" "time" + "github.com/ccoveille/go-safecast" "github.com/stretchr/testify/require" "github.com/authzed/spicedb/internal/datastore/memdb" @@ -44,11 +45,12 @@ func TestEstimatedDefinitionSizes(t *testing.T) { for _, filePath := range consistencyTestFiles { filePath := filePath t.Run(path.Base(filePath), func(t *testing.T) { + require := require.New(t) ds, err := memdb.NewMemdbDatastore(0, 1*time.Second, memdb.DisableGC) - require.NoError(t, err) + require.NoError(err) fullyResolved, _, err := validationfile.PopulateFromFiles(context.Background(), ds, []string{filePath}) - require.NoError(t, err) + require.NoError(err) for _, nsDef := range fullyResolved.NamespaceDefinitions { nsDef := nsDef @@ -68,19 +70,21 @@ func TestEstimatedDefinitionSizes(t *testing.T) { runtime.ReadMemStats(&m1) var def core.NamespaceDefinition - require.NoError(t, def.UnmarshalVT(serialized)) + require.NoError(def.UnmarshalVT(serialized)) runtime.ReadMemStats(&m2) used := m2.TotalAlloc - m1.TotalAlloc // Ensure the memory used is less than the SizeVT * the multiplier. - if used <= uint64(estimated) { + uintEstimated, err := safecast.ToUint64(estimated) + require.NoError(err) + if used <= uintEstimated { succeeded = true break } } - require.True(t, succeeded, "found size %d, for with SizeVT: %d", used, sizevt) + require.True(succeeded, "found size %d, for with SizeVT: %d", used, sizevt) }) } @@ -102,19 +106,21 @@ func TestEstimatedDefinitionSizes(t *testing.T) { runtime.ReadMemStats(&m1) var def core.CaveatDefinition - require.NoError(t, def.UnmarshalVT(serialized)) + require.NoError(def.UnmarshalVT(serialized)) runtime.ReadMemStats(&m2) used := m2.TotalAlloc - m1.TotalAlloc // Ensure the memory used is less than the SizeVT * the multiplier. - if used <= uint64(estimated) { + uintEstimated, err := safecast.ToUint64(estimated) + require.NoError(err) + if used <= uintEstimated { succeeded = true break } } - require.True(t, succeeded, "found size %d, for with SizeVT: %d", used, sizevt) + require.True(succeeded, "found size %d, for with SizeVT: %d", used, sizevt) }) } }) diff --git a/internal/datastore/revisions/optimized_test.go b/internal/datastore/revisions/optimized_test.go index 34f310a80b..d819b1ca4f 100644 --- a/internal/datastore/revisions/optimized_test.go +++ b/internal/datastore/revisions/optimized_test.go @@ -7,6 +7,7 @@ import ( "time" "github.com/benbjohnson/clock" + "github.com/ccoveille/go-safecast" "github.com/samber/lo" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" @@ -191,7 +192,9 @@ func BenchmarkOptimizedRevisions(b *testing.B) { nowNS := time.Now().UnixNano() validForNS := nowNS % quantization.Nanoseconds() roundedNS := nowNS - validForNS - rev := NewForTransactionID(uint64(roundedNS)) + // This should be non-negative. + uintRoundedNs, _ := safecast.ToUint64(roundedNS) + rev := NewForTransactionID(uintRoundedNs) return rev, time.Duration(validForNS) * time.Nanosecond, nil }) diff --git a/internal/datastore/spanner/readwrite.go b/internal/datastore/spanner/readwrite.go index dac9b46b91..72852ab997 100644 --- a/internal/datastore/spanner/readwrite.go +++ b/internal/datastore/spanner/readwrite.go @@ -8,6 +8,7 @@ import ( "cloud.google.com/go/spanner" sq "github.com/Masterminds/squirrel" v1 "github.com/authzed/authzed-go/proto/authzed/api/v1" + "github.com/ccoveille/go-safecast" "github.com/jzelinskie/stringz" "github.com/authzed/spicedb/internal/datastore/revisions" @@ -179,7 +180,12 @@ func deleteWithFilter(ctx context.Context, rwt *spanner.ReadWriteTransaction, fi numDeleted = nu } - if delLimit > 0 && uint64(numDeleted) == delLimit { + uintNumDeleted, err := safecast.ToUint64(numDeleted) + if err != nil { + return false, spiceerrors.MustBugf("numDeleted was negative") + } + + if delLimit > 0 && uintNumDeleted == delLimit { return true, nil } diff --git a/internal/datastore/spanner/stats.go b/internal/datastore/spanner/stats.go index 1107c35702..2dd422bc7a 100644 --- a/internal/datastore/spanner/stats.go +++ b/internal/datastore/spanner/stats.go @@ -8,8 +8,11 @@ import ( "cloud.google.com/go/spanner" "go.opentelemetry.io/otel/trace" + "github.com/ccoveille/go-safecast" + "github.com/authzed/spicedb/pkg/datastore" core "github.com/authzed/spicedb/pkg/proto/core/v1" + "github.com/authzed/spicedb/pkg/spiceerrors" ) var querySomeRandomRelationships = fmt.Sprintf(`SELECT %s FROM %s LIMIT 10`, @@ -101,7 +104,10 @@ func (sd *spannerDatastore) Statistics(ctx context.Context) (datastore.Stats, er }, nil } - estimatedBytesPerRelationship = uint64(totalByteCount / totalRelationships) + estimatedBytesPerRelationship, err = safecast.ToUint64(totalByteCount / totalRelationships) + if err != nil { + return datastore.Stats{}, spiceerrors.MustBugf("could not cast estimated bytes to uint64") + } if estimatedBytesPerRelationship > 0 { sd.cachedEstimatedBytesPerRelationshipLock.Lock() sd.cachedEstimatedBytesPerRelationship = estimatedBytesPerRelationship @@ -139,9 +145,14 @@ func (sd *spannerDatastore) Statistics(ctx context.Context) (datastore.Stats, er } } + uintByteEstimate, err := safecast.ToUint64(byteEstimate.Int64) + if err != nil { + return datastore.Stats{}, spiceerrors.MustBugf("unable to cast byteEstimate to uint64") + } + return datastore.Stats{ UniqueID: uniqueID, ObjectTypeStatistics: datastore.ComputeObjectTypeStats(allNamespaces), - EstimatedRelationshipCount: uint64(byteEstimate.Int64) / estimatedBytesPerRelationship, + EstimatedRelationshipCount: uintByteEstimate / estimatedBytesPerRelationship, }, nil } diff --git a/internal/developmentmembership/onrset.go b/internal/developmentmembership/onrset.go index caeea58f03..1da1cb5717 100644 --- a/internal/developmentmembership/onrset.go +++ b/internal/developmentmembership/onrset.go @@ -1,6 +1,8 @@ package developmentmembership import ( + "github.com/ccoveille/go-safecast" + "github.com/authzed/spicedb/pkg/genutil/mapz" core "github.com/authzed/spicedb/pkg/proto/core/v1" ) @@ -28,7 +30,9 @@ func NewONRSet(onrs ...*core.ObjectAndRelation) ONRSet { // Length returns the size of the set. func (ons ONRSet) Length() uint64 { - return uint64(ons.onrs.Len()) + // This is the length of a set so we should never fall out of bounds. + length, _ := safecast.ToUint64(ons.onrs.Len()) + return length } // IsEmpty returns whether the set is empty. diff --git a/internal/services/integrationtesting/benchmark_test.go b/internal/services/integrationtesting/benchmark_test.go index 46d7624453..ce93f64a41 100644 --- a/internal/services/integrationtesting/benchmark_test.go +++ b/internal/services/integrationtesting/benchmark_test.go @@ -12,6 +12,7 @@ import ( "github.com/stretchr/testify/require" v1 "github.com/authzed/authzed-go/proto/authzed/api/v1" + datastoremw "github.com/authzed/spicedb/internal/middleware/datastore" "github.com/authzed/spicedb/internal/services/integrationtesting/consistencytestutil" "github.com/authzed/spicedb/internal/testserver" diff --git a/internal/services/v1/experimental.go b/internal/services/v1/experimental.go index e1e9e563c0..5ef57b5194 100644 --- a/internal/services/v1/experimental.go +++ b/internal/services/v1/experimental.go @@ -11,6 +11,8 @@ import ( "google.golang.org/grpc/codes" + "github.com/ccoveille/go-safecast" + "github.com/authzed/spicedb/internal/dispatch" log "github.com/authzed/spicedb/internal/logging" "github.com/authzed/spicedb/internal/middleware" @@ -702,10 +704,15 @@ func (es *experimentalServer) ExperimentalCountRelationships(ctx context.Context return nil, shared.RewriteErrorWithoutConfig(ctx, err) } + uintCount, err := safecast.ToUint64(count) + if err != nil { + return nil, spiceerrors.MustBugf("count should not be negative") + } + return &v1.ExperimentalCountRelationshipsResponse{ CounterResult: &v1.ExperimentalCountRelationshipsResponse_ReadCounterValue{ ReadCounterValue: &v1.ReadCounterValue{ - RelationshipCount: uint64(count), + RelationshipCount: uintCount, ReadAt: zedtoken.MustNewFromRevision(headRev), }, }, diff --git a/internal/services/v1/experimental_test.go b/internal/services/v1/experimental_test.go index 15292c0836..5697fc4e14 100644 --- a/internal/services/v1/experimental_test.go +++ b/internal/services/v1/experimental_test.go @@ -39,7 +39,7 @@ const defaultFilterMaximumIDCountForTest = 100 func TestBulkImportRelationships(t *testing.T) { testCases := []struct { name string - batchSize func() int + batchSize func() uint64 numBatches int }{ {"one small batch", constBatch(1), 1}, @@ -68,13 +68,13 @@ func TestBulkImportRelationships(t *testing.T) { batchSize := tc.batchSize() batch := make([]*v1.Relationship, 0, batchSize) - for i := 0; i < batchSize; i++ { + for i := uint64(0); i < batchSize; i++ { batch = append(batch, rel( tf.DocumentNS.Name, - strconv.Itoa(batchNum)+"_"+strconv.Itoa(i), + strconv.Itoa(batchNum)+"_"+strconv.FormatUint(i, 10), "viewer", tf.UserNS.Name, - strconv.Itoa(i), + strconv.FormatUint(i, 10), "", )) } @@ -84,7 +84,7 @@ func TestBulkImportRelationships(t *testing.T) { }) require.NoError(err) - expectedTotal += uint64(batchSize) + expectedTotal += batchSize } resp, err := writer.CloseAndRecv() @@ -112,18 +112,19 @@ func TestBulkImportRelationships(t *testing.T) { } } -func constBatch(size int) func() int { - return func() int { +func constBatch(size uint64) func() uint64 { + return func() uint64 { return size } } -func randomBatch(minimum, maximum int) func() int { - return func() int { +func randomBatch(minimum, maximum int) func() uint64 { + return func() uint64 { // nolint:gosec // G404 use of non cryptographically secure random number generator is not a security concern here, // as this is only used for generating fixtures in testing. - return rand.Intn(maximum-minimum) + minimum + // This number should be non-negative + return uint64(rand.Intn(maximum-minimum) + minimum) } } @@ -206,7 +207,7 @@ func TestBulkExportRelationships(t *testing.T) { var totalRead uint64 remainingRels := expectedRels.Copy() - require.Equal(totalToWrite, uint64(expectedRels.Size())) + require.Equal(totalToWrite, expectedRels.Size()) var cursor *v1.Cursor var done bool @@ -353,7 +354,7 @@ func TestBulkExportRelationshipsWithFilter(t *testing.T) { _, err = writer.CloseAndRecv() require.NoError(err) - var totalRead uint64 + var totalRead int remainingRels := expectedRels.Copy() var cursor *v1.Cursor @@ -382,7 +383,7 @@ func TestBulkExportRelationshipsWithFilter(t *testing.T) { require.NotEmpty(batch.AfterResultCursor.Token) cursor = batch.AfterResultCursor - totalRead += uint64(len(batch.Relationships)) + totalRead += len(batch.Relationships) for _, rel := range batch.Relationships { if tc.filter != nil { @@ -399,7 +400,7 @@ func TestBulkExportRelationshipsWithFilter(t *testing.T) { cancel() } - require.Equal(uint64(tc.expectedCount), totalRead, "found: %v", foundRels.AsSlice()) + require.Equal(tc.expectedCount, totalRead, "found: %v", foundRels.AsSlice()) require.True(remainingRels.IsEmpty(), "rels were not exported %#v", remainingRels.List()) }) } diff --git a/internal/services/v1/options/zz_generated.query_options.go b/internal/services/v1/options/zz_generated.query_options.go index 5b75b5f6a5..09ec1f9736 100644 --- a/internal/services/v1/options/zz_generated.query_options.go +++ b/internal/services/v1/options/zz_generated.query_options.go @@ -2,9 +2,10 @@ package options import ( + "time" + defaults "github.com/creasty/defaults" helpers "github.com/ecordell/optgen/helpers" - "time" ) type ExperimentalServerOptionsOption func(e *ExperimentalServerOptions) diff --git a/internal/services/v1/relationships.go b/internal/services/v1/relationships.go index 95e21a0294..b8feb62b74 100644 --- a/internal/services/v1/relationships.go +++ b/internal/services/v1/relationships.go @@ -175,7 +175,7 @@ func (ps *permissionServer) ReadRelationships(req *v1.ReadRelationshipsRequest, DispatchCount: 1, }) - limit := 0 + limit := uint64(0) var startCursor options.Cursor rrRequestHash, err := computeReadRelationshipsRequestHash(req) @@ -203,9 +203,9 @@ func (ps *permissionServer) ReadRelationships(req *v1.ReadRelationshipsRequest, pageSize := ps.config.MaxDatastoreReadPageSize if req.OptionalLimit > 0 { - limit = int(req.OptionalLimit) - if uint64(limit) < pageSize { - pageSize = uint64(limit) + limit = uint64(req.OptionalLimit) + if limit < pageSize { + pageSize = limit } } @@ -232,7 +232,7 @@ func (ps *permissionServer) ReadRelationships(req *v1.ReadRelationshipsRequest, } targetRel := tuple.NewRelationship() targetCaveat := &v1.ContextualizedCaveat{} - returnedCount := 0 + var returnedCount uint64 dispatchCursor := &dispatchv1.Cursor{ DispatchVersion: 1, diff --git a/pkg/cache/cache.go b/pkg/cache/cache.go index 7a4c787259..c8a91f77d4 100644 --- a/pkg/cache/cache.go +++ b/pkg/cache/cache.go @@ -3,6 +3,7 @@ package cache import ( "time" + "github.com/ccoveille/go-safecast" "github.com/dustin/go-humanize" "github.com/rs/zerolog" ) @@ -52,8 +53,9 @@ type Config struct { } func (c *Config) MarshalZerologObject(e *zerolog.Event) { + maxCost, _ := safecast.ToUint64(c.MaxCost) e. - Str("maxCost", humanize.IBytes(uint64(c.MaxCost))). + Str("maxCost", humanize.IBytes(maxCost)). Int64("numCounters", c.NumCounters). Dur("defaultTTL", c.DefaultTTL) } diff --git a/pkg/datastore/test/bulk.go b/pkg/datastore/test/bulk.go index 1e20e3eb19..89da428ad6 100644 --- a/pkg/datastore/test/bulk.go +++ b/pkg/datastore/test/bulk.go @@ -6,6 +6,7 @@ import ( "strconv" "testing" + "github.com/ccoveille/go-safecast" "github.com/stretchr/testify/require" "google.golang.org/grpc/codes" @@ -37,10 +38,12 @@ func BulkUploadTest(t *testing.T, tester DatastoreTester) { t, ) + // This is statically defined so we can cast straight. + uintTc, _ := safecast.ToUint64(tc) _, err = ds.ReadWriteTx(ctx, func(ctx context.Context, rwt datastore.ReadWriteTransaction) error { loaded, err := rwt.BulkLoad(ctx, bulkSource) require.NoError(err) - require.Equal(uint64(tc), loaded) + require.Equal(uintTc, loaded) return err }) require.NoError(err) @@ -136,10 +139,12 @@ func BulkUploadEditCaveat(t *testing.T, tester DatastoreTester) { t, ) + // This is statically defined so we can cast straight. + uintTc, _ := safecast.ToUint64(tc) lastRevision, err := ds.ReadWriteTx(ctx, func(ctx context.Context, rwt datastore.ReadWriteTransaction) error { loaded, err := rwt.BulkLoad(ctx, bulkSource) require.NoError(err) - require.Equal(uint64(tc), loaded) + require.Equal(uintTc, loaded) return err }) require.NoError(err) diff --git a/pkg/datastore/test/pagination.go b/pkg/datastore/test/pagination.go index 9a351605ef..fe25022c50 100644 --- a/pkg/datastore/test/pagination.go +++ b/pkg/datastore/test/pagination.go @@ -6,6 +6,7 @@ import ( "sort" "testing" + "github.com/ccoveille/go-safecast" "github.com/samber/lo" "github.com/stretchr/testify/require" @@ -115,7 +116,7 @@ func LimitTest(t *testing.T, tester DatastoreTester) { for _, objectType := range testCases { expected := sortedStandardData(objectType, options.ByResource) for limit := 1; limit <= len(expected)+1; limit++ { - testLimit := uint64(limit) + testLimit, _ := safecast.ToUint64(limit) t.Run(fmt.Sprintf("%s-%d", objectType, limit), func(t *testing.T) { require := require.New(t) ctx := context.Background() @@ -214,7 +215,7 @@ func OrderedLimitTest(t *testing.T, tester DatastoreTester) { } for limit := 1; limit <= len(expected); limit++ { - testLimit := uint64(limit) + testLimit, _ := safecast.ToUint64(limit) t.Run(tc.name, func(t *testing.T) { require := require.New(t) @@ -267,7 +268,7 @@ func ResumeTest(t *testing.T, tester DatastoreTester) { } for batchSize := 1; batchSize <= len(expected); batchSize++ { - testLimit := uint64(batchSize) + testLimit, _ := safecast.ToUint64(batchSize) expected := expected t.Run(fmt.Sprintf("%s-batches-%d", tc.name, batchSize), func(t *testing.T) { diff --git a/pkg/datastore/test/tuples.go b/pkg/datastore/test/tuples.go index 7fe7ea71bd..a022a626e5 100644 --- a/pkg/datastore/test/tuples.go +++ b/pkg/datastore/test/tuples.go @@ -10,6 +10,7 @@ import ( "time" v1 "github.com/authzed/authzed-go/proto/authzed/api/v1" + "github.com/ccoveille/go-safecast" "github.com/stretchr/testify/require" "golang.org/x/sync/errgroup" "google.golang.org/grpc/codes" @@ -185,7 +186,8 @@ func SimpleTest(t *testing.T, tester DatastoreTester) { // Check limit. if len(testTuples) > 1 { - limit := uint64(len(testTuples) - 1) + // This should be non-negative. + limit, _ := safecast.ToUint64(len(testTuples) - 1) iter, err := dsReader.ReverseQueryRelationships(ctx, datastore.SubjectsFilter{ SubjectType: testUserNamespace, }, options.WithLimitForReverse(&limit)) diff --git a/pkg/schemadsl/compiler/translator.go b/pkg/schemadsl/compiler/translator.go index 4c0f491f51..477ad64fdc 100644 --- a/pkg/schemadsl/compiler/translator.go +++ b/pkg/schemadsl/compiler/translator.go @@ -5,6 +5,7 @@ import ( "fmt" "strings" + "github.com/ccoveille/go-safecast" "github.com/jzelinskie/stringz" "github.com/authzed/spicedb/pkg/caveats" @@ -261,9 +262,13 @@ func getSourcePosition(dslNode *dslNode, mapper input.PositionMapper) *core.Sour return nil } + // We're okay with these being zero if the cast fails. + uintLine, _ := safecast.ToUint64(line) + uintCol, _ := safecast.ToUint64(col) + return &core.SourcePosition{ - ZeroIndexedLineNumber: uint64(line), - ZeroIndexedColumnPosition: uint64(col), + ZeroIndexedLineNumber: uintLine, + ZeroIndexedColumnPosition: uintCol, } } diff --git a/pkg/validationfile/blocks/assertions.go b/pkg/validationfile/blocks/assertions.go index 4cb24cf08e..82b832cec8 100644 --- a/pkg/validationfile/blocks/assertions.go +++ b/pkg/validationfile/blocks/assertions.go @@ -6,6 +6,7 @@ import ( "strings" v1 "github.com/authzed/authzed-go/proto/authzed/api/v1" + "github.com/ccoveille/go-safecast" yamlv3 "gopkg.in/yaml.v3" "github.com/authzed/spicedb/pkg/spiceerrors" @@ -81,14 +82,23 @@ func (a *Assertion) UnmarshalYAML(node *yamlv3.Node) error { trimmed := strings.TrimSpace(relationshipWithContextString) + line, err := safecast.ToUint64(node.Line) + if err != nil { + return err + } + column, err := safecast.ToUint64(node.Column) + if err != nil { + return err + } + // Check for caveat context. parts := strings.SplitN(trimmed, " with ", 2) if len(parts) == 0 { return spiceerrors.NewErrorWithSource( fmt.Errorf("error parsing assertion `%s`", trimmed), trimmed, - uint64(node.Line), - uint64(node.Column), + line, + column, ) } @@ -97,8 +107,8 @@ func (a *Assertion) UnmarshalYAML(node *yamlv3.Node) error { return spiceerrors.NewErrorWithSource( fmt.Errorf("error parsing relationship in assertion `%s`", trimmed), trimmed, - uint64(node.Line), - uint64(node.Column), + line, + column, ) } @@ -111,8 +121,8 @@ func (a *Assertion) UnmarshalYAML(node *yamlv3.Node) error { return spiceerrors.NewErrorWithSource( fmt.Errorf("error parsing caveat context in assertion `%s`: %w", trimmed, err), trimmed, - uint64(node.Line), - uint64(node.Column), + line, + column, ) } diff --git a/pkg/validationfile/blocks/expectedrelations.go b/pkg/validationfile/blocks/expectedrelations.go index f58780b6c1..2c4c112799 100644 --- a/pkg/validationfile/blocks/expectedrelations.go +++ b/pkg/validationfile/blocks/expectedrelations.go @@ -8,6 +8,8 @@ import ( yamlv3 "gopkg.in/yaml.v3" + "github.com/ccoveille/go-safecast" + core "github.com/authzed/spicedb/pkg/proto/core/v1" "github.com/authzed/spicedb/pkg/spiceerrors" @@ -59,13 +61,22 @@ func (ors *ObjectRelation) UnmarshalYAML(node *yamlv3.Node) error { return convertYamlError(err) } + line, err := safecast.ToUint64(node.Line) + if err != nil { + return err + } + column, err := safecast.ToUint64(node.Column) + if err != nil { + return err + } + parsed := tuple.ParseONR(ors.ObjectRelationString) if parsed == nil { return spiceerrors.NewErrorWithSource( fmt.Errorf("could not parse %s", ors.ObjectRelationString), ors.ObjectRelationString, - uint64(node.Line), - uint64(node.Column), + line, + column, ) } @@ -122,13 +133,22 @@ func (es *ExpectedSubject) UnmarshalYAML(node *yamlv3.Node) error { return convertYamlError(err) } + line, err := safecast.ToUint64(node.Line) + if err != nil { + return err + } + column, err := safecast.ToUint64(node.Column) + if err != nil { + return err + } + subjectWithExceptions, subErr := es.ValidationString.Subject() if subErr != nil { return spiceerrors.NewErrorWithSource( subErr, subErr.SourceCodeString, - uint64(node.Line)+subErr.LineNumber, - uint64(node.Column)+subErr.ColumnPosition, + line+subErr.LineNumber, + column+subErr.ColumnPosition, ) } @@ -137,8 +157,8 @@ func (es *ExpectedSubject) UnmarshalYAML(node *yamlv3.Node) error { return spiceerrors.NewErrorWithSource( onrErr, onrErr.SourceCodeString, - uint64(node.Line)+onrErr.LineNumber, - uint64(node.Column)+onrErr.ColumnPosition, + line+onrErr.LineNumber, + column+onrErr.ColumnPosition, ) } diff --git a/pkg/validationfile/blocks/relationships.go b/pkg/validationfile/blocks/relationships.go index 47d4289017..562294e598 100644 --- a/pkg/validationfile/blocks/relationships.go +++ b/pkg/validationfile/blocks/relationships.go @@ -5,6 +5,7 @@ import ( "strings" v1 "github.com/authzed/authzed-go/proto/authzed/api/v1" + "github.com/ccoveille/go-safecast" yamlv3 "gopkg.in/yaml.v3" "github.com/authzed/spicedb/pkg/spiceerrors" @@ -44,13 +45,23 @@ func (pr *ParsedRelationships) UnmarshalYAML(node *yamlv3.Node) error { continue } + // +1 for the key, and *2 for newlines in YAML + errorLine, err := safecast.ToUint64(node.Line + 1 + (index * 2)) + if err != nil { + return err + } + column, err := safecast.ToUint64(node.Column) + if err != nil { + return err + } + tpl := tuple.Parse(trimmed) if tpl == nil { return spiceerrors.NewErrorWithSource( fmt.Errorf("error parsing relationship `%s`", trimmed), trimmed, - uint64(node.Line+1+(index*2)), // +1 for the key, and *2 for newlines in YAML - uint64(node.Column), + errorLine, + column, ) } @@ -59,8 +70,8 @@ func (pr *ParsedRelationships) UnmarshalYAML(node *yamlv3.Node) error { return spiceerrors.NewErrorWithSource( fmt.Errorf("found repeated relationship `%s`", trimmed), trimmed, - uint64(node.Line+1+(index*2)), // +1 for the key, and *2 for newlines in YAML - uint64(node.Column), + errorLine, + column, ) } seenTuples[tuple.StringWithoutCaveat(tpl)] = true diff --git a/pkg/validationfile/blocks/schema.go b/pkg/validationfile/blocks/schema.go index 6dd1d31df4..6286c3eda4 100644 --- a/pkg/validationfile/blocks/schema.go +++ b/pkg/validationfile/blocks/schema.go @@ -6,6 +6,8 @@ import ( yamlv3 "gopkg.in/yaml.v3" + "github.com/ccoveille/go-safecast" + "github.com/authzed/spicedb/pkg/schemadsl/compiler" "github.com/authzed/spicedb/pkg/schemadsl/input" "github.com/authzed/spicedb/pkg/spiceerrors" @@ -42,11 +44,20 @@ func (ps *ParsedSchema) UnmarshalYAML(node *yamlv3.Node) error { return lerr } + uintLine, err := safecast.ToUint64(line) + if err != nil { + return err + } + uintCol, err := safecast.ToUint64(col) + if err != nil { + return err + } + return spiceerrors.NewErrorWithSource( fmt.Errorf("error when parsing schema: %s", errWithContext.BaseMessage), errWithContext.ErrorSourceCode, - uint64(line+1), // source line is 0-indexed - uint64(col+1), // source col is 0-indexed + uintLine+1, // source line is 0-indexed + uintCol+1, // source col is 0-indexed ) }