[ART-4681] improve rpm modification scripts support

A few changes need to be made to support running microshift rebase
script:

- The modification script should be run inside of the upstream source
  directory instead of distgit directory. This is because distgit repos
for rpms only contain specfile, tarball source, and patch files, which
is not much useful. Though this is a breaking change, no rpms in
supported OCP versions use this feature so it is ok to make this
breaking change.
- Enhance `command` modification to pass environment variables to the
  script. We need this ability to pass the pullspecs of release images
to microshift rebase script. Example config:
  ```yaml
    modifications:
    - action: command
      command: microshift-rebase
      env:
        RELEASE_NAME: "{release_name}"
  ```

With this enhancement, we can rebase and build 4.11 microshift rpm (openshift-eng/ocp-build-data#1998) by explicitly invoking doozer with
`doozer --group openshift-4.11 --assembly=4.11.6 --rpms=microshift
rpms:rebase-and-build --version 4.11.6 --release $release.p?`

.devcontainer/dev.Dockerfile

@@ -12,8 +12,10 @@ RUN curl -o /etc/pki/ca-trust/source/anchors/RH-IT-Root-CA.crt --fail -L \
 
 RUN dnf install -y \
     # runtime dependencies
-    krb5-workstation git tig rsync koji skopeo podman docker \
+    krb5-workstation git tig rsync koji skopeo podman docker rpmdevtools \
     python3.8 python3-certifi \
+    # required by microshift
+    yq jq golang \
     # provides en_US.UTF-8 locale
     glibc-langpack-en \
     # development dependencies

doozerlib/distgit.py

@@ -2522,8 +2522,6 @@ class RPMDistGitRepo(DistGitRepo):
     def __init__(self, metadata, autoclone=True):
         super(RPMDistGitRepo, self).__init__(metadata, autoclone)
         self.source = self.config.content.source
-        # if self.source.specfile is Missing:
-        #     raise ValueError('Must specify spec file name for RPMs.')
 
     async def resolve_specfile_async(self) -> Tuple[pathlib.Path, Tuple[str, str, str], str]:
         """ Returns the path, NVR, and commit hash of the spec file in distgit_dir

doozerlib/rpm_builder.py

@@ -84,6 +84,13 @@ async def rebase(self, rpm: RPMMetadata, version: str, release: str) -> str:
 
         rpm.set_nvr(version, release)
 
+        # run modifications
+        if rpm.config.content.source.modifications is not Missing:
+            logger.info("Running custom modifications...")
+            await exectools.to_thread(
+                rpm._run_modifications, rpm.specfile, rpm.source_path
+            )
+
         # generate new specfile
         tarball_name = f"{rpm.config.name}-{rpm.version}-{rpm.release}.tar.gz"
         logger.info("Creating rpm spec file...")
@@ -153,19 +160,12 @@ async def rebase(self, rpm: RPMMetadata, version: str, release: str) -> str:
                     )
                 )
             dest.parent.mkdir(parents=True, exist_ok=True)
-            logging.debug("Copying %s", filename)
+            logger.debug("Copying %s", filename)
 
             shutil.copy(src, dest, follow_symlinks=False)
 
-        # run modifications
-        if rpm.config.content.source.modifications is not Missing:
-            logging.info("Running custom modifications...")
-            await exectools.to_thread(
-                rpm._run_modifications, dg_specfile_path, dg.dg_path
-            )
-
         # commit changes
-        logging.info("Committing distgit changes...")
+        logger.info("Committing distgit changes...")
         await aiofiles.os.remove(tarball_path)
         commit_hash = await exectools.to_thread(dg.commit,
                                                 f"Automatic commit of package [{rpm.config.name}] release [{rpm.version}-{rpm.release}].",

doozerlib/rpmcfg.py

@@ -3,6 +3,7 @@
 import os
 import threading
 from typing import Optional
+from doozerlib.assembly import AssemblyTypes
 
 from doozerlib.rpm_utils import labelCompare
 
@@ -88,7 +89,7 @@ def set_nvr(self, version, release):
 
     def _run_modifications(self, specfile: Optional[os.PathLike] = None, cwd: Optional[os.PathLike] = None):
         """
-        Interprets and applies content.source.modify steps in the image metadata.
+        Interprets and applies content.source.modify steps in the rpm metadata.
 
         :param specfile: Path to an alternative specfile. None means use the specfile in the source dir
         :param cwd: If set, change current working directory. None means use the source dir
@@ -106,18 +107,23 @@ def _run_modifications(self, specfile: Optional[os.PathLike] = None, cwd: Option
         metadata_scripts_path = self.runtime.data_dir + "/modifications"
         path = os.pathsep.join([os.environ['PATH'], metadata_scripts_path])
         new_specfile_data = specfile_data
+        context = {
+            "component_name": self.name,
+            "kind": "spec",
+            "content": new_specfile_data,
+            "set_env": {"PATH": path},
+            "runtime_assembly": self.runtime.assembly,
+        }
+
+        if self.runtime.assembly_type in [AssemblyTypes.STANDARD, AssemblyTypes.CANDIDATE, AssemblyTypes.PREVIEW]:
+            context["release_name"] = util.get_release_name(self.runtime.assembly_type, self.runtime.group, self.runtime.assembly, None)
 
         for modification in self.config.content.source.modifications:
             if self.source_modifier_factory.supports(modification.action):
                 # run additional modifications supported by source_modifier_factory
                 modifier = self.source_modifier_factory.create(**modification)
                 # pass context as a dict so that the act function can modify its content
-                context = {
-                    "component_name": self.name,
-                    "kind": "spec",
-                    "content": new_specfile_data,
-                    "set_env": {"PATH": path},
-                }
+                context["content"] = new_specfile_data
                 modifier.act(context=context, ceiling_dir=cwd or Dir.getcwd())
                 new_specfile_data = context.get("result")
             else:

doozerlib/source_modifications.py

@@ -1,10 +1,10 @@
 import io
 import os
+from pathlib import Path
 from urllib.parse import urlparse
 
 import requests
 import yaml
-from pathlib import Path
 
 from doozerlib.exceptions import DoozerFatalError
 from doozerlib.exectools import cmd_assert
@@ -180,16 +180,21 @@ def __init__(self, *args, **kwargs):
         :param command: a `str` or `list` of the command with arguments
         """
         self.command = kwargs["command"]
+        self.env = kwargs.get("env", {})
 
     def act(self, *args, **kwargs):
         """ Run the command
         :param context: A context dict. `context.set_env` is a `dict` of env vars to set for command (overriding existing).
         """
         context = kwargs["context"]
-        set_env = context["set_env"]
+        set_env = {}
+        for k, v in self.env.items():
+            set_env[k] = str(v).format(**context)
+        set_env.update(context["set_env"])
         ceiling_dir = kwargs["ceiling_dir"]
+
         with Dir(ceiling_dir):
-            cmd_assert(self.command, set_env=set_env)
+            cmd_assert(self.command, set_env=set_env, log_stdout=True, log_stderr=True)
 
 
 SourceModifierFactory.MODIFICATIONS["command"] = CommandModifier

doozerlib/util.py

@@ -24,7 +24,7 @@
 except ImportError:
     pass
 
-from doozerlib import constants, exectools
+from doozerlib import assembly, constants, exectools
 
 
 def stringify(val):
@@ -786,3 +786,36 @@ def find_manifest_list_sha(pull_spec):
     if 'listDigest' not in image_data:
         raise ValueError('Specified image is not a manifest-list.')
     return image_data['listDigest']
+
+
+def isolate_major_minor_in_group(group_name: str) -> Tuple[int, int]:
+    """
+    Given a group name, determines whether is contains
+    a OCP major.minor version. If it does, it returns the version value as (int, int).
+    If it is not found, (None, None) is returned.
+    """
+    match = re.fullmatch(r"openshift-(\d+).(\d+)", group_name)
+    if not match:
+        return None, None
+    return int(match[1]), int(match[2])
+
+
+def get_release_name(assembly_type: str, group_name: str, assembly_name: str, release_offset: Optional[int]):
+    major, minor = isolate_major_minor_in_group(group_name)
+    if major is None or minor is None:
+        raise ValueError(f"Invalid group name: {group_name}")
+    if assembly_type == assembly.AssemblyTypes.CUSTOM:
+        if release_offset is None:
+            raise ValueError("release_offset is required for a CUSTOM release.")
+        release_name = f"{major}.{minor}.{release_offset}-assembly.{assembly_name}"
+    elif assembly_type in [assembly.AssemblyTypes.CANDIDATE, assembly.AssemblyTypes.PREVIEW]:
+        if release_offset is not None:
+            raise ValueError(f"release_offset can't be set for a {assembly_type.value} release.")
+        release_name = f"{major}.{minor}.0-{assembly_name}"
+    elif assembly_type == assembly.AssemblyTypes.STANDARD:
+        if release_offset is not None:
+            raise ValueError("release_offset can't be set for a STANDARD release.")
+        release_name = f"{assembly_name}"
+    else:
+        raise ValueError(f"Assembly type {assembly_type} is not supported.")
+    return release_name

tests/test_rpm_builder.py

@@ -89,7 +89,7 @@ def test_rebase(self, mocked_cmd_assert_async: mock.Mock, mocked_open: mock.Mock
         mocked_cmd_assert_async.assert_called_with(["spectool", "--", dg.dg_path / "foo.spec"], cwd=dg.dg_path)
         mocked_copy.assert_any_call(Path(rpm.source_path) / "a.diff", dg.dg_path / "a.diff", follow_symlinks=False)
         mocked_copy.assert_any_call(Path(rpm.source_path) / "b/c.diff", dg.dg_path / "b/c.diff", follow_symlinks=False)
-        rpm._run_modifications.assert_called_once_with(dg.dg_path / "foo.spec", dg.dg_path)
+        rpm._run_modifications.assert_called_once_with(f"{rpm.source_path}/foo.spec", rpm.source_path)
         dg.commit.assert_called_once_with(f"Automatic commit of package [{rpm.config.name}] release [{rpm.version}-{rpm.release}].",
                                           commit_attributes={
                                               'version': '1.2.3',
@@ -129,7 +129,7 @@ def test_rebase_with_assembly(self, mocked_cmd_assert_async: mock.Mock, mocked_o
         mocked_cmd_assert_async.assert_called_with(["spectool", "--", dg.dg_path / "foo.spec"], cwd=dg.dg_path)
         mocked_copy.assert_any_call(Path(rpm.source_path) / "a.diff", dg.dg_path / "a.diff", follow_symlinks=False)
         mocked_copy.assert_any_call(Path(rpm.source_path) / "b/c.diff", dg.dg_path / "b/c.diff", follow_symlinks=False)
-        rpm._run_modifications.assert_called_once_with(dg.dg_path / "foo.spec", dg.dg_path)
+        rpm._run_modifications.assert_called_once_with(f'{rpm.source_path}/foo.spec', rpm.source_path)
         dg.commit.assert_called_once_with(f"Automatic commit of package [{rpm.config.name}] release [{rpm.version}-{rpm.release}].",
                                           commit_attributes={
                                               'version': '1.2.3',