rpm_transaction_conf: introduce actor configs

Add actor configuration using leapp's actor config features. The old
transaction conf files are still preserved, but they were never
advertised as an official way of modifying the transaction, whereas
configs added by this commit are. The configuration is functionally
equivalent to the old transaction configuration files.

repos/system_upgrade/common/actors/rpmtransactionconfigtaskscollector/actor.py

@@ -1,10 +1,9 @@
 from leapp.actors import Actor
+from leapp.configs.actor.rpm import Transaction_ToInstall, Transaction_ToKeep, Transaction_ToRemove
 from leapp.libraries.actor.rpmtransactionconfigtaskscollector import load_tasks
 from leapp.models import DistributionSignedRPM, RpmTransactionTasks
 from leapp.tags import FactsPhaseTag, IPUWorkflowTag
 
-CONFIGURATION_BASE_PATH = '/etc/leapp/transaction'
-
 
 class RpmTransactionConfigTasksCollector(Actor):
     """
@@ -13,11 +12,11 @@ class RpmTransactionConfigTasksCollector(Actor):
     After collecting task data from /etc/leapp/transaction directory, a message with relevant data
     will be produced.
     """
-
+    config_schemas = (Transaction_ToInstall, Transaction_ToKeep, Transaction_ToRemove)
     name = 'rpm_transaction_config_tasks_collector'
     consumes = (DistributionSignedRPM,)
     produces = (RpmTransactionTasks,)
     tags = (FactsPhaseTag, IPUWorkflowTag)
 
     def process(self):
-        self.produce(load_tasks(CONFIGURATION_BASE_PATH, self.log))
+        self.produce(load_tasks(self.config, self.log))

repos/system_upgrade/common/actors/rpmtransactionconfigtaskscollector/configs/rpm.py

@@ -0,0 +1,66 @@
+"""
+Configuration keys for dnf transactions.
+"""
+
+from leapp.actors.config import Config
+from leapp.models import fields
+
+TRANSACTION_CFG_SECTION_NAME = "transaction"
+
+
+# * Nested containers?
+# * Duplication of default value in type_ and Config.  If we eliminate that, we need to extract
+# default from the type_ for the documentation.
+# * We probably want to allow dicts in Config.  But IIRC, dicts were
+# specifically excluded for model fields.  Do we need something that restricts
+# where fields are valid?
+# * Test that type validation is strict.  For instance, giving an integer like 644 to
+# a field.String() is an error.
+class Transaction_ToInstall(Config):
+    section = TRANSACTION_CFG_SECTION_NAME
+    name = "to_install"
+    type_ = fields.List(fields.String(), default=[])
+    default = []
+    description = """
+        List of packages to be added to the upgrade transaction.
+        Signed packages which are already installed will be skipped.
+    """
+
+
+class Transaction_ToKeep(Config):
+    section = TRANSACTION_CFG_SECTION_NAME
+    name = "to_keep"
+    type_ = fields.List(fields.String(), default=[
+        "leapp",
+        "python2-leapp",
+        "python3-leapp",
+        "leapp-repository",
+        "snactor",
+    ])
+    default = [
+        "leapp",
+        "python2-leapp",
+        "python3-leapp",
+        "leapp-repository",
+        "snactor",
+    ]
+    description = """
+        List of packages to be kept in the upgrade transaction. The default is
+        leapp, python2-leapp, python3-leapp, leapp-repository, snactor. If you
+        override this, remember to include the default values if applicable.
+    """
+
+
+class Transaction_ToRemove(Config):
+    section = TRANSACTION_CFG_SECTION_NAME
+    name = "to_remove"
+    type_ = fields.List(fields.String(), default=[
+        "initial-setup",
+    ])
+    default = ["initial-setup"]
+    description = """
+        List of packages to be removed from the upgrade transaction. The default
+        is initial-setup which should be removed to avoid it asking for EULA
+        acceptance during upgrade. If you override this, remember to include the
+        default values if applicable.
+    """

repos/system_upgrade/common/actors/rpmtransactionconfigtaskscollector/libraries/rpmtransactionconfigtaskscollector.py

@@ -3,6 +3,10 @@
 from leapp.libraries.stdlib import api
 from leapp.models import DistributionSignedRPM, RpmTransactionTasks
 
+# Deprecated.  This is the old, pre-actor config method of customizing which
+# packages to keep, remove, and install.
+_CONFIGURATION_BASE_PATH = '/etc/leapp/transaction'
+
 
 def load_tasks_file(path, logger):
     # Loads the given file and converts it to a deduplicated list of strings that are stripped
@@ -18,21 +22,35 @@ def load_tasks_file(path, logger):
     return []
 
 
-def load_tasks(base_dir, logger):
+def load_tasks(config, logger, base_dir=_CONFIGURATION_BASE_PATH):
     # Loads configuration files to_install, to_keep, and to_remove from the given base directory
     rpms = next(api.consume(DistributionSignedRPM))
     rpm_names = [rpm.name for rpm in rpms.items]
-    to_install = load_tasks_file(os.path.join(base_dir, 'to_install'), logger)
-    # we do not want to put into rpm transaction what is already installed (it will go to "to_upgrade" bucket)
+    to_keep = frozenset(config['transaction']['to_keep'])
+    to_keep = to_keep.union(load_tasks_file(
+        os.path.join(base_dir, 'to_keep'), logger))
+    to_keep = list(to_keep)
+
+    to_remove = frozenset(config['transaction']['to_remove'])
+    to_remove = to_remove.union(load_tasks_file(
+        os.path.join(base_dir, 'to_remove'), logger))
+    to_remove = list(to_remove)
+
+    to_install = frozenset(config['transaction']['to_install'])
+    to_install = to_install.union(load_tasks_file(
+        os.path.join(base_dir, 'to_install'), logger))
+    # we do not want to put into rpm transaction what is already installed
+    # (it will go to "to_upgrade" bucket)
     to_install_filtered = [pkg for pkg in to_install if pkg not in rpm_names]
 
-    filtered = set(to_install) - set(to_install_filtered)
+    filtered = to_install.difference(to_install_filtered)
     if filtered:
         api.current_logger().debug(
-            'The following packages from "to_install" file will be ignored as they are already installed:'
-            '\n- ' + '\n- '.join(filtered))
+            'The following packages from "to_install" file will be ignored as'
+            ' they are already installed:\n- ' + '\n- '.join(filtered))
 
     return RpmTransactionTasks(
-        to_install=to_install_filtered,
-        to_keep=load_tasks_file(os.path.join(base_dir, 'to_keep'), logger),
-        to_remove=load_tasks_file(os.path.join(base_dir, 'to_remove'), logger))
+        to_install=sorted(to_install_filtered),
+        to_keep=sorted(to_keep),
+        to_remove=sorted(to_remove)
+    )

repos/system_upgrade/common/actors/rpmtransactionconfigtaskscollector/tests/test_load_tasks_rpmtransactionconfigtaskscollector.py

@@ -1,13 +1,82 @@
 import logging
 
+import pytest
+
+import leapp.libraries.actor.rpmtransactionconfigtaskscollector as rpm_transaction_cfg_actor_lib
+from leapp.configs.actor.rpm import (
+    TRANSACTION_CFG_SECTION_NAME,
+    Transaction_ToInstall,
+    Transaction_ToKeep,
+    Transaction_ToRemove
+)
 from leapp.libraries.actor.rpmtransactionconfigtaskscollector import load_tasks, load_tasks_file
+from leapp.libraries.common.testutils import _make_default_config, CurrentActorMocked, logger_mocked
 from leapp.libraries.stdlib import api
 from leapp.models import DistributionSignedRPM, RPM
 
 RH_PACKAGER = 'Red Hat, Inc. <http://bugzilla.redhat.com/bugzilla>'
 
 
-def test_load_tasks(tmpdir, monkeypatch):
+@pytest.mark.parametrize(
+    (
+        'to_install_file',
+        'to_keep_file',
+        'to_remove_file',
+        'to_install_config',
+        'to_keep_config',
+        'to_remove_config',
+        'to_install',
+        'to_keep',
+        'to_remove',
+    ),
+    (
+        (
+            'a\n b\n  c \n\n\nc\na\nc\nb',
+            'a\n b\n  c \n\n\nc\na\nc\nb',
+            'a\n b\n  c \n\n\nc\na\nc\nb',
+            [],
+            [],
+            [],
+            frozenset(('a', 'b')),
+            frozenset(('a', 'b', 'c')),
+            frozenset(('a', 'b', 'c')),
+        ),
+        (
+            'a\n b\n  c \n\n\nc\na\nc\nb',
+            'a\n b\n  c \n\n\nc\na\nc\nb',
+            'a\n b\n  c \n\n\nc\na\nc\nb',
+            ['a', 'd'],
+            ['a', 'd'],
+            ['a', 'd'],
+            frozenset(('a', 'b', 'd')),
+            frozenset(('a', 'b', 'c', 'd')),
+            frozenset(('a', 'b', 'c', 'd')),
+        ),
+        (
+            '',
+            '\n',
+            '',
+            ['a', 'b', 'c', 'c', 'a', 'c', 'b'],
+            ['a', 'b', 'c', 'c', 'a', 'c', 'b'],
+            ['a', 'b', 'c', 'c', 'a', 'c', 'b'],
+            frozenset(('a', 'b')),
+            frozenset(('a', 'b', 'c')),
+            frozenset(('a', 'b', 'c')),
+        ),
+    )
+)
+def test_load_tasks(to_install_file,
+                    to_keep_file,
+                    to_remove_file,
+                    to_install_config,
+                    to_keep_config,
+                    to_remove_config,
+                    to_install,
+                    to_keep,
+                    to_remove,
+                    tmpdir,
+                    monkeypatch,
+                    ):
 
     def consume_signed_rpms_mocked(*models):
         installed = [
@@ -18,14 +87,25 @@ def consume_signed_rpms_mocked(*models):
 
     monkeypatch.setattr(api, "consume", consume_signed_rpms_mocked)
 
-    tmpdir.join('to_install').write('a\n b\n  c \n\n\nc\na\nc\nb')
-    tmpdir.join('to_keep').write('a\n b\n  c \n\n\nc\na\nc\nb')
-    tmpdir.join('to_remove').write('a\n b\n  c \n\n\nc\na\nc\nb')
-    m = load_tasks(tmpdir.strpath, logging)
+    # Set values in the legacy configuration files
+    tmpdir.join('to_install').write(to_install_file)
+    tmpdir.join('to_keep').write(to_keep_file)
+    tmpdir.join('to_remove').write(to_remove_file)
+
+    # Simulate how the new actor config will come to us
+    config = {
+        'transaction': {
+            'to_install': to_install_config,
+            'to_keep': to_keep_config,
+            'to_remove': to_remove_config,
+        }
+    }
+
+    m = load_tasks(config, logging, base_dir=tmpdir.strpath)
     # c is not going to be in "to_install" as it is already installed
-    assert set(m.to_install) == set(['a', 'b'])
-    assert set(m.to_keep) == set(['a', 'b', 'c'])
-    assert set(m.to_remove) == set(['a', 'b', 'c'])
+    assert frozenset(m.to_install) == to_install
+    assert frozenset(m.to_keep) == to_keep
+    assert frozenset(m.to_remove) == to_remove
 
 
 def test_load_tasks_file(tmpdir):
@@ -35,3 +115,21 @@ def test_load_tasks_file(tmpdir):
     f = tmpdir.join('to_keep')
     f.write(' ')
     assert set(load_tasks_file(f.strpath, logging)) == set([])
+
+
+def test_config_is_reflected_in_actors_output(monkeypatch):
+    monkeypatch.setattr(rpm_transaction_cfg_actor_lib, 'load_tasks_file', lambda *args, **kwargs: set())
+    msgs = [DistributionSignedRPM(items=[])]
+    monkeypatch.setattr(api, 'current_actor', CurrentActorMocked(msgs=msgs))
+
+    config = _make_default_config((Transaction_ToInstall, Transaction_ToKeep, Transaction_ToRemove))
+    config[TRANSACTION_CFG_SECTION_NAME][Transaction_ToInstall.name] = ['install-a', 'install-b']
+    config[TRANSACTION_CFG_SECTION_NAME][Transaction_ToRemove.name] = ['remove-a', 'remove-b']
+    config[TRANSACTION_CFG_SECTION_NAME][Transaction_ToKeep.name] = ['keep-a', 'keep-b']
+
+    logger = logger_mocked()
+
+    produced_transaction_cfg = rpm_transaction_cfg_actor_lib.load_tasks(config, logger)
+    assert produced_transaction_cfg.to_install == ['install-a', 'install-b']
+    assert produced_transaction_cfg.to_remove == ['remove-a', 'remove-b']
+    assert produced_transaction_cfg.to_keep == ['keep-a', 'keep-b']