From ebe976e7a778c1d909b3ff1ad0b3377b338bb022 Mon Sep 17 00:00:00 2001 From: Hilmar Falkenberg Date: Thu, 2 May 2024 14:49:54 +0200 Subject: [PATCH 1/2] Don't Panik! (#758) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The answer is: 42 ## Description Populate error to top, if it's not a semver.org conform version. ## What type of PR is this? (check all applicable) - [ ] 🍕 Feature - [ ] 🎇 Restructuring - [ ] 🐛 Bug Fix - [ ] 📝 Documentation Update - [ ] 🎨 Style - [ ] 🧑‍💻 Code Refactor - [ ] 🔥 Performance Improvements - [ ] ✅ Test - [ ] 🤖 Build - [ ] 🔁 CI - [ ] 📦 Chore (Release) - [ ] ⏩ Revert ## Related Tickets & Documents - Closes #756 ## Added tests? - [ ] 👍 yes - [ ] 🙅 no, because they aren't needed - [ ] 🙋 no, because I need help - [ ] Separate ticket for tests # (issue/pr) Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration ## Added to documentation? - [ ] 📜 README.md - [ ] 🙅 no documentation needed ## Checklist: - [ ] My code follows the style guidelines of this project - [ ] I have performed a self-review of my code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [ ] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] New and existing unit tests pass locally with my changes - [ ] Any dependent changes have been merged and published in downstream modules --- .../repositories/genericocireg/component.go | 22 +++++++++++-------- .../genericocireg/componentversion.go | 10 ++++----- 2 files changed, 18 insertions(+), 14 deletions(-) diff --git a/pkg/contexts/ocm/repositories/genericocireg/component.go b/pkg/contexts/ocm/repositories/genericocireg/component.go index 02d7131414..75c9e9b0e0 100644 --- a/pkg/contexts/ocm/repositories/genericocireg/component.go +++ b/pkg/contexts/ocm/repositories/genericocireg/component.go @@ -1,7 +1,3 @@ -// SPDX-FileCopyrightText: 2022 SAP SE or an SAP affiliate company and Open Component Model contributors. -// -// SPDX-License-Identifier: Apache-2.0 - package genericocireg import ( @@ -73,12 +69,12 @@ func (c *componentAccessImpl) GetName() string { //////////////////////////////////////////////////////////////////////////////// -func toTag(v string) string { +func toTag(v string) (string, error) { _, err := semver.NewVersion(v) if err != nil { - panic(errors.Wrapf(err, "%s is no semver version", v)) + return "", err } - return strings.ReplaceAll(v, "+", META_SEPARATOR) + return strings.ReplaceAll(v, "+", META_SEPARATOR), nil } func toVersion(t string) string { @@ -132,7 +128,11 @@ func (c *componentAccessImpl) HasVersion(vers string) (bool, error) { } func (c *componentAccessImpl) LookupVersion(version string) (*repocpi.ComponentVersionAccessInfo, error) { - acc, err := c.namespace.GetArtifact(toTag(version)) + tag, err := toTag(version) + if err != nil { + return nil, err + } + acc, err := c.namespace.GetArtifact(tag) if err != nil { if errors.IsErrNotFound(err) { return nil, cpi.ErrComponentVersionNotFoundWrap(err, c.name, version) @@ -151,7 +151,11 @@ func (c *componentAccessImpl) NewVersion(version string, overrides ...bool) (*re return nil, accessio.ErrReadOnly } override := utils.Optional(overrides...) - acc, err := c.namespace.GetArtifact(toTag(version)) + tag, err := toTag(version) + if err != nil { + return nil, err + } + acc, err := c.namespace.GetArtifact(tag) if err == nil { if override { return newComponentVersionAccess(accessobj.ACC_CREATE, c, version, acc, false) diff --git a/pkg/contexts/ocm/repositories/genericocireg/componentversion.go b/pkg/contexts/ocm/repositories/genericocireg/componentversion.go index 322b8726b9..862f9986d5 100644 --- a/pkg/contexts/ocm/repositories/genericocireg/componentversion.go +++ b/pkg/contexts/ocm/repositories/genericocireg/componentversion.go @@ -1,7 +1,3 @@ -// SPDX-FileCopyrightText: 2022 SAP SE or an SAP affiliate company and Open Component Model contributors. -// -// SPDX-License-Identifier: Apache-2.0 - package genericocireg import ( @@ -222,7 +218,11 @@ func (c *ComponentVersionContainer) Update() error { } logger.Debug("add oci artifact") - if _, err := c.comp.namespace.AddArtifact(c.manifest, toTag(c.version)); err != nil { + tag, err := toTag(c.version) + if err != nil { + return err + } + if _, err := c.comp.namespace.AddArtifact(c.manifest, tag); err != nil { return fmt.Errorf("unable to add artifact: %w", err) } } From 0b2baf5bc3bd36279cdc4662297354636ed95c5d Mon Sep 17 00:00:00 2001 From: Hilmar Falkenberg Date: Thu, 2 May 2024 15:08:44 +0200 Subject: [PATCH 2/2] add negative test for non-semver version (#759) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description ## What type of PR is this? (check all applicable) - [ ] 🍕 Feature - [ ] 🎇 Restructuring - [ ] 🐛 Bug Fix - [ ] 📝 Documentation Update - [ ] 🎨 Style - [ ] 🧑‍💻 Code Refactor - [ ] 🔥 Performance Improvements - [x] ✅ Test - [ ] 🤖 Build - [ ] 🔁 CI - [ ] 📦 Chore (Release) - [ ] ⏩ Revert ## Related Issues - #758 - #756 ## Added tests? - [ ] 👍 yes - [ ] 🙅 no, because they aren't needed - [ ] 🙋 no, because I need help - [ ] Separate ticket for tests # (issue/pr) Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration ## Added to documentation? - [ ] 📜 README.md - [ ] 🙅 no documentation needed ## Checklist: - [ ] My code follows the style guidelines of this project - [ ] I have performed a self-review of my code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [ ] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] New and existing unit tests pass locally with my changes - [ ] Any dependent changes have been merged and published in downstream modules --- .../repositories/genericocireg/repo_test.go | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/pkg/contexts/ocm/repositories/genericocireg/repo_test.go b/pkg/contexts/ocm/repositories/genericocireg/repo_test.go index 51a7b35305..50ab789483 100644 --- a/pkg/contexts/ocm/repositories/genericocireg/repo_test.go +++ b/pkg/contexts/ocm/repositories/genericocireg/repo_test.go @@ -1,7 +1,3 @@ -// SPDX-FileCopyrightText: 2022 SAP SE or an SAP affiliate company and Open Component Model contributors. -// -// SPDX-License-Identifier: Apache-2.0 - package genericocireg_test import ( @@ -9,12 +5,10 @@ import ( "path" "reflect" - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - . "github.com/open-component-model/ocm/pkg/testutils" - "github.com/mandelsoft/vfs/pkg/osfs" "github.com/mandelsoft/vfs/pkg/vfs" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" "github.com/opencontainers/go-digest" "github.com/open-component-model/ocm/pkg/blobaccess" @@ -50,6 +44,7 @@ import ( "github.com/open-component-model/ocm/pkg/finalizer" "github.com/open-component-model/ocm/pkg/mime" "github.com/open-component-model/ocm/pkg/signing/hasher/sha256" + . "github.com/open-component-model/ocm/pkg/testutils" ) var DefaultContext = ocm.New() @@ -79,6 +74,15 @@ var _ = Describe("component repository mapping", func() { vfs.Cleanup(tempfs) }) + It("Don't Panik! When it's not a semver.org conform version. #756", func() { + repo := Must(DefaultContext.RepositoryForSpec(spec)) + comp := Must(repo.LookupComponent(COMPONENT)) + cva, err := comp.NewVersion("v1.two.zeo-2") + Expect(err).To(HaveOccurred()) + Expect(cva).To(BeNil()) + Expect(err.Error()).To(Equal("Invalid Semantic Version")) + }) + It("creates a dummy component", func() { var finalize finalizer.Finalizer defer Defer(finalize.Finalize)