Refactor DownloadManifests in deploy for clarity and modularity

.golangci.yml

@@ -71,6 +71,12 @@ linters-settings:
   perfsprint:
     sprintf1: false
     strconcat: false
+  # Enable gocritic for detecting bugs, performance, and style issues: https://golangci-lint.run/usage/linters/#gocritic
+  gocritic:
+    # https://go-critic.com/overview.html#checkers
+    enabled-checks:
+      - deferInLoop
+      - unnecessaryDefer
 
 linters:
   enable-all: true

pkg/deploy/deploy.go

@@ -52,19 +52,21 @@ import (
 )
 
 var (
-	DefaultManifestPath = os.Getenv("DEFAULT_MANIFESTS_PATH")
+	DefaultManifestPath     = os.Getenv("DEFAULT_MANIFESTS_PATH")
+	errPathResolutionFailed = errors.New("path resolution failed")
+	errPathIrrelevant       = errors.New("path is irrelevant")
 )
 
 // DownloadManifests function performs following tasks:
 // 1. It takes component URI and only downloads folder specified by component.ContextDir field
 // 2. It saves the manifests in the odh-manifests/component-name/ folder.
 func DownloadManifests(ctx context.Context, componentName string, manifestConfig common.ManifestsConfig) error {
-	// Get the component repo from the given url
-	// e.g.  https://github.com/example/tarball/master
+	// Download and validate the manifest archive from the given url, e.g.  https://github.com/example/tarball/master
 	req, err := http.NewRequestWithContext(ctx, http.MethodGet, manifestConfig.URI, nil)
 	if err != nil {
 		return err
 	}
+
 	resp, err := http.DefaultClient.Do(req)
 	if err != nil {
 		return fmt.Errorf("error downloading manifests: %w", err)
@@ -75,76 +77,111 @@ func DownloadManifests(ctx context.Context, componentName string, manifestConfig
 		return fmt.Errorf("error downloading manifests: %v HTTP status", resp.StatusCode)
 	}
 
-	// Create a new gzip reader
+	// Initialize a gzip reader for the response body
 	gzipReader, err := gzip.NewReader(resp.Body)
 	if err != nil {
 		return fmt.Errorf("error creating gzip reader: %w", err)
 	}
 	defer gzipReader.Close()
 
-	// Create a new TAR reader
-	tarReader := tar.NewReader(gzipReader)
+	// Ensure manifest directory exists
+	if err := createDirectory(DefaultManifestPath); err != nil {
+		return err
+	}
+
+	// Extract TAR contents
+	return unpackTarFromReader(gzipReader, DefaultManifestPath, componentName, manifestConfig.ContextDir)
+}
 
-	// Create manifest directory
-	mode := os.ModePerm
-	err = os.MkdirAll(DefaultManifestPath, mode)
+// createDirectory ensures the specified directory exists, creating it if necessary.
+func createDirectory(path string) error {
+	err := os.MkdirAll(path, os.ModePerm)
 	if err != nil {
-		return fmt.Errorf("error creating manifests directory : %w", err)
+		return fmt.Errorf("error creating directory %s: %w", path, err)
 	}
+	return nil
+}
+
+// unpackTarFromReader extracts files from a TAR reader into the target base path.
+func unpackTarFromReader(reader io.Reader, basePath, componentName, contextDir string) error {
+	tarReader := tar.NewReader(reader)
 
-	// Extract the contents of the TAR archive to the current directory
 	for {
 		header, err := tarReader.Next()
 		if errors.Is(err, io.EOF) {
 			break
 		}
+		if err != nil {
+			return fmt.Errorf("error reading tar header: %w", err)
+		}
+
+		targetPath, err := resolveTargetPath(header.Name, basePath, componentName, contextDir)
+		if errors.Is(err, errPathIrrelevant) {
+			continue
+		}
 		if err != nil {
 			return err
 		}
 
-		componentFiles := strings.Split(header.Name, "/")
-		componentFileName := header.Name
-		componentManifestPath := componentFiles[0] + "/" + manifestConfig.ContextDir
+		err = extractFileOrDirectory(header, tarReader, targetPath)
+		if err != nil {
+			return err
+		}
+	}
 
-		if strings.Contains(componentFileName, componentManifestPath) {
-			// Get manifest path relative to repo
-			// e.g. of repo/a/b/manifests/base --> base/
-			componentFoldersList := strings.Split(componentFileName, "/")
-			componentFileRelativePathFound := strings.Join(componentFoldersList[len(strings.Split(componentManifestPath, "/")):], "/")
+	return nil
+}
 
-			if header.Typeflag == tar.TypeDir {
-				err = os.MkdirAll(DefaultManifestPath+"/"+componentName+"/"+componentFileRelativePathFound, mode)
-				if err != nil {
-					return fmt.Errorf("error creating directory:%w", err)
-				}
+// resolveTargetPath computes the target file path based on the tar header and context directory.
+func resolveTargetPath(headerName, basePath, contextDir, componentName string) (string, error) {
+	componentFiles := strings.Split(headerName, "/")
+	componentManifestPath := filepath.Join(componentFiles[0], contextDir)
 
-				continue
-			}
+	if !strings.Contains(headerName, componentManifestPath) {
+		return "", errPathIrrelevant
+	}
 
-			if header.Typeflag == tar.TypeReg {
-				file, err := os.Create(DefaultManifestPath + "/" + componentName + "/" + componentFileRelativePathFound)
-				if err != nil {
-					return fmt.Errorf("error creating file: %w", err)
-				}
-
-				defer file.Close()
-
-				for {
-					_, err := io.CopyN(file, tarReader, 1024)
-					if err != nil {
-						if errors.Is(err, io.EOF) {
-							break
-						}
-						return fmt.Errorf("error downloading file contents: %w", err)
-					}
-				}
-
-				continue
-			}
-		}
+	componentFoldersList := strings.Split(headerName, "/")
+	if len(componentFoldersList) < len(strings.Split(componentManifestPath, "/")) {
+		return "", errPathResolutionFailed // Path resolution failed
 	}
 
-	return err
+	relativePath := strings.Join(componentFoldersList[len(strings.Split(componentManifestPath, "/")):], "/")
+
+	return filepath.Join(basePath, componentName, relativePath), nil
+}
+
+// processTarHeader processes a TAR header, creating files or directories as needed.
+func extractFileOrDirectory(header *tar.Header, tarReader *tar.Reader, targetPath string) error {
+	switch header.Typeflag {
+	case tar.TypeDir:
+		// Create a directory for the current header
+		return createDirectory(targetPath)
+
+	case tar.TypeReg:
+		// Create a file and copy its contents from the TAR reader
+		return writeFileFromTar(targetPath, tarReader)
+
+	default:
+		// Handle unsupported header types if needed
+		return nil
+	}
+}
+
+// writeFileFromTar writes a file from the tar reader to the target path.
+func writeFileFromTar(targetPath string, tarReader *tar.Reader) error {
+	file, err := os.Create(targetPath)
+	if err != nil {
+		return fmt.Errorf("error creating file %s: %w", targetPath, err)
+	}
+	defer file.Close()
+
+	_, err = io.Copy(file, tarReader)
+	if err != nil {
+		return fmt.Errorf("error writing to file %s: %w", targetPath, err)
+	}
+
+	return nil
 }
 
 func DeployManifestsFromPath(