From 768c5235239c5207b1a3c6ca0605bbe5ed4a6a8e Mon Sep 17 00:00:00 2001
From: Alan Bishop <abishop@redhat.com>
Date: Thu, 15 Mar 2018 13:04:46 -0400
Subject: [PATCH] Sync snapshot's encryption_key_id with volume's value

Sync the snapshot's encryption_key_id with the volume's value just
prior to updating the snapshot's DB entry. This ensures the snapshot
DB entry isn't out of date in case the volume's encryption_key_id
changed while the snapshot was in flight. The encryption_key_id will
change when keys based on the ConfKeyManager are migrated to Barbican.

Closes-Bug: #1756139
Change-Id: I65abb8dd17de7633828b731d1cf1f2321a6f3e5b
---
 cinder/keymgr/migration.py                |  3 ---
 cinder/tests/unit/volume/test_snapshot.py | 25 +++++++++++++++++++++++
 cinder/volume/manager.py                  |  4 ++++
 3 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/cinder/keymgr/migration.py b/cinder/keymgr/migration.py
index fbf6c153d48..fddce081a5c 100644
--- a/cinder/keymgr/migration.py
+++ b/cinder/keymgr/migration.py
@@ -138,9 +138,6 @@ class KeyMigrator(object):
         volume.encryption_key_id = encryption_key_id
         volume.save()
 
-        # TODO(abishop): need to determine if any snapshot creations are
-        # in-flight that might be added to the db with the volume's old
-        # fixed key ID.
         snapshots = objects.snapshot.SnapshotList.get_all_for_volume(
             self.admin_context,
             volume.id)
diff --git a/cinder/tests/unit/volume/test_snapshot.py b/cinder/tests/unit/volume/test_snapshot.py
index 942aba06020..dfd8c902062 100644
--- a/cinder/tests/unit/volume/test_snapshot.py
+++ b/cinder/tests/unit/volume/test_snapshot.py
@@ -476,6 +476,31 @@ class SnapshotTestCase(base.BaseVolumeTestCase):
         snapshot.destroy()
         db.volume_destroy(self.context, volume_id)
 
+    def test_create_snapshot_during_encryption_key_migration(self):
+        fixed_key_id = '00000000-0000-0000-0000-000000000000'
+        volume = tests_utils.create_volume(self.context, **self.volume_params)
+        volume['encryption_key_id'] = fixed_key_id
+        volume_id = volume['id']
+
+        self.volume.create_volume(self.context, volume)
+
+        kwargs = {'encryption_key_id': fixed_key_id}
+        snapshot = create_snapshot(volume['id'], **kwargs)
+
+        self.assertEqual(fixed_key_id, snapshot.encryption_key_id)
+        db.volume_update(self.context,
+                         volume_id,
+                         {'encryption_key_id': fake.ENCRYPTION_KEY_ID})
+
+        self.volume.create_snapshot(self.context, snapshot)
+
+        snap_db = db.snapshot_get(self.context, snapshot.id)
+        self.assertEqual(fake.ENCRYPTION_KEY_ID, snap_db.encryption_key_id)
+
+        # cleanup resource
+        snapshot.destroy()
+        db.volume_destroy(self.context, volume_id)
+
     def test_delete_busy_snapshot(self):
         """Test snapshot can be created and deleted."""
 
diff --git a/cinder/volume/manager.py b/cinder/volume/manager.py
index 4635f1a5d0e..91c746c05d4 100644
--- a/cinder/volume/manager.py
+++ b/cinder/volume/manager.py
@@ -1073,6 +1073,10 @@ class VolumeManager(manager.CleanableManager,
 
         snapshot.status = fields.SnapshotStatus.AVAILABLE
         snapshot.progress = '100%'
+        # Resync with the volume's DB value. This addresses the case where
+        # the snapshot creation was in flight just prior to when the volume's
+        # fixed_key encryption key ID was migrated to Barbican.
+        snapshot.encryption_key_id = vol_ref.encryption_key_id
         snapshot.save()
 
         self._notify_about_snapshot_usage(context, snapshot, "create.end")