Avoid decompressing the chart onto disk

MacroPower · Jan 3, 2025 · a3f8818 · a3f8818
1 parent 7bbeef7
commit a3f8818
Show file tree

Hide file tree

Showing 5 changed files with 56 additions and 35 deletions.
diff --git a/pkg/argoutil/helm/client.go b/pkg/argoutil/helm/client.go
@@ -46,7 +46,8 @@ type Creds struct {
 
 type Client interface {
 	CleanChartCache(chart string, version string, project string) error
-	ExtractChart(chart string, version string, project string, passCredentials bool, manifestMaxExtractedSize int64, disableManifestMaxExtractedSize bool) (string, argoio.Closer, error)
+	PullChart(chart string, version string, project string, passCredentials bool, manifestMaxExtractedSize int64, disableManifestMaxExtractedSize bool) (string, error)
+	ExtractChart(chart string, version string, project string, passCredentials bool, manifestMaxExtractedSize int64, disableManifestMaxExtractedSize bool) (string, io.Closer, error)
 	GetIndex(noCache bool, maxIndexSize int64) (*Index, error)
 	GetTags(chart string, noCache bool) (*TagsList, error)
 	TestHelmOCI() (bool, error)
@@ -125,23 +126,17 @@ func untarChart(tempDir string, cachedChartPath string, manifestMaxExtractedSize
 	return files.Untgz(tempDir, reader, manifestMaxExtractedSize, false)
 }
 
-func (c *nativeHelmChart) ExtractChart(chart string, version string, project string, passCredentials bool, manifestMaxExtractedSize int64, disableManifestMaxExtractedSize bool) (string, argoio.Closer, error) {
+func (c *nativeHelmChart) PullChart(chart string, version string, project string, passCredentials bool, manifestMaxExtractedSize int64, disableManifestMaxExtractedSize bool) (string, error) {
 	// always use Helm V3 since we don't have chart content to determine correct Helm version
 	helmCmd, err := NewCmdWithVersion("", c.enableOci, c.proxy, c.noProxy)
 	if err != nil {
-		return "", nil, fmt.Errorf("error creating Helm command: %w", err)
+		return "", fmt.Errorf("error creating Helm command: %w", err)
 	}
 	defer helmCmd.Close()
 
-	// throw away temp directory that stores extracted chart and should be deleted as soon as no longer needed by returned closer
-	tempDir, err := files.CreateTempDir(os.TempDir())
-	if err != nil {
-		return "", nil, fmt.Errorf("error creating temporary directory: %w", err)
-	}
-
 	cachedChartPath, err := c.getCachedChartPath(chart, version, project)
 	if err != nil {
-		return "", nil, fmt.Errorf("error getting cached chart path: %w", err)
+		return "", fmt.Errorf("error getting cached chart path: %w", err)
 	}
 
 	c.repoLock.Lock(cachedChartPath)
@@ -150,22 +145,22 @@ func (c *nativeHelmChart) ExtractChart(chart string, version string, project str
 	// check if chart tar is already downloaded
 	exists, err := fileExist(cachedChartPath)
 	if err != nil {
-		return "", nil, fmt.Errorf("error checking existence of cached chart path: %w", err)
+		return "", fmt.Errorf("error checking existence of cached chart path: %w", err)
 	}
 
 	if !exists {
 		// create empty temp directory to extract chart from the registry
 		tempDest, err := files.CreateTempDir(os.TempDir())
 		if err != nil {
-			return "", nil, fmt.Errorf("error creating temporary destination directory: %w", err)
+			return "", fmt.Errorf("error creating temporary destination directory: %w", err)
 		}
 		defer func() { _ = os.RemoveAll(tempDest) }()
 
 		if c.enableOci {
 			if c.creds.Password != "" && c.creds.Username != "" {
 				_, err = helmCmd.RegistryLogin(c.repoURL, c.creds)
 				if err != nil {
-					return "", nil, fmt.Errorf("error logging into OCI registry: %w", err)
+					return "", fmt.Errorf("error logging into OCI registry: %w", err)
 				}
 
 				defer func() {
@@ -176,37 +171,53 @@ func (c *nativeHelmChart) ExtractChart(chart string, version string, project str
 			// 'helm pull' ensures that chart is downloaded into temp directory
 			_, err = helmCmd.PullOCI(c.repoURL, chart, version, tempDest, c.creds)
 			if err != nil {
-				return "", nil, fmt.Errorf("error pulling OCI chart: %w", err)
+				return "", fmt.Errorf("error pulling OCI chart: %w", err)
 			}
 		} else {
 			_, err = helmCmd.Fetch(c.repoURL, chart, version, tempDest, c.creds, passCredentials)
 			if err != nil {
-				return "", nil, fmt.Errorf("error fetching chart: %w", err)
+				return "", fmt.Errorf("error fetching chart: %w", err)
 			}
 		}
 
 		// 'helm pull/fetch' file downloads chart into the tgz file and we move that to where we want it
 		infos, err := os.ReadDir(tempDest)
 		if err != nil {
-			return "", nil, fmt.Errorf("error reading directory %s: %w", tempDest, err)
+			return "", fmt.Errorf("error reading directory %s: %w", tempDest, err)
 		}
 		if len(infos) != 1 {
-			return "", nil, fmt.Errorf("expected 1 file, found %v", len(infos))
+			return "", fmt.Errorf("expected 1 file, found %v", len(infos))
 		}
 
 		chartFilePath := filepath.Join(tempDest, infos[0].Name())
 
 		err = os.Rename(chartFilePath, cachedChartPath)
 		if err != nil {
-			return "", nil, fmt.Errorf("error renaming file from %s to %s: %w", chartFilePath, cachedChartPath, err)
+			return "", fmt.Errorf("error renaming file from %s to %s: %w", chartFilePath, cachedChartPath, err)
 		}
 	}
 
+	return cachedChartPath, nil
+}
+
+func (c *nativeHelmChart) ExtractChart(chart string, version string, project string, passCredentials bool, manifestMaxExtractedSize int64, disableManifestMaxExtractedSize bool) (string, io.Closer, error) {
+	// throw away temp directory that stores extracted chart and should be deleted as soon as no longer needed by returned closer
+	tempDir, err := files.CreateTempDir(os.TempDir())
+	if err != nil {
+		return "", nil, fmt.Errorf("error creating temporary directory: %w", err)
+	}
+
+	cachedChartPath, err := c.PullChart(chart, version, project, passCredentials, manifestMaxExtractedSize, disableManifestMaxExtractedSize)
+	if err != nil {
+		return "", nil, fmt.Errorf("error extracting chart: %w", err)
+	}
+
 	err = untarChart(tempDir, cachedChartPath, manifestMaxExtractedSize, disableManifestMaxExtractedSize)
 	if err != nil {
 		_ = os.RemoveAll(tempDir)
 		return "", nil, fmt.Errorf("error untarring chart: %w", err)
 	}
+
 	return path.Join(tempDir, normalizeChartName(chart)), argoio.NewCloser(func() error {
 		return os.RemoveAll(tempDir)
 	}), nil

diff --git a/pkg/argoutil/helm/cmd.go b/pkg/argoutil/helm/cmd.go
@@ -255,11 +255,11 @@ func (c *Cmd) inspectValues(values string) (string, error) {
 }
 
 func (c *Cmd) template(chartPath string, opts *TemplateOpts) (string, string, error) {
-	if callback, err := cleanupChartLockFile(filepath.Clean(path.Join(c.WorkDir, chartPath))); err == nil {
-		defer callback()
-	} else {
-		return "", "", fmt.Errorf("failed to clean up chart lock file: %w", err)
-	}
+	// if callback, err := cleanupChartLockFile(filepath.Clean(path.Join(c.WorkDir, chartPath))); err == nil {
+	// 	defer callback()
+	// } else {
+	// 	return "", "", fmt.Errorf("failed to clean up chart lock file: %w", err)
+	// }
 
 	// Fail open instead of blocking the template.
 	kv := &chartutil.KubeVersion{

diff --git a/pkg/helm/chart.go b/pkg/helm/chart.go
@@ -38,7 +38,7 @@ type TemplateOpts struct {
 }
 
 type ChartClient interface {
-	PullWithCreds(chart, repoURL, targetRevision string, creds Creds, passCredentials bool) (string, io.Closer, error)
+	PullWithCreds(chart, repoURL, targetRevision string, creds Creds, extract, passCredentials bool) (string, io.Closer, error)
 }
 
 type JSONSchemaGenerator interface {
@@ -72,7 +72,7 @@ func (c *Chart) Template() ([]*unstructured.Unstructured, error) {
 
 func (c *Chart) template() ([]byte, error) {
 	chartPath, closer, err := c.Client.PullWithCreds(c.TemplateOpts.ChartName, c.TemplateOpts.RepoURL,
-		c.TemplateOpts.TargetRevision, c.TemplateOpts.Credentials, c.TemplateOpts.PassCredentials)
+		c.TemplateOpts.TargetRevision, c.TemplateOpts.Credentials, false, c.TemplateOpts.PassCredentials)
 	if err != nil {
 		return nil, fmt.Errorf("error pulling helm chart: %w", err)
 	}
@@ -120,7 +120,7 @@ func (c *Chart) template() ([]byte, error) {
 // of the pulled files in the chart directory for JSON Schema generation.
 func (c *Chart) GetValuesJSONSchema(gen JSONSchemaGenerator, match func(string) bool) ([]byte, error) {
 	chartPath, closer, err := c.Client.PullWithCreds(c.TemplateOpts.ChartName, c.TemplateOpts.RepoURL,
-		c.TemplateOpts.TargetRevision, c.TemplateOpts.Credentials, c.TemplateOpts.PassCredentials)
+		c.TemplateOpts.TargetRevision, c.TemplateOpts.Credentials, true, c.TemplateOpts.PassCredentials)
 	if err != nil {
 		return nil, fmt.Errorf("error pulling helm chart: %w", err)
 	}

diff --git a/pkg/helm/client.go b/pkg/helm/client.go
@@ -70,12 +70,12 @@ func MustNewClient(paths PathCacher, project, maxExtractSize string) *Client {
 // the extracted chart. Pulled charts will be stored in the injected [PathCacher]
 // in .tar.gz format, and subsequent requests will try to use [PathCacher] rather
 // than re-pulling the chart.
-func (c *Client) Pull(chart, repoURL, targetRevision string) (string, io.Closer, error) {
-	return c.PullWithCreds(chart, repoURL, targetRevision, Creds{}, false)
+func (c *Client) Pull(chart, repoURL, targetRevision string, extract bool) (string, io.Closer, error) {
+	return c.PullWithCreds(chart, repoURL, targetRevision, Creds{}, extract, false)
 }
 
 func (c *Client) PullWithCreds(
-	chart, repoURL, targetRevision string, creds Creds, passCredentials bool,
+	chart, repoURL, targetRevision string, creds Creds, extract, passCredentials bool,
 ) (string, io.Closer, error) {
 	repoNetURL, err := url.Parse(repoURL)
 	if err != nil {
@@ -109,12 +109,22 @@ func (c *Client) PullWithCreds(
 	ahc := argohelm.NewClient(repoNetURL.String(), argoCreds, enableOCI, c.Proxy, c.NoProxy,
 		argohelm.WithChartPaths(c.Paths))
 
+	var chartPath string
+	if !extract {
+		closer := io.NopCloser(bytes.NewReader(nil))
+		chartPath, err = ahc.PullChart(chart, targetRevision, c.Project, passCredentials,
+			c.MaxExtractSize.Value(), c.MaxExtractSize.IsZero())
+		if err != nil {
+			return "", closer, fmt.Errorf("error extracting helm chart: %w", err)
+		}
+		return chartPath, closer, nil
+	}
+
 	chartPath, closer, err := ahc.ExtractChart(chart, targetRevision, c.Project, passCredentials,
 		c.MaxExtractSize.Value(), c.MaxExtractSize.IsZero())
 	if err != nil {
 		return "", closer, fmt.Errorf("error extracting helm chart: %w", err)
 	}
-
 	return chartPath, closer, nil
 }
 

diff --git a/pkg/helmtest/clienttest.go b/pkg/helmtest/clienttest.go
@@ -24,21 +24,21 @@ func init() {
 }
 
 type ChartClient interface {
-	Pull(chart, repoURL, targetRevision string) (string, io.Closer, error)
+	Pull(chart, repoURL, targetRevision string, extract bool) (string, io.Closer, error)
 }
 
 type TestClient struct {
 	BaseClient ChartClient
 }
 
-func (c *TestClient) Pull(chart, repoURL, targetRevision string) (string, io.Closer, error) {
-	return c.PullWithCreds(chart, repoURL, targetRevision, helm.Creds{}, false)
+func (c *TestClient) Pull(chart, repoURL, targetRevision string, extract bool) (string, io.Closer, error) {
+	return c.PullWithCreds(chart, repoURL, targetRevision, helm.Creds{}, extract, false)
 }
 
 func (c *TestClient) PullWithCreds(
-	chart, repoURL, targetRevision string, _ helm.Creds, _ bool,
+	chart, repoURL, targetRevision string, _ helm.Creds, extract, _ bool,
 ) (string, io.Closer, error) {
-	chartPath, closer, err := c.BaseClient.Pull(chart, repoURL, targetRevision)
+	chartPath, closer, err := c.BaseClient.Pull(chart, repoURL, targetRevision, extract)
 	if err != nil {
 		return "", closer, fmt.Errorf("error pulling helm chart: %w", err)
 	}