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

Jump to bottom

abi: drop check for IsRootless() #21017

Merged
merged 1 commit into from
Dec 18, 2023
Merged

abi: drop check for IsRootless() #21017

merged 1 commit into from
Dec 18, 2023

Conversation

giuseppe
Copy link
Member

it is the wrong check to do here since we need to setup the user namespace even in the case we are running as root without capabilities.

[NO NEW TEST NEEDED] this happens in nested podman

Closes: #20908

Does this PR introduce a user-facing change?

podman running as euid=0 but without CAP_SYS_ADMIN doesn't crash if `podman info` is called multiple times

@openshift-ci openshift-ci bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. release-note labels Dec 14, 2023
@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented Dec 14, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 14, 2023
@giuseppe giuseppe mentioned this pull request Dec 14, 2023
Closed
@giuseppe giuseppe force-pushed the fix-podman-info-twice branch 3 times, most recently from ef1fe79 to 0ecb25a Compare December 17, 2023 18:24
@giuseppe
abi: drop check for IsRootless()
1322f31 
it is the wrong check to do here since we need to setup the user
namespace even in the case we are running as root without
capabilities.

[NO NEW TESTS NEEDED] this happens in nested podman

Closes: containers#20908

Signed-off-by: Giuseppe Scrivano <[email protected]>
@giuseppe giuseppe force-pushed the fix-podman-info-twice branch from 0ecb25a to 1322f31 Compare December 18, 2023 13:05
@giuseppe giuseppe marked this pull request as ready for review December 18, 2023 14:15
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 18, 2023
@giuseppe
Copy link
Member Author

ready for review

@rhatdan
Copy link
Member

rhatdan commented Dec 18, 2023

LGTM
@Luap99 @vrothberg @mheon @baude @ashley-cui @umohnani8 @nalind @lsm5 PTAL

@ashley-cui
Copy link
Member

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Dec 18, 2023
@mheon
Copy link
Member

mheon commented Dec 18, 2023

LGTM

@openshift-merge-bot openshift-merge-bot bot merged commit 26bcffd into containers:main Dec 18, 2023
91 of 92 checks passed
@giuseppe
Copy link
Member Author

/cherry-pick v4.9

@openshift-cherrypick-robot
Copy link
Collaborator

@giuseppe: #21017 failed to apply on top of branch "v4.9":

Applying: abi: drop check for IsRootless()
Using index info to reconstruct a base tree...
M	pkg/domain/infra/abi/system_linux.go
Falling back to patching base and 3-way merge...
Auto-merging pkg/domain/infra/abi/system_linux.go
CONFLICT (content): Merge conflict in pkg/domain/infra/abi/system_linux.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 abi: drop check for IsRootless()
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

In response to this:

/cherry-pick v4.9

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@giuseppe giuseppe mentioned this pull request Jan 30, 2024
Merged
@giuseppe
Copy link
Member Author

backport to 4.9 here: #21436

@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 Apr 30, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 30, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees

@ashley-cui ashley-cui

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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Second invocation of podman info in rootless container SIGSEGVs
5 participants
@giuseppe @rhatdan @ashley-cui @mheon @openshift-cherrypick-robot