treefile: Add new repo-packages field for pinning packages to repos

This addresses the server compose side of
coreos#2584.

One tricky bit is handling overrides across included treefiles (or
really, even within a single treefile): as usual, higher-level treefiles
should override lowel-level ones. Rust makes it pretty nice to handle.

For now this just supports a `repo` field, but one could imagine e.g.
`repos` (which takes an array of repoids instead), or e.g.
`exclude-repos`.

The actual core implementation otherwise is pretty straightforward.

This should help a lot in RHCOS where we currently use lots of
`exclude=` in repo files to get it to do what we want.

This is also a kind of a requirement for modularity support because as
soon as rpm-ostree becomes modules-aware, modular filtering logic will
break composes which assume rpm-ostree treats modular and non-modular
packages the same.

docs/treefile.md

@@ -73,6 +73,11 @@ It supports the following parameters:
    An example use case for this is for Fedora CoreOS, which will blacklist the `python` and `python3`
    packages to ensure that nothing included in the OS starts depending on it in the future.
 
+ * `repo-packages`: Array of objects, optional: Set of packages to install from
+   specific repos. Each object in the array supports the following keys:
+   * `packages`: Array of strings, required: List of packages to install.
+   * `repo`: String, required: Name of the repo from which to fetch packages.
+
  * `ostree-layers`: Array of strings, optional: After all packages are unpacked,
     check out these OSTree refs, which must already be in the destination repository.
     Any conflicts with packages will be an error.

rust/src/lib.rs

@@ -276,6 +276,15 @@ pub mod ffi {
         fn sanitycheck_externals(&self) -> Result<()>;
         fn get_checksum(&self, repo: Pin<&mut OstreeRepo>) -> Result<String>;
         fn get_ostree_ref(&self) -> String;
+        fn get_repo_packages(&self) -> &[RepoPackage];
+    }
+
+    // treefile.rs (split out from above to make &self nice to use)
+    extern "Rust" {
+        type RepoPackage;
+
+        fn get_repo(&self) -> &str;
+        fn get_packages(&self) -> &[String];
     }
 
     // utils.rs

rust/src/treefile.rs

@@ -135,6 +135,20 @@ fn treefile_parse_stream<R: io::Read>(
         }
     }
 
+    if let Some(repo_packages) = treefile.repo_packages.take() {
+        treefile.repo_packages = Some(
+            repo_packages
+                .into_iter()
+                .map(|rp| -> Result<RepoPackage> {
+                    Ok(RepoPackage {
+                        repo: rp.repo,
+                        packages: whitespace_split_packages(&rp.packages)?,
+                    })
+                })
+                .collect::<Result<Vec<RepoPackage>>>()?,
+        );
+    }
+
     treefile.packages = Some(pkgs);
     treefile = treefile.migrate_legacy_fields()?;
     Ok(treefile)
@@ -373,7 +387,8 @@ fn treefile_merge(dest: &mut TreeComposeConfig, src: &mut TreeComposeConfig) {
         postprocess,
         add_files,
         remove_files,
-        remove_from_packages
+        remove_from_packages,
+        repo_packages
     );
 }
 
@@ -464,6 +479,7 @@ impl Treefile {
     ) -> Result<Box<Treefile>> {
         let mut seen_includes = collections::BTreeMap::new();
         let mut parsed = treefile_parse_recurse(filename, basearch, 0, &mut seen_includes)?;
+        parsed.config.handle_repo_packages_overrides();
         parsed.config = parsed.config.substitute_vars()?;
         Treefile::validate_config(&parsed.config)?;
         let dfd = openat::Dir::open(utils::parent_dir(filename).unwrap())?;
@@ -626,6 +642,14 @@ impl Treefile {
         files_to_remove
     }
 
+    pub(crate) fn get_repo_packages(&self) -> &[RepoPackage] {
+        self.parsed
+            .repo_packages
+            .as_ref()
+            .map(|v| v.as_slice())
+            .unwrap_or_default()
+    }
+
     /// Do some upfront semantic checks we can do beyond just the type safety serde provides.
     fn validate_config(config: &TreeComposeConfig) -> Result<()> {
         // check add-files
@@ -807,6 +831,16 @@ for x in *; do mv ${{x}} %{{buildroot}}%{{_prefix}}/lib/ostree-jigdo/%{{name}};
     }
 }
 
+impl RepoPackage {
+    pub(crate) fn get_repo(&self) -> &str {
+        self.repo.as_str()
+    }
+
+    pub(crate) fn get_packages(&self) -> &[String] {
+        self.packages.as_slice()
+    }
+}
+
 fn hash_file(hasher: &mut glib::Checksum, mut f: &fs::File) -> Result<()> {
     let mut reader = io::BufReader::with_capacity(128 * 1024, f);
     loop {
@@ -1052,6 +1086,9 @@ pub(crate) struct TreeComposeConfig {
     // Core content
     #[serde(skip_serializing_if = "Option::is_none")]
     pub(crate) packages: Option<Vec<String>>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    #[serde(rename = "repo-packages")]
+    pub(crate) repo_packages: Option<Vec<RepoPackage>>,
     // Deprecated option
     #[serde(skip_serializing_if = "Option::is_none")]
     pub(crate) bootstrap_packages: Option<Vec<String>>,
@@ -1170,6 +1207,12 @@ pub(crate) struct TreeComposeConfig {
     pub(crate) extra: HashMap<String, serde_json::Value>,
 }
 
+#[derive(Serialize, Deserialize, Debug, Default)]
+pub(crate) struct RepoPackage {
+    pub(crate) repo: String,
+    pub(crate) packages: Vec<String>,
+}
+
 #[derive(Serialize, Deserialize, Debug, Default)]
 pub(crate) struct LegacyTreeComposeConfigFields {
     #[serde(skip_serializing)]
@@ -1255,6 +1298,23 @@ impl TreeComposeConfig {
         static DEFAULT: CheckGroups = CheckGroups::Previous;
         self.check_groups.as_ref().unwrap_or(&DEFAULT)
     }
+
+    // we need to ensure that higher repo packages override lower ones
+    fn handle_repo_packages_overrides(&mut self) {
+        if let Some(repo_packages) = self.repo_packages.take() {
+            let mut seen_pkgs: HashSet<String> = HashSet::new();
+            self.repo_packages = Some(
+                repo_packages
+                    .into_iter()
+                    .map(|mut rp| {
+                        rp.packages.retain(|p| seen_pkgs.insert(p.into()));
+                        rp
+                    })
+                    .filter(|rp| !rp.packages.is_empty())
+                    .collect(),
+            );
+        }
+    }
 }
 
 #[cfg(test)]
@@ -1278,6 +1338,10 @@ pub(crate) mod tests {
          - grub2 grub2-tools
         packages-s390x:
          - zipl
+        repo-packages:
+            - repo: baserepo
+              packages:
+                - blah bloo
     "#};
 
     // This one has "comments" (hence unknown keys)
@@ -1300,6 +1364,9 @@ pub(crate) mod tests {
         treefile = treefile.substitute_vars().unwrap();
         assert!(treefile.treeref.unwrap() == "exampleos/x86_64/blah");
         assert!(treefile.packages.unwrap().len() == 7);
+        assert!(treefile.repo_packages.as_ref().unwrap().len() == 1);
+        assert!(treefile.repo_packages.as_ref().unwrap()[0].repo == "baserepo");
+        assert!(treefile.repo_packages.as_ref().unwrap()[0].packages.len() == 2);
     }
 
     #[test]
@@ -1524,6 +1591,18 @@ pub(crate) mod tests {
                     - foo
                 packages:
                     - fooinclude
+                repo-packages:
+                    - repo: foo
+                      packages:
+                        - qwert
+                    # this entry is overridden by the prelude treefile; so will disappear
+                    - repo: foob
+                      packages:
+                        - blah
+                    # this entry is overridden by the first entry; so will disappear
+                    - repo: foo2
+                      packages:
+                        - qwert
             "},
         )?;
         let mut buf = VALID_PRELUDE.to_string();
@@ -1532,6 +1611,27 @@ pub(crate) mod tests {
         "});
         let tf = new_test_treefile(workdir.path(), buf.as_str(), None)?;
         assert!(tf.parsed.packages.unwrap().len() == 6);
+        assert_eq!(tf.parsed.repo_packages.as_ref().unwrap().len(), 2);
+        assert_eq!(
+            tf.parsed.repo_packages.as_ref().unwrap()[0].packages.len(),
+            2
+        );
+        assert_eq!(
+            tf.parsed.repo_packages.as_ref().unwrap()[0].packages[0],
+            "blah"
+        );
+        assert_eq!(
+            tf.parsed.repo_packages.as_ref().unwrap()[0].packages[1],
+            "bloo"
+        );
+        assert_eq!(
+            tf.parsed.repo_packages.as_ref().unwrap()[1].packages.len(),
+            1
+        );
+        assert_eq!(
+            tf.parsed.repo_packages.as_ref().unwrap()[1].packages[0],
+            "qwert"
+        );
         Ok(())
     }
 

src/libpriv/rpmostree-core.cxx

@@ -2200,6 +2200,35 @@ rpmostree_context_prepare (RpmOstreeContext *self,
   GLNX_HASH_TABLE_FOREACH_V (local_pkgs_to_install, DnfPackage*, pkg)
     hy_goal_install (goal,  pkg);
 
+  /* Now repo-packages; only supported during server composes for now. */
+  if (!self->is_system)
+    {
+      auto repo_pkgs = self->treefile_rs->get_repo_packages();
+      for (auto & repo_pkg : repo_pkgs)
+        {
+          auto repo = std::string(repo_pkg.get_repo());
+          auto pkgs = repo_pkg.get_packages();
+          for (auto & pkg_str : pkgs)
+            {
+              auto pkg = std::string(pkg_str);
+              g_auto(HySubject) subject = hy_subject_create(pkg.c_str());
+              /* this is basically like a slightly customized dnf_context_install() */
+              HyNevra nevra = NULL;
+              hy_autoquery HyQuery query =
+                hy_subject_get_best_solution(subject, sack, NULL, &nevra, FALSE, TRUE, TRUE, TRUE, FALSE);
+              hy_query_filter (query, HY_PKG_REPONAME, HY_EQ, repo.c_str());
+              g_autoptr(DnfPackageSet) pset = hy_query_run_set (query);
+              if (dnf_packageset_count (pset) == 0)
+                return glnx_throw (error, "No matches for '%s' in repo '%s'", pkg.c_str(), repo.c_str());
+
+              g_auto(HySelector) selector = hy_selector_create (sack);
+              hy_selector_pkg_set (selector, pset);
+              if (!hy_goal_install_selector (goal, selector, error))
+                return FALSE;
+            }
+        }
+    }
+
   /* And finally, handle repo packages to install */
   g_autoptr(GPtrArray) missing_pkgs = NULL;
   for (char **it = pkgnames; it && *it; it++)

tests/compose/test-basic-unified.sh

@@ -17,7 +17,14 @@ uinfo_cmd add-ref TEST-SEC-LOW 1 http://example.com/vuln1 "CVE-12-34 vuln1"
 
 echo gpgcheck=0 >> yumrepo.repo
 ln "$PWD/yumrepo.repo" config/yumrepo.repo
-treefile_append "packages" '["foobar", "vuln-pkg"]'
+treefile_append "packages" '["vuln-pkg"]'
+
+treefile_pyedit "
+tf['repo-packages'] = [{
+  'repo': 'test-repo',
+  'packages': ['foobar'],
+}]
+"
 
 # Test --print-only.  We also
 # just in this test (for now) use ${basearch} to test substitution.

tests/compose/test-basic.sh

@@ -15,7 +15,12 @@ build_rpm foobar-rec
 
 echo gpgcheck=0 >> yumrepo.repo
 ln "$PWD/yumrepo.repo" config/yumrepo.repo
-treefile_append "packages" '["foobar"]'
+treefile_pyedit "
+tf['repo-packages'] = [{
+  'repo': 'test-repo',
+  'packages': ['foobar'],
+}]
+"
 
 treefile_pyedit "tf['add-commit-metadata']['foobar'] = 'bazboo'"
 treefile_pyedit "tf['add-commit-metadata']['overrideme'] = 'old var'"