From 00b81d8f5285105aebed1d600a2dabfa947e2caf Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Tue, 27 Apr 2021 16:38:46 -0400 Subject: [PATCH] treefile: Add new `repo-packages` field for pinning packages to repos This addresses the server compose side of https://github.com/coreos/rpm-ostree/issues/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 | 5 ++ rust/src/lib.rs | 9 +++ rust/src/treefile.rs | 102 +++++++++++++++++++++++++++- src/libpriv/rpmostree-core.cxx | 29 ++++++++ tests/compose/test-basic-unified.sh | 9 ++- tests/compose/test-basic.sh | 7 +- 6 files changed, 158 insertions(+), 3 deletions(-) diff --git a/docs/treefile.md b/docs/treefile.md index 1ecbf78f18..1103d1529a 100644 --- a/docs/treefile.md +++ b/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. diff --git a/rust/src/lib.rs b/rust/src/lib.rs index 853c07a83e..29ede7b330 100644 --- a/rust/src/lib.rs +++ b/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; 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 diff --git a/rust/src/treefile.rs b/rust/src/treefile.rs index 9b310057c5..692d7fa4ac 100644 --- a/rust/src/treefile.rs +++ b/rust/src/treefile.rs @@ -135,6 +135,20 @@ fn treefile_parse_stream( } } + if let Some(repo_packages) = treefile.repo_packages.take() { + treefile.repo_packages = Some( + repo_packages + .into_iter() + .map(|rp| -> Result { + Ok(RepoPackage { + repo: rp.repo, + packages: whitespace_split_packages(&rp.packages)?, + }) + }) + .collect::>>()?, + ); + } + 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> { 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>, + #[serde(skip_serializing_if = "Option::is_none")] + #[serde(rename = "repo-packages")] + pub(crate) repo_packages: Option>, // Deprecated option #[serde(skip_serializing_if = "Option::is_none")] pub(crate) bootstrap_packages: Option>, @@ -1170,6 +1207,12 @@ pub(crate) struct TreeComposeConfig { pub(crate) extra: HashMap, } +#[derive(Serialize, Deserialize, Debug, Default)] +pub(crate) struct RepoPackage { + pub(crate) repo: String, + pub(crate) packages: Vec, +} + #[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 = 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(()) } diff --git a/src/libpriv/rpmostree-core.cxx b/src/libpriv/rpmostree-core.cxx index 2d459fadac..054f917e66 100644 --- a/src/libpriv/rpmostree-core.cxx +++ b/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++) diff --git a/tests/compose/test-basic-unified.sh b/tests/compose/test-basic-unified.sh index c50dd41daf..d6ce786922 100755 --- a/tests/compose/test-basic-unified.sh +++ b/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. diff --git a/tests/compose/test-basic.sh b/tests/compose/test-basic.sh index ba4a6d3f8f..16a4a4a1e6 100755 --- a/tests/compose/test-basic.sh +++ b/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'"