storage: remove obsolete encryption-at-rest code

pkg/ccl/storageccl/engineccl/BUILD.bazel

@@ -15,7 +15,6 @@ go_library(
         "//pkg/ccl/storageccl/engineccl/enginepbccl",
         "//pkg/storage",
         "//pkg/storage/enginepb",
-        "//pkg/storage/fs",
         "//pkg/util/log",
         "//pkg/util/protoutil",
         "//pkg/util/syncutil",
@@ -60,6 +59,7 @@ go_test(
         "@com_github_cockroachdb_errors//oserror",
         "@com_github_cockroachdb_pebble//:pebble",
         "@com_github_cockroachdb_pebble//vfs",
+        "@com_github_cockroachdb_pebble//vfs/atomicfs",
         "@com_github_gogo_protobuf//proto",
         "@com_github_kr_pretty//:pretty",
         "@com_github_stretchr_testify//require",

pkg/ccl/storageccl/engineccl/encrypted_fs.go

@@ -364,7 +364,6 @@ func newEncryptedEnv(
 			storeKM: storeKeyManager,
 			dataKM:  dataKeyManager,
 		},
-		UpgradeVersion: dataKeyManager.UseMarker,
 	}, nil
 }
 

pkg/ccl/storageccl/engineccl/pebble_key_manager.go

@@ -17,7 +17,6 @@ import (
 	"time"
 
 	"github.com/cockroachdb/cockroach/pkg/ccl/storageccl/engineccl/enginepbccl"
-	"github.com/cockroachdb/cockroach/pkg/storage/fs"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/cockroach/pkg/util/protoutil"
 	"github.com/cockroachdb/cockroach/pkg/util/syncutil"
@@ -178,12 +177,6 @@ type DataKeyManager struct {
 		// Transitions to true when SetActiveStoreKeyInfo() is called for the
 		// first time.
 		rotationEnabled bool
-		// useMarker indicates whether or not the data key registry
-		// should write new files and use an atomic marker to mark the
-		// active file. This is set to true once the 21.2 version is
-		// finalized.
-		// TODO(jackson): Remove after v21.2.
-		useMarker bool
 		// marker is an atomic file marker used to denote which of the
 		// data keys registry files is the current one. When we rotate
 		// files, the marker is atomically moved to the new file. It's
@@ -214,23 +207,10 @@ func (m *DataKeyManager) Load(ctx context.Context) error {
 	if err != nil {
 		return err
 	}
-	useMarker := filename != ""
-
-	// If the marker doesn't exist, filename is the empty string.  In
-	// this case, we fall back to looking for the file at the fixed
-	// `DATA_KEYS_REGISTRY` path.
-	if filename == "" {
-		filename = keyRegistryFilename
-		_, err = m.fs.Stat(m.fs.PathJoin(m.dbDir, filename))
-		if err != nil && !oserror.IsNotExist(err) {
-			return err
-		}
-	}
 
 	m.mu.Lock()
 	defer m.mu.Unlock()
 	m.mu.marker = marker
-	m.mu.useMarker = useMarker
 	if oserror.IsNotExist(err) {
 		// First run.
 		m.mu.keyRegistry = makeRegistryProto()
@@ -239,22 +219,26 @@ func (m *DataKeyManager) Load(ctx context.Context) error {
 
 	// Load the existing state from the file named by `filename`.
 	m.mu.filename = filename
-	f, err := m.fs.Open(m.fs.PathJoin(m.dbDir, filename))
-	if err != nil {
-		return err
-	}
-	defer f.Close()
-	b, err := ioutil.ReadAll(f)
-	if err != nil {
-		return err
-	}
 	m.mu.keyRegistry = makeRegistryProto()
-	if err = protoutil.Unmarshal(b, m.mu.keyRegistry); err != nil {
-		return err
-	}
-	if err = validateRegistry(m.mu.keyRegistry); err != nil {
-		return err
+	if filename != "" {
+		f, err := m.fs.Open(m.fs.PathJoin(m.dbDir, filename))
+		if err != nil {
+			return err
+		}
+		defer f.Close()
+		b, err := ioutil.ReadAll(f)
+		if err != nil {
+			return err
+		}
+		if err = protoutil.Unmarshal(b, m.mu.keyRegistry); err != nil {
+			return err
+		}
+		if err = validateRegistry(m.mu.keyRegistry); err != nil {
+			return err
+		}
 	}
+	// Else there is no DataKeysRegistry file yet.
+
 	if m.mu.keyRegistry.ActiveDataKeyId != "" {
 		key, found := m.mu.keyRegistry.DataKeys[m.mu.keyRegistry.ActiveDataKeyId]
 		if !found {
@@ -269,31 +253,6 @@ func (m *DataKeyManager) Load(ctx context.Context) error {
 	return nil
 }
 
-// UseMarker informs the data key manager that it should begin using the
-// marker file to denote which file is active.
-//
-// TODO(jackson): Remove this in 22.1. In 22.1 we can unconditionally
-// use the marker file.
-func (m *DataKeyManager) UseMarker() error {
-	if m.readOnly {
-		return nil
-	}
-
-	m.mu.Lock()
-	defer m.mu.Unlock()
-	m.mu.useMarker = true
-
-	// If there is no filename, the data keys registry has never been
-	// written. There's no file to mark yet. The first rotation will set
-	// the marker.
-	if m.mu.filename == "" {
-		return nil
-	}
-	// NB: This may move the marker to mark the file with the previous
-	// static filename "COCKROACHDB_DATA_KEYS".
-	return m.mu.marker.Move(m.mu.filename)
-}
-
 // ActiveKey implements PebbleKeyManager.ActiveKey.
 //
 // TODO(sbhola): do rotation via a background activity instead of in this function so that we don't
@@ -473,26 +432,6 @@ func (m *DataKeyManager) rotateDataKeyAndWrite(
 		return err
 	}
 
-	if !m.mu.useMarker {
-		// If the v21.2 version hasn't been finalized yet, write the
-		// registry to the static filename `COCKROACHDB_DATA_KEYS`. If
-		// there's a crash mid-Rename, it's possible that we'll be left
-		// with a corrupt data key registry.
-		// TODO(jackson): Remove this for 22.1.
-		path := m.fs.PathJoin(m.dbDir, keyRegistryFilename)
-		if err = fs.SafeWriteToFile(m.fs, m.dbDir, path, bytes); err != nil {
-			return
-		}
-		m.mu.filename = keyRegistryFilename
-		m.mu.keyRegistry = keyRegistry
-		m.mu.activeKey = newKey
-		return err
-	}
-
-	// The v21.2 version has been finalized. Write a new file
-	// containing the updated state, and move the atomic marker to
-	// point to the new file.
-
 	// Write the current registry state to a new file and sync it.
 	// The new file's filename incorporates the marker's iteration
 	// number to ensure we're not overwriting the existing registry.

pkg/ccl/storageccl/engineccl/pebble_key_manager_test.go

@@ -27,6 +27,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/timeutil"
 	"github.com/cockroachdb/datadriven"
 	"github.com/cockroachdb/pebble/vfs"
+	"github.com/cockroachdb/pebble/vfs/atomicfs"
 	"github.com/gogo/protobuf/proto"
 	"github.com/stretchr/testify/require"
 )
@@ -291,6 +292,10 @@ func TestDataKeyManager(t *testing.T) {
 						return err.Error()
 					}
 					writeToFile(t, memFS, memFS.PathJoin(data[0], keyRegistryFilename), b)
+					marker, _, err := atomicfs.LocateMarker(memFS, data[0], keysRegistryMarkerName)
+					require.NoError(t, err)
+					require.NoError(t, marker.Move(keyRegistryFilename))
+					require.NoError(t, marker.Close())
 				}
 				return ""
 			case "load":
@@ -447,9 +452,6 @@ func TestDataKeyManagerIO(t *testing.T) {
 				d.ScanArgs(t, "id", &id)
 				fmt.Fprintf(&buf, "%s", setActiveStoreKey(dkm, id, enginepbccl.EncryptionType_AES128_CTR))
 				return buf.String()
-			case "use-marker":
-				appendError(dkm.UseMarker())
-				return buf.String()
 			default:
 				return fmt.Sprintf("unknown command: %s\n", d.Cmd)
 			}

pkg/ccl/storageccl/engineccl/testdata/data_key_manager_io

@@ -10,19 +10,10 @@ OK
 load dir=data-dir
 ----
 open-dir("data-dir")
-stat("data-dir/COCKROACHDB_DATA_KEYS")
-OK
-
-# Since there's no data keys registry file yet, use-marker should be a
-# noop.
-
-use-marker
-----
 OK
 
 # Setting the active store key should trigger initializing the registry
-# on disk. Since use-marker has already been called, the registry should
-# be initialized with a marker file.
+# on disk.
 
 set-active-store-key id=foo
 ----
@@ -98,32 +89,17 @@ OK
 load dir=data-dir
 ----
 open-dir("data-dir")
-stat("data-dir/COCKROACHDB_DATA_KEYS")
 OK
 
-# Since use-marker has not yet been called, the registry shouldn't use
-# it. Instead, it should replace the static COCKROACHDB_DATA_KEYS file.
-
 set-active-store-key id=foo
 ----
-create("data-dir/COCKROACHDB_DATA_KEYS.crdbtmp")
-write("data-dir/COCKROACHDB_DATA_KEYS.crdbtmp", <...280 bytes...>)
-sync("data-dir/COCKROACHDB_DATA_KEYS.crdbtmp")
-close("data-dir/COCKROACHDB_DATA_KEYS.crdbtmp")
-rename("data-dir/COCKROACHDB_DATA_KEYS.crdbtmp", "data-dir/COCKROACHDB_DATA_KEYS")
-open-dir("data-dir")
-sync("data-dir")
-close("data-dir")
-
-# Calling use-marker should lay down a marker pointing to the existing
-# file.
-
-use-marker
-----
-create("data-dir/marker.datakeys.000001.COCKROACHDB_DATA_KEYS")
-close("data-dir/marker.datakeys.000001.COCKROACHDB_DATA_KEYS")
+create("data-dir/COCKROACHDB_DATA_KEYS_000001_monolith")
+write("data-dir/COCKROACHDB_DATA_KEYS_000001_monolith", <...280 bytes...>)
+sync("data-dir/COCKROACHDB_DATA_KEYS_000001_monolith")
+create("data-dir/marker.datakeys.000001.COCKROACHDB_DATA_KEYS_000001_monolith")
+close("data-dir/marker.datakeys.000001.COCKROACHDB_DATA_KEYS_000001_monolith")
 sync("data-dir")
-OK
+close("data-dir/COCKROACHDB_DATA_KEYS_000001_monolith")
 
 set-active-store-key id=bar
 ----
@@ -132,9 +108,9 @@ write("data-dir/COCKROACHDB_DATA_KEYS_000002_monolith", <...489 bytes...>)
 sync("data-dir/COCKROACHDB_DATA_KEYS_000002_monolith")
 create("data-dir/marker.datakeys.000002.COCKROACHDB_DATA_KEYS_000002_monolith")
 close("data-dir/marker.datakeys.000002.COCKROACHDB_DATA_KEYS_000002_monolith")
-remove("data-dir/marker.datakeys.000001.COCKROACHDB_DATA_KEYS")
+remove("data-dir/marker.datakeys.000001.COCKROACHDB_DATA_KEYS_000001_monolith")
 sync("data-dir")
-remove("data-dir/COCKROACHDB_DATA_KEYS")
+remove("data-dir/COCKROACHDB_DATA_KEYS_000001_monolith")
 close("data-dir/COCKROACHDB_DATA_KEYS_000002_monolith")
 
 close

pkg/storage/engine.go

@@ -765,10 +765,6 @@ type Engine interface {
 	// version that it must maintain compatibility with.
 	SetMinVersion(version roachpb.Version) error
 
-	// UsingRecordsEncryptionRegistry returns whether the engine is using the
-	// Records version incremental encryption-at-rest registry.
-	UsingRecordsEncryptionRegistry() (bool, error)
-
 	// MinVersionIsAtLeastTargetVersion returns whether the engine's recorded
 	// storage min version is at least the target version.
 	MinVersionIsAtLeastTargetVersion(target roachpb.Version) (bool, error)

pkg/storage/enginepb/file_registry.go

@@ -10,34 +10,6 @@
 
 package enginepb
 
-// ProcessBatch processes a batch of updates to the file registry.
-func (r *FileRegistry) ProcessBatch(batch *RegistryUpdateBatch) {
-	for _, update := range batch.Updates {
-		r.ProcessUpdate(update)
-	}
-}
-
-// ProcessUpdate processes a single update to the file registry.
-func (r *FileRegistry) ProcessUpdate(update *RegistryUpdate) {
-	if update.Entry == nil {
-		delete(r.Files, update.Filename)
-	} else {
-		if r.Files == nil {
-			r.Files = make(map[string]*FileEntry)
-		}
-		r.Files[update.Filename] = update.Entry
-	}
-}
-
-// SetVersion updates the version of the file registry. This function will
-// panic if the provided version is lower than the current version.
-func (r *FileRegistry) SetVersion(version RegistryVersion) {
-	if version < r.Version {
-		panic("illegal downgrade of file registry version")
-	}
-	r.Version = version
-}
-
 // Empty returns whether a batch is empty.
 func (b *RegistryUpdateBatch) Empty() bool {
 	return len(b.Updates) == 0

pkg/storage/min_version_test.go

@@ -164,9 +164,8 @@ func fauxNewEncryptedEnvFunc(
 	fs vfs.FS, fr *PebbleFileRegistry, dbDir string, readOnly bool, optionBytes []byte,
 ) (*EncryptionEnv, error) {
 	return &EncryptionEnv{
-		Closer:         nopCloser{},
-		FS:             fauxEncryptedFS{FS: fs},
-		UpgradeVersion: func() error { return nil },
+		Closer: nopCloser{},
+		FS:     fauxEncryptedFS{FS: fs},
 	}, nil
 }
 

pkg/storage/pebble.go

@@ -620,13 +620,6 @@ type EncryptionEnv struct {
 	FS vfs.FS
 	// StatsHandler exposes encryption-at-rest state for observability.
 	StatsHandler EncryptionStatsHandler
-	// UpgradeVersion is a temporary field that allows Pebble to inform
-	// low-level encryption-at-rest machinery that the CockroachDB 21.2
-	// version has been finalized, and it's okay to begin writing in a
-	// backwards in-compatible format.
-	//
-	// TODO(jackson): Remove this in 22.1.
-	UpgradeVersion func() error
 }
 
 var _ Engine = &Pebble{}
@@ -1580,32 +1573,9 @@ func (p *Pebble) SetMinVersion(version roachpb.Version) error {
 			return errors.Wrap(err, "ratcheting format major version")
 		}
 	}
-
-	if p.fileRegistry != nil {
-		if !version.Less(clusterversion.ByKey(clusterversion.TODOPreV21_2)) {
-			if err := p.fileRegistry.StopUsingOldRegistry(); err != nil {
-				return err
-			}
-		}
-	}
-	if p.encryption != nil {
-		if !version.Less(clusterversion.ByKey(clusterversion.TODOPreV21_2)) {
-			if err := p.encryption.UpgradeVersion(); err != nil {
-				return err
-			}
-		}
-	}
 	return nil
 }
 
-// UsingRecordsEncryptionRegistry implements the Engine interface.
-func (p *Pebble) UsingRecordsEncryptionRegistry() (bool, error) {
-	if p.fileRegistry != nil {
-		return p.fileRegistry.UpgradedToRecordsVersion(), nil
-	}
-	return true, nil
-}
-
 // MinVersionIsAtLeastTargetVersion implements the Engine interface.
 func (p *Pebble) MinVersionIsAtLeastTargetVersion(target roachpb.Version) (bool, error) {
 	return MinVersionIsAtLeastTargetVersion(p.unencryptedFS, p.path, target)