feat: expose digest for all charmhub download methods

Expose the digest for all charmhub download methods. This means we can start using it in the charm downloader. We don't need to reverify every single time now.
sinanawad · Nov 21, 2024 · cdfd6ef · cdfd6ef
1 parent 1b120a7
commit cdfd6ef
Show file tree

Hide file tree

Showing 19 changed files with 163 additions and 108 deletions.
diff --git a/apiserver/facades/client/charms/mocks/repository.go b/apiserver/facades/client/charms/mocks/repository.go
diff --git a/cmd/juju/application/refresh_test.go b/cmd/juju/application/refresh_test.go
@@ -1285,7 +1285,7 @@ type mockDownloadBundleClient struct {
 	bundle charm.Bundle
 }
 
-func (m *mockDownloadBundleClient) DownloadAndReadBundle(_ context.Context, resourceURL *url.URL, archivePath string, _ ...charmhub.DownloadOption) (charm.Bundle, error) {
+func (m *mockDownloadBundleClient) DownloadAndReadBundle(_ context.Context, resourceURL *url.URL, archivePath string, _ ...charmhub.DownloadOption) (charm.Bundle, *charmhub.Digest, error) {
 	m.MethodCall(m, "DownloadAndReadBundle", resourceURL, archivePath)
-	return m.bundle, m.NextErr()
+	return m.bundle, &charmhub.Digest{}, m.NextErr()
 }
diff --git a/cmd/juju/application/store/charmadaptor.go b/cmd/juju/application/store/charmadaptor.go
@@ -20,7 +20,7 @@ import (
 // DownloadBundleClient represents a way to download a bundle from a given
 // resource URL.
 type DownloadBundleClient interface {
-	DownloadAndReadBundle(context.Context, *url.URL, string, ...charmhub.DownloadOption) (charm.Bundle, error)
+	DownloadAndReadBundle(context.Context, *url.URL, string, ...charmhub.DownloadOption) (charm.Bundle, *charmhub.Digest, error)
 }
 
 // DownloadBundleClientFunc lazily construct a download bundle client.
@@ -120,5 +120,9 @@ func (ch chBundleFactory) GetBundle(ctx context.Context, curl *charm.URL, origin
 	if err != nil {
 		return nil, errors.Trace(err)
 	}
-	return client.DownloadAndReadBundle(ctx, url, path)
+	bundle, _, err := client.DownloadAndReadBundle(ctx, url, path)
+	if err != nil {
+		return nil, errors.Trace(err)
+	}
+	return bundle, nil
 }
diff --git a/cmd/juju/application/store/charmadaptor_test.go b/cmd/juju/application/store/charmadaptor_test.go
@@ -18,6 +18,7 @@ import (
 	"github.com/juju/juju/cmd/juju/application/store/mocks"
 	"github.com/juju/juju/core/base"
 	"github.com/juju/juju/internal/charm"
+	"github.com/juju/juju/internal/charmhub"
 )
 
 type resolveSuite struct {
@@ -213,5 +214,5 @@ func (s *resolveSuite) expectedCharmHubGetBundle(c *gc.C, curl *charm.URL, origi
 	}, nil)
 	url, err := url.Parse(surl)
 	c.Assert(err, jc.ErrorIsNil)
-	s.downloadClient.EXPECT().DownloadAndReadBundle(gomock.Any(), url, "/tmp/", gomock.Any()).Return(s.bundle, nil)
+	s.downloadClient.EXPECT().DownloadAndReadBundle(gomock.Any(), url, "/tmp/", gomock.Any()).Return(s.bundle, &charmhub.Digest{}, nil)
 }
diff --git a/cmd/juju/application/store/mocks/store_mock.go b/cmd/juju/application/store/mocks/store_mock.go
diff --git a/cmd/juju/charmhub/download.go b/cmd/juju/charmhub/download.go
@@ -338,7 +338,7 @@ func (c *downloadCommand) calculateHashFromDigest(path string, digest *charmhub.
 	if digest.DigestType != charmhub.SHA256 {
 		return "", errors.Errorf("expected SHA256 digest, got %s", digest.DigestType)
 	}
-	return digest.Value, nil
+	return digest.Hash, nil
 }
 
 func (c *downloadCommand) refresh(

diff --git a/cmd/juju/charmhub/download_test.go b/cmd/juju/charmhub/download_test.go
@@ -423,7 +423,8 @@ func (s *downloadSuite) expectDownload(c *gc.C, charmHubURL string) {
 	c.Assert(err, jc.ErrorIsNil)
 	s.charmHubAPI.EXPECT().Download(gomock.Any(), resourceURL, "test_r123.charm", gomock.Any(), gomock.Any()).Return(&charmhub.Digest{
 		DigestType: charmhub.SHA256,
-		Value:      "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855",
+		Hash:       "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855",
+		Size:       42,
 	}, nil)
 }
 
@@ -432,6 +433,7 @@ func (s *downloadSuite) expectResourceDownload(c *gc.C, resourceDownloadURL stri
 	c.Assert(err, jc.ErrorIsNil)
 	s.charmHubAPI.EXPECT().Download(gomock.Any(), resourceURL, "resource_foo_r5_a.tar.gz", gomock.Any(), gomock.Any()).Return(&charmhub.Digest{
 		DigestType: charmhub.SHA256,
-		Value:      "533513c1397cb8ccec05852b52514becd5fd8c9c21509f7bc2f5d460c6143dd8",
+		Hash:       "533513c1397cb8ccec05852b52514becd5fd8c9c21509f7bc2f5d460c6143dd8",
+		Size:       42,
 	}, nil)
 }
diff --git a/core/charm/repository.go b/core/charm/repository.go
@@ -9,6 +9,7 @@ import (
 
 	"github.com/juju/juju/internal/charm"
 	charmresource "github.com/juju/juju/internal/charm/resource"
+	"github.com/juju/juju/internal/charmhub"
 )
 
 // Repository describes an API for querying charm/bundle information and
@@ -21,7 +22,7 @@ type Repository interface {
 
 	// DownloadCharm retrieves specified charm from the store and saves its
 	// contents to the specified path.
-	DownloadCharm(ctx context.Context, charmName string, requestedOrigin Origin, archivePath string) (CharmArchive, Origin, error)
+	DownloadCharm(ctx context.Context, charmName string, requestedOrigin Origin, archivePath string) (CharmArchive, Origin, *charmhub.Digest, error)
 
 	// ResolveWithPreferredChannel verified that the charm with the requested
 	// channel exists.  If no channel is specified, the latests, most stable is

diff --git a/internal/bootstrap/charm_mock_test.go b/internal/bootstrap/charm_mock_test.go
diff --git a/internal/charm/downloader/downloader.go b/internal/charm/downloader/downloader.go
@@ -11,14 +11,14 @@ import (
 	"strings"
 
 	"github.com/juju/errors"
-	"github.com/juju/utils/v4"
 
 	"github.com/juju/juju/core/arch"
 	corecharm "github.com/juju/juju/core/charm"
 	"github.com/juju/juju/core/logger"
 	"github.com/juju/juju/core/lxdprofile"
 	"github.com/juju/juju/core/version"
 	"github.com/juju/juju/internal/charm"
+	"github.com/juju/juju/internal/charmhub"
 )
 
 // CharmArchive provides information about a downloaded charm archive.
@@ -30,7 +30,7 @@ type CharmArchive interface {
 type CharmRepository interface {
 	GetDownloadURL(context.Context, string, corecharm.Origin) (*url.URL, corecharm.Origin, error)
 	ResolveWithPreferredChannel(ctx context.Context, charmName string, requestedOrigin corecharm.Origin) (*charm.URL, corecharm.Origin, []corecharm.Platform, error)
-	DownloadCharm(ctx context.Context, charmName string, requestedOrigin corecharm.Origin, archivePath string) (corecharm.CharmArchive, corecharm.Origin, error)
+	DownloadCharm(ctx context.Context, charmName string, requestedOrigin corecharm.Origin, archivePath string) (corecharm.CharmArchive, corecharm.Origin, *charmhub.Digest, error)
 }
 
 // RepositoryGetter returns a suitable CharmRepository for the specified Source.
@@ -172,31 +172,17 @@ func (d *Downloader) DownloadAndStore(ctx context.Context, charmURL *charm.URL,
 
 func (d *Downloader) downloadAndHash(ctx context.Context, charmName string, requestedOrigin corecharm.Origin, repo CharmRepository, dstPath string) (DownloadedCharm, corecharm.Origin, error) {
 	d.logger.Debugf("downloading charm %q from requested origin %v", charmName, requestedOrigin)
-	chArchive, actualOrigin, err := repo.DownloadCharm(ctx, charmName, requestedOrigin, dstPath)
+	chArchive, actualOrigin, digest, err := repo.DownloadCharm(ctx, charmName, requestedOrigin, dstPath)
 	if err != nil {
 		return DownloadedCharm{}, corecharm.Origin{}, errors.Trace(err)
 	}
 	d.logger.Debugf("downloaded charm %q from actual origin %v", charmName, actualOrigin)
-
-	// Calculate SHA256 for the downloaded archive
-	f, err := os.Open(dstPath)
-	if err != nil {
-		return DownloadedCharm{}, corecharm.Origin{}, errors.Annotatef(err, "cannot read downloaded charm")
-	}
-	defer func() { _ = f.Close() }()
-
-	sha, size, err := utils.ReadSHA256(f)
-	if err != nil {
-		return DownloadedCharm{}, corecharm.Origin{}, errors.Annotate(err, "cannot calculate SHA256 hash of charm")
-	}
-
-	d.logger.Tracef("downloadResult(%q) sha: %q, size: %d", f.Name(), sha, size)
 	return DownloadedCharm{
 		Charm:        chArchive,
 		CharmVersion: chArchive.Version(),
-		Size:         size,
+		Size:         digest.Size,
 		LXDProfile:   chArchive.LXDProfile(),
-		SHA256:       sha,
+		SHA256:       digest.Hash,
 	}, actualOrigin, nil
 }
 

diff --git a/internal/charm/downloader/downloader_test.go b/internal/charm/downloader/downloader_test.go
@@ -21,6 +21,7 @@ import (
 	"github.com/juju/juju/internal/charm"
 	"github.com/juju/juju/internal/charm/downloader"
 	"github.com/juju/juju/internal/charm/downloader/mocks"
+	"github.com/juju/juju/internal/charmhub"
 	loggertesting "github.com/juju/juju/internal/logger/testing"
 )
 
@@ -128,7 +129,11 @@ func (s *downloaderSuite) TestDownloadAndHash(c *gc.C) {
 	requestedOrigin := corecharm.Origin{Source: corecharm.CharmHub, Channel: mustParseChannel(c, "20.04/edge")}
 	resolvedOrigin := corecharm.Origin{Source: corecharm.CharmHub, Channel: mustParseChannel(c, "20.04/candidate")}
 
-	s.repo.EXPECT().DownloadCharm(gomock.Any(), name, requestedOrigin, tmpFile).Return(s.charmArchive, resolvedOrigin, nil)
+	s.repo.EXPECT().DownloadCharm(gomock.Any(), name, requestedOrigin, tmpFile).Return(s.charmArchive, resolvedOrigin, &charmhub.Digest{
+		DigestType: charmhub.SHA256,
+		Hash:       "4e97ed7423be2ea12939e8fdd592cfb3dcd4d0097d7d193ef998ab6b4db70461",
+		Size:       10,
+	}, nil)
 	s.charmArchive.EXPECT().Version().Return("the-version")
 	s.charmArchive.EXPECT().LXDProfile().Return(nil)
 
@@ -228,11 +233,15 @@ func (s downloaderSuite) TestDownloadAndStore(c *gc.C) {
 			return "", nil
 		},
 	)
-	s.repoGetter.EXPECT().GetCharmRepository(gomock.Any(), corecharm.CharmHub).Return(repoAdaptor{s.repo}, nil)
+	s.repoGetter.EXPECT().GetCharmRepository(gomock.Any(), corecharm.CharmHub).Return(repoAdaptor{repo: s.repo}, nil)
 	s.repo.EXPECT().DownloadCharm(gomock.Any(), curl.Name, requestedOriginWithPlatform, gomock.Any()).DoAndReturn(
-		func(ctx context.Context, _ string, requestedOrigin corecharm.Origin, archivePath string) (corecharm.CharmArchive, corecharm.Origin, error) {
+		func(ctx context.Context, _ string, requestedOrigin corecharm.Origin, archivePath string) (corecharm.CharmArchive, corecharm.Origin, *charmhub.Digest, error) {
 			c.Assert(os.WriteFile(archivePath, []byte("meshuggah\n"), 0644), jc.ErrorIsNil)
-			return s.charmArchive, resolvedOrigin, nil
+			return s.charmArchive, resolvedOrigin, &charmhub.Digest{
+				DigestType: charmhub.SHA256,
+				Hash:       "4e97ed7423be2ea12939e8fdd592cfb3dcd4d0097d7d193ef998ab6b4db70461",
+				Size:       10,
+			}, nil
 		},
 	)
 	s.charmArchive.EXPECT().Meta().Return(&charm.Meta{
@@ -275,7 +284,7 @@ type repoAdaptor struct {
 	repo *mocks.MockCharmRepository
 }
 
-func (r repoAdaptor) DownloadCharm(ctx context.Context, charmName string, requestedOrigin corecharm.Origin, archivePath string) (corecharm.CharmArchive, corecharm.Origin, error) {
+func (r repoAdaptor) DownloadCharm(ctx context.Context, charmName string, requestedOrigin corecharm.Origin, archivePath string) (corecharm.CharmArchive, corecharm.Origin, *charmhub.Digest, error) {
 	return r.repo.DownloadCharm(ctx, charmName, requestedOrigin, archivePath)
 }
 

diff --git a/internal/charm/downloader/mocks/charm_archive_mocks.go b/internal/charm/downloader/mocks/charm_archive_mocks.go