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

ci: make build work on systems with SELinux #13003

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

yann-soubeyrand
Copy link
Contributor

@yann-soubeyrand yann-soubeyrand commented Mar 24, 2023

I came across this issue #3683 and I saw the fix seemed to be easy. I took 10 minutes to do the modifications to the code, but then I had a hard time building an image using the Makefile.

My workstation runs on Fedora 38 with SELinux in enforcing mode (the default) and it seems impossible to use BuildKit at the time on a system with SELinux: moby/buildkit#2320. In the meantime, one can use Podman on these systems.

Note on DCO:

If the DCO action in the integration test fails, one or more of your commits are not signed off. Please click on the Details link next to the DCO action for instructions on how to resolve this.

Checklist:

  • Either (a) I've created an enhancement proposal and discussed it with the community, (b) this is a bug fix, or (c) this does not need to be in the release notes.
  • The title of the PR states what changed and the related issues number (used for the release note).
  • I've included "Closes [ISSUE #]" or "Fixes [ISSUE #]" in the description to automatically close the associated issue.
  • I've updated both the CLI and UI to expose my feature, or I plan to submit a second PR with them.
  • Does this PR require documentation updates?
  • I've updated documentation as required by this PR.
  • Optional. My organization is added to USERS.md.
  • I have signed off all my commits as required by DCO
  • I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged.
  • My build is green (troubleshooting builds).
  • My new feature complies with the feature status guidelines.
  • I have added a brief description of why this PR is necessary and/or what this PR solves.

Please see Contribution FAQs if you have questions about your pull-request.

@@ -14,7 +14,7 @@ BUILD_DATE=$(shell date -u +'%Y-%m-%dT%H:%M:%SZ')
GIT_COMMIT=$(shell git rev-parse HEAD)
GIT_TAG=$(shell if [ -z "`git status --porcelain`" ]; then git describe --exact-match --tags HEAD 2>/dev/null; fi)
GIT_TREE_STATE=$(shell if [ -z "`git status --porcelain`" ]; then echo "clean" ; else echo "dirty"; fi)
VOLUME_MOUNT=$(shell if test "$(go env GOOS)" = "darwin"; then echo ":delegated"; elif test selinuxenabled; then echo ":delegated"; else echo ""; fi)
VOLUME_MOUNT=$(shell if test "$(go env GOOS)" = "darwin"; then echo ":delegated"; elif test selinuxenabled; then echo ":Z"; else echo ""; fi)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It used to be :Z when SELinux is enabled, but was later changed to :delegated. I’m not sure to understand why, maybe it was a mistake?

@@ -94,7 +97,6 @@ define run-in-test-server
-v ${GOPATH}/pkg/mod:/go/pkg/mod${VOLUME_MOUNT} \
-v ${GOCACHE}:/tmp/go-build-cache${VOLUME_MOUNT} \
-v ${HOME}/.kube:/home/user/.kube${VOLUME_MOUNT} \
-v /tmp:/tmp${VOLUME_MOUNT} \
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I’m not sure why /tmp was bind-mounted (for caching reasons maybe?), but one cannot relabel (change SELinux label, which happens when using :Z on a mount) /tmp.

@@ -118,15 +120,18 @@ define run-in-test-client
-v ${GOPATH}/pkg/mod:/go/pkg/mod${VOLUME_MOUNT} \
-v ${GOCACHE}:/tmp/go-build-cache${VOLUME_MOUNT} \
-v ${HOME}/.kube:/home/user/.kube${VOLUME_MOUNT} \
-v /tmp:/tmp${VOLUME_MOUNT} \
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same as above.

@yann-soubeyrand yann-soubeyrand changed the title build: make it work on systems with SELinux ci: make build work on systems with SELinux Mar 24, 2023
@codecov
Copy link

codecov bot commented Mar 24, 2023

Codecov Report

Patch coverage has no change and project coverage change: -0.01 ⚠️

Comparison is base (9b53eeb) 49.05% compared to head (7e7b7ff) 49.04%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #13003      +/-   ##
==========================================
- Coverage   49.05%   49.04%   -0.01%     
==========================================
  Files         246      246              
  Lines       42569    42569              
==========================================
- Hits        20882    20879       -3     
- Misses      19572    19574       +2     
- Partials     2115     2116       +1     

see 2 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Comment on lines -4 to -8
if test "$(id -u)" == "0" -a "${USER_ID}" != ""; then
useradd -u ${USER_ID} -d /home/user -s /bin/bash ${USER_NAME:-default}
chown -R "${USER_NAME:-default}" ${GOCACHE}
fi

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I’m not sure to understand this: the user is already created in the Dockerfile and Docker is explicitly run with a user ID, so this condition evaluates to false. In addition, it breaks with Podman, because Podman, while running unprivileged, appears to run as root from the container point of view (root being in fact mapped to the host user ID).

It seems impossible to use BuildKit at the time on a system with
SELinux: moby/buildkit#2320. In the meantime,
one can use Podman on these systems.

Signed-off-by: Yann Soubeyrand <yann.soubeyrand@camptocamp.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant