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

libpod: create /etc/passwd if missing #15003

Merged
merged 1 commit into from
Jul 21, 2022

Conversation

giuseppe
Copy link
Member

create the /etc/passwd file is it is missing in the image.

Closes: #14966

Signed-off-by: Giuseppe Scrivano [email protected]

Does this PR introduce a user-facing change?

If it is needed, create automatically the /etc/passwd file when it is missing in the image

@openshift-ci openshift-ci bot added release-note approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jul 21, 2022
@giuseppe giuseppe force-pushed the create-etc-passwd branch from 3643483 to bdcb98d Compare July 21, 2022 07:49
@@ -70,6 +70,11 @@ USER 1000`, ALPINE)
session.WaitWithDefaultTimeout()
Expect(session).Should(Exit(0))
Expect(session.OutputToString()).To(Not(ContainSubstring("passwd")))

// test that the /etc/passwd file is created
session = podmanTest.Podman([]string{"run", "--rm", "--user", "0:0", imgName, "ls", "/etc/passwd"})
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need --user to create it? Feels like it should work even when the default user is used, no?

Copy link
Member

Choose a reason for hiding this comment

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

I think it only creates the user account if --user is specified and user is not listed in /etc/passwd.
I guess we could remove check for --user and always check if the default user is listed in the /etc/passwd.

Copy link
Member Author

Choose a reason for hiding this comment

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

it is the current behavior. I am fine with changing it but it is a separate change

Copy link
Member

Choose a reason for hiding this comment

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

I think the current behavior makes sense; only codepaths that modify /etc/passwd will create it.

@cdoern
Copy link
Contributor

cdoern commented Jul 21, 2022

this needs to not happen if podman create --passwd=false right?

@giuseppe
Copy link
Member Author

this needs to not happen if podman create --passwd=false right?

right, with --passwd=false the file is not created

@giuseppe giuseppe force-pushed the create-etc-passwd branch from bdcb98d to d7c52de Compare July 21, 2022 14:49
@mheon
Copy link
Member

mheon commented Jul 21, 2022

Changes LGTM.

create the /etc/passwd and /etc/group files if they are missing in the
image.

Closes: containers#14966

Signed-off-by: Giuseppe Scrivano <[email protected]>
@giuseppe giuseppe force-pushed the create-etc-passwd branch from d7c52de to dd2b794 Compare July 21, 2022 15:58
@giuseppe
Copy link
Member Author

tests are fixed now

Copy link
Collaborator

@flouthoc flouthoc left a comment

Choose a reason for hiding this comment

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

LGTM

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jul 21, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: flouthoc, giuseppe

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@mheon
Copy link
Member

mheon commented Jul 21, 2022

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 21, 2022
@openshift-merge-robot openshift-merge-robot merged commit 5f53a67 into containers:main Jul 21, 2022
@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 21, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. release-note
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Option To Create /etc/passwd If It Does Not Exist
7 participants