Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow rebase to OSTree commits in container images #2940

Merged
merged 2 commits into from
Jul 16, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion rust/rpmostree-client/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,8 @@ pub struct Deployment {
pub staged: Option<bool>,
pub booted: bool,
pub serial: u32,
pub origin: String,
pub origin: Option<String>,
pub container_image_reference: Option<String>,
pub version: Option<String>,
}

Expand Down
40 changes: 40 additions & 0 deletions rust/src/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,16 @@ pub(crate) fn run_depmod(rootfs_dfd: i32, kver: &str, unified_core: bool) -> Cxx
Ok(())
}

/// Infer whether refspec is a ctonainer image reference.
pub(crate) fn is_container_image_reference(refspec: &str) -> bool {
// Currently, we are simply relying on the fact that there cannot be multiple colons
// or the `@` symbol in TYPE_OSTREE or TYPE_COMMIT refspecs. We may want a more robust
// and reliable way of determining the refspec type in the future, as some container
// transports may possibly not contain colons.
// https://github.com/coreos/rpm-ostree/issues/2909#issuecomment-868151689
refspec.split(':').nth(2).is_some() || refspec.contains('@')
}

/// Perform reversible filesystem transformations necessary before we execute scripts.
pub(crate) struct FilesystemScriptPrep {
rootfs: openat::Dir,
Expand Down Expand Up @@ -174,4 +184,34 @@ mod test {
}
Ok(())
}

#[test]
fn test_is_container_image_reference() -> Result<()> {
use super::is_container_image_reference;

let refspec_type_checksum =
"ee10f8e7ef638d78ba9a9596665067f58021624118875cc4079568da6c63efb0";
assert!(!is_container_image_reference(refspec_type_checksum));

let refspec_type_ostree_with_remote = "fedora:fedora/x86_64/coreos/testing-devel";
assert!(!is_container_image_reference(
refspec_type_ostree_with_remote
));
let refspec_type_ostree = "fedora/x86_64/coreos/foo-branch";
assert!(!is_container_image_reference(refspec_type_ostree));

const REFSPEC_TYPE_CONTAINER: &[&str] = &[
"containers-storage:localhost/fcos:latest",
"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 {
assert!(is_container_image_reference(refspec));
}

Ok(())
}
}
22 changes: 22 additions & 0 deletions rust/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,24 @@ pub mod ffi {
fn cliwrap_destdir() -> String;
}

/// `ContainerImport` is currently identical to ostree-rs-ext's `Import` struct, because
/// cxx.rs currently requires types used as extern Rust types to be defined by the same crate
/// that contains the bridge using them, so we redefine an `ContainerImport` struct here.
pub(crate) struct ContainerImport {
pub ostree_commit: String,
pub image_digest: String,
}

// sysroot_upgrade.rs
extern "Rust" {
fn import_container(
sysroot: Pin<&mut OstreeSysroot>,
imgref: String,
) -> Result<Box<ContainerImport>>;

fn fetch_digest(imgref: String) -> Result<String>;
}

// core.rs
extern "Rust" {
type TempEtcGuard;
Expand All @@ -140,6 +158,8 @@ pub mod ffi {
fn undo(self: &FilesystemScriptPrep) -> Result<()>;

fn run_depmod(rootfs_dfd: i32, kver: &str, unified_core: bool) -> Result<()>;

fn is_container_image_reference(refspec: &str) -> bool;
}

// composepost.rs
Expand Down Expand Up @@ -600,6 +620,8 @@ pub(crate) use self::console_progress::*;
mod progress;
mod scripts;
pub(crate) use self::scripts::*;
mod sysroot_upgrade;
pub(crate) use crate::sysroot_upgrade::*;
mod rpmutils;
pub(crate) use self::rpmutils::*;
mod testutils;
Expand Down
9 changes: 6 additions & 3 deletions rust/src/origin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,14 @@ fn origin_to_treefile_inner(kf: &KeyFile) -> Result<Box<Treefile>> {
let mut cfg: crate::treefile::TreeComposeConfig = Default::default();
let refspec_str = if let Some(r) = keyfile_get_optional_string(&kf, ORIGIN, "refspec")? {
Some(r)
} else if let Some(r) = keyfile_get_optional_string(&kf, ORIGIN, "baserefspec")? {
Some(r)
} else {
keyfile_get_optional_string(&kf, ORIGIN, "baserefspec")?
keyfile_get_optional_string(&kf, ORIGIN, "container-image-reference")?
};
let refspec_str = refspec_str
.ok_or_else(|| anyhow::anyhow!("Failed to find refspec/baserefspec in origin"))?;
let refspec_str = refspec_str.ok_or_else(|| {
anyhow::anyhow!("Failed to find refspec/baserefspec/container-image-reference in origin")
})?;
cfg.derive.base_refspec = Some(refspec_str);
cfg.packages = parse_stringlist(&kf, PACKAGES, "requested")?;
cfg.derive.packages_local = parse_localpkglist(&kf, PACKAGES, "requested-local")?;
Expand Down
41 changes: 41 additions & 0 deletions rust/src/sysroot_upgrade.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
//! Rust portion of `rpmostree-sysroot-upgrader.cxx`.

// SPDX-License-Identifier: Apache-2.0 OR MIT

use crate::cxxrsutil::*;
use crate::ffi::ContainerImport;
use anyhow::{Context, Result};
use std::convert::TryInto;
use std::pin::Pin;

/// Import ostree commit in container image using ostree-rs-ext's API.
pub(crate) fn import_container(
mut sysroot: Pin<&mut crate::FFIOstreeSysroot>,
imgref: String,
) -> CxxResult<Box<ContainerImport>> {
// TODO: take a GCancellable and monitor it, and drop the import task (which is how async cancellation works in Rust).
let sysroot = &sysroot.gobj_wrap();
let repo = &sysroot.get_repo(gio::NONE_CANCELLABLE)?;
let imgref = imgref.as_str().try_into()?;
let imported = build_runtime()?
.block_on(async { ostree_ext::container::import(&repo, &imgref, None).await })?;
Ok(Box::new(ContainerImport {
ostree_commit: imported.ostree_commit,
image_digest: imported.image_digest,
}))
}

/// Fetch the image digest for `imgref` using ostree-rs-ext's API.
pub(crate) fn fetch_digest(imgref: String) -> CxxResult<String> {
let imgref = imgref.as_str().try_into()?;
let digest = build_runtime()?
.block_on(async { ostree_ext::container::fetch_manifest_info(&imgref).await })?;
Ok(digest.manifest_digest)
}

fn build_runtime() -> Result<tokio::runtime::Runtime> {
tokio::runtime::Builder::new_multi_thread()
.enable_all()
.build()
.context("Failed to build tokio runtime")
}
10 changes: 10 additions & 0 deletions src/app/rpmostree-builtin-rebase.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,16 @@ rpmostree_builtin_rebase (int argc,
if (strlen (new_provided_refspec) == 0)
return glnx_throw (error, "Refspec is empty");

if (refspectype == RPMOSTREE_REFSPEC_TYPE_CONTAINER)
{
if (!opt_experimental)
return glnx_throw (error, "Rebasing to a container image reference requires --experimental");
/* When using the container refspec type, if rebasing to a specific commit, we expect a
* specific digest tag in the refspec, not in a separate argument */
if (revision)
return glnx_throw (error, "Unexpected ostree revision alongside container refspec type");
}

/* Check if remote refers to a local repo */
g_autofree char *local_repo_remote = NULL;
if (G_IN_SET (refspectype, RPMOSTREE_REFSPEC_TYPE_OSTREE,
Expand Down
10 changes: 8 additions & 2 deletions src/app/rpmostree-builtin-status.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -559,14 +559,16 @@ print_one_deployment (RPMOSTreeSysroot *sysroot_proxy,
return TRUE;

const gchar *origin_refspec;
RpmOstreeRefspecType refspectype = RPMOSTREE_REFSPEC_TYPE_OSTREE;
g_autofree const gchar **origin_packages = NULL;
g_autofree const gchar **origin_requested_packages = NULL;
g_autofree const gchar **origin_requested_local_packages = NULL;
g_autoptr(GVariant) origin_base_removals = NULL;
g_autofree const gchar **origin_requested_base_removals = NULL;
g_autoptr(GVariant) origin_base_local_replacements = NULL;
g_autofree const gchar **origin_requested_base_local_replacements = NULL;
if (g_variant_dict_lookup (dict, "origin", "&s", &origin_refspec))
if (g_variant_dict_lookup (dict, "origin", "&s", &origin_refspec) ||
g_variant_dict_lookup (dict, "container-image-reference", "&s", &origin_refspec))
{
origin_packages =
lookup_array_and_canonicalize (dict, "packages");
Expand Down Expand Up @@ -606,7 +608,6 @@ print_one_deployment (RPMOSTreeSysroot *sysroot_proxy,

g_print ("%s ", is_booted ? libsd_special_glyph (BLACK_CIRCLE) : " ");

RpmOstreeRefspecType refspectype = RPMOSTREE_REFSPEC_TYPE_OSTREE;
const char *custom_origin_url = NULL;
const char *custom_origin_description = NULL;
if (origin_refspec)
Expand Down Expand Up @@ -637,6 +638,11 @@ print_one_deployment (RPMOSTreeSysroot *sysroot_proxy,
g_print ("%s", origin_refspec);
}
break;
case RPMOSTREE_REFSPEC_TYPE_CONTAINER:
{
g_print ("%s", origin_refspec);
}
break;
}
}
else
Expand Down
27 changes: 27 additions & 0 deletions src/daemon/rpmostree-sysroot-upgrader.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -443,6 +443,33 @@ rpmostree_sysroot_upgrader_pull_base (RpmOstreeSysrootUpgrader *self,

switch (refspec_type)
{
case RPMOSTREE_REFSPEC_TYPE_CONTAINER:
{
if (override_commit)
return glnx_throw (error, "Specifying commit overrides for container-image-reference type refspecs is not supported");

if (strstr (refspec, "@"))
return glnx_throw (error, "Pinned to specific container image digest, cannot upgrade");

/* Check to see if the digest that `refspec` points to is the same before pulling a new container image. */
const char *cur_digest = rpmostree_origin_get_container_image_reference_digest (self->original_origin);
if (cur_digest)
{
auto new_digest = rpmostreecxx::fetch_digest(std::string(refspec));
if (strcmp (new_digest.c_str(), cur_digest) == 0)
{
/* No new digest. */
*out_changed = FALSE;
return TRUE;
}
}

auto import = rpmostreecxx::import_container(*self->sysroot, std::string(refspec));

rpmostree_origin_set_container_image_reference_digest (self->original_origin, import->image_digest.c_str());
new_base_rev = strdup (import->ostree_commit.c_str());
break;
}
case RPMOSTREE_REFSPEC_TYPE_CHECKSUM:
case RPMOSTREE_REFSPEC_TYPE_OSTREE:
{
Expand Down
9 changes: 6 additions & 3 deletions src/daemon/rpmostreed-deployment-utils.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,7 @@ rpmostreed_deployment_generate_variant (OstreeSysroot *sysroot,

RpmOstreeRefspecType refspec_type;
g_autofree char *refspec = rpmostree_origin_get_full_refspec (origin, &refspec_type);
g_assert (refspec);

gboolean is_layered = FALSE;
g_autofree char *base_checksum = NULL;
Expand Down Expand Up @@ -301,8 +302,12 @@ rpmostreed_deployment_generate_variant (OstreeSysroot *sysroot,

switch (refspec_type)
{
case RPMOSTREE_REFSPEC_TYPE_CONTAINER:
g_variant_dict_insert (dict, "container-image-reference", "s", refspec);
break;
case RPMOSTREE_REFSPEC_TYPE_CHECKSUM:
{
g_variant_dict_insert (dict, "origin", "s", refspec);
g_autofree char *custom_origin_url = NULL;
g_autofree char *custom_origin_description = NULL;
rpmostree_origin_get_custom_description (origin, &custom_origin_url,
Expand All @@ -315,6 +320,7 @@ rpmostreed_deployment_generate_variant (OstreeSysroot *sysroot,
break;
case RPMOSTREE_REFSPEC_TYPE_OSTREE:
{
g_variant_dict_insert (dict, "origin", "s", refspec);
if (!variant_add_remote_status (repo, refspec, base_checksum, dict, error))
return NULL;

Expand All @@ -339,9 +345,6 @@ rpmostreed_deployment_generate_variant (OstreeSysroot *sysroot,
break;
}

if (refspec)
g_variant_dict_insert (dict, "origin", "s", refspec);

variant_add_from_hash_table (dict, "requested-packages",
rpmostree_origin_get_packages (origin));
variant_add_from_hash_table (dict, "requested-local-packages",
Expand Down
39 changes: 38 additions & 1 deletion src/daemon/rpmostreed-transaction-types.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,37 @@ change_origin_refspec (GVariantDict *options,
gchar **out_new_refspec,
GError **error)
{
RpmOstreeRefspecType refspectype;
if (!rpmostree_refspec_classify (refspec, &refspectype, error))
return FALSE;

RpmOstreeRefspecType current_refspectype;
g_autofree gchar *current_refspec = rpmostree_origin_get_full_refspec (origin, &current_refspectype);


switch (refspectype)
{
case RPMOSTREE_REFSPEC_TYPE_CONTAINER:
{
if (!rpmostree_origin_set_rebase (origin, refspec, error))
return FALSE;

if (current_refspectype == RPMOSTREE_REFSPEC_TYPE_CONTAINER
&& strcmp (current_refspec, refspec) == 0)
return glnx_throw (error, "Old and new refs are equal: %s", refspec);

if (out_old_refspec != NULL)
*out_old_refspec = util::move_nullify (current_refspec);
if (out_new_refspec != NULL)
*out_new_refspec = g_strdup (refspec);
return TRUE;
}
case RPMOSTREE_REFSPEC_TYPE_OSTREE:
break;
case RPMOSTREE_REFSPEC_TYPE_CHECKSUM:
break;
}

/* The rest of the code assumes TYPE_OSTREE refspec */
g_autofree gchar *new_refspec = NULL;
if (!rpmostreed_refspec_parse_partial (refspec,
current_refspec,
Expand Down Expand Up @@ -151,6 +179,11 @@ apply_revision_override (RpmostreedTransaction *transaction,
if (refspectype == RPMOSTREE_REFSPEC_TYPE_CHECKSUM)
return glnx_throw (error, "Cannot look up version while pinned to commit");

if (refspectype == RPMOSTREE_REFSPEC_TYPE_CONTAINER)
/* NB: Not supported for now, but We can perhaps support this if we allow `revision` to
* possibly be a tag or digest */
return glnx_throw (error, "Cannot look up version while tracking a container image reference");

g_autofree char *checksum = NULL;
g_autofree char *version = NULL;
if (!rpmostreed_parse_revision (revision, &checksum, &version, error))
Expand All @@ -160,6 +193,8 @@ apply_revision_override (RpmostreedTransaction *transaction,
{
switch (refspectype)
{
case RPMOSTREE_REFSPEC_TYPE_CONTAINER:
g_assert_not_reached (); /* Handled above */
case RPMOSTREE_REFSPEC_TYPE_OSTREE:
{
/* Perhaps down the line we'll drive history traversal into libostree */
Expand All @@ -183,6 +218,8 @@ apply_revision_override (RpmostreedTransaction *transaction,

switch (refspectype)
{
case RPMOSTREE_REFSPEC_TYPE_CONTAINER:
g_assert_not_reached (); /* Handled above */
case RPMOSTREE_REFSPEC_TYPE_OSTREE:
if (!skip_branch_check)
{
Expand Down
6 changes: 6 additions & 0 deletions src/libpriv/rpmostree-core.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,12 @@ rpmostree_refspec_classify (const char *refspec,
RpmOstreeRefspecType *out_type,
GError **error)
{
if (rpmostreecxx::is_container_image_reference(refspec))
{
*out_type = RPMOSTREE_REFSPEC_TYPE_CONTAINER;
return TRUE;
}

/* Fall back to TYPE_OSTREE if we cannot infer type */
*out_type = RPMOSTREE_REFSPEC_TYPE_OSTREE;
if (ostree_validate_checksum_string (refspec, NULL))
Expand Down
5 changes: 5 additions & 0 deletions src/libpriv/rpmostree-core.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,13 @@ G_DECLARE_FINAL_TYPE (RpmOstreeContext, rpmostree_context, RPMOSTREE, CONTEXT, G
typedef enum {
RPMOSTREE_REFSPEC_TYPE_OSTREE,
RPMOSTREE_REFSPEC_TYPE_CHECKSUM,
RPMOSTREE_REFSPEC_TYPE_CONTAINER,
} RpmOstreeRefspecType;

#define RPMOSTREE_REFSPEC_OSTREE_ORIGIN_KEY "refspec"
#define RPMOSTREE_REFSPEC_OSTREE_BASE_ORIGIN_KEY "baserefspec"
#define RPMOSTREE_REFSPEC_CONTAINER_ORIGIN_KEY "container-image-reference"

gboolean rpmostree_refspec_classify (const char *refspec,
RpmOstreeRefspecType *out_type,
GError **error);
Expand Down
Loading