Simon McVittie [Mon, 28 Nov 2016 17:27:44 +0000 (17:27 +0000)]
travis-ci: Enable stretch (the future Debian 9), replacing unstable
My goal in building ostree for Debian unstable was that we would
have good coverage of "new code" paths. However, it was removed
for #571 as too much of a moving target. Debian testing is less of
a moving target, and in particular is always internally consistent
(packages are co-installable), which Debian unstable is not guaranteed
to be.
Debian 'stretch' is the future Debian 9, which should be released
next year.
Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk> Closes: #600
Approved by: cgwalters
glnx_make_lock_file requires that the dfd passed in survives the
lifetime of the lock. Since dfd_iter.fd gets cleaned up after the
function returns, this isn't the case. dfd_iter.fd should be equivalent
to tmpdir_dfd, since we iter on ".", and that survives past the
function, so just use that instead.
Colin Walters [Fri, 18 Nov 2016 20:07:52 +0000 (15:07 -0500)]
tests: Support TEST_SKIP_CLEANUP=err
I find myself often wanting to debug interactively failing tests.
This makes it more convenient to keep around the temporary directories
just for those tests, rather than accumulating tons of tempdirs from
the successful tests as well.
Colin Walters [Sun, 20 Nov 2016 21:17:22 +0000 (16:17 -0500)]
pull: Do GPG verify commit objects when using deltas
The fact that we weren't doing this is at best an oversight, and
for some deployment models a security vulnerability. Having both
`gpg-verify` and `gpg-verify-summary` shows that we were intending
them to be orthogonal/independent.
Lately I've been advocating moving towards pinned TLS instead of
gpg-signed summaries, and if we follow that path, performing GPG
verification of commit objects even if using deltas is more important,
as it provides an at-rest verifiable authenticity and integrity
mechanism.
Content providers which are signing their summary files and/or using
TLS (particularly pinned TLS) for transport should treat this as a
nice-to-have. However, for providers which are serving content over
plain HTTP and relying on GPG, this is a critical update.
Colin Walters [Thu, 17 Nov 2016 16:40:59 +0000 (11:40 -0500)]
[TSAN] Rework assertions to always access refcount atomically
`-fsanitize=address` complained that the `refcount > 0` assertions
were reading without atomics. We can fix this by reworking them
to read the previous value.
Colin Walters [Thu, 17 Nov 2016 16:39:27 +0000 (11:39 -0500)]
[TSAN] main: Stop calling g_set_prgname()
It turns out this is basically racy with the presence of other
threads. It was really cosmetic so let's stop doing it and make
`-fsanitize=thread` happy.
Colin Walters [Thu, 17 Nov 2016 18:48:58 +0000 (13:48 -0500)]
[UBSAN] deltas: Don't call memset(NULL, NULL, 0) with no xattrs
This is actually fine in practice, but it triggers this
`-fsanitize=undefined` warning I saw in the test suite log:
```
src/libostree/ostree-repo-static-delta-compilation.c:160:10: runtime error: null pointer passed as argument 1, which is declared to never be null
```
William Manley [Tue, 19 Jul 2016 02:14:26 +0000 (03:14 +0100)]
ostree commit: Fix combining trees with multiple --tree=ref arguments
You'd expect
ostree commit --tree=ref=A --tree=ref=B
to produce a commit with the union of the trees given. Instead you'd get
a commit with the contents of just the latter commit. This was due to an
optimisation where we'd skip filling out the `files` and `subdirs`
members of the mtree, just filling in the metadata instead. This backfires
becuase this same code relies on checking the `files` and `subdirs` members
itself to work out whether the mtree is empty.
This commit removes the optimisation, fixing the bug. Maybe there's a way
to keep the optimisation and still fix the bug but it's not obvious to
me.
Colin Walters [Wed, 16 Nov 2016 16:50:43 +0000 (11:50 -0500)]
lib: Add an API to GPG verify a commit given a remote
Conceptually we've been moving towards having our GPG verification
paths be per-remote. The code internally supports this, but we
didn't expose an API to use it conveniently.
This came up when trying to add a new `gpgkeypath` option, since
right now rpm-ostree manually finds keyrings for the remote, and
hence it wasn't looking at the keypath, and said "Unknown key"
in status.
Colin Walters [Wed, 16 Nov 2016 14:13:54 +0000 (09:13 -0500)]
Add "gpgkeypath" option to remotes
For Project Atomic, we already have RPM signatures which use files in
`/etc/pki/rpm-gpg`. It's convenient to simply bind the OSTree remote
configuration to those file paths, rather than having duplicate key
data.
This does mean that we need to parse the files for verification, so we
end up importing them into the verifier's temporary keyring, which is
a bit ugly, but it's what other projects do.
Closes: https://github.com/ostreedev/ostree/issues/573 Closes: #575
Approved by: giuseppe
When doing commit --tree=ref=XXX while at the same time applying some
form of modifier, ostree dies trying to read the xattrs using the
raw syscalls. We fix this by falling back to ostree_repo_file_get_xattrs()
in this case.
Colin Walters [Tue, 1 Nov 2016 17:51:55 +0000 (13:51 -0400)]
pull: Redo logic for "scanning"
What in the code is called "scanning" is ensuring (potentially
recursively) have an object, and if not, fetching it. And then if
it's metadata, parsing it and finding new objects to fetch.
This logic has grown fairly complex. What I'm trying to fix
right now is that if we're doing a pull-local to a remote repository
via `sshfs` (FUSE) we still end up scanning, which is inefficient.
We can take advantage of the "commitpartial" logic here - if a commit
isn't partial, it's complete, hence we don't need to scan it.
At the same time, I'm changing the logic here to *always* do scans for
dirtree objects. This will fix cases where multiple commits share
dirtree objects. We have "commitpartial" metadata, but no such concept
of partial/complete for dirtrees.
But, we'll only ever scan dirtrees if we scan commits, which is
what the section above fixes.
Colin Walters [Wed, 16 Nov 2016 02:03:26 +0000 (21:03 -0500)]
pull: Add support for `http-headers` option
Some deployments may want to gate access to content based on things
like OAuth. In this model, the client system would normally compute a
token and pass it to the server via an API.
We could theoretically support this in the remote config too, but
that'd be a bit weird for OAuth as the information is dynamic.
Therefore this cleans up the code a little bit to more clearly handle
the case that the fetcher is initialized from both remote config
data plus pull options.
Colin Walters [Thu, 3 Nov 2016 12:32:19 +0000 (08:32 -0400)]
deltas: Only keep one file open at a time during compilation
Otherwise it's possible for us to exhaust available file descriptors
or (on 32 bit) run up against mmap limits.
In the rollsum case, we didn't need to hold open the "from" object
at all. And in the bsdiff case, we weren't even looking at either of
the files until we started processing.
Also, while we have the patient open, switch to using O_TMPFILE
if available.
Sjoerd Simons [Mon, 17 Oct 2016 20:30:41 +0000 (22:30 +0200)]
OsreeFetcher: Treat 403 as not found
Private Cloudfront instances return 403 for objects which don't exist
rather then a 404.
Change the fetcher to assume 403 is ok for download that are "optional"
rather then erroring out at that step (e.g. trying to download a static
delta if the remote repo doesn't have those)
Sjoerd Simons [Mon, 17 Oct 2016 19:39:38 +0000 (21:39 +0200)]
pull: Add per-remote cookie jar
Optionally read cookie jars for a remote to be used when downloading
data. This can be used for private repositories which require specific
cookies to be present, e.g. repositories hosted on Amazon cloudfront
using signed cookies.
Colin Walters [Tue, 1 Nov 2016 18:17:57 +0000 (14:17 -0400)]
repo: Don't put remote refs in the summary file
I was doing a chain of mirroring like A -> B -> C
And repo B had A as a remote. When I added B as
a remote to C, the summary file of B had a ref
upstream:foo/bar/baz, which caused all pulls from
B to C to fail, since the summary file is only
expected to have refs, not refspecs.
Sjoerd Simons [Sun, 30 Oct 2016 20:06:27 +0000 (21:06 +0100)]
Filter bootloader supplied kernel cmdline options
Various bootloader add kernel commandline options dynamically, filter
these out when grabbing boot options from /proc/cmdline. Specifically
grub adds BOOT_IMAGE and systemd-boot adds initrd.
Simon McVittie [Sat, 29 Oct 2016 17:47:11 +0000 (18:47 +0100)]
Fix some leaks of floating GVariants
ostree_repo_pull_with_options() and ostree_repo_remote_change() don't
sink floating GVariant arguments, and doing so now would be an
ABI change; so don't rely on them to do so.
Leak found with valgrind memcheck.
Signed-off-by: Simon McVittie <smcv@debian.org> Closes: #556
Approved by: cgwalters
Simon McVittie [Sun, 30 Oct 2016 14:14:09 +0000 (14:14 +0000)]
ostree_admin_option_context_parse: explicitly clean up when exiting early
The cleanup attribute doesn't clean up before calling a noreturn
function like exit(). Explicitly clean up the pointer variables
(but don't assume that a simple g_object_unref() would be OK either,
in case the behaviour of the cleanup attribute changes).
This isn't a real leak since we're about to exit anyway, but if we
don't fix it then valgrind memcheck will make the tests fail.
Signed-off-by: Simon McVittie <smcv@debian.org> Closes: #559
Approved by: cgwalters
Simon McVittie [Sat, 29 Oct 2016 20:31:18 +0000 (21:31 +0100)]
_ostree_kernel_args_replace_take: don't leak when replacing
If !existed, then we add arg to kargs->order, where it will be freed
by that array's free-function. However, if the kernel argument did
already exist, we have to either free arg ourselves (and make sure
the old key is what appears in the hash table), or do a linear search
on kargs->order to replace the old key with the new.
Leak found by valgrind memcheck.
Signed-off-by: Simon McVittie <smcv@debian.org> Closes: #559
Approved by: cgwalters
Simon McVittie [Fri, 7 Oct 2016 22:09:29 +0000 (23:09 +0100)]
Distribute test scripts even if we wouldn't run them
This fixes a "make dist" tarball produced on a minimal system and run on a
non-minimal system. Automake knows that files that are only conditionally
included in dist_whatever_WHATEVER are to be distributed, but it does not
do the same for files that are only conditionally included in EXTRA_DIST,
which is how glib-tap.mk's various variables like dist_test_scripts work.
Signed-off-by: Simon McVittie <smcv@debian.org> Closes: #557
Approved by: cgwalters
Simon McVittie [Sat, 8 Oct 2016 13:19:01 +0000 (14:19 +0100)]
Force C.UTF-8 or C locale for tests
Otherwise several tests fail, for example in this build done in a French
locale by Debian's reproducible builds initiative, to check whether
the resulting binaries are identical to what was produced in an
English locale:
(test-pull-metalink)
# error: Erreur à la ligne 1, caractère 1 : Le document doit commencer avec un élément (par ex. <book>)
...
File 'err.txt' doesn't match regexp 'Document must begin with an element'
Signed-off-by: Simon McVittie <smcv@debian.org> Closes: #558
Approved by: cgwalters
static-delta: find a similar filename using what is before '.' or '-'
Improve the heuristic to use only the part before the first '.' when
looking for a similar file in the current directory.
last versions of dracut generate reproducible initramfs files, but we
still fallback to the full file download if there is any minimal
change that causes a different checksum and file name.
This change extends that case to deal better with similar files that
have a different suffix.
This is the difference generating a static delta from
fedora-atomic/f24/x86_64/docker-host to fedora-atomic/f24/x86_64/testing/docker-host
Jonathan Lebon [Thu, 27 Oct 2016 16:03:04 +0000 (12:03 -0400)]
.redhat-ci.yml: add clang
Clang has better detection for unused vars when using auto cleanup
functions. We should eventually just fold this back into the first
testsuite. But let's just turn it on for now, at least until it's
satisfied with the whole codebase.
Colin Walters [Tue, 25 Oct 2016 14:40:45 +0000 (10:40 -0400)]
tests: Skip libarchive/selinux tests if in container without SELinux
I'm doing builds and `make check` inside a Docker container, with
selinux on as a build-time option, but no policy in the container.
This currently aborts. Let's not do that.
(This type of thing is why installed tests are a better model)
If we have a partial commit it is not an error for a dirmeta to be
missing (in fact, that is likely), so instead of returning a not-found
error from ostree_repo_traverse_commit() we ignore the error and
continue.
In particular, this means we don't stop early at the first
missing dirmeta, which previously caused ostree_repo_prune() to
thing the dirmetas after that to be unreached and thus purged.
Also, we remove the special casing in ostree_repo_prune() to
not report errors for commitpartial, because these should not
be reported anymore.
This fixes https://github.com/ostreedev/ostree/issues/541
If there is a transaction active, then we put writes to detached
metadata into the staging dir, and when reading it we look there
first. This allows transactions to be aborted half-way without
writing the detached metadata into the repository (possibly
overwriting any old metadata from there).
This fixes https://github.com/ostreedev/ostree/issues/526
Commit 1d4f1b8878addd28059c3a3928640491755cd615 started using hardlinks
checkouts of symlinks. However, symlinks are not stored as symlink in the
repo for bare-user repos, so this breaks user-mode checkouts of such repos.
We fix this by checking for !is_symlink in the bare-user case.
This fixes:
https://github.com/ostreedev/ostree/issues/537
Jonathan Lebon [Wed, 19 Oct 2016 16:37:54 +0000 (12:37 -0400)]
add .redhat-ci.yml and .redhat-ci.Dockerfile
Add a YAML file for the new Red Hat CI framework.
Rather than re-installing all the build deps everytime, which takes
time, I added a Dockerfile that we can wire up to the Docker Hub. For
now it lives at `jlebon/ostree-tester:rhci`, but we can move it under
the `projectatomic` org (or a new `ostree` org).
Colin Walters [Tue, 4 Oct 2016 19:23:04 +0000 (15:23 -0400)]
core: Do create hardlinks to symlinks for checkouts
I was noticing a recent performance issue with checkouts
which seemed to be mostly us going back to doing a `fsync()` on
directories.
Regardless, while looking at that, I saw we were spending time
creating new symlinks. Even though symlinks are small, it's still
better to hardlink them.
Going way back in time, the reason we weren't doing this is
because we were hitting `EMFILE` on ext4, but that was for
gnome-continuous which creates *many* build roots. Even
there though, they're just a cache, and we handle `EMFILE`.
For ostree-for-host-system, we don't expect to have many roots (just 3
at most transiently), so hardlinking symlinks does make sense.
Jonathan Lebon [Fri, 14 Oct 2016 13:25:01 +0000 (09:25 -0400)]
OstreeFetcher: provide proxy credentials if needed
There seems to be an issue in libsoup which causes basic auth
credentials to not be passed to the proxy during requests. We thus have
to handle PROXY_UNAUTHORIZED responses and provide the auth ourselves.
Jonathan Lebon [Thu, 13 Oct 2016 16:05:54 +0000 (12:05 -0400)]
ostree-sysroot-deploy.c: delete redundant check
Just noticed this while inspecting the code. The deployments retrieved
by `_ostree_sysroot_list_deployment_dirs_for_os` will forcibly already
have a matching osname since it indirectly uses that same variable to
construct them. Having a check there makes it look like there may be
subtle corner cases, when there aren't.
Jonathan Lebon [Thu, 13 Oct 2016 15:56:46 +0000 (11:56 -0400)]
docs: amend vmlinuz & initramfs naming convention
I was confused while reading the docs how this could work, since in at
least the Fedora/CentOS/RHEL distros, they're named e.g.
initramfs-`uname -r`-$checksum.
If this is true, don't initiate, abort of commit a transaction, instead
it is assumed that the caller initiated the transaction, and that it
will eventually be commited.
This allows you to do multiple pulls or a combination of pulls and
commits in a single transaction.