8000 core/dracut/ignition-ostree: add ignition-ostree-sysusers service by lucab · Pull Request #774 · coreos/fedora-coreos-config · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

core/dracut/ignition-ostree: add ignition-ostree-sysusers service #774

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub 8000 ? Sign in to your account

Jump to bottom
Closed
wants to merge 3 commits into from
Closed

core/dracut/ignition-ostree: add ignition-ostree-sysusers service #774

wants to merge 3 commits into from

Conversation

lucab
Copy link
Contributor
@lucab lucab commented Dec 11, 2020

This introduces a new ignition-ostree-sysusers.service, which takes
care of poulating users and groups on the target sysroot before the
Ignition files stage.

@lucab lucab requested a review from jlebon December 11, 2020 15:07
@lucab lucab mentioned this pull request Dec 11, 2020
Closed
jlebon
jlebon approved these changes Dec 11, 2020
View reviewed changes
Copy link
Member
@jlebon jlebon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me! With this, are you able to refer to users/groups created via systemd-sysusers in your Ignition config?

@lucab
Copy link
Contributor Author
lucab commented Dec 11, 2020 •
edited
Loading

@jlebon yes, although testing this right now is skewed due to the pending dnsmasq fix at https://src.fedoraproject.org/rpms/dnsmasq/pull-request/6. I can add a regression test on top once that RPM is fixed.

On a custom image without dnsmasq, the result looks like this:

# grep zincati /usr/lib/passwd /etc/passwd
/etc/passwd:zincati:x:979:979:Zincati user for auto-updates:/:/usr/sbin/nologin

# jq .storage.files[0] /run/ignition.json 
{
  "group": {},
  "path": "/etc/zincati/config.d/empty.toml",
  "user": {
    "name": "zincati"
  },
  "contents": {
    "source": "data:,%23%20Empty%20placeholder%0A",
    "verification": {}
  },
  "mode": 420
}

# ls -laZ /etc/zincati/config.d/empty.toml
-rw-r--r--. 1 zincati root system_u:object_r:etc_t:s0 20 Dec 11 16:22 /etc/zincati/config.d/empty.toml

@lucab lucab force-pushed the ups/dracut-sysuers branch from 2f885b7 to a7e1233 Compare December 14, 2020 10:24
@lucab lucab changed the title [RFC] core/dracut/ignition-ostree: add ignition-ostree-sysusers service core/dracut/ignition-ostree: add ignition-ostree-sysusers service Dec 14, 2020
@lucab
Copy link
Contributor Author
lucab commented Dec 14, 2020

Added a test, dropped RFC, this should be ready to merge.

@lucab
Copy link
Contributor Author
lucab commented Dec 14, 2020

/cc @travier FYI this touches into sysusers.d topic, but should be already fitting into future reworks.

jlebon
jlebon previously approved these changes Dec 15, 2020
View reviewed changes
@travier
Copy link
Member
travier commented Dec 15, 2020 •
edited
Loading

As far as I understand, we could directly call systemd-sysusers --root=/sysroot. Any reason not to do that? See: #774 (comment)

@travier
Copy link
Member
travier commented Dec 15, 2020

This looks good to me! With this, are you able to refer to users/groups created via systemd-sysusers in your Ignition config?

I don't think this will work as this would require splitting the Ignition stage that creates users/groups from the files stage to make a similar call like the one included here to create the users. This will only setup users/groups that are declared in sysusers files in the ostree commit but not included in passwd/shadow/group.

@jlebon
Copy link
Member
jlebon commented Dec 15, 2020

This looks good to me! With this, are you able to refer to users/groups created via systemd-sysusers in your Ignition config?

I don't think this will work as this would require splitting the Ignition stage that creates users/groups from the files stage to make a similar call like the one included here to create the users. This will only setup users/groups that are declared in sysusers files in the ostree commit but not included in passwd/shadow/group.

Are you talking about the case where the Ignition config adds sysusers drop-in configs and in the same Ignition config there are files which reference those users/groups as owners? Hmm, do we really need to handle this? Ignition already provides a declarative way of creating system users/groups and it creates users/groups before creating files.

@travier
Copy link
Member
travier commented Dec 15, 2020

I don't think this will work as this would require splitting the Ignition stage that creates users/groups from the files stage to make a similar call like the one included here to create the users. This will only setup users/groups that are declared in sysusers files in the ostree commit but not included in passwd/shadow/group.

Are you talking about the case where the Ignition config adds sysusers drop-in configs and in the same Ignition config there are files which reference those users/groups as owners? Hmm, do we really need to handle this? Ignition already provides a declarative way of creating system users/groups and it creates users/groups before creating files.

Yes. From coreos/fedora-coreos-tracker#155, one of the option is to move to a fully sysusers defined list of users for the system for rpm-ostree & Ignition. This option could enable first boot removal among other things.

@travier
Copy link
Member
travier commented Dec 15, 2020 •
edited
Loading

I think we need a larger design discussion for this topic but this has not happened yet as this has not been a priority so far. But this should probably not prevent this PR from being merged.

cgwalters
cgwalters previously approved these changes Dec 15, 2020
View reviewed changes
Copy link
Member
@cgwalters cgwalters left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did it come up to have Ignition have support for this built in? It really feels like we're doing this special thing behind its back here that other Ignition users would want.

Ignition still carries code to use chroot() for getent lookups which is a "poor-man's" containerization.

overlay.d/05core/usr/lib/dracut/modules.d/40ignition-ostree/coreos-sysroot-bwrap
mount --bind /sysroot "${TMP_CHROOT_DIR}"/sysroot
}

run_chrooted_bwrap() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would probably be a good thing to live in rdcore (and in theory in the future it could be a crate shared between rdcore and rpm-ostree).

Not a blocker though.

@cgwalters
Copy link
Member

Also I agree strongly with #774 (comment)

@lucab
Copy link
Contributor Author
lucab commented Dec 16, 2020

@cgwalters it indeed started with coreos/ignition#1153 on Ignition side but got redirected here.

@lucab
core/dracut/ignition-ostree: add a bwrap-in-sysroot helper
36eafff 
This introduces a new `coreos-sysroot-bwrap` helper in initramfs,
for binaries that need to be executed with the final sysroot as
a target, but before the pivot-root happens.
@lucab lucab force-pushed the ups/dracut-sysuers branch from faba078 to b67b155 Compare December 16, 2020 11:35
lucab added 2 commits December 16, 2020 12:45
@lucab
core/dracut/ignition-ostree: add ignition-ostree-sysusers service
b39c055 
This introduces a new `ignition-ostree-sysusers.service`, which takes
care of poulating users and groups on the target sysroot before the
Ignition `files` stage.
@lucab
tests/ignition: check file ownership for system users
dd25144 
This ensure that entries in Ignition configuration can reference
system users even if not present in ostree commit (e.g. `zincati).
@lucab lucab force-pushed the ups/dracut-sysuers branch from b67b155 to dd25144 Compare December 16, 2020 12:47
@jlebon jlebon closed this Dec 17, 2020
@jlebon jlebon reopened this Dec 17, 2020
@coreosbot coreosbot dismissed stale reviews from cgwalters and jlebon via dd25144 July 27, 2021 20:55
@dustymabe
Copy link
Member

bump

@lucab lucab mentioned this pull request Jul 14, 2022
Closed
c4rt0 pushed a commit to c4rt0/fedora-coreos-config that referenced this pull request Mar 27, 2023
@openshift-merge-robot
Merge pull request coreos#774 from cheesesashimi/zzlotnik/fix-cluster…
50cdee6 
…-context

ci: remember test cluster context
@HuijingHei HuijingHei mentioned this pull request Aug 17, 2023
Open
@travier travier mentioned this pull request Aug 22, 2023
Open
@HuijingHei HuijingHei mentioned this pull request Oct 12, 2023
Open
@HuijingHei HuijingHei mentioned this pull request Oct 19, 2023
Open
@lucab lucab closed this Jan 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jlebon jlebon jlebon left review comments

@travier travier travier left review comments

@cgwalters cgwalters cgwalters left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@lucab @travier @jlebon @cgwalters @dustymabe
0