Skip to content
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.

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? Sign in to your account

tests/ignition: check file ownership for system users #2670

Open
wants to merge 2 commits into
base: testing-devel
Choose a base branch
from

Conversation

HuijingHei
Copy link
Member

Based on #774

This ensure that entries in Ignition configuration can reference
system users even if not present in ostree commit (e.g. `zincati).

(cherry picked from commit dd25144)
@HuijingHei
Copy link
Member Author

Check the test is passed without ignition-ostree-sysusers.service in #774, still not quite sure where the fix is.

$ kola run -E fedora-coreos-config --qemu-image rhcos-414.92.202310100209-0-qemu.x86_64.qcow2 ext.fedora-coreos-config.ignition.sysusers
=== RUN   ext.fedora-coreos-config.ignition.sysusers
--- PASS: ext.fedora-coreos-config.ignition.sysusers (41.09s)
PASS, output in _kola_temp/qemu-2023-10-12-1412-79

$ kola run -E fedora-coreos-config --qemu-image fedora-coreos-38.20231009.20.0-qemu.x86_64.qcow2 ext.fedora-coreos-config.ignition.sysusers
=== RUN   ext.fedora-coreos-config.ignition.sysusers
--- PASS: ext.fedora-coreos-config.ignition.sysusers (40.49s)
PASS, output in _kola_temp/qemu-2023-10-12-1420-138

jmarrero
jmarrero previously approved these changes Oct 12, 2023
Copy link
Member

@jmarrero jmarrero left a comment

Choose a reason for hiding this comment

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

lgtm

@travier
Copy link
Member

travier commented Oct 13, 2023

Some context for #774:

The use case where we need to run sysusers in the initramfs (#774) is when you want to declare a system user in a sysusers config file (written by Ignition to the real root) and also include a file in the Ignition config that is owned by that new system user. As the users will not be created in the real root when ignition-files runs, it would fail.

So we need something like #774 to pre-create the users so that Ignition can set the ownership of the files it writes to the real root.


This test looks fine to me but I'm not sure how much we already have testing that in kola.

@HuijingHei HuijingHei force-pushed the ignition-file-ownership-test branch 2 times, most recently from 3af42d3 to 4ebeb00 Compare October 17, 2023 07:45
@HuijingHei
Copy link
Member Author

Thanks @jmarrero @travier for the review, update the test script, might need re-review, thanks!

@travier
Copy link
Member

travier commented Oct 17, 2023

Here is an example config that should fail:

variant: fcos
version: 1.5.0
storage:
  files:
    - path: /etc/foo
      user:
        name: "foo"
      contents:
        inline: |
          # Dummy placeholder
    - path: /etc/sysusers.d/foo.conf
      contents:
        inline: |
          u foo - "Foo user" - -

See: https://www.freedesktop.org/software/systemd/man/sysusers.d.html

@HuijingHei
Copy link
Member Author

Here is an example config that should fail:

variant: fcos
version: 1.5.0
storage:
  files:
    - path: /etc/foo
      user:
        name: "foo"
      contents:
        inline: |
          # Dummy placeholder
    - path: /etc/sysusers.d/foo.conf
      contents:
        inline: |
          u foo - "Foo user" - -

See: https://www.freedesktop.org/software/systemd/man/sysusers.d.html

This should fail with ignition-ostree-sysusers.service refer to #774 (comment)

Use `dnsmasq` instead, as rhcos also includes this user.
@jlebon
Copy link
Member

jlebon commented Oct 23, 2023

So the issue here is that the sysusers.d dropin gets written by the files stage, so it's not yet present when the new ignition-ostree-sysusers.service unit in #2679 runs.

I commented in coreos/fedora-coreos-tracker#155 (comment) to try to keep design discussions there.

@HuijingHei
Copy link
Member Author

Thanks @jlebon @travier for the reply.

Does this script make sense?
Can I understand that we support that the entries in Ignition configuration can reference the existed system users (see doc)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants