WIP: compose: Add experimental/sysusers option
#1679
Conversation
|
Nice, code so far looks fine to me. In case you want to externalize part of this (now or at some later point), I started collecting other non-FFI systemd things here: https://github.com/lucab/libsystemd-rs/tree/master/src. It's also covered by quickcheck, and we can add the sysusers parser/encoder there if we like to. |
cgwalters
force-pushed
the
sysusers-2
branch
from
e26c35f
to
6e4bfea
Compare
|
☔ The latest upstream changes (presumably a177f55) made this pull request unmergeable. Please resolve the merge conflicts. |
cgwalters
force-pushed
the
sysusers-2
branch
3 times, most recently
from
323bfd9
to
1e2728c
Compare
|
While working on this I realized the next thing we really need is code to walk over the filesystem and verify that everything in I started on that code but will take me a bit to circle back to hacking on this actively. |
|
☔ The latest upstream changes (presumably 25d0213) made this pull request unmergeable. Please resolve the merge conflicts. |
cgwalters
force-pushed
the
sysusers-2
branch
2 times, most recently
from
5bcb580
to
5cfcdf9
Compare
|
OK been working on this slowly in the background...had to redesign things a few times. Here's a FCOS PR to use it: There's...a lot of details here. I think it's close to working. |
|
☔ The latest upstream changes (presumably e7f87b0) made this pull request unmergeable. Please resolve the merge conflicts. |
cgwalters
force-pushed
the
sysusers-2
branch
from
5cfcdf9
to
e7889a6
Compare
|
☔ The latest upstream changes (presumably 3f722b5) made this pull request unmergeable. Please resolve the merge conflicts. |
cgwalters
force-pushed
the
sysusers-2
branch
4 times, most recently
from
acd14df
to
9863d8e
Compare
cgwalters
changed the title
experimental/sysusers option
|
This is still WIP but...I added a new I'd like to consider landing this and making it easier for other people to play with it. There could be more tests...I could work on that. Any other blockers? |
|
how to migrate from old treefile and new with sysusers? I mean - if i create new commit with never rpm-ostree with sysusers and rebase to that tree, does it all? |
I'd wait until we sort this out with Fedora CoreOS. |
|
|
||
| ostreels /usr/lib/sysusers.d/rpmostree-auto.conf | ||
|
|
||
| echo "ok sysusers" |
There was a problem hiding this comment.
Hmm, we should have some basic package layering tests too. E.g. extending the layering-non-root-caps test with packages that drop static entries, dynamic entries, and then use the sysusers macros and drop files owned by the new users/groups. And also a test that uses useradd/groupadd with a specified id.
There was a problem hiding this comment.
Yeah...I hadn't thought hard about how this interacts with layering. But doing so for a minute now, offhand it seems a first cut is to just disable all of this sysusers interception there - we're just replacing the existing server side code.
It probably wouldn't hurt to also consistently generate sysusers entry for client side layering; at least it'd help with the story that one can "reset" /etc.
There was a problem hiding this comment.
The bigger problem though is that at the moment our "compose" tests and "vmcheck" are separate; we hack the vmcheck side sometimes to simulate server side options but this one would be harder.
I think we should brainstorm soon about using cosa in our tests, like we build a new rpm-ostree rpm, drop it in the cosa overrides/rpms dir, do a compose, then boot that and run tests.
(And here we get into the nested virt stuff. Need to unblock that)
There was a problem hiding this comment.
Following up on this, right now we are disabling sysusers interception at package layering time.
| /// | ||
| /// This function takes as input a file descriptor for an O_TMPFILE fd that has | ||
| /// newline-separated arguments. Note that we can get multiple invocations of | ||
| /// useradd/groupadd in a single call. |
There was a problem hiding this comment.
Hmm, this would also be a good test to add (multiple invocations in %pre).
| .arg(Arg::with_name("home-dir").short("d").takes_value(true)) | ||
| .arg(Arg::with_name("comment").short("c").takes_value(true)) | ||
| .arg(Arg::with_name("shell").short("s").takes_value(true)) | ||
| .arg(Arg::with_name("username").takes_value(true).required(true)); |
There was a problem hiding this comment.
Hmm, I'm worried providing useradd and groupadd APIs like this might become a maintenance burden. (Though agreed in the future, packages should be moving to sysusers anyway... but legacy :( ). Even here, e.g. there's a bunch of other flags that we're not supporting, right?
Would it work to just look through the args for the ones we're interested in, and then just pass through the rest to useradd (and drop calling systemd-sysusers manually)? In the end, the crucial bit we're interested in is whether or not the invocation is specifying a specific id or not, right?
There was a problem hiding this comment.
We need to handle our fixed overrides. I guess one way to do that would be to inject the required -u argument instead?
Either way though, we need to handle the arguments.
cgwalters
force-pushed
the
sysusers-2
branch
2 times, most recently
from
19e123d
to
73064d3
Compare
|
☔ The latest upstream changes (presumably d5b9077) made this pull request unmergeable. Please resolve the merge conflicts. |
|
☔ The latest upstream changes (presumably 6b2ac58) made this pull request unmergeable. Please resolve the merge conflicts. |
|
☔ The latest upstream changes (presumably 5f6578e) made this pull request unmergeable. Please resolve the merge conflicts. |
cgwalters
force-pushed
the
sysusers-2
branch
2 times, most recently
from
42b9a47
to
1a372bf
Compare
First, we add support for a new `experimental:` key. Then there's a new `sysusers` key underneath that. When enabled, we drop all of the other previous passwd handling. In practice the only one that was used was having static files. That is a pain to maintain. However, we need to statically assign non-zero uid/gid for any files that come from a base ostree commit. Anything else would mean the user/groups could be unpredictably assigned in different rpm-ostree runs. This code now checks for an errors out on that. In order to convert *fully* to sysusers, we install an interceptor for `useradd/groupadd` that talk back via a pipe to the compose process. These invocations then get translated to drop into a new` `sysusers.d/rpmostree-auto.conf` file. This way we don't need to require that every RPM have ported to sysusers.d. At the end, we drop everything in `/etc/passwd` and `/etc/group` except for the `root:` entries, relying on `systemd-sysusers` to readd everything at boot time. Closes: coreos#49
cgwalters
force-pushed
the
sysusers-2
branch
from
1a372bf
to
b90e0ec
Compare
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: cgwalters The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@cgwalters: PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
Any updates for this? |
|
Updates? I'm currently working on an rpm-ostree distro, and this functionality would be very useful. |
|
@cgwalters: The following tests failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
Prep for #49
The main thing this is starting to implement is support for "intercepting"
useraddetc.