From 2dc0134fdcff0a04376e77a6ef04833b7213f4ee Mon Sep 17 00:00:00 2001 From: Hilmar Falkenberg Date: Fri, 10 Jan 2025 16:09:55 +0100 Subject: [PATCH 1/4] fixes: https://github.com/open-component-model/ocm/issues/1235 --- api/ocm/extensions/repositories/genericocireg/component.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/api/ocm/extensions/repositories/genericocireg/component.go b/api/ocm/extensions/repositories/genericocireg/component.go index 5d96b32db2..13ca88c56b 100644 --- a/api/ocm/extensions/repositories/genericocireg/component.go +++ b/api/ocm/extensions/repositories/genericocireg/component.go @@ -128,6 +128,10 @@ func (c *componentAccessImpl) HasVersion(vers string) (bool, error) { } func (c *componentAccessImpl) LookupVersion(version string) (*repocpi.ComponentVersionAccessInfo, error) { + // LookupVersion '0.0.1-20250108132333.build-af79499' would fail with: + // unable to unref last: unable to cleanup component version [%v] while unref last: closing component version [%v]: check failed: component version [%v] is invalid + // This is because the version comes from an artifact tag, which doesn't allow '+' characters, which we replace with '.build-' during the push + version = toVersion(version) // ensure the cv doesn't contain '.build-' tag, err := toTag(version) if err != nil { return nil, err From 4d7d05fd1846624347b5e113d5b93499fc8c51ae Mon Sep 17 00:00:00 2001 From: Hilmar Falkenberg Date: Mon, 13 Jan 2025 16:35:12 +0100 Subject: [PATCH 2/4] check if version contained '+', which has been replaced by META_SEPARATOR to create OCI compliant tag --- .../extensions/repositories/genericocireg/component.go | 4 ---- .../repositories/genericocireg/componentversion.go | 8 ++++++++ 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/api/ocm/extensions/repositories/genericocireg/component.go b/api/ocm/extensions/repositories/genericocireg/component.go index 13ca88c56b..5d96b32db2 100644 --- a/api/ocm/extensions/repositories/genericocireg/component.go +++ b/api/ocm/extensions/repositories/genericocireg/component.go @@ -128,10 +128,6 @@ func (c *componentAccessImpl) HasVersion(vers string) (bool, error) { } func (c *componentAccessImpl) LookupVersion(version string) (*repocpi.ComponentVersionAccessInfo, error) { - // LookupVersion '0.0.1-20250108132333.build-af79499' would fail with: - // unable to unref last: unable to cleanup component version [%v] while unref last: closing component version [%v]: check failed: component version [%v] is invalid - // This is because the version comes from an artifact tag, which doesn't allow '+' characters, which we replace with '.build-' during the push - version = toVersion(version) // ensure the cv doesn't contain '.build-' tag, err := toTag(version) if err != nil { return nil, err diff --git a/api/ocm/extensions/repositories/genericocireg/componentversion.go b/api/ocm/extensions/repositories/genericocireg/componentversion.go index e0dd0f5470..b36470f1a9 100644 --- a/api/ocm/extensions/repositories/genericocireg/componentversion.go +++ b/api/ocm/extensions/repositories/genericocireg/componentversion.go @@ -102,6 +102,14 @@ func (c *ComponentVersionContainer) SetReadOnly() { func (c *ComponentVersionContainer) Check() error { if c.version != c.GetDescriptor().Version { + // check if version contained '+' which has been replaced by META_SEPARATOR to create OCI compliant tag + if replaced, _ := toTag(c.GetDescriptor().Version); replaced != c.GetDescriptor().Version && replaced == c.version { + Logger(c.GetContext()).Warn(fmt.Sprintf( + "checked version %q contains %q, this is discouraged and you should prefer the original component version %q", c.version, META_SEPARATOR, c.GetDescriptor().Version)) + return nil + } else { + return errors.ErrInvalid("component version", c.GetDescriptor().Version) + } return errors.ErrInvalid("component version", c.GetDescriptor().Version) } if c.comp.name != c.GetDescriptor().Name { From 1b7bb355a68fbcd608c7f47aa5c217ea5c12d65d Mon Sep 17 00:00:00 2001 From: Hilmar Falkenberg Date: Tue, 14 Jan 2025 10:14:35 +0100 Subject: [PATCH 3/4] add test --- .../genericocireg/componentversion.go | 1 - .../genericocireg/componentversion_test.go | 85 +++++++++++++++++++ 2 files changed, 85 insertions(+), 1 deletion(-) create mode 100644 api/ocm/extensions/repositories/genericocireg/componentversion_test.go diff --git a/api/ocm/extensions/repositories/genericocireg/componentversion.go b/api/ocm/extensions/repositories/genericocireg/componentversion.go index b36470f1a9..2e8c0a846d 100644 --- a/api/ocm/extensions/repositories/genericocireg/componentversion.go +++ b/api/ocm/extensions/repositories/genericocireg/componentversion.go @@ -110,7 +110,6 @@ func (c *ComponentVersionContainer) Check() error { } else { return errors.ErrInvalid("component version", c.GetDescriptor().Version) } - return errors.ErrInvalid("component version", c.GetDescriptor().Version) } if c.comp.name != c.GetDescriptor().Name { return errors.ErrInvalid("component name", c.GetDescriptor().Name) diff --git a/api/ocm/extensions/repositories/genericocireg/componentversion_test.go b/api/ocm/extensions/repositories/genericocireg/componentversion_test.go new file mode 100644 index 0000000000..0b79d0b968 --- /dev/null +++ b/api/ocm/extensions/repositories/genericocireg/componentversion_test.go @@ -0,0 +1,85 @@ +package genericocireg + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + "ocm.software/ocm/api/ocm/internal" + "ocm.software/ocm/api/utils/accessobj" +) + +func TestComponentVersionContainer_Check(t *testing.T) { + // Setup + state, err := accessobj.NewBlobStateForBlob(accessobj.ACC_READONLY, nil, NewStateHandler("mock.test.com/state_handler", "1.0.0")) + assert.NoError(t, err) + repo := &RepositoryImpl{ctx: internal.DefaultContext} + comp := &componentAccessImpl{repo: repo} + cvc := &ComponentVersionContainer{state: state, comp: comp} + + // Test cases + tests := []struct { + name string + setup func() + expectErr bool + }{ + { + name: "valid version and name", + setup: func() { + cvc.version = "1.0.0" + cvc.GetDescriptor().Version = "1.0.0" + cvc.comp.name = "test-component" + cvc.GetDescriptor().Name = "test-component" + }, + expectErr: false, + }, + { + name: "half valid version - containing META_SEPARATOR = " + META_SEPARATOR, + setup: func() { + cvc.version = "0.0.1-20250108132333.build-af79499" + cvc.GetDescriptor().Version = "0.0.1-20250108132333+af79499" + cvc.comp.name = "test-component" + cvc.GetDescriptor().Name = "test-component" + }, + expectErr: false, + }, + { + name: "valid version - containing '+'", + setup: func() { + cvc.version = "0.0.1-20250108132333+af79499" + cvc.GetDescriptor().Version = "0.0.1-20250108132333+af79499" + cvc.comp.name = "test-component" + cvc.GetDescriptor().Name = "test-component" + }, + expectErr: false, + }, + { + name: "invalid version", + setup: func() { + cvc.version = "1.0.0" + cvc.GetDescriptor().Version = "2.0.0" + }, + expectErr: true, + }, + { + name: "invalid name", + setup: func() { + cvc.comp.name = "test-component" + cvc.GetDescriptor().Name = "invalid-component" + }, + expectErr: true, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + test.setup() + err := cvc.Check() + if test.expectErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + }) + } +} From c1546d29f7fa6ec1172f306d38477ee24c7fc22a Mon Sep 17 00:00:00 2001 From: Hilmar Falkenberg Date: Tue, 14 Jan 2025 10:25:30 +0100 Subject: [PATCH 4/4] no 'else' --- .../extensions/repositories/genericocireg/componentversion.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/api/ocm/extensions/repositories/genericocireg/componentversion.go b/api/ocm/extensions/repositories/genericocireg/componentversion.go index 2e8c0a846d..950ff9018c 100644 --- a/api/ocm/extensions/repositories/genericocireg/componentversion.go +++ b/api/ocm/extensions/repositories/genericocireg/componentversion.go @@ -107,9 +107,8 @@ func (c *ComponentVersionContainer) Check() error { Logger(c.GetContext()).Warn(fmt.Sprintf( "checked version %q contains %q, this is discouraged and you should prefer the original component version %q", c.version, META_SEPARATOR, c.GetDescriptor().Version)) return nil - } else { - return errors.ErrInvalid("component version", c.GetDescriptor().Version) } + return errors.ErrInvalid("component version", c.GetDescriptor().Version) } if c.comp.name != c.GetDescriptor().Name { return errors.ErrInvalid("component name", c.GetDescriptor().Name)