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

Do not mount /dev/tty into rootless containers #17304

Merged
merged 1 commit into from
Jan 31, 2023

Conversation

mupuf
Copy link
Contributor

@mupuf mupuf commented Jan 31, 2023

Closes: https://bugzilla.redhat.com/show_bug.cgi?id=2165875

Does this PR introduce a user-facing change?

Rootless privileged containers will not mount /dev/tty in the container

This PR should superseed #17301.

@giuseppe: This should address the regression, without needing to revert the commits (which were themselves fixing a regression).

@mupuf
Copy link
Contributor Author

mupuf commented Jan 31, 2023

I would like to write more tests to catch regressions like this one, but I can't do it now. This could however be landed in a separate PR :)

@giuseppe
Copy link
Member

could you please add [NO NEW TESTS NEEDED] to the commit? I think it is fine to add this for now without having to add a test (can be added later), so I can backport it

[NO NEW TESTS NEEDED]

Closes: https://bugzilla.redhat.com/show_bug.cgi?id=2165875
Signed-off-by: Martin Roukala (né Peres) <[email protected]>
@mupuf mupuf force-pushed the podman_4.4_regression branch from 2574b8f to d10860a Compare January 31, 2023 20:10
@mupuf
Copy link
Contributor Author

mupuf commented Jan 31, 2023

could you please add [NO NEW TESTS NEEDED] to the commit? I think it is fine to add this for now without having to add a test (can be added later), so I can backport it

Done.

Copy link
Member

@giuseppe giuseppe 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 Jan 31, 2023

[APPROVALNOTIFIER] This PR is APPROVED

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

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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 31, 2023
@giuseppe
Copy link
Member

@mheon PTAL

@mheon
Copy link
Member

mheon commented Jan 31, 2023

/lgtm
/hold

Let's get this cherry-picked in 4.4 before we do v4.4.0

@openshift-ci openshift-ci bot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. lgtm Indicates that a PR is ready to be merged. labels Jan 31, 2023
@edsantiago
Copy link
Member

/hold cancel

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 31, 2023
@openshift-merge-robot openshift-merge-robot merged commit 68bbdc2 into containers:main Jan 31, 2023
@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 12, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 12, 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.

5 participants