Merge pull request #6550 from thaJeztah/deprecate_builder_utils

cli/command/image/build: deprecate IsArchive utility

Author: thaJeztah

cli/command/image/build.go

@@ -181,7 +181,6 @@ func (out *lastProgressOutput) WriteProgress(prog progress.Progress) error {
 //nolint:gocyclo
 func runBuild(ctx context.Context, dockerCli command.Cli, options buildOptions) error {
 	var (
-		err           error
 		buildCtx      io.ReadCloser
 		dockerfileCtx io.ReadCloser
 		contextDir    string
@@ -263,7 +262,7 @@ func runBuild(ctx context.Context, dockerCli command.Cli, options buildOptions)
 		}
 
 		if err := build.ValidateContextDirectory(contextDir, excludes); err != nil {
-			return fmt.Errorf("error checking context: %w", err)
+			return fmt.Errorf("checking context: %w", err)
 		}
 
 		// And canonicalize dockerfile name to a platform-independent one
@@ -287,9 +286,6 @@ func runBuild(ctx context.Context, dockerCli command.Cli, options buildOptions)
 		}
 	}
 
-	ctx, cancel := context.WithCancel(ctx)
-	defer cancel()
-
 	if options.compress {
 		buildCtx, err = build.Compress(buildCtx)
 		if err != nil {
@@ -330,17 +326,18 @@ func runBuild(ctx context.Context, dockerCli command.Cli, options buildOptions)
 		}
 	}
 	buildOpts := imageBuildOptions(dockerCli, options)
-	buildOpts.Version = buildtypes.BuilderV1
 	buildOpts.Dockerfile = relDockerfile
 	buildOpts.AuthConfigs = authConfigs
 	buildOpts.RemoteContext = remote
 
+	ctx, cancel := context.WithCancel(ctx)
+	defer cancel()
+
 	response, err := dockerCli.Client().ImageBuild(ctx, body, buildOpts)
 	if err != nil {
 		if options.quiet {
 			_, _ = fmt.Fprintf(dockerCli.Err(), "%s", progBuff)
 		}
-		cancel()
 		return err
 	}
 	defer response.Body.Close()
@@ -357,7 +354,8 @@ func runBuild(ctx context.Context, dockerCli command.Cli, options buildOptions)
 
 	err = jsonstream.Display(ctx, response.Body, streams.NewOut(buildBuff), jsonstream.WithAuxCallback(aux))
 	if err != nil {
-		if jerr, ok := err.(*jsonstream.JSONError); ok {
+		var jerr *jsonstream.JSONError
+		if errors.As(err, &jerr) {
 			// If no error code is set, default to 1
 			if jerr.Code == 0 {
 				jerr.Code = 1
@@ -402,6 +400,7 @@ func validateTag(rawRepo string) (string, error) {
 func imageBuildOptions(dockerCli command.Cli, options buildOptions) client.ImageBuildOptions {
 	configFile := dockerCli.ConfigFile()
 	return client.ImageBuildOptions{
+		Version:        buildtypes.BuilderV1,
 		Memory:         options.memory.Value(),
 		MemorySwap:     options.memorySwap.Value(),
 		Tags:           options.tags.GetSlice(),

cli/command/image/build/context.go

@@ -80,7 +80,7 @@ func ValidateContextDirectory(srcPath string, excludes []string) error {
 			if err != nil && os.IsPermission(err) {
 				return fmt.Errorf("no permission to read from '%s'", filePath)
 			}
-			currentFile.Close()
+			_ = currentFile.Close()
 		}
 		return nil
 	})
@@ -97,18 +97,18 @@ func filepathMatches(matcher *patternmatcher.PatternMatcher, file string) (bool,
 
 // DetectArchiveReader detects whether the input stream is an archive or a
 // Dockerfile and returns a buffered version of input, safe to consume in lieu
-// of input. If an archive is detected, isArchive is set to true, and to false
+// of input. If an archive is detected, ok is set to true, and to false
 // otherwise, in which case it is safe to assume input represents the contents
 // of a Dockerfile.
-func DetectArchiveReader(input io.ReadCloser) (rc io.ReadCloser, isArchive bool, err error) {
+func DetectArchiveReader(input io.ReadCloser) (rc io.ReadCloser, ok bool, err error) {
 	buf := bufio.NewReader(input)
 
 	magic, err := buf.Peek(archiveHeaderSize * 2)
 	if err != nil && err != io.EOF {
 		return nil, false, fmt.Errorf("failed to peek context header from STDIN: %w", err)
 	}
 
-	return newReadCloserWrapper(buf, func() error { return input.Close() }), IsArchive(magic), nil
+	return newReadCloserWrapper(buf, func() error { return input.Close() }), isArchive(magic), nil
 }
 
 // WriteTempDockerfile writes a Dockerfile stream to a temporary file with a
@@ -141,12 +141,12 @@ func WriteTempDockerfile(rc io.ReadCloser) (dockerfileDir string, err error) {
 // Dockerfile or tar archive. Returns a tar archive used as a context and a
 // path to the Dockerfile inside the tar.
 func GetContextFromReader(rc io.ReadCloser, dockerfileName string) (out io.ReadCloser, relDockerfile string, err error) {
-	rc, isArchive, err := DetectArchiveReader(rc)
+	rc, ok, err := DetectArchiveReader(rc)
 	if err != nil {
 		return nil, "", err
 	}
 
-	if isArchive {
+	if ok {
 		return rc, dockerfileName, nil
 	}
 
@@ -171,14 +171,22 @@ func GetContextFromReader(rc io.ReadCloser, dockerfileName string) (out io.ReadC
 
 	return newReadCloserWrapper(tarArchive, func() error {
 		err := tarArchive.Close()
-		os.RemoveAll(dockerfileDir)
+		_ = os.RemoveAll(dockerfileDir)
 		return err
 	}), DefaultDockerfileName, nil
 }
 
 // IsArchive checks for the magic bytes of a tar or any supported compression
 // algorithm.
+//
+// Deprecated: this utility was used internally and will be removed in the next release.
 func IsArchive(header []byte) bool {
+	return isArchive(header)
+}
+
+// isArchive checks for the magic bytes of a tar or any supported compression
+// algorithm.
+func isArchive(header []byte) bool {
 	if compression.Detect(header) != compression.None {
 		return true
 	}
@@ -242,7 +250,7 @@ func getWithStatusError(url string) (resp *http.Response, err error) {
 	}
 	msg := fmt.Sprintf("failed to GET %s with status %s", url, resp.Status)
 	body, err := io.ReadAll(resp.Body)
-	resp.Body.Close()
+	_ = resp.Body.Close()
 	if err != nil {
 		return nil, fmt.Errorf("%s: error reading body: %w", msg, err)
 	}
@@ -374,7 +382,7 @@ func isUNC(path string) bool {
 // the relative path to the dockerfile in the context.
 func AddDockerfileToBuildContext(dockerfileCtx io.ReadCloser, buildCtx io.ReadCloser) (io.ReadCloser, string, error) {
 	file, err := io.ReadAll(dockerfileCtx)
-	dockerfileCtx.Close()
+	_ = dockerfileCtx.Close()
 	if err != nil {
 		return nil, "", err
 	}
@@ -438,17 +446,19 @@ func Compress(buildCtx io.ReadCloser) (io.ReadCloser, error) {
 	go func() {
 		compressWriter, err := compression.CompressStream(pipeWriter, archive.Gzip)
 		if err != nil {
-			pipeWriter.CloseWithError(err)
+			_ = pipeWriter.CloseWithError(err)
 		}
-		defer buildCtx.Close()
+		defer func() {
+			_ = buildCtx.Close()
+		}()
 
 		if _, err := io.Copy(compressWriter, buildCtx); err != nil {
-			pipeWriter.CloseWithError(fmt.Errorf("failed to compress context: %w", err))
-			compressWriter.Close()
+			_ = pipeWriter.CloseWithError(fmt.Errorf("failed to compress context: %w", err))
+			_ = compressWriter.Close()
 			return
 		}
-		compressWriter.Close()
-		pipeWriter.Close()
+		_ = compressWriter.Close()
+		_ = pipeWriter.Close()
 	}()
 
 	return pipeReader, nil

cli/command/image/build/context_test.go

@@ -135,7 +135,7 @@ func TestGetContextFromReaderString(t *testing.T) {
 	}
 
 	buff := new(bytes.Buffer)
-	buff.ReadFrom(tarReader)
+	_, _ = buff.ReadFrom(tarReader)
 	contents := buff.String()
 
 	_, err = tarReader.Next()
@@ -182,7 +182,7 @@ func TestGetContextFromReaderTar(t *testing.T) {
 	}
 
 	buff := new(bytes.Buffer)
-	buff.ReadFrom(tarReader)
+	_, _ = buff.ReadFrom(tarReader)
 	contents := buff.String()
 
 	_, err = tarReader.Next()
@@ -263,7 +263,7 @@ func chdir(t *testing.T, dir string) {
 }
 
 func TestIsArchive(t *testing.T) {
-	testcases := []struct {
+	tests := []struct {
 		doc      string
 		header   []byte
 		expected bool
@@ -289,13 +289,15 @@ func TestIsArchive(t *testing.T) {
 			expected: false,
 		},
 	}
-	for _, testcase := range testcases {
-		assert.Check(t, is.Equal(testcase.expected, IsArchive(testcase.header)), testcase.doc)
+	for _, tc := range tests {
+		t.Run(tc.doc, func(t *testing.T) {
+			assert.Check(t, is.Equal(tc.expected, isArchive(tc.header)), tc.doc)
+		})
 	}
 }
 
 func TestDetectArchiveReader(t *testing.T) {
-	testcases := []struct {
+	tests := []struct {
 		file     string
 		desc     string
 		expected bool
@@ -316,14 +318,18 @@ func TestDetectArchiveReader(t *testing.T) {
 			expected: false,
 		},
 	}
-	for _, testcase := range testcases {
-		content, err := os.Open(testcase.file)
-		assert.NilError(t, err)
-		defer content.Close()
-
-		_, isArchive, err := DetectArchiveReader(content)
-		assert.NilError(t, err)
-		assert.Check(t, is.Equal(testcase.expected, isArchive), testcase.file)
+	for _, tc := range tests {
+		t.Run(tc.desc, func(t *testing.T) {
+			content, err := os.Open(tc.file)
+			assert.NilError(t, err)
+			defer func() {
+				_ = content.Close()
+			}()
+
+			_, isArchive, err := DetectArchiveReader(content)
+			assert.NilError(t, err)
+			assert.Check(t, is.Equal(tc.expected, isArchive), tc.file)
+		})
 	}
 }
 

cli/command/image/build/dockerignore.go

@@ -21,7 +21,9 @@ func ReadDockerignore(contextDir string) ([]string, error) {
 	case err != nil:
 		return nil, err
 	}
-	defer f.Close()
+	defer func() {
+		_ = f.Close()
+	}()
 
 	patterns, err := ignorefile.ReadAll(f)
 	if err != nil {