From 906bbb27f1d983b94ab097f12210cf76d8f2196f Mon Sep 17 00:00:00 2001 From: Kevin Cui Date: Wed, 12 Jun 2024 11:41:14 +0800 Subject: [PATCH] refactor(build): improve err when file specified by -f does not exist When the user specifies a Containerfile or Dockfile with the -f flag in podman build, if the file does not exist, the error should be intuitive to the user. Fixed: #22940 Signed-off-by: Kevin Cui Signed-off-by: Wanqi Li --- cmd/podman/common/build.go | 47 ++++++++++++++++++++++++++++++++++---- test/e2e/build_test.go | 19 +++++++++++++++ 2 files changed, 62 insertions(+), 4 deletions(-) diff --git a/cmd/podman/common/build.go b/cmd/podman/common/build.go index 2174b3bf8aa0..ceb8e664941b 100644 --- a/cmd/podman/common/build.go +++ b/cmd/podman/common/build.go @@ -202,10 +202,7 @@ func ParseBuildOpts(cmd *cobra.Command, args []string, buildOpts *BuildFlagsWrap // No context directory or URL was specified. Try to use the home of // the first locally-available Containerfile. for i := range containerFiles { - if strings.HasPrefix(containerFiles[i], "http://") || - strings.HasPrefix(containerFiles[i], "https://") || - strings.HasPrefix(containerFiles[i], "git://") || - strings.HasPrefix(containerFiles[i], "github.com/") { + if isURL(containerFiles[i]) { continue } absFile, err := filepath.Abs(containerFiles[i]) @@ -241,6 +238,10 @@ func ParseBuildOpts(cmd *cobra.Command, args []string, buildOpts *BuildFlagsWrap } } + if err := areContainerfilesValid(contextDir, containerFiles); err != nil { + return nil, err + } + var logFile *os.File if cmd.Flag("logfile").Changed { var err error @@ -628,3 +629,41 @@ func parseDockerignore(ignoreFile string) ([]string, error) { } return excludes, nil } + +func areContainerfilesValid(contextDir string, containerFiles []string) error { + for _, f := range containerFiles { + if isURL(f) || f == "/dev/stdin" { + continue + } + + // Because currently podman runs the test/bud.bats tests under the buildah project in CI, + // the following error messages need to be consistent with buildah; otherwise, the podman CI will fail. + // See: https://github.com/containers/buildah/blob/4c781b59b49d66e07324566555339888113eb7e2/imagebuildah/build.go#L139-L141 + // https://github.com/containers/buildah/blob/4c781b59b49d66e07324566555339888113eb7e2/tests/bud.bats#L3474-L3479 + if utils.IsDir(f) { + return fmt.Errorf("containerfile: %q cannot be path to a directory", f) + } + + // If the file is not found, try again with context directory prepended (if not prepended yet) + // Ref: https://github.com/containers/buildah/blob/4c781b59b49d66e07324566555339888113eb7e2/imagebuildah/build.go#L125-L135 + if utils.FileExists(f) { + continue + } + if !strings.HasPrefix(f, contextDir) { + if utils.FileExists(filepath.Join(contextDir, f)) { + continue + } + } + + return fmt.Errorf("the specified Containerfile or Dockerfile does not exist, %s: %w", f, syscall.ENOENT) + } + + return nil +} + +func isURL(s string) bool { + return strings.HasPrefix(s, "http://") || + strings.HasPrefix(s, "https://") || + strings.HasPrefix(s, "git://") || + strings.HasPrefix(s, "github.com/") +} diff --git a/test/e2e/build_test.go b/test/e2e/build_test.go index c72aebfd423c..d2d50d49c665 100644 --- a/test/e2e/build_test.go +++ b/test/e2e/build_test.go @@ -94,6 +94,25 @@ var _ = Describe("Podman build", func() { Expect(session).Should(ExitCleanly()) }) + It("podman build with not found Containerfile or Dockerfile", func() { + targetPath := filepath.Join(podmanTest.TempDir, "notfound") + err = os.Mkdir(targetPath, 0755) + Expect(err).ToNot(HaveOccurred()) + defer func() { + Expect(os.RemoveAll(targetPath)).To(Succeed()) + }() + + session := podmanTest.Podman([]string{"build", targetPath}) + session.WaitWithDefaultTimeout() + expectStderr := fmt.Sprintf("no Containerfile or Dockerfile specified or found in context directory, %s", targetPath) + Expect(session).Should(ExitWithError(125, expectStderr)) + + session = podmanTest.Podman([]string{"build", "-f", "foo", targetPath}) + session.WaitWithDefaultTimeout() + expectStderr = fmt.Sprintf("the specified Containerfile or Dockerfile does not exist, %s", "foo") + Expect(session).Should(ExitWithError(125, expectStderr)) + }) + It("podman build with logfile", func() { logfile := filepath.Join(podmanTest.TempDir, "logfile") session := podmanTest.Podman([]string{"build", "--pull=never", "--tag", "test", "--logfile", logfile, "build/basicalpine"})