Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix implicit version handling of an artifact identity #1026

Merged
merged 4 commits into from
Nov 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions api/helper/builder/ocm_identity.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
package builder

import (
metav1 "ocm.software/ocm/api/ocm/compdesc/meta/v1"
)

const T_OCMMETA = "element with metadata"

////////////////////////////////////////////////////////////////////////////////
Expand All @@ -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 {
jakobmoellerdev marked this conversation as resolved.
Show resolved Hide resolved
b.ocm_meta.ExtraIdentity = metav1.Identity{}
}
for k, v := range id {
b.ocm_meta.ExtraIdentity.Set(k, v)
}
}

////////////////////////////////////////////////////////////////////////////////

func (b *Builder) RemoveExtraIdentity(name string) {
Expand Down
3 changes: 2 additions & 1 deletion api/helper/builder/ocm_reference.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package builder

import (
"ocm.software/ocm/api/ocm"
"ocm.software/ocm/api/ocm/compdesc"
)

Expand All @@ -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())
}

////////////////////////////////////////////////////////////////////////////////
Expand Down
4 changes: 2 additions & 2 deletions api/helper/builder/ocm_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand Down
3 changes: 1 addition & 2 deletions api/oci/ociutils/ref.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 ""
Expand Down
14 changes: 7 additions & 7 deletions api/ocm/add_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
})
Expand Down
17 changes: 10 additions & 7 deletions api/ocm/compdesc/componentdescriptor.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
}
Expand Down
47 changes: 46 additions & 1 deletion api/ocm/compdesc/default.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down
26 changes: 0 additions & 26 deletions api/ocm/compdesc/versions/ocm.software/v3alpha1/default.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package v3alpha1

import (
v1 "ocm.software/ocm/api/ocm/compdesc/meta/v1"
"ocm.software/ocm/api/utils/runtime"
)

Expand All @@ -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
}
}
}
}
}
26 changes: 0 additions & 26 deletions api/ocm/compdesc/versions/v2/default.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package v2

import (
v1 "ocm.software/ocm/api/ocm/compdesc/meta/v1"
"ocm.software/ocm/api/utils/runtime"
)

Expand All @@ -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
}
}
}
}
}
8 changes: 4 additions & 4 deletions api/ocm/cpi/dummy.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}

Expand Down
22 changes: 17 additions & 5 deletions api/ocm/cpi/modopts.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
}
Expand Down Expand Up @@ -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)
}
Expand All @@ -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...)
}
Expand Down
Loading