fix: Quotes in dockerfiles env vars break copy dependency checks

[kadern0]

Fixes #6762
Signed-off-by: Pablo Caderno [email protected]

Description

Fixes the issue with quoted variables and "COPY" commands.

Before:

skaffold build
Generating tags...
 - bert -> bert:151b8d3-dirty
 - ernie -> ernie:151b8d3
Checking cache...
 - bert: Error checking cache.
getting hash for artifact "bert": getting dependencies for "bert": file pattern [./src/"some_dir"] must match at least one file

After:

skaffold build
Generating tags...
 - bert -> bert:151b8d3-dirty
 - ernie -> ernie:151b8d3
Checking cache...
 - bert: Not found. Building
 - ernie: Found Locally
Starting build...
Found [ed-ks2] context, using local docker daemon.
Building [bert]...
Sending build context to Docker daemon  7.168kB
Step 1/20 : FROM golang:1.15 as builder
 ---> 40349a2425ef
Step 2/20 : COPY main.go .
 ---> Using cache
 ---> 5bcddb464805
Step 3/20 : ARG SKAFFOLD_GO_GCFLAGS
 ---> Using cache
 ---> 0fe720d5c3d6
Step 4/20 : RUN go build -gcflags="${SKAFFOLD_GO_GCFLAGS}" -o /app main.go
 ---> Using cache
 ---> b0fa60be1cbd
Step 5/20 : FROM alpine:3.10
 ---> e7b300aee9f9
Step 6/20 : ENV MY_NAME="John Doe" MY_DOG=Rex\ The\ Dog     MY_CAT=fluffy
 ---> Using cache
 ---> 0a651a254ade
Step 7/20 : ENV GOTRACEBACK=single
 ---> Using cache
 ---> 0588ba733bf7
Step 8/20 : ENV DIR2=some_dir
 ---> Using cache
 ---> ac1901e9b83c
Step 9/20 : ENV DIRNAME="some_dir"
 ---> Using cache
 ---> 0a5beef983fa
Step 10/20 : ENV NOBREAK="asdf'asdf"
 ---> Running in 48d6824c02ad
 ---> 17df958a420a
Step 11/20 : COPY $NOBREAK .
 ---> 1fcafb39acb1
Step 12/20 : COPY a .
 ---> 428b63829342
Step 13/20 : COPY src/ .
 ---> 4e55fcb5d5c5
Step 14/20 : COPY ./src/$DIR2 /tmp/
 ---> 59df6382498d
Step 15/20 : COPY ./src/$DIRNAME  /tmp/
 ---> 4d182523b4f3
Step 16/20 : COPY ./src/$MY_NAME /tmp/
 ---> a4ef10e6fe6b
Step 17/20 : COPY ./src/$MY_DOG /tmp/
 ---> 043c8a3062a3
Step 18/20 : COPY ./src/$MY_CAT /tmp/
 ---> 8c1d5aa9b2c7
Step 19/20 : CMD ["./app"]
 ---> Running in 7e5a5b6a9c9d
 ---> 45c93161726d
Step 20/20 : COPY --from=builder /app .
 ---> 29276d711899
Successfully built 29276d711899
Successfully tagged bert:151b8d3-dirty

[codecov]

Codecov Report

Merging #6796 (cc06999) into main (290280e) will decrease coverage by 1.08%.
The diff coverage is 65.62%.

@@            Coverage Diff             @@
##             main    #6796      +/-   ##
==========================================
- Coverage   70.48%   69.40%   -1.09%     
==========================================
  Files         515      540      +25     
  Lines       23150    24596    +1446     
==========================================
+ Hits        16317    17070     +753     
- Misses       5776     6395     +619     
- Partials     1057     1131      +74     

Impacted Files	Coverage Δ
cmd/skaffold/app/cmd/deploy.go	52.00% <ø> (-1.85%)	⬇️
cmd/skaffold/app/cmd/flags.go	89.00% <0.00%> (-1.82%)	⬇️
cmd/skaffold/app/cmd/render.go	36.66% <0.00%> (-4.72%)	⬇️
cmd/skaffold/skaffold.go	0.00% <ø> (ø)
cmd/skaffold/app/cmd/find_configs.go	48.88% <50.00%> (+0.24%)	⬆️
cmd/skaffold/app/cmd/lint.go	52.94% <52.94%> (ø)
cmd/skaffold/app/cmd/cmd.go	70.49% <75.00%> (-0.57%)	⬇️
cmd/skaffold/app/cmd/debug.go	100.00% <100.00%> (ø)
cmd/skaffold/app/cmd/runner.go	64.17% <100.00%> (ø)
cmd/skaffold/app/flags/image.go	76.27% <100.00%> (+3.72%)	⬆️
... and 133 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d8104cf...cc06999. Read the comment docs.

[aaron-prindle]

LGTM

[kadern0]

Hi @aaron-prindle thank you very much. I noticed an estrange behavior when testing this change and wanted to ask someone before opening an issue. When building docker containers, skaffold build fails if you try to copy and empty dir. Is there any reason for skaffold build to work different than docker build ?

[aaron-prindle]

I noticed an estrange behavior when testing this change and wanted to ask someone before opening an issue. When building docker containers, skaffold build fails if you try to copy and empty dir. Is there any reason for skaffold build to work different than docker build ?

@kadern0 can you give a small sample-app/set-of-steps to reproduce the issue you mentioned above? I can investigate more thoroughly if you have an app setup I can use to repro. skaffold build should not be different than docker build ideally but because of how env vars, config, etc. are passed around there might be some discrepancies. I will hold off on merging this until I can look into the issue you mentioned

[kadern0]

I noticed an estrange behavior when testing this change and wanted to ask someone before opening an issue. When building docker containers, skaffold build fails if you try to copy and empty dir. Is there any reason for skaffold build to work different than docker build ?

@kadern0 can you give a small sample-app/set-of-steps to reproduce the issue you mentioned above? I can investigate more thoroughly if you have an app setup I can use to repro. skaffold build should not be different than docker build ideally but because of how env vars, config, etc. are passed around there might be some discrepancies. I will hold off on merging this until I can look into the issue you mentioned

Steps to reproduce:
cd into skaffold/examples/docker-deploy/bert/

mkdir test
# add following copy to Dockerfile
COPY test .
cd ..
skaffold build
Generating tags...
 - bert -> bert:151b8d3-dirty
 - ernie -> ernie:151b8d3
Checking cache...
 - bert: Not found. Building
 - ernie: Found Locally
Starting build...
Found [ed-ks2] context, using local docker daemon.
Building [bert]...
Sending build context to Docker daemon  3.072kB
Step 1/9 : FROM golang:1.15 as builder
 ---> 40349a2425ef
Step 2/9 : COPY main.go .
 ---> Using cache
 ---> 5bcddb464805
Step 3/9 : ARG SKAFFOLD_GO_GCFLAGS
 ---> Using cache
 ---> 0fe720d5c3d6
Step 4/9 : RUN go build -gcflags="${SKAFFOLD_GO_GCFLAGS}" -o /app main.go
 ---> Using cache
 ---> b0fa60be1cbd
Step 5/9 : FROM alpine:3.10
 ---> e7b300aee9f9
Step 6/9 : ENV GOTRACEBACK=single
 ---> Using cache
 ---> e850d4805844
Step 7/9 : COPY test .
unable to stream build output: COPY failed: file not found in build context or excluded by .dockerignore: stat test: file does not exist

Verify directory exists:

ls bert/
Dockerfile   main.go   src   test

Now, add a single file to the directory and try again:

touch bert/test/testfile
 skaffold build
Generating tags...
 - bert -> bert:151b8d3-dirty
 - ernie -> ernie:151b8d3
Checking cache...
 - bert: Not found. Building
 - ernie: Found Locally
Starting build...
Found [ed-ks2] context, using local docker daemon.
Building [bert]...
Sending build context to Docker daemon  3.584kB
Step 1/9 : FROM golang:1.15 as builder
 ---> 40349a2425ef
Step 2/9 : COPY main.go .
 ---> Using cache
 ---> 5bcddb464805
Step 3/9 : ARG SKAFFOLD_GO_GCFLAGS
 ---> Using cache
 ---> 0fe720d5c3d6
Step 4/9 : RUN go build -gcflags="${SKAFFOLD_GO_GCFLAGS}" -o /app main.go
 ---> Using cache
 ---> b0fa60be1cbd
Step 5/9 : FROM alpine:3.10
 ---> e7b300aee9f9
Step 6/9 : ENV GOTRACEBACK=single
 ---> Using cache
 ---> e850d4805844
Step 7/9 : COPY test .
 ---> 4c26c1bf872e
Step 8/9 : CMD ["./app"]
 ---> Running in 6c98a27efeca
 ---> 41b29bf39f5b
Step 9/9 : COPY --from=builder /app .
 ---> 69557d373998
Successfully built 69557d373998
Successfully tagged bert:151b8d3-dirty

It fails when the directory is empty.

[aaron-prindle]

@kadern0 the docker COPY empty directory appears to be a current bug in skaffold that is not related to this change. I attempted reproducing this error w/ various version of skaffold and this has been a skaffold bug from at least v1.32.0 onward:

aprindle@aprindle ~/skaffold/examples/docker-deploy  [main]$ skaffold-v1.32.0 version
v1.32.0
aprindle@aprindle ~/skaffold/examples/docker-deploy  [main]$ skaffold-v1.32.0 build
...  # ommitting middle of logs
unable to stream build output: COPY failed: file not found in build context or excluded by .dockerignore: stat test: file does not exist

aprindle@aprindle ~/skaffold/examples/docker-deploy  [main]$ skaffold-v1.33.0 version
v1.33.0
aprindle@aprindle ~/skaffold/examples/docker-deploy  [main]$ skaffold-v1.33.0 build
...  # ommitting middle of logs
unable to stream build output: COPY failed: file not found in build context or excluded by .dockerignore: stat test: file does not exist

aprindle@aprindle ~/skaffold/examples/docker-deploy  [main]$ skaffold v1.34.0 version
v1.34.0
aprindle@aprindle ~/skaffold/examples/docker-deploy  [main]$ skaffold-v1.34.0 build
...  # ommitting middle of logs
unable to stream build output: COPY failed: file not found in build context or excluded by .dockerignore: stat test: file does not exist

As such I do not believe this issue is related to this PR and the PR is the correct fix as is. I have created #6810 to track the issue w/ skaffold not properly handling Dockerfiles that COPY an empty directory

[aaron-prindle]

Merging, thanks for all of your work here @kadern0!