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

run: add container gid to additional groups #4200

Merged

Conversation

flouthoc
Copy link
Collaborator

When container is created with specific uid and gid also add container
gid to supplementary/additional group.

@flouthoc flouthoc added the kind/bug Categorizes issue or PR as related to a bug. label Aug 24, 2022
@flouthoc
Copy link
Collaborator Author

Needs green flag from @nalind and @giuseppe.

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 Aug 24, 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

@flouthoc flouthoc force-pushed the run-set-additional-gid branch 2 times, most recently from b4b6742 to 64106cf Compare August 24, 2022 10:20
@flouthoc
Copy link
Collaborator Author

Added additional test for --isolation chroot.

tests/bud.bats Outdated Show resolved Hide resolved
tests/bud.bats Outdated Show resolved Hide resolved
@flouthoc
Copy link
Collaborator Author

@nalind PTAL again.

tests/bud.bats Outdated

@test "build test if supplemental groups has gid with --isolation chroot" {
skip_if_rootless_environment
skip_if_no_runtime
Copy link
Member

Choose a reason for hiding this comment

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

Are either of these conditions required for this test?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Other tests which have --isolation chroot are being skipped for similar scenarios that's why i added it, I can try removing it.

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 we should probably try to remove them from all of the chroot tests.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed from this test, lets see if CI is happy. I'll do this for all the other tests in a different PR since they are not part of this PR.

@rhatdan
Copy link
Member

rhatdan commented Aug 24, 2022

LGTM

When container is created with specific uid and gid also add container
gid to supplementary/additional group.

Signed-off-by: Aditya R <[email protected]>
@nalind
Copy link
Member

nalind commented Aug 24, 2022

/lgtm

@openshift-ci openshift-ci bot added the lgtm label Aug 24, 2022
@openshift-merge-robot openshift-merge-robot merged commit f0b2d39 into containers:main Aug 24, 2022
@TomSweeneyRedHat
Copy link
Member

@flouthoc once a BZ gets spun for this, please add a comment with the BZ number if I neglect to.

@nalind
Copy link
Member

nalind commented Aug 26, 2022

Tracked in Red Hat bugzilla at https://bugzilla.redhat.com/show_bug.cgi?id=2121453

@TomSweeneyRedHat
Copy link
Member

I think we need to get this into Buildah release-1.27 too so we can spin this for 8.7 zero day.

@flouthoc
Copy link
Collaborator Author

Backport PR here: #4207

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved kind/bug Categorizes issue or PR as related to a bug. lgtm locked - please file new issue/PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants