Makefile: Fix potential uid/gid collision by using setpriv #821
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The release build should run with the same numeric user and group id as
the caller of the Makefile. This solves two problems:
1. The generated artefacts should be owned by the user who invoked the
Makefile
2. `go build -buildvcs=true` (the default since Go 1.18) invokes git,
and git requires that the .git folder is owned by the user invoking
it.
Previously, we achieved this by creating a new user and group inside the
container with the wanted uid/gid. The problem with that approach is it
fails if the uid/gid is already taken (such as e.g. gid 123, which seems
to be used by the GitHub runner as of recently). Therefore, instead of
trying to create a new user, this commit now uses `setpriv` from
util-linux to force the `make` process (and all its children) to run as
the RELEASE_{UID/GID}. This ensures that both `git` can access the
`.git` directory owned by the host user, and that the host user can
access the generated artifacts. One limitation of `setpriv` is that it
runs the child process without an accessible `$HOME` directory, which
`go build` needs for the GOCACHE. To solve this for now, we point it to
a temporary directory. In the future, we could consider using a GOCACHE
owned by the host, to allow cache-reuse across builds.
Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
gandro
added
🤖 area/CI
gandro
requested review from
michi-covalent,
kaworu and
tklauser
and removed request for
a team
rolinh
approved these changes
Nov 29, 2022
michi-covalent
approved these changes
Nov 29, 2022
This was referenced Nov 29, 2022
|
Nice workaround, TIL about |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The release build should run with the same numeric user and group id as the caller of the Makefile. This solves two problems:
go build -buildvcs=true(the default since Go 1.18) invokes git, and git requires that the .git folder is owned by the user invoking it.Previously, we achieved this by creating a new user and group inside the container with the wanted uid/gid. The problem with that approach is it fails if the uid/gid is already taken (such as e.g. gid 123, which seems to be used by the GitHub runner as of recently). Therefore, instead of trying to create a new user, this commit now uses
setprivfrom util-linux to force themakeprocess (and all its children) to run as the RELEASE_{UID/GID}. This ensures that bothgitcan access the.gitdirectory owned by the host user, and that the host user can access the generated artifacts. One limitation ofsetprivis that it runs the child process without an accessible$HOMEdirectory, whichgo buildneeds for the GOCACHE. To solve this for now, we point it to a temporary directory. In the future, we could consider using a GOCACHE owned by the host, to allow cache-reuse across builds.This fixes an issue where the GitHub release workflow fails with
addgroup: gid '123' in use