Merge pull request #5120 from jlebon/pr/custom-origin-containers

rpmostree-cxxrs.cxx

@@ -2167,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;
 
@@ -3884,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
 {

rpmostree-cxxrs.h

@@ -1842,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);

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::<OciReference>()
+                .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(())
     }

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<()>;

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);
               }

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);

src/daemon/rpmostreed-transaction-types.cxx

@@ -61,11 +61,30 @@ 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:
       {
-        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)
@@ -105,16 +124,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);
     }

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