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

fix: add dockerignore read check. #1939

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

Conversation

tao12345666333
Copy link
Contributor

Signed-off-by: Jintao Zhang zhangjintao9020@gmail.com

- What I did

fix #1938

- How I did it

add OpenDockerignore function.

- How to verify it

(MoeLove) ➜  x cat .dockerignore
cat: .dockerignore: Permission denied
(MoeLove) ➜  x /home/tao/go/src/github.com/docker/cli/build/docker build --no-cache  -t local/ignore . 
check .dockerignore failed: open .dockerignore: permission denied

- Description for the changelog

fix: add dockerignore read check.

- A picture of a cute animal (not mandatory but encouraged)

Signed-off-by: Jintao Zhang <zhangjintao9020@gmail.com>
@@ -37,3 +37,17 @@ func TrimBuildFilesFromExcludes(excludes []string, dockerfile string, dockerfile
}
return excludes
}

// OpenDockerignore try to open the .dockerignore file in the context directory
func OpenDockerignore(contextDir string) error {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe this function should be changed to a clearer name.

@codecov-io
Copy link

Codecov Report

Merging #1939 into master will decrease coverage by 0.02%.
The diff coverage is 0%.

@@            Coverage Diff            @@
##           master   #1939      +/-   ##
=========================================
- Coverage   56.72%   56.7%   -0.03%     
=========================================
  Files         310     310              
  Lines       21800   21811      +11     
=========================================
  Hits        12367   12367              
- Misses       8518    8529      +11     
  Partials      915     915

@@ -103,6 +103,11 @@ func runBuildBuildKit(dockerCli command.Cli, options buildOptions) error {
} else {
dockerfileDir = options.context
}

if err := build.OpenDockerignore(contextDir); err != nil {
Copy link
Member

Choose a reason for hiding this comment

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

When using buildkit, a session is opened; wondering if an error should be produced on any file that is attempted to send to the daemon (instead of checking for specific files).

@tonistiigi wdyt?

@tao12345666333
Copy link
Contributor Author

@tonistiigi

@thaJeztah
Copy link
Member

thaJeztah commented Jun 25, 2019

ping @tiborvass @AkihiroSuda ptal #1939 (comment)

@tiborvass
Copy link
Collaborator

@tao12345666333 thanks for the contribution. I'm continuing the discussion of the issue on #1938 (comment). We do want buildkit to be as close as possible/sensible to non-buildkit builds. If the bug is accepted, I believe the correct way would be to simply stat-ing .dockerignore inline in build_buildkit.go. No need for separate function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cli with buildkit will ignore dockerignore's check
6 participants