From e3f6eda6939b9c7ba7e9d53337340ea02f398d50 Mon Sep 17 00:00:00 2001 From: Tanner Stirrat Date: Mon, 9 Sep 2024 10:50:59 -0600 Subject: [PATCH 01/21] Take security patch --- go.mod | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/go.mod b/go.mod index e5b2d6d..aefe179 100644 --- a/go.mod +++ b/go.mod @@ -1,8 +1,8 @@ module github.com/authzed/zed -go 1.22.4 +go 1.22.7 -toolchain go1.22.5 +toolchain go1.22.7 require ( github.com/99designs/keyring v1.2.2 From edd98f3d6ffcd94ad57eb4fa7ca7eef022b91201 Mon Sep 17 00:00:00 2001 From: Tanner Stirrat Date: Tue, 10 Sep 2024 10:38:14 -0600 Subject: [PATCH 02/21] Bump built version --- Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Dockerfile b/Dockerfile index eb4cbf4..4e693d9 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,4 +1,4 @@ -FROM golang:1.22-alpine3.20 AS zed-builder +FROM golang:1.23-alpine3.20 AS zed-builder WORKDIR /go/src/app RUN apk update && apk add --no-cache git COPY . . From 9c3b6b8d4be14d054c0f0c734fd7aa65cb00dc67 Mon Sep 17 00:00:00 2001 From: Tanner Stirrat Date: Tue, 10 Sep 2024 10:45:07 -0600 Subject: [PATCH 03/21] Bump toolchain version --- go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go.mod b/go.mod index aefe179..a4024e5 100644 --- a/go.mod +++ b/go.mod @@ -2,7 +2,7 @@ module github.com/authzed/zed go 1.22.7 -toolchain go1.22.7 +toolchain go1.23.1 require ( github.com/99designs/keyring v1.2.2 From 8a1ae2c3700dc98aedf9be1b96d5a91c30c1c62a Mon Sep 17 00:00:00 2001 From: Tanner Stirrat Date: Thu, 12 Sep 2024 09:41:13 -0600 Subject: [PATCH 04/21] 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 a4024e5..1a9413b 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 4c27776..34f0b01 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 8b85208..666ddda 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 4445ef4..341fe5d 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 0529aa6..dc8439b 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 3d3d3ef..a291d6b 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 03a1e49..f46540e 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 9892901..640085c 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 0a93861..6ce4db3 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 } From b9033f8bcacda6f50daf9409518e082d76922514 Mon Sep 17 00:00:00 2001 From: Tanner Stirrat Date: Thu, 12 Sep 2024 09:50:22 -0600 Subject: [PATCH 05/21] Run go mod tidy --- go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go.mod b/go.mod index 1a9413b..f7fc387 100644 --- a/go.mod +++ b/go.mod @@ -11,6 +11,7 @@ require ( github.com/authzed/grpcutil v0.0.0-20240123194739-2ea1e3d2d98b github.com/authzed/spicedb v1.35.3 github.com/brianvoe/gofakeit/v6 v6.28.0 + github.com/ccoveille/go-safecast v1.1.0 github.com/cenkalti/backoff/v4 v4.3.0 github.com/charmbracelet/lipgloss v0.12.1 github.com/google/uuid v1.6.0 @@ -80,7 +81,6 @@ 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 From 9cfb6f558ab7ee520f9194b5cbb0c86a90f677c2 Mon Sep 17 00:00:00 2001 From: Tanner Stirrat Date: Thu, 12 Sep 2024 10:01:39 -0600 Subject: [PATCH 06/21] Remaining lint fixes --- internal/cmd/restorer.go | 4 +++- internal/cmd/restorer_test.go | 6 +++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/internal/cmd/restorer.go b/internal/cmd/restorer.go index 666ddda..acf042d 100644 --- a/internal/cmd/restorer.go +++ b/internal/cmd/restorer.go @@ -195,6 +195,8 @@ func (r *restorer) restoreFromDecoder(ctx context.Context) error { } totalTime := time.Since(relationshipWriteStart) + // This shouldn't realistically overflow. + writtenAndSkipped, _ := safecast.ToUint64(r.writtenRels + r.skippedRels) log.Info(). Int64("batches", r.writtenBatches). Int64("relationships_loaded", r.writtenRels). @@ -202,7 +204,7 @@ func (r *restorer) restoreFromDecoder(ctx context.Context) error { Int64("duplicate_relationships", r.duplicateRels). Int64("relationships_filtered_out", r.filteredOutRels). Int64("retried_errors", r.totalRetries). - Uint64("perSecond", perSec(uint64(r.writtenRels+r.skippedRels), totalTime)). + Uint64("perSecond", perSec(writtenAndSkipped, totalTime)). Stringer("duration", totalTime). Msg("finished restore") return nil diff --git a/internal/cmd/restorer_test.go b/internal/cmd/restorer_test.go index 341fe5d..617a5fd 100644 --- a/internal/cmd/restorer_test.go +++ b/internal/cmd/restorer_test.go @@ -174,11 +174,15 @@ func TestRestorer(t *testing.T) { require.Equal(expectedTouchedRels, c.touchedRels, "unexpected number of touched commits") // assert on restorer stats + uintExpectedConflicts, err := safecast.ToUint(expectedConflicts) + require.NoError(err) + uintDuplicateBatches, err := safecast.ToUint(r.duplicateBatches) + require.NoError(err) 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(uintExpectedConflicts*tt.batchesPerTransaction, uintDuplicateBatches, "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") From 42a9058cec80d8578a1d4b884cb686384b4d16fc Mon Sep 17 00:00:00 2001 From: Tanner Stirrat Date: Thu, 12 Sep 2024 10:31:58 -0600 Subject: [PATCH 07/21] Don't whack the error reference --- internal/cmd/restorer.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/cmd/restorer.go b/internal/cmd/restorer.go index acf042d..fc05e32 100644 --- a/internal/cmd/restorer.go +++ b/internal/cmd/restorer.go @@ -228,9 +228,9 @@ 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 + intExpectedLoaded, castErr := safecast.ToInt64(expectedLoaded) + if castErr != nil { + return castErr } intNumBatches := int64(len(batchesToBeCommitted)) From c3a80beb189ff90fec2f2438637165c09ac47527 Mon Sep 17 00:00:00 2001 From: Tanner Stirrat Date: Fri, 13 Sep 2024 10:47:22 -0600 Subject: [PATCH 08/21] Use uint instead of int for batch size --- internal/cmd/backup.go | 4 ++-- internal/cmd/backup_test.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/cmd/backup.go b/internal/cmd/backup.go index 6b2586f..95bf910 100644 --- a/internal/cmd/backup.go +++ b/internal/cmd/backup.go @@ -126,7 +126,7 @@ func registerBackupCmd(rootCmd *cobra.Command) { } func registerBackupRestoreFlags(cmd *cobra.Command) { - cmd.Flags().Int("batch-size", 1_000, "restore relationship write batch size") + cmd.Flags().Uint("batch-size", 1_000, "restore relationship write batch size") cmd.Flags().Uint("batches-per-transaction", 10, "number of batches per transaction") cmd.Flags().String("conflict-strategy", "fail", "strategy used when a conflicting relationship is found. Possible values: fail, skip, touch") cmd.Flags().Bool("disable-retries", false, "retries when an errors is determined to be retryable (e.g. serialization errors)") @@ -397,7 +397,7 @@ func backupRestoreCmdFunc(cmd *cobra.Command, args []string) error { return fmt.Errorf("unable to initialize client: %w", err) } - batchSize := cobrautil.MustGetInt(cmd, "batch-size") + batchSize := cobrautil.MustGetUint(cmd, "batch-size") batchesPerTransaction := cobrautil.MustGetUint(cmd, "batches-per-transaction") strategy, err := GetEnum[ConflictStrategy](cmd, "conflict-strategy", conflictStrategyMapping) diff --git a/internal/cmd/backup_test.go b/internal/cmd/backup_test.go index ee4b2ca..41cc77b 100644 --- a/internal/cmd/backup_test.go +++ b/internal/cmd/backup_test.go @@ -330,7 +330,7 @@ func TestBackupRestoreCmdFunc(t *testing.T) { zedtesting.BoolFlag{FlagName: "rewrite-legacy"}, zedtesting.StringFlag{FlagName: "conflict-strategy", FlagValue: "fail"}, zedtesting.BoolFlag{FlagName: "disable-retries"}, - zedtesting.IntFlag{FlagName: "batch-size", FlagValue: 100}, + zedtesting.UintFlag{FlagName: "batch-size", FlagValue: 100}, zedtesting.UintFlag{FlagName: "batches-per-transaction", FlagValue: 10}, zedtesting.DurationFlag{FlagName: "request-timeout"}, ) From a91f5032fb7312b9ba7dda8bfff75b76c4273cf4 Mon Sep 17 00:00:00 2001 From: Tanner Stirrat Date: Fri, 13 Sep 2024 10:51:04 -0600 Subject: [PATCH 09/21] Refactor to use uint for count variables --- internal/cmd/restorer.go | 120 +++++++++++++++------------------- internal/cmd/restorer_test.go | 79 ++++++++++------------ 2 files changed, 90 insertions(+), 109 deletions(-) diff --git a/internal/cmd/restorer.go b/internal/cmd/restorer.go index fc05e32..d5ad1a9 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/authzed/spicedb/pkg/spiceerrors" "github.com/ccoveille/go-safecast" "github.com/cenkalti/backoff/v4" "github.com/mattn/go-isatty" @@ -58,25 +59,25 @@ type restorer struct { decoder *backupformat.Decoder client client.Client prefixFilter string - batchSize int + batchSize uint batchesPerTransaction uint conflictStrategy ConflictStrategy disableRetryErrors bool bar *progressbar.ProgressBar // stats - filteredOutRels int64 - writtenRels int64 - writtenBatches int64 - skippedRels int64 - skippedBatches int64 - duplicateRels int64 - duplicateBatches int64 - totalRetries int64 + filteredOutRels uint + writtenRels uint + writtenBatches uint + skippedRels uint + skippedBatches uint + duplicateRels uint + duplicateBatches uint + totalRetries uint requestTimeout time.Duration } -func newRestorer(schema string, decoder *backupformat.Decoder, client client.Client, prefixFilter string, batchSize int, +func newRestorer(schema string, decoder *backupformat.Decoder, client client.Client, prefixFilter string, batchSize uint, batchesPerTransaction uint, conflictStrategy ConflictStrategy, disableRetryErrors bool, requestTimeout time.Duration, ) *restorer { @@ -130,7 +131,7 @@ func (r *restorer) restoreFromDecoder(ctx context.Context) error { batch = append(batch, rel) - if len(batch)%r.batchSize == 0 { + if uint(len(batch))%r.batchSize == 0 { batchesToBeCommitted = append(batchesToBeCommitted, batch) err := relationshipWriter.Send(&v1.BulkImportRelationshipsRequest{ Relationships: batch, @@ -195,16 +196,14 @@ func (r *restorer) restoreFromDecoder(ctx context.Context) error { } totalTime := time.Since(relationshipWriteStart) - // This shouldn't realistically overflow. - writtenAndSkipped, _ := safecast.ToUint64(r.writtenRels + r.skippedRels) log.Info(). - Int64("batches", r.writtenBatches). - Int64("relationships_loaded", r.writtenRels). - Int64("relationships_skipped", r.skippedRels). - Int64("duplicate_relationships", r.duplicateRels). - Int64("relationships_filtered_out", r.filteredOutRels). - Int64("retried_errors", r.totalRetries). - Uint64("perSecond", perSec(writtenAndSkipped, totalTime)). + Uint("batches", r.writtenBatches). + Uint("relationships_loaded", r.writtenRels). + Uint("relationships_skipped", r.skippedRels). + Uint("duplicate_relationships", r.duplicateRels). + Uint("relationships_filtered_out", r.filteredOutRels). + Uint("retried_errors", r.totalRetries). + Uint64("perSecond", perSec(uint64(r.writtenRels), totalTime)). Stringer("duration", totalTime). Msg("finished restore") return nil @@ -213,9 +212,9 @@ func (r *restorer) restoreFromDecoder(ctx context.Context) error { func (r *restorer) commitStream(ctx context.Context, bulkImportClient v1.ExperimentalService_BulkImportRelationshipsClient, batchesToBeCommitted [][]*v1.Relationship, ) error { - var numLoaded, expectedLoaded, retries uint64 + var numLoaded, expectedLoaded, retries uint for _, b := range batchesToBeCommitted { - expectedLoaded += uint64(len(b)) + expectedLoaded += uint(len(b)) } resp, err := bulkImportClient.CloseAndRecv() // transaction commit happens here @@ -228,12 +227,7 @@ func (r *restorer) commitStream(ctx context.Context, bulkImportClient v1.Experim canceled, cancelErr := isCanceledError(ctx.Err(), err) unknown := !retryable && !conflict && !canceled && err != nil - intExpectedLoaded, castErr := safecast.ToInt64(expectedLoaded) - if castErr != nil { - return castErr - } - - intNumBatches := int64(len(batchesToBeCommitted)) + numBatches := uint(len(batchesToBeCommitted)) switch { case canceled: @@ -245,29 +239,24 @@ func (r *restorer) commitStream(ctx context.Context, bulkImportClient v1.Experim case retryable && r.disableRetryErrors: return err case conflict && r.conflictStrategy == Skip: - r.skippedRels += intExpectedLoaded - r.skippedBatches += intNumBatches - r.duplicateBatches += intNumBatches - r.duplicateRels += intExpectedLoaded + r.skippedRels += expectedLoaded + r.skippedBatches += numBatches + r.duplicateBatches += numBatches + r.duplicateRels += expectedLoaded r.bar.Describe("skipping conflicting batch") case conflict && r.conflictStrategy == Touch: r.bar.Describe("touching conflicting batch") - r.duplicateRels += intExpectedLoaded - r.duplicateBatches += intNumBatches + r.duplicateRels += expectedLoaded + r.duplicateBatches += numBatches 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 += intNumBatches - r.writtenRels += intNumLoaded + r.writtenBatches += numBatches + r.writtenRels += numLoaded case conflict && r.conflictStrategy == Fail: r.bar.Describe("conflict detected, aborting restore") return fmt.Errorf("duplicate relationships found") @@ -279,45 +268,44 @@ 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 += intNumBatches - r.writtenRels += intNumLoaded + r.writtenBatches += numBatches + r.writtenRels += numLoaded default: r.bar.Describe("restoring relationships from backup") - r.writtenBatches += intNumBatches + r.writtenBatches += numBatches } // it was a successful transaction commit without duplicates if resp != nil { - intNumLoaded, err := safecast.ToInt64(resp.NumLoaded) + numLoaded, err := safecast.ToUint(resp.NumLoaded) if err != nil { - return err + return spiceerrors.MustBugf("could not cast numLoaded to uint") } - - r.writtenRels += intNumLoaded - if expectedLoaded != resp.NumLoaded { - log.Warn().Uint64("loaded", resp.NumLoaded).Uint64("expected", expectedLoaded).Msg("unexpected number of relationships loaded") + r.writtenRels += numLoaded + if uint64(expectedLoaded) != resp.NumLoaded { + log.Warn().Uint64("loaded", resp.NumLoaded).Uint("expected", expectedLoaded).Msg("unexpected number of relationships loaded") } } - if err := r.bar.Set64(r.writtenRels + r.skippedRels); err != nil { + writtenAndSkipped, err := safecast.ToInt64(r.writtenRels + r.skippedRels) + if err != nil { + return fmt.Errorf("too many written and skipped rels for an int64") + } + + if err := r.bar.Set64(writtenAndSkipped); err != nil { return fmt.Errorf("error incrementing progress bar: %w", err) } if !isatty.IsTerminal(os.Stderr.Fd()) { log.Trace(). - Int64("batches_written", r.writtenBatches). - Int64("relationships_written", r.writtenRels). - Int64("duplicate_batches", r.duplicateBatches). - Int64("duplicate_relationships", r.duplicateRels). - Int64("skipped_batches", r.skippedBatches). - Int64("skipped_relationships", r.skippedRels). - Uint64("retries", retries). + Uint("batches_written", r.writtenBatches). + Uint("relationships_written", r.writtenRels). + Uint("duplicate_batches", r.duplicateBatches). + Uint("duplicate_relationships", r.duplicateRels). + Uint("skipped_batches", r.skippedBatches). + Uint("skipped_relationships", r.skippedRels). + Uint("retries", retries). Msg("restore progress") } @@ -326,14 +314,14 @@ func (r *restorer) commitStream(ctx context.Context, bulkImportClient v1.Experim // writeBatchesWithRetry writes a set of batches using touch semantics and without transactional guarantees - // each batch will be committed independently. If a batch fails, it will be retried up to 10 times with a backoff. -func (r *restorer) writeBatchesWithRetry(ctx context.Context, batches [][]*v1.Relationship) (uint64, uint64, error) { +func (r *restorer) writeBatchesWithRetry(ctx context.Context, batches [][]*v1.Relationship) (uint, uint, error) { backoffInterval := backoff.NewExponentialBackOff() backoffInterval.InitialInterval = defaultBackoff backoffInterval.MaxInterval = 2 * time.Second backoffInterval.MaxElapsedTime = 0 backoffInterval.Reset() - var currentRetries, totalRetries, loadedRels uint64 + var currentRetries, totalRetries, loadedRels uint for _, batch := range batches { updates := lo.Map[*v1.Relationship, *v1.RelationshipUpdate](batch, func(item *v1.Relationship, _ int) *v1.RelationshipUpdate { return &v1.RelationshipUpdate{ @@ -364,7 +352,7 @@ func (r *restorer) writeBatchesWithRetry(ctx context.Context, batches [][]*v1.Re currentRetries = 0 backoffInterval.Reset() - loadedRels += uint64(len(batch)) + loadedRels += uint(len(batch)) break } } diff --git a/internal/cmd/restorer_test.go b/internal/cmd/restorer_test.go index 617a5fd..54f3a04 100644 --- a/internal/cmd/restorer_test.go +++ b/internal/cmd/restorer_test.go @@ -8,7 +8,6 @@ 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" @@ -31,7 +30,7 @@ func TestRestorer(t *testing.T) { for _, tt := range []struct { name string prefixFilter string - batchSize int + batchSize uint batchesPerTransaction uint conflictStrategy ConflictStrategy disableRetryErrors bool @@ -79,13 +78,11 @@ func TestRestorer(t *testing.T) { expectedFilteredRels = append(expectedFilteredRels, rel) } - expectedBatches := len(expectedFilteredRels) / tt.batchSize + expectedBatches := uint(len(expectedFilteredRels)) / tt.batchSize // there is always one extra commit, regardless there is or not a remainder batch - batchesPerTransaction, err := safecast.ToInt(tt.batchesPerTransaction) - require.NoError(err) - expectedCommits := expectedBatches/batchesPerTransaction + 1 + expectedCommits := expectedBatches/tt.batchesPerTransaction + 1 remainderBatch := false - if len(expectedFilteredRels)%tt.batchSize != 0 { + if uint(len(expectedFilteredRels))%tt.batchSize != 0 { expectedBatches++ remainderBatch = true } @@ -103,8 +100,8 @@ func TestRestorer(t *testing.T) { sendErrors: tt.sendErrors, } - expectedConflicts := 0 - expectedRetries := 0 + expectedConflicts := uint(0) + expectedRetries := uint(0) var expectsError error for _, err := range tt.commitErrors { if isRetryableError(err) { @@ -130,12 +127,12 @@ func TestRestorer(t *testing.T) { } // if skip is enabled, there will be N less relationships written, where N is the number of conflicts - expectedWrittenRels := len(expectedFilteredRels) + expectedWrittenRels := uint(len(expectedFilteredRels)) if tt.conflictStrategy == Skip { expectedWrittenRels -= expectedConflicts * tt.batchSize } - expectedWrittenBatches := len(expectedFilteredRels) / tt.batchSize + expectedWrittenBatches := uint(len(expectedFilteredRels)) / tt.batchSize if tt.conflictStrategy == Skip { expectedWrittenBatches -= expectedConflicts } @@ -146,12 +143,12 @@ func TestRestorer(t *testing.T) { expectedTouchedBatches := expectedRetries expectedTouchedRels := expectedRetries * tt.batchSize if tt.conflictStrategy == Touch { - expectedTouchedBatches += expectedConflicts * batchesPerTransaction - expectedTouchedRels += expectedConflicts * batchesPerTransaction * tt.batchSize + expectedTouchedBatches += expectedConflicts * tt.batchesPerTransaction + expectedTouchedRels += expectedConflicts * tt.batchesPerTransaction * tt.batchSize } - expectedSkippedBatches := 0 - expectedSkippedRels := 0 + expectedSkippedBatches := uint(0) + expectedSkippedRels := uint(0) if tt.conflictStrategy == Skip { expectedSkippedBatches += expectedConflicts expectedSkippedRels += expectedConflicts * tt.batchSize @@ -169,23 +166,19 @@ func TestRestorer(t *testing.T) { // assert on mock stats 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(uint(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 - uintExpectedConflicts, err := safecast.ToUint(expectedConflicts) - require.NoError(err) - uintDuplicateBatches, err := safecast.ToUint(r.duplicateBatches) - require.NoError(err) - 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(uintExpectedConflicts*tt.batchesPerTransaction, uintDuplicateBatches, "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") + require.Equal(expectedWrittenRels, r.writtenRels, "unexpected number of written relationships") + require.Equal(expectedWrittenBatches, r.writtenBatches, "unexpected number of written relationships") + require.Equal(expectedSkippedBatches, r.skippedBatches, "unexpected number of conflicting batches skipped") + require.Equal(expectedSkippedRels, r.skippedRels, "unexpected number of conflicting relationships skipped") + require.Equal(expectedConflicts*tt.batchesPerTransaction, r.duplicateBatches, "unexpected number of duplicate batches detected") + require.Equal(expectedConflicts*tt.batchesPerTransaction*tt.batchSize, r.duplicateRels, "unexpected number of duplicate relationships detected") + require.Equal(expectedRetries+expectedConflicts-expectedSkippedBatches, r.totalRetries, "unexpected number of retries") + require.Equal(uint(len(tt.relationships)-len(expectedFilteredRels)), r.filteredOutRels, "unexpected number of filtered out relationships") }) } } @@ -197,14 +190,14 @@ type mockClient struct { schema string remainderBatch bool expectedRels []string - expectedBatches int - requestedBatchSize int + expectedBatches uint + requestedBatchSize uint requestedBatchesPerTransaction uint - receivedBatches int - receivedCommits int - receivedRels int - touchedBatches int - touchedRels int + receivedBatches uint + receivedCommits uint + receivedRels uint + touchedBatches uint + touchedRels uint lastReceivedBatch []*v1.Relationship sendErrors []error commitErrors []error @@ -217,20 +210,20 @@ func (m *mockClient) BulkImportRelationships(_ context.Context, _ ...grpc.CallOp func (m *mockClient) Send(req *v1.BulkImportRelationshipsRequest) error { m.receivedBatches++ - m.receivedRels += len(req.Relationships) + m.receivedRels += uint(len(req.Relationships)) m.lastReceivedBatch = req.Relationships - if m.receivedBatches <= len(m.sendErrors) { + if m.receivedBatches <= uint(len(m.sendErrors)) { return m.sendErrors[m.receivedBatches-1] } if m.receivedBatches == m.expectedBatches && m.remainderBatch { - require.Equal(m.t, len(m.expectedRels)%m.requestedBatchSize, len(req.Relationships)) + require.Equal(m.t, uint(len(m.expectedRels))%m.requestedBatchSize, uint(len(req.Relationships))) } else { - require.Equal(m.t, m.requestedBatchSize, len(req.Relationships)) + require.Equal(m.t, m.requestedBatchSize, uint(len(req.Relationships))) } for i, rel := range req.Relationships { - require.True(m.t, proto.Equal(rel, tuple.ParseRel(m.expectedRels[((m.receivedBatches-1)*m.requestedBatchSize)+i]))) + require.True(m.t, proto.Equal(rel, tuple.ParseRel(m.expectedRels[((m.receivedBatches-1)*m.requestedBatchSize)+uint(i)]))) } return nil @@ -238,8 +231,8 @@ func (m *mockClient) Send(req *v1.BulkImportRelationshipsRequest) error { func (m *mockClient) WriteRelationships(_ context.Context, in *v1.WriteRelationshipsRequest, _ ...grpc.CallOption) (*v1.WriteRelationshipsResponse, error) { m.touchedBatches++ - m.touchedRels += len(in.Updates) - if m.touchedBatches <= len(m.touchErrors) { + m.touchedRels += uint(len(in.Updates)) + if m.touchedBatches <= uint(len(m.touchErrors)) { return nil, m.touchErrors[m.touchedBatches-1] } @@ -251,7 +244,7 @@ func (m *mockClient) CloseAndRecv() (*v1.BulkImportRelationshipsResponse, error) lastBatch := m.lastReceivedBatch defer func() { m.lastReceivedBatch = nil }() - if m.receivedCommits <= len(m.commitErrors) { + if m.receivedCommits <= uint(len(m.commitErrors)) { return nil, m.commitErrors[m.receivedCommits-1] } From d7b79950c980eb2b0f5d76a174dbce6fe9161dd4 Mon Sep 17 00:00:00 2001 From: Tanner Stirrat Date: Fri, 13 Sep 2024 11:11:03 -0600 Subject: [PATCH 10/21] Fix lint errors --- internal/cmd/restorer_test.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/internal/cmd/restorer_test.go b/internal/cmd/restorer_test.go index 54f3a04..8d23be4 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" @@ -178,7 +179,7 @@ func TestRestorer(t *testing.T) { require.Equal(expectedConflicts*tt.batchesPerTransaction, r.duplicateBatches, "unexpected number of duplicate batches detected") require.Equal(expectedConflicts*tt.batchesPerTransaction*tt.batchSize, r.duplicateRels, "unexpected number of duplicate relationships detected") require.Equal(expectedRetries+expectedConflicts-expectedSkippedBatches, r.totalRetries, "unexpected number of retries") - require.Equal(uint(len(tt.relationships)-len(expectedFilteredRels)), r.filteredOutRels, "unexpected number of filtered out relationships") + require.Equal(uint(len(tt.relationships))-uint(len(expectedFilteredRels)), r.filteredOutRels, "unexpected number of filtered out relationships") }) } } @@ -223,7 +224,9 @@ func (m *mockClient) Send(req *v1.BulkImportRelationshipsRequest) error { } for i, rel := range req.Relationships { - require.True(m.t, proto.Equal(rel, tuple.ParseRel(m.expectedRels[((m.receivedBatches-1)*m.requestedBatchSize)+uint(i)]))) + // This is a gosec115 false positive which should be fixed in a future version. + uinti, _ := safecast.ToUint(i) + require.True(m.t, proto.Equal(rel, tuple.ParseRel(m.expectedRels[((m.receivedBatches-1)*m.requestedBatchSize)+uinti]))) } return nil From 00a64d334e602cc83525e4759458342c058fbba8 Mon Sep 17 00:00:00 2001 From: Tanner Stirrat Date: Fri, 13 Sep 2024 11:21:56 -0600 Subject: [PATCH 11/21] Use isatty instead of term --- internal/cmd/version.go | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/internal/cmd/version.go b/internal/cmd/version.go index f46540e..8c67b16 100644 --- a/internal/cmd/version.go +++ b/internal/cmd/version.go @@ -6,12 +6,11 @@ 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/mattn/go-isatty" "github.com/rs/zerolog/log" "github.com/spf13/cobra" - "golang.org/x/term" "google.golang.org/grpc" "google.golang.org/grpc/metadata" @@ -21,11 +20,7 @@ import ( ) func versionCmdFunc(cmd *cobra.Command, _ []string) error { - intFd, err := safecast.ToInt(uint(os.Stdout.Fd())) - if err != nil { - return err - } - if !term.IsTerminal(intFd) { + if !isatty.IsTerminal(os.Stdout.Fd()) { color.Disable() } From 346961a19756caac02474b67c182699e4f12c08f Mon Sep 17 00:00:00 2001 From: Tanner Stirrat Date: Fri, 13 Sep 2024 12:25:16 -0600 Subject: [PATCH 12/21] Use term version that has better cross-platform support --- go.mod | 3 ++- go.sum | 4 ++++ internal/storage/secrets.go | 9 ++------- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/go.mod b/go.mod index f7fc387..d301859 100644 --- a/go.mod +++ b/go.mod @@ -85,6 +85,7 @@ require ( github.com/certifi/gocertifi v0.0.0-20210507211836-431795d63e8d // indirect github.com/cespare/xxhash/v2 v2.3.0 // indirect github.com/charmbracelet/x/ansi v0.1.4 // indirect + github.com/charmbracelet/x/term v0.2.0 // indirect github.com/cilium/ebpf v0.9.1 // indirect github.com/cloudspannerecosystem/spanner-change-streams-tail v0.3.1 // indirect github.com/cncf/xds/go v0.0.0-20240423153145-555b57ec207b // indirect @@ -231,7 +232,7 @@ require ( golang.org/x/crypto v0.25.0 // indirect golang.org/x/net v0.27.0 // indirect golang.org/x/oauth2 v0.21.0 // indirect - golang.org/x/sys v0.22.0 // indirect + golang.org/x/sys v0.24.0 // indirect golang.org/x/text v0.16.0 // indirect golang.org/x/time v0.5.0 // indirect golang.org/x/xerrors v0.0.0-20231012003039-104605ab7028 // indirect diff --git a/go.sum b/go.sum index 34f0b01..4b3bc69 100644 --- a/go.sum +++ b/go.sum @@ -740,6 +740,8 @@ github.com/charmbracelet/lipgloss v0.12.1 h1:/gmzszl+pedQpjCOH+wFkZr/N90Snz40J/N github.com/charmbracelet/lipgloss v0.12.1/go.mod h1:V2CiwIuhx9S1S1ZlADfOj9HmxeMAORuz5izHb0zGbB8= github.com/charmbracelet/x/ansi v0.1.4 h1:IEU3D6+dWwPSgZ6HBH+v6oUuZ/nVawMiWj5831KfiLM= github.com/charmbracelet/x/ansi v0.1.4/go.mod h1:dk73KoMTT5AX5BsX0KrqhsTqAnhZZoCBjs7dGWp4Ktw= +github.com/charmbracelet/x/term v0.2.0 h1:cNB9Ot9q8I711MyZ7myUR5HFWL/lc3OpU8jZ4hwm0x0= +github.com/charmbracelet/x/term v0.2.0/go.mod h1:GVxgxAbjUrmpvIINHIQnJJKpMlHiZ4cktEQCN6GWyF0= github.com/chzyer/logex v1.1.10/go.mod h1:+Ywpsq7O8HXn0nuIou7OrIPyXbp3wmkHB+jjWRnGsAI= github.com/chzyer/readline v0.0.0-20180603132655-2972be24d48e/go.mod h1:nSuG5e5PlCu98SY8svDHJxuZscDgtXS6KTTbou5AhLI= github.com/chzyer/test v0.0.0-20180213035817-a1ea475d72b1/go.mod h1:Q3SI9o4m/ZMnBNeIyt5eFwwo7qiLfzFZmjNmxjkiQlU= @@ -1711,6 +1713,8 @@ golang.org/x/sys v0.7.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.12.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.22.0 h1:RI27ohtqKCnwULzJLqkv897zojh5/DwS/ENaMzUOaWI= golang.org/x/sys v0.22.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= +golang.org/x/sys v0.24.0 h1:Twjiwq9dn6R1fQcyiK+wQyHWfaz/BJB+YIpzU/Cv3Xg= +golang.org/x/sys v0.24.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= golang.org/x/term v0.2.0/go.mod h1:TVmDHMZPmdnySmBfhjOoOdhjzdE1h4u1VwSiw2l1Nuc= diff --git a/internal/storage/secrets.go b/internal/storage/secrets.go index 6ce4db3..d29d9a9 100644 --- a/internal/storage/secrets.go +++ b/internal/storage/secrets.go @@ -8,9 +8,8 @@ import ( "strings" "github.com/99designs/keyring" - "github.com/ccoveille/go-safecast" + "github.com/charmbracelet/x/term" "github.com/jzelinskie/stringz" - "golang.org/x/term" "github.com/authzed/zed/internal/console" ) @@ -147,11 +146,7 @@ func fileExists(path string) (bool, error) { func promptPassword(prompt string) (string, error) { console.Printf(prompt) - intFd, err := safecast.ToInt(uint(os.Stdin.Fd())) - if err != nil { - return "", err - } - b, err := term.ReadPassword(intFd) + b, err := term.ReadPassword(os.Stdin.Fd()) if err != nil { return "", err } From 6a77581397634d3b465426f1035a3cf2df6a91d2 Mon Sep 17 00:00:00 2001 From: Tanner Stirrat Date: Fri, 13 Sep 2024 12:41:17 -0600 Subject: [PATCH 13/21] Fix check issues --- .goreleaser.yml | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/.goreleaser.yml b/.goreleaser.yml index fd98d0a..965d7d4 100644 --- a/.goreleaser.yml +++ b/.goreleaser.yml @@ -118,10 +118,6 @@ nfpms: builds: ["linux-amd64-musl", "linux-arm64-musl"] formats: ["apk"] -furies: - - account: "authzed" - secret_name: "GEMFURY_PUSH_TOKEN" - brews: - description: "command-line client for managing SpiceDB" homepage: "https://github.com/authzed/zed" @@ -158,7 +154,7 @@ checksum: name_template: "checksums.txt" snapshot: - name_template: "{{ incpatch .Version }}-next" + version_template: "{{ incpatch .Version }}-next" changelog: use: "github-native" From 6d13d7b08825739aa3ace4446ee9c2fe701a873b Mon Sep 17 00:00:00 2001 From: Tanner Stirrat Date: Fri, 13 Sep 2024 12:41:31 -0600 Subject: [PATCH 14/21] Try pinning to earlier version of goreleaser --- .github/workflows/release.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index 840e999..1ad1410 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -21,7 +21,7 @@ jobs: - uses: "goreleaser/goreleaser-action@v6" with: distribution: "goreleaser-pro" - version: "latest" + version: &goreleaser_version "2.2.0" args: "release --clean" env: GITHUB_TOKEN: "${{ secrets.GITHUB_TOKEN }}" @@ -46,7 +46,7 @@ jobs: - uses: "goreleaser/goreleaser-action@v6" with: distribution: "goreleaser-pro" - version: "latest" + version: *goreleaser_version args: "release --config=.goreleaser.docker.yml --clean" env: GITHUB_TOKEN: "${{ secrets.GITHUB_TOKEN }}" From 5c4fff5fa6debdb143910af4d90d8fcce052bbe6 Mon Sep 17 00:00:00 2001 From: Tanner Stirrat Date: Fri, 13 Sep 2024 12:43:51 -0600 Subject: [PATCH 15/21] Run go mod tidy --- go.mod | 2 +- go.sum | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/go.mod b/go.mod index d301859..44d1997 100644 --- a/go.mod +++ b/go.mod @@ -14,6 +14,7 @@ require ( github.com/ccoveille/go-safecast v1.1.0 github.com/cenkalti/backoff/v4 v4.3.0 github.com/charmbracelet/lipgloss v0.12.1 + github.com/charmbracelet/x/term v0.2.0 github.com/google/uuid v1.6.0 github.com/gookit/color v1.5.4 github.com/hamba/avro/v2 v2.22.1 @@ -85,7 +86,6 @@ require ( github.com/certifi/gocertifi v0.0.0-20210507211836-431795d63e8d // indirect github.com/cespare/xxhash/v2 v2.3.0 // indirect github.com/charmbracelet/x/ansi v0.1.4 // indirect - github.com/charmbracelet/x/term v0.2.0 // indirect github.com/cilium/ebpf v0.9.1 // indirect github.com/cloudspannerecosystem/spanner-change-streams-tail v0.3.1 // indirect github.com/cncf/xds/go v0.0.0-20240423153145-555b57ec207b // indirect diff --git a/go.sum b/go.sum index 4b3bc69..2caa71e 100644 --- a/go.sum +++ b/go.sum @@ -1711,7 +1711,6 @@ golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.7.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.12.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.22.0 h1:RI27ohtqKCnwULzJLqkv897zojh5/DwS/ENaMzUOaWI= golang.org/x/sys v0.22.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/sys v0.24.0 h1:Twjiwq9dn6R1fQcyiK+wQyHWfaz/BJB+YIpzU/Cv3Xg= golang.org/x/sys v0.24.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= From fdcd3f659d5f2c5ef3dfc1f01bde888d2e0d7067 Mon Sep 17 00:00:00 2001 From: Tanner Stirrat Date: Fri, 13 Sep 2024 12:44:26 -0600 Subject: [PATCH 16/21] Add clarifying comment --- .github/workflows/release.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index 1ad1410..77b4f1a 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -21,6 +21,7 @@ jobs: - uses: "goreleaser/goreleaser-action@v6" with: distribution: "goreleaser-pro" + # Pinning to v2.2.0 to work around a regression in 2.3.0 version: &goreleaser_version "2.2.0" args: "release --clean" env: From 817c383a3c6ba8165acd91bfc78071f8685b27b0 Mon Sep 17 00:00:00 2001 From: Tanner Stirrat Date: Fri, 13 Sep 2024 12:53:17 -0600 Subject: [PATCH 17/21] Use goreleaser v2.2.0 in the lint step --- .github/workflows/lint.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/lint.yaml b/.github/workflows/lint.yaml index 74eabee..7c5f38f 100644 --- a/.github/workflows/lint.yaml +++ b/.github/workflows/lint.yaml @@ -75,7 +75,7 @@ jobs: id: "goreleaser" with: distribution: "goreleaser-pro" - version: "latest" + version: "2.2.0" args: "release -f .goreleaser.docker.yml --clean --split --snapshot" env: GORELEASER_KEY: "${{ secrets.GORELEASER_KEY }}" From 204767262c361436b0f0fc55a964f3d316a27cd5 Mon Sep 17 00:00:00 2001 From: Tanner Stirrat Date: Fri, 13 Sep 2024 12:55:23 -0600 Subject: [PATCH 18/21] Add comment --- .github/workflows/lint.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/lint.yaml b/.github/workflows/lint.yaml index 7c5f38f..5378802 100644 --- a/.github/workflows/lint.yaml +++ b/.github/workflows/lint.yaml @@ -75,6 +75,7 @@ jobs: id: "goreleaser" with: distribution: "goreleaser-pro" + # Pinning to version 2.2.0 to get around a regression in 2.3.0. version: "2.2.0" args: "release -f .goreleaser.docker.yml --clean --split --snapshot" env: From 71f98c6d0a9399a1edc54a8e9a6cc9a634cb1593 Mon Sep 17 00:00:00 2001 From: Tanner Stirrat Date: Fri, 13 Sep 2024 13:14:31 -0600 Subject: [PATCH 19/21] Reinstate furies --- .goreleaser.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.goreleaser.yml b/.goreleaser.yml index 965d7d4..be308a0 100644 --- a/.goreleaser.yml +++ b/.goreleaser.yml @@ -118,6 +118,10 @@ nfpms: builds: ["linux-amd64-musl", "linux-arm64-musl"] formats: ["apk"] +furies: + - account: "authzed" + secret_name: "GEMFURY_PUSH_TOKEN" + brews: - description: "command-line client for managing SpiceDB" homepage: "https://github.com/authzed/zed" From 8b24f7656191799893a9b6169e79047cd2797fdb Mon Sep 17 00:00:00 2001 From: Tanner Stirrat Date: Mon, 16 Sep 2024 08:27:08 -0600 Subject: [PATCH 20/21] Try new version that shouldn't have regression --- .github/workflows/lint.yaml | 3 +-- .github/workflows/release.yaml | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/.github/workflows/lint.yaml b/.github/workflows/lint.yaml index 5378802..1a38af0 100644 --- a/.github/workflows/lint.yaml +++ b/.github/workflows/lint.yaml @@ -75,8 +75,7 @@ jobs: id: "goreleaser" with: distribution: "goreleaser-pro" - # Pinning to version 2.2.0 to get around a regression in 2.3.0. - version: "2.2.0" + version: "2.3.1" args: "release -f .goreleaser.docker.yml --clean --split --snapshot" env: GORELEASER_KEY: "${{ secrets.GORELEASER_KEY }}" diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index 77b4f1a..6b7cca4 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -21,8 +21,7 @@ jobs: - uses: "goreleaser/goreleaser-action@v6" with: distribution: "goreleaser-pro" - # Pinning to v2.2.0 to work around a regression in 2.3.0 - version: &goreleaser_version "2.2.0" + version: &goreleaser_version "2.3.1" args: "release --clean" env: GITHUB_TOKEN: "${{ secrets.GITHUB_TOKEN }}" From ce0ed022d9afcf49fa30c0a9b1740827684ab6f8 Mon Sep 17 00:00:00 2001 From: Tanner Stirrat Date: Mon, 16 Sep 2024 08:31:33 -0600 Subject: [PATCH 21/21] Revert "Try new version that shouldn't have regression" This reverts commit 8b24f7656191799893a9b6169e79047cd2797fdb. --- .github/workflows/lint.yaml | 3 ++- .github/workflows/release.yaml | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/.github/workflows/lint.yaml b/.github/workflows/lint.yaml index 1a38af0..5378802 100644 --- a/.github/workflows/lint.yaml +++ b/.github/workflows/lint.yaml @@ -75,7 +75,8 @@ jobs: id: "goreleaser" with: distribution: "goreleaser-pro" - version: "2.3.1" + # Pinning to version 2.2.0 to get around a regression in 2.3.0. + version: "2.2.0" args: "release -f .goreleaser.docker.yml --clean --split --snapshot" env: GORELEASER_KEY: "${{ secrets.GORELEASER_KEY }}" diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index 6b7cca4..77b4f1a 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -21,7 +21,8 @@ jobs: - uses: "goreleaser/goreleaser-action@v6" with: distribution: "goreleaser-pro" - version: &goreleaser_version "2.3.1" + # Pinning to v2.2.0 to work around a regression in 2.3.0 + version: &goreleaser_version "2.2.0" args: "release --clean" env: GITHUB_TOKEN: "${{ secrets.GITHUB_TOKEN }}"