Skip to content

OCPBUGS-64841: Ensure 'containers' user & group are part of the image #1917

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

Closed
travier wants to merge 1 commit into from
+32 −0
Closed

OCPBUGS-64841: Ensure 'containers' user & group are part of the image #1917

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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: 32 additions & 0 deletions packages-openshift.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,38 @@ postprocess:
fi
fi

- |
#!/usr/bin/env bash
set -xeo pipefail
# Ensure that the containers user & group are created as part of the image.
# We can not move users/groups from the image to dynamically created ones
# until we fix https://github.com/bootc-dev/bootc/issues/1179.
# See https://redhat.atlassian.net/browse/OCPBUGS-64841 and commit message
# for the full details.

# Only do that when doing a container build
if [[ -f /run/.containerenv ]] && [[ -f /usr/lib/sysusers.d/crio.conf ]]; then
Copy link
Copy Markdown
Member

@dustymabe dustymabe Mar 25, 2026

Choose a reason for hiding this comment

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

In what situation would we ever not be running in a container env?

Suggested change
if [[ -f /run/.containerenv ]] && [[ -f /usr/lib/sysusers.d/crio.conf ]]; then
if [[ -f /usr/lib/sysusers.d/crio.conf ]]; then

Copy link
Copy Markdown
Member Author

@travier travier Mar 25, 2026

Choose a reason for hiding this comment

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

I was conservative and copied the checks from above. I'll update both.

Copy link
Copy Markdown
Member

@jlebon jlebon Mar 25, 2026

Choose a reason for hiding this comment

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

Yeah the check above dates back from when we supported both layered and base composes for the node image.

Comment on lines +142 to +143
Copy link
Copy Markdown
Member

@dustymabe dustymabe Mar 25, 2026

Choose a reason for hiding this comment

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

We only ever run in a container environment.

Suggested change
# Only do that when doing a container build
if [[ -f /run/.containerenv ]] && [[ -f /usr/lib/sysusers.d/crio.conf ]]; then
if [[ -f /usr/lib/sysusers.d/crio.conf ]]; then

Am I missing something?

Copy link
Copy Markdown
Member

@dustymabe dustymabe Mar 25, 2026

Choose a reason for hiding this comment

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

Also, in what situations would crio.conf not exist? I'm thinking we should fail if crio.conf doesn't exist.

Copy link
Copy Markdown
Member Author

@travier travier Mar 25, 2026

Choose a reason for hiding this comment

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

Probably should fail indeed.

# First, cleanup the broken entries from /etc/passwd|group|shadow|gshadow
sed -i "/^containers:/d" /etc/{passwd,group,shadow,gshadow}
Comment on lines +144 to +145
Copy link
Copy Markdown
Member

@dustymabe dustymabe Mar 25, 2026

Choose a reason for hiding this comment

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

This contradicts your statement in the description of this PR:

Starting with OCP 4.19, new nodes start with no containers user/group defined (either in /usr/ or /etc)

If that is true then containers: should never be defined here.

Copy link
Copy Markdown
Member Author

@travier travier Mar 25, 2026

Choose a reason for hiding this comment

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

New nodes start with a pure RHEL boot image that does not have CRI-O installed. This is the container image with CRI-O installed.

Copy link
Copy Markdown
Member

@dustymabe dustymabe Mar 25, 2026

Choose a reason for hiding this comment

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

just for my understanding, since there are a lot of pieces here. Today we have:

  1. RHEL CoreOS Base Image build (no crio)
  2. OpenShift Node Image build FROM:rhel-coreos-base where crio gets installed
    a. RPM transation where crio gets installed
    b. postprocess scripts (not rpm scriptlets) where this sed statement runs

I think what you are saying is that in a. the containers: user gets added to /etc/{passwd,group,shadow,gshadow} and that's what we are cleaning up?

Copy link
Copy Markdown
Member Author

@travier travier Mar 27, 2026

Choose a reason for hiding this comment

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

yes, it gets added to the node layer /etc/{passwd,group,shadow,gshadow} and not to the /usr as part of the node layer build.


# We're running as part of a derivation; `systemd-sysusers` will not work
# because it doesn't go through NSS. Hackily put the /usr/lib files in /etc
# temporarily then put them back.
mv /etc/passwd /etc/passwd.bak
mv /etc/group /etc/group.bak
mv /usr/lib/passwd /etc/passwd
mv /usr/lib/group /etc/group

# Re-create the user/group/shadow/gshadow entries
systemd-sysusers crio.conf
Copy link
Copy Markdown
Member

@jlebon jlebon Mar 25, 2026

Choose a reason for hiding this comment

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

I think we need to go further and fixate the UID/GID to whatever it was in the base composes on 4.18. Basically same rationale as f202927.

I guess we could do that here, or just add it to r-c-c which already carries a bunch of other fixated users/groups the node image needs.

I'm not sure why containers wasn't part of that commit I did back then. I'm pretty sure the way I came up with this is that I booted a live ~4.18 RHCOS and added the dynamic entries, so perhaps containers wasn't added back then for some reason.


# Put everything back in place
mv /etc/passwd /usr/lib/passwd
mv /etc/group /usr/lib/group
mv /etc/passwd.bak /etc/passwd
mv /etc/group.bak /etc/group
Copy link
Copy Markdown
Member

@dustymabe dustymabe Mar 25, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

I'd like to keep this "hack" targeted.

Can we do something here that will make sure the only entry that was created was the containers: user and no other changes were made to the /usr/lib/passwd* and /etc/passwd* from the RHEL CoreOS Base image we are deriving from and fail if some other changes were made?

fi

- |
#!/usr/bin/env bash
set -xeuo pipefail
Expand Down