From 864213564fe9ac44c5939db235b70f75a46d9bd6 Mon Sep 17 00:00:00 2001 From: Chris Schinnerl Date: Thu, 19 Dec 2024 11:35:37 +0100 Subject: [PATCH 1/2] bus: add helper for fetching settings from store --- ...e_if_settings_are_not_found_in_database.md | 5 ++ bus/routes.go | 47 +++++++----------- bus/settings.go | 49 +++++++++++++++++++ 3 files changed, 71 insertions(+), 30 deletions(-) create mode 100644 .changeset/fix_default_settings_not_being_used_everywhere_if_settings_are_not_found_in_database.md create mode 100644 bus/settings.go diff --git a/.changeset/fix_default_settings_not_being_used_everywhere_if_settings_are_not_found_in_database.md b/.changeset/fix_default_settings_not_being_used_everywhere_if_settings_are_not_found_in_database.md new file mode 100644 index 000000000..8fdc8aaf3 --- /dev/null +++ b/.changeset/fix_default_settings_not_being_used_everywhere_if_settings_are_not_found_in_database.md @@ -0,0 +1,5 @@ +--- +default: patch +--- + +# Fix default settings not being used everywhere if settings are not found in database diff --git a/bus/routes.go b/bus/routes.go index 4a6f5c77a..a637fca3a 100644 --- a/bus/routes.go +++ b/bus/routes.go @@ -1406,10 +1406,8 @@ func (b *Bus) packedSlabsHandlerDonePOST(jc jape.Context) { } func (b *Bus) settingsGougingHandlerGET(jc jape.Context) { - gs, err := b.store.GougingSettings(jc.Request.Context()) - if errors.Is(err, sql.ErrSettingNotFound) { - gs = api.DefaultGougingSettings - } else if err != nil { + gs, err := b.gougingSettings(jc.Request.Context()) + if err != nil { jc.Error(err, http.StatusInternalServerError) return } @@ -1432,10 +1430,8 @@ func (b *Bus) settingsGougingHandlerPUT(jc jape.Context) { } func (b *Bus) settingsPinnedHandlerGET(jc jape.Context) { - ps, err := b.store.PinnedSettings(jc.Request.Context()) - if errors.Is(err, sql.ErrSettingNotFound) { - ps = api.DefaultPinnedSettings - } else if err != nil { + ps, err := b.pinnedSettings(jc.Request.Context()) + if err != nil { jc.Error(err, http.StatusInternalServerError) return } @@ -1461,10 +1457,8 @@ func (b *Bus) settingsPinnedHandlerPUT(jc jape.Context) { } func (b *Bus) settingsUploadHandlerGET(jc jape.Context) { - us, err := b.store.UploadSettings(jc.Request.Context()) - if errors.Is(err, sql.ErrSettingNotFound) { - us = api.DefaultUploadSettings(b.cm.TipState().Network.Name) - } else if err != nil { + us, err := b.uploadSettings(jc.Request.Context()) + if err != nil { jc.Error(err, http.StatusInternalServerError) return } @@ -1485,10 +1479,8 @@ func (b *Bus) settingsUploadHandlerPUT(jc jape.Context) { } func (b *Bus) settingsS3HandlerGET(jc jape.Context) { - s3s, err := b.store.S3Settings(jc.Request.Context()) - if errors.Is(err, sql.ErrSettingNotFound) { - s3s = api.DefaultS3Settings - } else if err != nil { + s3s, err := b.s3Settings(jc.Request.Context()) + if err != nil { jc.Error(err, http.StatusInternalServerError) return } @@ -1656,7 +1648,7 @@ func (b *Bus) slabsPartialHandlerPOST(jc jape.Context) { if jc.Check("failed to add partial slab", err) != nil { return } - us, err := b.store.UploadSettings(jc.Request.Context()) + us, err := b.uploadSettings(jc.Request.Context()) if err != nil { jc.Error(fmt.Errorf("could not get upload packing settings: %w", err), http.StatusInternalServerError) return @@ -1751,16 +1743,15 @@ func (b *Bus) paramsHandlerUploadGET(jc jape.Context) { return } - var uploadPacking bool - us, err := b.store.UploadSettings(jc.Request.Context()) - if jc.Check("could not get upload settings", err) == nil { - uploadPacking = us.Packing.Enabled + us, err := b.uploadSettings(jc.Request.Context()) + if jc.Check("could not get upload settings", err) != nil { + return } api.WriteResponse(jc, api.UploadParams{ CurrentHeight: b.cm.TipState().Index.Height, GougingParams: gp, - UploadPacking: uploadPacking, + UploadPacking: us.Packing.Enabled, }) } @@ -1792,17 +1783,13 @@ func (b *Bus) paramsHandlerGougingGET(jc jape.Context) { } func (b *Bus) gougingParams(ctx context.Context) (api.GougingParams, error) { - gs, err := b.store.GougingSettings(ctx) - if errors.Is(err, sql.ErrSettingNotFound) { - gs = api.DefaultGougingSettings - } else if err != nil { + gs, err := b.gougingSettings(ctx) + if err != nil { return api.GougingParams{}, err } - us, err := b.store.UploadSettings(ctx) - if errors.Is(err, sql.ErrSettingNotFound) { - us = api.DefaultUploadSettings(b.cm.TipState().Network.Name) - } else if err != nil { + us, err := b.uploadSettings(ctx) + if err != nil { return api.GougingParams{}, err } diff --git a/bus/settings.go b/bus/settings.go new file mode 100644 index 000000000..7a2cfa7a9 --- /dev/null +++ b/bus/settings.go @@ -0,0 +1,49 @@ +package bus + +import ( + "context" + "errors" + + "go.sia.tech/renterd/api" + "go.sia.tech/renterd/stores/sql" +) + +func (b Bus) gougingSettings(ctx context.Context) (api.GougingSettings, error) { + gs, err := b.store.GougingSettings(ctx) + if errors.Is(err, sql.ErrSettingNotFound) { + gs = api.DefaultGougingSettings + } else if err != nil { + return api.GougingSettings{}, err + } + return gs, nil +} + +func (b Bus) pinnedSettings(ctx context.Context) (api.PinnedSettings, error) { + ps, err := b.store.PinnedSettings(ctx) + if errors.Is(err, sql.ErrSettingNotFound) { + ps = api.DefaultPinnedSettings + } else if err != nil { + return api.PinnedSettings{}, err + } + return ps, nil +} + +func (b Bus) s3Settings(ctx context.Context) (api.S3Settings, error) { + s3s, err := b.store.S3Settings(ctx) + if errors.Is(err, sql.ErrSettingNotFound) { + s3s = api.DefaultS3Settings + } else if err != nil { + return api.S3Settings{}, err + } + return s3s, nil +} + +func (b Bus) uploadSettings(ctx context.Context) (api.UploadSettings, error) { + us, err := b.store.UploadSettings(ctx) + if errors.Is(err, sql.ErrSettingNotFound) { + us = api.DefaultUploadSettings(b.cm.TipState().Network.Name) + } else if err != nil { + return api.UploadSettings{}, err + } + return us, nil +} From 7dc696f82aba69e22688e47a73cdc9d007bd7e3b Mon Sep 17 00:00:00 2001 From: Chris Schinnerl Date: Thu, 19 Dec 2024 12:03:03 +0100 Subject: [PATCH 2/2] add TestDefaultSettingsUploadDownload --- internal/test/e2e/cluster.go | 11 +++--- internal/test/e2e/cluster_test.go | 60 +++++++++++++++++++++++++++++++ internal/test/e2e/host.go | 2 +- 3 files changed, 68 insertions(+), 5 deletions(-) diff --git a/internal/test/e2e/cluster.go b/internal/test/e2e/cluster.go index 17dde4647..9564e046f 100644 --- a/internal/test/e2e/cluster.go +++ b/internal/test/e2e/cluster.go @@ -193,8 +193,9 @@ type testClusterOptions struct { hosts int logger *zap.Logger uploadPacking bool - skipSettingAutopilot bool skipRunningAutopilot bool + skipSettingAutopilot bool + skipUpdatingSettings bool walletKey *types.PrivateKey autopilotCfg *config.Autopilot @@ -474,9 +475,11 @@ func newTestCluster(t *testing.T, opts testClusterOptions) *TestCluster { } // Update the bus settings. - tt.OK(busClient.UpdateGougingSettings(ctx, test.GougingSettings)) - tt.OK(busClient.UpdateUploadSettings(ctx, us)) - tt.OK(busClient.UpdateS3Settings(ctx, s3)) + if !opts.skipUpdatingSettings { + tt.OK(busClient.UpdateGougingSettings(ctx, test.GougingSettings)) + tt.OK(busClient.UpdateUploadSettings(ctx, us)) + tt.OK(busClient.UpdateS3Settings(ctx, s3)) + } // Fund the bus. if funding { diff --git a/internal/test/e2e/cluster_test.go b/internal/test/e2e/cluster_test.go index b7cdac701..c1ca0245a 100644 --- a/internal/test/e2e/cluster_test.go +++ b/internal/test/e2e/cluster_test.go @@ -36,6 +36,7 @@ import ( "go.sia.tech/renterd/internal/test" "go.sia.tech/renterd/internal/utils" "go.sia.tech/renterd/object" + "go.sia.tech/renterd/stores/sql" "go.sia.tech/renterd/stores/sql/sqlite" "lukechampine.com/frand" ) @@ -3087,3 +3088,62 @@ func TestV1ToV2Transition(t *testing.T) { return nil }) } + +func TestDefaultSettingsUploadDownload(t *testing.T) { + // create a test cluster + apCfg := test.AutopilotConfig + apCfg.Contracts.Amount = uint64(api.DefaultRedundancySettingsTestnet.TotalShards) + cluster := newTestCluster(t, testClusterOptions{ + hosts: api.DefaultRedundancySettingsTestnet.TotalShards, + logger: newTestLogger(false), + skipUpdatingSettings: true, + autopilotConfig: &apCfg, + }) + defer cluster.Shutdown() + + b := cluster.Bus + w := cluster.Worker + tt := cluster.tt + + // sanity check settings + _, err := cluster.bs.GougingSettings(context.Background()) + tt.AssertIs(err, sql.ErrSettingNotFound) + _, err = cluster.bs.PinnedSettings(context.Background()) + tt.AssertIs(err, sql.ErrSettingNotFound) + _, err = cluster.bs.S3Settings(context.Background()) + tt.AssertIs(err, sql.ErrSettingNotFound) + _, err = cluster.bs.UploadSettings(context.Background()) + tt.AssertIs(err, sql.ErrSettingNotFound) + + // prepare a file + data := make([]byte, 128) + tt.OKAll(frand.Read(data)) + + // upload and download data the native way + path := "/regularFile" + tt.OKAll(w.UploadObject(context.Background(), bytes.NewReader(data), testBucket, path, api.UploadObjectOptions{})) + tt.OK(w.DownloadObject(context.Background(), bytes.NewBuffer(nil), testBucket, path, api.DownloadObjectOptions{})) + + // upload and download a multipart upload + multipartPath := "/multipartFile" + mpu, err := b.CreateMultipartUpload(context.Background(), testBucket, multipartPath, api.CreateMultipartOptions{}) + tt.OK(err) + offset := 0 + part, err := w.UploadMultipartUploadPart(context.Background(), bytes.NewReader(data), testBucket, multipartPath, mpu.UploadID, 1, api.UploadMultipartUploadPartOptions{EncryptionOffset: &offset}) + tt.OK(err) + tt.OKAll(b.CompleteMultipartUpload(context.Background(), testBucket, multipartPath, mpu.UploadID, []api.MultipartCompletedPart{ + { + PartNumber: 1, + ETag: part.ETag, + }, + }, api.CompleteMultipartOptions{})) + tt.OK(err) + tt.OK(w.DownloadObject(context.Background(), bytes.NewBuffer(nil), testBucket, multipartPath, api.DownloadObjectOptions{})) + + // upload and download via s3 to test the default s3 settings + s3Path := "/s3File" + _, err = cluster.S3.PutObject(testBucket, s3Path, bytes.NewReader(data), putObjectOptions{}) + if err == nil || !strings.Contains(err.Error(), "AccessDenied") { + t.Fatal("expected access denied error") + } +} diff --git a/internal/test/e2e/host.go b/internal/test/e2e/host.go index fc3d1194c..fe673fa46 100644 --- a/internal/test/e2e/host.go +++ b/internal/test/e2e/host.go @@ -76,7 +76,7 @@ var defaultHostSettings = settings.Settings{ IngressPrice: types.Siacoins(100).Div64(1e12), WindowSize: 5, - PriceTableValidity: 10 * time.Second, + PriceTableValidity: 5 * time.Minute, AccountExpiry: 30 * 24 * time.Hour, // 1 month MaxAccountBalance: types.Siacoins(10),