From 2a7a59eb8ce12c14a3531bc804fa76a6d3192413 Mon Sep 17 00:00:00 2001 From: Guillaume Fouillet Date: Thu, 20 Feb 2025 15:56:40 +0100 Subject: [PATCH] feat(resource): stop updating origin and revision through StoreResources Since the new way to use resources tables is to add a resource and move association through application_resource and unit_resource table, StoreResources doesn't need to update origin or revision anymore. It will always upload a resource with valid origin and revision. So this commit remove those fields from all layers in the call stack. The only caveat is in resourcemigration handler, we check the compliance between query parameters and actual stored resources regarding origin and revision to avoid storing the wrong data for the wrong resource in DB (the UUID is queried from application and name in the handler and can be subject of data race) --- .../handlers/resources/resourcemigration.go | 23 ++++- .../resources/resourcemigration_test.go | 50 ++++++---- .../internal/handlers/resources/resources.go | 2 - .../handlers/resources/resources_test.go | 6 -- domain/resource/service/resource.go | 18 ---- domain/resource/service/resource_test.go | 59 ------------ domain/resource/state/resource.go | 5 - domain/resource/state/resource_test.go | 93 ------------------- domain/resource/types.go | 8 -- internal/resource/opener.go | 6 -- internal/resource/opener_test.go | 2 - 11 files changed, 51 insertions(+), 221 deletions(-) diff --git a/apiserver/internal/handlers/resources/resourcemigration.go b/apiserver/internal/handlers/resources/resourcemigration.go index d82c0089777..50b9de8257f 100644 --- a/apiserver/internal/handlers/resources/resourcemigration.go +++ b/apiserver/internal/handlers/resources/resourcemigration.go @@ -125,12 +125,31 @@ func (h *resourcesMigrationUploadHandler) processPost( return empty, internalerrors.Errorf("extracting resource details from request: %w", err) } + // Check that the resource associated with the UUID has the expected Origin + // and revision, to prevent updating the blob of another revision or origin + // that may have been updated between the start of the http call and the + // retrieval of resource UUID from application name and resource name. + res, err := resourceService.GetResource(ctx, resUUID) + if err != nil { + return empty, internalerrors.Errorf("getting resource %s on application %s: %w", appName, + resourceName, err) + } + if res.Origin != details.origin || (res.Origin != charmresource.OriginUpload && res.Revision != details.revision) { + // In this case, we won't interrupt migration, because the user will + // be able to recover after the migration. However, we log an error in + // order to make it notice something get wrong and why. + h.logger.Errorf(ctx, "resource upload failed: resource %s on application %s has unexpected origin or revision: %s != %s, %d != %d", + appName, resourceName, res.Origin, details.origin, res.Revision, details.revision, + ) + return res, nil + } + retrievedBy, retrievedByType := determineRetrievedBy(query) + reader, err := h.downloader.Download(r.Context(), r.Body, details.fingerprint.String(), details.size) if err != nil { return empty, internalerrors.Errorf("validating resource size and hash: %w", err) } - retrievedBy, retrievedByType := determineRetrievedBy(query) err = resourceService.StoreResource(ctx, resource.StoreResourceArgs{ ResourceUUID: resUUID, Reader: reader, @@ -138,8 +157,6 @@ func (h *resourcesMigrationUploadHandler) processPost( RetrievedByType: retrievedByType, Size: details.size, Fingerprint: details.fingerprint, - Origin: details.origin, - Revision: details.revision, }) if err != nil { return empty, internalerrors.Capture(err) diff --git a/apiserver/internal/handlers/resources/resourcemigration_test.go b/apiserver/internal/handlers/resources/resourcemigration_test.go index 4340a25b366..04a5b732fa7 100644 --- a/apiserver/internal/handlers/resources/resourcemigration_test.go +++ b/apiserver/internal/handlers/resources/resourcemigration_test.go @@ -163,6 +163,13 @@ func (s *resourcesUploadSuite) TestServeUploadApplicationStoreResourceError(c *g ).Return("res-uuid", nil) s.downloader.EXPECT().Download(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()) s.resourceService.EXPECT().StoreResource(gomock.Any(), gomock.Any()).Return(errors.New("cannot store resource")) + s.resourceService.EXPECT().GetResource(gomock.Any(), resource.UUID("res-uuid")).Return(resource.Resource{ + UUID: "res-uuid", + Resource: charmresource.Resource{ + Origin: s.origin, + Revision: s.revision, + }, + }, nil) // Act response, err := http.Post(s.srv.URL+migrateResourcesPrefix+"?"+query.Encode(), "application/octet-stream", nil) @@ -192,8 +199,6 @@ func (s *resourcesUploadSuite) TestServeUploadApplicationGetResourceError(c *gc. "app-name", "resource-name", ).Return("res-uuid", nil) - s.downloader.EXPECT().Download(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()) - s.resourceService.EXPECT().StoreResource(gomock.Any(), gomock.Any()).Return(nil) s.resourceService.EXPECT().GetResource(gomock.Any(), gomock.Any()).Return(resource.Resource{}, errors.New( "cannot get resource")) @@ -257,15 +262,17 @@ func (s *resourcesUploadSuite) TestServeUploadApplication(c *gc.C) { ResourceUUID: "res-uuid", Reader: http.NoBody, RetrievedByType: resource.Application, - Origin: s.origin, - Revision: s.revision, Fingerprint: s.fingerprint, Size: s.size, }).Return(nil) s.resourceService.EXPECT().GetResource(gomock.Any(), resource.UUID("res-uuid")).Return(resource.Resource{ - UUID: "res-uuid", + UUID: "res-uuid", + Resource: charmresource.Resource{ + Origin: s.origin, + Revision: s.revision, + }, Timestamp: now, - }, nil) + }, nil).Times(2) // Act response, err := http.Post(s.srv.URL+migrateResourcesPrefix+"?"+query.Encode(), "application/octet-stream", http.NoBody) @@ -299,7 +306,6 @@ func (s *resourcesUploadSuite) TestServeUploadApplicationRetrievedByUser(c *gc.C "origin": {"upload"}, "size": {s.sizeStr}, "fingerprint": {s.fingerprintStr}, - "revision": {s.revisionStr}, } s.resourceService.EXPECT().GetResourceUUIDByApplicationAndResourceName( gomock.Any(), @@ -317,15 +323,17 @@ func (s *resourcesUploadSuite) TestServeUploadApplicationRetrievedByUser(c *gc.C Reader: http.NoBody, RetrievedByType: resource.User, RetrievedBy: "username", - Origin: charmresource.OriginUpload, - Revision: -1, Fingerprint: s.fingerprint, Size: s.size, }).Return(nil) s.resourceService.EXPECT().GetResource(gomock.Any(), resource.UUID("res-uuid")).Return(resource.Resource{ - UUID: "res-uuid", + UUID: "res-uuid", + Resource: charmresource.Resource{ + Origin: charmresource.OriginUpload, + Revision: -1, + }, Timestamp: now, - }, nil) + }, nil).Times(2) // Act _, err := http.Post(s.srv.URL+migrateResourcesPrefix+"?"+query.Encode(), "application/octet-stream", http.NoBody) @@ -365,15 +373,17 @@ func (s *resourcesUploadSuite) TestServeUploadApplicationRetrievedByApplication( Reader: http.NoBody, RetrievedByType: resource.Application, RetrievedBy: "app-name", - Origin: s.origin, - Revision: s.revision, Fingerprint: s.fingerprint, Size: s.size, }).Return(nil) s.resourceService.EXPECT().GetResource(gomock.Any(), resource.UUID("res-uuid")).Return(resource.Resource{ - UUID: "res-uuid", + UUID: "res-uuid", + Resource: charmresource.Resource{ + Origin: s.origin, + Revision: s.revision, + }, Timestamp: now, - }, nil) + }, nil).Times(2) // Act _, err := http.Post(s.srv.URL+migrateResourcesPrefix+"?"+query.Encode(), "application/octet-stream", http.NoBody) @@ -412,15 +422,17 @@ func (s *resourcesUploadSuite) TestServeUploadApplicationRetrievedByUnit(c *gc.C Reader: http.NoBody, RetrievedByType: resource.Unit, RetrievedBy: "app-name/0", - Origin: s.origin, - Revision: s.revision, Fingerprint: s.fingerprint, Size: s.size, }).Return(nil) s.resourceService.EXPECT().GetResource(gomock.Any(), resource.UUID("res-uuid")).Return(resource.Resource{ - UUID: "res-uuid", + UUID: "res-uuid", + Resource: charmresource.Resource{ + Origin: s.origin, + Revision: s.revision, + }, Timestamp: now, - }, nil) + }, nil).Times(2) // Act _, err := http.Post(s.srv.URL+migrateResourcesPrefix+"?"+query.Encode(), "application/octet-stream", http.NoBody) diff --git a/apiserver/internal/handlers/resources/resources.go b/apiserver/internal/handlers/resources/resources.go index 277f0ac1b98..ab088af3b31 100644 --- a/apiserver/internal/handlers/resources/resources.go +++ b/apiserver/internal/handlers/resources/resources.go @@ -167,8 +167,6 @@ func (h *ResourceHandler) upload(service ResourceService, req *http.Request, use RetrievedByType: coreresource.User, Size: uploaded.size, Fingerprint: uploaded.fingerprint, - Origin: charmresource.OriginUpload, - Revision: -1, }, ) if err != nil { diff --git a/apiserver/internal/handlers/resources/resources_test.go b/apiserver/internal/handlers/resources/resources_test.go index 1724379526e..b65a3863aa0 100644 --- a/apiserver/internal/handlers/resources/resources_test.go +++ b/apiserver/internal/handlers/resources/resources_test.go @@ -262,8 +262,6 @@ func (s *ResourcesHandlerSuite) TestPutSuccess(c *gc.C) { RetrievedByType: coreresource.User, Size: s.resource.Size, Fingerprint: s.resource.Fingerprint, - Origin: charmresource.OriginUpload, - Revision: -1, }) // Second call to GetResource gets resource details after upload. @@ -347,8 +345,6 @@ func (s *ResourcesHandlerSuite) TestPutSuccessDockerResource(c *gc.C) { RetrievedByType: coreresource.User, Size: s.resource.Size, Fingerprint: s.resource.Fingerprint, - Origin: charmresource.OriginUpload, - Revision: -1, }) // Second call to GetResource gets resource details after upload. @@ -433,8 +429,6 @@ func (s *ResourcesHandlerSuite) TestPutWithPending(c *gc.C) { RetrievedByType: coreresource.User, Size: s.resource.Size, Fingerprint: s.resource.Fingerprint, - Origin: charmresource.OriginUpload, - Revision: -1, }) // Second call to GetResource gets resource details after upload. diff --git a/domain/resource/service/resource.go b/domain/resource/service/resource.go index d360f6449a2..9aa41e6b757 100644 --- a/domain/resource/service/resource.go +++ b/domain/resource/service/resource.go @@ -387,22 +387,6 @@ func (s *Service) storeResource( return resourceerrors.RetrievedByTypeNotValid } - if err := args.Origin.Validate(); err != nil { - return errors.Errorf("resource origin: %w", err) - } - if args.Origin == charmresource.OriginUpload && args.Revision != -1 { - return errors.Errorf( - "resource with origin upload must have positive -1, found %d: %w", - args.Revision, resourceerrors.ResourceRevisionNotValid, - ) - } - if args.Origin == charmresource.OriginStore && args.Revision < 0 { - return errors.Errorf( - "resource with origin store must have positive revision, found %d, %w", - args.Revision, resourceerrors.ResourceRevisionNotValid, - ) - } - res, err := s.st.GetResource(ctx, args.ResourceUUID) if err != nil { return errors.Errorf("getting resource: %w", err) @@ -448,8 +432,6 @@ func (s *Service) storeResource( IncrementCharmModifiedVersion: incrementCharmModifiedVersion, Size: args.Size, SHA384: args.Fingerprint.String(), - Origin: args.Origin, - Revision: args.Revision, }, ) if err != nil { diff --git a/domain/resource/service/resource_test.go b/domain/resource/service/resource_test.go index ee69fb78973..9321660396e 100644 --- a/domain/resource/service/resource_test.go +++ b/domain/resource/service/resource_test.go @@ -305,9 +305,6 @@ func (s *resourceServiceSuite) TestStoreResource(c *gc.C) { retrievedBy := "bob" retrievedByType := coreresource.User - origin := charmresource.OriginStore - revision := 17 - storageID := storetesting.GenFileResourceStoreID(c, objectstoretesting.GenObjectStoreUUID(c)) s.state.EXPECT().GetResource(gomock.Any(), resourceUUID).Return( coreresource.Resource{ @@ -335,8 +332,6 @@ func (s *resourceServiceSuite) TestStoreResource(c *gc.C) { IncrementCharmModifiedVersion: false, Size: size, SHA384: fp.String(), - Origin: origin, - Revision: revision, }) err = s.service.StoreResource( @@ -348,8 +343,6 @@ func (s *resourceServiceSuite) TestStoreResource(c *gc.C) { RetrievedByType: retrievedByType, Size: size, Fingerprint: fp, - Origin: origin, - Revision: revision, }, ) c.Assert(err, jc.ErrorIsNil) @@ -369,9 +362,6 @@ func (s *resourceServiceSuite) TestStoreResourceRemovedOnRecordError(c *gc.C) { retrievedBy := "bob" retrievedByType := coreresource.User - origin := charmresource.OriginStore - revision := 17 - storageID := storetesting.GenFileResourceStoreID(c, objectstoretesting.GenObjectStoreUUID(c)) s.state.EXPECT().GetResource(gomock.Any(), resourceUUID).Return( coreresource.Resource{ @@ -402,8 +392,6 @@ func (s *resourceServiceSuite) TestStoreResourceRemovedOnRecordError(c *gc.C) { IncrementCharmModifiedVersion: false, Size: size, SHA384: fp.String(), - Origin: origin, - Revision: revision, }).Return("", expectedErr) // Expect the removal of the resource. @@ -418,8 +406,6 @@ func (s *resourceServiceSuite) TestStoreResourceRemovedOnRecordError(c *gc.C) { RetrievedByType: retrievedByType, Size: size, Fingerprint: fp, - Origin: origin, - Revision: revision, }, ) c.Assert(err, jc.ErrorIs, expectedErr) @@ -439,9 +425,6 @@ func (s *resourceServiceSuite) TestStoreResourceRemovedOldResourceBlob(c *gc.C) retrievedBy := "bob" retrievedByType := coreresource.User - origin := charmresource.OriginStore - revision := 17 - storageID := storetesting.GenFileResourceStoreID(c, objectstoretesting.GenObjectStoreUUID(c)) s.state.EXPECT().GetResource(gomock.Any(), resourceUUID).Return( coreresource.Resource{ @@ -473,8 +456,6 @@ func (s *resourceServiceSuite) TestStoreResourceRemovedOldResourceBlob(c *gc.C) IncrementCharmModifiedVersion: false, Size: size, SHA384: fp.String(), - Origin: origin, - Revision: revision, }).Return(droppedFingerprint, nil) // Expect the removal of the resource. @@ -489,8 +470,6 @@ func (s *resourceServiceSuite) TestStoreResourceRemovedOldResourceBlob(c *gc.C) RetrievedByType: retrievedByType, Size: size, Fingerprint: fp, - Origin: origin, - Revision: revision, }, ) c.Assert(err, jc.ErrorIsNil) @@ -506,9 +485,6 @@ func (s *resourceServiceSuite) TestStoreResourceDoesNotStoreIdenticalBlobContain fp, err := charmresource.NewFingerprint(fingerprint) c.Assert(err, jc.ErrorIsNil) - origin := charmresource.OriginStore - revision := 17 - s.state.EXPECT().GetResource(gomock.Any(), resourceUUID).Return( coreresource.Resource{ Resource: charmresource.Resource{ @@ -536,8 +512,6 @@ func (s *resourceServiceSuite) TestStoreResourceDoesNotStoreIdenticalBlobContain ResourceUUID: resourceUUID, Reader: reader, Fingerprint: fp, - Origin: origin, - Revision: revision, }, ) @@ -554,9 +528,6 @@ func (s *resourceServiceSuite) TestStoreResourceDoesNotStoreIdenticalBlobFile(c fp, err := charmresource.NewFingerprint(fingerprint) c.Assert(err, jc.ErrorIsNil) - origin := charmresource.OriginStore - revision := 17 - s.state.EXPECT().GetResource(gomock.Any(), resourceUUID).Return( coreresource.Resource{ Resource: charmresource.Resource{ @@ -584,8 +555,6 @@ func (s *resourceServiceSuite) TestStoreResourceDoesNotStoreIdenticalBlobFile(c ResourceUUID: resourceUUID, Reader: reader, Fingerprint: fp, - Origin: origin, - Revision: revision, }, ) @@ -654,23 +623,6 @@ func (s *resourceServiceSuite) TestStoreResourceBadRetrievedBy(c *gc.C) { c.Assert(err, jc.ErrorIs, resourceerrors.RetrievedByTypeNotValid) } -func (s *resourceServiceSuite) TestStoreResourceOriginNotValid(c *gc.C) { - fp, err := charmresource.NewFingerprint(fingerprint) - c.Assert(err, jc.ErrorIsNil) - err = s.service.StoreResource( - context.Background(), - resource.StoreResourceArgs{ - ResourceUUID: resourcetesting.GenResourceUUID(c), - Reader: bytes.NewBufferString("spam"), - Fingerprint: fp, - RetrievedBy: "bob", - RetrievedByType: coreresource.User, - Origin: charmresource.Origin(0), - }, - ) - c.Assert(err, jc.ErrorIs, errors.NotValid) -} - func (s *resourceServiceSuite) TestStoreResourceRevisionNotValidOriginUpload(c *gc.C) { fp, err := charmresource.NewFingerprint(fingerprint) c.Assert(err, jc.ErrorIsNil) @@ -682,8 +634,6 @@ func (s *resourceServiceSuite) TestStoreResourceRevisionNotValidOriginUpload(c * Fingerprint: fp, RetrievedBy: "bob", RetrievedByType: coreresource.User, - Origin: charmresource.OriginUpload, - Revision: 17, }, ) c.Assert(err, jc.ErrorIs, resourceerrors.ResourceRevisionNotValid) @@ -700,8 +650,6 @@ func (s *resourceServiceSuite) TestStoreResourceRevisionNotValidOriginStore(c *g Fingerprint: fp, RetrievedBy: "bob", RetrievedByType: coreresource.User, - Origin: charmresource.OriginStore, - Revision: -1, }, ) c.Assert(err, jc.ErrorIs, resourceerrors.ResourceRevisionNotValid) @@ -721,9 +669,6 @@ func (s *resourceServiceSuite) TestStoreResourceAndIncrementCharmModifiedVersion retrievedBy := "bob" retrievedByType := coreresource.User - origin := charmresource.OriginStore - revision := 17 - storageID := storetesting.GenFileResourceStoreID(c, objectstoretesting.GenObjectStoreUUID(c)) s.state.EXPECT().GetResource(gomock.Any(), resourceUUID).Return( coreresource.Resource{ @@ -751,8 +696,6 @@ func (s *resourceServiceSuite) TestStoreResourceAndIncrementCharmModifiedVersion IncrementCharmModifiedVersion: true, Size: size, SHA384: fp.String(), - Origin: origin, - Revision: revision, }) err = s.service.StoreResourceAndIncrementCharmModifiedVersion( @@ -764,8 +707,6 @@ func (s *resourceServiceSuite) TestStoreResourceAndIncrementCharmModifiedVersion Fingerprint: fp, RetrievedBy: retrievedBy, RetrievedByType: retrievedByType, - Origin: origin, - Revision: revision, }, ) c.Assert(err, jc.ErrorIsNil) diff --git a/domain/resource/state/resource.go b/domain/resource/state/resource.go index 1b46c154f27..a54deafdb5f 100644 --- a/domain/resource/state/resource.go +++ b/domain/resource/state/resource.go @@ -683,11 +683,6 @@ func (st *State) RecordStoredResource( return errors.Errorf("unknown resource type: %q", args.ResourceType.String()) } - err := st.updateOriginAndRevision(ctx, tx, args.ResourceUUID, args.Origin, args.Revision) - if err != nil { - return errors.Errorf("updating resource revision and origin: %w", err) - } - if args.RetrievedBy != "" { err := st.upsertRetrievedBy(ctx, tx, args.ResourceUUID, args.RetrievedBy, args.RetrievedByType) if err != nil { diff --git a/domain/resource/state/resource_test.go b/domain/resource/state/resource_test.go index ffd6b874f48..944a30f6f44 100644 --- a/domain/resource/state/resource_test.go +++ b/domain/resource/state/resource_test.go @@ -847,7 +847,6 @@ func (s *resourceSuite) TestRecordStoredResourceWithContainerImage(c *gc.C) { ResourceType: charmresource.TypeContainerImage, Size: size, SHA384: hash, - Origin: charmresource.OriginStore, }, ) c.Assert(err, jc.ErrorIsNil, gc.Commentf("(Act) failed to execute RecordStoredResource: %v", errors.ErrorStack(err))) @@ -903,7 +902,6 @@ func (s *resourceSuite) TestRecordStoredResourceWithFile(c *gc.C) { ResourceType: charmresource.TypeFile, Size: size, SHA384: hash, - Origin: charmresource.OriginStore, }, ) c.Assert(err, jc.ErrorIsNil, gc.Commentf("(Act) failed to execute RecordStoredResource: %v", errors.ErrorStack(err))) @@ -960,7 +958,6 @@ func (s *resourceSuite) TestRecordStoredResourceIncrementCharmModifiedVersion(c IncrementCharmModifiedVersion: true, SHA384: hash, Size: size, - Origin: charmresource.OriginStore, }, ) c.Assert(err, jc.ErrorIsNil, gc.Commentf("(Act) failed to execute RecordStoredResource: %v", errors.ErrorStack(err))) @@ -976,7 +973,6 @@ func (s *resourceSuite) TestRecordStoredResourceIncrementCharmModifiedVersion(c IncrementCharmModifiedVersion: true, SHA384: hash2, Size: size2, - Origin: charmresource.OriginStore, }, ) c.Assert(err, jc.ErrorIsNil, gc.Commentf("(Act) failed to execute RecordStoredResource: %v", errors.ErrorStack(err))) @@ -1005,7 +1001,6 @@ func (s *resourceSuite) TestRecordStoredResourceDoNotIncrementCharmModifiedVersi ResourceType: charmresource.TypeContainerImage, SHA384: hash, Size: size, - Origin: charmresource.OriginStore, }, ) c.Assert(err, jc.ErrorIsNil, gc.Commentf("(Act) failed to execute RecordStoredResource: %v", errors.ErrorStack(err))) @@ -1032,7 +1027,6 @@ func (s *resourceSuite) TestRecordStoredResourceWithContainerImageAlreadyStored( Size: size1, RetrievedBy: retrievedBy1, RetrievedByType: retrievedByType1, - Origin: charmresource.OriginStore, }, ) c.Assert(err, jc.ErrorIsNil, gc.Commentf("(Arrange) failed to execute RecordStoredResource: %v", errors.ErrorStack(err))) @@ -1058,7 +1052,6 @@ func (s *resourceSuite) TestRecordStoredResourceWithContainerImageAlreadyStored( Size: size2, RetrievedBy: retrievedBy2, RetrievedByType: retrievedByType2, - Origin: charmresource.OriginStore, }, ) // Assert: Check the hash of the first blob was returned and changed to the @@ -1099,7 +1092,6 @@ func (s *resourceSuite) TestStoreWithFileResourceAlreadyStored(c *gc.C) { RetrievedByType: retrievedByType1, SHA384: hash1, Size: size1, - Origin: charmresource.OriginStore, }, ) c.Assert(err, jc.ErrorIsNil, gc.Commentf("(Arrange) failed to execute RecordStoredResource: %v", errors.ErrorStack(err))) @@ -1125,7 +1117,6 @@ func (s *resourceSuite) TestStoreWithFileResourceAlreadyStored(c *gc.C) { RetrievedByType: retrievedByType2, SHA384: hash2, Size: size2, - Origin: charmresource.OriginStore, }, ) // Assert: Check the hash of the first blob was returned and changed to the @@ -1162,7 +1153,6 @@ func (s *resourceSuite) TestRecordStoredResourceSameBlobAlreadyStoredContainerIm ResourceType: charmresource.TypeContainerImage, SHA384: hash, Size: size, - Origin: charmresource.OriginStore, }, ) c.Assert(err, jc.ErrorIsNil, gc.Commentf("(Act) failed to execute RecordStoredResource: %v", errors.ErrorStack(err))) @@ -1177,7 +1167,6 @@ func (s *resourceSuite) TestRecordStoredResourceSameBlobAlreadyStoredContainerIm ResourceType: charmresource.TypeContainerImage, SHA384: hash, Size: size, - Origin: charmresource.OriginStore, }, ) // Assert: That when record a blob twice, the second recording does not @@ -1199,7 +1188,6 @@ func (s *resourceSuite) TestRecordStoredResourceSameBlobAlreadyStoredFile(c *gc. ResourceType: charmresource.TypeFile, SHA384: hash, Size: size, - Origin: charmresource.OriginStore, }, ) c.Assert(err, jc.ErrorIsNil, gc.Commentf("(Act) failed to execute RecordStoredResource: %v", errors.ErrorStack(err))) @@ -1213,8 +1201,6 @@ func (s *resourceSuite) TestRecordStoredResourceSameBlobAlreadyStoredFile(c *gc. StorageID: storeID, ResourceType: charmresource.TypeFile, SHA384: hash, - Size: size, - Origin: charmresource.OriginStore, }, ) // Assert: That when record a blob twice, the second recording does not @@ -1223,84 +1209,6 @@ func (s *resourceSuite) TestRecordStoredResourceSameBlobAlreadyStoredFile(c *gc. c.Check(droppedHash2, gc.Equals, "") } -// TestRecordStoredResourceOriginAndRevision checks that resource origin and -// revision are set correctly. -func (s *resourceSuite) TestRecordStoredResourceStoreOriginAndRevision(c *gc.C) { - // Arrange: Create file resource. - resID, storeID, _, _ := s.createFileResourceAndBlob(c) - origin := charmresource.OriginStore - revision := 8 - - // Act: store the resource blob. - _, err := s.state.RecordStoredResource( - context.Background(), - resource.RecordStoredResourceArgs{ - ResourceUUID: resID, - StorageID: storeID, - ResourceType: charmresource.TypeFile, - Origin: origin, - Revision: revision, - }, - ) - c.Assert(err, jc.ErrorIsNil, gc.Commentf("(Act) failed to execute RecordStoredResource: %v", errors.ErrorStack(err))) - - // Assert: Check that the origin and revision have been set. - var ( - foundOrigin string - foundRevision int - ) - err = s.TxnRunner().StdTxn(context.Background(), func(ctx context.Context, tx *sql.Tx) error { - return tx.QueryRow(` -SELECT rot.name, r.revision -FROM resource r -JOIN resource_origin_type rot ON r.origin_type_id = rot.id -WHERE r.uuid = ?`, resID).Scan(&foundOrigin, &foundRevision) - }) - c.Assert(err, jc.ErrorIsNil, gc.Commentf("(Assert) origin and revision in resource table not updated: %v", errors.ErrorStack(err))) - c.Check(foundOrigin, gc.Equals, origin.String()) - c.Check(foundRevision, gc.Equals, revision) -} - -// TestRecordStoredResourceOriginAndRevision checks that resource origin and -// revision are set correctly. -func (s *resourceSuite) TestRecordStoredResourceUpdateOriginAndRevision(c *gc.C) { - // Arrange: Create file resource. - resID, storeID, _, _ := s.createFileResourceAndBlob(c) - origin1 := charmresource.OriginStore - revision1 := 8 - - origin2 := charmresource.OriginUpload - revision2 := -1 - - // Arrange: Store the first origin and revision. - _, err := s.state.RecordStoredResource( - context.Background(), - resource.RecordStoredResourceArgs{ - ResourceUUID: resID, - StorageID: storeID, - ResourceType: charmresource.TypeFile, - Origin: origin1, - Revision: revision1, - }, - ) - c.Assert(err, jc.ErrorIsNil, gc.Commentf("(Arrange) failed to execute RecordStoredResource: %v", errors.ErrorStack(err))) - - // Act: store the second origin and revision - _, err = s.state.RecordStoredResource( - context.Background(), - resource.RecordStoredResourceArgs{ - ResourceUUID: resID, - StorageID: storeID, - ResourceType: charmresource.TypeFile, - Origin: origin2, - Revision: revision2, - }, - ) - c.Assert(err, jc.ErrorIsNil, gc.Commentf("(Act) failed to execute RecordStoredResource: %v", errors.ErrorStack(err))) - - s.checkResourceOriginAndRevision(c, resID.String(), origin2.String(), revision2) -} - func (s *resourceSuite) TestRecordStoredResourceFileStoredResourceNotFoundInObjectStore(c *gc.C) { // Arrange: insert a resource. resID := s.addResource(c, charmresource.TypeFile) @@ -3094,7 +3002,6 @@ func (s *resourceSuite) setWithRetrievedBy( ResourceType: charmresource.TypeFile, RetrievedBy: retrievedBy, RetrievedByType: retrievedByType, - Origin: charmresource.OriginStore, }, ) return err diff --git a/domain/resource/types.go b/domain/resource/types.go index 5f6b72e9724..0a32e53f857 100644 --- a/domain/resource/types.go +++ b/domain/resource/types.go @@ -69,10 +69,6 @@ type StoreResourceArgs struct { Size int64 // Fingerprint is the hash of the resource blob. Fingerprint charmresource.Fingerprint - // Origin is where the resource blob comes from. - Origin charmresource.Origin - // Revision indicates the resource revision. - Revision int } // RecordStoredResourceArgs holds the arguments for record stored resource state @@ -97,10 +93,6 @@ type RecordStoredResourceArgs struct { Size int64 // SHA384 is the hash of the resource blob. SHA384 string - // Origin is where the resource blob comes from. - Origin charmresource.Origin - // Revision indicates the resource revision. - Revision int } // AddResourcesBeforeApplicationArgs holds arguments to indicate a resources revision or upload diff --git a/internal/resource/opener.go b/internal/resource/opener.go index e61d175f94e..201e75a7a75 100644 --- a/internal/resource/opener.go +++ b/internal/resource/opener.go @@ -311,8 +311,6 @@ func (ro ResourceOpener) getResource( data.ReadCloser, data.Resource.Size, data.Resource.Fingerprint, - data.Resource.Origin, - data.Resource.Revision, ) if err != nil { return coreresource.Opened{}, errors.Capture(err) @@ -338,8 +336,6 @@ func (ro ResourceOpener) store( reader io.Reader, size int64, fingerprint charmresource.Fingerprint, - origin charmresource.Origin, - revision int, ) (_ coreresource.Resource, _ io.ReadCloser, err error) { err = ro.resourceService.StoreResource( ctx, resource.StoreResourceArgs{ @@ -349,8 +345,6 @@ func (ro ResourceOpener) store( Fingerprint: fingerprint, RetrievedBy: ro.retrievedBy, RetrievedByType: ro.retrievedByType, - Origin: origin, - Revision: revision, }, ) if err != nil { diff --git a/internal/resource/opener_test.go b/internal/resource/opener_test.go index b94376ee5fd..fda003d69c2 100644 --- a/internal/resource/opener_test.go +++ b/internal/resource/opener_test.go @@ -311,8 +311,6 @@ func (s *OpenerSuite) expectServiceMethods( RetrievedByType: retrevedByType, Size: s.resourceSize, Fingerprint: s.resourceFingerprint, - Origin: charmresource.OriginStore, - Revision: s.resourceRevision, }, )