Example user story: Jane rebases her OS to a new major version N, and wants to
keep around N-1 even after a few upgrades for a while so she can easily roll
back. I plan to add `rpm-ostree rebase --pin` to opt-in to this for example.
Builds on the new `libostree-transient` group to store pinning state there.
Closes: https://github.com/ostreedev/ostree/issues/1460Closes: #1464
Approved by: jlebon
The `origin/unlocked` and `origin/override-commit` keys are examples of state
that's really transient; we don't want to maintain them across upgrades. Right
now there are bits for this in both `ostree admin upgrade` as well as in
rpm-ostree.
This new API will slightly clean up both cases, but it's really prep for adding
a concept of deployment "pinning" that will live in the new
`libostree-transient` group.
Closes: #1464
Approved by: jlebon
There are enough fixes here, and there are some potentially larger patches
incoming like wmanley's checkout speedups and the payload link that will need
soak time in master.
Closes: #1455
Approved by: jlebon
Since f4d1334e19 the primary pull code maintains a
maximum queue. In that commit message I said `Note that I kept an assertion.`.
But I think this is wrong since while it covers a lot of the normal cases, if
one is e.g. trying to fetch a ton of refs, the primary pull code doesn't yet
queue those. While it'd be nice to queue those, it isn't worth carrying
extra assertions in the backends that can still trigger.
Closes: https://github.com/ostreedev/ostree/issues/1451Closes: #1453
Approved by: dbnicholson
There are a few cases for knowing whether a commit has identical
content to another commit. Some people want to do a "promotion workflow",
where the content of a commit on a tesitng branch is then "promoted"
to a production branch with `ostree commit --tree=ref`.
Another use case I just hit in rpm-ostree deals with
[jigdo](https://github.com/projectatomic/rpm-ostree/issues/1081) where we're
importing RPMs on both the client and server, and will be using the
content checksum, since the client/server cases inject different metadata
into the commit object.
Closes: https://github.com/ostreedev/ostree/issues/1315Closes: #1449
Approved by: jlebon
Having the `uncompressed-object-cache` directory in `archive` repos by default
is clutter; the functionality should be considered deprecated.
Now we only create the directory if we're doing a checkout with the cache
enabled.
Closes: #1446
Approved by: jlebon
This will allow the text to be used in Wikipedia for example; it
also just makes more sense for documentation than the LGPLv2+.
Closes: #1431Closes: #1432
Approved by: jlebon
When we changed around the kernel location in rpm-ostree, we
started installing the kernel into `/boot` as `modules_object_t`,
and the current policy didn't permit that. For maximum compatibility,
relabel installed kernel/initramfs/dtb as `boot_t`.
https://bugzilla.redhat.com/show_bug.cgi?id=1536991Closes: #1444
Approved by: jlebon
Downstream BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1498281
This came up as a problem with `oci-umount` which was trying to ensure some host
mounts like `/var/lib/containers` don't leak into privileged containers. But
since our `/sysroot` mount wasn't private we also got a copy there.
We should have done this from the very start - it makes `findmnt` way, way less
ugly and is just the obviously right thing to do, will possibly create world
peace etc.
Closes: #1438
Approved by: rhvgoyal
Currently we were parsing `opt_filename` twice...I dug through
the history a bit and it looks like it may have been an accident
from refactoring.
What we're fixing here concretely is that using relative subdirectories
like `--filename somesubdir/foo` broke because we were incorrectly
passing the `somesubdir/` again.
Closes: #1423Closes: #1427
Approved by: jlebon
Prep for further work here. This diff is a bit noisy for the delta bits because
the identation was off originally as well.
Closes: #1424
Approved by: jlebon
In particular I'd like to get the `--copyup` changes out for an rpm-ostree
release that will use them. But there are other good changes here, and let's
keep up a regular release train 🚄 in general.
Closes: #1413
Approved by: jlebon
Previously we were doing e.g. `ot_util_filename_validate()` specifically inline
in dirtree objects, but only *after* writing them into the staging directory (by
default). In (non-default) cases such as not using a transaction, such an object
could be written directly into the repo.
A notable gap here is that `pull-local --untrusted` was *not* doing
this verification, just checksums. We harden that (and also the
static delta writing path, really *everything* that calls
`ostree_repo_write_metadata()` to also do "structure" validation
which includes path traversal checks. Basically, let's try hard
to avoid having badly structured objects even in the repo.
One thing that sucks in this patch is that we need to allocate a "bounce buffer"
for metadata in the static delta path, because GVariant imposes alignment
requirements, which I screwed up and didn't fulfill when designing deltas. It
actually didn't matter before because we weren't parsing them, but now we are.
In theory we could check alignment but ...eh, not worth it, at least not until
we change the delta compiler to emit aligned metadata which actually may be
quite tricky. (Big picture I doubt this really matters much right now
but I'm not going to pull out a profiler yet for this)
The pull test was extended to check we didn't even write a dirtree
with path traversal into the staging directory.
There's a bit of code motion in extracting
`_ostree_validate_structureof_metadata()` from `fsck_metadata_object()`.
Then `_ostree_verify_metadata_object()` builds on that to do checksum
verification too.
Closes: #1412
Approved by: jlebon
While we do protect against path traversal during pull, let's also validate
during checkout; it's a cheap operation and provides good last-mile protection.
Closes: #1412
Approved by: jlebon
I was reading about a recent security issue with both EMC and VMWare:
https://arstechnica.com/information-technology/2018/01/emc-vmware-security-bugs-throw-gasoline-on-cloud-security-fire/
It's a classic path traversal problem, and that made me think more about our
handling of this in libostree. Fortunately of course, not being new to
this rodeo, long ago I *did* consider path traversal. Inside the pull
code, we call `ot_util_filename_validate()`. Also, `fsck` does this too.
I have further followups here, but let's add some test cases for this. I crafted
a repository with a `../` in a dirtree object by patching libostree to inject
it, and that's included as a tarball.
This patch covers the two cases where we do already have checks; pulling
via HTTP, and in `fsck`.
Closes: #1412
Approved by: jlebon
Apparently there testing systems that literally install *all*
packages. Having `ostree-grub2` currently causes grub2 to fail
on a non-ostree managed system. Let's just gracefully exit
if there's no system repository.
https://bugzilla.redhat.com/show_bug.cgi?id=1532668Closes: #1399
Approved by: jlebon
This tripped up the `docbook-dtds` `%post` in my experiments
with doing rpm-ostree for buildroots.
I cloned and built [xfstests](https://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git)
but haven't yet investigated actually running it.
In the meantime let's do the obvious fix here; we need to distinguish
between "copyup enabled" and "actually did a copyup" in the open path
at least, since if we didn't do a copyup we don't need to re-open.
Closes: #1396
Approved by: jlebon
I didn't dive into this too much, it looks like something in rust changed that
broke our build. Probably libstd gained a dependency on `-ldl` or so, and that's
handled by cargo? Anyways linking against it isn't going to hurt.
Closes: #1391
Approved by: smcv
Will debug at some point but for now let's
unblock other things.
```
/usr/bin/ld: /var/tmp/checkout/target/release/libbupsplit_rs.a(bupsplit_rs-db7d02fa07221ce3.bupsplit_rs0.rust-cgu.o): undefined reference to symbol 'dladdr@@GLIBC_2.2.5'
```
Closes: #1387
Approved by: jlebon
Let's do a new release with the locking preview, the http2 disable options and
other misc bugfixes to close out the year.
Closes: #1386
Approved by: jlebon
See discussion in https://bugzilla.gnome.org/show_bug.cgi?id=791622
This is what e.g. systemd, the Linux kernel, and lots of other projects do. It's
astonishingly hard to reliably get right; the optimization IMO only really
matters for truly high performance inner loops, but if you're doing
that kind of stuff today you're probably doing it on a GPU anyways.
Closes: #1384
Approved by: pwithnall
We had this basically forced on in the CLI; down the line I'd really like to
make this an API option to commit or so, but given that we found a use case in
the rpm-ostree test suite for "unbound" commits, let's support creating them
from the cmdline.
See: https://github.com/ostreedev/ostree/pull/1379Closes: #1380
Approved by: jlebon
It'd all be really nice if there was some sort of `O_TMPFILE` for symlinks, but
anyways the way we were doing a generic "make temp file than rename" actually
defeats some of the point of `O_TMPFILE`. It's now fully safe to do "copy to
self", so let's do that for regfiles.
Closes: #1378
Approved by: jlebon
Today the rpm-ostree test suite uses `refs --create` to save
commits. I think this is a legitimate use case, and other
people may be doing something similar.
On the other hand, I think we should probably be changing the rpm-ostree test
suite to create "unbound" commits. But let's be maximially compatible here since
we hit a real-world case where something needed to change.
Closes: #1379
Approved by: pwithnall
For the [rpm-ostree jigdo ♲📦](https://github.com/projectatomic/rpm-ostree/issues/1081) work.
We're basically doing "pull" via a non-libostree mechanism, and this
should be fully supported. As I mentioned earlier we should try to
have `ostree-repo-pull.c` only use public APIs; this gets us closer
to that.
Closes: #1376
Approved by: jlebon
I want some time to play with this more with different callers and work through
test scenarios. Let's disable the locking by default for now, but make it easy
to enable.
Closes: #1375
Approved by: jlebon
Mostly adding this for use in test cases; it allows us to add e.g.
integers, and we need to deal with byteswapping those.
Someone mind also find it useful to add fully structured metadata, although most
of those users should be using a real language and not shell script.
Closes: #1372
Approved by: jlebon
In the non-`CONSUME` path for regfiles (which happens currently for
`bare-user`), we go to a lot of contortions to make an "object stream",
only to immediately parse it again.
Fixing this will also enable the `G_IS_FILE_DESCRIPTOR_BASED()` fast path in
commit, since the input stream will actually reference the file descriptor and
not be an `_OstreeChainInputStream`.
There's a slight concern here in that we're no longer checksumming *literally*
the object stream passed in for the stream case, but I mention in the comment,
the data should be the same, and if it's not somehow we're not adding risk,
since the checksum is still covering the data we actually care about.
Prep for further changes to break up the `write_content_object()` path into
separate paths for archive, as well as regfile vs symlink in non-archive.
Closes: #1371
Approved by: jlebon
A while ago I did `truncate -s 0 /path/to/repo/00/123.commit`, and expected a
checksum error, but I actually got a validation error due to us loading the
commit into a variant and trying to parse out the parent checksum, etc.
I first started by changing the `load_and_fsck_one_object()` function to
checksum before loading, but the problem is that we do a traverse of all objects
first. Fixing this is going to require an `OSTREE_REPO_COMMIT_TRAVER_FLAG_FSCK`
or something.
In the meantime at least though, let's add a public API to fsck a single object
which *does* checksum cleanly before parsing the object, and change the `fsck`
command to use it.
We then change the fsck binary to do this while iterating over the refs
and finding the commit object. This way we'll at least get a checksum
first for commit objects, even if not dirtree/dirmeta.
Closes: #1364
Approved by: jlebon
One major thing we can do to speed up local commits is multithreading. In
preparation for that, split up the recursion function so that the subdirectory
case is separate from the content (regfile/symlink) case. Then for non-subdirs,
we can easily peel off worker threads and gather the final checksums and update
the mtree from the main thread.
The diff here looks large but it's pretty straightforward; amazingly this change
compiled the very first time I tried it!
Closes: #1365
Approved by: jlebon
This seems to work around
https://github.com/ostreedev/ostree/issues/1362
Though I'm not entirely sure why yet. But at least with this it'll be easier for
people to work around things locally.
Closes: #1368
Approved by: jlebon
Test that concurrent commits and prunes can succeed. Mostly this is a
check that the new locking works correctly and the concurrent processes
will properly wait until they've acquired the appropriate repository
lock.
Closes: #1343
Approved by: cgwalters
I was getting a bare `error: Creating temp file: No such file or directory` when
debugging `test-concurrency.py`; with this I get
`error: Writing content object: Creating temp file: No such file or directory`
which helps me pin it down.
Closes: #1343
Approved by: cgwalters
For rpm-ostree I'd like to do importing in parallel with threads; the code is
*almost* ready for that except today it calls
`ostree_repo_transaction_set_ref()`.
Looking at the code, there's really a "transaction" struct here,
not just stats. Let's lift that struct out, and move the refs
into it under the existing lock.
Clarify the documentation around multithreading for various functions.
Closes: #1358
Approved by: jlebon
Time to cut a new release, we've got the libcurl cleanup ordering patch which
several people have hit, along with safe early fixes for tmpdir cleanup. Let's
try to land the locking PR early next cycle.
Closes: #1359
Approved by: jlebon
I was seeing the `Writing OSTree commit...` phase of rpm-ostree
being very slow lately. This turns out to be more fallout from
https://github.com/ostreedev/ostree/pull/1170
AKA commit: 8fe4536
Loading the xattrs is slow on my system (F27AW, XFS+LVM, NVMe). I haven't fully
traced through why, but AIUI at least on XFS the xattrs are often stored outside
of the inode so it's a little bit like doing an `open()+read()`. Plus there's
the LSM overhead, etc.
The thing is that for rpm-ostree's package layering use case, we
basically always want to treat the on-disk state as canonical. (There's
a subtle case here if one does overrides for something that contains
policy but we'll fix that).
Anyways, so we're in a state now where we do the slow but correct thing by
default, which seems sane. But let's allow the app to opt-in to telling us
"really trust devino". The difference between a `stat()` + hash table lookup
versus the full xattr load on my test case of `rpm-ostree install
./tree-1.7.0-10.fc27.x86_64.rpm` is absolutely dramatic; consistently on the
order of 10s without this support, and <1s with (800ms).
Closes: #1357
Approved by: jlebon
This squashes the last race condition I was actively hitting while running
`test-concurrency.py` in a loop. The race is when process A finds a tmpdir to
reuse, and goes to lock it. Meanwhile process B deletes it and unlocks the lock.
Process A then succeeds at grabbing a lock, but the tmpdir is deleted.
Closes: #1352
Approved by: dbnicholson
Previously we'd delete the tmpdir in `rename_pending_loose_objects()`
but do the unlock inside `ostree_repo_commit_transaction()`. Move
them into the same place in the latter function for consistency.
Doesn't fix anything, just a cleanup while reading the code and
working on `test-concurrency.py`.
Closes: #1352
Approved by: dbnicholson
Prep for future work here; let's cleanly separate the path for cleaning up the
txn staging directories from the code that cleans up "other stuff". Currently
only the former case uses the `GLnxLockFile` etc.
Closes: #1352
Approved by: dbnicholson
This closes a race condition I was seeing with `test-concurrency.py`. If we
don't have `O_TMPFILE` (or for symlinks) we'll create temporary files;
previously these would be subject to the date-based pruning because we set the
timestamp to 0 for objects.
Having our temporary files also in the txn staging dir ensures that they're
covered by the locking we do for that directory, and it's also generally cleaner
since the lifecycle of all the temporary data for a txn is in one place.
Closes: #1352
Approved by: dbnicholson
I was running this recently to test the last delta write changes, and this
helps. We should add an option to repo-init to make this easier at some point.
Closes: #1356
Approved by: jlebon
This lowers into the commit core what the static delta code
was doing, and improves the API.
The bigger picture issue is that for writing large files, our current "pull" API
where the caller provides a `GInputStream` is very awkward in some scenarios.
For example, we have a whole "libarchive input stream" that is a ~200 line
GObject that boils down to wrapping `archive_read_data()`.
This came more to a head when I was working on rpm-ostree jigdo since I had to
copy that object.
One step we can take after this is to further split `write_content_object()`
into a "write symlink or archive object" versus "write bare content object"
(it already has a mess of conditionals) and teach the latter case to call
this.
The eventual goal here is to make this API public.
Closes: #1355
Approved by: jlebon
For situations where fsync is disabled, there's basically
no reason to do the whole "staging directory" dance. Just
write directly into the repo.
Today I use `fsync=false` for my build/cache repos.
I briefly considered not allocating a tmpdir at all
in this case, but we actually do want the txn tmpdir
for the non-`O_TMPFILE` case.
Part of https://github.com/ostreedev/ostree/issues/1184Closes: #1354
Approved by: giuseppe
They don't play nicely currently with HTTP2 where we may
have lots of requests queued.
https://github.com/ostreedev/ostree/issues/878#issuecomment-347228854
In practice anyways I think issues here are better solved on a higher level -
e.g. apps today can use an overall timeout on pulls and if they exceed the limit
set the cancellable.
Closes: #1349
Approved by: jlebon
Fedora switched to 'xz' compress kernel modules, and recently
[RHEL7 did too](https://bugzilla.redhat.com/show_bug.cgi?id=1367496).
This compression defeats bsdiff.
While we have a "rollsum-able" test, we don't have a "bsdiff-able" test as it'd
be very expensive (we'd have to bsdiff, then apply it and compare the result).
Let's do the tactical quick fix here and just not try to delta files ending in
`.xz.`. This avoids us using bsdiff pointlessly for over 4000 files, which is
quite a notable speed increase for generating deltas.
Closes: #1333
Approved by: jlebon
A tricky thing here that caused this to go past a lot of our tests
is that the code was mostly OK if there was an available delta from
an older commit. But this case broke if we e.g. had a new OS
deployment and did a `--require-static-deltas` pull, i.e. the initial
state.
I cleaned up our "find static delta state" function to return an enumeration,
and extended it with an "already have the commit" state. A problem
I then hit is that we've historically fetched detached metadata for
non-delta pulls, even if the commit hasn't changed. I decided not to
do that for `--require-static-deltas` pulls for now; otherwise the
code gets notably more complex.
Closes: https://github.com/ostreedev/ostree/issues/1321Closes: #1323
Approved by: jlebon
The main thing here is that a ton of stuff has happened in gnulib since we
imported `parse-datetime.y`. I cherry-picked a little bit of it, but that
upstream doesn't seem to build with `-Wundef`, so I just deleted some hunks.
(Note I reindented the warnings consistently)
Update submodule: libglnx
Closes: #1320
Approved by: jlebon
It's the slowest part, let's show admins something. This "update every 10%" code
was copied from the fsck command; obviously a better approach would be "progress
every N seconds" but doing that somewhat accurately requires making things
async; not worth it here yet.
Closes: #1314
Approved by: jlebon
We never ended up using this, and I'm going to revisit this in another patch
with a different approach that has useful *content* and not just a lot of files.
Closes: #1314
Approved by: jlebon
I didn't fully spelunk this, but from what `static-delta-generate-crosscheck.sh`
had, we appeared to be doing this before, and it's clearly useful for local
testing rather than needing to spin up a HTTP server.
Closes: #1313
Approved by: jlebon
First, the manual crosscheck script bitrotted; it got caught up
in the "use libtest repo creation wrapper" bit, and also it
seems like at some point `pull --require-static-deltas` changed
meaning when dealing with `file:///` repos. I have more work to
unwind that.
Next, I'm seeing a delta failure which looks like a static delta
miscompilation with rollsums; change the compiler to print out
the source object too, which helped me debug this.
And finally in the processing code, fix incorrect error prefixing, which was
misleading.
Closes: #1311
Approved by: ashcrow
Fixes: 93457071cb "lib/deltas: Use pread() instead of lseek()+read()"
Caught this when trying to test alex's patch locally. I am going to review our
static delta pulls and try to get something more comprehensive locally. But in
the meantime this patch is clearly right.
Closes: #1312
Approved by: jlebon
This makes the code nicer too. Properly unit testing this though really wants
like a whole set of stuff around parent repos...but we do have coverage of the
non-parent path in the current pull tests.
Closes: https://github.com/ostreedev/ostree/issues/1306Closes: #1308
Approved by: alexlarsson
This is more subtle fallout from:
https://github.com/ostreedev/ostree/pull/1170
AKA commit: 8fe4536257
Before, if we found a devino cache hit, we'd use it unconditionally.
Recall that `bare-user` repositories are very special in that they're the only
mode where the on disk state ("physical state") is not the "real" state. The
latter is stored in the `user.ostreemeta` xattr. (`bare-user` repos are also
highly special in that symlinks are regular files physically, but that's not
immediately relevant here).
Since we now have `bare-user-only` for the "pure unprivileged container" case,
`bare-user` should just be used for "OS builds" which have nonzero uids (and
possibly SELinux labels etc.)
In an experimental tool I'm writing "skopeo2ostree" which imports OCI images
into refs, then squashes them together into a single final commit, we lost the
the `81` group ID for `/usr/libexec/dbus-1/dbus-daemon-launch-helper`.
This happened because the commit code was loading the "physical" disk state,
where the uid/gid are zero because that's the uid I happened to be using. We
didn't just directly do the link speedup because I was using `--selinux-policy`
which caused the xattrs to change, which caused us to re-commit objects from the
physical state.
The unit test I added actually doesn't quite trigger this, but I left
it because "why not". Really testing this requires the installed test
which uses SELinux policy from `/`.
The behavior without this fix looks like:
```
-00755 0 0 12 { [(b'user.ostreemeta', [byte 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x51, 0x00, 0x00, 0x81, 0xed]), (b'security.selinux', b'system_u:object_r:lib_t:s0')] } /usr/lib/dbus-daemon-helper
```
which was obviously totally broken - we shouldn't be picking up the
`user.ostreemeta` xattr and actually committing it of course.
Closes: #1297
Approved by: jlebon
This is a freeform string useful to track/display when a commit is "derived"
from some other format. For example, in the rpm-ostree test we make a
`vmcheck` ref that conceptually overlays the default ref like
`fedora-atomic:fedora/26/x86_64/atomic-host`.
My current patch sets the source title to e.g.
"Dev overlay on fedora-atomic:fedora/26/x86_64/atomic-host".
Another case I'm working on now is importing OCI images to use
as host images. For that case, the source title is
With this patch we could then set the original OCI image name + tag
as the source name, like:
"oci:cgwalters/demo-custom-fedora-atomic-host:26".
Closes: #1296
Approved by: jlebon
Pull out the commit metadata explicitly; still just rendering the version, but
this is prep for rendering other metadata keys.
Closes: #1296
Approved by: jlebon
I was trying to use this with pygobject for an OCI+ostree project, and pygobject
rejected simply assigning to the field (understandably, since it can't bind the
lifetime together).
Add a wrapper function, which is still unsafe, but hides that unsafety
where most people shouldn't find it. And if they do...well, sorry,
Rust wasn't invented when ostree was started.
Closes: #1295
Approved by: pwithnall
I'm playing around with some ostree ⇔ OCI/Docker bits, and ran
into this while importing an OCI image that built from the Fedora
base image where `/home` is a regular directory, and I added a layer
that did the ostree bits of moving it to `/var` and leaving a symlink.
OCI/Docker supports this. Now since "process whiteouts" is really the
"enable OCI/Docker" mode, let's only replace dirs if that's enabled.
This leaves the `UNION_FILES` targeted for its original use case
which is unioning components/packages. (Although that use case itself
is now a bit superceded by `UNION_IDENTICAL`, but eh).
Closes: #1294
Approved by: jlebon
Change the regexp for validating refs to require at least one letter or digit
before allowing the other special chars in the set `[.-_]`. Names that start
with `.` are traditionally Unix hidden files; let's ignore them under the
assumption they're metadata for some other tool, and we don't want to
potentially conflict with the special `.` and `..` Unix directory entries.
Further, names starting with `-` are problematic for Unix cmdline option
processing; there's no good reason to support that. Finally, disallow `_` just
on general principle - it's simpler to say that ref identifiers must start with
a letter or digit.
We also ignore any existing files (that might be previously created refs) that
start with `.` in the `refs/` directory - there's a Red Hat tool for content
management that injects `.rsync` files, which is why this patch was first
written.
V1: Update to ban all refs starting with a non-letter/digit, and
also add another call to `ostree_validate_rev` in the pull
code.
Closes: https://github.com/ostreedev/ostree/issues/1285Closes: #1286
Approved by: jlebon
The way `_ostree_repo_open_content_bare()` did both looking for the object and
possibly creating a new fd was just weird and inconsistent with e.g. the pull
code where we always call `has_object()` first.
Just call `has_object()` in the delta paths that used this too, making the
implementation right now a thin wrapper around
`glnx_open_tmpfile_linkable_at()`, but this is prep for a later patch which does
more.
Closes: #1283
Approved by: jlebon
This is another case where making an input stream out of a memory buffer is a
bit silly; just hash the `GBytes` directly.
Closes: #1287
Approved by: jlebon
A side effect of commit 8fe4536257 is that
we started listing all xattrs even for files with device/inode matches;
further, we did that using the dfd/name which means we went through
the `/proc` path, which is slower and uglier.
Noticed this in strace while looking at adoption code.
Closes: #1280
Approved by: jlebon
Colin Walters