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.

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

WIP: add sysusers to compose #1376

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

WIP: add sysusers to compose #1376

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
62751c1
compose: adds integration parts to compose
peterbaouoft Aug 30, 2018
4bb8db1
app: add temp test command, note, this is mainly for testing
peterbaouoft Aug 31, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 30 additions & 2 deletions src/app/rpmostree-compose-builtin-tree.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1326,15 +1326,43 @@ impl_commit_tree (RpmOstreeTreeComposeContext *self,
if (self->treefile)
{
g_autoptr(GFile) treefile_dirpath = g_file_get_parent (self->treefile_path);
g_autoptr(GPtrArray) sysuser_entries = NULL;
if (!rpmostree_check_passwd (self->repo, self->rootfs_dfd, treefile_dirpath, self->treefile,
self->previous_checksum,
self->previous_checksum, &sysuser_entries,
Copy link
Member

Choose a reason for hiding this comment

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

It seems to me that if we're using sysusers - there's really no reason to go into any of the rpmostree_check_passwd() code. With sysusers, the uids are assigned on the client side, unlike every other approach here. The code here was mostly concerned with comparing from the "previous" values, but conceptually we don't have any previous values.

It seems the main thing we're lifting there is the current values of the passwd/group database.

One thing it's tempting to do here too is add a new systemd-sysusers: true boolean, and then that overrides all of the other legacy stuff. Then here in compose-builtin-tree.c we'd do:

if (self->treefile)
  {
    if (sysusers)
      {
        if (!rpmostree_passwd_sysusers_convert (self->rootfs_fd, cancellable, error))
          return FALSE;
      }
    else
      {
        if (!rpmostree_check_passwd (self->repo, self->rootfs_dfd, ...)
          return FALSE;
      }
   }

or so?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems to me that if we're using sysusers - there's really no reason to go into any of the rpmostree_check_passwd() code. With sysusers, the uids are assigned on the client side, unlike every other approach here.

Ha, I was thinking to propose this approach, but then I figured some packages seem to own their specific uid/gid (Can't seem to find a source, so may be I was wrong). One example i believe is daemon sometimes own uid 2? So, I thought check_passwd and check_group was mainly used for that case (the exact mapping of package assignment).

But if not, then I agree, we don't necessarily need to check sysusers for "previous" values.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

One thing it's tempting to do here too is add a new systemd-sysusers: true boolean, and then that overrides all of the other legacy stuff. Then here in compose-builtin-tree.c we'd do:

Yea, this implementation makes sense, and if we are skipping that, may be we can also skip the sections here too?
https://github.com/projectatomic/rpm-ostree/blob/master/src/libpriv/rpmostree-postprocess.c#L971-L991.

I was thinking earlier if we could work around the sysuser logic a bit so that the migration of passwd needs not to happen (or skipped) in postprocess.

Copy link
Member

Choose a reason for hiding this comment

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

Ha, I was thinking to propose this approach, but then I figured some packages seem to own their specific uid/gid (Can't seem to find a source, so may be I was wrong). One example i believe is daemon sometimes own uid 2?

A bunch of uid/gid values are fixed; see https://fedoraproject.org/wiki/Packaging:UsersAndGroups and note https://pagure.io/setup/blob/master/f/uidgid

Is that what you're thinking of?

But that...oh wait, argh, I think this calls into question our entire approach here 😦 - I'll comment below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A bunch of uid/gid values are fixed; see https://fedoraproject.org/wiki/Packaging:UsersAndGroups and note https://pagure.io/setup/blob/master/f/uidgid
Is that what you're thinking of?

Yup. :D

cancellable, error))
return glnx_prefix_error (error, "Handling passwd db");

if (!rpmostree_check_groups (self->repo, self->rootfs_dfd, treefile_dirpath, self->treefile,
self->previous_checksum,
self->previous_checksum, &sysuser_entries,
cancellable, error))
return glnx_prefix_error (error, "Handling group db");

if (sysuser_entries)
{
g_autofree gchar *sysuser_content = NULL;
struct stat empty_stat;
const char *sysuser_folder = "usr/lib/sysusers.d";

if (!rpmostree_passwd_sysusers2char (sysuser_entries,
&sysuser_content, error))
return glnx_prefix_error (error, "Handling sysuser conversion");

/* Do a deletion of original /usr/lib/sysusers.d/ to
* avoid duplication of existing sysuser entries */
if (fstatat (self->rootfs_dfd, sysuser_folder, &empty_stat, AT_SYMLINK_NOFOLLOW) == 0)
if (!glnx_shutil_rm_rf_at (self->rootfs_dfd, sysuser_folder, cancellable, error))
return FALSE;

/* Creation of the converted sysuser entries into a conf file in
* sysuser folder */
if (!glnx_ensure_dir (self->rootfs_dfd, sysuser_folder, 0755, error))
return FALSE;
if (!glnx_file_replace_contents_at (self->rootfs_dfd, "usr/lib/sysusers.d/rpm-ostree-base.conf",
(guint8*)sysuser_content, -1,
GLNX_FILE_REPLACE_NODATASYNC,
cancellable, error))
return FALSE;
}
}

/* See comment above */
Expand Down
25 changes: 19 additions & 6 deletions src/libpriv/rpmostree-passwd-util.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,7 @@ rpmostree_check_passwd_groups (gboolean passwd,
GFile *treefile_dirpath,
JsonObject *treedata,
const char *previous_commit,
GPtrArray **out_sysuser_entries,
GCancellable *cancellable,
GError **error)
{
Expand Down Expand Up @@ -386,7 +387,7 @@ rpmostree_check_passwd_groups (gboolean passwd,
return TRUE; /* Note early return */
else if (g_str_equal (chk_type, "previous"))
; /* Handled below */
else if (g_str_equal (chk_type, "file"))
else if (g_str_equal (chk_type, "file") || g_str_equal (chk_type, "sysusers"))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For sysusers option, I think it might be good if we also check passwd/group as b4, which is why I added those lines. But let me know what you think about this. =)

{
direct = _rpmostree_jsonutil_object_require_string_member (chk,
"filename",
Expand Down Expand Up @@ -491,7 +492,7 @@ rpmostree_check_passwd_groups (gboolean passwd,
return TRUE;
}
}
else if (g_str_equal (chk_type, "file"))
else if (g_str_equal (chk_type, "file") || g_str_equal (chk_type, "sysusers"))
{
old_path = g_file_resolve_relative_path (treefile_dirpath, direct);
old_contents = glnx_file_get_contents_utf8_at (AT_FDCWD, gs_file_get_path_cached (old_path), NULL,
Expand All @@ -500,7 +501,7 @@ rpmostree_check_passwd_groups (gboolean passwd,
return FALSE;
}

if (g_str_equal (chk_type, "previous") || g_str_equal (chk_type, "file"))
if (g_str_equal (chk_type, "previous") || g_str_equal (chk_type, "file") || g_str_equal (chk_type, "sysusers"))
{
if (passwd)
old_ents = rpmostree_passwd_data2passwdents (old_contents);
Expand Down Expand Up @@ -681,6 +682,16 @@ rpmostree_check_passwd_groups (gboolean passwd,
}
}

/* Now, at the end of checking, if all goes correctly, we put entries into
* the array of sysusers */
if (g_str_equal (chk_type, "sysusers"))
{
if (passwd)
return rpmostree_passwdents2sysusers (new_ents, out_sysuser_entries, error);
else
return rpmostree_groupents2sysusers (new_ents, out_sysuser_entries, error);
}

return TRUE;
}

Expand All @@ -693,11 +704,12 @@ rpmostree_check_passwd (OstreeRepo *repo,
GFile *treefile_dirpath,
JsonObject *treedata,
const char *previous_commit,
GPtrArray **out_sysuser_entries,
GCancellable *cancellable,
GError **error)
{
return rpmostree_check_passwd_groups (TRUE, repo, rootfs_fd, treefile_dirpath,
treedata, previous_commit,
treedata, previous_commit, out_sysuser_entries,
cancellable, error);
}

Expand All @@ -710,11 +722,12 @@ rpmostree_check_groups (OstreeRepo *repo,
GFile *treefile_dirpath,
JsonObject *treedata,
const char *previous_commit,
GPtrArray **out_sysuser_entries,
GCancellable *cancellable,
GError **error)
{
return rpmostree_check_passwd_groups (FALSE, repo, rootfs_fd, treefile_dirpath,
treedata, previous_commit,
treedata, previous_commit, out_sysuser_entries,
cancellable, error);
}

Expand Down Expand Up @@ -1032,7 +1045,7 @@ _data_from_json (int rootfs_dfd,
if (!chk_type)
return FALSE;

if (!g_str_equal (chk_type, "file"))
if (!g_str_equal (chk_type, "file") && !g_str_equal (chk_type, "sysusers"))
return TRUE;

const char *filename =
Expand Down
2 changes: 2 additions & 0 deletions src/libpriv/rpmostree-passwd-util.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ rpmostree_check_passwd (OstreeRepo *repo,
GFile *treefile_path,
JsonObject *treedata,
const char *previous_commit,
GPtrArray **out_sysuser_entries,
GCancellable *cancellable,
GError **error);

Expand All @@ -39,6 +40,7 @@ rpmostree_check_groups (OstreeRepo *repo,
GFile *treefile_path,
JsonObject *treedata,
const char *previous_commit,
GPtrArray **out_sysuser_entries,
GCancellable *cancellable,
GError **error);

Expand Down
24 changes: 19 additions & 5 deletions src/libpriv/rpmostree-postprocess.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -982,9 +982,23 @@ rpmostree_postprocess_final (int rootfs_dfd,
cancellable, error))
return FALSE;

/* NSS configuration to look at the new files */
if (!replace_nsswitch (rootfs_dfd, cancellable, error))
return glnx_prefix_error (error, "nsswitch replacement");
gboolean is_sysuser_enabled = FALSE;
if (json_object_has_member (treefile, "check-passwd"))
{
JsonObject *chk = json_object_get_object_member (treefile, "check-passwd");
if (!chk)
return glnx_throw (error, "%s is not an object", "check-passwd");
const char *chk_type = _rpmostree_jsonutil_object_require_string_member (chk, "type", error);
if (!chk_type)
return FALSE;
is_sysuser_enabled = g_str_equal (chk_type, "sysusers");
}

/* Here, we check if sysuser option is enabled, if it is not,
* We will be writing NSS configuration to look at the new files */
if (!is_sysuser_enabled)
if (!replace_nsswitch (rootfs_dfd, cancellable, error))
return glnx_prefix_error (error, "nsswitch replacement");

if (selinux)
{
Expand Down Expand Up @@ -1922,10 +1936,10 @@ count_filesizes (int dfd,
GError **error)
{
g_auto(GLnxDirFdIterator) dfd_iter = { 0, };

if (!glnx_dirfd_iterator_init_at (dfd, path, TRUE, &dfd_iter, error))
return FALSE;

while (TRUE)
{
struct dirent *dent = NULL;
Expand Down