From 0859f1ba896ba0718780a8bd0429a5906eb18900 Mon Sep 17 00:00:00 2001
From: Yuming Zhu <yzhu@redhat.com>
Date: Thu, 19 Oct 2023 17:52:25 +0800
Subject: [PATCH] promoteBuild: update volume based on volume policy

---
 kojihub/kojihub.py                   | 19 ++++++-------------
 tests/test_hub/test_promote_build.py | 25 +++++--------------------
 2 files changed, 11 insertions(+), 33 deletions(-)

diff --git a/kojihub/kojihub.py b/kojihub/kojihub.py
index ff45dca5..d87ac91b 100644
--- a/kojihub/kojihub.py
+++ b/kojihub/kojihub.py
@@ -16000,9 +16000,8 @@ def _promote_build(build, user=None, strict=True, force=False):
             )
         else:
             return None
-    # task_id and volume_id are for apply_volume_policy
     # drop id to get build by NVR
-    target_build = dslice(binfo, ['name', 'version', 'task_id', 'volume_id'])
+    target_build = dslice(binfo, ['name', 'version'])
     target_build['release'] = target_release
     old_build = get_build(target_build)
     if old_build:
@@ -16022,17 +16021,6 @@ def _promote_build(build, user=None, strict=True, force=False):
         'user_id': user['id']
     }
     assert_policy('draft_promotion', policy_data, force=force)
-    # volume check, deny it if volume is changed as it's only allowed for admin
-    # after building, see applyVolumePolicy
-    new_volume = apply_volume_policy(target_build, strict=False, dry_run=True)
-    if new_volume is not None and new_volume['id'] != binfo['volume_id']:
-        # probably we can just apply the volume change here
-        if strict:
-            raise koji.GenericError(
-                f'Denial as volume will be changed to {new_volume["name"]}'
-            )
-        else:
-            return None
 
     koji.plugin.run_callbacks(
         'preBuildPromote',
@@ -16061,6 +16049,11 @@ def _promote_build(build, user=None, strict=True, force=False):
     move_and_symlink(koji.pathinfo.build(binfo), koji.pathinfo.build(new_binfo))
     ensure_volume_symlink(new_binfo)
 
+    # apply volume policy in case it's changed by release update.
+    apply_volume_policy(new_binfo, strict=False)
+
+    # adding DRAFT_PROMOTION for kojira,
+    # as the latest promoted build should be that latest one.
     for tag in list_tags(build=binfo['id']):
         set_tag_update(tag['id'], 'DRAFT_PROMOTION')
 
diff --git a/tests/test_hub/test_promote_build.py b/tests/test_hub/test_promote_build.py
index d987a806..7d7b87a9 100644
--- a/tests/test_hub/test_promote_build.py
+++ b/tests/test_hub/test_promote_build.py
@@ -2,6 +2,7 @@ import datetime
 import json
 import mock
 import unittest
+
 import koji
 import kojihub
 
@@ -101,6 +102,9 @@ class TestPromoteBuild(unittest.TestCase):
                                        'extra': extra})
         self.assertEqual(update.rawdata, {})
         self.assertEqual(update.clauses, ['id=%(id)i'])
+        self.apply_volume_policy.assert_called_once_with(
+            self.new_build, strict=False
+        )
 
     def test_promote_build_not_draft(self):
         self.get_build.return_value = {'draft': False}
@@ -189,9 +193,7 @@ class TestPromoteBuild(unittest.TestCase):
         self.get_build.assert_called_with({
             'name': 'foo',
             'version': 'bar',
-            'release': 'tgtrel_1',
-            'task_id': 222,
-            'volume_id': 99
+            'release': 'tgtrel_1'
         })
         
         self.get_build.reset_mock()
@@ -199,20 +201,3 @@ class TestPromoteBuild(unittest.TestCase):
         ret = self.exports.promoteBuild('a-regular-build', strict=False)
         self.assertIsNone(ret)
         self.assertEqual(len(self.updates), 0)
-
-    def test_promote_build_volume_changed(self):
-        self.get_build.side_effect = [self.draft_build, None]
-        self.apply_volume_policy.return_value = {
-            'id': 100,
-            'name': 'Y'
-        }
-        with self.assertRaises(koji.GenericError) as cm:
-            self.exports.promoteBuild('a-regular-build', strict=True)
-        self.assertEqual(str(cm.exception), f"Denial as volume will be changed to Y")
-        self.assertEqual(len(self.updates), 0)
-
-        self.get_build.reset_mock()
-        self.get_build.side_effect = [self.draft_build, None]
-        ret = self.exports.promoteBuild('a-regular-build', strict=False)
-        self.assertIsNone(ret)
-        self.assertEqual(len(self.updates), 0)