fix(cli): do not allow index downgrade by using set-parameter (#2629)

* disallow downgrades

* fixup merge

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.")

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)