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

Add secondary group impersonation w/ !cgo support #1164

Merged
merged 5 commits into from
May 4, 2020

Conversation

tstromberg
Copy link
Contributor

TL;DR: PR is a bit funky so as to support !cgo builds, such as what we use in the Kaniko image.

Without cgo, Go is not able to use getgroups() to resolve group information. This PR falls back to parsing /etc/group with the same logic as what is used in Go's source tree, but adds secondary group information.

Integration tests will require some hand-holding.

Fixes #1097

@googlebot googlebot added the cla: yes CLA signed by all commit authors label Mar 29, 2020
Copy link
Member

@tejal29 tejal29 left a comment

Choose a reason for hiding this comment

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

Thanks for tracking this issue down. This looks good to me expect few nits!

pkg/commands/run.go Show resolved Hide resolved
pkg/util/groupids_cgo.go Show resolved Hide resolved
pkg/util/groupids_fallback.go Show resolved Hide resolved
pkg/util/groupids_fallback.go Outdated Show resolved Hide resolved
@tejal29
Copy link
Member

tejal29 commented Apr 14, 2020

Regarding integration tests, all the integration tests pass. Did you mean, you need to add new intergration test? You could add the dockerfile mentioned in the issue to integration/dockerfiles/ and the tests infrastructure will pick this file up.

  1. It will perform a build using docker
  2. It will perform a build using kaniko
  3. compare the images generated using container-diff

@tejal29 tejal29 merged commit e32715e into GoogleContainerTools:master May 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes CLA signed by all commit authors
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The USER command does not set the correct gids, so extra groups are dropped
3 participants