moby / buildkit

concurrent, cache-efficient, and Dockerfile-agnostic builder toolkit
https://github.com/moby/moby/issues/34227
Apache License 2.0
7.89k stars 1.09k forks source link

`ADD` with git repo url and `--keep-git-dir` is left in detached HEAD state #4446

Open tspearconquest opened 7 months ago

tspearconquest commented 7 months ago

Hi,

I'm trying to build Zed Attack Proxy inside a docker container.

My dockerfile is as follows:

# syntax=docker/dockerfile:1.5-labs

ARG IMAGE_VERSION
ARG BUILDER_VERSION
ARG BUILDER_TYPE

ARG FIREFOX_VERSION="115.5.0esr-1~deb11u1"

FROM scratch as clone
USER 65532:65532
ARG PACKAGE_VERSION
ADD --link https://github.com/zaproxy/zaproxy.git#${PACKAGE_VERSION} /zap

FROM scratch as clone-extensions
USER 65532:65532
ARG PACKAGE_VERSION
ADD --link --keep-git-dir=true https://github.com/zaproxy/zap-extensions.git#main /zap/build/tmp/buildWeeklyAddOns/zap-extensions

FROM eclipse-temurin:${BUILDER_VERSION}${BUILDER_TYPE} as deps
USER 0
RUN addgroup -S -g 65532 zapbuild \
 && adduser -S -D -G zapbuild -u 65532  zapbuild
USER 65532:65532
COPY --link --from=clone --chown=65532:65532 /zap/ /home/zapbuild/zaproxy/
COPY --link --from=clone-extensions --chown=65532:65532 / /home/zapbuild/zaproxy/
WORKDIR /home/zapbuild/zaproxy

FROM deps as builder

## NEXT 2 LINES ARE FOR DEBUGGING
USER 0
RUN apk --quiet --update --no-cache add git

USER 65532:65532
WORKDIR /home/zapbuild/zaproxy

## DEBUGGING COMMANDS ADDED BEFORE THE export COMMAND BELOW

RUN cd zap/build/tmp/buildWeeklyAddOns/zap-extensions && git branch && ls -la && exit 1 && export JAVA_HOME='/opt/java/openjdk' \
           ZAP_WEEKLY_ADDONS_NO_TEST=true \
           GRADLE_OPTS="-Dorg.gradle.daemon=false -Dorg.gradle.logging.level=info" \
 && ./gradlew :zap:distWeekly \
 ## We extract the newly built zip in this image because copying the whole zip in the next stage
 ## only to immediately extract and remove it consumes a ton of extra space in the final image
 && cd .. \
 && unzip -qq zaproxy/zap/build/distributions/*.zip \
 && rm -rf zaproxy
## There's more but this is enough to reproduce the issue.

Built with:

DOCKER_BUILDKIT=1 docker build --progress=plain --build-arg BUILDER_VERSION=11.0.21_9 --build-arg BUILDER_TYPE="-jdk-alpine" --build-arg PACKAGE_VERSION=v2.14.0 --build-arg IMAGE_VERSION=stable -t zaproxy:test .

This results in buildkit running git branch during the build and showing that the main branch has been cloned but not checked out, so we're not sitting on any branch. I would have expected for the main branch to be checked out.

Unfortunately, the fact that no branch was checked out breaks the gradle build of the addons, because it depends on being on a checked-out branch. Thus, when you remove the debugging commands and build, the following is output:

#24 140.5 Tasks to be executed: [task ':zap:buildWeeklyAddOns', task ':zap:cyclonedxRuntimeBom', task ':zap:compileJava', task ':zap:processResources', task ':zap:classes', task ':zap:jar', task ':zap:jarWithBom', task ':zap:distFiles', task ':zap:jarDaily', task ':zap:prepareDistWeekly', task ':zap:distWeekly']
#24 140.5 Tasks that were excluded: []
#24 140.5 Resolve mutations for :zap:buildWeeklyAddOns (Thread[included builds,5,main]) started.
#24 140.5 :zap:buildWeeklyAddOns (Thread[included builds,5,main]) started.
#24 141.1 
#24 141.1 FAILURE: Build failed with an exception.
#24 141.1 
#24 141.1 * What went wrong:
#24 141.1 Execution failed for task ':zap:buildWeeklyAddOns'.
#24 141.1 > org.eclipse.jgit.api.errors.NoHeadException: Cannot check out from unborn branch
#24 141.1 
#24 141.1 * Try:
#24 141.1 > Run with --stacktrace option to get the stack trace.
#24 141.1 > Run with --debug option to get more log output.
#24 141.1 > Run with --scan to get full insights.
#24 141.1 > Get more help at https://help.gradle.org/.
#24 141.1 
#24 141.1 BUILD FAILED in 2m 20s
#24 141.1 
#24 141.1 > Task :zap:buildWeeklyAddOns FAILED

I looked through the docs but did not find any method to make buildkit ADD do a checkout after cloning the main branch. Is this a bug or by design? If by design, can support be added, possibly with another flag like --checkout=<true|false> where the default is false?

jedevc commented 7 months ago

After performing the git checkout, buildkit doesn't keep the .git directory by default. You can keep it by using the --keep-git-dir=true flag:

https://docs.docker.com/engine/reference/builder/#adding-a-git-repository-add-git-ref-dir

That should resolve your issue :tada:

tspearconquest commented 7 months ago

Hi, please reopen and read the provided dockerfile where I used this flag already and demonstrated that the checkout is not happening.

tspearconquest commented 7 months ago

Under the FROM scratch as clone-extensions layer, I am already using the flag...

jedevc commented 7 months ago

Apologies @tspearconquest, I read through too quickly :heart:

@tonistiigi is there a reason we couldn't swap from git checkout FETCH_HEAD to git checkout ${ref} here?

https://github.com/moby/buildkit/blob/f8937901d3486d2f3482163aa152b5ae9fb204f6/source/git/source.go#L542-L545

Or is there a deliberate reason we should be in a detached HEAD state?

tonistiigi commented 7 months ago

@jedevc Ref does not need to mean branch, #tag , #pull/NR/head, #sha are all valid as well. But I agree that for branches, the current current branch should be set as well.