From 213b8608ea579b6cd0fb4140e9689fc814f36c26 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 10 Sep 2021 17:06:56 -0400 Subject: [PATCH 01/11] tests/pull-test: Avoid duplicating test numbers We do this in other places; avoids touching two numbers when adding tests. Let computers do the addition. --- tests/pull-test.sh | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/pull-test.sh b/tests/pull-test.sh index fcc22812..7d363f47 100644 --- a/tests/pull-test.sh +++ b/tests/pull-test.sh @@ -54,11 +54,12 @@ function verify_initial_contents() { assert_file_has_content baz/cow '^moo$' } +n_base_tests=35 +gpg_tests=3 if has_gpgme; then - echo "1..38" + echo "1..$(($n_base_tests+$gpg_tests))" else - # 3 tests needs GPG support - echo "1..35" + echo "1..$((n_base_tests))" fi # Try both syntaxes From bc30806c6edc005de9fe32dd645ed9a13469796f Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 10 Sep 2021 17:07:42 -0400 Subject: [PATCH 02/11] tests: Add new TAP APIs Having to touch a global test counter when adding tests is a recipe for conflicts between PRs. The TAP protocol allows *ending* with the expected number of tests, so the best way to do this is to have an explicit API like our `tap_ok` which bumps a counter, then end with `tap_end`. I ported one test as a demo. --- tests/kolainst/libtest-core.sh | 11 +++++++++++ tests/test-admin-deploy-etcmerge-cornercases.sh | 8 ++++---- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/tests/kolainst/libtest-core.sh b/tests/kolainst/libtest-core.sh index 9632e905..75b9063a 100644 --- a/tests/kolainst/libtest-core.sh +++ b/tests/kolainst/libtest-core.sh @@ -35,6 +35,17 @@ assert_not_reached () { fatal "$@" } +# Output an ok message for TAP +n_tap_tests=0 +tap_ok() { + echo "ok" "$@" + n_tap_tests=$(($n_tap_tests+1)) +} + +tap_end() { + echo "1..${n_tap_tests}" +} + # Some tests look for specific English strings. Use a UTF-8 version # of the C (POSIX) locale if we have one, or fall back to en_US.UTF-8 # (https://sourceware.org/glibc/wiki/Proposals/C.UTF-8) diff --git a/tests/test-admin-deploy-etcmerge-cornercases.sh b/tests/test-admin-deploy-etcmerge-cornercases.sh index 4f55bc3a..98e2bbe6 100755 --- a/tests/test-admin-deploy-etcmerge-cornercases.sh +++ b/tests/test-admin-deploy-etcmerge-cornercases.sh @@ -26,8 +26,6 @@ set -euo pipefail # Exports OSTREE_SYSROOT so --sysroot not needed. setup_os_repository "archive" "syslinux" -echo "1..2" - ${CMD_PREFIX} ostree --repo=sysroot/ostree/repo pull-local --remote=testos testos-repo testos/buildmain/x86_64-runtime rev=$(${CMD_PREFIX} ostree --repo=sysroot/ostree/repo rev-parse testos/buildmain/x86_64-runtime) export rev @@ -89,7 +87,7 @@ test -L ${newetc}/a/link-to-no-such-file || assert_not_reached "should have syml assert_has_dir ${newroot}/usr/etc/testdirectory assert_not_has_dir ${newetc}/testdirectory -echo "ok" +tap_ok first # Add /etc/initially-empty cd "${test_tmpdir}/osdata" @@ -141,4 +139,6 @@ assert_not_has_file sysroot/ostree/deploy/testos/deploy/${rev}.0/usr/etc/initial assert_has_file sysroot/ostree/deploy/testos/deploy/${rev}.0/etc/initially-empty/mynewfile rm ${newconfpath} -echo "ok" +tap_ok second + +tap_end From ab12e380fc51487672d07ddf47295ee182e62d36 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 29 Sep 2021 09:03:24 -0400 Subject: [PATCH 03/11] bin/commit: Fix --tree=tar with --selinux-policy The logic for `--selinux-policy` ended up in the `--tree=dir` path, but there's no reason for that. Fix the imported labeling with `--tree=tar`. Prep for use with containers. We had this bug because the previous logic was trying to avoid duplicating the code for generic `--selinux-policy` and the case of `--selinux-policy-from-base --tree=dir`. It's a bit more code, but it's cleaner if we dis-entangle them. --- src/ostree/ot-builtin-commit.c | 19 ++++++++++++------- .../destructive/itest-label-selinux.sh | 13 +++++++++++++ 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/src/ostree/ot-builtin-commit.c b/src/ostree/ot-builtin-commit.c index 370e085c..b993678e 100644 --- a/src/ostree/ot-builtin-commit.c +++ b/src/ostree/ot-builtin-commit.c @@ -602,6 +602,17 @@ ostree_builtin_commit (int argc, char **argv, OstreeCommandInvocation *invocatio filter_data.skip_list = skip_list; modifier = ostree_repo_commit_modifier_new (flags, commit_filter, &filter_data, NULL); + + if (opt_selinux_policy) + { + glnx_autofd int rootfs_dfd = -1; + if (!glnx_opendirat (AT_FDCWD, opt_selinux_policy, TRUE, &rootfs_dfd, error)) + goto out; + policy = ostree_sepolicy_new_at (rootfs_dfd, cancellable, error); + if (!policy) + goto out; + ostree_repo_commit_modifier_set_sepolicy (modifier, policy); + } } if (opt_editor) @@ -691,14 +702,8 @@ ostree_builtin_commit (int argc, char **argv, OstreeCommandInvocation *invocatio { if (first && opt_selinux_policy_from_base) { - opt_selinux_policy = g_strdup (tree); - opt_selinux_policy_from_base = FALSE; - } - if (first && opt_selinux_policy) - { - g_assert (modifier); glnx_autofd int rootfs_dfd = -1; - if (!glnx_opendirat (AT_FDCWD, opt_selinux_policy, TRUE, &rootfs_dfd, error)) + if (!glnx_opendirat (AT_FDCWD, tree, TRUE, &rootfs_dfd, error)) goto out; policy = ostree_sepolicy_new_at (rootfs_dfd, cancellable, error); if (!policy) diff --git a/tests/kolainst/destructive/itest-label-selinux.sh b/tests/kolainst/destructive/itest-label-selinux.sh index d7337124..97b5cc54 100755 --- a/tests/kolainst/destructive/itest-label-selinux.sh +++ b/tests/kolainst/destructive/itest-label-selinux.sh @@ -104,3 +104,16 @@ assert_file_has_content newls.txt ':lib_t:' ostree ls -X newbase /usr/etc/some.conf > newls.txt assert_file_has_content newls.txt ':etc_t:' echo "ok commit --selinux-policy-from-base" + +rm rootfs -rf +mkdir rootfs +mkdir -p rootfs/usr/{bin,lib,etc} +echo 'somebinary' > rootfs/usr/bin/somebinary +ls -Z rootfs/usr/bin/somebinary > lsz.txt +assert_not_file_has_content lsz.txt ':bin_t:' +rm -f lsz.txt +tar -C rootfs -cf rootfs.tar . +ostree commit -b newbase --selinux-policy / --tree=tar=rootfs.tar +ostree ls -X newbase /usr/bin/somebinary > newls.txt +assert_file_has_content newls.txt ':bin_t:' +echo "ok commit --selinux-policy with --tree=tar" From bcc0ef7583508a857c6ed32a2cec3e20c2a2bd23 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 30 Sep 2021 13:38:25 -0400 Subject: [PATCH 04/11] tests: Use ostree-ext 0.3.0 This updates to the modern glib 0.14 and paves the way for some reverse dependency testing by using ostree-ext's code. --- tests/inst/Cargo.toml | 4 +--- tests/inst/src/destructive.rs | 12 +++++++----- tests/inst/src/sysroot.rs | 8 ++++---- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/tests/inst/Cargo.toml b/tests/inst/Cargo.toml index bac4e42d..146744b8 100644 --- a/tests/inst/Cargo.toml +++ b/tests/inst/Cargo.toml @@ -17,9 +17,7 @@ serde_json = "1.0" sh-inline = "0.1.0" anyhow = "1.0" tempfile = "3.1.0" -glib = "0.10" -gio = "0.9" -ostree = { version = "0.10.0", features = ["v2021_1"] } +ostree-ext = { version = "0.3.0" } libtest-mimic = "0.3.0" twoway = "0.2.1" hyper = { version = "0.14", features = ["runtime", "http1", "http2", "tcp", "server"] } diff --git a/tests/inst/src/destructive.rs b/tests/inst/src/destructive.rs index 5f4fb790..2e2bd374 100644 --- a/tests/inst/src/destructive.rs +++ b/tests/inst/src/destructive.rs @@ -20,6 +20,8 @@ //! AUTOPKGTEST_REBOOT_MARK. use anyhow::{Context, Result}; +use ostree_ext::gio; +use ostree_ext::ostree; use rand::seq::SliceRandom; use rand::Rng; use serde::{Deserialize, Serialize}; @@ -283,7 +285,7 @@ fn parse_and_validate_reboot_mark>( // Since we successfully updated, generate a new commit to target generate_update(&firstdeploy.checksum)?; // Update the target state - let srvrepo_obj = ostree::Repo::new(&gio::File::new_for_path(SRVREPO)); + let srvrepo_obj = ostree::Repo::new(&gio::File::for_path(SRVREPO)); srvrepo_obj.open(gio::NONE_CANCELLABLE)?; commitstates.target = srvrepo_obj.resolve_rev(TESTREF, false)?.unwrap().into(); } else if commitstates.booted == commitstates.orig || commitstates.booted == commitstates.prev { @@ -352,9 +354,9 @@ fn impl_transaction_test>( // Gather the expected possible commits let mut commitstates = { - let srvrepo_obj = ostree::Repo::new(&gio::File::new_for_path(SRVREPO)); + let srvrepo_obj = ostree::Repo::new(&gio::File::for_path(SRVREPO)); srvrepo_obj.open(gio::NONE_CANCELLABLE)?; - let sysrepo_obj = ostree::Repo::new(&gio::File::new_for_path("/sysroot/ostree/repo")); + let sysrepo_obj = ostree::Repo::new(&gio::File::for_path("/sysroot/ostree/repo")); sysrepo_obj.open(gio::NONE_CANCELLABLE)?; CommitStates { @@ -543,8 +545,8 @@ fn transactionality() -> Result<()> { let sysroot = ostree::Sysroot::new_default(); sysroot.load(cancellable.as_ref())?; assert!(sysroot.is_booted()); - let booted = sysroot.get_booted_deployment().expect("booted deployment"); - let commit: String = booted.get_csum().expect("booted csum").into(); + let booted = sysroot.booted_deployment().expect("booted deployment"); + let commit: String = booted.csum().expect("booted csum").into(); // We need this static across reboots let srvrepo = Path::new(SRVREPO); let firstrun = !srvrepo.exists(); diff --git a/tests/inst/src/sysroot.rs b/tests/inst/src/sysroot.rs index ce0378eb..301ef8b3 100644 --- a/tests/inst/src/sysroot.rs +++ b/tests/inst/src/sysroot.rs @@ -1,8 +1,8 @@ //! Tests that mostly use the API and access the booted sysroot read-only. use anyhow::Result; -use gio::prelude::*; -use ostree::prelude::*; +use ostree_ext::prelude::*; +use ostree_ext::{gio, ostree}; use crate::test::*; @@ -21,11 +21,11 @@ fn test_sysroot_ro() -> Result<()> { sysroot.load(cancellable.as_ref())?; assert!(sysroot.is_booted()); - let booted = sysroot.get_booted_deployment().expect("booted deployment"); + let booted = sysroot.booted_deployment().expect("booted deployment"); assert!(!booted.is_staged()); let repo = sysroot.repo().expect("repo"); - let csum = booted.get_csum().expect("booted csum"); + let csum = booted.csum().expect("booted csum"); let csum = csum.as_str(); let (root, rev) = repo.read_commit(csum, cancellable.as_ref())?; From 1ed290c7d93e24fd3986dd3c355b0d323dcdcd99 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 30 Sep 2021 14:09:22 -0400 Subject: [PATCH 05/11] fsck: Print a success message There's a general Unix philosophy that "silence is golden". However, when one is explicitly invoking an error check it's nice to see explicit success. We already print various statistics, so ending with a happy note has no extra cost. --- src/ostree/ot-builtin-fsck.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/ostree/ot-builtin-fsck.c b/src/ostree/ot-builtin-fsck.c index dea03af4..f7a72601 100644 --- a/src/ostree/ot-builtin-fsck.c +++ b/src/ostree/ot-builtin-fsck.c @@ -459,5 +459,8 @@ ostree_builtin_fsck (int argc, char **argv, OstreeCommandInvocation *invocation, if (n_fsck_partial > 0) return glnx_throw (error, "%u partial commits from fsck-detected corruption", n_partial); + g_print ("object fsck of %d commits completed successfully - no errors found.\n", + (guint)g_hash_table_size (commits)); + return TRUE; } From 1b9e3a9375a2da70b6ee049c6ff0989c9831963b Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 28 Sep 2021 17:40:28 -0400 Subject: [PATCH 06/11] repo: Add an API to init `OstreeSePolicy` from commit directly This is part of `OstreeCommitModifier`, but I'm not using that in some of the ostree-ext Rust code. It just makes more sense as a direct policy API, where it should have been in the first place. There's already support for setting a policy object on a commit modifier, so that's all the old API needs to do now. --- Makefile-libostree.am | 6 ++-- apidoc/ostree-sections.txt | 1 + src/libostree/libostree-devel.sym | 5 +++ src/libostree/ostree-repo-commit.c | 33 ++---------------- src/libostree/ostree-sepolicy.c | 56 ++++++++++++++++++++++++++++++ src/libostree/ostree-sepolicy.h | 5 +++ 6 files changed, 72 insertions(+), 34 deletions(-) diff --git a/Makefile-libostree.am b/Makefile-libostree.am index dd396974..d40de48d 100644 --- a/Makefile-libostree.am +++ b/Makefile-libostree.am @@ -173,9 +173,9 @@ endif # USE_GPGME symbol_files = $(top_srcdir)/src/libostree/libostree-released.sym # Uncomment this include when adding new development symbols. -#if BUILDOPT_IS_DEVEL_BUILD -#symbol_files += $(top_srcdir)/src/libostree/libostree-devel.sym -#endif +if BUILDOPT_IS_DEVEL_BUILD +symbol_files += $(top_srcdir)/src/libostree/libostree-devel.sym +endif # http://blog.jgc.org/2007/06/escaping-comma-and-space-in-gnu-make.html wl_versionscript_arg = -Wl,--version-script= diff --git a/apidoc/ostree-sections.txt b/apidoc/ostree-sections.txt index f0901f21..ae8abe81 100644 --- a/apidoc/ostree-sections.txt +++ b/apidoc/ostree-sections.txt @@ -522,6 +522,7 @@ ostree_repo_file_get_type OstreeSePolicy ostree_sepolicy_new ostree_sepolicy_new_at +ostree_sepolicy_new_from_commit ostree_sepolicy_get_path ostree_sepolicy_get_name ostree_sepolicy_get_label diff --git a/src/libostree/libostree-devel.sym b/src/libostree/libostree-devel.sym index e3cd14a4..35d53956 100644 --- a/src/libostree/libostree-devel.sym +++ b/src/libostree/libostree-devel.sym @@ -22,6 +22,11 @@ - uncomment the include in Makefile-libostree.am */ +LIBOSTREE_2021.5 { +global: + ostree_sepolicy_new_from_commit; +} LIBOSTREE_2021.4; + /* Stub section for the stable release *after* this development one; don't * edit this other than to update the year. This is just a copy/paste * source. Replace $LASTSTABLE with the last stable version, and $NEWVERSION diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c index 8dc2355e..c87e8de8 100644 --- a/src/libostree/ostree-repo-commit.c +++ b/src/libostree/ostree-repo-commit.c @@ -4314,7 +4314,6 @@ ostree_repo_commit_modifier_unref (OstreeRepoCommitModifier *modifier) g_clear_pointer (&modifier->devino_cache, (GDestroyNotify)g_hash_table_unref); g_clear_object (&modifier->sepolicy); - (void) glnx_tmpdir_delete (&modifier->sepolicy_tmpdir, NULL, NULL); g_free (modifier); return; @@ -4386,38 +4385,10 @@ ostree_repo_commit_modifier_set_sepolicy_from_commit (OstreeRepoCommitModifier GCancellable *cancellable, GError **error) { - GLNX_AUTO_PREFIX_ERROR ("setting sepolicy from commit", error); - g_autofree char *commit = NULL; - g_autoptr(GFile) root = NULL; - if (!ostree_repo_read_commit (repo, rev, &root, &commit, cancellable, error)) - return FALSE; - const char policypath[] = "usr/etc/selinux"; - g_autoptr(GFile) policyroot = g_file_get_child (root, policypath); - if (!g_file_query_exists (policyroot, NULL)) - return TRUE; /* No policy, nothing to do */ - - GLnxTmpDir tmpdir = {0,}; - if (!glnx_mkdtemp ("ostree-commit-sepolicy-XXXXXX", 0700, &tmpdir, error)) - return FALSE; - if (!glnx_shutil_mkdir_p_at (tmpdir.fd, "usr/etc", 0755, cancellable, error)) - return FALSE; - - OstreeRepoCheckoutAtOptions coopts = {0,}; - coopts.mode = OSTREE_REPO_CHECKOUT_MODE_USER; - coopts.subpath = glnx_strjoina ("/", policypath); - - if (!ostree_repo_checkout_at (repo, &coopts, tmpdir.fd, policypath, commit, cancellable, error)) - return glnx_prefix_error (error, "policy checkout"); - - g_autoptr(OstreeSePolicy) policy = ostree_sepolicy_new_at (tmpdir.fd, cancellable, error); + g_autoptr(OstreeSePolicy) policy = ostree_sepolicy_new_from_commit (repo, rev, cancellable, error); if (!policy) - return glnx_prefix_error (error, "reading policy"); - + return FALSE; ostree_repo_commit_modifier_set_sepolicy (modifier, policy); - /* Transfer ownership */ - modifier->sepolicy_tmpdir = tmpdir; - tmpdir.initialized = FALSE; - return TRUE; } diff --git a/src/libostree/ostree-sepolicy.c b/src/libostree/ostree-sepolicy.c index e6b9a0e1..8aa1df96 100644 --- a/src/libostree/ostree-sepolicy.c +++ b/src/libostree/ostree-sepolicy.c @@ -29,6 +29,7 @@ #include "otutil.h" #include "ostree-sepolicy.h" +#include "ostree-repo.h" #include "ostree-sepolicy-private.h" #include "ostree-bootloader-uboot.h" #include "ostree-bootloader-syslinux.h" @@ -47,6 +48,7 @@ struct OstreeSePolicy { int rootfs_dfd; int rootfs_dfd_owned; GFile *path; + GLnxTmpDir tmpdir; #ifdef HAVE_SELINUX GFile *selinux_policy_root; @@ -77,6 +79,8 @@ ostree_sepolicy_finalize (GObject *object) { OstreeSePolicy *self = OSTREE_SEPOLICY (object); + (void) glnx_tmpdir_delete (&self->tmpdir, NULL, NULL); + g_clear_object (&self->path); if (self->rootfs_dfd_owned != -1) (void) close (self->rootfs_dfd_owned); @@ -266,6 +270,58 @@ get_policy_checksum (char **out_csum, #endif +/** + * ostree_sepolicy_new_from_commit: + * @repo: The repo + * @rev: ostree ref or checksum + * @cancellable: Cancellable + * @error: Error + * + * Extract the SELinux policy from a commit object via a partial checkout. This is useful + * for labeling derived content as separate commits. + * + * This function is the backend of `ostree_repo_commit_modifier_set_sepolicy_from_commit()`. + * + * Returns: (transfer full): A new policy + */ +OstreeSePolicy* +ostree_sepolicy_new_from_commit (OstreeRepo *repo, + const char *rev, + GCancellable *cancellable, + GError **error) +{ + GLNX_AUTO_PREFIX_ERROR ("setting sepolicy from commit", error); + g_autoptr(GFile) root = NULL; + g_autofree char *commit = NULL; + if (!ostree_repo_read_commit (repo, rev, &root, &commit, cancellable, error)) + return NULL; + const char policypath[] = "usr/etc/selinux"; + g_autoptr(GFile) policyroot = g_file_get_child (root, policypath); + + GLnxTmpDir tmpdir = {0,}; + if (!glnx_mkdtemp ("ostree-commit-sepolicy-XXXXXX", 0700, &tmpdir, error)) + return FALSE; + if (!glnx_shutil_mkdir_p_at (tmpdir.fd, "usr/etc", 0755, cancellable, error)) + return FALSE; + + if (g_file_query_exists (policyroot, NULL)) + { + OstreeRepoCheckoutAtOptions coopts = {0,}; + coopts.mode = OSTREE_REPO_CHECKOUT_MODE_USER; + coopts.subpath = glnx_strjoina ("/", policypath); + + if (!ostree_repo_checkout_at (repo, &coopts, tmpdir.fd, policypath, commit, cancellable, error)) + return glnx_prefix_error_null (error, "policy checkout"); + } + + OstreeSePolicy *ret = ostree_sepolicy_new_at (tmpdir.fd, cancellable, error); + if (!ret) + return NULL; + /* Transfer ownership of tmpdir */ + ret->tmpdir = tmpdir; + tmpdir.initialized = FALSE; + return ret; +} /* Workaround for http://marc.info/?l=selinux&m=149323809332417&w=2 */ #ifdef HAVE_SELINUX diff --git a/src/libostree/ostree-sepolicy.h b/src/libostree/ostree-sepolicy.h index 7e90527f..0e8cf5af 100644 --- a/src/libostree/ostree-sepolicy.h +++ b/src/libostree/ostree-sepolicy.h @@ -44,6 +44,11 @@ OstreeSePolicy* ostree_sepolicy_new_at (int rootfs_dfd, GCancellable *cancellable, GError **error); +_OSTREE_PUBLIC +OstreeSePolicy* ostree_sepolicy_new_from_commit (OstreeRepo *repo, + const char *rev, + GCancellable *cancellable, + GError **error); _OSTREE_PUBLIC GFile * ostree_sepolicy_get_path (OstreeSePolicy *self); From ddc0d54b784ca01a2d9a582fc9aa1f31dd57ed08 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 30 Sep 2021 11:38:10 -0400 Subject: [PATCH 07/11] sepolicy: Add deprecation comment for `_get_path()` Came up in review https://github.com/ostreedev/ostree/pull/2447#issuecomment-931428312 --- src/libostree/ostree-sepolicy.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/libostree/ostree-sepolicy.c b/src/libostree/ostree-sepolicy.c index 8aa1df96..9b2ce0ab 100644 --- a/src/libostree/ostree-sepolicy.c +++ b/src/libostree/ostree-sepolicy.c @@ -499,7 +499,11 @@ ostree_sepolicy_new_at (int rootfs_dfd, /** * ostree_sepolicy_get_path: - * @self: + * @self: A SePolicy object + * + * This API should be considered deprecated, because it's supported for + * policy objects to be created from file-descriptor relative paths, which + * may not be globally accessible. * * Returns: (transfer none): Path to rootfs */ From 868776a2961069b3502a24d36cd6e38e058a22ee Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 30 Sep 2021 13:21:15 -0400 Subject: [PATCH 08/11] lib: Add an API to construct a `MutableTree` from a commit This is nicer than having the caller parse the commit object, or indirect via the `OstreeRepoFile*` object of the root. Will be used in ostree-rs-ext around tar parsing. --- apidoc/ostree-sections.txt | 1 + src/libostree/libostree-devel.sym | 1 + src/libostree/ostree-mutable-tree.c | 36 +++++++++++++++++++++++++++++ src/libostree/ostree-mutable-tree.h | 6 +++++ src/ostree/ot-builtin-commit.c | 12 +++------- 5 files changed, 47 insertions(+), 9 deletions(-) diff --git a/apidoc/ostree-sections.txt b/apidoc/ostree-sections.txt index ae8abe81..aa74c839 100644 --- a/apidoc/ostree-sections.txt +++ b/apidoc/ostree-sections.txt @@ -268,6 +268,7 @@ OstreeLzmaDecompressorClass ostree-mutable-tree OstreeMutableTree ostree_mutable_tree_new +ostree_mutable_tree_new_from_commit ostree_mutable_tree_new_from_checksum ostree_mutable_tree_check_error ostree_mutable_tree_set_metadata_checksum diff --git a/src/libostree/libostree-devel.sym b/src/libostree/libostree-devel.sym index 35d53956..6b6b5c6c 100644 --- a/src/libostree/libostree-devel.sym +++ b/src/libostree/libostree-devel.sym @@ -25,6 +25,7 @@ LIBOSTREE_2021.5 { global: ostree_sepolicy_new_from_commit; + ostree_mutable_tree_new_from_commit; } LIBOSTREE_2021.4; /* Stub section for the stable release *after* this development one; don't diff --git a/src/libostree/ostree-mutable-tree.c b/src/libostree/ostree-mutable-tree.c index 8509d156..bba3cf91 100644 --- a/src/libostree/ostree-mutable-tree.c +++ b/src/libostree/ostree-mutable-tree.c @@ -681,3 +681,39 @@ ostree_mutable_tree_new_from_checksum (OstreeRepo *repo, out->metadata_checksum = g_strdup (metadata_checksum); return out; } + +/** + * ostree_mutable_tree_new_from_commit: + * @repo: The repo which contains the objects refered by the checksums. + * @rev: ref or SHA-256 checksum + * + * Creates a new OstreeMutableTree with the contents taken from the given commit. + * The data will be loaded from the repo lazily as needed. + * + * Returns: (transfer full): A new tree + * Since: 2021.5 + */ +OstreeMutableTree * +ostree_mutable_tree_new_from_commit (OstreeRepo *repo, + const char *rev, + GError **error) +{ + g_autofree char *commit = NULL; + if (!ostree_repo_resolve_rev (repo, rev, FALSE, &commit, error)) + return NULL; + g_autoptr(GVariant) commit_v = NULL; + if (!ostree_repo_load_commit (repo, commit, &commit_v, NULL, error)) + return NULL; + + g_autoptr(GVariant) contents_checksum_v = NULL; + g_autoptr(GVariant) metadata_checksum_v = NULL; + char contents_checksum[OSTREE_SHA256_STRING_LEN + 1]; + char metadata_checksum[OSTREE_SHA256_STRING_LEN + 1]; + g_variant_get_child (commit_v, 6, "@ay", &contents_checksum_v); + ostree_checksum_inplace_from_bytes (g_variant_get_data (contents_checksum_v), + contents_checksum); + g_variant_get_child (commit_v, 7, "@ay", &metadata_checksum_v); + ostree_checksum_inplace_from_bytes (g_variant_get_data (metadata_checksum_v), + metadata_checksum); + return ostree_mutable_tree_new_from_checksum (repo, contents_checksum, metadata_checksum); +} diff --git a/src/libostree/ostree-mutable-tree.h b/src/libostree/ostree-mutable-tree.h index 753f96e7..9bf36802 100644 --- a/src/libostree/ostree-mutable-tree.h +++ b/src/libostree/ostree-mutable-tree.h @@ -52,6 +52,12 @@ GType ostree_mutable_tree_get_type (void) G_GNUC_CONST; _OSTREE_PUBLIC OstreeMutableTree *ostree_mutable_tree_new (void); +_OSTREE_PUBLIC +OstreeMutableTree * +ostree_mutable_tree_new_from_commit (OstreeRepo *repo, + const char *rev, + GError **error); + _OSTREE_PUBLIC OstreeMutableTree * ostree_mutable_tree_new_from_checksum (OstreeRepo *repo, const char *contents_checksum, diff --git a/src/ostree/ot-builtin-commit.c b/src/ostree/ot-builtin-commit.c index b993678e..a306c114 100644 --- a/src/ostree/ot-builtin-commit.c +++ b/src/ostree/ot-builtin-commit.c @@ -638,20 +638,14 @@ ostree_builtin_commit (int argc, char **argv, OstreeCommandInvocation *invocatio if (opt_base) { - g_autofree char *base_commit = NULL; - g_autoptr(GFile) base_root = NULL; - if (!ostree_repo_read_commit (repo, opt_base, &base_root, &base_commit, cancellable, error)) + mtree = ostree_mutable_tree_new_from_commit (repo, opt_base, error); + if (!mtree) goto out; - OstreeRepoFile *rootf = (OstreeRepoFile*) base_root; - - mtree = ostree_mutable_tree_new_from_checksum (repo, - ostree_repo_file_tree_get_contents_checksum (rootf), - ostree_repo_file_tree_get_metadata_checksum (rootf)); if (opt_selinux_policy_from_base) { g_assert (modifier); - if (!ostree_repo_commit_modifier_set_sepolicy_from_commit (modifier, repo, base_commit, cancellable, error)) + if (!ostree_repo_commit_modifier_set_sepolicy_from_commit (modifier, repo, opt_base, cancellable, error)) goto out; /* Don't try to handle it twice */ opt_selinux_policy_from_base = FALSE; From e6a560b40797324aa8b90e7100c6d50bff91f14d Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 30 Sep 2021 15:53:18 -0400 Subject: [PATCH 09/11] deploy: Ignore sockets, fifos in /etc during merge https://bugzilla.redhat.com/show_bug.cgi?id=1945274 is an issue where a privileged kubernetes daemonset is writing a socket into `/etc`. This makes ostree upgrades barf. Now, they should clearly move it to `/run`. However, one option is for us to just ignore it instead of erroring out. Some brief investigation shows that e.g. `git add somesocket` is a silent no-op, which is an argument in favor of ignoring it. Closes: https://github.com/ostreedev/ostree/issues/2446 --- src/libostree/ostree-sysroot-deploy.c | 4 +--- tests/kolainst/destructive/staged-deploy.sh | 20 +++++++++++++++++++ .../test-admin-deploy-etcmerge-cornercases.sh | 7 +++++++ 3 files changed, 28 insertions(+), 3 deletions(-) diff --git a/src/libostree/ostree-sysroot-deploy.c b/src/libostree/ostree-sysroot-deploy.c index 6a13a41b..a8bf9f44 100644 --- a/src/libostree/ostree-sysroot-deploy.c +++ b/src/libostree/ostree-sysroot-deploy.c @@ -488,9 +488,7 @@ copy_modified_config_file (int orig_etc_fd, } else { - return glnx_throw (error, - "Unsupported non-regular/non-symlink file in /etc '%s'", - path); + ot_journal_print (LOG_INFO, "Ignoring non-regular/non-symlink file found during /etc merge: %s", path); } return TRUE; diff --git a/tests/kolainst/destructive/staged-deploy.sh b/tests/kolainst/destructive/staged-deploy.sh index f55bb2c8..baadb3d8 100755 --- a/tests/kolainst/destructive/staged-deploy.sh +++ b/tests/kolainst/destructive/staged-deploy.sh @@ -12,6 +12,23 @@ case "${AUTOPKGTEST_REBOOT_MARK:-}" in test -f /run/systemd/generator/multi-user.target.wants/ostree-finalize-staged.path test -f /run/systemd/generator/local-fs.target.requires/ostree-remount.service + cat >/etc/systemd/system/sock-to-ignore.socket << 'EOF' +[Socket] +ListenStream=/etc/sock-to-ignore +EOF + cat >/etc/systemd/system/sock-to-ignore.service << 'EOF' +[Service] +ExecStart=/bin/cat +EOF + # policy denies systemd listening on a socket in /etc (arguably correctly) + setenforce 0 + systemctl daemon-reload + systemctl start --now sock-to-ignore.socket + setenforce 1 + + test -S /etc/sock-to-ignore + mkfifo /etc/fifo-to-ignore + # Initial cleanup to handle the cosa fast-build case ## TODO remove workaround for https://github.com/coreos/rpm-ostree/pull/2021 mkdir -p /var/lib/rpm-ostree/history @@ -54,6 +71,9 @@ case "${AUTOPKGTEST_REBOOT_MARK:-}" in # Assert that the previous boot had a journal entry for it journalctl -b "-1" -u ostree-finalize-staged.service > svc.txt assert_file_has_content svc.txt 'Bootloader updated; bootconfig swap: yes;.*deployment count change: 1' + # Also validate ignoring socket and fifo + assert_file_has_content svc.txt 'Ignoring.*during /etc merge:.*sock-to-ignore' + assert_file_has_content svc.txt 'Ignoring.*during /etc merge:.*fifo-to-ignore' rm -f svc.txt # And there should not be a staged deployment test '!' -f /run/ostree/staged-deployment diff --git a/tests/test-admin-deploy-etcmerge-cornercases.sh b/tests/test-admin-deploy-etcmerge-cornercases.sh index 98e2bbe6..ef4ddeec 100755 --- a/tests/test-admin-deploy-etcmerge-cornercases.sh +++ b/tests/test-admin-deploy-etcmerge-cornercases.sh @@ -51,6 +51,9 @@ chmod 700 ${etc}/a/long/dir/forking # Symlink to nonexistent path, to ensure we aren't walking symlinks ln -s no-such-file ${etc}/a/link-to-no-such-file +# fifo which should be ignored +mkfifo "${etc}/fifo-to-ignore" + # Remove a directory rm ${etc}/testdirectory -rf @@ -66,6 +69,10 @@ newetc=${newroot}/etc assert_file_has_content ${newroot}/usr/etc/NetworkManager/nm.conf "a default daemon file" assert_file_has_content ${newetc}/NetworkManager/nm.conf "a modified config file" +if test -e "${newetc}"/fifo-to-ignore; then + fatal "Should not have copied fifo!" +fi + assert_file_has_mode() { stat -c '%a' $1 > mode.txt if ! grep -q -e "$2" mode.txt; then From c9875345959fb4a028a7596b643d922a952277d0 Mon Sep 17 00:00:00 2001 From: Luca BRUNO Date: Fri, 1 Oct 2021 16:04:02 +0000 Subject: [PATCH 10/11] repo/private: allow committing/aborting through a transaction guard This enhances the auto-transaction logic, augmenting the scope of a transaction guard. It allows committing or aborting a transaction through its guard. It also supports tracking the completion status of a transaction guard, avoiding double commits/aborts, while retaining the auto-cleanup logic. --- src/libostree/ostree-repo-commit.c | 10 ++- src/libostree/ostree-repo-private.h | 65 +++++++------- src/libostree/ostree-repo.c | 112 +++++++++++++++++++++++++ src/libostree/ostree-sysroot-cleanup.c | 4 +- 4 files changed, 156 insertions(+), 35 deletions(-) diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c index c87e8de8..8ac963e7 100644 --- a/src/libostree/ostree-repo-commit.c +++ b/src/libostree/ostree-repo-commit.c @@ -1672,14 +1672,18 @@ ostree_repo_prepare_transaction (OstreeRepo *self, GCancellable *cancellable, GError **error) { + g_assert (self != NULL); + guint64 reserved_bytes = 0; g_return_val_if_fail (self->in_transaction == FALSE, FALSE); g_debug ("Preparing transaction in repository %p", self); - /* Set up to abort the transaction if we return early from this function. */ - g_autoptr(_OstreeRepoAutoTransaction) txn = self; + /* Set up to abort the transaction if we return early from this function. + * This needs to be manually built here due to a circular dependency. */ + g_autoptr(OstreeRepoAutoTransaction) txn = g_malloc(sizeof(OstreeRepoAutoTransaction)); + txn->repo = self; (void) txn; /* Add use to silence static analysis */ memset (&self->txn.stats, 0, sizeof (OstreeRepoTransactionStats)); @@ -1736,7 +1740,7 @@ ostree_repo_prepare_transaction (OstreeRepo *self, return FALSE; /* Success: do not abort the transaction when returning. */ - txn = NULL; (void) txn; + txn->repo = NULL; (void) txn; if (out_transaction_resume) *out_transaction_resume = ret_transaction_resume; diff --git a/src/libostree/ostree-repo-private.h b/src/libostree/ostree-repo-private.h index 67f755bd..a2666dec 100644 --- a/src/libostree/ostree-repo-private.h +++ b/src/libostree/ostree-repo-private.h @@ -229,36 +229,6 @@ struct OstreeRepo { OstreeRepo *parent_repo; }; -/* Taken from flatpak; may be made into public API later */ -typedef OstreeRepo _OstreeRepoAutoTransaction; -static inline void -_ostree_repo_auto_transaction_cleanup (void *p) -{ - if (p == NULL) - return; - g_return_if_fail (OSTREE_IS_REPO (p)); - - OstreeRepo *repo = p; - g_autoptr(GError) error = NULL; - - if (!ostree_repo_abort_transaction (repo, NULL, &error)) - g_warning("Failed to auto-cleanup OSTree transaction: %s", error->message); - - g_object_unref (repo); -} - -static inline _OstreeRepoAutoTransaction * -_ostree_repo_auto_transaction_start (OstreeRepo *repo, - GCancellable *cancellable, - GError **error) -{ - if (!ostree_repo_prepare_transaction (repo, NULL, cancellable, error)) - return NULL; - - return (_OstreeRepoAutoTransaction *) g_object_ref (repo); -} -G_DEFINE_AUTOPTR_CLEANUP_FUNC (_OstreeRepoAutoTransaction, _ostree_repo_auto_transaction_cleanup) - typedef struct { dev_t dev; ino_t ino; @@ -544,4 +514,39 @@ _ostree_repo_verify_bindings (const char *collection_id, GVariant *commit, GError **error); +/** + * OstreeRepoAutoTransaction: + * + * A transaction guard for a specific #OstreeRepo. It can be explicitly + * completed through abort/commit. If the guard has not been completed + * beforehand, on cleanup it is automatically aborted. + * + * Taken from flatpak; may be made into public API later + */ +typedef struct +{ + OstreeRepo *repo; +} OstreeRepoAutoTransaction; + +OstreeRepoAutoTransaction * +_ostree_repo_auto_transaction_start (OstreeRepo *repo, + GCancellable *cancellable, + GError **error); + +gboolean +_ostree_repo_auto_transaction_abort (OstreeRepoAutoTransaction *txn, + GCancellable *cancellable, + GError **error); + +gboolean +_ostree_repo_auto_transaction_commit (OstreeRepoAutoTransaction *txn, + OstreeRepoTransactionStats *out_stats, + GCancellable *cancellable, + GError **error); + +void +_ostree_repo_auto_transaction_cleanup (void *p); + +G_DEFINE_AUTOPTR_CLEANUP_FUNC (OstreeRepoAutoTransaction, _ostree_repo_auto_transaction_cleanup); + G_END_DECLS diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index 42d2b0e0..772eae26 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -711,6 +711,118 @@ ostree_repo_auto_lock_cleanup (OstreeRepoAutoLock *auto_lock) } } + +/** + * _ostree_repo_auto_transaction_start: + * @repo: an #OsreeRepo object + * @cancellable: Cancellable + * @error: a #GError + * + * Start a transaction and return a guard for it. + * + * Returns: (transfer full): an #OsreeRepoAutoTransaction guard on success, + * %NULL otherwise. + */ +OstreeRepoAutoTransaction * +_ostree_repo_auto_transaction_start (OstreeRepo *repo, + GCancellable *cancellable, + GError **error) +{ + g_assert (repo != NULL); + + if (!ostree_repo_prepare_transaction (repo, NULL, cancellable, error)) + return NULL; + + OstreeRepoAutoTransaction *txn = g_malloc(sizeof(OstreeRepoAutoTransaction)); + txn->repo = g_object_ref (repo); + + return g_steal_pointer (&txn); +} + +/** + * _ostree_repo_auto_transaction_abort: + * @txn: an #OsreeRepoAutoTransaction guard + * @cancellable: Cancellable + * @error: a #GError + * + * Abort a transaction, marking the related guard as completed. + * + * Returns: %TRUE on successful commit, %FALSE otherwise. + */ +gboolean +_ostree_repo_auto_transaction_abort (OstreeRepoAutoTransaction *txn, + GCancellable *cancellable, + GError **error) +{ + g_assert (txn != NULL); + + if (txn->repo == NULL) { + return glnx_throw (error, "transaction already completed"); + } + + if (!ostree_repo_abort_transaction (txn->repo, cancellable, error)) + return FALSE; + + g_clear_object (&txn->repo); + + return TRUE; +} + +/** + * _ostree_repo_auto_transaction_commit: + * @txn: an #OsreeRepoAutoTransaction guard + * @cancellable: Cancellable + * @error: a #GError + * + * Commit a transaction, marking the related guard as completed. + * + * Returns: %TRUE on successful aborting, %FALSE otherwise. + */ +gboolean +_ostree_repo_auto_transaction_commit (OstreeRepoAutoTransaction *txn, + OstreeRepoTransactionStats *out_stats, + GCancellable *cancellable, + GError **error) +{ + g_assert (txn != NULL); + + if (txn->repo == NULL) { + return glnx_throw (error, "transaction already completed"); + } + + if (!ostree_repo_commit_transaction (txn->repo, out_stats, cancellable, error)) + return FALSE; + + g_clear_object (&txn->repo); + + return TRUE; +} + +/** + * _ostree_repo_auto_transaction_cleanup: + * @p: pointer to an #OsreeRepoAutoTransaction guard + * + * Destroy a transaction guard. If the transaction has not yet been completed, + * it gets aborted. + */ +void +_ostree_repo_auto_transaction_cleanup (void *p) +{ + if (p == NULL) + return; + + OstreeRepoAutoTransaction *txn = p; + // Auto-abort only if transaction has not already been aborted/committed. + if (txn->repo != NULL) + { + g_autoptr(GError) error = NULL; + if (!_ostree_repo_auto_transaction_abort (txn, NULL, &error)) { + g_warning("Failed to auto-cleanup OSTree transaction: %s", error->message); + g_clear_object (&txn->repo); + } + } +} + static GFile * get_remotes_d_dir (OstreeRepo *self, GFile *sysroot); diff --git a/src/libostree/ostree-sysroot-cleanup.c b/src/libostree/ostree-sysroot-cleanup.c index 91381cb0..c22a6851 100644 --- a/src/libostree/ostree-sysroot-cleanup.c +++ b/src/libostree/ostree-sysroot-cleanup.c @@ -445,7 +445,7 @@ generate_deployment_refs (OstreeSysroot *self, cancellable, error)) return FALSE; - g_autoptr(_OstreeRepoAutoTransaction) txn = + g_autoptr(OstreeRepoAutoTransaction) txn = _ostree_repo_auto_transaction_start (repo, cancellable, error); if (!txn) return FALSE; @@ -458,7 +458,7 @@ generate_deployment_refs (OstreeSysroot *self, ostree_repo_transaction_set_refspec (repo, refname, ostree_deployment_get_csum (deployment)); } - if (!ostree_repo_commit_transaction (repo, NULL, cancellable, error)) + if (!_ostree_repo_auto_transaction_commit (txn, NULL, cancellable, error)) return FALSE; return TRUE; From 63bf5e606b24c7343bfe622bf6af2110ee6beabc Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 5 Oct 2021 15:59:43 -0400 Subject: [PATCH 11/11] Release 2021.5 --- Makefile-libostree.am | 6 +++--- configure.ac | 2 +- src/libostree/libostree-devel.sym | 6 ------ src/libostree/libostree-released.sym | 6 ++++++ tests/test-symbols.sh | 2 +- 5 files changed, 11 insertions(+), 11 deletions(-) diff --git a/Makefile-libostree.am b/Makefile-libostree.am index d40de48d..6b94f76f 100644 --- a/Makefile-libostree.am +++ b/Makefile-libostree.am @@ -173,9 +173,9 @@ endif # USE_GPGME symbol_files = $(top_srcdir)/src/libostree/libostree-released.sym # Uncomment this include when adding new development symbols. -if BUILDOPT_IS_DEVEL_BUILD -symbol_files += $(top_srcdir)/src/libostree/libostree-devel.sym -endif +# if BUILDOPT_IS_DEVEL_BUILD +# symbol_files += $(top_srcdir)/src/libostree/libostree-devel.sym +# endif # http://blog.jgc.org/2007/06/escaping-comma-and-space-in-gnu-make.html wl_versionscript_arg = -Wl,--version-script= diff --git a/configure.ac b/configure.ac index 5678145a..b5a3c82a 100644 --- a/configure.ac +++ b/configure.ac @@ -4,7 +4,7 @@ m4_define([year_version], [2021]) m4_define([release_version], [5]) m4_define([package_version], [year_version.release_version]) AC_INIT([libostree], [package_version], [walters@verbum.org]) -is_release_build=no +is_release_build=yes AC_CONFIG_HEADER([config.h]) AC_CONFIG_MACRO_DIR([buildutil]) AC_CONFIG_AUX_DIR([build-aux]) diff --git a/src/libostree/libostree-devel.sym b/src/libostree/libostree-devel.sym index 6b6b5c6c..e3cd14a4 100644 --- a/src/libostree/libostree-devel.sym +++ b/src/libostree/libostree-devel.sym @@ -22,12 +22,6 @@ - uncomment the include in Makefile-libostree.am */ -LIBOSTREE_2021.5 { -global: - ostree_sepolicy_new_from_commit; - ostree_mutable_tree_new_from_commit; -} LIBOSTREE_2021.4; - /* Stub section for the stable release *after* this development one; don't * edit this other than to update the year. This is just a copy/paste * source. Replace $LASTSTABLE with the last stable version, and $NEWVERSION diff --git a/src/libostree/libostree-released.sym b/src/libostree/libostree-released.sym index 1e359bfb..d38362ba 100644 --- a/src/libostree/libostree-released.sym +++ b/src/libostree/libostree-released.sym @@ -668,6 +668,12 @@ global: ostree_repo_signature_verify_commit_data; } LIBOSTREE_2021.3; +LIBOSTREE_2021.5 { +global: + ostree_sepolicy_new_from_commit; + ostree_mutable_tree_new_from_commit; +} LIBOSTREE_2021.4; + /* NOTE: Only add more content here in release commits! See the * comments at the top of this file. */ diff --git a/tests/test-symbols.sh b/tests/test-symbols.sh index 0de87be4..a094a944 100755 --- a/tests/test-symbols.sh +++ b/tests/test-symbols.sh @@ -56,7 +56,7 @@ echo 'ok documented symbols' # ONLY update this checksum in release commits! cat > released-sha256.txt <