From 8a1ae2c3700dc98aedf9be1b96d5a91c30c1c62a Mon Sep 17 00:00:00 2001 From: Tanner Stirrat Date: Thu, 12 Sep 2024 09:41:13 -0600 Subject: [PATCH] Fix lint issues --- go.mod | 1 + go.sum | 2 ++ internal/cmd/restorer.go | 47 ++++++++++++++++++++++++++--------- internal/cmd/restorer_test.go | 46 ++++++++++++++++++---------------- internal/cmd/schema.go | 7 +++++- internal/cmd/validate.go | 8 ++++-- internal/cmd/version.go | 7 +++++- internal/grpcutil/batch.go | 4 +-- internal/storage/secrets.go | 9 +++++-- 9 files changed, 90 insertions(+), 41 deletions(-) diff --git a/go.mod b/go.mod index a4024e5b..1a9413b9 100644 --- a/go.mod +++ b/go.mod @@ -80,6 +80,7 @@ require ( github.com/beorn7/perks v1.0.1 // indirect github.com/bits-and-blooms/bitset v1.13.0 // indirect github.com/bits-and-blooms/bloom/v3 v3.7.0 // indirect + github.com/ccoveille/go-safecast v1.1.0 // indirect github.com/census-instrumentation/opencensus-proto v0.4.1 // indirect github.com/certifi/gocertifi v0.0.0-20210507211836-431795d63e8d // indirect github.com/cespare/xxhash/v2 v2.3.0 // indirect diff --git a/go.sum b/go.sum index 4c277764..34f0b01a 100644 --- a/go.sum +++ b/go.sum @@ -720,6 +720,8 @@ github.com/boombuler/barcode v1.0.0/go.mod h1:paBWMcWSl3LHKBqUq+rly7CNSldXjb2rDl github.com/boombuler/barcode v1.0.1/go.mod h1:paBWMcWSl3LHKBqUq+rly7CNSldXjb2rDl3JlRe0mD8= github.com/brianvoe/gofakeit/v6 v6.28.0 h1:Xib46XXuQfmlLS2EXRuJpqcw8St6qSZz75OUo0tgAW4= github.com/brianvoe/gofakeit/v6 v6.28.0/go.mod h1:Xj58BMSnFqcn/fAQeSK+/PLtC5kSb7FJIq4JyGa8vEs= +github.com/ccoveille/go-safecast v1.1.0 h1:iHKNWaZm+OznO7Eh6EljXPjGfGQsSfa6/sxPlIEKO+g= +github.com/ccoveille/go-safecast v1.1.0/go.mod h1:QqwNjxQ7DAqY0C721OIO9InMk9zCwcsO7tnRuHytad8= github.com/cenkalti/backoff/v4 v4.3.0 h1:MyRJ/UdXutAwSAT+s3wNd7MfTIcy71VQueUuFK343L8= github.com/cenkalti/backoff/v4 v4.3.0/go.mod h1:Y3VNntkOUPxTVeUxJ/G5vcM//AlwfmyYozVcomhLiZE= github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU= diff --git a/internal/cmd/restorer.go b/internal/cmd/restorer.go index 8b852086..666ddda2 100644 --- a/internal/cmd/restorer.go +++ b/internal/cmd/restorer.go @@ -9,6 +9,7 @@ import ( "time" v1 "github.com/authzed/authzed-go/proto/authzed/api/v1" + "github.com/ccoveille/go-safecast" "github.com/cenkalti/backoff/v4" "github.com/mattn/go-isatty" "github.com/rs/zerolog/log" @@ -225,6 +226,13 @@ func (r *restorer) commitStream(ctx context.Context, bulkImportClient v1.Experim canceled, cancelErr := isCanceledError(ctx.Err(), err) unknown := !retryable && !conflict && !canceled && err != nil + intExpectedLoaded, err := safecast.ToInt64(expectedLoaded) + if err != nil { + return err + } + + intNumBatches := int64(len(batchesToBeCommitted)) + switch { case canceled: r.bar.Describe("backup restore aborted") @@ -235,24 +243,29 @@ func (r *restorer) commitStream(ctx context.Context, bulkImportClient v1.Experim case retryable && r.disableRetryErrors: return err case conflict && r.conflictStrategy == Skip: - r.skippedRels += int64(expectedLoaded) - r.skippedBatches += int64(len(batchesToBeCommitted)) - r.duplicateBatches += int64(len(batchesToBeCommitted)) - r.duplicateRels += int64(expectedLoaded) + r.skippedRels += intExpectedLoaded + r.skippedBatches += intNumBatches + r.duplicateBatches += intNumBatches + r.duplicateRels += intExpectedLoaded r.bar.Describe("skipping conflicting batch") case conflict && r.conflictStrategy == Touch: r.bar.Describe("touching conflicting batch") - r.duplicateRels += int64(expectedLoaded) - r.duplicateBatches += int64(len(batchesToBeCommitted)) + r.duplicateRels += intExpectedLoaded + r.duplicateBatches += intNumBatches r.totalRetries++ numLoaded, retries, err = r.writeBatchesWithRetry(ctx, batchesToBeCommitted) if err != nil { return fmt.Errorf("failed to write retried batch: %w", err) } + intNumLoaded, err := safecast.ToInt64(numLoaded) + if err != nil { + return err + } + retries++ // account for the initial attempt - r.writtenBatches += int64(len(batchesToBeCommitted)) - r.writtenRels += int64(numLoaded) + r.writtenBatches += intNumBatches + r.writtenRels += intNumLoaded case conflict && r.conflictStrategy == Fail: r.bar.Describe("conflict detected, aborting restore") return fmt.Errorf("duplicate relationships found") @@ -264,17 +277,27 @@ func (r *restorer) commitStream(ctx context.Context, bulkImportClient v1.Experim return fmt.Errorf("failed to write retried batch: %w", err) } + intNumLoaded, err := safecast.ToInt64(numLoaded) + if err != nil { + return err + } + retries++ // account for the initial attempt - r.writtenBatches += int64(len(batchesToBeCommitted)) - r.writtenRels += int64(numLoaded) + r.writtenBatches += intNumBatches + r.writtenRels += intNumLoaded default: r.bar.Describe("restoring relationships from backup") - r.writtenBatches += int64(len(batchesToBeCommitted)) + r.writtenBatches += intNumBatches } // it was a successful transaction commit without duplicates if resp != nil { - r.writtenRels += int64(resp.NumLoaded) + intNumLoaded, err := safecast.ToInt64(resp.NumLoaded) + if err != nil { + return err + } + + r.writtenRels += intNumLoaded if expectedLoaded != resp.NumLoaded { log.Warn().Uint64("loaded", resp.NumLoaded).Uint64("expected", expectedLoaded).Msg("unexpected number of relationships loaded") } diff --git a/internal/cmd/restorer_test.go b/internal/cmd/restorer_test.go index 4445ef4a..341fe5dd 100644 --- a/internal/cmd/restorer_test.go +++ b/internal/cmd/restorer_test.go @@ -8,6 +8,7 @@ import ( v1 "github.com/authzed/authzed-go/proto/authzed/api/v1" "github.com/authzed/spicedb/pkg/tuple" + "github.com/ccoveille/go-safecast" "github.com/stretchr/testify/require" "google.golang.org/grpc" "google.golang.org/grpc/codes" @@ -60,12 +61,13 @@ func TestRestorer(t *testing.T) { } { tt := tt t.Run(tt.name, func(t *testing.T) { + require := require.New(t) backupFileName := createTestBackup(t, testSchema, tt.relationships) d, closer, err := decoderFromArgs(backupFileName) - require.NoError(t, err) + require.NoError(err) t.Cleanup(func() { - require.NoError(t, closer.Close()) - require.NoError(t, os.Remove(backupFileName)) + require.NoError(closer.Close()) + require.NoError(os.Remove(backupFileName)) }) expectedFilteredRels := make([]string, 0, len(tt.relationships)) @@ -79,7 +81,9 @@ func TestRestorer(t *testing.T) { expectedBatches := len(expectedFilteredRels) / tt.batchSize // there is always one extra commit, regardless there is or not a remainder batch - expectedCommits := expectedBatches/int(tt.batchesPerTransaction) + 1 + batchesPerTransaction, err := safecast.ToInt(tt.batchesPerTransaction) + require.NoError(err) + expectedCommits := expectedBatches/batchesPerTransaction + 1 remainderBatch := false if len(expectedFilteredRels)%tt.batchSize != 0 { expectedBatches++ @@ -142,8 +146,8 @@ func TestRestorer(t *testing.T) { expectedTouchedBatches := expectedRetries expectedTouchedRels := expectedRetries * tt.batchSize if tt.conflictStrategy == Touch { - expectedTouchedBatches += expectedConflicts * int(tt.batchesPerTransaction) - expectedTouchedRels += expectedConflicts * int(tt.batchesPerTransaction) * tt.batchSize + expectedTouchedBatches += expectedConflicts * batchesPerTransaction + expectedTouchedRels += expectedConflicts * batchesPerTransaction * tt.batchSize } expectedSkippedBatches := 0 @@ -156,28 +160,28 @@ func TestRestorer(t *testing.T) { r := newRestorer(testSchema, d, c, tt.prefixFilter, tt.batchSize, tt.batchesPerTransaction, tt.conflictStrategy, tt.disableRetryErrors, 0*time.Second) err = r.restoreFromDecoder(context.Background()) if expectsError != nil || (expectedConflicts > 0 && tt.conflictStrategy == Fail) { - require.ErrorIs(t, err, expectsError) + require.ErrorIs(err, expectsError) return } - require.NoError(t, err) + require.NoError(err) // assert on mock stats - require.Equal(t, expectedBatches, c.receivedBatches, "unexpected number of received batches") - require.Equal(t, expectedCommits, c.receivedCommits, "unexpected number of batch commits") - require.Equal(t, len(expectedFilteredRels), c.receivedRels, "unexpected number of received relationships") - require.Equal(t, expectedTouchedBatches, c.touchedBatches, "unexpected number of touched batches") - require.Equal(t, expectedTouchedRels, c.touchedRels, "unexpected number of touched commits") + require.Equal(expectedBatches, c.receivedBatches, "unexpected number of received batches") + require.Equal(expectedCommits, c.receivedCommits, "unexpected number of batch commits") + require.Equal(len(expectedFilteredRels), c.receivedRels, "unexpected number of received relationships") + require.Equal(expectedTouchedBatches, c.touchedBatches, "unexpected number of touched batches") + require.Equal(expectedTouchedRels, c.touchedRels, "unexpected number of touched commits") // assert on restorer stats - require.Equal(t, expectedWrittenRels, int(r.writtenRels), "unexpected number of written relationships") - require.Equal(t, expectedWrittenBatches, int(r.writtenBatches), "unexpected number of written relationships") - require.Equal(t, expectedSkippedBatches, int(r.skippedBatches), "unexpected number of conflicting batches skipped") - require.Equal(t, expectedSkippedRels, int(r.skippedRels), "unexpected number of conflicting relationships skipped") - require.Equal(t, uint(expectedConflicts)*tt.batchesPerTransaction, uint(r.duplicateBatches), "unexpected number of duplicate batches detected") - require.Equal(t, expectedConflicts*int(tt.batchesPerTransaction)*tt.batchSize, int(r.duplicateRels), "unexpected number of duplicate relationships detected") - require.Equal(t, int64(expectedRetries+expectedConflicts-expectedSkippedBatches), r.totalRetries, "unexpected number of retries") - require.Equal(t, len(tt.relationships)-len(expectedFilteredRels), int(r.filteredOutRels), "unexpected number of filtered out relationships") + require.Equal(expectedWrittenRels, int(r.writtenRels), "unexpected number of written relationships") + require.Equal(expectedWrittenBatches, int(r.writtenBatches), "unexpected number of written relationships") + require.Equal(expectedSkippedBatches, int(r.skippedBatches), "unexpected number of conflicting batches skipped") + require.Equal(expectedSkippedRels, int(r.skippedRels), "unexpected number of conflicting relationships skipped") + require.Equal(uint(expectedConflicts)*tt.batchesPerTransaction, uint(r.duplicateBatches), "unexpected number of duplicate batches detected") + require.Equal(expectedConflicts*batchesPerTransaction*tt.batchSize, int(r.duplicateRels), "unexpected number of duplicate relationships detected") + require.Equal(int64(expectedRetries+expectedConflicts-expectedSkippedBatches), r.totalRetries, "unexpected number of retries") + require.Equal(len(tt.relationships)-len(expectedFilteredRels), int(r.filteredOutRels), "unexpected number of filtered out relationships") }) } } diff --git a/internal/cmd/schema.go b/internal/cmd/schema.go index 0529aa67..dc8439b3 100644 --- a/internal/cmd/schema.go +++ b/internal/cmd/schema.go @@ -13,6 +13,7 @@ import ( "github.com/authzed/spicedb/pkg/schemadsl/compiler" "github.com/authzed/spicedb/pkg/schemadsl/generator" "github.com/authzed/spicedb/pkg/schemadsl/input" + "github.com/ccoveille/go-safecast" "github.com/jzelinskie/cobrautil/v2" "github.com/jzelinskie/stringz" "github.com/rs/zerolog/log" @@ -119,7 +120,11 @@ func schemaCopyCmdFunc(cmd *cobra.Command, args []string) error { } func schemaWriteCmdFunc(cmd *cobra.Command, args []string) error { - if len(args) == 0 && term.IsTerminal(int(os.Stdin.Fd())) { + intFd, err := safecast.ToInt(uint(os.Stdout.Fd())) + if err != nil { + return err + } + if len(args) == 0 && term.IsTerminal(intFd) { return fmt.Errorf("must provide file path or contents via stdin") } diff --git a/internal/cmd/validate.go b/internal/cmd/validate.go index 3d3d3ef2..a291d6bc 100644 --- a/internal/cmd/validate.go +++ b/internal/cmd/validate.go @@ -7,6 +7,7 @@ import ( "os" "strings" + "github.com/ccoveille/go-safecast" "github.com/spf13/cobra" v1 "github.com/authzed/authzed-go/proto/authzed/api/v1" @@ -166,10 +167,13 @@ func ouputErrorWithSource(validateContents []byte, errWithSource spiceerrors.Err func outputForLine(validateContents []byte, oneIndexedLineNumber uint64, sourceCodeString string, oneIndexedColumnPosition uint64) { lines := strings.Split(string(validateContents), "\n") - errorLineNumber := int(oneIndexedLineNumber) - 1 + // These should be fine to be zero if the cast fails. + intLineNumber, _ := safecast.ToInt(oneIndexedLineNumber) + intColumnPosition, _ := safecast.ToInt(oneIndexedColumnPosition) + errorLineNumber := intLineNumber - 1 for i := errorLineNumber - 3; i < errorLineNumber+3; i++ { if i == errorLineNumber { - renderLine(lines, i, sourceCodeString, errorLineNumber, int(oneIndexedColumnPosition)-1) + renderLine(lines, i, sourceCodeString, errorLineNumber, intColumnPosition-1) } else { renderLine(lines, i, "", errorLineNumber, -1) } diff --git a/internal/cmd/version.go b/internal/cmd/version.go index 03a1e494..f46540e8 100644 --- a/internal/cmd/version.go +++ b/internal/cmd/version.go @@ -6,6 +6,7 @@ import ( "github.com/authzed/authzed-go/pkg/responsemeta" v1 "github.com/authzed/authzed-go/proto/authzed/api/v1" + "github.com/ccoveille/go-safecast" "github.com/gookit/color" "github.com/jzelinskie/cobrautil/v2" "github.com/rs/zerolog/log" @@ -20,7 +21,11 @@ import ( ) func versionCmdFunc(cmd *cobra.Command, _ []string) error { - if !term.IsTerminal(int(os.Stdout.Fd())) { + intFd, err := safecast.ToInt(uint(os.Stdout.Fd())) + if err != nil { + return err + } + if !term.IsTerminal(intFd) { color.Disable() } diff --git a/internal/grpcutil/batch.go b/internal/grpcutil/batch.go index 98929015..640085c6 100644 --- a/internal/grpcutil/batch.go +++ b/internal/grpcutil/batch.go @@ -10,7 +10,7 @@ import ( "golang.org/x/sync/semaphore" ) -func min(a int, b int) int { +func minimum(a int, b int) int { if a <= b { return a } @@ -60,7 +60,7 @@ func ConcurrentBatch(ctx context.Context, n int, batchSize int, maxWorkers int, g.Go(func() error { defer sem.Release(1) start := batchNum * batchSize - end := min(start+batchSize, n) + end := minimum(start+batchSize, n) return each(ctx, batchNum, start, end) }) } diff --git a/internal/storage/secrets.go b/internal/storage/secrets.go index 0a93861f..6ce4db34 100644 --- a/internal/storage/secrets.go +++ b/internal/storage/secrets.go @@ -8,6 +8,7 @@ import ( "strings" "github.com/99designs/keyring" + "github.com/ccoveille/go-safecast" "github.com/jzelinskie/stringz" "golang.org/x/term" @@ -27,7 +28,7 @@ type Token struct { } func (t Token) Certificate() (cert []byte, ok bool) { - if t.CACert != nil && len(t.CACert) > 0 { + if len(t.CACert) > 0 { return t.CACert, true } return nil, false @@ -146,7 +147,11 @@ func fileExists(path string) (bool, error) { func promptPassword(prompt string) (string, error) { console.Printf(prompt) - b, err := term.ReadPassword(int(os.Stdin.Fd())) + intFd, err := safecast.ToInt(uint(os.Stdin.Fd())) + if err != nil { + return "", err + } + b, err := term.ReadPassword(intFd) if err != nil { return "", err }