From 3a6e5cbe8d8538d779e0c27ab902335d37f5d7f4 Mon Sep 17 00:00:00 2001 From: Simon Richardson Date: Fri, 29 Nov 2024 13:18:09 +0000 Subject: [PATCH 1/7] feat: add download info to charm Adding download info to charm allows us to ensure that we correctly store the charm information once and only once. --- .../facades/client/application/deploy.go | 4 + .../client/application/deployrepository.go | 3 + apiserver/facades/client/charms/client.go | 3 + apiserver/objects.go | 4 + domain/application/charm/types.go | 16 ++ domain/application/errors/errors.go | 69 +++-- domain/application/modelmigration/import.go | 7 + domain/application/service/application.go | 34 ++- .../application/service/application_test.go | 206 +++++++++++--- domain/application/service/charm.go | 31 +- domain/application/service/charm_test.go | 168 ++++++++++- .../application/service/package_mock_test.go | 27 +- domain/application/service/package_test.go | 15 + domain/application/service/params.go | 6 + domain/application/service/service.go | 4 +- domain/application/service/service_test.go | 28 +- domain/application/state/application.go | 4 +- domain/application/state/application_test.go | 38 +-- domain/application/state/charm.go | 20 +- domain/application/state/charm_test.go | 266 +++++++++++++++--- domain/application/state/package_test.go | 27 ++ domain/application/state/state.go | 266 +++++++++++------- domain/application/state/types.go | 15 + domain/application/types.go | 9 +- domain/schema/model/sql/0015-charm.sql | 19 +- .../triggers/application-triggers.gen.go | 1 - internal/bootstrap/deployer.go | 2 + internal/testing/factory/factory.go | 2 + 28 files changed, 1010 insertions(+), 284 deletions(-) diff --git a/apiserver/facades/client/application/deploy.go b/apiserver/facades/client/application/deploy.go index 882150fac51..b12c51c28f9 100644 --- a/apiserver/facades/client/application/deploy.go +++ b/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" @@ -169,6 +170,9 @@ func DeployApplication( _, err = applicationService.CreateApplication(ctx, args.ApplicationName, args.Charm, args.CharmOrigin, applicationservice.AddApplicationArgs{ ReferenceName: chURL.Name, Storage: args.Storage, + // TODO (stickupkid): Fill this in correctly when we have the + // charmhub information. + DownloadInfo: &applicationcharm.DownloadInfo{}, }, unitArgs...) if err != nil { return nil, errors.Trace(err) diff --git a/apiserver/facades/client/application/deployrepository.go b/apiserver/facades/client/application/deployrepository.go index 79825906e13..1d0416407c3 100644 --- a/apiserver/facades/client/application/deployrepository.go +++ b/apiserver/facades/client/application/deployrepository.go @@ -171,6 +171,9 @@ 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, + // TODO (stickupkid): Fill this in correctly when we have the + // charmhub information. + DownloadInfo: &applicationcharm.DownloadInfo{}, }, unitArgs...) } diff --git a/apiserver/facades/client/charms/client.go b/apiserver/facades/client/charms/client.go index 09e8bb54147..558f4a69362 100644 --- a/apiserver/facades/client/charms/client.go +++ b/apiserver/facades/client/charms/client.go @@ -311,6 +311,9 @@ func (a *API) queueAsyncCharmDownload(ctx context.Context, args params.AddCharmW ReferenceName: charmURL.Name, Revision: revision, Hash: metaRes.ResolvedOrigin.Hash, + // TODO (stickupkid): Fill this information in from the essential + // metadata. + DownloadInfo: &applicationcharm.DownloadInfo{}, }); err != nil && !errors.Is(err, applicationerrors.CharmAlreadyExists) { return corecharm.Origin{}, errors.Annotatef(err, "setting charm %q", args.URL) } diff --git a/apiserver/objects.go b/apiserver/objects.go index 15b48dce8ba..941093a232d 100644 --- a/apiserver/objects.go +++ b/apiserver/objects.go @@ -245,6 +245,10 @@ 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{}, }); err != nil { return nil, errors.Trace(err) } diff --git a/domain/application/charm/types.go b/domain/application/charm/types.go index 14b752bd5d4..15a4bfe67d4 100644 --- a/domain/application/charm/types.go +++ b/domain/application/charm/types.go @@ -82,6 +82,8 @@ type SetCharmArgs struct { Version string // Architecture is the architecture of the charm. Architecture arch.Arch + // DownloadInfo holds the information needed to download a charmhub charm. + DownloadInfo *DownloadInfo } // Revision is the charm revision. @@ -151,6 +153,20 @@ type CharmLocator struct { Architecture architecture.Architecture } +// DownloadInfo holds the information needed to download a charmhub charm. +type DownloadInfo struct { + // CharmhubIdentifier is the instance ID of the charm in relation to + // charmhub. + CharmhubIdentifier string + + // DownloadURL is the URL to download the charm from. + DownloadURL string + + // DownloadSize is the size of the charm in bytes that the download URL + // points to. + DownloadSize int64 +} + // Metadata represents the metadata of a charm from the perspective of the // service. This is the golden source of charm metadata. If the charm changes // at the wire format level, we should be able to map it to this struct. diff --git a/domain/application/errors/errors.go b/domain/application/errors/errors.go index 5035a0902f3..77d4c5f12ee 100644 --- a/domain/application/errors/errors.go +++ b/domain/application/errors/errors.go @@ -16,18 +16,20 @@ const ( // application being created already exists. ApplicationAlreadyExists = errors.ConstError("application already exists") - // ApplicationNotAlive describes an error that occurs when trying to update an application that is not alive. + // ApplicationNotAlive describes an error that occurs when trying to update + // an application that is not alive. ApplicationNotAlive = errors.ConstError("application is not alive") // ApplicationHasUnits describes an error that occurs when the application // being deleted still has associated units. ApplicationHasUnits = errors.ConstError("application has units") - // ScalingStateInconsistent is returned by SetScalingState when the scaling state - // is inconsistent with the application scale. + // ScalingStateInconsistent is returned by SetScalingState when the scaling + // state is inconsistent with the application scale. ScalingStateInconsistent = errors.ConstError("scaling state is inconsistent") - // ScaleChangeInvalid is returned when an attempt is made to set an invalid application scale value. + // ScaleChangeInvalid is returned when an attempt is made to set an invalid + // application scale value. ScaleChangeInvalid = errors.ConstError("scale change invalid") // MissingStorageDirective describes an error that occurs when expected @@ -42,37 +44,39 @@ const ( // not valid. ApplicationIDNotValid = errors.ConstError("application ID not valid") - // UnitNotFound describes an error that occurs when the unit being operated on - // does not exist. + // UnitNotFound describes an error that occurs when the unit being operated + // on does not exist. UnitNotFound = errors.ConstError("unit not found") // UnitAlreadyExists describes an error that occurs when the // unit being created already exists. UnitAlreadyExists = errors.ConstError("unit already exists") - // UnitNotAssigned describes an error that occurs when the unit being operated on - // is not assigned. + // UnitNotAssigned describes an error that occurs when the unit being + // operated on is not assigned. UnitNotAssigned = errors.ConstError("unit not assigned") - // UnitHasSubordinates describes an error that occurs when trying to set a unit's life - // to Dead but it still has subordinates. + // UnitHasSubordinates describes an error that occurs when trying to set a + // unit's life to Dead but it still has subordinates. UnitHasSubordinates = errors.ConstError("unit has subordinates") - // UnitHasStorageAttachments describes an error that occurs when trying to set a unit's life - // to Dead but it still has storage attachments. + // UnitHasStorageAttachments describes an error that occurs when trying to + // set a unit's life to Dead but it still has storage attachments. UnitHasStorageAttachments = errors.ConstError("unit has storage attachments") - // UnitIsAlive describes an error that occurs when trying to remove a unit that is still alive. + // UnitIsAlive describes an error that occurs when trying to remove a unit + // that is still alive. UnitIsAlive = errors.ConstError("unit is alive") - // InvalidApplicationState describes an error where the application state is invalid. - // There are missing required fields. + // InvalidApplicationState describes an error where the application state is + // invalid. There are missing required fields. InvalidApplicationState = errors.ConstError("invalid application state") // CharmNotValid describes an error that occurs when the charm is not valid. CharmNotValid = errors.ConstError("charm not valid") - // CharmOriginNotValid describes an error that occurs when the charm origin is not valid. + // CharmOriginNotValid describes an error that occurs when the charm origin + // is not valid. CharmOriginNotValid = errors.ConstError("charm origin not valid") // CharmNameNotValid describes an error that occurs when attempting to get @@ -83,7 +87,8 @@ const ( // a charm using an invalid charm source. CharmSourceNotValid = errors.ConstError("charm source not valid") - // CharmNotFound describes an error that occurs when a charm cannot be found. + // CharmNotFound describes an error that occurs when a charm cannot be + // found. CharmNotFound = errors.ConstError("charm not found") // LXDProfileNotFound describes an error that occurs when an LXD profile @@ -94,16 +99,20 @@ const ( // exists for the given natural key. CharmAlreadyExists = errors.ConstError("charm already exists") - // CharmRevisionNotValid describes an error that occurs when attempting to get - // a charm using an invalid revision. + // CharmRevisionNotValid describes an error that occurs when attempting to + // get a charm using an invalid revision. CharmRevisionNotValid = errors.ConstError("charm revision not valid") - // CharmMetadataNotValid describes an error that occurs when the charm metadata - // is not valid. + // CharmMetadataNotValid describes an error that occurs when the charm + // metadata is not valid. CharmMetadataNotValid = errors.ConstError("charm metadata not valid") - // CharmManifestNotValid describes an error that occurs when the charm manifest - // is not valid. + // CharmManifestNotFound describes an error that occurs when the charm + // manifest is not found. + CharmManifestNotFound = errors.ConstError("charm manifest not found") + + // CharmManifestNotValid describes an error that occurs when the charm + // manifest is not valid. CharmManifestNotValid = errors.ConstError("charm manifest not valid") // CharmBaseNameNotValid describes an error that occurs when the charm base @@ -118,8 +127,8 @@ const ( // has multiple relations with the same name CharmRelationNameConflict = errors.ConstError("charm relation name conflict") - // CharmRelationReservedNameMisuse describes an error that occurs when the charm - // relation name is a reserved name which it is not allowed to use. + // CharmRelationReservedNameMisuse describes an error that occurs when the + // charm relation name is a reserved name which it is not allowed to use. CharmRelationReservedNameMisuse = errors.ConstError("charm relation reserved name misuse") // CharmRelationRoleNotValid describes an error that occurs when the charm @@ -127,9 +136,9 @@ const ( // wrong value. CharmRelationRoleNotValid = errors.ConstError("charm relation role not valid") - // MultipleCharmHashes describes and error that occurs when a charm has multiple - // hash values. At the moment, we only support sha256 hash format, so if another - // is found, an error is returned. + // MultipleCharmHashes describes and error that occurs when a charm has + // multiple hash values. At the moment, we only support sha256 hash format, + // so if another is found, an error is returned. MultipleCharmHashes = errors.ConstError("multiple charm hashes found") // ResourceNotFound describes an error that occurs when a resource is @@ -169,4 +178,8 @@ const ( // resolved. This means the charm for the hash does not exist and needs to // be downloaded. CharmNotResolved = errors.ConstError("charm not resolved") + + // CharmDownloadInfoNotFound describes an error that occurs when the charm + // download info is not found. + CharmDownloadInfoNotFound = errors.ConstError("charm download info not found") ) diff --git a/domain/application/modelmigration/import.go b/domain/application/modelmigration/import.go index 7112199a485..0ff2060fe41 100644 --- a/domain/application/modelmigration/import.go +++ b/domain/application/modelmigration/import.go @@ -21,6 +21,7 @@ import ( corestatus "github.com/juju/juju/core/status" corestorage "github.com/juju/juju/core/storage" coreunit "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/application/state" internalcharm "github.com/juju/juju/internal/charm" @@ -161,6 +162,12 @@ func (i *importOperation) Execute(ctx context.Context, model description.Model) err = i.service.ImportApplication( ctx, app.Name(), charm, *origin, service.AddApplicationArgs{ ReferenceName: chURL.Name, + // TODO (stickupkid): When we're importing a charm during a + // migration, we should fill this in with the correct value. + // If not, we should indicate that the charm can not be + // downloaded without a new request to the charm store to + // fetch the charm. + DownloadInfo: &applicationcharm.DownloadInfo{}, }, unitArgs..., ) if err != nil { diff --git a/domain/application/service/application.go b/domain/application/service/application.go index 12796e13b4c..b64b59d1605 100644 --- a/domain/application/service/application.go +++ b/domain/application/service/application.go @@ -237,7 +237,7 @@ func (s *Service) CreateApplication( args AddApplicationArgs, units ...AddUnitArg, ) (coreapplication.ID, error) { - if err := validateCreateApplicationParams(name, args.ReferenceName, charm, origin); err != nil { + if err := validateCreateApplicationParams(name, args.ReferenceName, charm, origin, args.DownloadInfo); err != nil { return "", errors.Annotatef(err, "invalid application args") } @@ -284,23 +284,36 @@ func validateCreateApplicationParams( name, referenceName string, charm internalcharm.Charm, origin corecharm.Origin, + downloadInfo *domaincharm.DownloadInfo, ) error { if !isValidApplicationName(name) { return applicationerrors.ApplicationNameNotValid } - // Validate that we have a valid charm and name. - meta := charm.Meta() - if meta == nil { + // We require a valid charm metadata. + if meta := charm.Meta(); meta == nil { return applicationerrors.CharmMetadataNotValid + } else if !isValidCharmName(meta.Name) { + return applicationerrors.CharmNameNotValid } - if !isValidCharmName(meta.Name) { - return applicationerrors.CharmNameNotValid - } else if !isValidReferenceName(referenceName) { + // We require a valid charm manifest. + if manifest := charm.Manifest(); manifest == nil { + return applicationerrors.CharmManifestNotFound + } else if len(manifest.Bases) == 0 { + return applicationerrors.CharmManifestNotValid + } + + // If the reference name is provided, it must be valid. + if !isValidReferenceName(referenceName) { return fmt.Errorf("reference name: %w", applicationerrors.CharmNameNotValid) } + // If the origin is from charmhub, then we require the download info. + if origin.Source == corecharm.CharmHub && downloadInfo == nil { + return applicationerrors.CharmDownloadInfoNotFound + } + // Validate the origin of the charm. if err := origin.Validate(); err != nil { return fmt.Errorf("%w: %v", applicationerrors.CharmOriginNotValid, err) @@ -372,9 +385,10 @@ func makeCreateApplicationArgs( } return application.AddApplicationArg{ - Charm: ch, - Platform: platformArg, - Channel: channelArg, + Charm: ch, + CharmDownloadInfo: args.DownloadInfo, + Platform: platformArg, + Channel: channelArg, }, nil } diff --git a/domain/application/service/application_test.go b/domain/application/service/application_test.go index 79038c5a34b..6579f923f43 100644 --- a/domain/application/service/application_test.go +++ b/domain/application/service/application_test.go @@ -78,6 +78,7 @@ func (s *applicationServiceSuite) TestCreateApplication(c *gc.C) { Name: "ubuntu", RunAs: "default", }, + Manifest: s.minimalManifest(c), ReferenceName: "ubuntu", Source: domaincharm.CharmHubSource, Revision: 42, @@ -89,7 +90,12 @@ func (s *applicationServiceSuite) TestCreateApplication(c *gc.C) { Architecture: architecture.ARM64, } app := application.AddApplicationArg{ - Charm: ch, + Charm: ch, + CharmDownloadInfo: &domaincharm.DownloadInfo{ + CharmhubIdentifier: "foo", + DownloadURL: "https://example.com/foo", + DownloadSize: 42, + }, Platform: platform, Scale: 1, } @@ -97,12 +103,23 @@ func (s *applicationServiceSuite) TestCreateApplication(c *gc.C) { s.state.EXPECT().StorageDefaults(gomock.Any()).Return(domainstorage.StorageDefaults{}, nil) s.state.EXPECT().CreateApplication(domaintesting.IsAtomicContextChecker, "ubuntu", app).Return(id, nil) s.state.EXPECT().AddUnits(domaintesting.IsAtomicContextChecker, id, u) - s.charm.EXPECT().Manifest().Return(&charm.Manifest{}) + s.charm.EXPECT().Actions().Return(&charm.Actions{}) s.charm.EXPECT().Config().Return(&charm.Config{}) + s.charm.EXPECT().Manifest().Return(&charm.Manifest{ + Bases: []charm.Base{ + { + Name: "ubuntu", + Channel: charm.Channel{ + Risk: charm.Stable, + }, + Architectures: []string{"amd64"}, + }, + }, + }).MinTimes(1) s.charm.EXPECT().Meta().Return(&charm.Meta{ Name: "ubuntu", - }).AnyTimes() + }).MinTimes(1) a := AddUnitArg{ UnitName: "ubuntu/666", @@ -113,6 +130,11 @@ func (s *applicationServiceSuite) TestCreateApplication(c *gc.C) { Revision: ptr(42), }, AddApplicationArgs{ ReferenceName: "ubuntu", + DownloadInfo: &domaincharm.DownloadInfo{ + CharmhubIdentifier: "foo", + DownloadURL: "https://example.com/foo", + DownloadSize: 42, + }, }, a) c.Assert(err, jc.ErrorIsNil) } @@ -153,6 +175,9 @@ func (s *applicationServiceSuite) TestCreateApplicationWithInvalidReferenceName( s.charm.EXPECT().Meta().Return(&charm.Meta{ Name: "ubuntu", }).AnyTimes() + s.charm.EXPECT().Manifest().Return(&charm.Manifest{ + Bases: []charm.Base{{}}, + }).AnyTimes() _, err := s.service.CreateApplication(context.Background(), "ubuntu", s.charm, corecharm.Origin{ Source: corecharm.CharmHub, @@ -160,6 +185,11 @@ func (s *applicationServiceSuite) TestCreateApplicationWithInvalidReferenceName( Revision: ptr(42), }, AddApplicationArgs{ ReferenceName: "666", + DownloadInfo: &domaincharm.DownloadInfo{ + CharmhubIdentifier: "foo", + DownloadURL: "https://example.com/foo", + DownloadSize: 42, + }, }) c.Assert(err, jc.ErrorIs, applicationerrors.CharmNameNotValid) } @@ -189,7 +219,7 @@ func (s *applicationServiceSuite) TestCreateApplicationWithNoApplicationOrCharmN func (s *applicationServiceSuite) TestCreateApplicationWithNoMeta(c *gc.C) { defer s.setupMocks(c).Finish() - s.charm.EXPECT().Meta().Return(nil).AnyTimes() + s.charm.EXPECT().Meta().Return(nil).MinTimes(1) _, err := s.service.CreateApplication(context.Background(), "foo", s.charm, corecharm.Origin{ Platform: corecharm.MustParsePlatform("arm64/ubuntu/24.04"), @@ -200,13 +230,21 @@ func (s *applicationServiceSuite) TestCreateApplicationWithNoMeta(c *gc.C) { func (s *applicationServiceSuite) TestCreateApplicationWithNoArchitecture(c *gc.C) { defer s.setupMocks(c).Finish() - s.charm.EXPECT().Meta().Return(&charm.Meta{Name: "foo"}).AnyTimes() + s.charm.EXPECT().Meta().Return(&charm.Meta{Name: "foo"}).MinTimes(1) + s.charm.EXPECT().Manifest().Return(&charm.Manifest{ + Bases: []charm.Base{{}}, + }).MinTimes(1) _, err := s.service.CreateApplication(context.Background(), "foo", s.charm, corecharm.Origin{ Source: corecharm.CharmHub, Platform: corecharm.Platform{Channel: "24.04", OS: "ubuntu"}, }, AddApplicationArgs{ ReferenceName: "foo", + DownloadInfo: &domaincharm.DownloadInfo{ + CharmhubIdentifier: "foo", + DownloadURL: "https://example.com/foo", + DownloadSize: 42, + }, }) c.Assert(err, jc.ErrorIs, applicationerrors.CharmOriginNotValid) } @@ -220,18 +258,28 @@ func (s *applicationServiceSuite) TestCreateApplicationError(c *gc.C) { s.state.EXPECT().GetModelType(gomock.Any()).Return("caas", nil) s.state.EXPECT().StorageDefaults(gomock.Any()).Return(domainstorage.StorageDefaults{}, nil) s.state.EXPECT().CreateApplication(domaintesting.IsAtomicContextChecker, "foo", gomock.Any()).Return(id, rErr) - s.charm.EXPECT().Manifest().Return(&charm.Manifest{}) - s.charm.EXPECT().Actions().Return(&charm.Actions{}) - s.charm.EXPECT().Config().Return(&charm.Config{}) + s.charm.EXPECT().Meta().Return(&charm.Meta{ Name: "foo", - }).AnyTimes() + }).MinTimes(1) + s.charm.EXPECT().Manifest().Return(&charm.Manifest{Bases: []charm.Base{{ + Name: "ubuntu", + Channel: charm.Channel{Risk: charm.Beta}, + Architectures: []string{"arm64"}, + }}}).MinTimes(1) + s.charm.EXPECT().Actions().Return(&charm.Actions{}) + s.charm.EXPECT().Config().Return(&charm.Config{}) _, err := s.service.CreateApplication(context.Background(), "foo", s.charm, corecharm.Origin{ Source: corecharm.CharmHub, Platform: corecharm.MustParsePlatform("arm64/ubuntu/24.04"), }, AddApplicationArgs{ ReferenceName: "foo", + DownloadInfo: &domaincharm.DownloadInfo{ + CharmhubIdentifier: "foo", + DownloadURL: "https://example.com/foo", + DownloadSize: 42, + }, }) c.Check(err, jc.ErrorIs, rErr) c.Assert(err, gc.ErrorMatches, `creating application "foo": boom`) @@ -275,18 +323,24 @@ func (s *applicationServiceSuite) TestCreateWithStorageBlock(c *gc.C) { }, }, }, + Manifest: s.minimalManifest(c), ReferenceName: "foo", Source: domaincharm.LocalSource, Revision: 42, - Architecture: architecture.ARM64, + Architecture: architecture.AMD64, } platform := application.Platform{ Channel: "24.04", OSType: application.Ubuntu, - Architecture: architecture.ARM64, + Architecture: architecture.AMD64, } app := application.AddApplicationArg{ - Charm: ch, + Charm: ch, + CharmDownloadInfo: &domaincharm.DownloadInfo{ + CharmhubIdentifier: "foo", + DownloadURL: "https://example.com/foo", + DownloadSize: 42, + }, Platform: platform, Scale: 1, } @@ -294,7 +348,7 @@ func (s *applicationServiceSuite) TestCreateWithStorageBlock(c *gc.C) { s.state.EXPECT().StorageDefaults(gomock.Any()).Return(domainstorage.StorageDefaults{}, nil) s.state.EXPECT().CreateApplication(domaintesting.IsAtomicContextChecker, "foo", app).Return(id, nil) s.state.EXPECT().AddUnits(domaintesting.IsAtomicContextChecker, id, u) - s.charm.EXPECT().Manifest().Return(&charm.Manifest{}) + s.charm.EXPECT().Actions().Return(&charm.Actions{}) s.charm.EXPECT().Config().Return(&charm.Config{}) s.charm.EXPECT().Meta().Return(&charm.Meta{ @@ -309,7 +363,13 @@ func (s *applicationServiceSuite) TestCreateWithStorageBlock(c *gc.C) { MinimumSize: 10, }, }, - }).AnyTimes() + }).MinTimes(1) + s.charm.EXPECT().Manifest().Return(&charm.Manifest{Bases: []charm.Base{{ + Name: "ubuntu", + Channel: charm.Channel{Risk: charm.Stable}, + Architectures: []string{"amd64"}, + }}}).MinTimes(1) + pool := domainstorage.StoragePoolDetails{Name: "loop", Provider: "loop"} s.state.EXPECT().GetStoragePoolByName(gomock.Any(), "loop").Return(pool, nil) @@ -318,10 +378,15 @@ func (s *applicationServiceSuite) TestCreateWithStorageBlock(c *gc.C) { } _, err := s.service.CreateApplication(context.Background(), "foo", s.charm, corecharm.Origin{ Source: corecharm.Local, - Platform: corecharm.MustParsePlatform("arm64/ubuntu/24.04"), + Platform: corecharm.MustParsePlatform("amd64/ubuntu/24.04"), Revision: ptr(42), }, AddApplicationArgs{ ReferenceName: "foo", + DownloadInfo: &domaincharm.DownloadInfo{ + CharmhubIdentifier: "foo", + DownloadURL: "https://example.com/foo", + DownloadSize: 42, + }, }, a) c.Assert(err, jc.ErrorIsNil) } @@ -364,18 +429,24 @@ func (s *applicationServiceSuite) TestCreateWithStorageBlockDefaultSource(c *gc. }, }, }, + Manifest: s.minimalManifest(c), ReferenceName: "foo", Source: domaincharm.CharmHubSource, Revision: 42, - Architecture: architecture.ARM64, + Architecture: architecture.AMD64, } platform := application.Platform{ Channel: "24.04", OSType: application.Ubuntu, - Architecture: architecture.ARM64, + Architecture: architecture.AMD64, } app := application.AddApplicationArg{ - Charm: ch, + Charm: ch, + CharmDownloadInfo: &domaincharm.DownloadInfo{ + CharmhubIdentifier: "foo", + DownloadURL: "https://example.com/foo", + DownloadSize: 42, + }, Platform: platform, Scale: 1, } @@ -383,7 +454,7 @@ func (s *applicationServiceSuite) TestCreateWithStorageBlockDefaultSource(c *gc. s.state.EXPECT().StorageDefaults(gomock.Any()).Return(domainstorage.StorageDefaults{DefaultBlockSource: ptr("fast")}, nil) s.state.EXPECT().CreateApplication(domaintesting.IsAtomicContextChecker, "foo", app).Return(id, nil) s.state.EXPECT().AddUnits(domaintesting.IsAtomicContextChecker, id, u) - s.charm.EXPECT().Manifest().Return(&charm.Manifest{}) + s.charm.EXPECT().Actions().Return(&charm.Actions{}) s.charm.EXPECT().Config().Return(&charm.Config{}) s.charm.EXPECT().Meta().Return(&charm.Meta{ @@ -398,7 +469,13 @@ func (s *applicationServiceSuite) TestCreateWithStorageBlockDefaultSource(c *gc. MinimumSize: 10, }, }, - }).AnyTimes() + }).MinTimes(1) + s.charm.EXPECT().Manifest().Return(&charm.Manifest{Bases: []charm.Base{{ + Name: "ubuntu", + Channel: charm.Channel{Risk: charm.Stable}, + Architectures: []string{"amd64"}, + }}}).MinTimes(1) + pool := domainstorage.StoragePoolDetails{Name: "fast", Provider: "modelscoped-block"} s.state.EXPECT().GetStoragePoolByName(gomock.Any(), "fast").Return(pool, nil) @@ -407,10 +484,15 @@ func (s *applicationServiceSuite) TestCreateWithStorageBlockDefaultSource(c *gc. } _, err := s.service.CreateApplication(context.Background(), "foo", s.charm, corecharm.Origin{ Source: corecharm.CharmHub, - Platform: corecharm.MustParsePlatform("arm64/ubuntu/24.04"), + Platform: corecharm.MustParsePlatform("amd64/ubuntu/24.04"), Revision: ptr(42), }, AddApplicationArgs{ ReferenceName: "foo", + DownloadInfo: &domaincharm.DownloadInfo{ + CharmhubIdentifier: "foo", + DownloadURL: "https://example.com/foo", + DownloadSize: 42, + }, Storage: map[string]storage.Directive{ "data": {Count: 2}, }, @@ -456,18 +538,24 @@ func (s *applicationServiceSuite) TestCreateWithStorageFilesystem(c *gc.C) { }, }, }, + Manifest: s.minimalManifest(c), ReferenceName: "foo", Source: domaincharm.CharmHubSource, Revision: 42, - Architecture: architecture.ARM64, + Architecture: architecture.AMD64, } platform := application.Platform{ Channel: "24.04", OSType: application.Ubuntu, - Architecture: architecture.ARM64, + Architecture: architecture.AMD64, } app := application.AddApplicationArg{ - Charm: ch, + Charm: ch, + CharmDownloadInfo: &domaincharm.DownloadInfo{ + CharmhubIdentifier: "foo", + DownloadURL: "https://example.com/foo", + DownloadSize: 42, + }, Platform: platform, Scale: 1, } @@ -475,7 +563,7 @@ func (s *applicationServiceSuite) TestCreateWithStorageFilesystem(c *gc.C) { s.state.EXPECT().StorageDefaults(gomock.Any()).Return(domainstorage.StorageDefaults{}, nil) s.state.EXPECT().CreateApplication(domaintesting.IsAtomicContextChecker, "foo", app).Return(id, nil) s.state.EXPECT().AddUnits(domaintesting.IsAtomicContextChecker, id, u) - s.charm.EXPECT().Manifest().Return(&charm.Manifest{}) + s.charm.EXPECT().Actions().Return(&charm.Actions{}) s.charm.EXPECT().Config().Return(&charm.Config{}) s.charm.EXPECT().Meta().Return(&charm.Meta{ @@ -490,7 +578,13 @@ func (s *applicationServiceSuite) TestCreateWithStorageFilesystem(c *gc.C) { MinimumSize: 10, }, }, - }).AnyTimes() + }).MinTimes(1) + s.charm.EXPECT().Manifest().Return(&charm.Manifest{Bases: []charm.Base{{ + Name: "ubuntu", + Channel: charm.Channel{Risk: charm.Stable}, + Architectures: []string{"amd64"}, + }}}).MinTimes(1) + pool := domainstorage.StoragePoolDetails{Name: "rootfs", Provider: "rootfs"} s.state.EXPECT().GetStoragePoolByName(gomock.Any(), "rootfs").Return(pool, nil) @@ -499,10 +593,15 @@ func (s *applicationServiceSuite) TestCreateWithStorageFilesystem(c *gc.C) { } _, err := s.service.CreateApplication(context.Background(), "foo", s.charm, corecharm.Origin{ Source: corecharm.CharmHub, - Platform: corecharm.MustParsePlatform("arm64/ubuntu/24.04"), + Platform: corecharm.MustParsePlatform("amd64/ubuntu/24.04"), Revision: ptr(42), }, AddApplicationArgs{ ReferenceName: "foo", + DownloadInfo: &domaincharm.DownloadInfo{ + CharmhubIdentifier: "foo", + DownloadURL: "https://example.com/foo", + DownloadSize: 42, + }, }, a) c.Assert(err, jc.ErrorIsNil) } @@ -545,18 +644,24 @@ func (s *applicationServiceSuite) TestCreateWithStorageFilesystemDefaultSource(c }, }, }, + Manifest: s.minimalManifest(c), ReferenceName: "foo", Source: domaincharm.CharmHubSource, Revision: 42, - Architecture: architecture.ARM64, + Architecture: architecture.AMD64, } platform := application.Platform{ Channel: "24.04", OSType: application.Ubuntu, - Architecture: architecture.ARM64, + Architecture: architecture.AMD64, } app := application.AddApplicationArg{ - Charm: ch, + Charm: ch, + CharmDownloadInfo: &domaincharm.DownloadInfo{ + CharmhubIdentifier: "foo", + DownloadURL: "https://example.com/foo", + DownloadSize: 42, + }, Platform: platform, Scale: 1, } @@ -564,7 +669,7 @@ func (s *applicationServiceSuite) TestCreateWithStorageFilesystemDefaultSource(c s.state.EXPECT().StorageDefaults(gomock.Any()).Return(domainstorage.StorageDefaults{DefaultFilesystemSource: ptr("fast")}, nil) s.state.EXPECT().CreateApplication(domaintesting.IsAtomicContextChecker, "foo", app).Return(id, nil) s.state.EXPECT().AddUnits(domaintesting.IsAtomicContextChecker, id, u) - s.charm.EXPECT().Manifest().Return(&charm.Manifest{}) + s.charm.EXPECT().Actions().Return(&charm.Actions{}) s.charm.EXPECT().Config().Return(&charm.Config{}) s.charm.EXPECT().Meta().Return(&charm.Meta{ @@ -579,7 +684,13 @@ func (s *applicationServiceSuite) TestCreateWithStorageFilesystemDefaultSource(c MinimumSize: 10, }, }, - }).AnyTimes() + }).MinTimes(1) + s.charm.EXPECT().Manifest().Return(&charm.Manifest{Bases: []charm.Base{{ + Name: "ubuntu", + Channel: charm.Channel{Risk: charm.Stable}, + Architectures: []string{"amd64"}, + }}}).MinTimes(1) + pool := domainstorage.StoragePoolDetails{Name: "fast", Provider: "modelscoped"} s.state.EXPECT().GetStoragePoolByName(gomock.Any(), "fast").Return(pool, nil) @@ -588,10 +699,15 @@ func (s *applicationServiceSuite) TestCreateWithStorageFilesystemDefaultSource(c } _, err := s.service.CreateApplication(context.Background(), "foo", s.charm, corecharm.Origin{ Source: corecharm.CharmHub, - Platform: corecharm.MustParsePlatform("arm64/ubuntu/24.04"), + Platform: corecharm.MustParsePlatform("amd64/ubuntu/24.04"), Revision: ptr(42), }, AddApplicationArgs{ ReferenceName: "foo", + DownloadInfo: &domaincharm.DownloadInfo{ + CharmhubIdentifier: "foo", + DownloadURL: "https://example.com/foo", + DownloadSize: 42, + }, Storage: map[string]storage.Directive{ "data": {Count: 2}, }, @@ -613,7 +729,12 @@ func (s *applicationServiceSuite) TestCreateWithSharedStorageMissingDirectives(c Shared: true, }, }, - }).AnyTimes() + }).MinTimes(1) + s.charm.EXPECT().Manifest().Return(&charm.Manifest{Bases: []charm.Base{{ + Name: "ubuntu", + Channel: charm.Channel{Risk: charm.Stable}, + Architectures: []string{"amd64"}, + }}}).MinTimes(1) a := AddUnitArg{ UnitName: "ubuntu/666", @@ -622,6 +743,11 @@ func (s *applicationServiceSuite) TestCreateWithSharedStorageMissingDirectives(c Platform: corecharm.MustParsePlatform("arm64/ubuntu/24.04"), }, AddApplicationArgs{ ReferenceName: "foo", + DownloadInfo: &domaincharm.DownloadInfo{ + CharmhubIdentifier: "foo", + DownloadURL: "https://example.com/foo", + DownloadSize: 42, + }, }, a) c.Assert(err, jc.ErrorIs, storageerrors.MissingSharedStorageDirectiveError) c.Assert(err, gc.ErrorMatches, `.*adding default storage directives: no storage directive specified for shared charm storage "data"`) @@ -640,7 +766,12 @@ func (s *applicationServiceSuite) TestCreateWithStorageValidates(c *gc.C) { Type: charm.StorageBlock, }, }, - }).AnyTimes() + }).MinTimes(1) + s.charm.EXPECT().Manifest().Return(&charm.Manifest{Bases: []charm.Base{{ + Name: "ubuntu", + Channel: charm.Channel{Risk: charm.Beta}, + Architectures: []string{"arm64"}, + }}}).MinTimes(1) // Depending on the map serialization order, the loop may or may not be the // first element. In that case, we need to handle it with a mock if it is @@ -655,6 +786,11 @@ func (s *applicationServiceSuite) TestCreateWithStorageValidates(c *gc.C) { Platform: corecharm.MustParsePlatform("arm64/ubuntu/24.04"), }, AddApplicationArgs{ ReferenceName: "foo", + DownloadInfo: &domaincharm.DownloadInfo{ + CharmhubIdentifier: "foo", + DownloadURL: "https://example.com/foo", + DownloadSize: 42, + }, Storage: map[string]storage.Directive{ "logs": {Count: 2}, }, diff --git a/domain/application/service/charm.go b/domain/application/service/charm.go index 799cf4333e0..1f0d839c4eb 100644 --- a/domain/application/service/charm.go +++ b/domain/application/service/charm.go @@ -118,11 +118,11 @@ type CharmState interface { SetCharmAvailable(ctx context.Context, charmID corecharm.ID) error // GetCharm returns the charm using the charm ID. - GetCharm(ctx context.Context, id corecharm.ID) (charm.Charm, error) + GetCharm(ctx context.Context, id corecharm.ID) (charm.Charm, *charm.DownloadInfo, error) // SetCharm persists the charm metadata, actions, config and manifest to // state. - SetCharm(ctx context.Context, charm charm.Charm) (corecharm.ID, error) + SetCharm(ctx context.Context, charm charm.Charm, downloadInfo *charm.DownloadInfo) (corecharm.ID, error) // DeleteCharm removes the charm from the state. If the charm does not // exist, a [applicationerrors.CharmNotFound] error is returned. @@ -234,7 +234,7 @@ func (s *Service) GetCharm(ctx context.Context, id corecharm.ID) (internalcharm. return nil, charm.CharmLocator{}, fmt.Errorf("charm id: %w", err) } - ch, err := s.st.GetCharm(ctx, id) + ch, _, err := s.st.GetCharm(ctx, id) if err != nil { return nil, charm.CharmLocator{}, errors.Trace(err) } @@ -497,13 +497,30 @@ func (s *Service) SetCharmAvailable(ctx context.Context, id corecharm.ID) error // If there are any non-blocking issues with the charm metadata, actions, // config or manifest, a set of warnings will be returned. func (s *Service) SetCharm(ctx context.Context, args charm.SetCharmArgs) (corecharm.ID, []string, error) { - meta := args.Charm.Meta() - if meta == nil { + // We require a valid charm metadata. + if meta := args.Charm.Meta(); meta == nil { return "", nil, applicationerrors.CharmMetadataNotValid - } else if meta.Name == "" { + } else if !isValidCharmName(meta.Name) { return "", nil, applicationerrors.CharmNameNotValid } + // We require a valid charm manifest. + if manifest := args.Charm.Manifest(); manifest == nil { + return "", nil, applicationerrors.CharmManifestNotFound + } else if len(manifest.Bases) == 0 { + return "", nil, applicationerrors.CharmManifestNotValid + } + + // If the reference name is provided, it must be valid. + if !isValidReferenceName(args.ReferenceName) { + return "", nil, fmt.Errorf("reference name: %w", applicationerrors.CharmNameNotValid) + } + + // If the origin is from charmhub, then we require the download info. + if args.Source == corecharm.CharmHub && args.DownloadInfo == nil { + return "", nil, applicationerrors.CharmDownloadInfoNotFound + } + source, err := encodeCharmSource(args.Source) if err != nil { return "", nil, fmt.Errorf("encoding charm source: %w", err) @@ -523,7 +540,7 @@ func (s *Service) SetCharm(ctx context.Context, args charm.SetCharmArgs) (corech ch.Available = args.ArchivePath != "" ch.Architecture = architecture - charmID, err := s.st.SetCharm(ctx, ch) + charmID, err := s.st.SetCharm(ctx, ch, args.DownloadInfo) if err != nil { return "", warnings, errors.Trace(err) } diff --git a/domain/application/service/charm_test.go b/domain/application/service/charm_test.go index 0d723dee8a8..bbe63a71d53 100644 --- a/domain/application/service/charm_test.go +++ b/domain/application/service/charm_test.go @@ -196,7 +196,7 @@ func (s *charmServiceSuite) TestGetCharm(c *gc.C) { }, Source: charm.LocalSource, Revision: 42, - }, nil) + }, nil, nil) metadata, locator, err := s.service.GetCharm(context.Background(), id) c.Assert(err, jc.ErrorIsNil) @@ -216,7 +216,7 @@ func (s *charmServiceSuite) TestGetCharmCharmNotFound(c *gc.C) { id := charmtesting.GenCharmID(c) - s.state.EXPECT().GetCharm(gomock.Any(), id).Return(charm.Charm{}, applicationerrors.CharmNotFound) + s.state.EXPECT().GetCharm(gomock.Any(), id).Return(charm.Charm{}, nil, applicationerrors.CharmNotFound) _, _, err := s.service.GetCharm(context.Background(), id) c.Assert(err, jc.ErrorIs, applicationerrors.CharmNotFound) @@ -454,23 +454,31 @@ func (s *charmServiceSuite) TestSetCharm(c *gc.C) { id := charmtesting.GenCharmID(c) + s.charm.EXPECT().Actions().Return(&internalcharm.Actions{}) + s.charm.EXPECT().Config().Return(&internalcharm.Config{}) + s.charm.EXPECT().Meta().Return(&internalcharm.Meta{ Name: "foo", }).Times(2) - s.charm.EXPECT().Manifest().Return(&internalcharm.Manifest{}) - s.charm.EXPECT().Actions().Return(&internalcharm.Actions{}) - s.charm.EXPECT().Config().Return(&internalcharm.Config{}) + s.charm.EXPECT().Manifest().Return(&internalcharm.Manifest{Bases: []internalcharm.Base{{ + Name: "ubuntu", + Channel: internalcharm.Channel{Risk: internalcharm.Stable}, + Architectures: []string{"amd64"}, + }}}).MinTimes(1) + + var downloadInfo *charm.DownloadInfo s.state.EXPECT().SetCharm(gomock.Any(), charm.Charm{ Metadata: charm.Metadata{ Name: "foo", RunAs: "default", }, + Manifest: s.minimalManifest(c), ReferenceName: "baz", Source: charm.LocalSource, Revision: 1, Architecture: architecture.AMD64, - }).Return(id, nil) + }, downloadInfo).Return(id, nil) got, warnings, err := s.service.SetCharm(context.Background(), charm.SetCharmArgs{ Charm: s.charm, @@ -478,6 +486,80 @@ func (s *charmServiceSuite) TestSetCharm(c *gc.C) { ReferenceName: "baz", Revision: 1, Architecture: arch.AMD64, + DownloadInfo: downloadInfo, + }) + c.Assert(err, jc.ErrorIsNil) + c.Check(warnings, gc.HasLen, 0) + c.Check(got, gc.DeepEquals, id) +} + +func (s *charmServiceSuite) TestSetCharmCharmhubWithNoDownloadInfo(c *gc.C) { + defer s.setupMocks(c).Finish() + + s.charm.EXPECT().Meta().Return(&internalcharm.Meta{ + Name: "foo", + }).MinTimes(1) + s.charm.EXPECT().Manifest().Return(&internalcharm.Manifest{Bases: []internalcharm.Base{{ + Name: "ubuntu", + Channel: internalcharm.Channel{Risk: internalcharm.Stable}, + Architectures: []string{"amd64"}, + }}}).MinTimes(1) + + var downloadInfo *charm.DownloadInfo + + _, _, err := s.service.SetCharm(context.Background(), charm.SetCharmArgs{ + Charm: s.charm, + Source: corecharm.CharmHub, + ReferenceName: "baz", + Revision: 1, + Architecture: arch.AMD64, + DownloadInfo: downloadInfo, + }) + c.Assert(err, jc.ErrorIs, applicationerrors.CharmDownloadInfoNotFound) +} + +func (s *charmServiceSuite) TestSetCharmCharmhub(c *gc.C) { + defer s.setupMocks(c).Finish() + + id := charmtesting.GenCharmID(c) + + s.charm.EXPECT().Actions().Return(&internalcharm.Actions{}) + s.charm.EXPECT().Config().Return(&internalcharm.Config{}) + + s.charm.EXPECT().Meta().Return(&internalcharm.Meta{ + Name: "foo", + }).Times(2) + s.charm.EXPECT().Manifest().Return(&internalcharm.Manifest{Bases: []internalcharm.Base{{ + Name: "ubuntu", + Channel: internalcharm.Channel{Risk: internalcharm.Stable}, + Architectures: []string{"amd64"}, + }}}).MinTimes(1) + + downloadInfo := &charm.DownloadInfo{ + CharmhubIdentifier: "foo", + DownloadURL: "http://example.com/foo", + DownloadSize: 42, + } + + s.state.EXPECT().SetCharm(gomock.Any(), charm.Charm{ + Metadata: charm.Metadata{ + Name: "foo", + RunAs: "default", + }, + Manifest: s.minimalManifest(c), + ReferenceName: "baz", + Source: charm.CharmHubSource, + Revision: 1, + Architecture: architecture.AMD64, + }, downloadInfo).Return(id, nil) + + got, warnings, err := s.service.SetCharm(context.Background(), charm.SetCharmArgs{ + Charm: s.charm, + Source: corecharm.CharmHub, + ReferenceName: "baz", + Revision: 1, + Architecture: arch.AMD64, + DownloadInfo: downloadInfo, }) c.Assert(err, jc.ErrorIsNil) c.Check(warnings, gc.HasLen, 0) @@ -504,6 +586,11 @@ func (s *charmServiceSuite) TestSetCharmInvalidSource(c *gc.C) { s.charm.EXPECT().Meta().Return(&internalcharm.Meta{ Name: "foo", }) + s.charm.EXPECT().Manifest().Return(&internalcharm.Manifest{Bases: []internalcharm.Base{{ + Name: "ubuntu", + Channel: internalcharm.Channel{Risk: internalcharm.Beta}, + Architectures: []string{"arm64"}, + }}}).MinTimes(1) _, _, err := s.service.SetCharm(context.Background(), charm.SetCharmArgs{ Charm: s.charm, @@ -535,6 +622,11 @@ func (s *charmServiceSuite) TestSetCharmRelationNameConflict(c *gc.C) { }, }, }).Times(2) + s.charm.EXPECT().Manifest().Return(&internalcharm.Manifest{Bases: []internalcharm.Base{{ + Name: "ubuntu", + Channel: internalcharm.Channel{Risk: internalcharm.Beta}, + Architectures: []string{"arm64"}, + }}}).MinTimes(1) _, _, err := s.service.SetCharm(context.Background(), charm.SetCharmArgs{ Charm: s.charm, @@ -559,6 +651,11 @@ func (s *charmServiceSuite) TestSetCharmRelationUnknownRole(c *gc.C) { }, }, }).Times(2) + s.charm.EXPECT().Manifest().Return(&internalcharm.Manifest{Bases: []internalcharm.Base{{ + Name: "ubuntu", + Channel: internalcharm.Channel{Risk: internalcharm.Beta}, + Architectures: []string{"arm64"}, + }}}).MinTimes(1) _, _, err := s.service.SetCharm(context.Background(), charm.SetCharmArgs{ Charm: s.charm, @@ -583,6 +680,11 @@ func (s *charmServiceSuite) TestSetCharmRelationRoleMismatch(c *gc.C) { }, }, }).Times(2) + s.charm.EXPECT().Manifest().Return(&internalcharm.Manifest{Bases: []internalcharm.Base{{ + Name: "ubuntu", + Channel: internalcharm.Channel{Risk: internalcharm.Beta}, + Architectures: []string{"arm64"}, + }}}).MinTimes(1) _, _, err := s.service.SetCharm(context.Background(), charm.SetCharmArgs{ Charm: s.charm, @@ -608,6 +710,11 @@ func (s *charmServiceSuite) TestSetCharmRelationToReservedNameJuju(c *gc.C) { }, }, }).Times(2) + s.charm.EXPECT().Manifest().Return(&internalcharm.Manifest{Bases: []internalcharm.Base{{ + Name: "ubuntu", + Channel: internalcharm.Channel{Risk: internalcharm.Beta}, + Architectures: []string{"arm64"}, + }}}).MinTimes(1) _, _, err := s.service.SetCharm(context.Background(), charm.SetCharmArgs{ Charm: s.charm, @@ -633,6 +740,11 @@ func (s *charmServiceSuite) TestSetCharmRelationToReservedNameJujuBlah(c *gc.C) }, }, }).Times(2) + s.charm.EXPECT().Manifest().Return(&internalcharm.Manifest{Bases: []internalcharm.Base{{ + Name: "ubuntu", + Channel: internalcharm.Channel{Risk: internalcharm.Beta}, + Architectures: []string{"arm64"}, + }}}).MinTimes(1) _, _, err := s.service.SetCharm(context.Background(), charm.SetCharmArgs{ Charm: s.charm, @@ -658,6 +770,11 @@ func (s *charmServiceSuite) TestSetCharmRelationNameToReservedNameJuju(c *gc.C) }, }, }).Times(2) + s.charm.EXPECT().Manifest().Return(&internalcharm.Manifest{Bases: []internalcharm.Base{{ + Name: "ubuntu", + Channel: internalcharm.Channel{Risk: internalcharm.Beta}, + Architectures: []string{"arm64"}, + }}}).MinTimes(1) _, _, err := s.service.SetCharm(context.Background(), charm.SetCharmArgs{ Charm: s.charm, @@ -683,6 +800,11 @@ func (s *charmServiceSuite) TestSetCharmRelationNameToReservedNameJujuBlah(c *gc }, }, }).Times(2) + s.charm.EXPECT().Manifest().Return(&internalcharm.Manifest{Bases: []internalcharm.Base{{ + Name: "ubuntu", + Channel: internalcharm.Channel{Risk: internalcharm.Beta}, + Architectures: []string{"arm64"}, + }}}).MinTimes(1) _, _, err := s.service.SetCharm(context.Background(), charm.SetCharmArgs{ Charm: s.charm, @@ -699,7 +821,6 @@ func (s *charmServiceSuite) TestSetCharmRequireRelationToReservedNameSucceeds(c id := charmtesting.GenCharmID(c) - s.charm.EXPECT().Manifest().Return(&internalcharm.Manifest{}) s.charm.EXPECT().Actions().Return(&internalcharm.Actions{}) s.charm.EXPECT().Config().Return(&internalcharm.Config{}) @@ -714,8 +835,13 @@ func (s *charmServiceSuite) TestSetCharmRequireRelationToReservedNameSucceeds(c }, }, }).Times(2) + s.charm.EXPECT().Manifest().Return(&internalcharm.Manifest{Bases: []internalcharm.Base{{ + Name: "ubuntu", + Channel: internalcharm.Channel{Risk: internalcharm.Beta}, + Architectures: []string{"arm64"}, + }}}).MinTimes(1) - s.state.EXPECT().SetCharm(gomock.Any(), gomock.Any()).Return(id, nil) + s.state.EXPECT().SetCharm(gomock.Any(), gomock.Any(), nil).Return(id, nil) got, warnings, err := s.service.SetCharm(context.Background(), charm.SetCharmArgs{ Charm: s.charm, @@ -743,6 +869,11 @@ func (s *charmServiceSuite) TestSetCharmRequireRelationNameToReservedName(c *gc. }, }, }).Times(2) + s.charm.EXPECT().Manifest().Return(&internalcharm.Manifest{Bases: []internalcharm.Base{{ + Name: "ubuntu", + Channel: internalcharm.Channel{Risk: internalcharm.Beta}, + Architectures: []string{"arm64"}, + }}}).MinTimes(1) _, _, err := s.service.SetCharm(context.Background(), charm.SetCharmArgs{ Charm: s.charm, @@ -759,7 +890,6 @@ func (s *charmServiceSuite) TestSetCharmRelationToReservedNameWithSpecialCharm(c id := charmtesting.GenCharmID(c) - s.charm.EXPECT().Manifest().Return(&internalcharm.Manifest{}) s.charm.EXPECT().Actions().Return(&internalcharm.Actions{}) s.charm.EXPECT().Config().Return(&internalcharm.Config{}) @@ -774,8 +904,13 @@ func (s *charmServiceSuite) TestSetCharmRelationToReservedNameWithSpecialCharm(c }, }, }).Times(2) + s.charm.EXPECT().Manifest().Return(&internalcharm.Manifest{Bases: []internalcharm.Base{{ + Name: "ubuntu", + Channel: internalcharm.Channel{Risk: internalcharm.Beta}, + Architectures: []string{"arm64"}, + }}}).MinTimes(1) - s.state.EXPECT().SetCharm(gomock.Any(), gomock.Any()).Return(id, nil) + s.state.EXPECT().SetCharm(gomock.Any(), gomock.Any(), nil).Return(id, nil) got, warnings, err := s.service.SetCharm(context.Background(), charm.SetCharmArgs{ Charm: s.charm, @@ -794,7 +929,6 @@ func (s *charmServiceSuite) TestSetCharmRelationToReservedNameOnRequiresValid(c id := charmtesting.GenCharmID(c) - s.charm.EXPECT().Manifest().Return(&internalcharm.Manifest{}) s.charm.EXPECT().Actions().Return(&internalcharm.Actions{}) s.charm.EXPECT().Config().Return(&internalcharm.Config{}) @@ -810,8 +944,13 @@ func (s *charmServiceSuite) TestSetCharmRelationToReservedNameOnRequiresValid(c }, }, }).Times(2) + s.charm.EXPECT().Manifest().Return(&internalcharm.Manifest{Bases: []internalcharm.Base{{ + Name: "ubuntu", + Channel: internalcharm.Channel{Risk: internalcharm.Beta}, + Architectures: []string{"arm64"}, + }}}).MinTimes(1) - s.state.EXPECT().SetCharm(gomock.Any(), gomock.Any()).Return(id, nil) + s.state.EXPECT().SetCharm(gomock.Any(), gomock.Any(), nil).Return(id, nil) got, warnings, err := s.service.SetCharm(context.Background(), charm.SetCharmArgs{ Charm: s.charm, @@ -839,6 +978,11 @@ func (s *charmServiceSuite) TestSetCharmRelationToReservedNameOnRequiresInvalid( }, }, }).Times(2) + s.charm.EXPECT().Manifest().Return(&internalcharm.Manifest{Bases: []internalcharm.Base{{ + Name: "ubuntu", + Channel: internalcharm.Channel{Risk: internalcharm.Beta}, + Architectures: []string{"arm64"}, + }}}).MinTimes(1) _, _, err := s.service.SetCharm(context.Background(), charm.SetCharmArgs{ Charm: s.charm, diff --git a/domain/application/service/package_mock_test.go b/domain/application/service/package_mock_test.go index 00ee747d040..e7ec468af25 100644 --- a/domain/application/service/package_mock_test.go +++ b/domain/application/service/package_mock_test.go @@ -536,12 +536,13 @@ func (c *MockStateGetApplicationsWithPendingCharmsFromUUIDsCall) DoAndReturn(f f } // GetCharm mocks base method. -func (m *MockState) GetCharm(arg0 context.Context, arg1 charm.ID) (charm0.Charm, error) { +func (m *MockState) GetCharm(arg0 context.Context, arg1 charm.ID) (charm0.Charm, *charm0.DownloadInfo, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "GetCharm", arg0, arg1) ret0, _ := ret[0].(charm0.Charm) - ret1, _ := ret[1].(error) - return ret0, ret1 + ret1, _ := ret[1].(*charm0.DownloadInfo) + ret2, _ := ret[2].(error) + return ret0, ret1, ret2 } // GetCharm indicates an expected call of GetCharm. @@ -557,19 +558,19 @@ type MockStateGetCharmCall struct { } // Return rewrite *gomock.Call.Return -func (c *MockStateGetCharmCall) Return(arg0 charm0.Charm, arg1 error) *MockStateGetCharmCall { - c.Call = c.Call.Return(arg0, arg1) +func (c *MockStateGetCharmCall) Return(arg0 charm0.Charm, arg1 *charm0.DownloadInfo, arg2 error) *MockStateGetCharmCall { + c.Call = c.Call.Return(arg0, arg1, arg2) return c } // Do rewrite *gomock.Call.Do -func (c *MockStateGetCharmCall) Do(f func(context.Context, charm.ID) (charm0.Charm, error)) *MockStateGetCharmCall { +func (c *MockStateGetCharmCall) Do(f func(context.Context, charm.ID) (charm0.Charm, *charm0.DownloadInfo, error)) *MockStateGetCharmCall { c.Call = c.Call.Do(f) return c } // DoAndReturn rewrite *gomock.Call.DoAndReturn -func (c *MockStateGetCharmCall) DoAndReturn(f func(context.Context, charm.ID) (charm0.Charm, error)) *MockStateGetCharmCall { +func (c *MockStateGetCharmCall) DoAndReturn(f func(context.Context, charm.ID) (charm0.Charm, *charm0.DownloadInfo, error)) *MockStateGetCharmCall { c.Call = c.Call.DoAndReturn(f) return c } @@ -2093,18 +2094,18 @@ func (c *MockStateSetApplicationScalingStateCall) DoAndReturn(f func(domain.Atom } // SetCharm mocks base method. -func (m *MockState) SetCharm(arg0 context.Context, arg1 charm0.Charm) (charm.ID, error) { +func (m *MockState) SetCharm(arg0 context.Context, arg1 charm0.Charm, arg2 *charm0.DownloadInfo) (charm.ID, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "SetCharm", arg0, arg1) + ret := m.ctrl.Call(m, "SetCharm", arg0, arg1, arg2) ret0, _ := ret[0].(charm.ID) ret1, _ := ret[1].(error) return ret0, ret1 } // SetCharm indicates an expected call of SetCharm. -func (mr *MockStateMockRecorder) SetCharm(arg0, arg1 any) *MockStateSetCharmCall { +func (mr *MockStateMockRecorder) SetCharm(arg0, arg1, arg2 any) *MockStateSetCharmCall { mr.mock.ctrl.T.Helper() - call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetCharm", reflect.TypeOf((*MockState)(nil).SetCharm), arg0, arg1) + call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetCharm", reflect.TypeOf((*MockState)(nil).SetCharm), arg0, arg1, arg2) return &MockStateSetCharmCall{Call: call} } @@ -2120,13 +2121,13 @@ func (c *MockStateSetCharmCall) Return(arg0 charm.ID, arg1 error) *MockStateSetC } // Do rewrite *gomock.Call.Do -func (c *MockStateSetCharmCall) Do(f func(context.Context, charm0.Charm) (charm.ID, error)) *MockStateSetCharmCall { +func (c *MockStateSetCharmCall) Do(f func(context.Context, charm0.Charm, *charm0.DownloadInfo) (charm.ID, error)) *MockStateSetCharmCall { c.Call = c.Call.Do(f) return c } // DoAndReturn rewrite *gomock.Call.DoAndReturn -func (c *MockStateSetCharmCall) DoAndReturn(f func(context.Context, charm0.Charm) (charm.ID, error)) *MockStateSetCharmCall { +func (c *MockStateSetCharmCall) DoAndReturn(f func(context.Context, charm0.Charm, *charm0.DownloadInfo) (charm.ID, error)) *MockStateSetCharmCall { c.Call = c.Call.DoAndReturn(f) return c } diff --git a/domain/application/service/package_test.go b/domain/application/service/package_test.go index 526914aa72c..96fa7cf01c8 100644 --- a/domain/application/service/package_test.go +++ b/domain/application/service/package_test.go @@ -19,6 +19,7 @@ import ( modeltesting "github.com/juju/juju/core/model/testing" corestorage "github.com/juju/juju/core/storage" "github.com/juju/juju/domain" + "github.com/juju/juju/domain/application/charm" domaintesting "github.com/juju/juju/domain/testing" "github.com/juju/juju/internal/charm/resource" loggertesting "github.com/juju/juju/internal/logger/testing" @@ -148,6 +149,20 @@ func (s *baseSuite) setupMocksWithAtomic(c *gc.C, fn func(domain.AtomicContext) return ctrl } +func (s *baseSuite) minimalManifest(c *gc.C) charm.Manifest { + return charm.Manifest{ + Bases: []charm.Base{ + { + Name: "ubuntu", + Channel: charm.Channel{ + Risk: charm.RiskStable, + }, + Architectures: []string{"amd64"}, + }, + }, + } +} + func ptr[T any](v T) *T { return &v } diff --git a/domain/application/service/params.go b/domain/application/service/params.go index 912ec9e2c79..c26468fc64b 100644 --- a/domain/application/service/params.go +++ b/domain/application/service/params.go @@ -9,6 +9,7 @@ import ( "github.com/juju/juju/core/network" corestatus "github.com/juju/juju/core/status" coreunit "github.com/juju/juju/core/unit" + domaincharm "github.com/juju/juju/domain/application/charm" "github.com/juju/juju/internal/charm" "github.com/juju/juju/internal/storage" ) @@ -27,10 +28,15 @@ type AddApplicationArgs struct { // that specific revision. The only way to then locate the charm directly // via the name is use the proxy name. ReferenceName string + // CharmStoragePath is the path to the charm in the storage. CharmStoragePath string + // Storage contains the application's storage directives. Storage map[string]storage.Directive + + // DownloadInfo contains the download information for the charm. + DownloadInfo *domaincharm.DownloadInfo } // CloudContainerParams contains parameters for a unit cloud container. diff --git a/domain/application/service/service.go b/domain/application/service/service.go index 207c62da6cc..cd3af4e7129 100644 --- a/domain/application/service/service.go +++ b/domain/application/service/service.go @@ -426,7 +426,7 @@ func (s *MigrationService) GetCharm(ctx context.Context, id corecharm.ID) (inter return nil, charm.CharmLocator{}, fmt.Errorf("charm id: %w", err) } - ch, err := s.st.GetCharm(ctx, id) + ch, _, err := s.st.GetCharm(ctx, id) if err != nil { return nil, charm.CharmLocator{}, errors.Trace(err) } @@ -482,7 +482,7 @@ func (s *MigrationService) ImportApplication( charm internalcharm.Charm, origin corecharm.Origin, args AddApplicationArgs, units ...ImportUnitArg, ) error { - if err := validateCreateApplicationParams(appName, args.ReferenceName, charm, origin); err != nil { + if err := validateCreateApplicationParams(appName, args.ReferenceName, charm, origin, args.DownloadInfo); err != nil { return errors.Annotatef(err, "invalid application args") } diff --git a/domain/application/service/service_test.go b/domain/application/service/service_test.go index d50ab11ca9c..9836f9b3004 100644 --- a/domain/application/service/service_test.go +++ b/domain/application/service/service_test.go @@ -65,6 +65,7 @@ func (s *migrationServiceSuite) TestImportApplication(c *gc.C) { Name: "ubuntu", RunAs: "default", }, + Manifest: s.minimalManifest(c), ReferenceName: "ubuntu", Source: domaincharm.CharmHubSource, Revision: 42, @@ -75,21 +76,37 @@ func (s *migrationServiceSuite) TestImportApplication(c *gc.C) { OSType: application.Ubuntu, Architecture: architecture.ARM64, } + downloadInfo := &domaincharm.DownloadInfo{ + DownloadURL: "http://example.com", + DownloadSize: 24, + CharmhubIdentifier: "foobar", + } app := application.AddApplicationArg{ - Charm: ch, - Platform: platform, - Scale: 1, + Charm: ch, + Platform: platform, + Scale: 1, + CharmDownloadInfo: downloadInfo, } s.state.EXPECT().GetModelType(gomock.Any()).Return("iaas", nil) s.state.EXPECT().StorageDefaults(gomock.Any()).Return(domainstorage.StorageDefaults{}, nil) s.state.EXPECT().CreateApplication(domaintesting.IsAtomicContextChecker, "ubuntu", app).Return(id, nil) s.state.EXPECT().InsertUnit(domaintesting.IsAtomicContextChecker, id, u) - s.charm.EXPECT().Manifest().Return(&charm.Manifest{}) s.charm.EXPECT().Actions().Return(&charm.Actions{}) s.charm.EXPECT().Config().Return(&charm.Config{}) s.charm.EXPECT().Meta().Return(&charm.Meta{ Name: "ubuntu", - }).AnyTimes() + }).MinTimes(1) + s.charm.EXPECT().Manifest().Return(&charm.Manifest{ + Bases: []charm.Base{ + { + Name: "ubuntu", + Channel: charm.Channel{ + Risk: charm.Stable, + }, + Architectures: []string{"amd64"}, + }, + }, + }).MinTimes(1) err := s.service.ImportApplication(context.Background(), "ubuntu", s.charm, corecharm.Origin{ Source: corecharm.CharmHub, @@ -97,6 +114,7 @@ func (s *migrationServiceSuite) TestImportApplication(c *gc.C) { Revision: ptr(42), }, AddApplicationArgs{ ReferenceName: "ubuntu", + DownloadInfo: downloadInfo, }, ImportUnitArg{ UnitName: "ubuntu/666", PasswordHash: ptr("passwordhash"), diff --git a/domain/application/state/application.go b/domain/application/state/application.go index 666e3f3b1ee..800e232fa8b 100644 --- a/domain/application/state/application.go +++ b/domain/application/state/application.go @@ -153,7 +153,7 @@ func (st *State) CreateApplication(ctx domain.AtomicContext, name string, app ap } if shouldInsertCharm { - if err := st.setCharm(ctx, tx, charmID, app.Charm); err != nil { + if err := st.setCharm(ctx, tx, charmID, app.Charm, app.CharmDownloadInfo); err != nil { return errors.Annotate(err, "setting charm") } } @@ -1780,7 +1780,7 @@ WHERE uuid = $applicationID.uuid // Now get the charm by the UUID, but if it doesn't exist, return an // error. chIdent := charmID{UUID: charmIdent.CharmUUID} - ch, err = st.getCharm(ctx, tx, chIdent) + ch, _, err = st.getCharm(ctx, tx, chIdent) if err != nil { if errors.Is(err, sqlair.ErrNoRows) { return internalerrors.Errorf("application %s: %w", appID, applicationerrors.CharmNotFound) diff --git a/domain/application/state/application_test.go b/domain/application/state/application_test.go index 7256ddf74ed..367cead9287 100644 --- a/domain/application/state/application_test.go +++ b/domain/application/state/application_test.go @@ -32,7 +32,6 @@ import ( "github.com/juju/juju/domain/life" "github.com/juju/juju/domain/linklayerdevice" portstate "github.com/juju/juju/domain/port/state" - schematesting "github.com/juju/juju/domain/schema/testing" domainsecret "github.com/juju/juju/domain/secret" secretstate "github.com/juju/juju/domain/secret/state" domainstorage "github.com/juju/juju/domain/storage" @@ -43,7 +42,7 @@ import ( ) type applicationStateSuite struct { - schematesting.ModelSuite + baseSuite state *State } @@ -87,14 +86,18 @@ func (s *applicationStateSuite) TestCreateApplication(c *gc.C) { _, err := s.state.CreateApplication(ctx, "666", application.AddApplicationArg{ Platform: platform, Charm: charm.Charm{ - Metadata: charm.Metadata{ - Name: "666", - }, + Metadata: s.minimalMetadata(c, "666"), + Manifest: s.minimalManifest(c), Source: charm.CharmHubSource, ReferenceName: "666", Revision: 42, Architecture: architecture.ARM64, }, + CharmDownloadInfo: &charm.DownloadInfo{ + CharmhubIdentifier: "ident-1", + DownloadURL: "http://example.com/charm", + DownloadSize: 666, + }, Scale: 1, Channel: channel, }) @@ -121,9 +124,8 @@ func (s *applicationStateSuite) TestCreateApplicationsWithSameCharm(c *gc.C) { Platform: platform, Channel: channel, Charm: charm.Charm{ - Metadata: charm.Metadata{ - Name: "foo", - }, + Metadata: s.minimalMetadata(c, "foo"), + Manifest: s.minimalManifest(c), Source: charm.LocalSource, Revision: 42, Architecture: architecture.ARM64, @@ -137,9 +139,8 @@ func (s *applicationStateSuite) TestCreateApplicationsWithSameCharm(c *gc.C) { Platform: platform, Channel: channel, Charm: charm.Charm{ - Metadata: charm.Metadata{ - Name: "foo", - }, + Metadata: s.minimalMetadata(c, "foo"), + Manifest: s.minimalManifest(c), Source: charm.LocalSource, Revision: 42, Architecture: architecture.ARM64, @@ -167,6 +168,7 @@ func (s *applicationStateSuite) TestCreateApplicationWithoutChannel(c *gc.C) { Metadata: charm.Metadata{ Name: "666", }, + Manifest: s.minimalManifest(c), Source: charm.LocalSource, ReferenceName: "666", Revision: 42, @@ -191,9 +193,8 @@ func (s *applicationStateSuite) TestCreateApplicationWithEmptyChannel(c *gc.C) { _, err := s.state.CreateApplication(ctx, "666", application.AddApplicationArg{ Platform: platform, Charm: charm.Charm{ - Metadata: charm.Metadata{ - Name: "666", - }, + Metadata: s.minimalMetadata(c, "666"), + Manifest: s.minimalManifest(c), Source: charm.LocalSource, Revision: 42, }, @@ -217,9 +218,8 @@ func (s *applicationStateSuite) TestCreateApplicationWithCharmStoragePath(c *gc. _, err := s.state.CreateApplication(ctx, "666", application.AddApplicationArg{ Platform: platform, Charm: charm.Charm{ - Metadata: charm.Metadata{ - Name: "666", - }, + Metadata: s.minimalMetadata(c, "666"), + Manifest: s.minimalManifest(c), Source: charm.LocalSource, Revision: 42, ArchivePath: "/some/path", @@ -1640,7 +1640,7 @@ func (s *applicationStateSuite) TestGetCharmIDByApplicationName(c *gc.C) { ReferenceName: expectedMetadata.Name, Revision: 42, Architecture: architecture.AMD64, - }) + }, nil) c.Assert(err, jc.ErrorIsNil) chID, err := s.state.GetCharmIDByApplicationName(context.Background(), "foo") @@ -1850,6 +1850,7 @@ func (s *applicationStateSuite) TestSetCharmThenGetCharmByApplicationNameInvalid _, err := s.state.CreateApplication(ctx, "foo", application.AddApplicationArg{ Charm: charm.Charm{ Metadata: expectedMetadata, + Manifest: s.minimalManifest(c), Source: charm.LocalSource, }, }) @@ -2014,6 +2015,7 @@ func (s *applicationStateSuite) createApplication(c *gc.C, name string, l life.L }, }, }, + Manifest: s.minimalManifest(c), ReferenceName: name, Source: charm.CharmHubSource, Revision: 42, diff --git a/domain/application/state/charm.go b/domain/application/state/charm.go index 35e212516b5..7ebfda5164f 100644 --- a/domain/application/state/charm.go +++ b/domain/application/state/charm.go @@ -545,33 +545,37 @@ func (s *State) GetCharmActions(ctx context.Context, id corecharm.ID) (charm.Act } // GetCharm returns the charm using the charm ID. +// DownloadInfo is optional, and is only returned for charms from a charm store. // If the charm does not exist, a [errors.CharmNotFound] error is returned. -func (s *State) GetCharm(ctx context.Context, id corecharm.ID) (charm.Charm, error) { +func (s *State) GetCharm(ctx context.Context, id corecharm.ID) (charm.Charm, *charm.DownloadInfo, error) { db, err := s.DB() if err != nil { - return charm.Charm{}, internalerrors.Capture(err) + return charm.Charm{}, nil, internalerrors.Capture(err) } ident := charmID{UUID: id.String()} - var ch charm.Charm + var ( + ch charm.Charm + di *charm.DownloadInfo + ) if err := db.Txn(ctx, func(ctx context.Context, tx *sqlair.TX) error { var err error - ch, err = s.getCharm(ctx, tx, ident) + ch, di, err = s.getCharm(ctx, tx, ident) if err != nil { return internalerrors.Capture(err) } return nil }); err != nil { - return ch, internalerrors.Errorf("getting charm: %w", err) + return ch, nil, internalerrors.Errorf("getting charm: %w", err) } - return ch, nil + return ch, di, nil } // SetCharm persists the charm metadata, actions, config and manifest to // state. -func (s *State) SetCharm(ctx context.Context, charm charm.Charm) (corecharm.ID, error) { +func (s *State) SetCharm(ctx context.Context, charm charm.Charm, downloadInfo *charm.DownloadInfo) (corecharm.ID, error) { db, err := s.DB() if err != nil { return "", internalerrors.Capture(err) @@ -590,7 +594,7 @@ func (s *State) SetCharm(ctx context.Context, charm charm.Charm) (corecharm.ID, return internalerrors.Capture(err) } - if err := s.setCharm(ctx, tx, id, charm); err != nil { + if err := s.setCharm(ctx, tx, id, charm, downloadInfo); err != nil { return internalerrors.Capture(err) } diff --git a/domain/application/state/charm_test.go b/domain/application/state/charm_test.go index e6ec03cac8e..e2eeeb357d6 100644 --- a/domain/application/state/charm_test.go +++ b/domain/application/state/charm_test.go @@ -20,13 +20,12 @@ import ( "github.com/juju/juju/domain/application/architecture" "github.com/juju/juju/domain/application/charm" applicationerrors "github.com/juju/juju/domain/application/errors" - schematesting "github.com/juju/juju/domain/schema/testing" loggertesting "github.com/juju/juju/internal/logger/testing" "github.com/juju/juju/internal/uuid" ) type charmStateSuite struct { - schematesting.ModelSuite + baseSuite } var _ = gc.Suite(&charmStateSuite{}) @@ -108,12 +107,13 @@ func (s *charmStateSuite) TestSetCharmNotAvailable(c *gc.C) { id, err := st.SetCharm(context.Background(), charm.Charm{ Metadata: expected, + Manifest: s.minimalManifest(c), Source: charm.LocalSource, Revision: 42, ReferenceName: "foo", Hash: "hash", Version: "deadbeef", - }) + }, nil) c.Assert(err, jc.ErrorIsNil) charmID, err := st.GetCharmID(context.Background(), "foo", 42, charm.LocalSource) @@ -143,13 +143,14 @@ func (s *charmStateSuite) TestSetCharmGetCharmID(c *gc.C) { id, err := st.SetCharm(context.Background(), charm.Charm{ Metadata: expected, + Manifest: s.minimalManifest(c), Source: charm.LocalSource, Revision: 42, ReferenceName: "foo", Hash: "hash", ArchivePath: "archive", Version: "deadbeef", - }) + }, nil) c.Assert(err, jc.ErrorIsNil) charmID, err := st.GetCharmID(context.Background(), "foo", 42, charm.LocalSource) @@ -1191,6 +1192,169 @@ func (s *charmStateSuite) TestDeleteCharm(c *gc.C) { c.Assert(err, jc.ErrorIs, applicationerrors.CharmNotFound) } +func (s *charmStateSuite) TestSetCharmDownloadInfoForCharmhub(c *gc.C) { + st := NewState(s.TxnRunnerFactory(), clock.WallClock, loggertesting.WrapCheckLog(c)) + + info := &charm.DownloadInfo{ + CharmhubIdentifier: "ident-1", + DownloadURL: "https://example.com/charmhub/ident-1", + DownloadSize: 1234, + } + + id, err := st.SetCharm(context.Background(), charm.Charm{ + Metadata: charm.Metadata{ + Name: "ubuntu", + Summary: "summary", + Description: "description", + Subordinate: true, + RunAs: charm.RunAsRoot, + MinJujuVersion: version.MustParse("4.0.0"), + Assumes: []byte("null"), + }, + Manifest: charm.Manifest{ + Bases: []charm.Base{ + charm.Base{ + Name: "ubuntu", + Channel: charm.Channel{ + Risk: charm.RiskCandidate, + }, + Architectures: []string{"amd64"}, + }, + }, + }, + Source: charm.CharmHubSource, + Revision: 42, + ReferenceName: "ubuntu", + Hash: "hash", + ArchivePath: "archive", + Version: "deadbeef", + }, info) + c.Assert(err, jc.ErrorIsNil) + + _, downloadInfo, err := st.GetCharm(context.Background(), id) + c.Assert(err, jc.ErrorIsNil) + c.Check(downloadInfo, gc.DeepEquals, info) +} + +func (s *charmStateSuite) TestSetCharmDownloadInfoForCharmhubWithoutDownloadInfo(c *gc.C) { + st := NewState(s.TxnRunnerFactory(), clock.WallClock, loggertesting.WrapCheckLog(c)) + + id, err := st.SetCharm(context.Background(), charm.Charm{ + Metadata: charm.Metadata{ + Name: "ubuntu", + Summary: "summary", + Description: "description", + Subordinate: true, + RunAs: charm.RunAsRoot, + MinJujuVersion: version.MustParse("4.0.0"), + Assumes: []byte("null"), + }, + Manifest: charm.Manifest{ + Bases: []charm.Base{ + charm.Base{ + Name: "ubuntu", + Channel: charm.Channel{ + Risk: charm.RiskCandidate, + }, + Architectures: []string{"amd64"}, + }, + }, + }, + Source: charm.CharmHubSource, + Revision: 42, + ReferenceName: "ubuntu", + Hash: "hash", + ArchivePath: "archive", + Version: "deadbeef", + }, nil) + c.Assert(err, jc.ErrorIsNil) + + _, _, err = st.GetCharm(context.Background(), id) + c.Assert(err, jc.ErrorIs, applicationerrors.CharmDownloadInfoNotFound) +} + +func (s *charmStateSuite) TestSetCharmDownloadInfoForLocal(c *gc.C) { + st := NewState(s.TxnRunnerFactory(), clock.WallClock, loggertesting.WrapCheckLog(c)) + + info := &charm.DownloadInfo{ + CharmhubIdentifier: "ident-1", + DownloadURL: "https://example.com/charmhub/ident-1", + DownloadSize: 1234, + } + + id, err := st.SetCharm(context.Background(), charm.Charm{ + Metadata: charm.Metadata{ + Name: "ubuntu", + Summary: "summary", + Description: "description", + Subordinate: true, + RunAs: charm.RunAsRoot, + MinJujuVersion: version.MustParse("4.0.0"), + Assumes: []byte("null"), + }, + Manifest: charm.Manifest{ + Bases: []charm.Base{ + charm.Base{ + Name: "ubuntu", + Channel: charm.Channel{ + Risk: charm.RiskCandidate, + }, + Architectures: []string{"amd64"}, + }, + }, + }, + Source: charm.LocalSource, + Revision: 42, + ReferenceName: "ubuntu", + Hash: "hash", + ArchivePath: "archive", + Version: "deadbeef", + }, info) + c.Assert(err, jc.ErrorIsNil) + + _, downloadInfo, err := st.GetCharm(context.Background(), id) + c.Assert(err, jc.ErrorIsNil) + c.Check(downloadInfo, gc.IsNil) +} + +func (s *charmStateSuite) TestSetCharmDownloadInfoForLocalWithoutInfo(c *gc.C) { + st := NewState(s.TxnRunnerFactory(), clock.WallClock, loggertesting.WrapCheckLog(c)) + + id, err := st.SetCharm(context.Background(), charm.Charm{ + Metadata: charm.Metadata{ + Name: "ubuntu", + Summary: "summary", + Description: "description", + Subordinate: true, + RunAs: charm.RunAsRoot, + MinJujuVersion: version.MustParse("4.0.0"), + Assumes: []byte("null"), + }, + Manifest: charm.Manifest{ + Bases: []charm.Base{ + charm.Base{ + Name: "ubuntu", + Channel: charm.Channel{ + Risk: charm.RiskCandidate, + }, + Architectures: []string{"amd64"}, + }, + }, + }, + Source: charm.LocalSource, + Revision: 42, + ReferenceName: "ubuntu", + Hash: "hash", + ArchivePath: "archive", + Version: "deadbeef", + }, nil) + c.Assert(err, jc.ErrorIsNil) + + _, downloadInfo, err := st.GetCharm(context.Background(), id) + c.Assert(err, jc.ErrorIsNil) + c.Check(downloadInfo, gc.IsNil) +} + func (s *charmStateSuite) TestSetCharmTwice(c *gc.C) { st := NewState(s.TxnRunnerFactory(), clock.WallClock, loggertesting.WrapCheckLog(c)) @@ -1206,13 +1370,14 @@ func (s *charmStateSuite) TestSetCharmTwice(c *gc.C) { id, err := st.SetCharm(context.Background(), charm.Charm{ Metadata: expected, + Manifest: s.minimalManifest(c), Source: charm.LocalSource, Revision: 42, ReferenceName: "ubuntu", Hash: "hash", ArchivePath: "archive", Version: "deadbeef", - }) + }, nil) c.Assert(err, jc.ErrorIsNil) got, err := st.GetCharmMetadata(context.Background(), id) @@ -1221,13 +1386,14 @@ func (s *charmStateSuite) TestSetCharmTwice(c *gc.C) { _, err = st.SetCharm(context.Background(), charm.Charm{ Metadata: expected, + Manifest: s.minimalManifest(c), Source: charm.LocalSource, Revision: 42, ReferenceName: "ubuntu", Hash: "hash", ArchivePath: "archive", Version: "deadbeef", - }) + }, nil) c.Assert(err, jc.ErrorIs, applicationerrors.CharmAlreadyExists) } @@ -1288,10 +1454,10 @@ func (s *charmStateSuite) TestSetCharmThenGetCharm(c *gc.C) { Hash: "hash", ArchivePath: "archive", Version: "deadbeef", - }) + }, nil) c.Assert(err, jc.ErrorIsNil) - gotCharm, err := st.GetCharm(context.Background(), id) + gotCharm, _, err := st.GetCharm(context.Background(), id) c.Assert(err, jc.ErrorIsNil) c.Check(gotCharm, gc.DeepEquals, charm.Charm{ Metadata: expectedMetadata, @@ -1368,13 +1534,13 @@ func (s *charmStateSuite) TestSetCharmThenGetCharmWithDifferentReferenceName(c * Hash: "hash", ArchivePath: "archive", Version: "deadbeef", - }) + }, nil) c.Assert(err, jc.ErrorIsNil) id, err := st.GetCharmID(context.Background(), "baz", 42, charm.LocalSource) c.Assert(err, jc.ErrorIsNil) - gotCharm, err := st.GetCharm(context.Background(), id) + gotCharm, _, err := st.GetCharm(context.Background(), id) c.Assert(err, jc.ErrorIsNil) c.Check(gotCharm, gc.DeepEquals, charm.Charm{ Metadata: expectedMetadata, @@ -1406,13 +1572,14 @@ func (s *charmStateSuite) TestSetCharmAllowsSameNameButDifferentRevision(c *gc.C id1, err := st.SetCharm(context.Background(), charm.Charm{ Metadata: expected, + Manifest: s.minimalManifest(c), Source: charm.LocalSource, Revision: 1, ReferenceName: "ubuntu", Hash: "hash", ArchivePath: "archive", Version: "deadbeef", - }) + }, nil) c.Assert(err, jc.ErrorIsNil) got, err := st.GetCharmMetadata(context.Background(), id1) @@ -1421,13 +1588,14 @@ func (s *charmStateSuite) TestSetCharmAllowsSameNameButDifferentRevision(c *gc.C id2, err := st.SetCharm(context.Background(), charm.Charm{ Metadata: expected, + Manifest: s.minimalManifest(c), Source: charm.LocalSource, Revision: 2, ReferenceName: "ubuntu", Hash: "hash", ArchivePath: "archive", Version: "deadbeef", - }) + }, nil) c.Assert(err, jc.ErrorIsNil) got, err = st.GetCharmMetadata(context.Background(), id2) @@ -1450,13 +1618,14 @@ func (s *charmStateSuite) TestSetCharmThenGetCharmMetadata(c *gc.C) { id, err := st.SetCharm(context.Background(), charm.Charm{ Metadata: expected, + Manifest: s.minimalManifest(c), Source: charm.LocalSource, Revision: 42, ReferenceName: "ubuntu", Hash: "hash", ArchivePath: "archive", Version: "deadbeef", - }) + }, nil) c.Assert(err, jc.ErrorIsNil) got, err := st.GetCharmMetadata(context.Background(), id) @@ -1487,13 +1656,14 @@ func (s *charmStateSuite) TestSetCharmThenGetCharmMetadataWithTagsAndCategories( id, err := st.SetCharm(context.Background(), charm.Charm{ Metadata: expected, + Manifest: s.minimalManifest(c), Source: charm.LocalSource, Revision: 42, ReferenceName: "ubuntu", Hash: "hash", ArchivePath: "archive", Version: "deadbeef", - }) + }, nil) c.Assert(err, jc.ErrorIsNil) got, err := st.GetCharmMetadata(context.Background(), id) @@ -1523,13 +1693,14 @@ func (s *charmStateSuite) TestSetCharmThenGetCharmMetadataWithTerms(c *gc.C) { id, err := st.SetCharm(context.Background(), charm.Charm{ Metadata: expected, + Manifest: s.minimalManifest(c), Source: charm.LocalSource, Revision: 42, ReferenceName: "ubuntu", Hash: "hash", ArchivePath: "archive", Version: "deadbeef", - }) + }, nil) c.Assert(err, jc.ErrorIsNil) got, err := st.GetCharmMetadata(context.Background(), id) @@ -1588,13 +1759,14 @@ func (s *charmStateSuite) TestSetCharmThenGetCharmMetadataWithRelations(c *gc.C) id, err := st.SetCharm(context.Background(), charm.Charm{ Metadata: expected, + Manifest: s.minimalManifest(c), Source: charm.LocalSource, Revision: 42, ReferenceName: "ubuntu", Hash: "hash", ArchivePath: "archive", Version: "deadbeef", - }) + }, nil) c.Assert(err, jc.ErrorIsNil) got, err := st.GetCharmMetadata(context.Background(), id) @@ -1631,13 +1803,14 @@ func (s *charmStateSuite) TestSetCharmThenGetCharmMetadataWithExtraBindings(c *g id, err := st.SetCharm(context.Background(), charm.Charm{ Metadata: expected, + Manifest: s.minimalManifest(c), Source: charm.LocalSource, Revision: 42, ReferenceName: "ubuntu", Hash: "hash", ArchivePath: "archive", Version: "deadbeef", - }) + }, nil) c.Assert(err, jc.ErrorIsNil) got, err := st.GetCharmMetadata(context.Background(), id) @@ -1690,13 +1863,14 @@ func (s *charmStateSuite) TestSetCharmThenGetCharmMetadataWithStorageWithNoPrope id, err := st.SetCharm(context.Background(), charm.Charm{ Metadata: expected, + Manifest: s.minimalManifest(c), Source: charm.LocalSource, Revision: 42, ReferenceName: "ubuntu", Hash: "hash", ArchivePath: "archive", Version: "deadbeef", - }) + }, nil) c.Assert(err, jc.ErrorIsNil) got, err := st.GetCharmMetadata(context.Background(), id) @@ -1751,13 +1925,14 @@ func (s *charmStateSuite) TestSetCharmThenGetCharmMetadataWithStorageWithPropert id, err := st.SetCharm(context.Background(), charm.Charm{ Metadata: expected, + Manifest: s.minimalManifest(c), Source: charm.LocalSource, Revision: 42, ReferenceName: "ubuntu", Hash: "hash", ArchivePath: "archive", Version: "deadbeef", - }) + }, nil) c.Assert(err, jc.ErrorIsNil) got, err := st.GetCharmMetadata(context.Background(), id) @@ -1803,13 +1978,14 @@ func (s *charmStateSuite) TestSetCharmThenGetCharmMetadataWithDevices(c *gc.C) { id, err := st.SetCharm(context.Background(), charm.Charm{ Metadata: expected, + Manifest: s.minimalManifest(c), Source: charm.LocalSource, Revision: 42, ReferenceName: "ubuntu", Hash: "hash", ArchivePath: "archive", Version: "deadbeef", - }) + }, nil) c.Assert(err, jc.ErrorIsNil) got, err := st.GetCharmMetadata(context.Background(), id) @@ -1848,13 +2024,14 @@ func (s *charmStateSuite) TestSetCharmThenGetCharmMetadataWithPayloadClasses(c * id, err := st.SetCharm(context.Background(), charm.Charm{ Metadata: expected, + Manifest: s.minimalManifest(c), Source: charm.LocalSource, Revision: 42, ReferenceName: "ubuntu", Hash: "hash", ArchivePath: "archive", Version: "deadbeef", - }) + }, nil) c.Assert(err, jc.ErrorIsNil) got, err := st.GetCharmMetadata(context.Background(), id) @@ -1897,13 +2074,14 @@ func (s *charmStateSuite) TestSetCharmThenGetCharmMetadataWithResources(c *gc.C) id, err := st.SetCharm(context.Background(), charm.Charm{ Metadata: expected, + Manifest: s.minimalManifest(c), Source: charm.LocalSource, Revision: 42, ReferenceName: "ubuntu", Hash: "hash", ArchivePath: "archive", Version: "deadbeef", - }) + }, nil) c.Assert(err, jc.ErrorIsNil) got, err := st.GetCharmMetadata(context.Background(), id) @@ -1942,13 +2120,14 @@ func (s *charmStateSuite) TestSetCharmThenGetCharmMetadataWithContainersWithNoMo id, err := st.SetCharm(context.Background(), charm.Charm{ Metadata: expected, + Manifest: s.minimalManifest(c), Source: charm.LocalSource, Revision: 42, ReferenceName: "ubuntu", Hash: "hash", ArchivePath: "archive", Version: "deadbeef", - }) + }, nil) c.Assert(err, jc.ErrorIsNil) got, err := st.GetCharmMetadata(context.Background(), id) @@ -2007,13 +2186,14 @@ func (s *charmStateSuite) TestSetCharmThenGetCharmMetadataWithContainersWithMoun id, err := st.SetCharm(context.Background(), charm.Charm{ Metadata: expected, + Manifest: s.minimalManifest(c), Source: charm.LocalSource, Revision: 42, ReferenceName: "ubuntu", Hash: "hash", ArchivePath: "archive", Version: "deadbeef", - }) + }, nil) c.Assert(err, jc.ErrorIsNil) got, err := st.GetCharmMetadata(context.Background(), id) @@ -2147,7 +2327,7 @@ func (s *charmStateSuite) TestSetCharmThenGetCharmManifest(c *gc.C) { Hash: "hash", ArchivePath: "archive", Version: "deadbeef", - }) + }, nil) c.Assert(err, jc.ErrorIsNil) got, err := st.GetCharmManifest(context.Background(), id) @@ -2346,6 +2526,7 @@ func (s *charmStateSuite) TestSetCharmThenGetCharmConfig(c *gc.C) { Metadata: charm.Metadata{ Name: "ubuntu", }, + Manifest: s.minimalManifest(c), Config: expected, Source: charm.LocalSource, Revision: 42, @@ -2353,7 +2534,7 @@ func (s *charmStateSuite) TestSetCharmThenGetCharmConfig(c *gc.C) { Hash: "hash", ArchivePath: "archive", Version: "deadbeef", - }) + }, nil) c.Assert(err, jc.ErrorIsNil) got, err := st.GetCharmConfig(context.Background(), id) @@ -2470,6 +2651,7 @@ func (s *charmStateSuite) TestSetCharmThenGetCharmActions(c *gc.C) { Metadata: charm.Metadata{ Name: "ubuntu", }, + Manifest: s.minimalManifest(c), Actions: expected, Source: charm.LocalSource, Revision: 42, @@ -2477,7 +2659,7 @@ func (s *charmStateSuite) TestSetCharmThenGetCharmActions(c *gc.C) { Hash: "hash", ArchivePath: "archive", Version: "deadbeef", - }) + }, nil) c.Assert(err, jc.ErrorIsNil) got, err := st.GetCharmActions(context.Background(), id) @@ -2528,13 +2710,14 @@ func (s *charmStateSuite) TestSetCharmThenGetCharmArchivePath(c *gc.C) { Metadata: charm.Metadata{ Name: "ubuntu", }, + Manifest: s.minimalManifest(c), Source: charm.LocalSource, Revision: 42, ReferenceName: "ubuntu", Hash: "hash", ArchivePath: "archive", Version: "deadbeef", - }) + }, nil) c.Assert(err, jc.ErrorIsNil) got, err := st.GetCharmArchivePath(context.Background(), id) @@ -2564,15 +2747,16 @@ func (s *charmStateSuite) TestSetCharmWithDuplicatedEndpointNames(c *gc.C) { }, }, }, + Manifest: s.minimalManifest(c), Source: charm.LocalSource, Revision: 42, ReferenceName: "ubuntu", Hash: "hash", ArchivePath: "archive", Version: "deadbeef", - }) + }, nil) - c.Assert(err, gc.ErrorMatches, `.*failed to insert charm relations.*`) + c.Assert(err, jc.ErrorIs, applicationerrors.CharmRelationNameConflict) } func (s *charmStateSuite) TestGetCharmArchivePathCharmNotFound(c *gc.C) { @@ -2591,13 +2775,14 @@ func (s *charmStateSuite) TestGetCharmArchiveMetadata(c *gc.C) { Metadata: charm.Metadata{ Name: "ubuntu", }, + Manifest: s.minimalManifest(c), Source: charm.LocalSource, Revision: 42, ReferenceName: "ubuntu", Hash: "hash", ArchivePath: "archive", Version: "deadbeef", - }) + }, nil) c.Assert(err, jc.ErrorIsNil) got, hash, err := st.GetCharmArchiveMetadata(context.Background(), id) @@ -2614,17 +2799,18 @@ func (s *charmStateSuite) TestGetCharmArchiveMetadataInsertAdditionalHashKind(c Metadata: charm.Metadata{ Name: "ubuntu", }, + Manifest: s.minimalManifest(c), Source: charm.LocalSource, Revision: 42, ReferenceName: "ubuntu", Hash: "hash", ArchivePath: "archive", Version: "deadbeef", - }) + }, nil) c.Assert(err, jc.ErrorIsNil) err = s.TxnRunner().StdTxn(context.Background(), func(ctx context.Context, tx *sql.Tx) error { - return insertAdditonalHashKindForCharm(ctx, c, tx, id, "sha386", "hash386") + return insertAdditionalHashKindForCharm(ctx, c, tx, id, "sha386", "hash386") }) c.Assert(err, jc.ErrorIsNil) @@ -2656,13 +2842,14 @@ func (s *charmStateSuite) TestListCharmLocators(c *gc.C) { Metadata: charm.Metadata{ Name: "ubuntu", }, + Manifest: s.minimalManifest(c), Source: charm.LocalSource, Revision: 42, ReferenceName: "ubuntu", Hash: "hash", ArchivePath: "archive", Version: "deadbeef", - }) + }, nil) c.Assert(err, jc.ErrorIsNil) results, err := st.ListCharmLocators(context.Background()) @@ -2686,13 +2873,14 @@ func (s *charmStateSuite) TestListCharmLocatorsMultipleEntries(c *gc.C) { Metadata: charm.Metadata{ Name: name, }, + Manifest: s.minimalManifest(c), Source: charm.LocalSource, Revision: 42, ReferenceName: name, Hash: "hash", ArchivePath: "archive", Version: "deadbeef", - }) + }, nil) c.Assert(err, jc.ErrorIsNil) expected = append(expected, charm.CharmLocator{ @@ -2727,13 +2915,14 @@ func (s *charmStateSuite) TestListCharmLocatorsByNamesMultipleEntries(c *gc.C) { Metadata: charm.Metadata{ Name: name, }, + Manifest: s.minimalManifest(c), Source: charm.LocalSource, Revision: 42, ReferenceName: name, Hash: "hash", ArchivePath: "archive", Version: "deadbeef", - }) + }, nil) c.Assert(err, jc.ErrorIsNil) // We only want to check the first and last entries. @@ -2764,13 +2953,14 @@ func (s *charmStateSuite) TestListCharmLocatorsByNamesInvalidEntries(c *gc.C) { Metadata: charm.Metadata{ Name: name, }, + Manifest: s.minimalManifest(c), Source: charm.LocalSource, Revision: 42, ReferenceName: name, Hash: "hash", ArchivePath: "archive", Version: "deadbeef", - }) + }, nil) c.Assert(err, jc.ErrorIsNil) } @@ -2822,7 +3012,7 @@ func insertCharmManifest(ctx context.Context, c *gc.C, tx *sql.Tx, uuid string) return charm.Manifest{}, nil } -func insertAdditonalHashKindForCharm(ctx context.Context, c *gc.C, tx *sql.Tx, charmId corecharm.ID, kind, hash string) error { +func insertAdditionalHashKindForCharm(ctx context.Context, c *gc.C, tx *sql.Tx, charmId corecharm.ID, kind, hash string) error { var kindId int rows, err := tx.QueryContext(ctx, `SELECT id FROM hash_kind`) c.Assert(err, jc.ErrorIsNil) diff --git a/domain/application/state/package_test.go b/domain/application/state/package_test.go index 689a9b62b54..5090f2c2928 100644 --- a/domain/application/state/package_test.go +++ b/domain/application/state/package_test.go @@ -7,8 +7,35 @@ import ( "testing" gc "gopkg.in/check.v1" + + "github.com/juju/juju/domain/application/charm" + schematesting "github.com/juju/juju/domain/schema/testing" ) func TestPackage(t *testing.T) { gc.TestingT(t) } + +type baseSuite struct { + schematesting.ModelSuite +} + +func (s *baseSuite) minimalMetadata(c *gc.C, name string) charm.Metadata { + return charm.Metadata{ + Name: name, + } +} + +func (s *baseSuite) minimalManifest(c *gc.C) charm.Manifest { + return charm.Manifest{ + Bases: []charm.Base{ + { + Name: "ubuntu", + Channel: charm.Channel{ + Risk: charm.RiskStable, + }, + Architectures: []string{"amd64"}, + }, + }, + } +} diff --git a/domain/application/state/state.go b/domain/application/state/state.go index cc15194debc..c07d4887027 100644 --- a/domain/application/state/state.go +++ b/domain/application/state/state.go @@ -20,6 +20,7 @@ import ( "github.com/juju/juju/domain/application/architecture" "github.com/juju/juju/domain/application/charm" applicationerrors "github.com/juju/juju/domain/application/errors" + internaldatabase "github.com/juju/juju/internal/database" internalerrors "github.com/juju/juju/internal/errors" ) @@ -48,7 +49,7 @@ WHERE uuid = $charmID.uuid; result := charmID{UUID: id.UUID} selectStmt, err := s.Prepare(selectQuery, result) if err != nil { - return internalerrors.Errorf("failed to prepare query: %w", err) + return internalerrors.Errorf("preparing query: %w", err) } if err := tx.Query(ctx, selectStmt, result).Get(&result); err != nil { if errors.Is(err, sqlair.ErrNoRows) { @@ -79,7 +80,7 @@ AND revision = $charmReferenceNameRevisionSource.revision var result charmID selectStmt, err := s.Prepare(selectQuery, result, ref) if err != nil { - return "", fmt.Errorf("failed to prepare query: %w", err) + return "", fmt.Errorf("preparing query: %w", err) } if err := tx.Query(ctx, selectStmt, ref).Get(&result); err != nil { if errors.Is(err, sqlair.ErrNoRows) { @@ -96,67 +97,74 @@ AND revision = $charmReferenceNameRevisionSource.revision return id, applicationerrors.CharmAlreadyExists } -func (s *State) setCharm(ctx context.Context, tx *sqlair.TX, uuid corecharm.ID, charm charm.Charm) error { - if err := s.setCharmState(ctx, tx, uuid, charm); err != nil { +func (s *State) setCharm(ctx context.Context, tx *sqlair.TX, uuid corecharm.ID, ch charm.Charm, downloadInfo *charm.DownloadInfo) error { + if err := s.setCharmState(ctx, tx, uuid, ch); err != nil { return errors.Trace(err) } - if err := s.setCharmMetadata(ctx, tx, uuid, charm.Metadata, charm.LXDProfile); err != nil { + if err := s.setCharmMetadata(ctx, tx, uuid, ch.Metadata, ch.LXDProfile); err != nil { return errors.Trace(err) } - if err := s.setCharmTags(ctx, tx, uuid, charm.Metadata.Tags); err != nil { + if err := s.setCharmTags(ctx, tx, uuid, ch.Metadata.Tags); err != nil { return errors.Trace(err) } - if err := s.setCharmCategories(ctx, tx, uuid, charm.Metadata.Categories); err != nil { + if err := s.setCharmCategories(ctx, tx, uuid, ch.Metadata.Categories); err != nil { return errors.Trace(err) } - if err := s.setCharmTerms(ctx, tx, uuid, charm.Metadata.Terms); err != nil { + if err := s.setCharmTerms(ctx, tx, uuid, ch.Metadata.Terms); err != nil { return errors.Trace(err) } - if err := s.setCharmRelations(ctx, tx, uuid, charm.Metadata); err != nil { + if err := s.setCharmRelations(ctx, tx, uuid, ch.Metadata); err != nil { return errors.Trace(err) } - if err := s.setCharmExtraBindings(ctx, tx, uuid, charm.Metadata.ExtraBindings); err != nil { + if err := s.setCharmExtraBindings(ctx, tx, uuid, ch.Metadata.ExtraBindings); err != nil { return errors.Trace(err) } - if err := s.setCharmStorage(ctx, tx, uuid, charm.Metadata.Storage); err != nil { + if err := s.setCharmStorage(ctx, tx, uuid, ch.Metadata.Storage); err != nil { return errors.Trace(err) } - if err := s.setCharmDevices(ctx, tx, uuid, charm.Metadata.Devices); err != nil { + if err := s.setCharmDevices(ctx, tx, uuid, ch.Metadata.Devices); err != nil { return errors.Trace(err) } - if err := s.setCharmPayloads(ctx, tx, uuid, charm.Metadata.PayloadClasses); err != nil { + if err := s.setCharmPayloads(ctx, tx, uuid, ch.Metadata.PayloadClasses); err != nil { return errors.Trace(err) } - if err := s.setCharmResources(ctx, tx, uuid, charm.Metadata.Resources); err != nil { + if err := s.setCharmResources(ctx, tx, uuid, ch.Metadata.Resources); err != nil { return errors.Trace(err) } - if err := s.setCharmContainers(ctx, tx, uuid, charm.Metadata.Containers); err != nil { + if err := s.setCharmContainers(ctx, tx, uuid, ch.Metadata.Containers); err != nil { return errors.Trace(err) } - if err := s.setCharmActions(ctx, tx, uuid, charm.Actions); err != nil { + if err := s.setCharmActions(ctx, tx, uuid, ch.Actions); err != nil { return errors.Trace(err) } - if err := s.setCharmConfig(ctx, tx, uuid, charm.Config); err != nil { + if err := s.setCharmConfig(ctx, tx, uuid, ch.Config); err != nil { return errors.Trace(err) } - if err := s.setCharmManifest(ctx, tx, uuid, charm.Manifest); err != nil { + if err := s.setCharmManifest(ctx, tx, uuid, ch.Manifest); err != nil { return errors.Trace(err) } + // Insert the download info if the charm is from CharmHub. + if ch.Source == charm.CharmHubSource { + if err := s.setCharmDownloadInfo(ctx, tx, uuid, downloadInfo); err != nil { + return errors.Trace(err) + } + } + return nil } @@ -198,7 +206,7 @@ func (s *State) setCharmState( charmQuery := `INSERT INTO charm (*) VALUES ($setCharmState.*);` charmStmt, err := s.Prepare(charmQuery, chState) if err != nil { - return fmt.Errorf("failed to prepare query: %w", err) + return fmt.Errorf("preparing query: %w", err) } hash := setCharmHash{ @@ -210,15 +218,40 @@ func (s *State) setCharmState( hashQuery := `INSERT INTO charm_hash (*) VALUES ($setCharmHash.*);` hashStmt, err := s.Prepare(hashQuery, hash) if err != nil { - return fmt.Errorf("failed to prepare query: %w", err) + return fmt.Errorf("preparing query: %w", err) } if err := tx.Query(ctx, charmStmt, chState).Run(); err != nil { - return fmt.Errorf("failed to insert charm state: %w", err) + return fmt.Errorf("inserting charm state: %w", err) } if err := tx.Query(ctx, hashStmt, hash).Run(); err != nil { - return fmt.Errorf("failed to insert charm hash: %w", err) + return fmt.Errorf("inserting charm hash: %w", err) + } + + return nil +} + +func (s *State) setCharmDownloadInfo(ctx context.Context, tx *sqlair.TX, id corecharm.ID, downloadInfo *charm.DownloadInfo) error { + if downloadInfo == nil { + return nil + } + + downloadInfoState := setCharmDownloadInfo{ + CharmUUID: id.String(), + CharmhubIdentifier: downloadInfo.CharmhubIdentifier, + DownloadURL: downloadInfo.DownloadURL, + DownloadSize: downloadInfo.DownloadSize, + } + + query := `INSERT INTO charm_download_info (*) VALUES ($setCharmDownloadInfo.*);` + stmt, err := s.Prepare(query, downloadInfoState) + if err != nil { + return fmt.Errorf("preparing query: %w", err) + } + + if err := tx.Query(ctx, stmt, downloadInfoState).Run(); err != nil { + return fmt.Errorf("inserting charm download info: %w", err) } return nil @@ -239,11 +272,11 @@ func (s *State) setCharmMetadata( query := `INSERT INTO charm_metadata (*) VALUES ($setCharmMetadata.*);` stmt, err := s.Prepare(query, encodedMetadata) if err != nil { - return fmt.Errorf("failed to prepare query: %w", err) + return fmt.Errorf("preparing query: %w", err) } if err := tx.Query(ctx, stmt, encodedMetadata).Run(); err != nil { - return fmt.Errorf("failed to insert charm metadata: %w", err) + return fmt.Errorf("inserting charm metadata: %w", err) } return nil @@ -258,11 +291,11 @@ func (s *State) setCharmTags(ctx context.Context, tx *sqlair.TX, id corecharm.ID query := `INSERT INTO charm_tag (*) VALUES ($setCharmTag.*);` stmt, err := s.Prepare(query, setCharmTag{}) if err != nil { - return fmt.Errorf("failed to prepare query: %w", err) + return fmt.Errorf("preparing query: %w", err) } if err := tx.Query(ctx, stmt, encodeTags(id, tags)).Run(); err != nil { - return fmt.Errorf("failed to insert charm tag: %w", err) + return fmt.Errorf("inserting charm tag: %w", err) } return nil @@ -277,11 +310,11 @@ func (s *State) setCharmCategories(ctx context.Context, tx *sqlair.TX, id corech query := `INSERT INTO charm_category (*) VALUES ($setCharmCategory.*);` stmt, err := s.Prepare(query, setCharmCategory{}) if err != nil { - return fmt.Errorf("failed to prepare query: %w", err) + return fmt.Errorf("preparing query: %w", err) } if err := tx.Query(ctx, stmt, encodeCategories(id, categories)).Run(); err != nil { - return fmt.Errorf("failed to insert charm categories: %w", err) + return fmt.Errorf("inserting charm categories: %w", err) } return nil @@ -296,11 +329,11 @@ func (s *State) setCharmTerms(ctx context.Context, tx *sqlair.TX, id corecharm.I query := `INSERT INTO charm_term (*) VALUES ($setCharmTerm.*);` stmt, err := s.Prepare(query, setCharmTerm{}) if err != nil { - return fmt.Errorf("failed to prepare query: %w", err) + return fmt.Errorf("preparing query: %w", err) } if err := tx.Query(ctx, stmt, encodeTerms(id, terms)).Run(); err != nil { - return fmt.Errorf("failed to insert charm terms: %w", err) + return fmt.Errorf("inserting charm terms: %w", err) } return nil @@ -320,11 +353,13 @@ func (s *State) setCharmRelations(ctx context.Context, tx *sqlair.TX, id corecha query := `INSERT INTO charm_relation (*) VALUES ($setCharmRelation.*);` stmt, err := s.Prepare(query, setCharmRelation{}) if err != nil { - return fmt.Errorf("failed to prepare query: %w", err) + return fmt.Errorf("preparing query: %w", err) } - if err := tx.Query(ctx, stmt, encodedRelations).Run(); err != nil { - return fmt.Errorf("failed to insert charm relations: %w", err) + if err := tx.Query(ctx, stmt, encodedRelations).Run(); internaldatabase.IsErrConstraintUnique(err) { + return applicationerrors.CharmRelationNameConflict + } else if err != nil { + return fmt.Errorf("inserting charm relations: %w", err) } return nil @@ -339,11 +374,11 @@ func (s *State) setCharmExtraBindings(ctx context.Context, tx *sqlair.TX, id cor query := `INSERT INTO charm_extra_binding (*) VALUES ($setCharmExtraBinding.*);` stmt, err := s.Prepare(query, setCharmExtraBinding{}) if err != nil { - return fmt.Errorf("failed to prepare query: %w", err) + return fmt.Errorf("preparing query: %w", err) } if err := tx.Query(ctx, stmt, encodeExtraBindings(id, extraBindings)).Run(); err != nil { - return fmt.Errorf("failed to insert charm extra bindings: %w", err) + return fmt.Errorf("inserting charm extra bindings: %w", err) } return nil @@ -363,11 +398,11 @@ func (s *State) setCharmStorage(ctx context.Context, tx *sqlair.TX, id corecharm storageQuery := `INSERT INTO charm_storage (*) VALUES ($setCharmStorage.*);` storageStmt, err := s.Prepare(storageQuery, setCharmStorage{}) if err != nil { - return fmt.Errorf("failed to prepare query: %w", err) + return fmt.Errorf("preparing query: %w", err) } if err := tx.Query(ctx, storageStmt, encodedStorage).Run(); err != nil { - return fmt.Errorf("failed to insert charm storage: %w", err) + return fmt.Errorf("inserting charm storage: %w", err) } // Only insert properties if there are any. @@ -375,11 +410,11 @@ func (s *State) setCharmStorage(ctx context.Context, tx *sqlair.TX, id corecharm propertiesQuery := `INSERT INTO charm_storage_property (*) VALUES ($setCharmStorageProperty.*);` propertiesStmt, err := s.Prepare(propertiesQuery, setCharmStorageProperty{}) if err != nil { - return fmt.Errorf("failed to prepare query: %w", err) + return fmt.Errorf("preparing query: %w", err) } if err := tx.Query(ctx, propertiesStmt, encodedProperties).Run(); err != nil { - return fmt.Errorf("failed to insert charm storage properties: %w", err) + return fmt.Errorf("inserting charm storage properties: %w", err) } } return nil @@ -394,11 +429,11 @@ func (s *State) setCharmDevices(ctx context.Context, tx *sqlair.TX, id corecharm query := `INSERT INTO charm_device (*) VALUES ($setCharmDevice.*);` stmt, err := s.Prepare(query, setCharmDevice{}) if err != nil { - return fmt.Errorf("failed to prepare query: %w", err) + return fmt.Errorf("preparing query: %w", err) } if err := tx.Query(ctx, stmt, encodeDevices(id, devices)).Run(); err != nil { - return fmt.Errorf("failed to insert charm devices: %w", err) + return fmt.Errorf("inserting charm devices: %w", err) } return nil @@ -413,11 +448,11 @@ func (s *State) setCharmPayloads(ctx context.Context, tx *sqlair.TX, id corechar query := `INSERT INTO charm_payload (*) VALUES ($setCharmPayload.*);` stmt, err := s.Prepare(query, setCharmPayload{}) if err != nil { - return fmt.Errorf("failed to prepare query: %w", err) + return fmt.Errorf("preparing query: %w", err) } if err := tx.Query(ctx, stmt, encodePayloads(id, payloads)).Run(); err != nil { - return fmt.Errorf("failed to insert charm payloads: %w", err) + return fmt.Errorf("inserting charm payloads: %w", err) } return nil @@ -437,11 +472,11 @@ func (s *State) setCharmResources(ctx context.Context, tx *sqlair.TX, id corecha query := `INSERT INTO charm_resource (*) VALUES ($setCharmResource.*);` stmt, err := s.Prepare(query, setCharmResource{}) if err != nil { - return fmt.Errorf("failed to prepare query: %w", err) + return fmt.Errorf("preparing query: %w", err) } if err := tx.Query(ctx, stmt, encodedResources).Run(); err != nil { - return fmt.Errorf("failed to insert charm resources: %w", err) + return fmt.Errorf("inserting charm resources: %w", err) } return nil @@ -461,11 +496,11 @@ func (s *State) setCharmContainers(ctx context.Context, tx *sqlair.TX, id corech containerQuery := `INSERT INTO charm_container (*) VALUES ($setCharmContainer.*);` containerStmt, err := s.Prepare(containerQuery, setCharmContainer{}) if err != nil { - return fmt.Errorf("failed to prepare query: %w", err) + return fmt.Errorf("preparing query: %w", err) } if err := tx.Query(ctx, containerStmt, encodedContainers).Run(); err != nil { - return fmt.Errorf("failed to insert charm containers: %w", err) + return fmt.Errorf("inserting charm containers: %w", err) } // Only insert mounts if there are any. @@ -473,11 +508,11 @@ func (s *State) setCharmContainers(ctx context.Context, tx *sqlair.TX, id corech mountQuery := `INSERT INTO charm_container_mount (*) VALUES ($setCharmMount.*);` mountStmt, err := s.Prepare(mountQuery, setCharmMount{}) if err != nil { - return fmt.Errorf("failed to prepare query: %w", err) + return fmt.Errorf("preparing query: %w", err) } if err := tx.Query(ctx, mountStmt, encodedMounts).Run(); err != nil { - return fmt.Errorf("failed to insert charm container mounts: %w", err) + return fmt.Errorf("inserting charm container mounts: %w", err) } } @@ -493,11 +528,11 @@ func (s *State) setCharmActions(ctx context.Context, tx *sqlair.TX, id corecharm query := `INSERT INTO charm_action (*) VALUES ($setCharmAction.*);` stmt, err := s.Prepare(query, setCharmAction{}) if err != nil { - return fmt.Errorf("failed to prepare query: %w", err) + return fmt.Errorf("preparing query: %w", err) } if err := tx.Query(ctx, stmt, encodeActions(id, actions)).Run(); err != nil { - return fmt.Errorf("failed to insert charm actions: %w", err) + return fmt.Errorf("inserting charm actions: %w", err) } return nil @@ -517,20 +552,19 @@ func (s *State) setCharmConfig(ctx context.Context, tx *sqlair.TX, id corecharm. query := `INSERT INTO charm_config (*) VALUES ($setCharmConfig.*);` stmt, err := s.Prepare(query, setCharmConfig{}) if err != nil { - return fmt.Errorf("failed to prepare query: %w", err) + return fmt.Errorf("preparing query: %w", err) } if err := tx.Query(ctx, stmt, encodedConfig).Run(); err != nil { - return fmt.Errorf("failed to insert charm config: %w", err) + return fmt.Errorf("inserting charm config: %w", err) } return nil } func (s *State) setCharmManifest(ctx context.Context, tx *sqlair.TX, id corecharm.ID, manifest charm.Manifest) error { - // If there are no resources, we don't need to do anything. if len(manifest.Bases) == 0 { - return nil + return applicationerrors.CharmManifestNotFound } encodedManifest, err := encodeManifest(id, manifest) @@ -541,11 +575,11 @@ func (s *State) setCharmManifest(ctx context.Context, tx *sqlair.TX, id corechar query := `INSERT INTO charm_manifest_base (*) VALUES ($setCharmManifest.*);` stmt, err := s.Prepare(query, setCharmManifest{}) if err != nil { - return fmt.Errorf("failed to prepare query: %w", err) + return fmt.Errorf("preparing query: %w", err) } if err := tx.Query(ctx, stmt, encodedManifest).Run(); err != nil { - return fmt.Errorf("failed to insert charm manifest: %w", err) + return fmt.Errorf("inserting charm manifest: %w", err) } return nil @@ -554,31 +588,43 @@ func (s *State) setCharmManifest(ctx context.Context, tx *sqlair.TX, id corechar // getCharm returns the charm for the given charm ID. // This will delegate to the various get methods to get the charm metadata, // config, manifest, actions and LXD profile. -func (s *State) getCharm(ctx context.Context, tx *sqlair.TX, ident charmID) (charm.Charm, error) { - charm, err := s.getCharmState(ctx, tx, ident) +func (s *State) getCharm(ctx context.Context, tx *sqlair.TX, ident charmID) (charm.Charm, *charm.DownloadInfo, error) { + ch, err := s.getCharmState(ctx, tx, ident) if err != nil { - return charm, errors.Trace(err) + return ch, nil, errors.Trace(err) + } + + if ch.Metadata, err = s.getMetadata(ctx, tx, ident); err != nil { + return ch, nil, errors.Trace(err) } - if charm.Metadata, err = s.getMetadata(ctx, tx, ident); err != nil { - return charm, errors.Trace(err) + + if ch.Config, err = s.getCharmConfig(ctx, tx, ident); err != nil { + return ch, nil, errors.Trace(err) } - if charm.Config, err = s.getCharmConfig(ctx, tx, ident); err != nil { - return charm, errors.Trace(err) + if ch.Manifest, err = s.getCharmManifest(ctx, tx, ident); err != nil { + return ch, nil, errors.Trace(err) } - if charm.Manifest, err = s.getCharmManifest(ctx, tx, ident); err != nil { - return charm, errors.Trace(err) + if ch.Actions, err = s.getCharmActions(ctx, tx, ident); err != nil { + return ch, nil, errors.Trace(err) } - if charm.Actions, err = s.getCharmActions(ctx, tx, ident); err != nil { - return charm, errors.Trace(err) + if ch.LXDProfile, _, err = s.getCharmLXDProfile(ctx, tx, ident); err != nil { + return ch, nil, errors.Trace(err) } - if charm.LXDProfile, _, err = s.getCharmLXDProfile(ctx, tx, ident); err != nil { - return charm, errors.Trace(err) + // Download information should only be recorded for charmhub charms. + // If it's not present, ensure we report it as not found. + var downloadInfo *charm.DownloadInfo + if info, err := s.getCharmDownloadInfo(ctx, tx, ident); err != nil && !errors.Is(err, sql.ErrNoRows) { + return ch, nil, errors.Trace(err) + } else if err == nil { + downloadInfo = &info + } else if ch.Source == charm.CharmHubSource { + return ch, nil, applicationerrors.CharmDownloadInfoNotFound } - return charm, nil + return ch, downloadInfo, nil } func (s *State) getCharmState(ctx context.Context, tx *sqlair.TX, ident charmID) (charm.Charm, error) { @@ -590,7 +636,7 @@ WHERE uuid = $charmID.uuid; charmStmt, err := s.Prepare(charmQuery, charmState{}, ident) if err != nil { - return charm.Charm{}, fmt.Errorf("failed to prepare query: %w", err) + return charm.Charm{}, fmt.Errorf("preparing query: %w", err) } hashQuery := ` @@ -629,6 +675,32 @@ WHERE charm_uuid = $charmID.uuid; return result, nil } +func (s *State) getCharmDownloadInfo(ctx context.Context, tx *sqlair.TX, ident charmID) (charm.DownloadInfo, error) { + query := ` +SELECT &charmDownloadInfo.* +FROM charm AS c +JOIN charm_download_info AS cdi ON c.uuid = cdi.charm_uuid +WHERE c.uuid = $charmID.uuid +AND c.source_id = 1; +` + + stmt, err := s.Prepare(query, charmDownloadInfo{}, ident) + if err != nil { + return charm.DownloadInfo{}, fmt.Errorf("preparing query: %w", err) + } + + var downloadInfo charmDownloadInfo + if err := tx.Query(ctx, stmt, ident).Get(&downloadInfo); err != nil { + return charm.DownloadInfo{}, fmt.Errorf("getting charm download info: %w", err) + } + + return charm.DownloadInfo{ + CharmhubIdentifier: downloadInfo.CharmhubIdentifier, + DownloadURL: downloadInfo.DownloadURL, + DownloadSize: downloadInfo.DownloadSize, + }, nil +} + // getMetadata returns the metadata for the charm using the charm ID. // If the charm does not exist, a [errors.CharmNotFound] error is returned. // It's safe to do this in the transaction loop, the query will cached against @@ -732,7 +804,7 @@ ORDER BY array_index ASC, nested_array_index ASC; stmt, err := s.Prepare(query, charmManifest{}, ident) if err != nil { - return charm.Manifest{}, fmt.Errorf("failed to prepare query: %w", err) + return charm.Manifest{}, fmt.Errorf("preparing query: %w", err) } var manifests []charmManifest @@ -740,7 +812,7 @@ ORDER BY array_index ASC, nested_array_index ASC; if errors.Is(err, sqlair.ErrNoRows) { return charm.Manifest{}, applicationerrors.CharmNotFound } - return charm.Manifest{}, fmt.Errorf("failed to get charm manifest: %w", err) + return charm.Manifest{}, fmt.Errorf("getting charm manifest: %w", err) } return decodeManifest(manifests) @@ -768,26 +840,26 @@ WHERE uuid = $charmID.uuid; charmStmt, err := s.Prepare(charmQuery, ident) if err != nil { - return nil, -1, fmt.Errorf("failed to prepare charm query: %w", err) + return nil, -1, fmt.Errorf("preparing charm query: %w", err) } var profile charmLXDProfile lxdProfileStmt, err := s.Prepare(lxdProfileQuery, profile, ident) if err != nil { - return nil, -1, fmt.Errorf("failed to prepare lxd profile query: %w", err) + return nil, -1, fmt.Errorf("preparing lxd profile query: %w", err) } if err := tx.Query(ctx, charmStmt, ident).Get(&ident); err != nil { if errors.Is(err, sqlair.ErrNoRows) { return nil, -1, applicationerrors.CharmNotFound } - return nil, -1, fmt.Errorf("failed to get charm: %w", err) + return nil, -1, fmt.Errorf("getting charm: %w", err) } if err := tx.Query(ctx, lxdProfileStmt, ident).Get(&profile); err != nil { if errors.Is(err, sqlair.ErrNoRows) { return nil, -1, applicationerrors.LXDProfileNotFound } - return nil, -1, fmt.Errorf("failed to get charm lxd profile: %w", err) + return nil, -1, fmt.Errorf("getting charm lxd profile: %w", err) } // TODO - figure out why this is happening @@ -818,18 +890,18 @@ WHERE charm_uuid = $charmID.uuid; charmStmt, err := s.Prepare(charmQuery, ident) if err != nil { - return charm.Config{}, fmt.Errorf("failed to prepare charm query: %w", err) + return charm.Config{}, fmt.Errorf("preparing charm query: %w", err) } configStmt, err := s.Prepare(configQuery, charmConfig{}, ident) if err != nil { - return charm.Config{}, fmt.Errorf("failed to prepare config query: %w", err) + return charm.Config{}, fmt.Errorf("preparing config query: %w", err) } if err := tx.Query(ctx, charmStmt, ident).Get(&ident); err != nil { if errors.Is(err, sqlair.ErrNoRows) { return charm.Config{}, applicationerrors.CharmNotFound } - return charm.Config{}, fmt.Errorf("failed to get charm: %w", err) + return charm.Config{}, fmt.Errorf("getting charm: %w", err) } var configs []charmConfig @@ -837,7 +909,7 @@ WHERE charm_uuid = $charmID.uuid; if errors.Is(err, sqlair.ErrNoRows) { return charm.Config{}, nil } - return charm.Config{}, fmt.Errorf("failed to get charm config: %w", err) + return charm.Config{}, fmt.Errorf("getting charm config: %w", err) } return decodeConfig(configs) @@ -862,18 +934,18 @@ WHERE charm_uuid = $charmID.uuid; charmStmt, err := s.Prepare(charmQuery, ident) if err != nil { - return charm.Actions{}, fmt.Errorf("failed to prepare charm query: %w", err) + return charm.Actions{}, fmt.Errorf("preparing charm query: %w", err) } actionsStmt, err := s.Prepare(actionQuery, charmAction{}, ident) if err != nil { - return charm.Actions{}, fmt.Errorf("failed to prepare action query: %w", err) + return charm.Actions{}, fmt.Errorf("preparing action query: %w", err) } if err := tx.Query(ctx, charmStmt, ident).Get(&ident); err != nil { if errors.Is(err, sqlair.ErrNoRows) { return charm.Actions{}, applicationerrors.CharmNotFound } - return charm.Actions{}, fmt.Errorf("failed to get charm: %w", err) + return charm.Actions{}, fmt.Errorf("getting charm: %w", err) } var actions []charmAction @@ -881,7 +953,7 @@ WHERE charm_uuid = $charmID.uuid; if errors.Is(err, sqlair.ErrNoRows) { return charm.Actions{}, nil } - return charm.Actions{}, fmt.Errorf("failed to get charm actions: %w", err) + return charm.Actions{}, fmt.Errorf("getting charm actions: %w", err) } return decodeActions(actions), nil @@ -898,7 +970,7 @@ WHERE uuid = $charmID.uuid; var metadata charmMetadata stmt, err := s.Prepare(query, metadata, ident) if err != nil { - return metadata, fmt.Errorf("failed to prepare query: %w", err) + return metadata, fmt.Errorf("preparing query: %w", err) } if err := tx.Query(ctx, stmt, ident).Get(&metadata); err != nil { @@ -927,7 +999,7 @@ ORDER BY array_index ASC; ` stmt, err := s.Prepare(query, charmTag{}, ident) if err != nil { - return nil, fmt.Errorf("failed to prepare query: %w", err) + return nil, fmt.Errorf("preparing query: %w", err) } var result []charmTag @@ -957,7 +1029,7 @@ ORDER BY array_index ASC; ` stmt, err := s.Prepare(query, charmCategory{}, ident) if err != nil { - return nil, fmt.Errorf("failed to prepare query: %w", err) + return nil, fmt.Errorf("preparing query: %w", err) } var result []charmCategory @@ -987,7 +1059,7 @@ ORDER BY array_index ASC; ` stmt, err := s.Prepare(query, charmTerm{}, ident) if err != nil { - return nil, fmt.Errorf("failed to prepare query: %w", err) + return nil, fmt.Errorf("preparing query: %w", err) } var result []charmTerm @@ -1014,7 +1086,7 @@ WHERE charm_uuid = $charmID.uuid; ` stmt, err := s.Prepare(query, charmRelation{}, ident) if err != nil { - return nil, fmt.Errorf("failed to prepare query: %w", err) + return nil, fmt.Errorf("preparing query: %w", err) } var result []charmRelation @@ -1043,7 +1115,7 @@ WHERE charm_uuid = $charmID.uuid; stmt, err := s.Prepare(query, charmExtraBinding{}, ident) if err != nil { - return nil, fmt.Errorf("failed to prepare query: %w", err) + return nil, fmt.Errorf("preparing query: %w", err) } var result []charmExtraBinding @@ -1071,7 +1143,7 @@ ORDER BY property_index ASC; stmt, err := s.Prepare(query, charmStorage{}, ident) if err != nil { - return nil, fmt.Errorf("failed to prepare query: %w", err) + return nil, fmt.Errorf("preparing query: %w", err) } var result []charmStorage @@ -1097,7 +1169,7 @@ WHERE charm_uuid = $charmID.uuid; stmt, err := s.Prepare(query, charmDevice{}, ident) if err != nil { - return nil, fmt.Errorf("failed to prepare query: %w", err) + return nil, fmt.Errorf("preparing query: %w", err) } var result []charmDevice @@ -1123,7 +1195,7 @@ WHERE charm_uuid = $charmID.uuid; stmt, err := s.Prepare(query, charmPayload{}, ident) if err != nil { - return nil, fmt.Errorf("failed to prepare query: %w", err) + return nil, fmt.Errorf("preparing query: %w", err) } var result []charmPayload @@ -1149,7 +1221,7 @@ WHERE charm_uuid = $charmID.uuid; stmt, err := s.Prepare(query, charmResource{}, ident) if err != nil { - return nil, fmt.Errorf("failed to prepare query: %w", err) + return nil, fmt.Errorf("preparing query: %w", err) } var result []charmResource @@ -1176,7 +1248,7 @@ ORDER BY array_index ASC; stmt, err := s.Prepare(query, charmContainer{}, ident) if err != nil { - return nil, fmt.Errorf("failed to prepare query: %w", err) + return nil, fmt.Errorf("preparing query: %w", err) } var result []charmContainer diff --git a/domain/application/state/types.go b/domain/application/state/types.go index ac481688ae0..5a9c3d79e59 100644 --- a/domain/application/state/types.go +++ b/domain/application/state/types.go @@ -253,6 +253,21 @@ type setCharmState struct { Version string `db:"version"` } +// charmDownloadInfo is used to get the download info of a charm. +type charmDownloadInfo struct { + CharmhubIdentifier string `db:"charmhub_identifier"` + DownloadURL string `db:"download_url"` + DownloadSize int64 `db:"download_size"` +} + +// setCharmDownloadInfo is used to set the download info of a charm. +type setCharmDownloadInfo struct { + CharmUUID string `db:"charm_uuid"` + CharmhubIdentifier string `db:"charmhub_identifier"` + DownloadURL string `db:"download_url"` + DownloadSize int64 `db:"download_size"` +} + // charmMetadata is used to get the metadata of a charm. type charmMetadata struct { Name string `db:"name"` diff --git a/domain/application/types.go b/domain/application/types.go index 7bb8fd76ec1..c8136c08f53 100644 --- a/domain/application/types.go +++ b/domain/application/types.go @@ -16,10 +16,11 @@ import ( // AddApplicationArg contains parameters for saving an application to state. type AddApplicationArg struct { - Charm domaincharm.Charm - Scale int - Platform Platform - Channel *Channel + Charm domaincharm.Charm + CharmDownloadInfo *domaincharm.DownloadInfo + Scale int + Platform Platform + Channel *Channel } // Channel represents the channel of a application charm. diff --git a/domain/schema/model/sql/0015-charm.sql b/domain/schema/model/sql/0015-charm.sql index 601d9adf4f3..13a42ea96ab 100644 --- a/domain/schema/model/sql/0015-charm.sql +++ b/domain/schema/model/sql/0015-charm.sql @@ -24,10 +24,6 @@ CREATE TABLE charm ( archive_path TEXT, available BOOLEAN DEFAULT FALSE, - -- charmhub_identifier is the identifier that charmhub uses to identify the - -- charm. This is used to refresh the charm from charmhub. The - -- reference_name can change but the charmhub_identifier will not. - charmhub_identifier TEXT, version TEXT, -- The following fields are purely here to reconstruct the charm URL. @@ -68,6 +64,21 @@ CREATE TABLE charm ( CREATE UNIQUE INDEX idx_charm_reference_name_revision ON charm (reference_name, revision); +CREATE TABLE charm_download_info ( + charm_uuid TEXT NOT NULL PRIMARY KEY, + -- charmhub_identifier is the identifier that charmhub uses to identify the + -- charm. This is used to refresh the charm from charmhub. The + -- reference_name can change but the charmhub_identifier will not. + charmhub_identifier TEXT NOT NULL, + + download_url TEXT NOT NULL, + download_size INT NOT NULL, + + CONSTRAINT fk_charm_download_info_charm + FOREIGN KEY (charm_uuid) + REFERENCES charm (uuid) +); + CREATE TABLE charm_metadata ( charm_uuid TEXT NOT NULL, -- name represents the original name of the charm. This is what is stored diff --git a/domain/schema/model/triggers/application-triggers.gen.go b/domain/schema/model/triggers/application-triggers.gen.go index 8eab457acf5..d34ca353d53 100644 --- a/domain/schema/model/triggers/application-triggers.gen.go +++ b/domain/schema/model/triggers/application-triggers.gen.go @@ -114,7 +114,6 @@ WHEN NEW.uuid != OLD.uuid OR (NEW.archive_path != OLD.archive_path OR (NEW.archive_path IS NOT NULL AND OLD.archive_path IS NULL) OR (NEW.archive_path IS NULL AND OLD.archive_path IS NOT NULL)) OR (NEW.available != OLD.available OR (NEW.available IS NOT NULL AND OLD.available IS NULL) OR (NEW.available IS NULL AND OLD.available IS NOT NULL)) OR - (NEW.charmhub_identifier != OLD.charmhub_identifier OR (NEW.charmhub_identifier IS NOT NULL AND OLD.charmhub_identifier IS NULL) OR (NEW.charmhub_identifier IS NULL AND OLD.charmhub_identifier IS NOT NULL)) OR (NEW.version != OLD.version OR (NEW.version IS NOT NULL AND OLD.version IS NULL) OR (NEW.version IS NULL AND OLD.version IS NOT NULL)) OR NEW.source_id != OLD.source_id OR NEW.revision != OLD.revision OR diff --git a/internal/bootstrap/deployer.go b/internal/bootstrap/deployer.go index 6914546a11c..b3a91a2b1be 100644 --- a/internal/bootstrap/deployer.go +++ b/internal/bootstrap/deployer.go @@ -29,6 +29,7 @@ import ( "github.com/juju/juju/core/network" "github.com/juju/juju/core/objectstore" "github.com/juju/juju/core/unit" + applicationcharm "github.com/juju/juju/domain/application/charm" applicationservice "github.com/juju/juju/domain/application/service" "github.com/juju/juju/environs/bootstrap" "github.com/juju/juju/internal/charm" @@ -392,6 +393,7 @@ func (b *baseDeployer) AddControllerApplication(ctx context.Context, curl string ch, origin, applicationservice.AddApplicationArgs{ ReferenceName: bootstrap.ControllerApplicationName, + DownloadInfo: &applicationcharm.DownloadInfo{}, }, applicationservice.AddUnitArg{UnitName: unitName}, ) diff --git a/internal/testing/factory/factory.go b/internal/testing/factory/factory.go index f0259bb3a3c..779d1b2289f 100644 --- a/internal/testing/factory/factory.go +++ b/internal/testing/factory/factory.go @@ -28,6 +28,7 @@ import ( "github.com/juju/juju/core/status" coreunit "github.com/juju/juju/core/unit" jujuversion "github.com/juju/juju/core/version" + applicationcharm "github.com/juju/juju/domain/application/charm" applicationerrors "github.com/juju/juju/domain/application/errors" applicationservice "github.com/juju/juju/domain/application/service" "github.com/juju/juju/environs/config" @@ -505,6 +506,7 @@ func (factory *Factory) MakeApplicationReturningPassword(c *gc.C, params *Applic }, applicationservice.AddApplicationArgs{ ReferenceName: params.Name, Storage: directives, + DownloadInfo: &applicationcharm.DownloadInfo{}, }) } c.Assert(err, jc.ErrorIsNil) From f79e349de9242ea4638da34aed12a740a2c5056a Mon Sep 17 00:00:00 2001 From: Simon Richardson Date: Mon, 2 Dec 2024 12:11:58 +0000 Subject: [PATCH 2/7] feat: start wire up download info --- .../client/application/deployrepository.go | 67 +++++++++++++------ core/charm/repository.go | 20 ++++++ internal/charm/repository/charmhub.go | 5 ++ internal/charm/repository/charmhub_test.go | 37 ++++++---- 4 files changed, 94 insertions(+), 35 deletions(-) diff --git a/apiserver/facades/client/application/deployrepository.go b/apiserver/facades/client/application/deployrepository.go index 1d0416407c3..fe2b9226dbb 100644 --- a/apiserver/facades/client/application/deployrepository.go +++ b/apiserver/facades/client/application/deployrepository.go @@ -171,9 +171,12 @@ 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, - // TODO (stickupkid): Fill this in correctly when we have the - // charmhub information. - DownloadInfo: &applicationcharm.DownloadInfo{}, + // We always have download info for a charm from the charmhub store. + DownloadInfo: &applicationcharm.DownloadInfo{ + CharmhubIdentifier: dt.downloadInfo.CharmhubIdentifier, + DownloadURL: dt.downloadInfo.DownloadURL, + DownloadSize: dt.downloadInfo.DownloadSize, + }, }, unitArgs...) } @@ -282,6 +285,7 @@ type deployTemplate struct { storage map[string]storage.Directive pendingResourceUploads []*params.PendingResourceUpload resolvedResources []resource.Resource + downloadInfo corecharm.DownloadInfo } type validatorConfig struct { @@ -400,24 +404,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 { @@ -428,14 +431,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)) @@ -893,12 +898,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)) @@ -907,17 +919,17 @@ 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 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) 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 @@ -927,7 +939,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, @@ -937,19 +949,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: resolvedData.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: resolvedData.DownloadInfo, + }, nil } func (v *deployFromRepositoryValidator) appCharmSettings(appName string, trust bool, chCfg *charm.Config, configYAML string) (*config.Config, charm.Settings, error) { diff --git a/core/charm/repository.go b/core/charm/repository.go index af205eb27d0..87026216b03 100644 --- a/core/charm/repository.go +++ b/core/charm/repository.go @@ -107,4 +107,24 @@ type ResolvedDataForDeploy struct { // Resources is a map of resource names to their current repository revision // based on the supplied origin Resources map[string]charmresource.Resource + + // 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 +} + +// 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 } diff --git a/internal/charm/repository/charmhub.go b/internal/charm/repository/charmhub.go index e2d6de213bc..a67b99c6832 100644 --- a/internal/charm/repository/charmhub.go +++ b/internal/charm/repository/charmhub.go @@ -104,6 +104,11 @@ func (c *CharmHubRepository) ResolveForDeploy(ctx context.Context, arg corecharm URL: resultURL, EssentialMetadata: essMeta, Resources: resourceResults, + DownloadInfo: corecharm.DownloadInfo{ + CharmhubIdentifier: resp.Entity.ID, + DownloadURL: resp.Entity.Download.URL, + DownloadSize: int64(resp.Entity.Download.Size), + }, } return thing, nil } diff --git a/internal/charm/repository/charmhub_test.go b/internal/charm/repository/charmhub_test.go index 2b5ffab00bb..95700371622 100644 --- a/internal/charm/repository/charmhub_test.go +++ b/internal/charm/repository/charmhub_test.go @@ -251,8 +251,13 @@ func (s *charmHubRepositorySuite) TestResolveForDeployWithRevisionSuccess(c *gc. expected := s.expectedCURL(curl, 16, arch.DefaultArchitecture) - c.Assert(obtainedData.URL, jc.DeepEquals, expected) - c.Assert(obtainedData.EssentialMetadata.ResolvedOrigin, jc.DeepEquals, expectedOrigin) + c.Check(obtainedData.URL, jc.DeepEquals, expected) + c.Check(obtainedData.EssentialMetadata.ResolvedOrigin, jc.DeepEquals, expectedOrigin) + c.Check(obtainedData.DownloadInfo, jc.DeepEquals, corecharm.DownloadInfo{ + CharmhubIdentifier: "charmCHARMcharmCHARMcharmCHARM01", + DownloadURL: "http://example.com/wordpress-42", + DownloadSize: 42, + }) } func (s *charmHubRepositorySuite) TestResolveForDeploySuccessChooseBase(c *gc.C) { @@ -297,13 +302,19 @@ func (s *charmHubRepositorySuite) TestResolveForDeploySuccessChooseBase(c *gc.C) expected := s.expectedCURL(curl, 16, arch.DefaultArchitecture) - c.Assert(obtainedData.URL, jc.DeepEquals, expected) - c.Assert(obtainedData.EssentialMetadata.ResolvedOrigin, jc.DeepEquals, expectedOrigin) - c.Assert(obtainedData.Resources, gc.HasLen, 1) + c.Check(obtainedData.URL, jc.DeepEquals, expected) + c.Check(obtainedData.EssentialMetadata.ResolvedOrigin, jc.DeepEquals, expectedOrigin) + c.Check(obtainedData.Resources, gc.HasLen, 1) + c.Check(obtainedData.DownloadInfo, jc.DeepEquals, corecharm.DownloadInfo{ + CharmhubIdentifier: "charmCHARMcharmCHARMcharmCHARM01", + DownloadURL: "http://example.com/wordpress-42", + DownloadSize: 42, + }) + foundResource := obtainedData.Resources["wal-e"] - c.Assert(foundResource.Name, gc.Equals, "wal-e") - c.Assert(foundResource.Path, gc.Equals, "wal-e.snap") - c.Assert(foundResource.Revision, gc.Equals, 5) + c.Check(foundResource.Name, gc.Equals, "wal-e") + c.Check(foundResource.Path, gc.Equals, "wal-e.snap") + c.Check(foundResource.Revision, gc.Equals, 5) } func (s *charmHubRepositorySuite) TestResolveWithBundles(c *gc.C) { defer s.setupMocks(c).Finish() @@ -447,7 +458,7 @@ func (s *charmHubRepositorySuite) TestDownload(c *gc.C) { }, } - resolvedURL, err := url.Parse("ch:amd64/focal/wordpress-42") + resolvedURL, err := url.Parse("http://example.com/wordpress-42") c.Assert(err, jc.ErrorIsNil) s.expectCharmRefreshInstallOneFromChannel(c, hash) @@ -501,7 +512,7 @@ func (s *charmHubRepositorySuite) TestDownloadCharm(c *gc.C) { }, } - resolvedURL, err := url.Parse("ch:amd64/focal/wordpress-42") + resolvedURL, err := url.Parse("http://example.com/wordpress-42") c.Assert(err, jc.ErrorIsNil) resolvedArchive := new(charm.CharmArchive) @@ -557,7 +568,7 @@ func (s *charmHubRepositorySuite) TestGetDownloadURL(c *gc.C) { }, } - resolvedURL, err := url.Parse("ch:amd64/focal/wordpress-42") + resolvedURL, err := url.Parse("http://example.com/wordpress-42") c.Assert(err, jc.ErrorIsNil) s.expectCharmRefreshInstallOneFromChannel(c, hash) @@ -813,7 +824,7 @@ func (s *charmHubRepositorySuite) expectCharmRefresh(c *gc.C, cfg charmhub.Refre HashSHA256: hash, HashSHA384: "SHA384 hash", Size: 42, - URL: "ch:amd64/focal/wordpress-42", + URL: "http://example.com/wordpress-42", }, // Bases: []transport.Base{ @@ -936,7 +947,7 @@ func (s *charmHubRepositorySuite) expectCharmRefreshFullWithResources(c *gc.C, c HashSHA256: "SHA256 hash", HashSHA384: "SHA384 hash", Size: 42, - URL: "ch:amd64/focal/wordpress-42", + URL: "http://example.com/wordpress-42", }, // Bases: []transport.Base{ From b69cba0d7360a962d1daa64ccb4233c14b2822e8 Mon Sep 17 00:00:00 2001 From: Simon Richardson Date: Mon, 2 Dec 2024 15:08:29 +0000 Subject: [PATCH 3/7] feat: download provenance In order to know where the download info comes from, add a provenance to track it. --- .../facades/client/application/deploy.go | 4 +- .../client/application/deployrepository.go | 1 + apiserver/facades/client/charms/client.go | 4 +- apiserver/objects.go | 6 +- domain/application/charm/types.go | 38 ++++++++++ domain/application/errors/errors.go | 8 +++ domain/application/modelmigration/import.go | 17 +++-- .../application/modelmigration/import_test.go | 4 ++ domain/application/service/application.go | 9 ++- .../application/service/application_test.go | 10 +++ domain/application/service/charm.go | 9 ++- domain/application/service/charm_test.go | 1 + domain/application/service/service_test.go | 1 + domain/application/service_test.go | 34 +++++---- domain/application/state/application_test.go | 1 + domain/application/state/charm.go | 1 + domain/application/state/charm_test.go | 1 + domain/application/state/state.go | 69 +++++++++++++++---- domain/application/state/types.go | 10 +-- domain/application/watcher_test.go | 12 +++- domain/schema/model/sql/0015-charm.sql | 24 +++++++ internal/bootstrap/controller.go | 1 + internal/bootstrap/deployer.go | 6 +- internal/testing/factory/factory.go | 4 +- 24 files changed, 226 insertions(+), 49 deletions(-) diff --git a/apiserver/facades/client/application/deploy.go b/apiserver/facades/client/application/deploy.go index b12c51c28f9..d5eca031919 100644 --- a/apiserver/facades/client/application/deploy.go +++ b/apiserver/facades/client/application/deploy.go @@ -172,7 +172,9 @@ func DeployApplication( Storage: args.Storage, // TODO (stickupkid): Fill this in correctly when we have the // charmhub information. - DownloadInfo: &applicationcharm.DownloadInfo{}, + DownloadInfo: &applicationcharm.DownloadInfo{ + DownloadProvenance: applicationcharm.ProvenanceDownload, + }, }, unitArgs...) if err != nil { return nil, errors.Trace(err) diff --git a/apiserver/facades/client/application/deployrepository.go b/apiserver/facades/client/application/deployrepository.go index fe2b9226dbb..867766bf52e 100644 --- a/apiserver/facades/client/application/deployrepository.go +++ b/apiserver/facades/client/application/deployrepository.go @@ -173,6 +173,7 @@ func (api *DeployFromRepositoryAPI) DeployFromRepository(ctx context.Context, ar Storage: dt.storage, // We always have download info for a charm from the charmhub store. DownloadInfo: &applicationcharm.DownloadInfo{ + DownloadProvenance: applicationcharm.ProvenanceDownload, CharmhubIdentifier: dt.downloadInfo.CharmhubIdentifier, DownloadURL: dt.downloadInfo.DownloadURL, DownloadSize: dt.downloadInfo.DownloadSize, diff --git a/apiserver/facades/client/charms/client.go b/apiserver/facades/client/charms/client.go index 558f4a69362..a96e7319938 100644 --- a/apiserver/facades/client/charms/client.go +++ b/apiserver/facades/client/charms/client.go @@ -313,7 +313,9 @@ func (a *API) queueAsyncCharmDownload(ctx context.Context, args params.AddCharmW Hash: metaRes.ResolvedOrigin.Hash, // TODO (stickupkid): Fill this information in from the essential // metadata. - DownloadInfo: &applicationcharm.DownloadInfo{}, + DownloadInfo: &applicationcharm.DownloadInfo{ + DownloadProvenance: applicationcharm.ProvenanceDownload, + }, }); err != nil && !errors.Is(err, applicationerrors.CharmAlreadyExists) { return corecharm.Origin{}, errors.Annotatef(err, "setting charm %q", args.URL) } diff --git a/apiserver/objects.go b/apiserver/objects.go index 941093a232d..bd924cebb9e 100644 --- a/apiserver/objects.go +++ b/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{ @@ -248,7 +250,9 @@ func (h *objectsCharmHTTPHandler) processPut(r *http.Request, st *state.State, c // 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{}, + DownloadInfo: &applicationcharm.DownloadInfo{ + DownloadProvenance: provenance, + }, }); err != nil { return nil, errors.Trace(err) } diff --git a/domain/application/charm/types.go b/domain/application/charm/types.go index 15a4bfe67d4..f2cd21a06c0 100644 --- a/domain/application/charm/types.go +++ b/domain/application/charm/types.go @@ -153,8 +153,27 @@ type CharmLocator struct { Architecture architecture.Architecture } +// DownloadProvenance represents the provenance of a charm download. +// Ideally this would be called origin, but that's already used for an origin +// of a charm. +type DownloadProvenance string + +const ( + // ProvenanceDownload represents a charm download from the charmhub. + ProvenanceDownload DownloadProvenance = "download" + // ProvenanceUpload represents a charm download from an upload. + ProvenanceUpload DownloadProvenance = "upload" + // ProvenanceMigration represents a charm download from a migration. + ProvenanceMigration DownloadProvenance = "migration" + // ProvenanceBootstrap represents a charm placement during bootstrap. + ProvenanceBootstrap DownloadProvenance = "bootstrap" +) + // DownloadInfo holds the information needed to download a charmhub charm. type DownloadInfo struct { + // DownloadProvenance is the provenance of the download. + DownloadProvenance DownloadProvenance + // CharmhubIdentifier is the instance ID of the charm in relation to // charmhub. CharmhubIdentifier string @@ -167,6 +186,25 @@ type DownloadInfo struct { DownloadSize int64 } +// Validate validates the download information. +func (d DownloadInfo) Validate() error { + if d.DownloadProvenance == "" { + return applicationerrors.CharmDownloadProvenanceNotValid + } + + // We don't validate the download information if it's not a download. + if d.DownloadProvenance != ProvenanceDownload { + return nil + } + + // Download URL is required for a download. + if d.DownloadURL == "" { + return applicationerrors.CharmDownloadURLNotValid + } + + return nil +} + // Metadata represents the metadata of a charm from the perspective of the // service. This is the golden source of charm metadata. If the charm changes // at the wire format level, we should be able to map it to this struct. diff --git a/domain/application/errors/errors.go b/domain/application/errors/errors.go index 77d4c5f12ee..08b91e92fb9 100644 --- a/domain/application/errors/errors.go +++ b/domain/application/errors/errors.go @@ -182,4 +182,12 @@ const ( // CharmDownloadInfoNotFound describes an error that occurs when the charm // download info is not found. CharmDownloadInfoNotFound = errors.ConstError("charm download info not found") + + // CharmDownloadURLNotValid describes an error that occurs when the charm + // download URL is not valid. + CharmDownloadURLNotValid = errors.ConstError("charm download URL not valid") + + // CharmDownloadProvenanceNotValid describes an error that occurs when the + // charm download provenance is not valid. + CharmDownloadProvenanceNotValid = errors.ConstError("charm download provenance not valid") ) diff --git a/domain/application/modelmigration/import.go b/domain/application/modelmigration/import.go index 0ff2060fe41..fe872dccff0 100644 --- a/domain/application/modelmigration/import.go +++ b/domain/application/modelmigration/import.go @@ -162,12 +162,17 @@ func (i *importOperation) Execute(ctx context.Context, model description.Model) err = i.service.ImportApplication( ctx, app.Name(), charm, *origin, service.AddApplicationArgs{ ReferenceName: chURL.Name, - // TODO (stickupkid): When we're importing a charm during a - // migration, we should fill this in with the correct value. - // If not, we should indicate that the charm can not be - // downloaded without a new request to the charm store to - // fetch the charm. - DownloadInfo: &applicationcharm.DownloadInfo{}, + // When importing a charm, we don't have all the information + // about the charm. We could call charmhub store directly, but + // that has the potential to block a migration if the charmhub + // store is down. If we require that information, then it's + // possible to fill this missing information in the charmhub + // store using the charmhub identifier. + // If the controllers do not have the same charmhub url, then + // all bets are off. + DownloadInfo: &applicationcharm.DownloadInfo{ + DownloadProvenance: applicationcharm.ProvenanceMigration, + }, }, unitArgs..., ) if err != nil { diff --git a/domain/application/modelmigration/import_test.go b/domain/application/modelmigration/import_test.go index f060b095b91..2fb6892d997 100644 --- a/domain/application/modelmigration/import_test.go +++ b/domain/application/modelmigration/import_test.go @@ -18,6 +18,7 @@ import ( corecharm "github.com/juju/juju/core/charm" "github.com/juju/juju/core/network" + "github.com/juju/juju/domain/application/charm" "github.com/juju/juju/domain/application/service" internalcharm "github.com/juju/juju/internal/charm" "github.com/juju/juju/internal/charm/assumes" @@ -133,6 +134,9 @@ func (s *importSuite) TestApplicationImportWithMinimalCharm(c *gc.C) { }, service.AddApplicationArgs{ ReferenceName: "prometheus", + DownloadInfo: &charm.DownloadInfo{ + DownloadProvenance: charm.ProvenanceMigration, + }, }, []service.ImportUnitArg{{ UnitName: "prometheus/0", diff --git a/domain/application/service/application.go b/domain/application/service/application.go index b64b59d1605..7d8369ea9da 100644 --- a/domain/application/service/application.go +++ b/domain/application/service/application.go @@ -310,8 +310,13 @@ func validateCreateApplicationParams( } // If the origin is from charmhub, then we require the download info. - if origin.Source == corecharm.CharmHub && downloadInfo == nil { - return applicationerrors.CharmDownloadInfoNotFound + if origin.Source == corecharm.CharmHub { + if downloadInfo == nil { + return applicationerrors.CharmDownloadInfoNotFound + } + if err := downloadInfo.Validate(); err != nil { + return fmt.Errorf("download info: %w", err) + } } // Validate the origin of the charm. diff --git a/domain/application/service/application_test.go b/domain/application/service/application_test.go index 6579f923f43..40668ab1ab6 100644 --- a/domain/application/service/application_test.go +++ b/domain/application/service/application_test.go @@ -92,6 +92,7 @@ func (s *applicationServiceSuite) TestCreateApplication(c *gc.C) { app := application.AddApplicationArg{ Charm: ch, CharmDownloadInfo: &domaincharm.DownloadInfo{ + DownloadProvenance: domaincharm.ProvenanceDownload, CharmhubIdentifier: "foo", DownloadURL: "https://example.com/foo", DownloadSize: 42, @@ -131,6 +132,7 @@ func (s *applicationServiceSuite) TestCreateApplication(c *gc.C) { }, AddApplicationArgs{ ReferenceName: "ubuntu", DownloadInfo: &domaincharm.DownloadInfo{ + DownloadProvenance: domaincharm.ProvenanceDownload, CharmhubIdentifier: "foo", DownloadURL: "https://example.com/foo", DownloadSize: 42, @@ -241,6 +243,7 @@ func (s *applicationServiceSuite) TestCreateApplicationWithNoArchitecture(c *gc. }, AddApplicationArgs{ ReferenceName: "foo", DownloadInfo: &domaincharm.DownloadInfo{ + DownloadProvenance: domaincharm.ProvenanceDownload, CharmhubIdentifier: "foo", DownloadURL: "https://example.com/foo", DownloadSize: 42, @@ -276,6 +279,7 @@ func (s *applicationServiceSuite) TestCreateApplicationError(c *gc.C) { }, AddApplicationArgs{ ReferenceName: "foo", DownloadInfo: &domaincharm.DownloadInfo{ + DownloadProvenance: domaincharm.ProvenanceDownload, CharmhubIdentifier: "foo", DownloadURL: "https://example.com/foo", DownloadSize: 42, @@ -443,6 +447,7 @@ func (s *applicationServiceSuite) TestCreateWithStorageBlockDefaultSource(c *gc. app := application.AddApplicationArg{ Charm: ch, CharmDownloadInfo: &domaincharm.DownloadInfo{ + DownloadProvenance: domaincharm.ProvenanceDownload, CharmhubIdentifier: "foo", DownloadURL: "https://example.com/foo", DownloadSize: 42, @@ -489,6 +494,7 @@ func (s *applicationServiceSuite) TestCreateWithStorageBlockDefaultSource(c *gc. }, AddApplicationArgs{ ReferenceName: "foo", DownloadInfo: &domaincharm.DownloadInfo{ + DownloadProvenance: domaincharm.ProvenanceDownload, CharmhubIdentifier: "foo", DownloadURL: "https://example.com/foo", DownloadSize: 42, @@ -552,6 +558,7 @@ func (s *applicationServiceSuite) TestCreateWithStorageFilesystem(c *gc.C) { app := application.AddApplicationArg{ Charm: ch, CharmDownloadInfo: &domaincharm.DownloadInfo{ + DownloadProvenance: domaincharm.ProvenanceDownload, CharmhubIdentifier: "foo", DownloadURL: "https://example.com/foo", DownloadSize: 42, @@ -598,6 +605,7 @@ func (s *applicationServiceSuite) TestCreateWithStorageFilesystem(c *gc.C) { }, AddApplicationArgs{ ReferenceName: "foo", DownloadInfo: &domaincharm.DownloadInfo{ + DownloadProvenance: domaincharm.ProvenanceDownload, CharmhubIdentifier: "foo", DownloadURL: "https://example.com/foo", DownloadSize: 42, @@ -658,6 +666,7 @@ func (s *applicationServiceSuite) TestCreateWithStorageFilesystemDefaultSource(c app := application.AddApplicationArg{ Charm: ch, CharmDownloadInfo: &domaincharm.DownloadInfo{ + DownloadProvenance: domaincharm.ProvenanceDownload, CharmhubIdentifier: "foo", DownloadURL: "https://example.com/foo", DownloadSize: 42, @@ -704,6 +713,7 @@ func (s *applicationServiceSuite) TestCreateWithStorageFilesystemDefaultSource(c }, AddApplicationArgs{ ReferenceName: "foo", DownloadInfo: &domaincharm.DownloadInfo{ + DownloadProvenance: domaincharm.ProvenanceDownload, CharmhubIdentifier: "foo", DownloadURL: "https://example.com/foo", DownloadSize: 42, diff --git a/domain/application/service/charm.go b/domain/application/service/charm.go index 1f0d839c4eb..929f97923a8 100644 --- a/domain/application/service/charm.go +++ b/domain/application/service/charm.go @@ -517,8 +517,13 @@ func (s *Service) SetCharm(ctx context.Context, args charm.SetCharmArgs) (corech } // If the origin is from charmhub, then we require the download info. - if args.Source == corecharm.CharmHub && args.DownloadInfo == nil { - return "", nil, applicationerrors.CharmDownloadInfoNotFound + if args.Source == corecharm.CharmHub { + if args.DownloadInfo == nil { + return "", nil, applicationerrors.CharmDownloadInfoNotFound + } + if err := args.DownloadInfo.Validate(); err != nil { + return "", nil, fmt.Errorf("download info: %w", err) + } } source, err := encodeCharmSource(args.Source) diff --git a/domain/application/service/charm_test.go b/domain/application/service/charm_test.go index bbe63a71d53..6c7eb2d5639 100644 --- a/domain/application/service/charm_test.go +++ b/domain/application/service/charm_test.go @@ -536,6 +536,7 @@ func (s *charmServiceSuite) TestSetCharmCharmhub(c *gc.C) { }}}).MinTimes(1) downloadInfo := &charm.DownloadInfo{ + DownloadProvenance: charm.ProvenanceDownload, CharmhubIdentifier: "foo", DownloadURL: "http://example.com/foo", DownloadSize: 42, diff --git a/domain/application/service/service_test.go b/domain/application/service/service_test.go index 9836f9b3004..e46fbd2c3bc 100644 --- a/domain/application/service/service_test.go +++ b/domain/application/service/service_test.go @@ -77,6 +77,7 @@ func (s *migrationServiceSuite) TestImportApplication(c *gc.C) { Architecture: architecture.ARM64, } downloadInfo := &domaincharm.DownloadInfo{ + DownloadProvenance: domaincharm.ProvenanceDownload, DownloadURL: "http://example.com", DownloadSize: 24, CharmhubIdentifier: "foobar", diff --git a/domain/application/service_test.go b/domain/application/service_test.go index eb2234e39df..150fa042add 100644 --- a/domain/application/service_test.go +++ b/domain/application/service_test.go @@ -23,6 +23,7 @@ import ( jujuversion "github.com/juju/juju/core/version" "github.com/juju/juju/domain" "github.com/juju/juju/domain/application" + applicationcharm "github.com/juju/juju/domain/application/charm" applicationerrors "github.com/juju/juju/domain/application/errors" "github.com/juju/juju/domain/application/resource" "github.com/juju/juju/domain/application/service" @@ -80,21 +81,6 @@ func (s *serviceSuite) SetUpTest(c *gc.C) { c.Assert(err, jc.ErrorIsNil) } -func (s *serviceSuite) createApplication(c *gc.C, name string, units ...service.AddUnitArg) coreapplication.ID { - appID, err := s.svc.CreateApplication(context.Background(), name, &stubCharm{}, corecharm.Origin{ - Source: corecharm.CharmHub, - Platform: corecharm.Platform{ - Channel: "24.04", - OS: "ubuntu", - Architecture: "amd64", - }, - }, service.AddApplicationArgs{ - ReferenceName: name, - }, units...) - c.Assert(err, jc.ErrorIsNil) - return appID -} - func (s *serviceSuite) TestGetApplicationLife(c *gc.C) { s.createApplication(c, "foo") @@ -856,3 +842,21 @@ func (s *serviceSuite) TestCAASUnitTerminatingUnitNumGreaterThanDesired(c *gc.C) c.Assert(err, jc.ErrorIsNil) c.Assert(willRestart, jc.IsFalse) } +func (s *serviceSuite) createApplication(c *gc.C, name string, units ...service.AddUnitArg) coreapplication.ID { + appID, err := s.svc.CreateApplication(context.Background(), name, &stubCharm{}, corecharm.Origin{ + Source: corecharm.CharmHub, + Platform: corecharm.Platform{ + Channel: "24.04", + OS: "ubuntu", + Architecture: "amd64", + }, + }, service.AddApplicationArgs{ + ReferenceName: name, + DownloadInfo: &applicationcharm.DownloadInfo{ + DownloadProvenance: applicationcharm.ProvenanceDownload, + DownloadURL: "https://example.com", + }, + }, units...) + c.Assert(err, jc.ErrorIsNil) + return appID +} diff --git a/domain/application/state/application_test.go b/domain/application/state/application_test.go index 367cead9287..6ded89cd56d 100644 --- a/domain/application/state/application_test.go +++ b/domain/application/state/application_test.go @@ -94,6 +94,7 @@ func (s *applicationStateSuite) TestCreateApplication(c *gc.C) { Architecture: architecture.ARM64, }, CharmDownloadInfo: &charm.DownloadInfo{ + DownloadProvenance: charm.ProvenanceDownload, CharmhubIdentifier: "ident-1", DownloadURL: "http://example.com/charm", DownloadSize: 666, diff --git a/domain/application/state/charm.go b/domain/application/state/charm.go index 7ebfda5164f..bb1f0be5df8 100644 --- a/domain/application/state/charm.go +++ b/domain/application/state/charm.go @@ -704,6 +704,7 @@ var tablesToDeleteFrom = []string{ "charm_container_mount", "charm_container", "charm_device", + "charm_download_info", "charm_extra_binding", "charm_hash", "charm_manifest_base", diff --git a/domain/application/state/charm_test.go b/domain/application/state/charm_test.go index e2eeeb357d6..aa3f0b5fa29 100644 --- a/domain/application/state/charm_test.go +++ b/domain/application/state/charm_test.go @@ -1196,6 +1196,7 @@ func (s *charmStateSuite) TestSetCharmDownloadInfoForCharmhub(c *gc.C) { st := NewState(s.TxnRunnerFactory(), clock.WallClock, loggertesting.WrapCheckLog(c)) info := &charm.DownloadInfo{ + DownloadProvenance: charm.ProvenanceDownload, CharmhubIdentifier: "ident-1", DownloadURL: "https://example.com/charmhub/ident-1", DownloadSize: 1234, diff --git a/domain/application/state/state.go b/domain/application/state/state.go index c07d4887027..1f612c6e985 100644 --- a/domain/application/state/state.go +++ b/domain/application/state/state.go @@ -176,12 +176,12 @@ func (s *State) setCharmState( ) error { sourceID, err := encodeCharmSource(ch.Source) if err != nil { - return fmt.Errorf("failed to encode charm source: %w", err) + return fmt.Errorf("encoding charm source: %w", err) } architectureID, err := encodeArchitecture(ch.Architecture) if err != nil { - return fmt.Errorf("failed to encode charm architecture: %w", err) + return fmt.Errorf("encoding charm architecture: %w", err) } nullableArchitectureID := sql.NullInt64{} @@ -237,11 +237,17 @@ func (s *State) setCharmDownloadInfo(ctx context.Context, tx *sqlair.TX, id core return nil } + provenance, err := encodeProvenance(downloadInfo.DownloadProvenance) + if err != nil { + return fmt.Errorf("encoding charm provenance: %w", err) + } + downloadInfoState := setCharmDownloadInfo{ - CharmUUID: id.String(), - CharmhubIdentifier: downloadInfo.CharmhubIdentifier, - DownloadURL: downloadInfo.DownloadURL, - DownloadSize: downloadInfo.DownloadSize, + CharmUUID: id.String(), + DownloadProvenanceID: provenance, + CharmhubIdentifier: downloadInfo.CharmhubIdentifier, + DownloadURL: downloadInfo.DownloadURL, + DownloadSize: downloadInfo.DownloadSize, } query := `INSERT INTO charm_download_info (*) VALUES ($setCharmDownloadInfo.*);` @@ -266,7 +272,7 @@ func (s *State) setCharmMetadata( ) error { encodedMetadata, err := encodeMetadata(id, metadata, lxdProfile) if err != nil { - return fmt.Errorf("failed to encode charm metadata: %w", err) + return fmt.Errorf("encoding charm metadata: %w", err) } query := `INSERT INTO charm_metadata (*) VALUES ($setCharmMetadata.*);` @@ -342,7 +348,7 @@ func (s *State) setCharmTerms(ctx context.Context, tx *sqlair.TX, id corecharm.I func (s *State) setCharmRelations(ctx context.Context, tx *sqlair.TX, id corecharm.ID, metadata charm.Metadata) error { encodedRelations, err := encodeRelations(id, metadata) if err != nil { - return fmt.Errorf("failed to encode charm relations: %w", err) + return fmt.Errorf("encoding charm relations: %w", err) } // If there are no relations, we don't need to do anything. @@ -392,7 +398,7 @@ func (s *State) setCharmStorage(ctx context.Context, tx *sqlair.TX, id corecharm encodedStorage, encodedProperties, err := encodeStorage(id, storage) if err != nil { - return fmt.Errorf("failed to encode charm storage: %w", err) + return fmt.Errorf("encoding charm storage: %w", err) } storageQuery := `INSERT INTO charm_storage (*) VALUES ($setCharmStorage.*);` @@ -466,7 +472,7 @@ func (s *State) setCharmResources(ctx context.Context, tx *sqlair.TX, id corecha encodedResources, err := encodeResources(id, resources) if err != nil { - return fmt.Errorf("failed to encode charm resources: %w", err) + return fmt.Errorf("encoding charm resources: %w", err) } query := `INSERT INTO charm_resource (*) VALUES ($setCharmResource.*);` @@ -490,7 +496,7 @@ func (s *State) setCharmContainers(ctx context.Context, tx *sqlair.TX, id corech encodedContainers, encodedMounts, err := encodeContainers(id, containers) if err != nil { - return fmt.Errorf("failed to encode charm containers: %w", err) + return fmt.Errorf("encoding charm containers: %w", err) } containerQuery := `INSERT INTO charm_container (*) VALUES ($setCharmContainer.*);` @@ -546,7 +552,7 @@ func (s *State) setCharmConfig(ctx context.Context, tx *sqlair.TX, id corecharm. encodedConfig, err := encodeConfig(id, config) if err != nil { - return fmt.Errorf("failed to encode charm config: %w", err) + return fmt.Errorf("encoding charm config: %w", err) } query := `INSERT INTO charm_config (*) VALUES ($setCharmConfig.*);` @@ -569,7 +575,7 @@ func (s *State) setCharmManifest(ctx context.Context, tx *sqlair.TX, id corechar encodedManifest, err := encodeManifest(id, manifest) if err != nil { - return fmt.Errorf("failed to encode charm manifest: %w", err) + return fmt.Errorf("encoding charm manifest: %w", err) } query := `INSERT INTO charm_manifest_base (*) VALUES ($setCharmManifest.*);` @@ -680,6 +686,7 @@ func (s *State) getCharmDownloadInfo(ctx context.Context, tx *sqlair.TX, ident c SELECT &charmDownloadInfo.* FROM charm AS c JOIN charm_download_info AS cdi ON c.uuid = cdi.charm_uuid +JOIN charm_provenance AS cp ON cp.id = cdi.provenance_id WHERE c.uuid = $charmID.uuid AND c.source_id = 1; ` @@ -694,7 +701,13 @@ AND c.source_id = 1; return charm.DownloadInfo{}, fmt.Errorf("getting charm download info: %w", err) } + provenance, err := decodeProvenance(downloadInfo.DownloadProvenance) + if err != nil { + return charm.DownloadInfo{}, fmt.Errorf("decoding charm provenance: %w", err) + } + return charm.DownloadInfo{ + DownloadProvenance: provenance, CharmhubIdentifier: downloadInfo.CharmhubIdentifier, DownloadURL: downloadInfo.DownloadURL, DownloadSize: downloadInfo.DownloadSize, @@ -1348,3 +1361,33 @@ func encodeCharmSource(source charm.CharmSource) (int, error) { return 0, internalerrors.Errorf("unsupported source type: %s", source) } } + +func encodeProvenance(provenance charm.DownloadProvenance) (int, error) { + switch provenance { + case charm.ProvenanceDownload: + return 0, nil + case charm.ProvenanceMigration: + return 1, nil + case charm.ProvenanceUpload: + return 2, nil + case charm.ProvenanceBootstrap: + return 3, nil + default: + return 0, internalerrors.Errorf("unsupported provenance type: %s", provenance) + } +} + +func decodeProvenance(provenance string) (charm.DownloadProvenance, error) { + switch provenance { + case "download": + return charm.ProvenanceDownload, nil + case "migration": + return charm.ProvenanceMigration, nil + case "upload": + return charm.ProvenanceUpload, nil + case "bootstrap": + return charm.ProvenanceBootstrap, nil + default: + return "", fmt.Errorf("unknown provenance: %s", provenance) + } +} diff --git a/domain/application/state/types.go b/domain/application/state/types.go index 5a9c3d79e59..79fda9100e3 100644 --- a/domain/application/state/types.go +++ b/domain/application/state/types.go @@ -255,6 +255,7 @@ type setCharmState struct { // charmDownloadInfo is used to get the download info of a charm. type charmDownloadInfo struct { + DownloadProvenance string `db:"name"` CharmhubIdentifier string `db:"charmhub_identifier"` DownloadURL string `db:"download_url"` DownloadSize int64 `db:"download_size"` @@ -262,10 +263,11 @@ type charmDownloadInfo struct { // setCharmDownloadInfo is used to set the download info of a charm. type setCharmDownloadInfo struct { - CharmUUID string `db:"charm_uuid"` - CharmhubIdentifier string `db:"charmhub_identifier"` - DownloadURL string `db:"download_url"` - DownloadSize int64 `db:"download_size"` + CharmUUID string `db:"charm_uuid"` + DownloadProvenanceID int `db:"provenance_id"` + CharmhubIdentifier string `db:"charmhub_identifier"` + DownloadURL string `db:"download_url"` + DownloadSize int64 `db:"download_size"` } // charmMetadata is used to get the metadata of a charm. diff --git a/domain/application/watcher_test.go b/domain/application/watcher_test.go index 032a64fc3d4..629a5221820 100644 --- a/domain/application/watcher_test.go +++ b/domain/application/watcher_test.go @@ -74,6 +74,10 @@ func (s *watcherSuite) TestWatchCharm(c *gc.C) { ReferenceName: "test", Revision: 1, Architecture: arch.AMD64, + DownloadInfo: &charm.DownloadInfo{ + DownloadProvenance: charm.ProvenanceDownload, + DownloadURL: "http://example.com", + }, }) c.Assert(err, jc.ErrorIsNil) }, func(w watchertest.WatcherC[[]string]) { @@ -476,9 +480,9 @@ func (s *watcherSuite) createApplication(c *gc.C, svc *service.WatchableService, return s.createApplicationWithCharmAndStoragePath(c, svc, name, &stubCharm{}, "", units...) } -func (s *watcherSuite) createApplicationWithCharmAndStoragePath(c *gc.C, svc *service.WatchableService, name string, charm internalcharm.Charm, storagePath string, units ...service.AddUnitArg) coreapplication.ID { +func (s *watcherSuite) createApplicationWithCharmAndStoragePath(c *gc.C, svc *service.WatchableService, name string, ch internalcharm.Charm, storagePath string, units ...service.AddUnitArg) coreapplication.ID { ctx := context.Background() - appID, err := svc.CreateApplication(ctx, name, charm, corecharm.Origin{ + appID, err := svc.CreateApplication(ctx, name, ch, corecharm.Origin{ Source: corecharm.CharmHub, Platform: corecharm.Platform{ Channel: "24.04", @@ -488,6 +492,10 @@ func (s *watcherSuite) createApplicationWithCharmAndStoragePath(c *gc.C, svc *se }, service.AddApplicationArgs{ ReferenceName: name, CharmStoragePath: storagePath, + DownloadInfo: &charm.DownloadInfo{ + DownloadProvenance: charm.ProvenanceDownload, + DownloadURL: "http://example.com", + }, }, units...) c.Assert(err, jc.ErrorIsNil) return appID diff --git a/domain/schema/model/sql/0015-charm.sql b/domain/schema/model/sql/0015-charm.sql index 13a42ea96ab..0ea6724bb09 100644 --- a/domain/schema/model/sql/0015-charm.sql +++ b/domain/schema/model/sql/0015-charm.sql @@ -64,8 +64,32 @@ CREATE TABLE charm ( CREATE UNIQUE INDEX idx_charm_reference_name_revision ON charm (reference_name, revision); + +CREATE TABLE charm_provenance ( + id INT PRIMARY KEY, + name TEXT NOT NULL +); + +CREATE UNIQUE INDEX idx_charm_provenance_name +ON charm_provenance (name); + +-- The provenance of the charm. This is used to determine where the charm +-- came from, which can then determine if the download information is still +-- relevant. +INSERT INTO charm_provenance VALUES +(0, 'download'), +(1, 'migration'), +(2, 'upload'), +(3, 'bootstrap'); + CREATE TABLE charm_download_info ( charm_uuid TEXT NOT NULL PRIMARY KEY, + + -- The provenance_id is the origin from which the download information + -- was obtained. Ideally, we would have used origin, but that's already + -- taken and I don't want to confuse the two. + provenance_id INT NOT NULL, + -- charmhub_identifier is the identifier that charmhub uses to identify the -- charm. This is used to refresh the charm from charmhub. The -- reference_name can change but the charmhub_identifier will not. diff --git a/internal/bootstrap/controller.go b/internal/bootstrap/controller.go index e0ac95f97be..d89ea4aba97 100644 --- a/internal/bootstrap/controller.go +++ b/internal/bootstrap/controller.go @@ -40,6 +40,7 @@ func PopulateControllerCharm(ctx context.Context, deployer ControllerCharmDeploy if err != nil && !errors.Is(err, errors.NotFound) { return errors.Annotatef(err, "deploying local controller charm") } + // If the errors is not found locally, we'll try to download it from // charm hub. if errors.Is(err, errors.NotFound) { diff --git a/internal/bootstrap/deployer.go b/internal/bootstrap/deployer.go index b3a91a2b1be..f8241097300 100644 --- a/internal/bootstrap/deployer.go +++ b/internal/bootstrap/deployer.go @@ -393,7 +393,11 @@ func (b *baseDeployer) AddControllerApplication(ctx context.Context, curl string ch, origin, applicationservice.AddApplicationArgs{ ReferenceName: bootstrap.ControllerApplicationName, - DownloadInfo: &applicationcharm.DownloadInfo{}, + DownloadInfo: &applicationcharm.DownloadInfo{ + // We do have all the information we need to download the charm, + // we should fill this in with the correct information. + DownloadProvenance: applicationcharm.ProvenanceBootstrap, + }, }, applicationservice.AddUnitArg{UnitName: unitName}, ) diff --git a/internal/testing/factory/factory.go b/internal/testing/factory/factory.go index 779d1b2289f..b3c766e454e 100644 --- a/internal/testing/factory/factory.go +++ b/internal/testing/factory/factory.go @@ -506,7 +506,9 @@ func (factory *Factory) MakeApplicationReturningPassword(c *gc.C, params *Applic }, applicationservice.AddApplicationArgs{ ReferenceName: params.Name, Storage: directives, - DownloadInfo: &applicationcharm.DownloadInfo{}, + DownloadInfo: &applicationcharm.DownloadInfo{ + DownloadProvenance: applicationcharm.ProvenanceUpload, + }, }) } c.Assert(err, jc.ErrorIsNil) From 91f06f54b273d93e3df264630abc40d3cf56d913 Mon Sep 17 00:00:00 2001 From: Simon Richardson Date: Mon, 2 Dec 2024 16:51:17 +0000 Subject: [PATCH 4/7] feat: wireup download info in client facades The follow wires up the download info for charmhub downloads for client facades. --- .../facades/client/application/deploy.go | 23 ++++-- .../client/application/deployrepository.go | 9 ++- .../facades/client/application/service.go | 4 + .../client/application/services_mock_test.go | 39 +++++++++ apiserver/facades/client/charms/client.go | 17 ++-- core/charm/repository.go | 12 +-- domain/application/service/charm.go | 16 ++++ domain/application/service/charm_test.go | 19 +++++ .../application/service/package_mock_test.go | 39 +++++++++ domain/application/state/charm.go | 29 +++++++ domain/application/state/charm_test.go | 80 +++++++++++++++++++ internal/charm/repository/charmhub.go | 34 +++++--- internal/charm/repository/charmhub_test.go | 11 ++- 13 files changed, 296 insertions(+), 36 deletions(-) diff --git a/apiserver/facades/client/application/deploy.go b/apiserver/facades/client/application/deploy.go index d5eca031919..81fb33d3a3a 100644 --- a/apiserver/facades/client/application/deploy.go +++ b/apiserver/facades/client/application/deploy.go @@ -167,14 +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, - // TODO (stickupkid): Fill this in correctly when we have the - // charmhub information. - DownloadInfo: &applicationcharm.DownloadInfo{ - DownloadProvenance: applicationcharm.ProvenanceDownload, - }, + DownloadInfo: downloadInfo, }, unitArgs...) if err != nil { return nil, errors.Trace(err) diff --git a/apiserver/facades/client/application/deployrepository.go b/apiserver/facades/client/application/deployrepository.go index 867766bf52e..8e1d5a1c730 100644 --- a/apiserver/facades/client/application/deployrepository.go +++ b/apiserver/facades/client/application/deployrepository.go @@ -922,13 +922,14 @@ func (v *deployFromRepositoryValidator) getCharm(ctx context.Context, arg params if err != nil { 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 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 charmResult{}, errors.NotSupportedf("manual deploy of the controller charm") } @@ -956,7 +957,7 @@ func (v *deployFromRepositoryValidator) getCharm(ctx context.Context, arg params CharmURL: resolvedData.URL, Origin: resolvedOrigin, Charm: deployedCharm, - DownloadInfo: resolvedData.DownloadInfo, + DownloadInfo: essentialMetadata.DownloadInfo, }, nil } if !errors.Is(err, applicationerrors.CharmNotFound) { @@ -972,7 +973,7 @@ func (v *deployFromRepositoryValidator) getCharm(ctx context.Context, arg params CharmURL: resolvedData.URL, Origin: resolvedOrigin, Charm: resolvedCharm, - DownloadInfo: resolvedData.DownloadInfo, + DownloadInfo: essentialMetadata.DownloadInfo, }, nil } diff --git a/apiserver/facades/client/application/service.go b/apiserver/facades/client/application/service.go index 7e61053632b..07e77ae2ea7 100644 --- a/apiserver/facades/client/application/service.go +++ b/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. diff --git a/apiserver/facades/client/application/services_mock_test.go b/apiserver/facades/client/application/services_mock_test.go index d1cfea14344..997563079ff 100644 --- a/apiserver/facades/client/application/services_mock_test.go +++ b/apiserver/facades/client/application/services_mock_test.go @@ -1157,6 +1157,45 @@ func (c *MockApplicationServiceGetCharmConfigCall) DoAndReturn(f func(context.Co return c } +// GetCharmDownloadInfo mocks base method. +func (m *MockApplicationService) GetCharmDownloadInfo(arg0 context.Context, arg1 charm.ID) (*charm0.DownloadInfo, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetCharmDownloadInfo", arg0, arg1) + ret0, _ := ret[0].(*charm0.DownloadInfo) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetCharmDownloadInfo indicates an expected call of GetCharmDownloadInfo. +func (mr *MockApplicationServiceMockRecorder) GetCharmDownloadInfo(arg0, arg1 any) *MockApplicationServiceGetCharmDownloadInfoCall { + mr.mock.ctrl.T.Helper() + call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetCharmDownloadInfo", reflect.TypeOf((*MockApplicationService)(nil).GetCharmDownloadInfo), arg0, arg1) + return &MockApplicationServiceGetCharmDownloadInfoCall{Call: call} +} + +// MockApplicationServiceGetCharmDownloadInfoCall wrap *gomock.Call +type MockApplicationServiceGetCharmDownloadInfoCall struct { + *gomock.Call +} + +// Return rewrite *gomock.Call.Return +func (c *MockApplicationServiceGetCharmDownloadInfoCall) Return(arg0 *charm0.DownloadInfo, arg1 error) *MockApplicationServiceGetCharmDownloadInfoCall { + c.Call = c.Call.Return(arg0, arg1) + return c +} + +// Do rewrite *gomock.Call.Do +func (c *MockApplicationServiceGetCharmDownloadInfoCall) Do(f func(context.Context, charm.ID) (*charm0.DownloadInfo, error)) *MockApplicationServiceGetCharmDownloadInfoCall { + c.Call = c.Call.Do(f) + return c +} + +// DoAndReturn rewrite *gomock.Call.DoAndReturn +func (c *MockApplicationServiceGetCharmDownloadInfoCall) DoAndReturn(f func(context.Context, charm.ID) (*charm0.DownloadInfo, error)) *MockApplicationServiceGetCharmDownloadInfoCall { + c.Call = c.Call.DoAndReturn(f) + return c +} + // GetCharmID mocks base method. func (m *MockApplicationService) GetCharmID(arg0 context.Context, arg1 charm0.GetCharmArgs) (charm.ID, error) { m.ctrl.T.Helper() diff --git a/apiserver/facades/client/charms/client.go b/apiserver/facades/client/charms/client.go index a96e7319938..e86dc05c1e8 100644 --- a/apiserver/facades/client/charms/client.go +++ b/apiserver/facades/client/charms/client.go @@ -290,37 +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, - // TODO (stickupkid): Fill this information in from the essential - // metadata. + Hash: essentialMetadata.ResolvedOrigin.Hash, DownloadInfo: &applicationcharm.DownloadInfo{ DownloadProvenance: 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) { diff --git a/core/charm/repository.go b/core/charm/repository.go index 87026216b03..55f0baf7c6d 100644 --- a/core/charm/repository.go +++ b/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. @@ -107,12 +113,6 @@ type ResolvedDataForDeploy struct { // Resources is a map of resource names to their current repository revision // based on the supplied origin Resources map[string]charmresource.Resource - - // 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 } // DownloadInfo contains the information needed to download a charm from the diff --git a/domain/application/service/charm.go b/domain/application/service/charm.go index 929f97923a8..83b1c29f5c6 100644 --- a/domain/application/service/charm.go +++ b/domain/application/service/charm.go @@ -138,6 +138,10 @@ type CharmState interface { // The locator allows the reconstruction of the charm URL for the client // response. If no names are provided, then nothing is returned. ListCharmLocatorsByNames(ctx context.Context, names []string) ([]charm.CharmLocator, error) + + // GetCharmDownloadInfo returns the download info for the charm using the + // charm ID. + GetCharmDownloadInfo(ctx context.Context, id corecharm.ID) (*charm.DownloadInfo, error) } // CharmStore defines the interface for storing and retrieving charms archive blobs @@ -556,6 +560,9 @@ func (s *Service) SetCharm(ctx context.Context, args charm.SetCharmArgs) (corech // DeleteCharm removes the charm from the state. // Returns an error if the charm does not exist. func (s *Service) DeleteCharm(ctx context.Context, id corecharm.ID) error { + if err := id.Validate(); err != nil { + return fmt.Errorf("charm id: %w", err) + } return s.st.DeleteCharm(ctx, id) } @@ -570,6 +577,15 @@ func (s *Service) ListCharmLocators(ctx context.Context, names ...string) ([]cha return s.st.ListCharmLocatorsByNames(ctx, names) } +// GetCharmDownloadInfo returns the download info for the charm using the +// charm ID. +func (s *Service) GetCharmDownloadInfo(ctx context.Context, id corecharm.ID) (*charm.DownloadInfo, error) { + if err := id.Validate(); err != nil { + return nil, fmt.Errorf("charm id: %w", err) + } + return s.st.GetCharmDownloadInfo(ctx, id) +} + // WatchCharms returns a watcher that observes changes to charms. func (s *WatchableService) WatchCharms() (watcher.StringsWatcher, error) { return s.watcherFactory.NewUUIDsWatcher( diff --git a/domain/application/service/charm_test.go b/domain/application/service/charm_test.go index 6c7eb2d5639..592c2cc6626 100644 --- a/domain/application/service/charm_test.go +++ b/domain/application/service/charm_test.go @@ -1044,6 +1044,25 @@ func (s *charmServiceSuite) TestListCharmLocatorsWithoutName(c *gc.C) { c.Check(results, gc.DeepEquals, expected) } +func (s *charmServiceSuite) TestGetCharmDownloadInfo(c *gc.C) { + defer s.setupMocks(c).Finish() + + id := charmtesting.GenCharmID(c) + + expected := &charm.DownloadInfo{ + DownloadProvenance: charm.ProvenanceDownload, + CharmhubIdentifier: "foo", + DownloadURL: "http://example.com/foo", + DownloadSize: 42, + } + + s.state.EXPECT().GetCharmDownloadInfo(gomock.Any(), id).Return(expected, nil) + + result, err := s.service.GetCharmDownloadInfo(context.Background(), id) + c.Assert(err, jc.ErrorIsNil) + c.Check(result, gc.DeepEquals, expected) +} + type watchableServiceSuite struct { baseSuite diff --git a/domain/application/service/package_mock_test.go b/domain/application/service/package_mock_test.go index e7ec468af25..77e935efb0a 100644 --- a/domain/application/service/package_mock_test.go +++ b/domain/application/service/package_mock_test.go @@ -771,6 +771,45 @@ func (c *MockStateGetCharmConfigCall) DoAndReturn(f func(context.Context, charm. return c } +// GetCharmDownloadInfo mocks base method. +func (m *MockState) GetCharmDownloadInfo(arg0 context.Context, arg1 charm.ID) (*charm0.DownloadInfo, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetCharmDownloadInfo", arg0, arg1) + ret0, _ := ret[0].(*charm0.DownloadInfo) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetCharmDownloadInfo indicates an expected call of GetCharmDownloadInfo. +func (mr *MockStateMockRecorder) GetCharmDownloadInfo(arg0, arg1 any) *MockStateGetCharmDownloadInfoCall { + mr.mock.ctrl.T.Helper() + call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetCharmDownloadInfo", reflect.TypeOf((*MockState)(nil).GetCharmDownloadInfo), arg0, arg1) + return &MockStateGetCharmDownloadInfoCall{Call: call} +} + +// MockStateGetCharmDownloadInfoCall wrap *gomock.Call +type MockStateGetCharmDownloadInfoCall struct { + *gomock.Call +} + +// Return rewrite *gomock.Call.Return +func (c *MockStateGetCharmDownloadInfoCall) Return(arg0 *charm0.DownloadInfo, arg1 error) *MockStateGetCharmDownloadInfoCall { + c.Call = c.Call.Return(arg0, arg1) + return c +} + +// Do rewrite *gomock.Call.Do +func (c *MockStateGetCharmDownloadInfoCall) Do(f func(context.Context, charm.ID) (*charm0.DownloadInfo, error)) *MockStateGetCharmDownloadInfoCall { + c.Call = c.Call.Do(f) + return c +} + +// DoAndReturn rewrite *gomock.Call.DoAndReturn +func (c *MockStateGetCharmDownloadInfoCall) DoAndReturn(f func(context.Context, charm.ID) (*charm0.DownloadInfo, error)) *MockStateGetCharmDownloadInfoCall { + c.Call = c.Call.DoAndReturn(f) + return c +} + // GetCharmID mocks base method. func (m *MockState) GetCharmID(arg0 context.Context, arg1 string, arg2 int, arg3 charm0.CharmSource) (charm.ID, error) { m.ctrl.T.Helper() diff --git a/domain/application/state/charm.go b/domain/application/state/charm.go index bb1f0be5df8..3a6e2be75ac 100644 --- a/domain/application/state/charm.go +++ b/domain/application/state/charm.go @@ -5,6 +5,7 @@ package state import ( "context" + "database/sql" "fmt" "github.com/canonical/sqlair" @@ -676,6 +677,34 @@ WHERE name IN ($nameSelector[:]); return decodeCharmLocators(results) } +// GetCharmDownloadInfo returns the download info for the charm using the +// charm ID. +func (s *State) GetCharmDownloadInfo(ctx context.Context, id corecharm.ID) (*charm.DownloadInfo, error) { + db, err := s.DB() + if err != nil { + return nil, internalerrors.Capture(err) + } + + ident := charmID{UUID: id.String()} + + var downloadInfo *charm.DownloadInfo + if err := db.Txn(ctx, func(ctx context.Context, tx *sqlair.TX) error { + info, err := s.getCharmDownloadInfo(ctx, tx, ident) + if errors.Is(err, sql.ErrNoRows) { + return nil + } else if err != nil { + return internalerrors.Capture(err) + } + downloadInfo = &info + + return nil + }); err != nil { + return nil, internalerrors.Errorf("getting charm download info: %w", err) + } + + return downloadInfo, nil +} + func decodeCharmLocators(results []charmLocator) ([]charm.CharmLocator, error) { return transform.SliceOrErr(results, func(c charmLocator) (charm.CharmLocator, error) { source, err := decodeCharmSource(c.SourceID) diff --git a/domain/application/state/charm_test.go b/domain/application/state/charm_test.go index aa3f0b5fa29..055f6f7c7cb 100644 --- a/domain/application/state/charm_test.go +++ b/domain/application/state/charm_test.go @@ -2970,6 +2970,86 @@ func (s *charmStateSuite) TestListCharmLocatorsByNamesInvalidEntries(c *gc.C) { c.Check(results, gc.HasLen, 0) } +func (s *charmStateSuite) TestGetCharmDownloadInfoWithNoInfo(c *gc.C) { + st := NewState(s.TxnRunnerFactory(), clock.WallClock, loggertesting.WrapCheckLog(c)) + + id, err := st.SetCharm(context.Background(), charm.Charm{ + Metadata: charm.Metadata{ + Name: "foo", + }, + Manifest: s.minimalManifest(c), + Source: charm.LocalSource, + Revision: 42, + ReferenceName: "foo", + Hash: "hash", + ArchivePath: "archive", + Version: "deadbeef", + }, nil) + c.Assert(err, jc.ErrorIsNil) + + result, err := st.GetCharmDownloadInfo(context.Background(), id) + c.Assert(err, jc.ErrorIsNil) + c.Check(result, gc.IsNil) +} + +func (s *charmStateSuite) TestGetCharmDownloadInfoWithInfoForLocal(c *gc.C) { + st := NewState(s.TxnRunnerFactory(), clock.WallClock, loggertesting.WrapCheckLog(c)) + + info := &charm.DownloadInfo{ + DownloadProvenance: charm.ProvenanceDownload, + CharmhubIdentifier: "foo", + DownloadURL: "https://example.com/foo", + DownloadSize: 42, + } + + id, err := st.SetCharm(context.Background(), charm.Charm{ + Metadata: charm.Metadata{ + Name: "foo", + }, + Manifest: s.minimalManifest(c), + Source: charm.LocalSource, + Revision: 42, + ReferenceName: "foo", + Hash: "hash", + ArchivePath: "archive", + Version: "deadbeef", + }, info) + c.Assert(err, jc.ErrorIsNil) + + result, err := st.GetCharmDownloadInfo(context.Background(), id) + c.Assert(err, jc.ErrorIsNil) + c.Check(result, gc.IsNil) +} + +func (s *charmStateSuite) TestGetCharmDownloadInfoWithInfoForCharmhub(c *gc.C) { + st := NewState(s.TxnRunnerFactory(), clock.WallClock, loggertesting.WrapCheckLog(c)) + + info := &charm.DownloadInfo{ + DownloadProvenance: charm.ProvenanceDownload, + CharmhubIdentifier: "foo", + DownloadURL: "https://example.com/foo", + DownloadSize: 42, + } + + id, err := st.SetCharm(context.Background(), charm.Charm{ + Metadata: charm.Metadata{ + Name: "foo", + }, + Manifest: s.minimalManifest(c), + Source: charm.CharmHubSource, + Revision: 42, + ReferenceName: "foo", + Hash: "hash", + ArchivePath: "archive", + Version: "deadbeef", + }, info) + c.Assert(err, jc.ErrorIsNil) + + result, err := st.GetCharmDownloadInfo(context.Background(), id) + c.Assert(err, jc.ErrorIsNil) + c.Check(result, jc.DeepEquals, info) +} + func insertCharmState(ctx context.Context, c *gc.C, tx *sql.Tx, uuid string) error { _, err := tx.ExecContext(ctx, ` INSERT INTO charm (uuid, archive_path, available, reference_name, revision, version, architecture_id) diff --git a/internal/charm/repository/charmhub.go b/internal/charm/repository/charmhub.go index a67b99c6832..7d9cf1c165d 100644 --- a/internal/charm/repository/charmhub.go +++ b/internal/charm/repository/charmhub.go @@ -92,6 +92,13 @@ func (c *CharmHubRepository) ResolveForDeploy(ctx context.Context, arg corecharm essMeta.ResolvedOrigin = resolvedOrigin + // DownloadInfo is required for downloading the charm asynchronously. + essMeta.DownloadInfo = corecharm.DownloadInfo{ + CharmhubIdentifier: resp.Entity.ID, + DownloadURL: resp.Entity.Download.URL, + DownloadSize: int64(resp.Entity.Download.Size), + } + // Resources are best attempt here. If we were able to resolve the charm // via a channel, the resource data will be here. If using a revision, // then not. However, that does not mean that the charm has no resources. @@ -104,11 +111,6 @@ func (c *CharmHubRepository) ResolveForDeploy(ctx context.Context, arg corecharm URL: resultURL, EssentialMetadata: essMeta, Resources: resourceResults, - DownloadInfo: corecharm.DownloadInfo{ - CharmhubIdentifier: resp.Entity.ID, - DownloadURL: resp.Entity.Download.URL, - DownloadSize: int64(resp.Entity.Download.Size), - }, } return thing, nil } @@ -331,15 +333,17 @@ func (c *CharmHubRepository) retryResolveWithPreferredChannel(ctx context.Contex } func transformRefreshResult(charmName string, refreshResult transport.RefreshResponse) (corecharm.EssentialMetadata, error) { - if refreshResult.Entity.MetadataYAML == "" { + entity := refreshResult.Entity + + if entity.MetadataYAML == "" { return corecharm.EssentialMetadata{}, errors.NotValidf("charmhub refresh response for %q does not include the contents of metadata.yaml", charmName) } - chMeta, err := charm.ReadMeta(strings.NewReader(refreshResult.Entity.MetadataYAML)) + chMeta, err := charm.ReadMeta(strings.NewReader(entity.MetadataYAML)) if err != nil { return corecharm.EssentialMetadata{}, errors.Annotatef(err, "parsing metadata.yaml for %q", charmName) } - configYAML := refreshResult.Entity.ConfigYAML + configYAML := entity.ConfigYAML var chConfig *charm.Config // NOTE: Charmhub returns a "{}\n" when no config.yaml exists for // the charm, e.g. postgreql. However, this will fail the charm @@ -355,7 +359,7 @@ func transformRefreshResult(charmName string, refreshResult transport.RefreshRes } chManifest := new(charm.Manifest) - for _, base := range refreshResult.Entity.Bases { + for _, base := range entity.Bases { baseCh, err := charm.ParseChannelNormalize(base.Channel) if err != nil { return corecharm.EssentialMetadata{}, errors.Annotatef(err, "parsing base channel for %q", charmName) @@ -367,7 +371,17 @@ func transformRefreshResult(charmName string, refreshResult transport.RefreshRes Architectures: []string{base.Architecture}, }) } - return corecharm.EssentialMetadata{Meta: chMeta, Config: chConfig, Manifest: chManifest}, nil + + return corecharm.EssentialMetadata{ + Meta: chMeta, + Config: chConfig, + Manifest: chManifest, + DownloadInfo: corecharm.DownloadInfo{ + CharmhubIdentifier: entity.ID, + DownloadURL: entity.Download.URL, + DownloadSize: int64(entity.Download.Size), + }, + }, nil } // Download retrieves a blob from the store and saves its contents to the diff --git a/internal/charm/repository/charmhub_test.go b/internal/charm/repository/charmhub_test.go index 95700371622..9d8745a5ae2 100644 --- a/internal/charm/repository/charmhub_test.go +++ b/internal/charm/repository/charmhub_test.go @@ -253,7 +253,7 @@ func (s *charmHubRepositorySuite) TestResolveForDeployWithRevisionSuccess(c *gc. c.Check(obtainedData.URL, jc.DeepEquals, expected) c.Check(obtainedData.EssentialMetadata.ResolvedOrigin, jc.DeepEquals, expectedOrigin) - c.Check(obtainedData.DownloadInfo, jc.DeepEquals, corecharm.DownloadInfo{ + c.Check(obtainedData.EssentialMetadata.DownloadInfo, jc.DeepEquals, corecharm.DownloadInfo{ CharmhubIdentifier: "charmCHARMcharmCHARMcharmCHARM01", DownloadURL: "http://example.com/wordpress-42", DownloadSize: 42, @@ -304,13 +304,13 @@ func (s *charmHubRepositorySuite) TestResolveForDeploySuccessChooseBase(c *gc.C) c.Check(obtainedData.URL, jc.DeepEquals, expected) c.Check(obtainedData.EssentialMetadata.ResolvedOrigin, jc.DeepEquals, expectedOrigin) - c.Check(obtainedData.Resources, gc.HasLen, 1) - c.Check(obtainedData.DownloadInfo, jc.DeepEquals, corecharm.DownloadInfo{ + c.Check(obtainedData.EssentialMetadata.DownloadInfo, jc.DeepEquals, corecharm.DownloadInfo{ CharmhubIdentifier: "charmCHARMcharmCHARMcharmCHARM01", DownloadURL: "http://example.com/wordpress-42", DownloadSize: 42, }) + c.Assert(obtainedData.Resources, gc.HasLen, 1) foundResource := obtainedData.Resources["wal-e"] c.Check(foundResource.Name, gc.Equals, "wal-e") c.Check(foundResource.Path, gc.Equals, "wal-e.snap") @@ -650,6 +650,11 @@ func (s *charmHubRepositorySuite) TestGetEssentialMetadata(c *gc.C) { Risk: "stable", }, }, + DownloadInfo: corecharm.DownloadInfo{ + CharmhubIdentifier: "charmCHARMcharmCHARMcharmCHARM01", + DownloadURL: "http://example.com/wordpress-42", + DownloadSize: 42, + }, }) } From 5d65555128c674e3aa343b7fbaf675c372fd7158 Mon Sep 17 00:00:00 2001 From: Simon Richardson Date: Tue, 3 Dec 2024 10:12:06 +0000 Subject: [PATCH 5/7] chore: simplify --- domain/application/state/charm_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/domain/application/state/charm_test.go b/domain/application/state/charm_test.go index 055f6f7c7cb..f68ed1a6adb 100644 --- a/domain/application/state/charm_test.go +++ b/domain/application/state/charm_test.go @@ -1214,7 +1214,7 @@ func (s *charmStateSuite) TestSetCharmDownloadInfoForCharmhub(c *gc.C) { }, Manifest: charm.Manifest{ Bases: []charm.Base{ - charm.Base{ + { Name: "ubuntu", Channel: charm.Channel{ Risk: charm.RiskCandidate, @@ -1252,7 +1252,7 @@ func (s *charmStateSuite) TestSetCharmDownloadInfoForCharmhubWithoutDownloadInfo }, Manifest: charm.Manifest{ Bases: []charm.Base{ - charm.Base{ + { Name: "ubuntu", Channel: charm.Channel{ Risk: charm.RiskCandidate, @@ -1295,7 +1295,7 @@ func (s *charmStateSuite) TestSetCharmDownloadInfoForLocal(c *gc.C) { }, Manifest: charm.Manifest{ Bases: []charm.Base{ - charm.Base{ + { Name: "ubuntu", Channel: charm.Channel{ Risk: charm.RiskCandidate, @@ -1333,7 +1333,7 @@ func (s *charmStateSuite) TestSetCharmDownloadInfoForLocalWithoutInfo(c *gc.C) { }, Manifest: charm.Manifest{ Bases: []charm.Base{ - charm.Base{ + { Name: "ubuntu", Channel: charm.Channel{ Risk: charm.RiskCandidate, From 4151e837230c69fc4c87ce1549af60c5bccd2bf2 Mon Sep 17 00:00:00 2001 From: Simon Richardson Date: Tue, 3 Dec 2024 11:05:20 +0000 Subject: [PATCH 6/7] test: fix missing download and manifest info --- .../agent/caasapplication/application_test.go | 4 + .../agent/caasapplication/mock_test.go | 8 +- domain/port/state/state_test.go | 7 + domain/port/watcher_test.go | 7 + domain/schema/schema_test.go | 2 + domain/secret/service_test.go | 114 +++++------ domain/secret/watcher_test.go | 185 ++++++++++-------- domain/stub/service_test.go | 7 + domain/unitstate/state/state_test.go | 7 + internal/bootstrap/deployer_test.go | 4 + internal/testing/factory/factory.go | 3 + 11 files changed, 205 insertions(+), 143 deletions(-) diff --git a/apiserver/facades/agent/caasapplication/application_test.go b/apiserver/facades/agent/caasapplication/application_test.go index e02316e3068..891791b7d0e 100644 --- a/apiserver/facades/agent/caasapplication/application_test.go +++ b/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{ + DownloadProvenance: applicationcharm.ProvenanceUpload, + }, }, service.AddUnitArg{ UnitName: unitName, }) diff --git a/apiserver/facades/agent/caasapplication/mock_test.go b/apiserver/facades/agent/caasapplication/mock_test.go index 76af652430f..2a00902b7ab 100644 --- a/apiserver/facades/agent/caasapplication/mock_test.go +++ b/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 { diff --git a/domain/port/state/state_test.go b/domain/port/state/state_test.go index d46c2bf1cba..4ffc3e05e8c 100644 --- a/domain/port/state/state_test.go +++ b/domain/port/state/state_test.go @@ -80,6 +80,13 @@ func (s *baseSuite) createApplicationWithRelations(c *gc.C, appName string, rela Name: appName, Requires: relationsMap, }, + Manifest: charm.Manifest{ + Bases: []charm.Base{{ + Name: "ubuntu", + Channel: charm.Channel{Risk: charm.RiskStable}, + Architectures: []string{"amd64"}, + }}, + }, ReferenceName: appName, Architecture: architecture.AMD64, Revision: 1, diff --git a/domain/port/watcher_test.go b/domain/port/watcher_test.go index 4fc17fa2451..54befb0c2a1 100644 --- a/domain/port/watcher_test.go +++ b/domain/port/watcher_test.go @@ -97,6 +97,13 @@ func (s *watcherSuite) createApplicationWithRelations(c *gc.C, appName string, r Name: appName, Requires: relationsMap, }, + Manifest: charm.Manifest{ + Bases: []charm.Base{{ + Name: "ubuntu", + Channel: charm.Channel{Risk: charm.RiskStable}, + Architectures: []string{"amd64"}, + }}, + }, ReferenceName: appName, Architecture: architecture.AMD64, Revision: 1, diff --git a/domain/schema/schema_test.go b/domain/schema/schema_test.go index 7c9a82e2dbd..108dd166127 100644 --- a/domain/schema/schema_test.go +++ b/domain/schema/schema_test.go @@ -380,11 +380,13 @@ func (s *schemaSuite) TestModelTables(c *gc.C) { "charm_container_mount", "charm_container", "charm_device", + "charm_download_info", "charm_extra_binding", "charm_hash", "charm_manifest_base", "charm_metadata", "charm_payload", + "charm_provenance", "charm_relation_kind", "charm_relation_role", "charm_relation_scope", diff --git a/domain/secret/service_test.go b/domain/secret/service_test.go index ea06a518311..e225c277b05 100644 --- a/domain/secret/service_test.go +++ b/domain/secret/service_test.go @@ -47,7 +47,9 @@ var _ = gc.Suite(&serviceSuite{}) func (s *serviceSuite) SetUpTest(c *gc.C) { s.ControllerSuite.SetUpTest(c) + s.modelUUID = modeltesting.CreateTestModel(c, s.TxnRunnerFactory(), "test-model") + err := s.ModelTxnRunner(c, s.modelUUID.String()).StdTxn(context.Background(), func(ctx context.Context, tx *sql.Tx) error { _, err := tx.ExecContext(ctx, ` INSERT INTO model (uuid, controller_uuid, target_agent_version, name, type, cloud, cloud_type) @@ -58,38 +60,62 @@ func (s *serviceSuite) SetUpTest(c *gc.C) { c.Assert(err, jc.ErrorIsNil) } -func (s *serviceSuite) setup(c *gc.C) *gomock.Controller { - ctrl := gomock.NewController(c) - s.secretBackendState = secret.NewMockSecretBackendState(ctrl) +func (s *serviceSuite) TestDeleteSecretInternal(c *gc.C) { + defer s.setupMocks(c).Finish() - s.svc = service.NewSecretService( - state.NewState(func() (database.TxnRunner, error) { return s.ModelTxnRunner(c, s.modelUUID.String()), nil }, loggertesting.WrapCheckLog(c)), - s.secretBackendState, - loggertesting.WrapCheckLog(c), - service.SecretServiceParams{}, - ) + s.secretBackendState.EXPECT().AddSecretBackendReference(gomock.Any(), nil, s.modelUUID, gomock.Any()) + uri := s.createSecret(c, map[string]string{"foo": "bar"}, nil) - return ctrl + err := s.svc.DeleteSecret(context.Background(), uri, service.DeleteSecretParams{ + LeaderToken: successfulToken{}, + Accessor: service.SecretAccessor{ + Kind: service.UnitAccessor, + ID: "mariadb/0", + }, + Revisions: []int{1}, + }) + c.Assert(err, jc.ErrorIsNil) + + _, err = s.svc.GetSecret(context.Background(), uri) + c.Assert(err, jc.ErrorIs, secreterrors.SecretNotFound) } -type successfulToken struct{} +func (s *serviceSuite) TestDeleteSecretExternal(c *gc.C) { + defer s.setupMocks(c).Finish() -func (t successfulToken) Check() error { - return nil -} + ref := &coresecrets.ValueRef{ + BackendID: "backend-id", + RevisionID: "rev-id", + } + s.secretBackendState.EXPECT().AddSecretBackendReference(gomock.Any(), ref, s.modelUUID, gomock.Any()) + uri := s.createSecret(c, nil, ref) -type noopSecretDeleter struct{} + err := s.svc.DeleteSecret(context.Background(), uri, service.DeleteSecretParams{ + LeaderToken: successfulToken{}, + Accessor: service.SecretAccessor{ + Kind: service.UnitAccessor, + ID: "mariadb/0", + }, + Revisions: []int{1}, + }) + c.Assert(err, jc.ErrorIsNil) -func (noopSecretDeleter) DeleteSecret(ctx domain.AtomicContext, uri *coresecrets.URI, revs []int) error { - return nil + _, err = s.svc.GetSecret(context.Background(), uri) + c.Assert(err, jc.ErrorIs, secreterrors.SecretNotFound) } -type noopResourceStoreGetter struct{} +func (s *serviceSuite) setupMocks(c *gc.C) *gomock.Controller { + ctrl := gomock.NewController(c) + s.secretBackendState = secret.NewMockSecretBackendState(ctrl) -func (noopResourceStoreGetter) AddStore(t charmresource.Type, store resource.ResourceStore) {} + s.svc = service.NewSecretService( + state.NewState(func() (database.TxnRunner, error) { return s.ModelTxnRunner(c, s.modelUUID.String()), nil }, loggertesting.WrapCheckLog(c)), + s.secretBackendState, + loggertesting.WrapCheckLog(c), + service.SecretServiceParams{}, + ) -func (noopResourceStoreGetter) GetResourceStore(context.Context, charmresource.Type) (resource.ResourceStore, error) { - return nil, nil + return ctrl } func (s *serviceSuite) createSecret(c *gc.C, data map[string]string, valueRef *coresecrets.ValueRef) *coresecrets.URI { @@ -140,46 +166,22 @@ func (s *serviceSuite) createSecret(c *gc.C, data map[string]string, valueRef *c return uri } -func (s *serviceSuite) TestDeleteSecretInternal(c *gc.C) { - defer s.setup(c).Finish() +type successfulToken struct{} - s.secretBackendState.EXPECT().AddSecretBackendReference(gomock.Any(), nil, s.modelUUID, gomock.Any()) - uri := s.createSecret(c, map[string]string{"foo": "bar"}, nil) +func (t successfulToken) Check() error { + return nil +} - err := s.svc.DeleteSecret(context.Background(), uri, service.DeleteSecretParams{ - LeaderToken: successfulToken{}, - Accessor: service.SecretAccessor{ - Kind: service.UnitAccessor, - ID: "mariadb/0", - }, - Revisions: []int{1}, - }) - c.Assert(err, jc.ErrorIsNil) +type noopSecretDeleter struct{} - _, err = s.svc.GetSecret(context.Background(), uri) - c.Assert(err, jc.ErrorIs, secreterrors.SecretNotFound) +func (noopSecretDeleter) DeleteSecret(ctx domain.AtomicContext, uri *coresecrets.URI, revs []int) error { + return nil } -func (s *serviceSuite) TestDeleteSecretExternal(c *gc.C) { - defer s.setup(c).Finish() - - ref := &coresecrets.ValueRef{ - BackendID: "backend-id", - RevisionID: "rev-id", - } - s.secretBackendState.EXPECT().AddSecretBackendReference(gomock.Any(), ref, s.modelUUID, gomock.Any()) - uri := s.createSecret(c, nil, ref) +type noopResourceStoreGetter struct{} - err := s.svc.DeleteSecret(context.Background(), uri, service.DeleteSecretParams{ - LeaderToken: successfulToken{}, - Accessor: service.SecretAccessor{ - Kind: service.UnitAccessor, - ID: "mariadb/0", - }, - Revisions: []int{1}, - }) - c.Assert(err, jc.ErrorIsNil) +func (noopResourceStoreGetter) AddStore(t charmresource.Type, store resource.ResourceStore) {} - _, err = s.svc.GetSecret(context.Background(), uri) - c.Assert(err, jc.ErrorIs, secreterrors.SecretNotFound) +func (noopResourceStoreGetter) GetResourceStore(context.Context, charmresource.Type) (resource.ResourceStore, error) { + return nil, nil } diff --git a/domain/secret/watcher_test.go b/domain/secret/watcher_test.go index 5d6d3ff9411..9f8e0c42837 100644 --- a/domain/secret/watcher_test.go +++ b/domain/secret/watcher_test.go @@ -23,6 +23,7 @@ import ( corewatcher "github.com/juju/juju/core/watcher" "github.com/juju/juju/core/watcher/watchertest" "github.com/juju/juju/domain" + applicationcharm "github.com/juju/juju/domain/application/charm" applicationservice "github.com/juju/juju/domain/application/service" applicationstate "github.com/juju/juju/domain/application/state" "github.com/juju/juju/domain/secret" @@ -55,91 +56,6 @@ VALUES (?, ?, ?, "test", "iaas", "fluffy", "ec2") c.Assert(err, jc.ErrorIsNil) } -func (s *watcherSuite) setupUnits(c *gc.C, appName string) { - logger := loggertesting.WrapCheckLog(c) - st := applicationstate.NewState(s.TxnRunnerFactory(), clock.WallClock, logger) - svc := applicationservice.NewService(st, nil, - corestorage.ConstModelStorageRegistry(func() storage.ProviderRegistry { - return storage.NotImplementedProviderRegistry{} - }), - nil, - noopResourceStoreGetter{}, - clock.WallClock, - logger, - ) - - unitName, err := unit.NewNameFromParts(appName, 0) - c.Assert(err, jc.ErrorIsNil) - _, err = svc.CreateApplication(context.Background(), - appName, - &stubCharm{}, - corecharm.Origin{ - Source: corecharm.CharmHub, - Platform: corecharm.Platform{ - Channel: "24.04", - OS: "ubuntu", - Architecture: "amd64", - }, - }, - applicationservice.AddApplicationArgs{ - ReferenceName: appName, - }, - applicationservice.AddUnitArg{UnitName: unitName}, - ) - c.Assert(err, jc.ErrorIsNil) -} - -func revID(uri *coresecrets.URI, rev int) string { - return fmt.Sprintf("%s/%d", uri.ID, rev) -} - -func createNewRevision(c *gc.C, st *state.State, uri *coresecrets.URI) { - sp := secret.UpsertSecretParams{ - Data: coresecrets.SecretData{"foo-new": "bar-new"}, - RevisionID: ptr(uuid.MustNewUUID().String()), - } - err := st.RunAtomic(context.Background(), func(ctx domain.AtomicContext) error { - return st.UpdateSecret(ctx, uri, sp) - }) - c.Assert(err, jc.ErrorIsNil) -} - -func (s *watcherSuite) setupServiceAndState(c *gc.C) (*service.WatchableService, *state.State) { - logger := loggertesting.WrapCheckLog(c) - st := state.NewState(s.TxnRunnerFactory(), logger) - factory := domain.NewWatcherFactory( - changestream.NewWatchableDBFactoryForNamespace(s.GetWatchableDB, "secret_revision"), - logger, - ) - return service.NewWatchableService(st, nil, logger, factory, service.SecretServiceParams{}), st -} - -func createUserSecret(ctx context.Context, st *state.State, version int, uri *coresecrets.URI, secret secret.UpsertSecretParams) error { - return st.RunAtomic(ctx, func(ctx domain.AtomicContext) error { - return st.CreateUserSecret(ctx, version, uri, secret) - }) -} - -func createCharmApplicationSecret(ctx context.Context, st *state.State, version int, uri *coresecrets.URI, appName string, secret secret.UpsertSecretParams) error { - return st.RunAtomic(ctx, func(ctx domain.AtomicContext) error { - appUUID, err := st.GetApplicationUUID(ctx, appName) - if err != nil { - return err - } - return st.CreateCharmApplicationSecret(ctx, version, uri, appUUID, secret) - }) -} - -func createCharmUnitSecret(ctx context.Context, st *state.State, version int, uri *coresecrets.URI, unitName string, secret secret.UpsertSecretParams) error { - return st.RunAtomic(ctx, func(ctx domain.AtomicContext) error { - unitUUID, err := st.GetUnitUUID(ctx, unitName) - if err != nil { - return err - } - return st.CreateCharmUnitSecret(ctx, version, uri, unitUUID, secret) - }) -} - func (s *watcherSuite) TestWatchObsoleteForAppsAndUnitsOwned(c *gc.C) { s.setupUnits(c, "mysql") s.setupUnits(c, "mediawiki") @@ -826,6 +742,97 @@ func (s *watcherSuite) TestWatchSecretsRevisionExpiryChanges(c *gc.C) { wc1.AssertNoChange() } +func (s *watcherSuite) setupUnits(c *gc.C, appName string) { + logger := loggertesting.WrapCheckLog(c) + st := applicationstate.NewState(s.TxnRunnerFactory(), clock.WallClock, logger) + svc := applicationservice.NewService(st, nil, + corestorage.ConstModelStorageRegistry(func() storage.ProviderRegistry { + return storage.NotImplementedProviderRegistry{} + }), + nil, + noopResourceStoreGetter{}, + clock.WallClock, + logger, + ) + + unitName, err := unit.NewNameFromParts(appName, 0) + c.Assert(err, jc.ErrorIsNil) + _, err = svc.CreateApplication(context.Background(), + appName, + &stubCharm{}, + corecharm.Origin{ + Source: corecharm.CharmHub, + Platform: corecharm.Platform{ + Channel: "24.04", + OS: "ubuntu", + Architecture: "amd64", + }, + }, + applicationservice.AddApplicationArgs{ + ReferenceName: appName, + DownloadInfo: &applicationcharm.DownloadInfo{ + DownloadProvenance: applicationcharm.ProvenanceDownload, + CharmhubIdentifier: "wordpress-1", + DownloadURL: "https://example.com/wordpress-1", + DownloadSize: 1000, + }, + }, + applicationservice.AddUnitArg{UnitName: unitName}, + ) + c.Assert(err, jc.ErrorIsNil) +} + +func (s *watcherSuite) setupServiceAndState(c *gc.C) (*service.WatchableService, *state.State) { + logger := loggertesting.WrapCheckLog(c) + st := state.NewState(s.TxnRunnerFactory(), logger) + factory := domain.NewWatcherFactory( + changestream.NewWatchableDBFactoryForNamespace(s.GetWatchableDB, "secret_revision"), + logger, + ) + return service.NewWatchableService(st, nil, logger, factory, service.SecretServiceParams{}), st +} + +func revID(uri *coresecrets.URI, rev int) string { + return fmt.Sprintf("%s/%d", uri.ID, rev) +} + +func createNewRevision(c *gc.C, st *state.State, uri *coresecrets.URI) { + sp := secret.UpsertSecretParams{ + Data: coresecrets.SecretData{"foo-new": "bar-new"}, + RevisionID: ptr(uuid.MustNewUUID().String()), + } + err := st.RunAtomic(context.Background(), func(ctx domain.AtomicContext) error { + return st.UpdateSecret(ctx, uri, sp) + }) + c.Assert(err, jc.ErrorIsNil) +} + +func createUserSecret(ctx context.Context, st *state.State, version int, uri *coresecrets.URI, secret secret.UpsertSecretParams) error { + return st.RunAtomic(ctx, func(ctx domain.AtomicContext) error { + return st.CreateUserSecret(ctx, version, uri, secret) + }) +} + +func createCharmApplicationSecret(ctx context.Context, st *state.State, version int, uri *coresecrets.URI, appName string, secret secret.UpsertSecretParams) error { + return st.RunAtomic(ctx, func(ctx domain.AtomicContext) error { + appUUID, err := st.GetApplicationUUID(ctx, appName) + if err != nil { + return err + } + return st.CreateCharmApplicationSecret(ctx, version, uri, appUUID, secret) + }) +} + +func createCharmUnitSecret(ctx context.Context, st *state.State, version int, uri *coresecrets.URI, unitName string, secret secret.UpsertSecretParams) error { + return st.RunAtomic(ctx, func(ctx domain.AtomicContext) error { + unitUUID, err := st.GetUnitUUID(ctx, unitName) + if err != nil { + return err + } + return st.CreateCharmUnitSecret(ctx, version, uri, unitUUID, secret) + }) +} + type stubCharm struct{} var _ charm.Charm = (*stubCharm)(nil) @@ -837,7 +844,13 @@ func (m *stubCharm) Meta() *charm.Meta { } func (m *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 (m *stubCharm) Config() *charm.Config { diff --git a/domain/stub/service_test.go b/domain/stub/service_test.go index 8edcb36818b..0c0ae7fcc3f 100644 --- a/domain/stub/service_test.go +++ b/domain/stub/service_test.go @@ -41,6 +41,13 @@ var addApplicationArg = application.AddApplicationArg{ Metadata: charm.Metadata{ Name: "foo", }, + Manifest: charm.Manifest{ + Bases: []charm.Base{{ + Name: "ubuntu", + Channel: charm.Channel{Risk: charm.RiskStable}, + Architectures: []string{"amd64"}, + }}, + }, Source: charm.LocalSource, Architecture: architecture.AMD64, ReferenceName: "foo", diff --git a/domain/unitstate/state/state_test.go b/domain/unitstate/state/state_test.go index 3e7dc46d138..37feea8997c 100644 --- a/domain/unitstate/state/state_test.go +++ b/domain/unitstate/state/state_test.go @@ -40,6 +40,13 @@ func (s *stateSuite) SetUpTest(c *gc.C) { Metadata: charm.Metadata{ Name: "app", }, + Manifest: charm.Manifest{ + Bases: []charm.Base{{ + Name: "ubuntu", + Channel: charm.Channel{Risk: charm.RiskStable}, + Architectures: []string{"amd64"}, + }}, + }, ReferenceName: "app", Source: charm.LocalSource, Architecture: architecture.AMD64, diff --git a/internal/bootstrap/deployer_test.go b/internal/bootstrap/deployer_test.go index dbd196bda4d..4f6a4f1ef79 100644 --- a/internal/bootstrap/deployer_test.go +++ b/internal/bootstrap/deployer_test.go @@ -25,6 +25,7 @@ import ( "github.com/juju/juju/core/constraints" "github.com/juju/juju/core/objectstore" "github.com/juju/juju/core/unit" + applicationcharm "github.com/juju/juju/domain/application/charm" applicationservice "github.com/juju/juju/domain/application/service" "github.com/juju/juju/environs/bootstrap" "github.com/juju/juju/internal/charm" @@ -282,6 +283,9 @@ func (s *deployerSuite) TestAddControllerApplication(c *gc.C) { }, applicationservice.AddApplicationArgs{ ReferenceName: bootstrap.ControllerApplicationName, + DownloadInfo: &applicationcharm.DownloadInfo{ + DownloadProvenance: applicationcharm.ProvenanceBootstrap, + }, }, applicationservice.AddUnitArg{UnitName: unitName}, ) diff --git a/internal/testing/factory/factory.go b/internal/testing/factory/factory.go index b3c766e454e..75cb870d132 100644 --- a/internal/testing/factory/factory.go +++ b/internal/testing/factory/factory.go @@ -611,6 +611,9 @@ func (factory *Factory) MakeUnitReturningPassword(c *gc.C, params *UnitParams) ( chOrigin.AsCoreCharmOrigin(), applicationservice.AddApplicationArgs{ ReferenceName: params.Application.Name(), Storage: directives, + DownloadInfo: &applicationcharm.DownloadInfo{ + DownloadProvenance: applicationcharm.ProvenanceUpload, + }, }) if !errors.Is(err, applicationerrors.ApplicationAlreadyExists) { c.Assert(err, jc.ErrorIsNil) From 97b7ef7cece699e8a20dbe877208afdb3e7397c7 Mon Sep 17 00:00:00 2001 From: Simon Richardson Date: Wed, 4 Dec 2024 16:56:22 +0000 Subject: [PATCH 7/7] chore: rename to provenance --- .../agent/caasapplication/application_test.go | 2 +- .../client/application/deployrepository.go | 2 +- apiserver/facades/client/charms/client.go | 2 +- apiserver/objects.go | 2 +- domain/application/charm/types.go | 25 +++++----- domain/application/errors/errors.go | 4 +- domain/application/modelmigration/import.go | 2 +- .../application/modelmigration/import_test.go | 2 +- .../application/service/application_test.go | 20 ++++---- domain/application/service/charm.go | 47 +++++++++---------- domain/application/service/charm_test.go | 4 +- domain/application/service/service_test.go | 2 +- domain/application/service_test.go | 4 +- domain/application/state/application_test.go | 2 +- domain/application/state/charm_test.go | 6 +-- domain/application/state/state.go | 20 ++++---- domain/application/state/types.go | 12 ++--- domain/application/watcher_test.go | 8 ++-- domain/secret/watcher_test.go | 2 +- internal/bootstrap/deployer.go | 2 +- internal/bootstrap/deployer_test.go | 2 +- internal/testing/factory/factory.go | 4 +- 22 files changed, 86 insertions(+), 90 deletions(-) diff --git a/apiserver/facades/agent/caasapplication/application_test.go b/apiserver/facades/agent/caasapplication/application_test.go index 891791b7d0e..2e63feff166 100644 --- a/apiserver/facades/agent/caasapplication/application_test.go +++ b/apiserver/facades/agent/caasapplication/application_test.go @@ -75,7 +75,7 @@ func (s *CAASApplicationSuite) SetUpTest(c *gc.C) { context.Background(), "gitlab", &stubCharm{}, origin, service.AddApplicationArgs{ ReferenceName: "gitlab", DownloadInfo: &applicationcharm.DownloadInfo{ - DownloadProvenance: applicationcharm.ProvenanceUpload, + Provenance: applicationcharm.ProvenanceUpload, }, }, service.AddUnitArg{ UnitName: unitName, diff --git a/apiserver/facades/client/application/deployrepository.go b/apiserver/facades/client/application/deployrepository.go index 8e1d5a1c730..6a299fa9a38 100644 --- a/apiserver/facades/client/application/deployrepository.go +++ b/apiserver/facades/client/application/deployrepository.go @@ -173,7 +173,7 @@ func (api *DeployFromRepositoryAPI) DeployFromRepository(ctx context.Context, ar Storage: dt.storage, // We always have download info for a charm from the charmhub store. DownloadInfo: &applicationcharm.DownloadInfo{ - DownloadProvenance: applicationcharm.ProvenanceDownload, + Provenance: applicationcharm.ProvenanceDownload, CharmhubIdentifier: dt.downloadInfo.CharmhubIdentifier, DownloadURL: dt.downloadInfo.DownloadURL, DownloadSize: dt.downloadInfo.DownloadSize, diff --git a/apiserver/facades/client/charms/client.go b/apiserver/facades/client/charms/client.go index e86dc05c1e8..13f2cc0825c 100644 --- a/apiserver/facades/client/charms/client.go +++ b/apiserver/facades/client/charms/client.go @@ -312,7 +312,7 @@ func (a *API) queueAsyncCharmDownload(ctx context.Context, args params.AddCharmW Revision: revision, Hash: essentialMetadata.ResolvedOrigin.Hash, DownloadInfo: &applicationcharm.DownloadInfo{ - DownloadProvenance: applicationcharm.ProvenanceDownload, + Provenance: applicationcharm.ProvenanceDownload, CharmhubIdentifier: essentialMetadata.DownloadInfo.CharmhubIdentifier, DownloadURL: essentialMetadata.DownloadInfo.DownloadURL, DownloadSize: essentialMetadata.DownloadInfo.DownloadSize, diff --git a/apiserver/objects.go b/apiserver/objects.go index bd924cebb9e..25ab41a3513 100644 --- a/apiserver/objects.go +++ b/apiserver/objects.go @@ -251,7 +251,7 @@ func (h *objectsCharmHTTPHandler) processPut(r *http.Request, st *state.State, c // We can not re-download this charm from the charm store again, without // another call directly to the charm store. DownloadInfo: &applicationcharm.DownloadInfo{ - DownloadProvenance: provenance, + Provenance: provenance, }, }); err != nil { return nil, errors.Trace(err) diff --git a/domain/application/charm/types.go b/domain/application/charm/types.go index f2cd21a06c0..27719991aa4 100644 --- a/domain/application/charm/types.go +++ b/domain/application/charm/types.go @@ -153,26 +153,25 @@ type CharmLocator struct { Architecture architecture.Architecture } -// DownloadProvenance represents the provenance of a charm download. -// Ideally this would be called origin, but that's already used for an origin -// of a charm. -type DownloadProvenance string +// Provenance represents the provenance of a charm download. Ideally this would +// be called origin, but that's already used for an origin of a charm. +type Provenance string const ( // ProvenanceDownload represents a charm download from the charmhub. - ProvenanceDownload DownloadProvenance = "download" + ProvenanceDownload Provenance = "download" // ProvenanceUpload represents a charm download from an upload. - ProvenanceUpload DownloadProvenance = "upload" + ProvenanceUpload Provenance = "upload" // ProvenanceMigration represents a charm download from a migration. - ProvenanceMigration DownloadProvenance = "migration" + ProvenanceMigration Provenance = "migration" // ProvenanceBootstrap represents a charm placement during bootstrap. - ProvenanceBootstrap DownloadProvenance = "bootstrap" + ProvenanceBootstrap Provenance = "bootstrap" ) // DownloadInfo holds the information needed to download a charmhub charm. type DownloadInfo struct { - // DownloadProvenance is the provenance of the download. - DownloadProvenance DownloadProvenance + // Provenance is the provenance of the download. + Provenance Provenance // CharmhubIdentifier is the instance ID of the charm in relation to // charmhub. @@ -188,12 +187,12 @@ type DownloadInfo struct { // Validate validates the download information. func (d DownloadInfo) Validate() error { - if d.DownloadProvenance == "" { - return applicationerrors.CharmDownloadProvenanceNotValid + if d.Provenance == "" { + return applicationerrors.CharmProvenanceNotValid } // We don't validate the download information if it's not a download. - if d.DownloadProvenance != ProvenanceDownload { + if d.Provenance != ProvenanceDownload { return nil } diff --git a/domain/application/errors/errors.go b/domain/application/errors/errors.go index 08b91e92fb9..3a1d6852029 100644 --- a/domain/application/errors/errors.go +++ b/domain/application/errors/errors.go @@ -187,7 +187,7 @@ const ( // download URL is not valid. CharmDownloadURLNotValid = errors.ConstError("charm download URL not valid") - // CharmDownloadProvenanceNotValid describes an error that occurs when the + // CharmProvenanceNotValid describes an error that occurs when the // charm download provenance is not valid. - CharmDownloadProvenanceNotValid = errors.ConstError("charm download provenance not valid") + CharmProvenanceNotValid = errors.ConstError("charm provenance not valid") ) diff --git a/domain/application/modelmigration/import.go b/domain/application/modelmigration/import.go index fe872dccff0..df3bb144afb 100644 --- a/domain/application/modelmigration/import.go +++ b/domain/application/modelmigration/import.go @@ -171,7 +171,7 @@ func (i *importOperation) Execute(ctx context.Context, model description.Model) // If the controllers do not have the same charmhub url, then // all bets are off. DownloadInfo: &applicationcharm.DownloadInfo{ - DownloadProvenance: applicationcharm.ProvenanceMigration, + Provenance: applicationcharm.ProvenanceMigration, }, }, unitArgs..., ) diff --git a/domain/application/modelmigration/import_test.go b/domain/application/modelmigration/import_test.go index 2fb6892d997..1be77607d58 100644 --- a/domain/application/modelmigration/import_test.go +++ b/domain/application/modelmigration/import_test.go @@ -135,7 +135,7 @@ func (s *importSuite) TestApplicationImportWithMinimalCharm(c *gc.C) { service.AddApplicationArgs{ ReferenceName: "prometheus", DownloadInfo: &charm.DownloadInfo{ - DownloadProvenance: charm.ProvenanceMigration, + Provenance: charm.ProvenanceMigration, }, }, []service.ImportUnitArg{{ diff --git a/domain/application/service/application_test.go b/domain/application/service/application_test.go index 40668ab1ab6..759d9e94408 100644 --- a/domain/application/service/application_test.go +++ b/domain/application/service/application_test.go @@ -92,7 +92,7 @@ func (s *applicationServiceSuite) TestCreateApplication(c *gc.C) { app := application.AddApplicationArg{ Charm: ch, CharmDownloadInfo: &domaincharm.DownloadInfo{ - DownloadProvenance: domaincharm.ProvenanceDownload, + Provenance: domaincharm.ProvenanceDownload, CharmhubIdentifier: "foo", DownloadURL: "https://example.com/foo", DownloadSize: 42, @@ -132,7 +132,7 @@ func (s *applicationServiceSuite) TestCreateApplication(c *gc.C) { }, AddApplicationArgs{ ReferenceName: "ubuntu", DownloadInfo: &domaincharm.DownloadInfo{ - DownloadProvenance: domaincharm.ProvenanceDownload, + Provenance: domaincharm.ProvenanceDownload, CharmhubIdentifier: "foo", DownloadURL: "https://example.com/foo", DownloadSize: 42, @@ -243,7 +243,7 @@ func (s *applicationServiceSuite) TestCreateApplicationWithNoArchitecture(c *gc. }, AddApplicationArgs{ ReferenceName: "foo", DownloadInfo: &domaincharm.DownloadInfo{ - DownloadProvenance: domaincharm.ProvenanceDownload, + Provenance: domaincharm.ProvenanceDownload, CharmhubIdentifier: "foo", DownloadURL: "https://example.com/foo", DownloadSize: 42, @@ -279,7 +279,7 @@ func (s *applicationServiceSuite) TestCreateApplicationError(c *gc.C) { }, AddApplicationArgs{ ReferenceName: "foo", DownloadInfo: &domaincharm.DownloadInfo{ - DownloadProvenance: domaincharm.ProvenanceDownload, + Provenance: domaincharm.ProvenanceDownload, CharmhubIdentifier: "foo", DownloadURL: "https://example.com/foo", DownloadSize: 42, @@ -447,7 +447,7 @@ func (s *applicationServiceSuite) TestCreateWithStorageBlockDefaultSource(c *gc. app := application.AddApplicationArg{ Charm: ch, CharmDownloadInfo: &domaincharm.DownloadInfo{ - DownloadProvenance: domaincharm.ProvenanceDownload, + Provenance: domaincharm.ProvenanceDownload, CharmhubIdentifier: "foo", DownloadURL: "https://example.com/foo", DownloadSize: 42, @@ -494,7 +494,7 @@ func (s *applicationServiceSuite) TestCreateWithStorageBlockDefaultSource(c *gc. }, AddApplicationArgs{ ReferenceName: "foo", DownloadInfo: &domaincharm.DownloadInfo{ - DownloadProvenance: domaincharm.ProvenanceDownload, + Provenance: domaincharm.ProvenanceDownload, CharmhubIdentifier: "foo", DownloadURL: "https://example.com/foo", DownloadSize: 42, @@ -558,7 +558,7 @@ func (s *applicationServiceSuite) TestCreateWithStorageFilesystem(c *gc.C) { app := application.AddApplicationArg{ Charm: ch, CharmDownloadInfo: &domaincharm.DownloadInfo{ - DownloadProvenance: domaincharm.ProvenanceDownload, + Provenance: domaincharm.ProvenanceDownload, CharmhubIdentifier: "foo", DownloadURL: "https://example.com/foo", DownloadSize: 42, @@ -605,7 +605,7 @@ func (s *applicationServiceSuite) TestCreateWithStorageFilesystem(c *gc.C) { }, AddApplicationArgs{ ReferenceName: "foo", DownloadInfo: &domaincharm.DownloadInfo{ - DownloadProvenance: domaincharm.ProvenanceDownload, + Provenance: domaincharm.ProvenanceDownload, CharmhubIdentifier: "foo", DownloadURL: "https://example.com/foo", DownloadSize: 42, @@ -666,7 +666,7 @@ func (s *applicationServiceSuite) TestCreateWithStorageFilesystemDefaultSource(c app := application.AddApplicationArg{ Charm: ch, CharmDownloadInfo: &domaincharm.DownloadInfo{ - DownloadProvenance: domaincharm.ProvenanceDownload, + Provenance: domaincharm.ProvenanceDownload, CharmhubIdentifier: "foo", DownloadURL: "https://example.com/foo", DownloadSize: 42, @@ -713,7 +713,7 @@ func (s *applicationServiceSuite) TestCreateWithStorageFilesystemDefaultSource(c }, AddApplicationArgs{ ReferenceName: "foo", DownloadInfo: &domaincharm.DownloadInfo{ - DownloadProvenance: domaincharm.ProvenanceDownload, + Provenance: domaincharm.ProvenanceDownload, CharmhubIdentifier: "foo", DownloadURL: "https://example.com/foo", DownloadSize: 42, diff --git a/domain/application/service/charm.go b/domain/application/service/charm.go index 83b1c29f5c6..1f67d6473f6 100644 --- a/domain/application/service/charm.go +++ b/domain/application/service/charm.go @@ -128,15 +128,14 @@ type CharmState interface { // exist, a [applicationerrors.CharmNotFound] error is returned. DeleteCharm(ctx context.Context, id corecharm.ID) error - // ListCharmLocatorsByNames returns a list of charm locators. - // The locator allows the reconstruction of the charm URL for the client - // response. + // ListCharmLocators returns a list of charm locators. The locator allows + // the reconstruction of the charm URL for the client response. ListCharmLocators(ctx context.Context) ([]charm.CharmLocator, error) - // ListCharmLocatorsByNames returns a list of charm locators for the specified - // charm names. - // The locator allows the reconstruction of the charm URL for the client - // response. If no names are provided, then nothing is returned. + // ListCharmLocatorsByNames returns a list of charm locators for the + // specified charm names. The locator allows the reconstruction of the charm + // URL for the client response. If no names are provided, then nothing is + // returned. ListCharmLocatorsByNames(ctx context.Context, names []string) ([]charm.CharmLocator, error) // GetCharmDownloadInfo returns the download info for the charm using the @@ -144,19 +143,18 @@ type CharmState interface { GetCharmDownloadInfo(ctx context.Context, id corecharm.ID) (*charm.DownloadInfo, error) } -// CharmStore defines the interface for storing and retrieving charms archive blobs -// from the underlying storage. +// CharmStore defines the interface for storing and retrieving charms archive +// blobs from the underlying storage. type CharmStore interface { - // GetCharm retrieves a ReadCloser for the charm archive at the give path from - // the underlying storage. + // GetCharm retrieves a ReadCloser for the charm archive at the give path + // from the underlying storage. Get(ctx context.Context, archivePath string) (io.ReadCloser, error) } -// GetCharmID returns a charm ID by name. It returns an error if the charm -// can not be found by the name. -// This can also be used as a cheap way to see if a charm exists without -// needing to load the charm metadata. -// Returns [applicationerrors.CharmNameNotValid] if the name is not valid, and +// GetCharmID returns a charm ID by name. It returns an error if the charm can +// not be found by the name. This can also be used as a cheap way to see if a +// charm exists without needing to load the charm metadata. Returns +// [applicationerrors.CharmNameNotValid] if the name is not valid, and // [applicationerrors.CharmNotFound] if the charm is not found. func (s *Service) GetCharmID(ctx context.Context, args charm.GetCharmArgs) (corecharm.ID, error) { if !isValidCharmName(args.Name) { @@ -190,9 +188,8 @@ func (s *Service) IsControllerCharm(ctx context.Context, id corecharm.ID) (bool, } // SupportsContainers returns whether the charm supports containers. This -// currently means that the charm is a kubernetes charm. -// This will return true if the charm is a controller charm, and false -// otherwise. +// currently means that the charm is a kubernetes charm. This will return true +// if the charm is a controller charm, and false otherwise. // // If the charm does not exist, a [applicationerrors.CharmNotFound] error is // returned. @@ -224,12 +221,12 @@ func (s *Service) IsSubordinateCharm(ctx context.Context, id corecharm.ID) (bool return b, nil } -// GetCharm returns the charm using the charm ID. -// Calling this method will return all the data associated with the charm. -// It is not expected to call this method for all calls, instead use the move -// focused and specific methods. That's because this method is very expensive -// to call. This is implemented for the cases where all the charm data is -// needed; model migration, charm export, etc. +// GetCharm returns the charm using the charm ID. Calling this method will +// return all the data associated with the charm. It is not expected to call +// this method for all calls, instead use the move focused and specific methods. +// That's because this method is very expensive to call. This is implemented for +// the cases where all the charm data is needed; model migration, charm export, +// etc. // // If the charm does not exist, a [applicationerrors.CharmNotFound] error is // returned. diff --git a/domain/application/service/charm_test.go b/domain/application/service/charm_test.go index 592c2cc6626..10020c92dde 100644 --- a/domain/application/service/charm_test.go +++ b/domain/application/service/charm_test.go @@ -536,7 +536,7 @@ func (s *charmServiceSuite) TestSetCharmCharmhub(c *gc.C) { }}}).MinTimes(1) downloadInfo := &charm.DownloadInfo{ - DownloadProvenance: charm.ProvenanceDownload, + Provenance: charm.ProvenanceDownload, CharmhubIdentifier: "foo", DownloadURL: "http://example.com/foo", DownloadSize: 42, @@ -1050,7 +1050,7 @@ func (s *charmServiceSuite) TestGetCharmDownloadInfo(c *gc.C) { id := charmtesting.GenCharmID(c) expected := &charm.DownloadInfo{ - DownloadProvenance: charm.ProvenanceDownload, + Provenance: charm.ProvenanceDownload, CharmhubIdentifier: "foo", DownloadURL: "http://example.com/foo", DownloadSize: 42, diff --git a/domain/application/service/service_test.go b/domain/application/service/service_test.go index e46fbd2c3bc..d319d4669fb 100644 --- a/domain/application/service/service_test.go +++ b/domain/application/service/service_test.go @@ -77,7 +77,7 @@ func (s *migrationServiceSuite) TestImportApplication(c *gc.C) { Architecture: architecture.ARM64, } downloadInfo := &domaincharm.DownloadInfo{ - DownloadProvenance: domaincharm.ProvenanceDownload, + Provenance: domaincharm.ProvenanceDownload, DownloadURL: "http://example.com", DownloadSize: 24, CharmhubIdentifier: "foobar", diff --git a/domain/application/service_test.go b/domain/application/service_test.go index 150fa042add..be0ae80cd84 100644 --- a/domain/application/service_test.go +++ b/domain/application/service_test.go @@ -853,8 +853,8 @@ func (s *serviceSuite) createApplication(c *gc.C, name string, units ...service. }, service.AddApplicationArgs{ ReferenceName: name, DownloadInfo: &applicationcharm.DownloadInfo{ - DownloadProvenance: applicationcharm.ProvenanceDownload, - DownloadURL: "https://example.com", + Provenance: applicationcharm.ProvenanceDownload, + DownloadURL: "https://example.com", }, }, units...) c.Assert(err, jc.ErrorIsNil) diff --git a/domain/application/state/application_test.go b/domain/application/state/application_test.go index 6ded89cd56d..da81dbb3560 100644 --- a/domain/application/state/application_test.go +++ b/domain/application/state/application_test.go @@ -94,7 +94,7 @@ func (s *applicationStateSuite) TestCreateApplication(c *gc.C) { Architecture: architecture.ARM64, }, CharmDownloadInfo: &charm.DownloadInfo{ - DownloadProvenance: charm.ProvenanceDownload, + Provenance: charm.ProvenanceDownload, CharmhubIdentifier: "ident-1", DownloadURL: "http://example.com/charm", DownloadSize: 666, diff --git a/domain/application/state/charm_test.go b/domain/application/state/charm_test.go index f68ed1a6adb..0bef9bb9301 100644 --- a/domain/application/state/charm_test.go +++ b/domain/application/state/charm_test.go @@ -1196,7 +1196,7 @@ func (s *charmStateSuite) TestSetCharmDownloadInfoForCharmhub(c *gc.C) { st := NewState(s.TxnRunnerFactory(), clock.WallClock, loggertesting.WrapCheckLog(c)) info := &charm.DownloadInfo{ - DownloadProvenance: charm.ProvenanceDownload, + Provenance: charm.ProvenanceDownload, CharmhubIdentifier: "ident-1", DownloadURL: "https://example.com/charmhub/ident-1", DownloadSize: 1234, @@ -2996,7 +2996,7 @@ func (s *charmStateSuite) TestGetCharmDownloadInfoWithInfoForLocal(c *gc.C) { st := NewState(s.TxnRunnerFactory(), clock.WallClock, loggertesting.WrapCheckLog(c)) info := &charm.DownloadInfo{ - DownloadProvenance: charm.ProvenanceDownload, + Provenance: charm.ProvenanceDownload, CharmhubIdentifier: "foo", DownloadURL: "https://example.com/foo", DownloadSize: 42, @@ -3025,7 +3025,7 @@ func (s *charmStateSuite) TestGetCharmDownloadInfoWithInfoForCharmhub(c *gc.C) { st := NewState(s.TxnRunnerFactory(), clock.WallClock, loggertesting.WrapCheckLog(c)) info := &charm.DownloadInfo{ - DownloadProvenance: charm.ProvenanceDownload, + Provenance: charm.ProvenanceDownload, CharmhubIdentifier: "foo", DownloadURL: "https://example.com/foo", DownloadSize: 42, diff --git a/domain/application/state/state.go b/domain/application/state/state.go index 1f612c6e985..8f9b7fd0ec6 100644 --- a/domain/application/state/state.go +++ b/domain/application/state/state.go @@ -237,17 +237,17 @@ func (s *State) setCharmDownloadInfo(ctx context.Context, tx *sqlair.TX, id core return nil } - provenance, err := encodeProvenance(downloadInfo.DownloadProvenance) + provenance, err := encodeProvenance(downloadInfo.Provenance) if err != nil { return fmt.Errorf("encoding charm provenance: %w", err) } downloadInfoState := setCharmDownloadInfo{ - CharmUUID: id.String(), - DownloadProvenanceID: provenance, - CharmhubIdentifier: downloadInfo.CharmhubIdentifier, - DownloadURL: downloadInfo.DownloadURL, - DownloadSize: downloadInfo.DownloadSize, + CharmUUID: id.String(), + ProvenanceID: provenance, + CharmhubIdentifier: downloadInfo.CharmhubIdentifier, + DownloadURL: downloadInfo.DownloadURL, + DownloadSize: downloadInfo.DownloadSize, } query := `INSERT INTO charm_download_info (*) VALUES ($setCharmDownloadInfo.*);` @@ -701,13 +701,13 @@ AND c.source_id = 1; return charm.DownloadInfo{}, fmt.Errorf("getting charm download info: %w", err) } - provenance, err := decodeProvenance(downloadInfo.DownloadProvenance) + provenance, err := decodeProvenance(downloadInfo.Provenance) if err != nil { return charm.DownloadInfo{}, fmt.Errorf("decoding charm provenance: %w", err) } return charm.DownloadInfo{ - DownloadProvenance: provenance, + Provenance: provenance, CharmhubIdentifier: downloadInfo.CharmhubIdentifier, DownloadURL: downloadInfo.DownloadURL, DownloadSize: downloadInfo.DownloadSize, @@ -1362,7 +1362,7 @@ func encodeCharmSource(source charm.CharmSource) (int, error) { } } -func encodeProvenance(provenance charm.DownloadProvenance) (int, error) { +func encodeProvenance(provenance charm.Provenance) (int, error) { switch provenance { case charm.ProvenanceDownload: return 0, nil @@ -1377,7 +1377,7 @@ func encodeProvenance(provenance charm.DownloadProvenance) (int, error) { } } -func decodeProvenance(provenance string) (charm.DownloadProvenance, error) { +func decodeProvenance(provenance string) (charm.Provenance, error) { switch provenance { case "download": return charm.ProvenanceDownload, nil diff --git a/domain/application/state/types.go b/domain/application/state/types.go index 79fda9100e3..18fc9256e8c 100644 --- a/domain/application/state/types.go +++ b/domain/application/state/types.go @@ -255,7 +255,7 @@ type setCharmState struct { // charmDownloadInfo is used to get the download info of a charm. type charmDownloadInfo struct { - DownloadProvenance string `db:"name"` + Provenance string `db:"name"` CharmhubIdentifier string `db:"charmhub_identifier"` DownloadURL string `db:"download_url"` DownloadSize int64 `db:"download_size"` @@ -263,11 +263,11 @@ type charmDownloadInfo struct { // setCharmDownloadInfo is used to set the download info of a charm. type setCharmDownloadInfo struct { - CharmUUID string `db:"charm_uuid"` - DownloadProvenanceID int `db:"provenance_id"` - CharmhubIdentifier string `db:"charmhub_identifier"` - DownloadURL string `db:"download_url"` - DownloadSize int64 `db:"download_size"` + CharmUUID string `db:"charm_uuid"` + ProvenanceID int `db:"provenance_id"` + CharmhubIdentifier string `db:"charmhub_identifier"` + DownloadURL string `db:"download_url"` + DownloadSize int64 `db:"download_size"` } // charmMetadata is used to get the metadata of a charm. diff --git a/domain/application/watcher_test.go b/domain/application/watcher_test.go index 629a5221820..d68bf289c60 100644 --- a/domain/application/watcher_test.go +++ b/domain/application/watcher_test.go @@ -75,8 +75,8 @@ func (s *watcherSuite) TestWatchCharm(c *gc.C) { Revision: 1, Architecture: arch.AMD64, DownloadInfo: &charm.DownloadInfo{ - DownloadProvenance: charm.ProvenanceDownload, - DownloadURL: "http://example.com", + Provenance: charm.ProvenanceDownload, + DownloadURL: "http://example.com", }, }) c.Assert(err, jc.ErrorIsNil) @@ -493,8 +493,8 @@ func (s *watcherSuite) createApplicationWithCharmAndStoragePath(c *gc.C, svc *se ReferenceName: name, CharmStoragePath: storagePath, DownloadInfo: &charm.DownloadInfo{ - DownloadProvenance: charm.ProvenanceDownload, - DownloadURL: "http://example.com", + Provenance: charm.ProvenanceDownload, + DownloadURL: "http://example.com", }, }, units...) c.Assert(err, jc.ErrorIsNil) diff --git a/domain/secret/watcher_test.go b/domain/secret/watcher_test.go index 9f8e0c42837..0781f0d0a4a 100644 --- a/domain/secret/watcher_test.go +++ b/domain/secret/watcher_test.go @@ -771,7 +771,7 @@ func (s *watcherSuite) setupUnits(c *gc.C, appName string) { applicationservice.AddApplicationArgs{ ReferenceName: appName, DownloadInfo: &applicationcharm.DownloadInfo{ - DownloadProvenance: applicationcharm.ProvenanceDownload, + Provenance: applicationcharm.ProvenanceDownload, CharmhubIdentifier: "wordpress-1", DownloadURL: "https://example.com/wordpress-1", DownloadSize: 1000, diff --git a/internal/bootstrap/deployer.go b/internal/bootstrap/deployer.go index f8241097300..482e64b30ea 100644 --- a/internal/bootstrap/deployer.go +++ b/internal/bootstrap/deployer.go @@ -396,7 +396,7 @@ func (b *baseDeployer) AddControllerApplication(ctx context.Context, curl string DownloadInfo: &applicationcharm.DownloadInfo{ // We do have all the information we need to download the charm, // we should fill this in with the correct information. - DownloadProvenance: applicationcharm.ProvenanceBootstrap, + Provenance: applicationcharm.ProvenanceBootstrap, }, }, applicationservice.AddUnitArg{UnitName: unitName}, diff --git a/internal/bootstrap/deployer_test.go b/internal/bootstrap/deployer_test.go index 4f6a4f1ef79..7bee6a03d9d 100644 --- a/internal/bootstrap/deployer_test.go +++ b/internal/bootstrap/deployer_test.go @@ -284,7 +284,7 @@ func (s *deployerSuite) TestAddControllerApplication(c *gc.C) { applicationservice.AddApplicationArgs{ ReferenceName: bootstrap.ControllerApplicationName, DownloadInfo: &applicationcharm.DownloadInfo{ - DownloadProvenance: applicationcharm.ProvenanceBootstrap, + Provenance: applicationcharm.ProvenanceBootstrap, }, }, applicationservice.AddUnitArg{UnitName: unitName}, diff --git a/internal/testing/factory/factory.go b/internal/testing/factory/factory.go index 75cb870d132..7fa798f9d4a 100644 --- a/internal/testing/factory/factory.go +++ b/internal/testing/factory/factory.go @@ -507,7 +507,7 @@ func (factory *Factory) MakeApplicationReturningPassword(c *gc.C, params *Applic ReferenceName: params.Name, Storage: directives, DownloadInfo: &applicationcharm.DownloadInfo{ - DownloadProvenance: applicationcharm.ProvenanceUpload, + Provenance: applicationcharm.ProvenanceUpload, }, }) } @@ -612,7 +612,7 @@ func (factory *Factory) MakeUnitReturningPassword(c *gc.C, params *UnitParams) ( ReferenceName: params.Application.Name(), Storage: directives, DownloadInfo: &applicationcharm.DownloadInfo{ - DownloadProvenance: applicationcharm.ProvenanceUpload, + Provenance: applicationcharm.ProvenanceUpload, }, }) if !errors.Is(err, applicationerrors.ApplicationAlreadyExists) {