diff --git a/cmd/kyma/alpha/create/module/module.go b/cmd/kyma/alpha/create/module/module.go index b7a7cd9b0..5f4580232 100644 --- a/cmd/kyma/alpha/create/module/module.go +++ b/cmd/kyma/alpha/create/module/module.go @@ -479,11 +479,12 @@ func (cmd *command) validateDefaultCR(ctx context.Context, modDef *module.Defini func (cmd *command) getRemote(nameMapping module.NameMapping) (*module.Remote, error) { res := &module.Remote{ - Registry: cmd.opts.RegistryURL, - NameMapping: nameMapping, - Credentials: cmd.opts.Credentials, - Token: cmd.opts.Token, - Insecure: cmd.opts.Insecure, + Registry: cmd.opts.RegistryURL, + NameMapping: nameMapping, + Credentials: cmd.opts.Credentials, + Token: cmd.opts.Token, + Insecure: cmd.opts.Insecure, + OciRepoAccess: &module.OciRepo{}, } if strings.HasPrefix(strings.ToLower(cmd.opts.RegistryURL), "https:") { diff --git a/cmd/kyma/alpha/verify/module/module.go b/cmd/kyma/alpha/verify/module/module.go index 71560b1af..2363ec91b 100644 --- a/cmd/kyma/alpha/verify/module/module.go +++ b/cmd/kyma/alpha/verify/module/module.go @@ -1,9 +1,10 @@ package module import ( + "github.com/spf13/cobra" + "github.com/kyma-project/cli/internal/cli" "github.com/kyma-project/cli/pkg/module" - "github.com/spf13/cobra" ) type command struct { @@ -80,11 +81,12 @@ func (c *command) Run(_ []string) error { } remote := &module.Remote{ - Registry: c.opts.RegistryURL, - NameMapping: nameMappingMode, - Credentials: c.opts.Credentials, - Token: c.opts.Token, - Insecure: c.opts.Insecure, + Registry: c.opts.RegistryURL, + NameMapping: nameMappingMode, + Credentials: c.opts.Credentials, + Token: c.opts.Token, + Insecure: c.opts.Insecure, + OciRepoAccess: &module.OciRepo{}, } if err := module.Verify(signCfg, remote); err != nil { diff --git a/pkg/module/mocks/OciRepoAccess.go b/pkg/module/mocks/OciRepoAccess.go index 19d4855b4..b43be64ba 100644 --- a/pkg/module/mocks/OciRepoAccess.go +++ b/pkg/module/mocks/OciRepoAccess.go @@ -3,10 +3,10 @@ package mocks import ( - internal "github.com/open-component-model/ocm/pkg/contexts/ocm/cpi" comparch "github.com/open-component-model/ocm/pkg/contexts/ocm/repositories/comparch" - mock "github.com/stretchr/testify/mock" + + ocm "github.com/open-component-model/ocm/pkg/contexts/ocm" ) // OciRepoAccess is an autogenerated mock type for the OciRepoAccess type @@ -15,7 +15,7 @@ type OciRepoAccess struct { } // ComponentVersionExists provides a mock function with given fields: archive, repo -func (_m *OciRepoAccess) ComponentVersionExists(archive *comparch.ComponentArchive, repo internal.Repository) (bool, error) { +func (_m *OciRepoAccess) ComponentVersionExists(archive *comparch.ComponentArchive, repo ocm.Repository) (bool, error) { ret := _m.Called(archive, repo) if len(ret) == 0 { @@ -24,16 +24,16 @@ func (_m *OciRepoAccess) ComponentVersionExists(archive *comparch.ComponentArchi var r0 bool var r1 error - if rf, ok := ret.Get(0).(func(*comparch.ComponentArchive, internal.Repository) (bool, error)); ok { + if rf, ok := ret.Get(0).(func(*comparch.ComponentArchive, ocm.Repository) (bool, error)); ok { return rf(archive, repo) } - if rf, ok := ret.Get(0).(func(*comparch.ComponentArchive, internal.Repository) bool); ok { + if rf, ok := ret.Get(0).(func(*comparch.ComponentArchive, ocm.Repository) bool); ok { r0 = rf(archive, repo) } else { r0 = ret.Get(0).(bool) } - if rf, ok := ret.Get(1).(func(*comparch.ComponentArchive, internal.Repository) error); ok { + if rf, ok := ret.Get(1).(func(*comparch.ComponentArchive, ocm.Repository) error); ok { r1 = rf(archive, repo) } else { r1 = ret.Error(1) @@ -42,28 +42,46 @@ func (_m *OciRepoAccess) ComponentVersionExists(archive *comparch.ComponentArchi return r0, r1 } +// DescriptorResourcesAreEquivalent provides a mock function with given fields: archive, remoteVersion +func (_m *OciRepoAccess) DescriptorResourcesAreEquivalent(archive *comparch.ComponentArchive, remoteVersion ocm.ComponentVersionAccess) bool { + ret := _m.Called(archive, remoteVersion) + + if len(ret) == 0 { + panic("no return value specified for DescriptorResourcesAreEquivalent") + } + + var r0 bool + if rf, ok := ret.Get(0).(func(*comparch.ComponentArchive, ocm.ComponentVersionAccess) bool); ok { + r0 = rf(archive, remoteVersion) + } else { + r0 = ret.Get(0).(bool) + } + + return r0 +} + // GetComponentVersion provides a mock function with given fields: archive, repo -func (_m *OciRepoAccess) GetComponentVersion(archive *comparch.ComponentArchive, repo internal.Repository) (internal.ComponentVersionAccess, error) { +func (_m *OciRepoAccess) GetComponentVersion(archive *comparch.ComponentArchive, repo ocm.Repository) (ocm.ComponentVersionAccess, error) { ret := _m.Called(archive, repo) if len(ret) == 0 { panic("no return value specified for GetComponentVersion") } - var r0 internal.ComponentVersionAccess + var r0 ocm.ComponentVersionAccess var r1 error - if rf, ok := ret.Get(0).(func(*comparch.ComponentArchive, internal.Repository) (internal.ComponentVersionAccess, error)); ok { + if rf, ok := ret.Get(0).(func(*comparch.ComponentArchive, ocm.Repository) (ocm.ComponentVersionAccess, error)); ok { return rf(archive, repo) } - if rf, ok := ret.Get(0).(func(*comparch.ComponentArchive, internal.Repository) internal.ComponentVersionAccess); ok { + if rf, ok := ret.Get(0).(func(*comparch.ComponentArchive, ocm.Repository) ocm.ComponentVersionAccess); ok { r0 = rf(archive, repo) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(internal.ComponentVersionAccess) + r0 = ret.Get(0).(ocm.ComponentVersionAccess) } } - if rf, ok := ret.Get(1).(func(*comparch.ComponentArchive, internal.Repository) error); ok { + if rf, ok := ret.Get(1).(func(*comparch.ComponentArchive, ocm.Repository) error); ok { r1 = rf(archive, repo) } else { r1 = ret.Error(1) @@ -73,7 +91,7 @@ func (_m *OciRepoAccess) GetComponentVersion(archive *comparch.ComponentArchive, } // PushComponentVersion provides a mock function with given fields: archive, repository, overwrite -func (_m *OciRepoAccess) PushComponentVersion(archive *comparch.ComponentArchive, repository internal.Repository, overwrite bool) error { +func (_m *OciRepoAccess) PushComponentVersion(archive *comparch.ComponentArchive, repository ocm.Repository, overwrite bool) error { ret := _m.Called(archive, repository, overwrite) if len(ret) == 0 { @@ -81,7 +99,7 @@ func (_m *OciRepoAccess) PushComponentVersion(archive *comparch.ComponentArchive } var r0 error - if rf, ok := ret.Get(0).(func(*comparch.ComponentArchive, internal.Repository, bool) error); ok { + if rf, ok := ret.Get(0).(func(*comparch.ComponentArchive, ocm.Repository, bool) error); ok { r0 = rf(archive, repository, overwrite) } else { r0 = ret.Error(0) diff --git a/pkg/module/oci_repo.go b/pkg/module/oci_repo.go index 207ee0b75..035eef614 100644 --- a/pkg/module/oci_repo.go +++ b/pkg/module/oci_repo.go @@ -5,17 +5,21 @@ import ( "github.com/open-component-model/ocm/pkg/common" "github.com/open-component-model/ocm/pkg/contexts/ocm" + "github.com/open-component-model/ocm/pkg/contexts/ocm/accessmethods/localblob" + "github.com/open-component-model/ocm/pkg/contexts/ocm/compdesc" "github.com/open-component-model/ocm/pkg/contexts/ocm/cpi" "github.com/open-component-model/ocm/pkg/contexts/ocm/repositories/comparch" "github.com/open-component-model/ocm/pkg/contexts/ocm/transfer" "github.com/open-component-model/ocm/pkg/contexts/ocm/transfer/transferhandler/standard" + "github.com/open-component-model/ocm/pkg/runtime" ) -//go:generate mockery --name OciRepoAccess --replace-type github.com/open-component-model/ocm/pkg/contexts/ocm/internal=internal:github.com/open-component-model/ocm/pkg/contexts/ocm/cpi +//go:generate mockery --name OciRepoAccess --replace-type github.com/open-component-model/ocm/pkg/contexts/ocm/internal=ocm:github.com/open-component-model/ocm/pkg/contexts/ocm type OciRepoAccess interface { ComponentVersionExists(archive *comparch.ComponentArchive, repo cpi.Repository) (bool, error) GetComponentVersion(archive *comparch.ComponentArchive, repo cpi.Repository) (ocm.ComponentVersionAccess, error) PushComponentVersion(archive *comparch.ComponentArchive, repository cpi.Repository, overwrite bool) error + DescriptorResourcesAreEquivalent(archive *comparch.ComponentArchive, remoteVersion ocm.ComponentVersionAccess) bool } type OciRepo struct{} @@ -45,3 +49,49 @@ func (r *OciRepo) PushComponentVersion(archive *comparch.ComponentArchive, repo } return nil } + +func (r *OciRepo) DescriptorResourcesAreEquivalent(archive *comparch.ComponentArchive, + remoteVersion ocm.ComponentVersionAccess) bool { + localResources := archive.GetDescriptor().Resources + remoteResources := remoteVersion.GetDescriptor().Resources + if len(localResources) != len(remoteResources) { + return false + } + + localResourcesMap := map[string]compdesc.Resource{} + for _, res := range localResources { + localResourcesMap[res.Name] = res + } + + for _, res := range remoteResources { + localResource := localResourcesMap[res.Name] + if res.Name == RawManifestLayerName { + remoteAccess, ok := res.Access.(*runtime.UnstructuredVersionedTypedObject) + if !ok { + return false + } + + _, ok = localResourcesMap[res.Name] + if !ok { + return false + } + localAccessObject, ok := localResource.Access.(*localblob.AccessSpec) + if !ok { + return false + } + + remoteAccessLocalReference, ok := remoteAccess.Object[accessLocalReferenceFieldName].(string) + if !ok { + return false + } + // Trimming 7 characters because locally the sha256 is followed by '.' but remote it is followed by ':' + if remoteAccessLocalReference[7:] != localAccessObject.LocalReference[7:] { + return false + } + } else if !res.IsEquivalent(&localResource) { + return false + } + } + + return true +} diff --git a/pkg/module/remote.go b/pkg/module/remote.go index 931edc2b9..b31fba6e7 100644 --- a/pkg/module/remote.go +++ b/pkg/module/remote.go @@ -11,7 +11,6 @@ import ( "github.com/open-component-model/ocm/pkg/contexts/credentials/repositories/dockerconfig" oci "github.com/open-component-model/ocm/pkg/contexts/oci/repositories/ocireg" "github.com/open-component-model/ocm/pkg/contexts/ocm" - "github.com/open-component-model/ocm/pkg/contexts/ocm/accessmethods/localblob" "github.com/open-component-model/ocm/pkg/contexts/ocm/compdesc" "github.com/open-component-model/ocm/pkg/contexts/ocm/cpi" "github.com/open-component-model/ocm/pkg/contexts/ocm/repositories/comparch" @@ -143,8 +142,7 @@ func (r *Remote) Push(archive *comparch.ComponentArchive, overwrite bool) (ocm.C return nil, false, fmt.Errorf("could not lookup component version: %w", err) } - if descriptorResourcesAreEquivalent(archive.GetDescriptor().Resources, - versionAccess.GetDescriptor().Resources) { + if r.DescriptorResourcesAreEquivalent(archive, versionAccess) { return versionAccess, false, nil } return nil, false, fmt.Errorf("version %s already exists with different content, please use "+ @@ -171,46 +169,3 @@ func (h *customTransferHandler) TransferVersion(repo ocm.Repository, src ocm.Com error) { return h.TransferHandler.TransferVersion(repo, src, meta, tgt) } - -func descriptorResourcesAreEquivalent(localResources, remoteResources compdesc.Resources) bool { - if len(localResources) != len(remoteResources) { - return false - } - - localResourcesMap := map[string]compdesc.Resource{} - for _, res := range localResources { - localResourcesMap[res.Name] = res - } - - for _, res := range remoteResources { - localResource := localResourcesMap[res.Name] - if res.Name == RawManifestLayerName { - remoteAccess, ok := res.Access.(*runtime.UnstructuredVersionedTypedObject) - if !ok { - return false - } - - _, ok = localResourcesMap[res.Name] - if !ok { - return false - } - localAccessObject, ok := localResource.Access.(*localblob.AccessSpec) - if !ok { - return false - } - - remoteAccessLocalReference, ok := remoteAccess.Object[accessLocalReferenceFieldName].(string) - if !ok { - return false - } - // Trimming 7 characters because locally the sha256 is followed by '.' but remote it is followed by ':' - if remoteAccessLocalReference[7:] != localAccessObject.LocalReference[7:] { - return false - } - } else if !res.IsEquivalent(&localResource) { - return false - } - } - - return true -} diff --git a/pkg/module/remote_test.go b/pkg/module/remote_test.go index 2023e6728..e5be9413a 100644 --- a/pkg/module/remote_test.go +++ b/pkg/module/remote_test.go @@ -5,7 +5,6 @@ import ( "testing" "github.com/mandelsoft/vfs/pkg/memoryfs" - "github.com/open-component-model/ocm/pkg/contexts/ocm" "github.com/open-component-model/ocm/pkg/contexts/ocm/compdesc" ocmv1 "github.com/open-component-model/ocm/pkg/contexts/ocm/compdesc/meta/v1" "github.com/open-component-model/ocm/pkg/contexts/ocm/repositories/comparch" @@ -60,25 +59,18 @@ func TestRemote_Push(t *testing.T) { compdesc.DefaultResources(cd) archive, _ := module.CreateArchive(archiveFS, "./mod", cd) - ociRepoAccessMock := mocks.OciRepoAccess{} - ociRepoAccessMock.On("PushComponentVersion", archive, mock.Anything, mock.Anything).Return(nil) - ociRepoAccessMock.On("GetComponentVersion", archive, - mock.Anything).Return(mock.AnythingOfType("internal.ComponentVersionAccess")) - - type fields struct { - OciRepoAccess module.OciRepoAccess - } + var ociRepoAccessMock mocks.OciRepoAccess type args struct { archive *comparch.ComponentArchive overwrite bool } tests := []struct { - name string - fields fields - args args - want ocm.ComponentVersionAccess - want1 bool - wantErr assert.ErrorAssertionFunc + name string + versionAlreadyExists bool + sameContent bool + args args + wantIsPushed bool + assertFn func(err error, i ...interface{}) }{ { name: "Same version, same content, with overwrite flag", @@ -86,38 +78,121 @@ func TestRemote_Push(t *testing.T) { overwrite: true, archive: archive, }, - fields: fields{ - OciRepoAccess: &ociRepoAccessMock, + assertFn: func(err error, i ...interface{}) { + assert.NoError(t, err) + ociRepoAccessMock.AssertNumberOfCalls(t, "ComponentVersionExists", 0) + ociRepoAccessMock.AssertNumberOfCalls(t, "GetComponentVersion", 1) + ociRepoAccessMock.AssertNumberOfCalls(t, "PushComponentVersion", 1) + ociRepoAccessMock.AssertNumberOfCalls(t, "DescriptorResourcesAreEquivalent", 0) }, + wantIsPushed: true, + versionAlreadyExists: false, + sameContent: true, }, { name: "Same version, same content, without overwrite flag", + args: args{ + overwrite: false, + archive: archive, + }, + assertFn: func(err error, i ...interface{}) { + assert.NoError(t, err) + ociRepoAccessMock.AssertNumberOfCalls(t, "ComponentVersionExists", 1) + ociRepoAccessMock.AssertNumberOfCalls(t, "GetComponentVersion", 1) + ociRepoAccessMock.AssertNumberOfCalls(t, "PushComponentVersion", 0) + ociRepoAccessMock.AssertNumberOfCalls(t, "DescriptorResourcesAreEquivalent", 1) + }, + wantIsPushed: false, + versionAlreadyExists: true, + sameContent: true, }, { name: "Same version, different content, with overwrite flag", + args: args{ + overwrite: true, + archive: archive, + }, + assertFn: func(err error, i ...interface{}) { + assert.NoError(t, err) + ociRepoAccessMock.AssertNumberOfCalls(t, "ComponentVersionExists", 0) + ociRepoAccessMock.AssertNumberOfCalls(t, "GetComponentVersion", 1) + ociRepoAccessMock.AssertNumberOfCalls(t, "PushComponentVersion", 1) + ociRepoAccessMock.AssertNumberOfCalls(t, "DescriptorResourcesAreEquivalent", 0) + }, + wantIsPushed: true, + versionAlreadyExists: true, + sameContent: false, }, { name: "Same version, different content, without overwrite flag", + args: args{ + overwrite: false, + archive: archive, + }, + assertFn: func(err error, i ...interface{}) { + assert.Errorf(t, err, + "version 1.0.0 already exists with different content, please use --module-archive-version-overwrite flag to overwrite it") + ociRepoAccessMock.AssertNumberOfCalls(t, "ComponentVersionExists", 1) + ociRepoAccessMock.AssertNumberOfCalls(t, "GetComponentVersion", 1) + ociRepoAccessMock.AssertNumberOfCalls(t, "PushComponentVersion", 0) + ociRepoAccessMock.AssertNumberOfCalls(t, "DescriptorResourcesAreEquivalent", 1) + }, + wantIsPushed: false, + versionAlreadyExists: true, + sameContent: false, }, { name: "different version, with overwrite flag", + args: args{ + overwrite: true, + archive: archive, + }, + assertFn: func(err error, i ...interface{}) { + assert.NoError(t, err) + ociRepoAccessMock.AssertNumberOfCalls(t, "ComponentVersionExists", 0) + ociRepoAccessMock.AssertNumberOfCalls(t, "GetComponentVersion", 1) + ociRepoAccessMock.AssertNumberOfCalls(t, "PushComponentVersion", 1) + ociRepoAccessMock.AssertNumberOfCalls(t, "DescriptorResourcesAreEquivalent", 0) + }, + wantIsPushed: true, + versionAlreadyExists: false, + sameContent: false, }, { name: "different version, without overwrite flag", + args: args{ + overwrite: false, + archive: archive, + }, + assertFn: func(err error, i ...interface{}) { + assert.NoError(t, err) + ociRepoAccessMock.AssertNumberOfCalls(t, "ComponentVersionExists", 1) + ociRepoAccessMock.AssertNumberOfCalls(t, "GetComponentVersion", 1) + ociRepoAccessMock.AssertNumberOfCalls(t, "PushComponentVersion", 1) + ociRepoAccessMock.AssertNumberOfCalls(t, "DescriptorResourcesAreEquivalent", 0) + }, + wantIsPushed: true, + versionAlreadyExists: false, + sameContent: false, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + ociRepoAccessMock = mocks.OciRepoAccess{} + ociRepoAccessMock.On("PushComponentVersion", archive, mock.Anything, mock.Anything).Return(nil) + ociRepoAccessMock.On("GetComponentVersion", archive, + mock.Anything).Return(nil, nil) + ociRepoAccessMock.On("ComponentVersionExists", archive, mock.Anything).Return(tt.versionAlreadyExists, nil) + ociRepoAccessMock.On("DescriptorResourcesAreEquivalent", mock.Anything, + mock.Anything).Return(tt.sameContent) r := &module.Remote{ Insecure: true, - OciRepoAccess: tt.fields.OciRepoAccess, - } - got, got1, err := r.Push(tt.args.archive, tt.args.overwrite) - if !tt.wantErr(t, err, fmt.Sprintf("Push(%v, %v)", tt.args.archive, tt.args.overwrite)) { - return + OciRepoAccess: &ociRepoAccessMock, } - assert.Equalf(t, tt.want, got, "Push(%v, %v)", tt.args.archive, tt.args.overwrite) - assert.Equalf(t, tt.want1, got1, "Push(%v, %v)", tt.args.archive, tt.args.overwrite) + _, isPushed, err := r.Push(tt.args.archive, tt.args.overwrite) + tt.assertFn(err, fmt.Sprintf("Push(%v, %v)", tt.args.archive, tt.args.overwrite)) + + assert.Equalf(t, tt.wantIsPushed, isPushed, "Push(%v, %v)", tt.args.archive, tt.args.overwrite) }) } }