Merge pull request juju#18466 from SimonRichardson/include-charm-down…

…load-info

juju#18466

Store the charm download information on the very first call when either setting a charm or creating an application. The driver behind this change is to allow the async charm downloader to retrieve the download URL from the application service without renegotiating the charm download URL again.

Before this change, the deployment code was reliant on the charmhub store being up, even though the charm was resolved with the essential metadata (metadata, config). This also removes any chance of accidentally getting the wrong charm blob for the request. Lastly, it was possible to deploy an application and then change the charmhub URL in the model config to a proxy, which would then prevent the download of the charm. This scenario then becomes obsolete, the charm download URL is stored and we've just have direct access.

----

In order to push this through, we only store the download information for charmhub charms. It's in a new table called `charm_download_info`. We're introducing a new concept called a `charm_provenance` (I'm not wedded to that name though). It should indicate at what stage in the apiserver/service acquisition did we get the download information. The async charm downloader should only care about the `download` provenance.

We won't have the download information for every iteration of the provenance, only `download` currently. Both `migration` and `upload` require changes to 3.6 to ensure that we have all that information and I'm loathed to do that at this time. At bootstrap, the `bootstrap` download information will be plumbed in at a later date.

----

This pull request introduces several changes aimed at enhancing the handling of charm download information within the application deployment process. The primary modifications include adding a new `DownloadInfo` structure, updating various functions to utilize this new structure, and adjusting mock services accordingly.

Key changes include:

### Enhancements to Charm Download Information Handling:
* Added `DownloadInfo` structure to store details necessary for downloading charms from CharmHub, including provenance, identifier, URL, and size (`core/charm/repository.go`, `domain/application/charm/types.go`). [[1]](diffhunk://#diff-5ae9a3927c9b2dc399e7a5f3ec6eed25fd35797095619ae9a9024754de846645R117-R130) [[2]](diffhunk://#diff-fc00f97e772cd00ee380e1996104a5ca579a356a9a07028d1d75f28161df0bbfR156-R207)
* Updated `DeployApplication` and `DeployFromRepository` functions to handle `DownloadInfo` when deploying charms (`apiserver/facades/client/application/deploy.go`, `apiserver/facades/client/application/deployrepository.go`). [[1]](diffhunk://#diff-1939e2205f5e423b58928b9628a4023f3d526a88e846da6ef2c5b0d14a7148edR169-R189) [[2]](diffhunk://#diff-fec6df289495bdefe1c76addc3c69be52ed4cfffccd124b83e06f9dc3abba18cR173-R179)
* Modified `GetCharmDownloadInfo` method in `ApplicationService` interface to retrieve charm download information using charm ID (`apiserver/facades/client/application/service.go`).

### Adjustments to Validation and Resource Resolution:
* Refined the `validate` function in `deployrepository.go` to incorporate `DownloadInfo` during charm validation and resource resolution (`apiserver/facades/client/application/deployrepository.go`). [[1]](diffhunk://#diff-fec6df289495bdefe1c76addc3c69be52ed4cfffccd124b83e06f9dc3abba18cL399-R423) [[2]](diffhunk://#diff-fec6df289495bdefe1c76addc3c69be52ed4cfffccd124b83e06f9dc3abba18cL427-R443)
* Introduced `charmResult` struct to encapsulate charm-related data, including `DownloadInfo`, in the `getCharm` function (`apiserver/facades/client/application/deployrepository.go`). [[1]](diffhunk://#diff-fec6df289495bdefe1c76addc3c69be52ed4cfffccd124b83e06f9dc3abba18cR901-R913) [[2]](diffhunk://#diff-fec6df289495bdefe1c76addc3c69be52ed4cfffccd124b83e06f9dc3abba18cL906-R933)

### Updates to Mock Services:
* Added mock implementations for `GetCharmDownloadInfo` in `MockApplicationService` to support unit testing (`apiserver/facades/client/application/services_mock_test.go`).

These changes collectively enhance the robustness and flexibility of the charm deployment process by ensuring that detailed download information is consistently handled and validated across various components of the application.

## QA steps

Bundles are currently toast, so we'll need to sweep back around and fix them at some point.

```sh
$ juju bootstrap lxd test
$ juju add-model default
$ juju deploy ubuntu
$ make repl-install
$ make repl
dqlite> SELECT prdesc FROM namespace_list
$ DB_NAME="<select the last one in the list>" make repl
dqlite> SELECT prdesc FROM charm
dqlite> SELECT prdesc FROM charm_download_info
```

Testing migration should also be done.


## Links

https://warthogs.atlassian.net/browse/JUJU-7166

apiserver/facades/agent/caasapplication/application_test.go

@@ -21,6 +21,7 @@ import (
 	"github.com/juju/juju/caas"
 	corecharm "github.com/juju/juju/core/charm"
 	"github.com/juju/juju/core/unit"
+	applicationcharm "github.com/juju/juju/domain/application/charm"
 	"github.com/juju/juju/domain/application/service"
 	"github.com/juju/juju/domain/services/testing"
 	loggertesting "github.com/juju/juju/internal/logger/testing"
@@ -73,6 +74,9 @@ func (s *CAASApplicationSuite) SetUpTest(c *gc.C) {
 	_, err := s.applicationService.CreateApplication(
 		context.Background(), "gitlab", &stubCharm{}, origin, service.AddApplicationArgs{
 			ReferenceName: "gitlab",
+			DownloadInfo: &applicationcharm.DownloadInfo{
+				Provenance: applicationcharm.ProvenanceUpload,
+			},
 		}, service.AddUnitArg{
 			UnitName: unitName,
 		})

apiserver/facades/agent/caasapplication/mock_test.go

@@ -246,7 +246,13 @@ func (s *stubCharm) Meta() *charm.Meta {
 }
 
 func (s *stubCharm) Manifest() *charm.Manifest {
-	return &charm.Manifest{}
+	return &charm.Manifest{
+		Bases: []charm.Base{{
+			Name:          "ubuntu",
+			Channel:       charm.Channel{Risk: charm.Stable},
+			Architectures: []string{"amd64"},
+		}},
+	}
 }
 
 func (s *stubCharm) Config() *charm.Config {

apiserver/facades/client/application/deploy.go

@@ -21,6 +21,7 @@ import (
 	coremodel "github.com/juju/juju/core/model"
 	"github.com/juju/juju/core/objectstore"
 	coreunit "github.com/juju/juju/core/unit"
+	applicationcharm "github.com/juju/juju/domain/application/charm"
 	applicationservice "github.com/juju/juju/domain/application/service"
 	machineerrors "github.com/juju/juju/domain/machine/errors"
 	"github.com/juju/juju/environs/bootstrap"
@@ -166,9 +167,27 @@ func DeployApplication(
 			return nil, errors.Trace(err)
 		}
 
+		var downloadInfo *applicationcharm.DownloadInfo
+		if args.CharmOrigin.Source == corecharm.CharmHub {
+			charmID, err := applicationService.GetCharmID(ctx, applicationcharm.GetCharmArgs{
+				Source:   applicationcharm.CharmHubSource,
+				Name:     args.ApplicationName,
+				Revision: args.CharmOrigin.Revision,
+			})
+			if err != nil {
+				return nil, errors.Trace(err)
+			}
+
+			downloadInfo, err = applicationService.GetCharmDownloadInfo(ctx, charmID)
+			if err != nil {
+				return nil, errors.Trace(err)
+			}
+		}
+
 		_, err = applicationService.CreateApplication(ctx, args.ApplicationName, args.Charm, args.CharmOrigin, applicationservice.AddApplicationArgs{
 			ReferenceName: chURL.Name,
 			Storage:       args.Storage,
+			DownloadInfo:  downloadInfo,
 		}, unitArgs...)
 		if err != nil {
 			return nil, errors.Trace(err)

apiserver/facades/client/application/deployrepository.go

@@ -171,6 +171,13 @@ func (api *DeployFromRepositoryAPI) DeployFromRepository(ctx context.Context, ar
 		_, addApplicationErr = api.applicationService.CreateApplication(ctx, dt.applicationName, ch, dt.origin, applicationservice.AddApplicationArgs{
 			ReferenceName: dt.charmURL.Name,
 			Storage:       dt.storage,
+			// We always have download info for a charm from the charmhub store.
+			DownloadInfo: &applicationcharm.DownloadInfo{
+				Provenance:         applicationcharm.ProvenanceDownload,
+				CharmhubIdentifier: dt.downloadInfo.CharmhubIdentifier,
+				DownloadURL:        dt.downloadInfo.DownloadURL,
+				DownloadSize:       dt.downloadInfo.DownloadSize,
+			},
 		}, unitArgs...)
 	}
 
@@ -279,6 +286,7 @@ type deployTemplate struct {
 	storage                map[string]storage.Directive
 	pendingResourceUploads []*params.PendingResourceUpload
 	resolvedResources      []resource.Resource
+	downloadInfo           corecharm.DownloadInfo
 }
 
 type validatorConfig struct {
@@ -397,24 +405,23 @@ func (v *deployFromRepositoryValidator) validate(ctx context.Context, arg params
 
 	// get the charm data to validate against, either a previously deployed
 	// charm or the essential metadata from a charm to be async downloaded.
-	charmURL, resolvedOrigin, resolvedCharm, getCharmErr := v.getCharm(ctx, arg)
-	if getCharmErr != nil {
-		errs = append(errs, getCharmErr)
+	charmResult, err := v.getCharm(ctx, arg)
+	if err != nil {
 		// return any errors here, there is no need to continue with
 		// validation if we cannot find the charm.
-		return deployTemplate{}, errs
+		return deployTemplate{}, append(errs, err)
 	}
 
 	// Various checks of the resolved charm against the arg provided.
-	dt, rcErrs := v.resolvedCharmValidation(ctx, resolvedCharm, arg)
+	dt, rcErrs := v.resolvedCharmValidation(ctx, charmResult.Charm, arg)
 	if len(rcErrs) > 0 {
 		errs = append(errs, rcErrs...)
 	}
 
-	dt.charmURL = charmURL
+	dt.charmURL = charmResult.CharmURL
 	dt.dryRun = arg.DryRun
 	dt.force = arg.Force
-	dt.origin = resolvedOrigin
+	dt.origin = charmResult.Origin
 	dt.placement = arg.Placement
 	dt.storage = arg.Storage
 	if len(arg.EndpointBindings) > 0 {
@@ -425,14 +432,16 @@ func (v *deployFromRepositoryValidator) validate(ctx context.Context, arg params
 			dt.endpoints = bindings.Map()
 		}
 	}
-	// resolve and validate resources
-	resources, pendingResourceUploads, resolveResErr := v.resolveResources(ctx, dt.charmURL, dt.origin, dt.resources, resolvedCharm.Meta().Resources)
+
+	// Resolve resources and validate against the charm metadata.
+	resources, pendingResourceUploads, resolveResErr := v.resolveResources(ctx, dt.charmURL, dt.origin, dt.resources, charmResult.Charm.Meta().Resources)
 	if resolveResErr != nil {
 		errs = append(errs, resolveResErr)
 	}
 
 	dt.pendingResourceUploads = pendingResourceUploads
 	dt.resolvedResources = resources
+	dt.downloadInfo = charmResult.DownloadInfo
 
 	if v.logger.IsLevelEnabled(corelogger.TRACE) {
 		v.logger.Tracef("validateDeployFromRepositoryArgs returning: %s", pretty.Sprint(dt))
@@ -890,12 +899,19 @@ func (v *deployFromRepositoryValidator) resolveCharm(ctx context.Context, curl *
 	return resolvedData, nil
 }
 
+type charmResult struct {
+	CharmURL     *charm.URL
+	Origin       corecharm.Origin
+	Charm        charm.Charm
+	DownloadInfo corecharm.DownloadInfo
+}
+
 // getCharm returns the charm being deployed. Either it already has been
 // used once, and we get the data from state. Or we get the essential metadata.
-func (v *deployFromRepositoryValidator) getCharm(ctx context.Context, arg params.DeployFromRepositoryArg) (*charm.URL, corecharm.Origin, charm.Charm, error) {
+func (v *deployFromRepositoryValidator) getCharm(ctx context.Context, arg params.DeployFromRepositoryArg) (charmResult, error) {
 	initialCurl, requestedOrigin, usedModelDefaultBase, err := v.createOrigin(ctx, arg)
 	if err != nil {
-		return nil, corecharm.Origin{}, nil, errors.Trace(err)
+		return charmResult{}, errors.Trace(err)
 	}
 	v.logger.Tracef("from createOrigin: %s, %s", initialCurl, pretty.Sprint(requestedOrigin))
 
@@ -904,17 +920,18 @@ func (v *deployFromRepositoryValidator) getCharm(ctx context.Context, arg params
 	// be populated once the charm gets downloaded.
 	resolvedData, err := v.resolveCharm(ctx, initialCurl, requestedOrigin, arg.Force, usedModelDefaultBase, arg.Cons)
 	if err != nil {
-		return nil, corecharm.Origin{}, nil, err
+		return charmResult{}, errors.Trace(err)
 	}
-	resolvedOrigin := resolvedData.EssentialMetadata.ResolvedOrigin
+	essentialMetadata := resolvedData.EssentialMetadata
+	resolvedOrigin := essentialMetadata.ResolvedOrigin
 	v.logger.Tracef("from resolveCharm: %s, %s", resolvedData.URL, pretty.Sprint(resolvedOrigin))
 	if resolvedOrigin.Type != "charm" {
-		return nil, corecharm.Origin{}, nil, errors.BadRequestf("%q is not a charm", arg.CharmName)
+		return charmResult{}, errors.BadRequestf("%q is not a charm", arg.CharmName)
 	}
 
-	resolvedCharm := corecharm.NewCharmInfoAdaptor(resolvedData.EssentialMetadata)
+	resolvedCharm := corecharm.NewCharmInfoAdaptor(essentialMetadata)
 	if resolvedCharm.Meta().Name == bootstrap.ControllerCharmName {
-		return nil, corecharm.Origin{}, nil, errors.NotSupportedf("manual deploy of the controller charm")
+		return charmResult{}, errors.NotSupportedf("manual deploy of the controller charm")
 	}
 
 	// Check if a charm already exists for this charm URL. If so, the
@@ -924,7 +941,7 @@ func (v *deployFromRepositoryValidator) getCharm(ctx context.Context, arg params
 	// channels.
 	charmSource, err := applicationcharm.ParseCharmSchema(charm.Schema(resolvedData.URL.Schema))
 	if err != nil {
-		return nil, corecharm.Origin{}, nil, errors.Trace(err)
+		return charmResult{}, errors.Trace(err)
 	}
 	deployedCharmId, err := v.applicationService.GetCharmID(ctx, applicationcharm.GetCharmArgs{
 		Name:     resolvedData.URL.Name,
@@ -934,19 +951,30 @@ func (v *deployFromRepositoryValidator) getCharm(ctx context.Context, arg params
 	if err == nil {
 		deployedCharm, _, err := v.applicationService.GetCharm(ctx, deployedCharmId)
 		if err != nil {
-			return nil, corecharm.Origin{}, nil, errors.Trace(err)
+			return charmResult{}, errors.Trace(err)
 		}
-		return resolvedData.URL, resolvedOrigin, deployedCharm, nil
+		return charmResult{
+			CharmURL:     resolvedData.URL,
+			Origin:       resolvedOrigin,
+			Charm:        deployedCharm,
+			DownloadInfo: essentialMetadata.DownloadInfo,
+		}, nil
 	}
 	if !errors.Is(err, applicationerrors.CharmNotFound) {
-		return nil, corecharm.Origin{}, nil, errors.Trace(err)
+		return charmResult{}, errors.Trace(err)
 	}
 
 	// This charm needs to be downloaded, remove the ID and Hash to
 	// allow it to happen.
 	resolvedOrigin.ID = ""
 	resolvedOrigin.Hash = ""
-	return resolvedData.URL, resolvedOrigin, resolvedCharm, nil
+
+	return charmResult{
+		CharmURL:     resolvedData.URL,
+		Origin:       resolvedOrigin,
+		Charm:        resolvedCharm,
+		DownloadInfo: essentialMetadata.DownloadInfo,
+	}, nil
 }
 
 func (v *deployFromRepositoryValidator) appCharmSettings(appName string, trust bool, chCfg *charm.Config, configYAML string) (*config.Config, charm.Settings, error) {

apiserver/facades/client/application/service.go

@@ -179,6 +179,10 @@ type ApplicationService interface {
 	// GetCharmMetadataName returns the name for the charm using the
 	// charm ID.
 	GetCharmMetadataName(ctx context.Context, id corecharm.ID) (string, error)
+
+	// GetCharmDownloadInfo returns the download info for the charm using the
+	// charm ID.
+	GetCharmDownloadInfo(ctx context.Context, id corecharm.ID) (*applicationcharm.DownloadInfo, error)
 }
 
 // ModelConfigService provides access to the model configuration.

apiserver/facades/client/charms/client.go

@@ -290,32 +290,38 @@ func (a *API) queueAsyncCharmDownload(ctx context.Context, args params.AddCharmW
 	if len(essentialMeta) != 1 {
 		return corecharm.Origin{}, errors.Errorf("expected 1 metadata result, got %d", len(essentialMeta))
 	}
-	metaRes := essentialMeta[0]
+	essentialMetadata := essentialMeta[0]
 
 	_, err = a.backendState.AddCharmMetadata(state.CharmInfo{
-		Charm: corecharm.NewCharmInfoAdaptor(metaRes),
+		Charm: corecharm.NewCharmInfoAdaptor(essentialMetadata),
 		ID:    args.URL,
 	})
 	if err != nil {
 		return corecharm.Origin{}, errors.Trace(err)
 	}
 
-	revision, err := makeCharmRevision(metaRes.ResolvedOrigin, args.URL)
+	revision, err := makeCharmRevision(essentialMetadata.ResolvedOrigin, args.URL)
 	if err != nil {
 		return corecharm.Origin{}, errors.Annotatef(err, "making revision for charm %q", args.URL)
 	}
 
 	if _, _, err := a.applicationService.SetCharm(ctx, applicationcharm.SetCharmArgs{
-		Charm:         corecharm.NewCharmInfoAdaptor(metaRes),
+		Charm:         corecharm.NewCharmInfoAdaptor(essentialMetadata),
 		Source:        requestedOrigin.Source,
 		ReferenceName: charmURL.Name,
 		Revision:      revision,
-		Hash:          metaRes.ResolvedOrigin.Hash,
+		Hash:          essentialMetadata.ResolvedOrigin.Hash,
+		DownloadInfo: &applicationcharm.DownloadInfo{
+			Provenance:         applicationcharm.ProvenanceDownload,
+			CharmhubIdentifier: essentialMetadata.DownloadInfo.CharmhubIdentifier,
+			DownloadURL:        essentialMetadata.DownloadInfo.DownloadURL,
+			DownloadSize:       essentialMetadata.DownloadInfo.DownloadSize,
+		},
 	}); err != nil && !errors.Is(err, applicationerrors.CharmAlreadyExists) {
 		return corecharm.Origin{}, errors.Annotatef(err, "setting charm %q", args.URL)
 	}
 
-	return metaRes.ResolvedOrigin, nil
+	return essentialMetadata.ResolvedOrigin, nil
 }
 
 func makeCharmRevision(origin corecharm.Origin, url string) (int, error) {

apiserver/objects.go

@@ -232,8 +232,10 @@ func (h *objectsCharmHTTPHandler) processPut(r *http.Request, st *state.State, c
 	// This can be done, once all the charm service methods are being used,
 	// instead of the state methods.
 	csSource := corecharm.CharmHub
+	provenance := applicationcharm.ProvenanceMigration
 	if source == charm.Local {
 		csSource = corecharm.Local
+		provenance = applicationcharm.ProvenanceUpload
 	}
 
 	if _, _, err := charmService.SetCharm(r.Context(), applicationcharm.SetCharmArgs{
@@ -245,6 +247,12 @@ func (h *objectsCharmHTTPHandler) processPut(r *http.Request, st *state.State, c
 		ArchivePath:   storagePath,
 		Version:       version,
 		Architecture:  curl.Architecture,
+		// If this is a charmhub charm, this will be coming from a migration.
+		// We can not re-download this charm from the charm store again, without
+		// another call directly to the charm store.
+		DownloadInfo: &applicationcharm.DownloadInfo{
+			Provenance: provenance,
+		},
 	}); err != nil {
 		return nil, errors.Trace(err)
 	}

core/charm/repository.go

@@ -83,6 +83,12 @@ type EssentialMetadata struct {
 	Meta     *charm.Meta
 	Manifest *charm.Manifest
 	Config   *charm.Config
+
+	// DownloadInfo is the information needed to download the charm
+	// directly from the charm store.
+	// This should always be present if the charm is being downloaded from
+	// charmhub.
+	DownloadInfo DownloadInfo
 }
 
 // CharmID encapsulates data for identifying a unique charm in a charm repository.
@@ -108,3 +114,17 @@ type ResolvedDataForDeploy struct {
 	// based on the supplied origin
 	Resources map[string]charmresource.Resource
 }
+
+// DownloadInfo contains the information needed to download a charm from the
+// charm store.
+type DownloadInfo struct {
+	// CharmHubIdentifier is the identifier used to download the charm from
+	// the charm store without referring to the name of the charm.
+	CharmhubIdentifier string
+
+	// DownloadURL is the URL to download the charm from the charm store.
+	DownloadURL string
+
+	// DownloadSize is the size of the charm to be downloaded.
+	DownloadSize int64
+}