From fe66deaf9c14b23960595267c856fe779835b3c0 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Fri, 4 Oct 2024 17:08:33 -0400 Subject: [PATCH 1/6] rust/core: Add util function to check for digest pullspec Add a new `is_container_image_digest_reference()` function which checks if the container image pullspec is using a digest. Prep for next patch. --- rpmostree-cxxrs.cxx | 10 ++++++++++ rpmostree-cxxrs.h | 4 ++++ rust/src/core.rs | 26 +++++++++++++++++++++++++- rust/src/lib.rs | 1 + 4 files changed, 40 insertions(+), 1 deletion(-) diff --git a/rpmostree-cxxrs.cxx b/rpmostree-cxxrs.cxx index 649d1b7b8c..6582db43cf 100644 --- a/rpmostree-cxxrs.cxx +++ b/rpmostree-cxxrs.cxx @@ -192,6 +192,8 @@ class Slice final : private detail::copy_assignable_if::value> Slice () noexcept; Slice (T *, std::size_t count) noexcept; + template explicit Slice (C &c) : Slice (c.data (), c.size ()) {} + Slice &operator= (const Slice &) &noexcept = default; Slice &operator= (Slice &&) &noexcept = default; @@ -2165,6 +2167,8 @@ extern "C" bool rpmostreecxx$cxxbridge1$is_container_image_reference (::rust::Str refspec) noexcept; + bool rpmostreecxx$cxxbridge1$is_container_image_digest_reference (::rust::Str refspec) noexcept; + ::rpmostreecxx::RefspecType rpmostreecxx$cxxbridge1$refspec_classify (::rust::Str refspec) noexcept; @@ -3882,6 +3886,12 @@ is_container_image_reference (::rust::Str refspec) noexcept return rpmostreecxx$cxxbridge1$is_container_image_reference (refspec); } +bool +is_container_image_digest_reference (::rust::Str refspec) noexcept +{ + return rpmostreecxx$cxxbridge1$is_container_image_digest_reference (refspec); +} + ::rpmostreecxx::RefspecType refspec_classify (::rust::Str refspec) noexcept { diff --git a/rpmostree-cxxrs.h b/rpmostree-cxxrs.h index d38fd1dbbd..9354436eb2 100644 --- a/rpmostree-cxxrs.h +++ b/rpmostree-cxxrs.h @@ -191,6 +191,8 @@ class Slice final : private detail::copy_assignable_if::value> Slice () noexcept; Slice (T *, std::size_t count) noexcept; + template explicit Slice (C &c) : Slice (c.data (), c.size ()) {} + Slice &operator= (const Slice &) &noexcept = default; Slice &operator= (Slice &&) &noexcept = default; @@ -1840,6 +1842,8 @@ void log_treefile (::rpmostreecxx::Treefile const &tf) noexcept; bool is_container_image_reference (::rust::Str refspec) noexcept; +bool is_container_image_digest_reference (::rust::Str refspec) noexcept; + ::rpmostreecxx::RefspecType refspec_classify (::rust::Str refspec) noexcept; void verify_kernel_hmac (::std::int32_t rootfs, ::rust::Str moddir); diff --git a/rust/src/core.rs b/rust/src/core.rs index 02255a8481..69ded654c5 100644 --- a/rust/src/core.rs +++ b/rust/src/core.rs @@ -17,6 +17,7 @@ use glib::prelude::StaticVariantType; use libdnf_sys::*; use ostree_ext::container::OstreeImageReference; use ostree_ext::glib; +use ostree_ext::oci_spec::distribution::Reference as OciReference; use ostree_ext::ostree; use std::fs::File; use std::io::{BufReader, Read}; @@ -114,6 +115,21 @@ pub(crate) fn is_container_image_reference(refspec: &str) -> bool { refspec_classify(refspec) == crate::ffi::RefspecType::Container } +/// Infer whether string is a container image reference. +pub(crate) fn is_container_image_digest_reference(refspec: &str) -> bool { + OstreeImageReference::try_from(refspec) + .and_then(|ostree_imgref| { + ostree_imgref + .imgref + .name + .parse::() + .map_err(anyhow::Error::msg) + }) + .map(|oci_ref| oci_ref.digest().is_some()) + .ok() + .unwrap_or_default() +} + /// Given a refspec, infer its type and return it. pub(crate) fn refspec_classify(refspec: &str) -> crate::ffi::RefspecType { if OstreeImageReference::try_from(refspec).is_ok() { @@ -503,17 +519,25 @@ mod test { "docker://quay.io/test-repository/os:version1", "registry:docker.io/test-repository/os:latest", "registry:customhostname.com:8080/test-repository/os:latest", - "docker://quay.io/test-repository/os@sha256:6006dca86c2dc549c123ff4f1dcbe60105fb05886531c93a3351ebe81dbe772f", ]; for refspec in REFSPEC_TYPE_CONTAINER { let refspec = format!("ostree-unverified-image:{}", refspec); assert!(is_container_image_reference(&refspec)); + assert!(!is_container_image_digest_reference(&refspec)); assert_eq!( refspec_classify(&refspec), crate::ffi::RefspecType::Container ); } + let refspec_type_container_digest = "docker://quay.io/test-repository/os@sha256:6006dca86c2dc549c123ff4f1dcbe60105fb05886531c93a3351ebe81dbe772f"; + let refspec = format!("ostree-unverified-image:{}", refspec_type_container_digest); + assert!(is_container_image_reference(&refspec)); + assert!(is_container_image_digest_reference(&refspec)); + assert_eq!( + refspec_classify(&refspec), + crate::ffi::RefspecType::Container + ); Ok(()) } diff --git a/rust/src/lib.rs b/rust/src/lib.rs index 56d8b57fa3..1405d64723 100644 --- a/rust/src/lib.rs +++ b/rust/src/lib.rs @@ -262,6 +262,7 @@ pub mod ffi { fn log_treefile(tf: &Treefile); fn is_container_image_reference(refspec: &str) -> bool; + fn is_container_image_digest_reference(refspec: &str) -> bool; fn refspec_classify(refspec: &str) -> RefspecType; fn verify_kernel_hmac(rootfs: i32, moddir: &str) -> Result<()>; From ec47f5d4b6f9d9018366a7edaebdcc42726229c5 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Fri, 4 Oct 2024 17:10:14 -0400 Subject: [PATCH 2/6] daemon/transaction-types: Lookup custom origin options earlier Almost no functional change (we do now error out if no description is given even if we're not going to use those fields, but that seems fine). Prep for next patch. --- src/daemon/rpmostreed-transaction-types.cxx | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/src/daemon/rpmostreed-transaction-types.cxx b/src/daemon/rpmostreed-transaction-types.cxx index dde4377002..f8a5a232bf 100644 --- a/src/daemon/rpmostreed-transaction-types.cxx +++ b/src/daemon/rpmostreed-transaction-types.cxx @@ -61,6 +61,17 @@ change_origin_refspec (GVariantDict *options, OstreeSysroot *sysroot, RpmOstreeO auto current_refspec = rpmostree_origin_get_refspec (origin); + const char *custom_origin_url = NULL; + const char *custom_origin_description = NULL; + g_variant_dict_lookup (options, "custom-origin", "(&s&s)", &custom_origin_url, + &custom_origin_description); + if (custom_origin_url && *custom_origin_url) + { + g_assert (custom_origin_description); + if (!*custom_origin_description) + return glnx_throw (error, "Invalid custom-origin"); + } + switch (refspectype) { case rpmostreecxx::RefspecType::Container: @@ -105,16 +116,6 @@ change_origin_refspec (GVariantDict *options, OstreeSysroot *sysroot, RpmOstreeO if (new_refspectype == rpmostreecxx::RefspecType::Checksum) { - const char *custom_origin_url = NULL; - const char *custom_origin_description = NULL; - g_variant_dict_lookup (options, "custom-origin", "(&s&s)", &custom_origin_url, - &custom_origin_description); - if (custom_origin_url && *custom_origin_url) - { - g_assert (custom_origin_description); - if (!*custom_origin_description) - return glnx_throw (error, "Invalid custom-origin"); - } rpmostree_origin_set_rebase_custom (origin, new_refspec, custom_origin_url, custom_origin_description); } From 83f15d6bcd3c03fa08f37e6f02cd22097d37be44 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Fri, 4 Oct 2024 17:11:35 -0400 Subject: [PATCH 3/6] daemon/transaction-types: Support custom origins for digested pullspecs Having a digested pullspec is the container flow equivalent of an ostree checksum origin. So having a custom origin in that case is useful there for the same reasons (i.e. it means that something else is likely managing the host and we want to reflect that info in the origin). We're thinking of using this in FCOS when we move from OSTree to OCI for updates. --- src/daemon/rpmostreed-transaction-types.cxx | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/daemon/rpmostreed-transaction-types.cxx b/src/daemon/rpmostreed-transaction-types.cxx index f8a5a232bf..fb7ef532a5 100644 --- a/src/daemon/rpmostreed-transaction-types.cxx +++ b/src/daemon/rpmostreed-transaction-types.cxx @@ -76,7 +76,15 @@ change_origin_refspec (GVariantDict *options, OstreeSysroot *sysroot, RpmOstreeO { case rpmostreecxx::RefspecType::Container: { - rpmostree_origin_set_rebase (origin, refspec); + // only pay attention to the custom origin stuff if using a pinned digest + if (!rpmostreecxx::is_container_image_digest_reference (refspec)) + { + custom_origin_url = NULL; + custom_origin_description = NULL; + } + + rpmostree_origin_set_rebase_custom (origin, refspec, custom_origin_url, + custom_origin_description); if (current_refspec.kind == rpmostreecxx::RefspecType::Container && strcmp (current_refspec.refspec.c_str (), refspec) == 0) From ae91092c3af9fd3e93a2519d5e99345cadfea653 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Fri, 4 Oct 2024 17:16:17 -0400 Subject: [PATCH 4/6] daemon/deployment-utils: Always add custom origin to deployment variant The rebase code already filters out the custom origin bits in cases where it doesn't make sense, so there's no need to additionally filter down here. This makes it work now in both the OSTree checksum case and the container digested pullspec case. --- src/daemon/rpmostreed-deployment-utils.cxx | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/daemon/rpmostreed-deployment-utils.cxx b/src/daemon/rpmostreed-deployment-utils.cxx index 2a987484bb..a31ea4f901 100644 --- a/src/daemon/rpmostreed-deployment-utils.cxx +++ b/src/daemon/rpmostreed-deployment-utils.cxx @@ -237,11 +237,6 @@ rpmostreed_deployment_generate_variant (OstreeSysroot *sysroot, OstreeDeployment case rpmostreecxx::RefspecType::Checksum: { g_variant_dict_insert (dict, "origin", "s", refspec); - auto custom_origin_url = rpmostree_origin_get_custom_url (origin); - auto custom_origin_description = rpmostree_origin_get_custom_description (origin); - if (!custom_origin_url.empty ()) - g_variant_dict_insert (dict, "custom-origin", "(ss)", custom_origin_url.c_str (), - custom_origin_description.c_str ()); } break; case rpmostreecxx::RefspecType::Ostree: @@ -269,6 +264,12 @@ rpmostreed_deployment_generate_variant (OstreeSysroot *sysroot, OstreeDeployment break; } + auto custom_origin_url = rpmostree_origin_get_custom_url (origin); + auto custom_origin_description = rpmostree_origin_get_custom_description (origin); + if (!custom_origin_url.empty ()) + g_variant_dict_insert (dict, "custom-origin", "(ss)", custom_origin_url.c_str (), + custom_origin_description.c_str ()); + g_variant_dict_insert (dict, "packages", "^as", layered_pkgs); g_variant_dict_insert_value (dict, "base-removals", removed_base_pkgs); g_variant_dict_insert_value (dict, "base-local-replacements", replaced_base_local_pkgs); From 8009fb185e6c343c765a24e9aa4a86fadfa4db58 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Fri, 4 Oct 2024 17:17:49 -0400 Subject: [PATCH 5/6] app/status: Print custom origin for digested pullspecs as well Now that we support this, let's make sure we render it as well. --- src/app/rpmostree-builtin-status.cxx | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/src/app/rpmostree-builtin-status.cxx b/src/app/rpmostree-builtin-status.cxx index 120c917e6a..3016ca5da1 100644 --- a/src/app/rpmostree-builtin-status.cxx +++ b/src/app/rpmostree-builtin-status.cxx @@ -659,6 +659,12 @@ print_one_deployment (RPMOSTreeSysroot *sysroot_proxy, GVariant *child, gint ind const char *custom_origin_url = NULL; const char *custom_origin_description = NULL; + g_variant_dict_lookup (dict, "custom-origin", "(&s&s)", &custom_origin_url, + &custom_origin_description); + /* Canonicalize the empty string to NULL */ + if (custom_origin_url && !*custom_origin_url) + custom_origin_url = NULL; + const char *container_image_reference_digest = NULL; if (origin_refspec) { @@ -667,11 +673,6 @@ print_one_deployment (RPMOSTreeSysroot *sysroot_proxy, GVariant *child, gint ind { case rpmostreecxx::RefspecType::Checksum: { - g_variant_dict_lookup (dict, "custom-origin", "(&s&s)", &custom_origin_url, - &custom_origin_description); - /* Canonicalize the empty string to NULL */ - if (custom_origin_url && !*custom_origin_url) - custom_origin_url = NULL; if (custom_origin_url) { g_assert (custom_origin_description && *custom_origin_description); @@ -688,8 +689,14 @@ print_one_deployment (RPMOSTreeSysroot *sysroot_proxy, GVariant *child, gint ind break; case rpmostreecxx::RefspecType::Container: { - if (g_variant_dict_lookup (dict, "container-image-reference-digest", "s", - &container_image_reference_digest)) + if (rpmostreecxx::is_container_image_digest_reference (origin_refspec) + && custom_origin_url) + { + g_assert (custom_origin_description && *custom_origin_description); + g_print ("%s", custom_origin_url); + } + else if (g_variant_dict_lookup (dict, "container-image-reference-digest", "s", + &container_image_reference_digest)) { g_print ("%s", origin_refspec); } From a65b3af36dd7d3b5085012fb5b6ba5dbf792eee8 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Fri, 4 Oct 2024 21:05:39 -0400 Subject: [PATCH 6/6] tests: Check custom origin with digest pullspec Verify that one can use a custom origin with a digest pullspec. While we're here, use testing-devel instead of stable since it's going to be much closer to what we're running on. --- tests/kolainst/destructive/container-rebase-upgrade | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/tests/kolainst/destructive/container-rebase-upgrade b/tests/kolainst/destructive/container-rebase-upgrade index 487e25b477..aee8332ebc 100755 --- a/tests/kolainst/destructive/container-rebase-upgrade +++ b/tests/kolainst/destructive/container-rebase-upgrade @@ -29,15 +29,20 @@ set -x libtest_prepare_offline cd "$(mktemp -d)" -image=quay.io/fedora/fedora-coreos:stable -image_pull=ostree-remote-registry:fedora:$image +image=quay.io/fedora/fedora-coreos +image_tag=testing-devel +digest=$(skopeo inspect -n docker://$image:$image_tag | jq -r .Digest) +image_pull=ostree-remote-registry:fedora:$image@$digest systemctl mask --now zincati # Test for https://github.com/ostreedev/ostree/issues/3228 rpm-ostree kargs --append "foo=\"a b c\"" rpm-ostree kargs > kargs.txt assert_file_has_content_literal kargs.txt "foo=\"a b c\"" -rpm-ostree rebase --experimental ${image_pull} +# also test custom origin stuff +rpm-ostree rebase "${image_pull}" --custom-origin-description "Fedora CoreOS $image_tag stream" --custom-origin-url "$image:$image_tag" +rpm-ostree status > status.txt +assert_file_has_content_literal status.txt "$image:$image_tag" "Fedora CoreOS $image_tag stream" rpm-ostree upgrade # This provokes # https://github.com/coreos/rpm-ostree/issues/4107