From ca7df80e64c53cfdc84752f363b9c4fce5c9936c Mon Sep 17 00:00:00 2001 From: Aaron Alpar <55999015+aaron-kasten@users.noreply.github.com> Date: Tue, 6 Dec 2022 18:33:55 -0800 Subject: [PATCH] fix(cli): do not allow index downgrade by using `set-parameter` (#2629) * disallow downgrades * fixup merge --- cli/command_repository_set_parameters.go | 101 ++++++++++++------ cli/command_repository_set_parameters_test.go | 43 ++++++++ 2 files changed, 109 insertions(+), 35 deletions(-) diff --git a/cli/command_repository_set_parameters.go b/cli/command_repository_set_parameters.go index 95a0189fe..30cc0df01 100644 --- a/cli/command_repository_set_parameters.go +++ b/cli/command_repository_set_parameters.go @@ -122,9 +122,58 @@ func (c *commandRepositorySetParameters) setRetentionModeParameter(ctx context.C log(ctx).Infof(" - setting %v to %s.\n", desc, v) } -func (c *commandRepositorySetParameters) run(ctx context.Context, rep repo.DirectRepositoryWriter) error { - var anyChange bool +func updateRepositoryParameters( + ctx context.Context, + upgradeToEpochManager bool, + mp format.MutableParameters, + rep repo.DirectRepositoryWriter, + blobcfg format.BlobStorageConfiguration, + requiredFeatures []feature.Required, +) error { + if upgradeToEpochManager { + log(ctx).Infof("migrating current indexes to epoch format") + if err := rep.ContentManager().PrepareUpgradeToIndexBlobManagerV1(ctx, mp.EpochParameters); err != nil { + return errors.Wrap(err, "error upgrading indexes") + } + } + + if err := rep.FormatManager().SetParameters(ctx, mp, blobcfg, requiredFeatures); err != nil { + return errors.Wrap(err, "error setting parameters") + } + + if upgradeToEpochManager { + if err := content.WriteLegacyIndexPoisonBlob(ctx, rep.BlobStorage()); err != nil { + log(ctx).Errorf("unable to write legacy index poison blob: %v", err) + } + } + + return nil +} + +func updateEpochParameters(mp *format.MutableParameters, anyChange, upgradeToEpochManager *bool) { + *anyChange = true + + if !mp.EpochParameters.Enabled { + mp.EpochParameters = epoch.DefaultParameters() + mp.IndexVersion = 2 + *upgradeToEpochManager = true + } + + if mp.Version < format.FormatVersion2 { + mp.Version = format.FormatVersion2 + } +} + +func (c *commandRepositorySetParameters) disableBlobRetention(ctx context.Context, blobcfg *format.BlobStorageConfiguration, anyChange *bool) { + log(ctx).Infof("disabling blob retention") + + blobcfg.RetentionMode = "" + blobcfg.RetentionPeriod = 0 + *anyChange = true +} + +func (c *commandRepositorySetParameters) run(ctx context.Context, rep repo.DirectRepositoryWriter) error { mp, err := rep.FormatManager().GetMutableParameters() if err != nil { return errors.Wrap(err, "mutable parameters") @@ -140,32 +189,28 @@ func (c *commandRepositorySetParameters) run(ctx context.Context, rep repo.Direc return errors.Wrap(err, "unable to get required features") } + anyChange := false upgradeToEpochManager := false if c.upgradeRepositoryFormat { - anyChange = true - - if !mp.EpochParameters.Enabled { - mp.EpochParameters = epoch.DefaultParameters() - upgradeToEpochManager = true - mp.IndexVersion = 2 - } - - if mp.Version < format.FormatVersion2 { - mp.Version = format.FormatVersion2 - } + updateEpochParameters(&mp, &anyChange, &upgradeToEpochManager) } c.setSizeMBParameter(ctx, c.maxPackSizeMB, "maximum pack size", &mp.MaxPackSize, &anyChange) - c.setIntParameter(ctx, c.indexFormatVersion, "index format version", &mp.IndexVersion, &anyChange) + + // prevent downgrade of index format + if c.indexFormatVersion != 0 && c.indexFormatVersion != mp.IndexVersion { + if c.indexFormatVersion > mp.IndexVersion { + c.setIntParameter(ctx, c.indexFormatVersion, "index format version", &mp.IndexVersion, &anyChange) + } else { + return errors.Errorf("index format version can only be upgraded") + } + } if c.retentionMode == "none" { if blobcfg.IsRetentionEnabled() { - log(ctx).Infof("disabling blob retention") - - blobcfg.RetentionMode = "" - blobcfg.RetentionPeriod = 0 - anyChange = true + // disable blob retention if already enabled + c.disableBlobRetention(ctx, &blobcfg, &anyChange) } } else { c.setRetentionModeParameter(ctx, blob.RetentionMode(c.retentionMode), "storage backend blob retention mode", &blobcfg.RetentionMode, &anyChange) @@ -186,22 +231,8 @@ func (c *commandRepositorySetParameters) run(ctx context.Context, rep repo.Direc return errors.Errorf("no changes") } - if upgradeToEpochManager { - log(ctx).Infof("migrating current indexes to epoch format") - - if err := rep.ContentManager().PrepareUpgradeToIndexBlobManagerV1(ctx, mp.EpochParameters); err != nil { - return errors.Wrap(err, "error upgrading indexes") - } - } - - if err := rep.FormatManager().SetParameters(ctx, mp, blobcfg, requiredFeatures); err != nil { - return errors.Wrap(err, "error setting parameters") - } - - if upgradeToEpochManager { - if err := content.WriteLegacyIndexPoisonBlob(ctx, rep.BlobStorage()); err != nil { - log(ctx).Errorf("unable to write legacy index poison blob: %v", err) - } + if err := updateRepositoryParameters(ctx, upgradeToEpochManager, mp, rep, blobcfg, requiredFeatures); err != nil { + return errors.Wrap(err, "error updating repository parameters") } log(ctx).Infof("NOTE: Repository parameters updated, you must disconnect and re-connect all other Kopia clients.") diff --git a/cli/command_repository_set_parameters_test.go b/cli/command_repository_set_parameters_test.go index 136785ac9..87caecaf6 100644 --- a/cli/command_repository_set_parameters_test.go +++ b/cli/command_repository_set_parameters_test.go @@ -182,6 +182,49 @@ func (s *formatSpecificTestSuite) TestRepositorySetParametersUpgrade(t *testing. env.RunAndExpectSuccess(t, "index", "epoch", "list") } +// TestRepositorySetParametersDowngrade test that a repository cannot be downgraded by using `set-parameters`. +func (s *formatSpecificTestSuite) TestRepositorySetParametersDowngrade(t *testing.T) { + env := s.setupInMemoryRepo(t) + + // checkStatusForVersion is a function with stanzas to check that the repository has the expected version. + // its saved into a variable to prevent repetition and enforce that nothing has changed between invocations + // if `set-parameters` + checkStatusForVersion := func() { + out := env.RunAndExpectSuccess(t, "repository", "status") + + // default values + require.Contains(t, out, "Max pack length: 21 MB") + + switch s.formatVersion { + case format.FormatVersion1: + require.Contains(t, out, "Format version: 1") + require.Contains(t, out, "Epoch Manager: disabled") + env.RunAndExpectFailure(t, "index", "epoch", "list") + case format.FormatVersion2: + require.Contains(t, out, "Format version: 2") + require.Contains(t, out, "Epoch Manager: enabled") + env.RunAndExpectSuccess(t, "index", "epoch", "list") + default: + require.Contains(t, out, "Format version: 3") + require.Contains(t, out, "Epoch Manager: enabled") + env.RunAndExpectSuccess(t, "index", "epoch", "list") + } + } + + checkStatusForVersion() + + env.RunAndExpectFailure(t, "repository", "set-parameters", "--index-version=1") + + checkStatusForVersion() + + // run basic check to ensure that an upgrade can still be performed as expected + env.RunAndExpectSuccess(t, "repository", "set-parameters", "--upgrade") + + out := env.RunAndExpectSuccess(t, "repository", "status") + require.Contains(t, out, "Epoch Manager: enabled") + require.Contains(t, out, "Index Format: v2") +} + func (s *formatSpecificTestSuite) TestRepositorySetParametersRequiredFeatures(t *testing.T) { env := s.setupInMemoryRepo(t)