From 479406e6a587809cd38550745c6f74d680d7c809 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Thu, 31 May 2018 16:31:22 -0400 Subject: [PATCH] Add support for YAML treefiles Let's modernize and start supporting YAML treefiles. I'll dare make the sweeping generalization that most people would prefer reading and writing YAML over JSON. This takes bits from coreos-assembler[1] that know how to serialize a YAML file and spit it back out as a JSON and makes it into a shared lib that we can link against. We could use this eventually for JSON inputs as well to force a validation check before composing. If we go this route, we could then turn on `--enable-rust` in FAHC for now and drop the duplicate code in coreos-assembler. [1] https://github.com/cgwalters/coreos-assembler Closes: #1377 Approved by: cgwalters --- .papr.yml | 2 + Makefile-rpm-ostree.am | 17 ++ Makefile.am | 24 +++ configure.ac | 36 ++++ rust/Cargo.toml | 22 +++ rust/cargo-vendor-config | 8 + rust/src/glibutils.rs | 60 +++++++ rust/src/treefile.rs | 211 +++++++++++++++++++++++ src/app/libtreefile_rs.h | 23 +++ src/app/rpmostree-compose-builtin-tree.c | 56 ++++-- tests/compose-tests/libcomposetest.sh | 6 +- tests/compose-tests/test-basic.sh | 15 ++ 12 files changed, 467 insertions(+), 13 deletions(-) create mode 100644 rust/Cargo.toml create mode 100644 rust/cargo-vendor-config create mode 100644 rust/src/glibutils.rs create mode 100644 rust/src/treefile.rs create mode 100644 src/app/libtreefile_rs.h diff --git a/.papr.yml b/.papr.yml index 40d5e00e..400e51ea 100644 --- a/.papr.yml +++ b/.papr.yml @@ -131,6 +131,8 @@ host: # since https://github.com/projectatomic/rpm-ostree/pull/875 tests: - docker run --privileged --rm + -e CONFIGOPTS=--enable-rust + -e CI_PKGS=cargo -e RPMOSTREE_COMPOSE_TEST_USE_REPOS=/etc/yum.repos.d.host -v /etc/yum.repos.d:/etc/yum.repos.d.host:ro -v $(pwd):/srv/code -w /srv/code diff --git a/Makefile-rpm-ostree.am b/Makefile-rpm-ostree.am index 924478ca..46f9d244 100644 --- a/Makefile-rpm-ostree.am +++ b/Makefile-rpm-ostree.am @@ -70,6 +70,10 @@ rpm_ostree_CFLAGS = $(AM_CFLAGS) -I$(srcdir)/src/app -I$(srcdir)/src/daemon \ -I$(srcdir)/src/lib -I$(srcdir)/src/libpriv -I$(libglnx_srcpath) \ -fvisibility=hidden -DPKGLIBDIR=\"$(pkglibdir)\" $(PKGDEP_RPMOSTREE_CFLAGS) rpm_ostree_LDADD = $(PKGDEP_RPMOSTREE_LIBS) librpmostreepriv.la librpmostree-1.la librpmostreed.la +# https://github.com/ostreedev/ostree/commit/1f832597fc83fda6cb8daf48c4495a9e1590774c +if ENABLE_RUST +rpm_ostree_LDADD += -ldl +endif privdatadir=$(pkglibdir) privdata_DATA = src/app/rpm-ostree-0-integration.conf @@ -80,3 +84,16 @@ install-bin-hook: if BUILDOPT_NEW_NAME INSTALL_DATA_HOOKS += install-bin-hook endif + +if ENABLE_RUST +libtreefilepath = @abs_top_builddir@/target/@RUST_TARGET_SUBDIR@/libtreefile_rs.a +TREEFILE_RUST_SRCS = rust/src/treefile.rs +$(libtreefilepath): Makefile $(TREEFILE_RUST_SRCS) + cd $(top_srcdir)/rust && \ + CARGO_TARGET_DIR=@abs_top_builddir@/target \ + $(cargo) build --verbose $(CARGO_RELEASE_ARGS) +EXTRA_DIST += $(TREEFILE_RUST_SRCS) rust/Cargo.lock + +rpm_ostree_SOURCES += src/app/libtreefile_rs.h +rpm_ostree_LDADD += $(libtreefilepath) +endif # ENABLE_RUST diff --git a/Makefile.am b/Makefile.am index 003859d3..854c4b00 100644 --- a/Makefile.am +++ b/Makefile.am @@ -67,6 +67,30 @@ include $(INTROSPECTION_MAKEFILE) GIRS = TYPELIBS = $(GIRS:.gir=.typelib) +# These bits based on gnome:librsvg/Makefile.am +if ENABLE_RUST +if RUST_DEBUG +CARGO_RELEASE_ARGS= +else +CARGO_RELEASE_ARGS=--release +endif + +check-local: + cd $(srcdir)/rust && CARGO_TARGET_DIR=$(abs_top_builddir)/target cargo test + +clean-local: + cd $(srcdir)/rust && CARGO_TARGET_DIR=$(abs_top_builddir)/target cargo clean + +dist-hook: + (cd $(distdir)/rust && \ + cp $(abs_top_srcdir)/rust/Cargo.lock . && \ + cargo vendor -q && \ + mkdir .cargo && \ + cp cargo-vendor-config .cargo/config) + +EXTRA_DIST += $(srcdir)/rust/Cargo.toml $(srcdir)/rust/cargo-vendor-config +endif # end ENABLE_RUST + include libglnx/Makefile-libglnx.am.inc noinst_LTLIBRARIES += libglnx.la include Makefile-libpriv.am diff --git a/configure.ac b/configure.ac index ef9b62ae..24a53652 100644 --- a/configure.ac +++ b/configure.ac @@ -182,6 +182,41 @@ AS_IF([test x$enable_compose_tooling = xyes], [ ]) if test x$enable_compose_tooling != xno; then RPM_OSTREE_FEATURES="$RPM_OSTREE_FEATURES compose"; fi +AC_ARG_ENABLE(rust, + AS_HELP_STRING([--enable-rust], + [Compile Rust features (e.g. compose tree --yaml)]),, + [enable_rust=no; rust_debug_release=no]) + +AS_IF([test x$enable_rust = xyes], [ + AC_PATH_PROG([cargo], [cargo]) + AS_IF([test -z "$cargo"], [AC_MSG_ERROR([cargo is required for --enable-rust])]) + AC_PATH_PROG([rustc], [rustc]) + AS_IF([test -z "$rustc"], [AC_MSG_ERROR([rustc is required for --enable-rust])]) + AC_DEFINE(HAVE_RUST, 1, [Define if we are building with Rust]) + + dnl These bits based on gnome:librsvg/configure.ac + + dnl By default, we build in public release mode. + AC_ARG_ENABLE(rust-debug, + AC_HELP_STRING([--enable-rust-debug], + [Build Rust code with debugging information [default=no]]), + [rust_debug_release=$enableval], + [rust_debug_release=release]) + + AC_MSG_CHECKING(whether to build Rust code with debugging information) + if test "x$rust_debug_release" = "xyes" ; then + rust_debug_release=debug + AC_MSG_RESULT(yes) + else + AC_MSG_RESULT(no) + fi + RUST_TARGET_SUBDIR=${rust_debug_release} + AC_SUBST([RUST_TARGET_SUBDIR]) +]) +AM_CONDITIONAL(RUST_DEBUG, [test "x$rust_debug_release" = "xdebug"]) +AM_CONDITIONAL(ENABLE_RUST, [test x$enable_rust = xyes]) +if test x$enable_rust != xno; then RPM_OSTREE_FEATURES="$RPM_OSTREE_FEATURES rust"; fi + dnl Try to automatically determine cmake type from CFLAGS if $(echo $CFLAGS |grep -q -E "(-O0|-Og)"); then cmake_args="-DCMAKE_BUILD_TYPE=Debug" @@ -222,4 +257,5 @@ echo " introspection: $found_introspection bubblewrap: $with_bubblewrap gtk-doc: $enable_gtk_doc + rust: $enable_rust " diff --git a/rust/Cargo.toml b/rust/Cargo.toml new file mode 100644 index 00000000..74821297 --- /dev/null +++ b/rust/Cargo.toml @@ -0,0 +1,22 @@ +[package] +name = "treefile" +version = "0.1.0" +authors = ["Colin Walters "] + +[dependencies] +serde = "1.0" +serde_derive = "1.0" +serde_json = "1.0" +serde_yaml = "0.7" +libc = "0.2" +glib-sys = "0.6.0" +gio-sys = "0.6.0" + +[lib] +name = "treefile_rs" +path = "src/treefile.rs" +crate-type = ["staticlib"] + +[profile.release] +panic = "abort" +lto = true diff --git a/rust/cargo-vendor-config b/rust/cargo-vendor-config new file mode 100644 index 00000000..5407266e --- /dev/null +++ b/rust/cargo-vendor-config @@ -0,0 +1,8 @@ +# This is used after `cargo vendor` is run from `make dist` + +[source.crates-io] +registry = 'https://github.com/rust-lang/crates.io-index' +replace-with = 'vendored-sources' + +[source.vendored-sources] +directory = './vendor' diff --git a/rust/src/glibutils.rs b/rust/src/glibutils.rs new file mode 100644 index 00000000..17589972 --- /dev/null +++ b/rust/src/glibutils.rs @@ -0,0 +1,60 @@ +/* + * Copyright (C) 2018 Red Hat, Inc. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA + */ + +/* Copied and adapted from: + * https://github.com/cgwalters/coreos-assembler + * */ + +use gio_sys; +use glib_sys; +use libc; +use std; +use std::ffi::CString; +use std::ptr; + +// Consume a Result into the "GError convention": +// https://developer.gnome.org/glib/stable/glib-Error-Reporting.html +// To use, just add .to_glib_convention(error) at the end of function calls that +// return a Result (using the std Error). +pub trait ToGlibConvention { + fn to_glib_convention(self: Self, error: *mut *mut glib_sys::GError) -> libc::c_int; +} + +// TODO: Add a variant for io::Result? Or try upstreaming this into the glib crate? +impl ToGlibConvention for Result +where + E: std::error::Error, +{ + fn to_glib_convention(self: Result, error: *mut *mut glib_sys::GError) -> libc::c_int { + match &self { + &Ok(_) => 1, + &Err(ref e) => { + unsafe { + assert!(*error == ptr::null_mut()); + let c_msg = CString::new(e.description()).unwrap(); + *error = glib_sys::g_error_new_literal( + gio_sys::g_io_error_quark(), + gio_sys::G_IO_ERROR_FAILED, + c_msg.as_ptr(), + ); + }; + 0 + } + } + } +} diff --git a/rust/src/treefile.rs b/rust/src/treefile.rs new file mode 100644 index 00000000..36f15de7 --- /dev/null +++ b/rust/src/treefile.rs @@ -0,0 +1,211 @@ +/* + * Copyright (C) 2018 Red Hat, Inc. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA + */ + +/* Copied and adapted from: + * https://github.com/cgwalters/coreos-assembler + * */ + +extern crate gio_sys; +extern crate glib_sys; +extern crate libc; + +#[macro_use] +extern crate serde_derive; +extern crate serde; +extern crate serde_json; +extern crate serde_yaml; + +use std::ffi::{CStr, OsStr}; +use std::os::unix::ffi::OsStrExt; +use std::os::unix::io::FromRawFd; +use std::path::Path; +use std::{fs, io}; + +mod glibutils; +use glibutils::*; + +#[no_mangle] +pub extern "C" fn treefile_read( + filename: *const libc::c_char, + fd: libc::c_int, + error: *mut *mut glib_sys::GError, +) -> libc::c_int { + // using an O_TMPFILE is an easy way to avoid ownership transfer issues w/ returning allocated + // memory across the Rust/C boundary + + // let's just get the unsafory stuff (see what I did there?) out of the way first + let output_file = unsafe { fs::File::from_raw_fd(libc::dup(fd)) }; + let c_str: &CStr = unsafe { CStr::from_ptr(filename) }; + let filename_path = Path::new(OsStr::from_bytes(c_str.to_bytes())); + + treefile_read_impl(filename_path, output_file).to_glib_convention(error) +} + +fn treefile_read_impl(filename: &Path, output: fs::File) -> io::Result<()> { + let f = io::BufReader::new(fs::File::open(filename)?); + + let mut treefile: TreeComposeConfig = match serde_yaml::from_reader(f) { + Ok(t) => t, + Err(e) => { + return Err(io::Error::new( + io::ErrorKind::InvalidInput, + format!("{}", e), + )) + } + }; + + // special handling for packages, since we allow whitespaces within items + if let Some(pkgs) = treefile.packages { + treefile.packages = Some(whitespace_split_packages(&pkgs)); + } + + serde_json::to_writer_pretty(output, &treefile)?; + + Ok(()) +} + +fn whitespace_split_packages(pkgs: &Vec) -> Vec { + let mut ret = Vec::with_capacity(pkgs.len()); + for pkg in pkgs { + for pkg_item in pkg.split_whitespace() { + ret.push(pkg_item.into()); + } + } + return ret; +} + +#[derive(Serialize, Deserialize, Debug)] +pub enum BootLocation { + #[serde(rename = "both")] + Both, + #[serde(rename = "legacy")] + Legacy, + #[serde(rename = "new")] + New, +} + +impl Default for BootLocation { + fn default() -> Self { + BootLocation::Both + } +} + +#[derive(Serialize, Deserialize, Debug)] +pub enum CheckPasswdType { + #[serde(rename = "none")] + None, + #[serde(rename = "previous")] + Previous, + #[serde(rename = "file")] + File, + #[serde(rename = "data")] + Data, +} + +#[derive(Serialize, Deserialize, Debug)] +pub struct CheckPasswd { + #[serde(rename = "type")] + variant: CheckPasswdType, + filename: Option, + // Skip this for now, a separate file is easier + // and anyways we want to switch to sysusers + // entries: OptionString>, +} + +#[derive(Serialize, Deserialize, Debug)] +pub struct TreeComposeConfig { + // Compose controls + #[serde(rename = "ref")] + pub treeref: String, + #[serde(skip_serializing_if = "Option::is_none")] + repos: Option>, + #[serde(skip_serializing_if = "Option::is_none")] + pub selinux: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub gpg_key: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub include: Option, + + // Core content + #[serde(skip_serializing_if = "Option::is_none")] + pub packages: Option>, + #[serde(skip_serializing_if = "Option::is_none")] + pub bootstrap_packages: Option>, + + // Content installation opts + #[serde(skip_serializing_if = "Option::is_none")] + pub documentation: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub install_langs: Option>, + #[serde(skip_serializing_if = "Option::is_none")] + #[serde(rename = "initramfs-args")] + pub initramfs_args: Option>, + + // Tree layout options + #[serde(default)] + pub boot_location: BootLocation, + #[serde(default)] + pub tmp_is_dir: bool, + + // systemd + #[serde(skip_serializing_if = "Option::is_none")] + pub units: Option>, + #[serde(skip_serializing_if = "Option::is_none")] + pub default_target: Option, + + // versioning + #[serde(skip_serializing_if = "Option::is_none")] + pub releasever: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub automatic_version_prefix: Option, + #[serde(skip_serializing_if = "Option::is_none")] + #[serde(rename = "mutate-os-relase")] + pub mutate_os_release: Option, + + // passwd-related bits + #[serde(skip_serializing_if = "Option::is_none")] + pub etc_group_members: Option>, + #[serde(skip_serializing_if = "Option::is_none")] + #[serde(rename = "preserve-passwd")] + pub preserve_passwd: Option, + #[serde(skip_serializing_if = "Option::is_none")] + #[serde(rename = "check-passwd")] + pub check_passwd: Option, + #[serde(skip_serializing_if = "Option::is_none")] + #[serde(rename = "check-groups")] + pub check_groups: Option, + #[serde(skip_serializing_if = "Option::is_none")] + #[serde(rename = "ignore-removed-users")] + pub ignore_removed_users: Option>, + #[serde(skip_serializing_if = "Option::is_none")] + #[serde(rename = "ignore-removed-groups")] + pub ignore_removed_groups: Option>, + + // Content manimulation + #[serde(skip_serializing_if = "Option::is_none")] + #[serde(rename = "postprocess-script")] + pub postprocess_script: Option, + #[serde(skip_serializing_if = "Option::is_none")] + #[serde(rename = "add-files")] + pub add_files: Option>, + #[serde(skip_serializing_if = "Option::is_none")] + pub remove_files: Option>, + #[serde(skip_serializing_if = "Option::is_none")] + #[serde(rename = "remove-from-packages")] + pub remove_from_packages: Option>>, +} diff --git a/src/app/libtreefile_rs.h b/src/app/libtreefile_rs.h new file mode 100644 index 00000000..b0e5aa36 --- /dev/null +++ b/src/app/libtreefile_rs.h @@ -0,0 +1,23 @@ +/* -*- mode: C; c-file-style: "gnu"; indent-tabs-mode: nil; -*- + * + * Copyright (C) 2018 Jonathan Lebon + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the + * Free Software Foundation, Inc., 59 Temple Place - Suite 330, + * Boston, MA 02111-1307, USA. + */ + +#pragma once + +int treefile_read (const char *filename, int output_fd, GError **error); diff --git a/src/app/rpmostree-compose-builtin-tree.c b/src/app/rpmostree-compose-builtin-tree.c index 73a3c1d3..914d7d1e 100644 --- a/src/app/rpmostree-compose-builtin-tree.c +++ b/src/app/rpmostree-compose-builtin-tree.c @@ -44,6 +44,9 @@ #include "rpmostree-passwd-util.h" #include "rpmostree-libbuiltin.h" #include "rpmostree-rpm-util.h" +#ifdef HAVE_RUST +#include "libtreefile_rs.h" +#endif #include "libglnx.h" @@ -670,6 +673,43 @@ install_packages_in_root (RpmOstreeTreeComposeContext *self, return TRUE; } +static gboolean +parse_treefile_to_json (const char *treefile_path, + JsonParser **out_parser, + GError **error) +{ +#ifdef HAVE_RUST + g_autofree char *fdpath = NULL; + g_auto(GLnxTmpfile) json_contents = { 0, }; +#endif + + if (g_str_has_suffix (treefile_path, ".yaml") || + g_str_has_suffix (treefile_path, ".yml")) + { +#ifndef HAVE_RUST + return glnx_throw (error, "This version of rpm-ostree was built without " + "rust, and doesn't support YAML treefiles"); +#else + if (!glnx_open_anonymous_tmpfile (O_RDWR | O_CLOEXEC, &json_contents, error)) + return FALSE; + + if (!treefile_read (treefile_path, json_contents.fd, error)) + return glnx_prefix_error (error, "Failed to load YAML treefile"); + + /* or just lseek back to 0 and use json_parser_load_from_data here? */ + treefile_path = fdpath = g_strdup_printf ("/proc/self/fd/%d", json_contents.fd); +#endif + } + + g_autoptr(JsonParser) parser = json_parser_new (); + if (!json_parser_load_from_file (parser, treefile_path, error)) + return FALSE; + + *out_parser = g_steal_pointer (&parser); + return TRUE; +} + + static gboolean process_includes (RpmOstreeTreeComposeContext *self, GFile *treefile_path, @@ -706,15 +746,14 @@ process_includes (RpmOstreeTreeComposeContext *self, { g_autoptr(GFile) treefile_dirpath = g_file_get_parent (treefile_path); g_autoptr(GFile) parent_path = g_file_resolve_relative_path (treefile_dirpath, include_path); - glnx_unref_object JsonParser *parent_parser = json_parser_new (); + g_autoptr(JsonParser) parent_parser = NULL; JsonNode *parent_rootval; JsonObject *parent_root; GList *members; GList *iter; - if (!json_parser_load_from_file (parent_parser, - gs_file_get_path_cached (parent_path), - error)) + if (!parse_treefile_to_json (gs_file_get_path_cached (treefile_path), + &parent_parser, error)) return FALSE; parent_rootval = json_parser_get_root (parent_parser); @@ -920,10 +959,8 @@ rpm_ostree_compose_context_new (const char *treefile_pathstr, if (!self->corectx) return FALSE; - self->treefile_parser = json_parser_new (); - if (!json_parser_load_from_file (self->treefile_parser, - gs_file_get_path_cached (self->treefile_path), - error)) + if (!parse_treefile_to_json (gs_file_get_path_cached (self->treefile_path), + &self->treefile_parser, error)) return FALSE; self->treefile_rootval = json_parser_get_root (self->treefile_parser); @@ -1436,8 +1473,7 @@ rpmostree_compose_builtin_postprocess (int argc, JsonObject *treefile = NULL; /* Owned by parser */ if (treefile_path) { - treefile_parser = json_parser_new (); - if (!json_parser_load_from_file (treefile_parser, treefile_path, error)) + if (!parse_treefile_to_json (treefile_path, &treefile_parser, error)) return FALSE; JsonNode *treefile_rootval = json_parser_get_root (treefile_parser); diff --git a/tests/compose-tests/libcomposetest.sh b/tests/compose-tests/libcomposetest.sh index 1269beb1..1925940c 100644 --- a/tests/compose-tests/libcomposetest.sh +++ b/tests/compose-tests/libcomposetest.sh @@ -5,6 +5,9 @@ trap _cleanup_tmpdir EXIT cd ${test_tmpdir} . ${dn}/../common/libtest.sh +export repo=$(pwd)/repo +export repobuild=$(pwd)/repo-build + pyeditjson() { cat >editjson.py < ${treefile}.new && mv ${treefile}{.new,} } -export repo=$(pwd)/repo -export repobuild=$(pwd)/repo-build - prepare_compose_test() { name=$1 shift diff --git a/tests/compose-tests/test-basic.sh b/tests/compose-tests/test-basic.sh index 15d11edd..402a59d2 100755 --- a/tests/compose-tests/test-basic.sh +++ b/tests/compose-tests/test-basic.sh @@ -29,3 +29,18 @@ assert_file_has_content_literal autovar.txt 'd /var/cache 0755 root root - -' # And this one has a non-root uid assert_file_has_content_literal autovar.txt 'd /var/log/chrony 0755 chrony chrony - -' echo "ok autovar" + +if ! rpm-ostree --version | grep -q rust; then + echo "ok yaml (SKIP)" +else + prepare_compose_test "from-yaml" + python <