fix implicit version handling of an artifact identity (#1026)

<!-- markdownlint-disable MD041 -->
#### What this PR does / why we need it
The defaulting of the extraIdentity did only work, if there was an
identity map already set.

So far the version was implicutly added to the extraIdentity of a CV if
the rest is not
unique. In the future the extraIdentity should be explicitly set to be
unique. Therefore,
the version was now implicitly added to the extraIdentity. This has two
problems:
- it was only done, if there was already an identity map set,
- it was always done, when serializing a CD

To be comparable with older signed component versions, such defaulting
may only be done
if the content of a component version is changed, otherwise the
signature would be brocken.

#### Which issue(s) this PR fixes
<!--
Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.
-->

api/helper/builder/ocm_identity.go

@@ -1,5 +1,9 @@
 package builder
 
+import (
+	metav1 "ocm.software/ocm/api/ocm/compdesc/meta/v1"
+)
+
 const T_OCMMETA = "element with metadata"
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -10,6 +14,18 @@ func (b *Builder) ExtraIdentity(name string, value string) {
 	b.ocm_meta.ExtraIdentity.Set(name, value)
 }
 
+func (b *Builder) ExtraIdentities(extras ...string) {
+	b.expect(b.ocm_meta, T_OCMMETA)
+
+	id := metav1.NewExtraIdentity(extras...)
+	if b.ocm_meta.ExtraIdentity == nil {
+		b.ocm_meta.ExtraIdentity = metav1.Identity{}
+	}
+	for k, v := range id {
+		b.ocm_meta.ExtraIdentity.Set(k, v)
+	}
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 
 func (b *Builder) RemoveExtraIdentity(name string) {

api/helper/builder/ocm_reference.go

@@ -1,6 +1,7 @@
 package builder
 
 import (
+	"ocm.software/ocm/api/ocm"
 	"ocm.software/ocm/api/ocm/compdesc"
 )
 
@@ -22,7 +23,7 @@ func (r *ocmReference) Set() {
 }
 
 func (r *ocmReference) Close() error {
-	return r.ocm_vers.SetReference(&r.meta)
+	return r.ocm_vers.SetReference(&r.meta, ocm.ModifyElement())
 }
 
 ////////////////////////////////////////////////////////////////////////////////

api/helper/builder/ocm_resource.go

@@ -42,9 +42,9 @@ func (r *ocmResource) Close() error {
 	}
 	switch {
 	case r.access != nil:
-		return r.Builder.ocm_vers.SetResource(&r.meta, r.access, r.opts.ApplyModificationOptions((ocm.ModifyResource())))
+		return r.Builder.ocm_vers.SetResource(&r.meta, r.access, r.opts.ApplyModificationOptions((ocm.ModifyElement())))
 	case r.blob != nil:
-		return r.Builder.ocm_vers.SetResourceBlob(&r.meta, r.blob, r.hint, nil, r.opts.ApplyModificationOptions((ocm.ModifyResource())))
+		return r.Builder.ocm_vers.SetResourceBlob(&r.meta, r.blob, r.hint, nil, r.opts.ApplyModificationOptions((ocm.ModifyElement())))
 	}
 	return errors.New("access or blob required")
 }

api/oci/ociutils/ref.go

@@ -89,8 +89,7 @@ func (v *ArtVersion) IsDigested() bool {
 }
 
 func (v *ArtVersion) GetTag() string {
-	if v != nil &&
-		v.Tag != nil {
+	if v != nil && v.Tag != nil {
 		return *v.Tag
 	}
 	return ""

api/ocm/add_test.go

@@ -179,51 +179,51 @@ var _ = Describe("add resources", func() {
 	Context("references", func() {
 		It("adds reference", func() {
 			ref := ocm.NewComponentReference("test", COMPONENT+"/sub", "v1")
-			MustBeSuccessful(cv.SetReference(ref))
+			MustBeSuccessful(cv.SetReference(ref, ocm.ModifyElement()))
 			Expect(len(cv.GetDescriptor().References)).To(Equal(1))
 		})
 
 		It("replaces reference", func() {
 			ref := ocm.NewComponentReference("test", COMPONENT+"/sub", "v1")
-			MustBeSuccessful(cv.SetReference(ref))
+			MustBeSuccessful(cv.SetReference(ref, ocm.ModifyElement()))
 
 			MustBeSuccessful(cv.SetReference(ref.WithVersion("v1")))
 			Expect(len(Must(cv.SelectReferences(selectors.Name("test"))))).To(Equal(1))
 		})
 
 		It("replaces source (enforced)", func() {
 			ref := ocm.NewComponentReference("test", COMPONENT+"/sub", "v1")
-			MustBeSuccessful(cv.SetReference(ref))
+			MustBeSuccessful(cv.SetReference(ref, ocm.ModifyElement()))
 
 			MustBeSuccessful(cv.SetReference(ref.WithVersion("v2")))
 			Expect(len(Must(cv.SelectReferences(selectors.Name("test"))))).To(Equal(1))
 		})
 
 		It("fails replace non-existent source)", func() {
 			ref := ocm.NewComponentReference("test", COMPONENT+"/sub", "v1")
-			MustBeSuccessful(cv.SetReference(ref))
+			MustBeSuccessful(cv.SetReference(ref, ocm.ModifyElement()))
 
 			Expect(cv.SetReference(ref.WithExtraIdentity("attr", "value"), ocm.UpdateElement)).To(
 				MatchError("element \"attr\"=\"value\",\"name\"=\"test\" not found"))
 		})
 
 		It("adds duplicate reference with different version", func() {
 			ref := ocm.NewComponentReference("test", COMPONENT+"/sub", "v1")
-			MustBeSuccessful(cv.SetReference(ref))
+			MustBeSuccessful(cv.SetReference(ref, ocm.ModifyElement()))
 			MustBeSuccessful(cv.SetReference(ref.WithVersion("v2"), ocm.AppendElement))
 			Expect(len(Must(cv.SelectReferences(selectors.Name("test"))))).To(Equal(2))
 		})
 
 		It("rejects duplicate reference with same version", func() {
 			ref := ocm.NewComponentReference("test", COMPONENT+"/sub", "v1")
-			MustBeSuccessful(cv.SetReference(ref))
+			MustBeSuccessful(cv.SetReference(ref, ocm.ModifyElement()))
 			Expect(cv.SetReference(ref.WithVersion("v1"), ocm.AppendElement)).
 				To(MatchError("adding a new reference with same base identity requires different version"))
 		})
 
 		It("rejects duplicate reference with extra identity", func() {
 			ref := ocm.NewComponentReference("test", COMPONENT+"/sub", "v1").WithExtraIdentity("attr", "value")
-			MustBeSuccessful(cv.SetReference(ref))
+			MustBeSuccessful(cv.SetReference(ref, ocm.ModifyElement()))
 			Expect(cv.SetReference(ref, ocm.AppendElement)).
 				To(MatchError("adding a new reference with same base identity requires different version"))
 		})

api/ocm/compdesc/componentdescriptor.go

@@ -238,18 +238,21 @@ func (o *ElementMeta) GetIdentity(accessor ElementListAccessor) metav1.Identity
 		identity = metav1.Identity{}
 	}
 	identity[SystemIdentityName] = o.Name
-	if accessor != nil {
+	if identity.Get(SystemIdentityVersion) == "" && accessor != nil {
 		found := false
 		l := accessor.Len()
 		for i := 0; i < l; i++ {
 			m := accessor.Get(i).GetMeta()
-			if m.GetName() == o.Name && m.GetExtraIdentity().Equals(o.ExtraIdentity) {
-				if found {
-					identity[SystemIdentityVersion] = o.Version
-
-					break
+			if m.GetName() == o.Name {
+				mid := m.GetExtraIdentity()
+				mid.Remove(SystemIdentityVersion)
+				if mid.Equals(o.ExtraIdentity) {
+					if found {
+						identity[SystemIdentityVersion] = o.Version
+						break
+					}
+					found = true
 				}
-				found = true
 			}
 		}
 	}

api/ocm/compdesc/default.go

@@ -27,9 +27,16 @@ func DefaultComponent(component *ComponentDescriptor) *ComponentDescriptor {
 	return component
 }
 
+func DefaultElements(component *ComponentDescriptor) {
+	DefaultResources(component)
+	DefaultSources(component)
+	DefaultReferences(component)
+}
+
 // DefaultResources defaults a list of resources.
 // The version of the component is defaulted for local resources that do not contain a version.
 // adds the version as identity if the resource identity would clash otherwise.
+// The version is added to an extraIdentity, if it is not unique without it.
 func DefaultResources(component *ComponentDescriptor) {
 	for i, res := range component.Resources {
 		if res.Relation == v1.LocalRelation && len(res.Version) == 0 {
@@ -39,7 +46,45 @@ func DefaultResources(component *ComponentDescriptor) {
 		id := res.GetIdentity(component.Resources)
 		if v, ok := id[SystemIdentityVersion]; ok {
 			if res.ExtraIdentity == nil {
-				res.ExtraIdentity = v1.Identity{
+				component.Resources[i].ExtraIdentity = v1.Identity{
+					SystemIdentityVersion: v,
+				}
+			} else {
+				if _, ok := res.ExtraIdentity[SystemIdentityVersion]; !ok {
+					res.ExtraIdentity[SystemIdentityVersion] = v
+				}
+			}
+		}
+	}
+}
+
+// DefaultSources defaults a list of sources.
+// The version is added to an extraIdentity, if it is not unique without it.
+func DefaultSources(component *ComponentDescriptor) {
+	for i, res := range component.Sources {
+		id := res.GetIdentity(component.Resources)
+		if v, ok := id[SystemIdentityVersion]; ok {
+			if res.ExtraIdentity == nil {
+				component.Sources[i].ExtraIdentity = v1.Identity{
+					SystemIdentityVersion: v,
+				}
+			} else {
+				if _, ok := res.ExtraIdentity[SystemIdentityVersion]; !ok {
+					res.ExtraIdentity[SystemIdentityVersion] = v
+				}
+			}
+		}
+	}
+}
+
+// DefaultReferences defaults a list of references.
+// The version is added to an extraIdentity, if it is not unique without it.
+func DefaultReferences(component *ComponentDescriptor) {
+	for i, res := range component.References {
+		id := res.GetIdentity(component.Resources)
+		if v, ok := id[SystemIdentityVersion]; ok {
+			if res.ExtraIdentity == nil {
+				component.References[i].ExtraIdentity = v1.Identity{
 					SystemIdentityVersion: v,
 				}
 			} else {

api/ocm/compdesc/versions/ocm.software/v3alpha1/default.go

@@ -1,7 +1,6 @@
 package v3alpha1
 
 import (
-	v1 "ocm.software/ocm/api/ocm/compdesc/meta/v1"
 	"ocm.software/ocm/api/utils/runtime"
 )
 
@@ -20,30 +19,5 @@ func (cd *ComponentDescriptor) Default() error {
 		cd.Spec.Resources = make([]Resource, 0)
 	}
 
-	DefaultResources(cd)
 	return nil
 }
-
-// DefaultResources defaults a list of resources.
-// The version of the component is defaulted for local resources that do not contain a version.
-// adds the version as identity if the resource identity would clash otherwise.
-func DefaultResources(component *ComponentDescriptor) {
-	for i, res := range component.Spec.Resources {
-		if res.Relation == v1.LocalRelation && len(res.Version) == 0 {
-			component.Spec.Resources[i].Version = component.GetVersion()
-		}
-
-		id := res.GetIdentity(component.Spec.Resources)
-		if v, ok := id[SystemIdentityVersion]; ok {
-			if res.ExtraIdentity == nil {
-				res.ExtraIdentity = v1.Identity{
-					SystemIdentityVersion: v,
-				}
-			} else {
-				if _, ok := res.ExtraIdentity[SystemIdentityVersion]; !ok {
-					res.ExtraIdentity[SystemIdentityVersion] = v
-				}
-			}
-		}
-	}
-}

api/ocm/compdesc/versions/v2/default.go

@@ -1,7 +1,6 @@
 package v2
 
 import (
-	v1 "ocm.software/ocm/api/ocm/compdesc/meta/v1"
 	"ocm.software/ocm/api/utils/runtime"
 )
 
@@ -20,30 +19,5 @@ func (cd *ComponentDescriptor) Default() error {
 		cd.Resources = make([]Resource, 0)
 	}
 
-	DefaultResources(cd)
 	return nil
 }
-
-// DefaultResources defaults a list of resources.
-// The version of the component is defaulted for local resources that do not contain a version.
-// adds the version as identity if the resource identity would clash otherwise.
-func DefaultResources(component *ComponentDescriptor) {
-	for i, res := range component.Resources {
-		if res.Relation == v1.LocalRelation && len(res.Version) == 0 {
-			component.Resources[i].Version = component.GetVersion()
-		}
-
-		id := res.GetIdentity(component.Resources)
-		if v, ok := id[SystemIdentityVersion]; ok {
-			if res.ExtraIdentity == nil {
-				res.ExtraIdentity = v1.Identity{
-					SystemIdentityVersion: v,
-				}
-			} else {
-				if _, ok := res.ExtraIdentity[SystemIdentityVersion]; !ok {
-					res.ExtraIdentity[SystemIdentityVersion] = v
-				}
-			}
-		}
-	}
-}

api/ocm/cpi/dummy.go

@@ -164,19 +164,19 @@ func (d *DummyComponentVersionAccess) SetResourceByAccess(art ResourceAccess, mo
 	return errors.ErrNotSupported("resource modification")
 }
 
-func (d *DummyComponentVersionAccess) SetSourceBlob(meta *SourceMeta, blob BlobAccess, refname string, global AccessSpec, opts ...TargetOption) error {
+func (d *DummyComponentVersionAccess) SetSourceBlob(meta *SourceMeta, blob BlobAccess, refname string, global AccessSpec, opts ...TargetElementOption) error {
 	return errors.ErrNotSupported("source modification")
 }
 
-func (d *DummyComponentVersionAccess) SetSource(meta *SourceMeta, spec compdesc.AccessSpec, opts ...TargetOption) error {
+func (d *DummyComponentVersionAccess) SetSource(meta *SourceMeta, spec compdesc.AccessSpec, opts ...TargetElementOption) error {
 	return errors.ErrNotSupported("source modification")
 }
 
-func (d *DummyComponentVersionAccess) SetSourceByAccess(art SourceAccess, opts ...TargetOption) error {
+func (d *DummyComponentVersionAccess) SetSourceByAccess(art SourceAccess, opts ...TargetElementOption) error {
 	return errors.ErrNotSupported()
 }
 
-func (d *DummyComponentVersionAccess) SetReference(ref *ComponentReference, opts ...TargetOption) error {
+func (d *DummyComponentVersionAccess) SetReference(ref *ComponentReference, opts ...ElementModificationOption) error {
 	return errors.ErrNotSupported()
 }
 

api/ocm/cpi/modopts.go

@@ -9,9 +9,12 @@ import (
 )
 
 type (
-	TargetElement = internal.TargetElement
-	TargetOption  = internal.TargetOption
-	TargetOptions = internal.TargetOptions
+	TargetElement        = internal.TargetElement
+	TargetElementOption  = internal.TargetElementOption
+	TargetElementOptions = internal.TargetElementOptions
+
+	ElementModificationOption  = internal.ElementModificationOption
+	ElementModificationOptions = internal.ElementModificationOptions
 
 	ModificationOption  = internal.ModificationOption
 	ModificationOptions = internal.ModificationOptions
@@ -26,8 +29,8 @@ type (
 	AddVersionOptions = internal.AddVersionOptions
 )
 
-func NewTargetOptions(list ...TargetOption) *TargetOptions {
-	var m TargetOptions
+func NewTargetElementOptions(list ...TargetElementOption) *TargetElementOptions {
+	var m TargetElementOptions
 	m.ApplyTargetOptions(list...)
 	return &m
 }
@@ -65,6 +68,10 @@ func NewModificationOptions(list ...ModificationOption) *ModificationOptions {
 	return internal.NewModificationOptions(list...)
 }
 
+func NewElementModificationOptions(list ...ElementModificationOption) *ElementModificationOptions {
+	return internal.NewElementModificationOptions(list...)
+}
+
 func TargetIndex(idx int) internal.TargetIndex {
 	return internal.TargetIndex(-1)
 }
@@ -77,10 +84,15 @@ func TargetIdentity(id v1.Identity) internal.TargetIdentity {
 	return internal.TargetIdentity(id)
 }
 
+// Deprecated: use ModifyElement.
 func ModifyResource(flag ...bool) internal.ModOptionImpl {
 	return internal.ModifyResource(flag...)
 }
 
+func ModifyElement(flag ...bool) internal.ElemModOptionImpl {
+	return internal.ModifyElement(flag...)
+}
+
 func AcceptExistentDigests(flag ...bool) internal.ModOptionImpl {
 	return internal.AcceptExistentDigests(flag...)
 }