From 26cb8e0901df829e640f9b6003a091976bc4c108 Mon Sep 17 00:00:00 2001 From: vincent Date: Mon, 24 Aug 2026 09:56:51 -0400 Subject: [PATCH 1/3] tigris: fix update sending options as metadata MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit runUpdate initialized the mutation's metadata argument from addOn.Options instead of addOn.Metadata, aliasing the same map used for the options argument. An ordinary `fly storage update` would overwrite real stored metadata with the mutated options — including fly-statics-app-id, fly-statics-bucket-name, and fly-statics-tokenized-auth, breaking statics discovery/move/cleanup. Read addOn.Metadata directly instead. As a side effect, non-map metadata is now preserved verbatim instead of being silently dropped to nil. --- internal/command/extensions/tigris/update.go | 3 +- .../command/extensions/tigris/update_test.go | 124 ++++++++++++++++++ 2 files changed, 125 insertions(+), 2 deletions(-) create mode 100644 internal/command/extensions/tigris/update_test.go diff --git a/internal/command/extensions/tigris/update.go b/internal/command/extensions/tigris/update.go index d237cef580..e6081bcea1 100644 --- a/internal/command/extensions/tigris/update.go +++ b/internal/command/extensions/tigris/update.go @@ -70,8 +70,7 @@ func runUpdate(ctx context.Context) (err error) { options = make(map[string]any) } - metadata, _ := addOn.Options.(map[string]any) - + metadata := addOn.Metadata if metadata == nil { metadata = make(map[string]any) } diff --git a/internal/command/extensions/tigris/update_test.go b/internal/command/extensions/tigris/update_test.go new file mode 100644 index 0000000000..dff91e54bd --- /dev/null +++ b/internal/command/extensions/tigris/update_test.go @@ -0,0 +1,124 @@ +package tigris + +import ( + "context" + "encoding/json" + "testing" + + graphql "github.com/Khan/genqlient/graphql" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/superfly/flyctl/gql" + "github.com/superfly/flyctl/internal/flag" + "github.com/superfly/flyctl/internal/flyutil" + "github.com/superfly/flyctl/internal/mock" + "github.com/superfly/flyctl/iostreams" +) + +type recordingGenqClient struct { + t *testing.T + metadata any + updateOptions map[string]any + updateMetadata any +} + +func (c *recordingGenqClient) MakeRequest(_ context.Context, req *graphql.Request, resp *graphql.Response) error { + switch req.OpName { + case "GetAddOn": + decodeResponse(c.t, resp, map[string]any{ + "addOn": map[string]any{ + "id": "addon-id", + "name": "test-bucket", + "status": "ready", + "options": map[string]any{"public": false}, + "metadata": c.metadata, + "addOnPlan": map[string]any{ + "id": "plan-id", + }, + "addOnProvider": map[string]any{ + "name": "tigris", + }, + "organization": map[string]any{ + "slug": "test-org", + }, + }, + }) + case "UpdateAddOn": + require.Equal(c.t, gql.UpdateAddOn_Operation, req.Query) + + variables := req.Variables.(interface { + GetOptions() any + GetMetadata() any + }) + c.updateOptions = variables.GetOptions().(map[string]any) + c.updateMetadata = variables.GetMetadata() + + decodeResponse(c.t, resp, map[string]any{ + "updateAddOn": map[string]any{ + "addOn": map[string]any{"id": "addon-id"}, + }, + }) + default: + c.t.Fatalf("unexpected GraphQL operation %q", req.OpName) + } + + return nil +} + +func TestRunUpdatePreservesMetadataWithoutAliasingOptions(t *testing.T) { + genqClient := runUpdateWithMetadata(t, map[string]any{ + "fly-statics-app-id": "42", + "fly-statics-bucket-name": "test-statics", + "fly-statics-tokenized-auth": "test-tokenized-auth", + "provider": map[string]any{ + "regions": []any{"iad", "fra"}, + }, + }) + + assert.Equal(t, map[string]any{"public": true}, genqClient.updateOptions) + assert.Equal(t, map[string]any{ + "fly-statics-app-id": "42", + "fly-statics-bucket-name": "test-statics", + "fly-statics-tokenized-auth": "test-tokenized-auth", + "provider": map[string]any{ + "regions": []any{"iad", "fra"}, + }, + }, genqClient.updateMetadata) +} + +func TestRunUpdatePreservesNonObjectMetadata(t *testing.T) { + genqClient := runUpdateWithMetadata(t, "opaque-provider-metadata") + assert.Equal(t, "opaque-provider-metadata", genqClient.updateMetadata) +} + +func TestRunUpdateNormalizesNilMetadata(t *testing.T) { + genqClient := runUpdateWithMetadata(t, nil) + require.IsType(t, map[string]any{}, genqClient.updateMetadata) + assert.Empty(t, genqClient.updateMetadata) +} + +func runUpdateWithMetadata(t *testing.T, metadata any) *recordingGenqClient { + t.Helper() + + genqClient := &recordingGenqClient{t: t, metadata: metadata} + client := &mock.Client{GenqClientFunc: func() graphql.Client { return genqClient }} + + cmd := update() + require.NoError(t, cmd.Flags().Parse([]string{"--public", "test-bucket"})) + + io, _, _, _ := iostreams.Test() + ctx := iostreams.NewContext(context.Background(), io) + ctx = flag.NewContext(ctx, cmd.Flags()) + ctx = flyutil.NewContextWithClient(ctx, client) + + require.NoError(t, runUpdate(ctx)) + return genqClient +} + +func decodeResponse(t *testing.T, resp *graphql.Response, payload any) { + t.Helper() + + data, err := json.Marshal(payload) + require.NoError(t, err) + require.NoError(t, json.Unmarshal(data, resp.Data)) +} From 83ca3a594de4a3b3386b1ba364abb156bd457fc0 Mon Sep 17 00:00:00 2001 From: vincent Date: Mon, 24 Aug 2026 10:11:04 -0400 Subject: [PATCH 2/3] fix lint: nlreturn --- internal/command/extensions/tigris/update_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/command/extensions/tigris/update_test.go b/internal/command/extensions/tigris/update_test.go index dff91e54bd..151ed10932 100644 --- a/internal/command/extensions/tigris/update_test.go +++ b/internal/command/extensions/tigris/update_test.go @@ -112,6 +112,7 @@ func runUpdateWithMetadata(t *testing.T, metadata any) *recordingGenqClient { ctx = flyutil.NewContextWithClient(ctx, client) require.NoError(t, runUpdate(ctx)) + return genqClient } From 09b67442a5ba0c6c7e62f9c6ab5b2903d11474ac Mon Sep 17 00:00:00 2001 From: vincent Date: Mon, 24 Aug 2026 10:27:23 -0400 Subject: [PATCH 3/3] tigris: preserve shadow write-through when flag omitted on update runUpdate unconditionally set write_through from flag.GetBool, whose zero value is false. Any shadow bucket update that didn't re-pass --shadow-write-through (e.g. rotating just the access key) silently disabled write-through replication, even if it was previously enabled. This is inconsistent with how the same function handles public/accelerate/custom-domain, all of which use flag.IsSpecified to leave the existing value alone when the flag is omitted. Fall back to the existing stored write_through value when the flag isn't specified, matching that pattern. New shadow buckets still default to false, since there's no prior value to preserve. --- internal/command/extensions/tigris/update.go | 10 ++- .../command/extensions/tigris/update_test.go | 85 ++++++++++++++++++- 2 files changed, 93 insertions(+), 2 deletions(-) diff --git a/internal/command/extensions/tigris/update.go b/internal/command/extensions/tigris/update.go index e6081bcea1..91ebfb2cdb 100644 --- a/internal/command/extensions/tigris/update.go +++ b/internal/command/extensions/tigris/update.go @@ -80,7 +80,6 @@ func runUpdate(ctx context.Context) (err error) { region := flag.GetString(ctx, "shadow-region") shadowName := flag.GetString(ctx, "shadow-name") endpoint := flag.GetString(ctx, "shadow-endpoint") - writeThrough := flag.GetBool(ctx, "shadow-write-through") clearShadow := flag.GetBool(ctx, "clear-shadow") // Check for shadow bucket values @@ -96,6 +95,15 @@ func runUpdate(ctx context.Context) (err error) { if clearShadow { options["shadow_bucket"] = map[string]any{} } else if shadowBucketSpecified { + writeThrough := flag.GetBool(ctx, "shadow-write-through") + if !flag.IsSpecified(ctx, "shadow-write-through") { + if existing, ok := options["shadow_bucket"].(map[string]any); ok { + if existingWriteThrough, ok := existing["write_through"].(bool); ok { + writeThrough = existingWriteThrough + } + } + } + options["shadow_bucket"] = map[string]any{ "access_key": accessKey, "secret_key": secretKey, diff --git a/internal/command/extensions/tigris/update_test.go b/internal/command/extensions/tigris/update_test.go index 151ed10932..2054af3077 100644 --- a/internal/command/extensions/tigris/update_test.go +++ b/internal/command/extensions/tigris/update_test.go @@ -18,6 +18,7 @@ import ( type recordingGenqClient struct { t *testing.T metadata any + options map[string]any updateOptions map[string]any updateMetadata any } @@ -25,12 +26,17 @@ type recordingGenqClient struct { func (c *recordingGenqClient) MakeRequest(_ context.Context, req *graphql.Request, resp *graphql.Response) error { switch req.OpName { case "GetAddOn": + options := c.options + if options == nil { + options = map[string]any{"public": false} + } + decodeResponse(c.t, resp, map[string]any{ "addOn": map[string]any{ "id": "addon-id", "name": "test-bucket", "status": "ready", - "options": map[string]any{"public": false}, + "options": options, "metadata": c.metadata, "addOnPlan": map[string]any{ "id": "plan-id", @@ -97,6 +103,83 @@ func TestRunUpdateNormalizesNilMetadata(t *testing.T) { assert.Empty(t, genqClient.updateMetadata) } +func TestRunUpdatePreservesWriteThroughWhenFlagOmitted(t *testing.T) { + genqClient := &recordingGenqClient{ + t: t, + options: map[string]any{ + "shadow_bucket": map[string]any{ + "access_key": "old", + "secret_key": "old", + "region": "us-east-1", + "name": "source-bucket", + "endpoint": "https://s3.us-east-1.amazonaws.com", + "write_through": true, + }, + }, + } + client := &mock.Client{GenqClientFunc: func() graphql.Client { return genqClient }} + + cmd := update() + require.NoError(t, cmd.Flags().Parse([]string{ + "--shadow-access-key", "123", + "--shadow-secret-key", "abc", + "--shadow-endpoint", "https://s3.us-east-1.amazonaws.com", + "--shadow-region", "us-east-1", + "--shadow-name", "source-bucket", + "test-bucket", + })) + + io, _, _, _ := iostreams.Test() + ctx := iostreams.NewContext(context.Background(), io) + ctx = flag.NewContext(ctx, cmd.Flags()) + ctx = flyutil.NewContextWithClient(ctx, client) + + require.NoError(t, runUpdate(ctx)) + + shadowBucket, ok := genqClient.updateOptions["shadow_bucket"].(map[string]any) + require.True(t, ok) + assert.Equal(t, true, shadowBucket["write_through"]) +} + +func TestRunUpdateSetsWriteThroughWhenFlagSpecified(t *testing.T) { + genqClient := &recordingGenqClient{ + t: t, + options: map[string]any{ + "shadow_bucket": map[string]any{ + "access_key": "old", + "secret_key": "old", + "region": "us-east-1", + "name": "source-bucket", + "endpoint": "https://s3.us-east-1.amazonaws.com", + "write_through": true, + }, + }, + } + client := &mock.Client{GenqClientFunc: func() graphql.Client { return genqClient }} + + cmd := update() + require.NoError(t, cmd.Flags().Parse([]string{ + "--shadow-access-key", "123", + "--shadow-secret-key", "abc", + "--shadow-endpoint", "https://s3.us-east-1.amazonaws.com", + "--shadow-region", "us-east-1", + "--shadow-name", "source-bucket", + "--shadow-write-through=false", + "test-bucket", + })) + + io, _, _, _ := iostreams.Test() + ctx := iostreams.NewContext(context.Background(), io) + ctx = flag.NewContext(ctx, cmd.Flags()) + ctx = flyutil.NewContextWithClient(ctx, client) + + require.NoError(t, runUpdate(ctx)) + + shadowBucket, ok := genqClient.updateOptions["shadow_bucket"].(map[string]any) + require.True(t, ok) + assert.Equal(t, false, shadowBucket["write_through"]) +} + func runUpdateWithMetadata(t *testing.T, metadata any) *recordingGenqClient { t.Helper()