Skip to content

Commit

Permalink
fix(transfer): accidental extra identity defaulting during transfer (#…
Browse files Browse the repository at this point in the history
…1236)

<!-- markdownlint-disable MD041 -->
#### What this PR does / why we need it

replaces #1223 as it
does not contain the revert for the normalization algorithm

Introduces DisableExtraIdentityDefaulting which is enabled by default
during transport because IsModifyElement is not enough to differentiate
if the component version is allowed to be modified during transfer.

#### Which issue(s) this PR fixes

PR that tackles a case in which `ocm transfer cv` caused accidental
defaults to the extra identity.

---------

Co-authored-by: Uwe Krueger <uwe.krueger@sap.com>
  • Loading branch information
jakobmoellerdev and mandelsoft authored Jan 9, 2025
1 parent 8a5f983 commit 3e5f69b
Show file tree
Hide file tree
Showing 13 changed files with 121 additions and 6 deletions.
4 changes: 4 additions & 0 deletions api/ocm/cpi/modopts.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@ func NewTargetElementOptions(list ...TargetElementOption) *TargetElementOptions
return &m
}

func DisableExtraIdentityDefaulting(flag ...bool) internal.TargetOptionImpl {
return internal.DisableExtraIdentityDefaulting(flag...)
}

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

func NewAddVersionOptions(list ...AddVersionOption) *AddVersionOptions {
Expand Down
9 changes: 6 additions & 3 deletions api/ocm/cpi/repocpi/view_cv.go
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,7 @@ func (c *componentVersionAccessView) SetResource(meta *cpi.ResourceMeta, acc com
} else {
cd.Resources[idx] = *res
}
if opts.IsModifyElement() {
if opts.IsModifyElement() && !opts.IsDisableExtraIdentityDefaulting() {
// default handling for completing an extra identity for modifications, only.
compdesc.DefaultResources(cd)
}
Expand Down Expand Up @@ -527,6 +527,7 @@ func (c *componentVersionAccessView) SetSource(meta *cpi.SourceMeta, acc compdes
return accessio.ErrReadOnly
}

opts := cpi.NewTargetElementOptions(optlist...)
res := &compdesc.Source{
SourceMeta: *meta.Copy(),
Access: acc,
Expand All @@ -548,7 +549,9 @@ func (c *componentVersionAccessView) SetSource(meta *cpi.SourceMeta, acc compdes
} else {
cd.Sources[idx] = *res
}
compdesc.DefaultSources(cd)
if !opts.IsDisableExtraIdentityDefaulting() {
compdesc.DefaultSources(cd)
}
return c.bridge.Update(false)
})
}
Expand Down Expand Up @@ -584,7 +587,7 @@ func (c *componentVersionAccessView) SetReference(ref *cpi.ComponentReference, o
cd.References[idx].Equivalent(ref)
cd.References[idx] = *ref
}
if opts.IsModifyElement(moddef) {
if opts.IsModifyElement(moddef) && !opts.IsDisableExtraIdentityDefaulting() {
compdesc.DefaultReferences(cd)
}
return c.bridge.Update(false)
Expand Down
34 changes: 34 additions & 0 deletions api/ocm/internal/modopts.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,10 @@ type TargetOptionImpl interface {

type TargetElementOptions struct {
TargetElement TargetElement

// DisableExtraIdentityDefaulting disables the implicit defaulting of the extraIdentity.
// A transfer operation must set this flag to preserve the normalizations.
DisableExtraIdentityDefaulting *bool
}

type TargetElementOption interface {
Expand All @@ -128,6 +132,11 @@ func (m *TargetElementOptions) ApplyElementModificationOption(opts *ElementModif

func (m *TargetElementOptions) ApplyTargetOption(opts *TargetElementOptions) {
optionutils.Transfer(&opts.TargetElement, m.TargetElement)
optionutils.Transfer(&opts.DisableExtraIdentityDefaulting, m.DisableExtraIdentityDefaulting)
}

func (m *TargetElementOptions) IsDisableExtraIdentityDefaulting() bool {
return utils.AsBool(m.DisableExtraIdentityDefaulting)
}

func (m *TargetElementOptions) ApplyTargetOptions(list ...TargetElementOption) *TargetElementOptions {
Expand Down Expand Up @@ -355,6 +364,31 @@ func (m TargetIdentity) ApplyTargetOption(opts *TargetElementOptions) {

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

type disableextraidentitydefaulting bool

func (m disableextraidentitydefaulting) ApplyBlobModificationOption(opts *BlobModificationOptions) {
m.ApplyModificationOption(&opts.ModificationOptions)
}

func (m disableextraidentitydefaulting) ApplyModificationOption(opts *ModificationOptions) {
m.ApplyTargetOption(&opts.TargetElementOptions)
}

func (m disableextraidentitydefaulting) ApplyElementModificationOption(opts *ElementModificationOptions) {
m.ApplyTargetOption(&opts.TargetElementOptions)
}

func (m disableextraidentitydefaulting) ApplyTargetOption(opts *TargetElementOptions) {
opts.DisableExtraIdentityDefaulting = utils.BoolP(m)
}

// DisableExtraIdentityDefaulting disables the defaulting of the extra identity.
func DisableExtraIdentityDefaulting(flag ...bool) TargetOptionImpl {
return disableextraidentitydefaulting(utils.OptionalDefaultedBool(true, flag...))
}

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

type replaceElement struct{}

var UpdateElement = replaceElement{}
Expand Down
5 changes: 5 additions & 0 deletions api/ocm/modopts.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,11 @@ func TargetIndex(idx int) internal.TargetOptionImpl {
return internal.TargetIndex(idx)
}

// DisableExtraIdentityDefaulting disables the defaulting of the extra identity
func DisableExtraIdentityDefaulting(flag ...bool) internal.TargetOptionImpl {
return internal.DisableExtraIdentityDefaulting(flag...)
}

const AppendElement = internal.TargetIndex(-1)

var UpdateElement = internal.UpdateElement
Expand Down
2 changes: 1 addition & 1 deletion api/ocm/tools/transfer/transfer.go
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ func copyVersion(printer common.Printer, log logging.Logger, hist common.History
err = handler.HandleTransferResource(r, m, hint, t)
} else {
if err == nil { // old resource found -> keep current access method
t.SetResource(r.Meta(), old.Access, ocm.ModifyElement(), ocm.SkipVerify())
t.SetResource(r.Meta(), old.Access, ocm.ModifyElement(), ocm.SkipVerify(), ocm.DisableExtraIdentityDefaulting())
}
notifyArtifactInfo(printer, log, "resource", i, r.Meta(), hint, "already present")
}
Expand Down
64 changes: 64 additions & 0 deletions api/ocm/tools/transfer/transferhandler/standard/compat_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
package standard_test

import (
. "github.com/mandelsoft/goutils/testutils"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
. "ocm.software/ocm/cmds/ocm/testhelper"

"ocm.software/ocm/api/ocm/extensions/repositories/ctf"
"ocm.software/ocm/api/ocm/tools/transfer"
"ocm.software/ocm/api/utils/accessio"
"ocm.software/ocm/api/utils/accessobj"
common "ocm.software/ocm/api/utils/misc"
)

const COMPAT_ARCH = "/testdata/v0.18.0"
const COMPAT_COMP = "github.com/mandelsoft/test1"
const COMPAT_VERS = "1.0.0"

var _ = Describe("Transfer Test Environment", func() {
Context("extraid compatibility transfer", func() {

var env *TestEnv

BeforeEach(func() {
env = NewTestEnv(TestData())
})

It("", func() {
src := Must(ctf.Open(env.OCMContext(), accessobj.ACC_READONLY, COMPAT_ARCH, 0, env))
defer Close(src, "source")
cv := Must(src.LookupComponentVersion(COMPAT_COMP, COMPAT_VERS))
defer Close(cv, "source cv")
tgt := Must(ctf.Create(env.OCMContext(), accessobj.ACC_WRITABLE|accessobj.ACC_CREATE, OUT, 0o700, accessio.FormatDirectory, env))
defer Close(tgt, "target")

p, buf := common.NewBufferedPrinter()
MustBeSuccessful(transfer.Transfer(cv, tgt, transfer.WithPrinter(p)))
Expect(env.DirExists(OUT)).To(BeTrue())

Expect(buf.String()).To(StringEqualTrimmedWithContext(`
transferring version "github.com/mandelsoft/test1:1.0.0"...
...resource 0 multi-implicit[plainText]...
...resource 1 multi-implicit[plainText]...
...resource 2 multi-explicit[plainText]...
...resource 3 multi-explicit[plainText]...
...source 0 multi-implicit[plainText]...
...source 1 multi-implicit[plainText]...
...source 2 multi-explicit[plainText]...
...source 3 multi-explicit[plainText]...
...adding component version...
`))

tcv := Must(tgt.LookupComponentVersion(COMPAT_COMP, COMPAT_VERS))
defer Close(tcv, "target cv")
Expect(tcv.GetDescriptor()).To(YAMLEqual(cv.GetDescriptor()))

Expect(tcv.GetDescriptor().Resources[0].ExtraIdentity).To(BeNil())
Expect(tcv.GetDescriptor().Resources[1].ExtraIdentity).To(BeNil())
Expect(tcv.GetDescriptor().Sources[0].ExtraIdentity).To(BeNil())
Expect(tcv.GetDescriptor().Sources[1].ExtraIdentity).To(BeNil())
})
})
})
4 changes: 2 additions & 2 deletions api/ocm/tools/transfer/transferhandler/standard/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ func (h *Handler) HandleTransferResource(r ocm.ResourceAccess, m cpi.AccessMetho
}
defer blob.Close()
return accessio.Retry(h.opts.GetRetries(), time.Second, func() error {
return t.SetResourceBlob(r.Meta(), blob, hint, h.GlobalAccess(t.GetContext(), m), ocm.SkipVerify())
return t.SetResourceBlob(r.Meta(), blob, hint, h.GlobalAccess(t.GetContext(), m), ocm.SkipVerify(), ocm.DisableExtraIdentityDefaulting())
})
}

Expand All @@ -98,7 +98,7 @@ func (h *Handler) HandleTransferSource(r ocm.SourceAccess, m cpi.AccessMethod, h
}
defer blob.Close()
return accessio.Retry(h.opts.GetRetries(), time.Second, func() error {
return t.SetSourceBlob(r.Meta(), blob, hint, h.GlobalAccess(t.GetContext(), m))
return t.SetSourceBlob(r.Meta(), blob, hint, h.GlobalAccess(t.GetContext(), m), ocm.DisableExtraIdentityDefaulting())
})
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{"schemaVersion":1,"artifacts":[{"repository":"component-descriptors/github.com/mandelsoft/test1","tag":"1.0.0","digest":"sha256:210dda6808d47862660fdf5753d17e2b6152ff31fbd07ff947b821f717f1232a"}]}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{"schemaVersion":2,"mediaType":"application/vnd.oci.image.manifest.v1+json","config":{"mediaType":"application/vnd.ocm.software.component.config.v1+json","digest":"sha256:958eb6b2928a569d7aaab2aa924fb236e977bfbc43309908aa9c45b154f689cd","size":201},"layers":[{"mediaType":"application/vnd.ocm.software.component-descriptor.v2+yaml+tar","digest":"sha256:a4b84597729cfa026c51a47e903f449ad7b4a9ec9897f2084b982a66fcdbfb5f","size":4608},{"mediaType":"application/octet-stream","digest":"sha256:916f0027a575074ce72a331777c3478d6513f786a591bd892da1a577bf2335f9","size":9,"annotations":{"software.ocm.artifact":"[{\"kind\":\"resource\",\"identity\":{\"name\":\"multi-implicit\",\"version\":\"v1\"}},{\"kind\":\"resource\",\"identity\":{\"name\":\"multi-explicit\",\"version\":\"v1\"}},{\"kind\":\"source\",\"identity\":{\"name\":\"multi-implicit\",\"version\":\"v1\"}},{\"kind\":\"source\",\"identity\":{\"name\":\"multi-explicit\",\"version\":\"v1\"}}]"}},{"mediaType":"application/octet-stream","digest":"sha256:920ce99fb13b43ca0408caee6e61f6335ea5156d79aa98e733e1ed2393e0f649","size":18,"annotations":{"software.ocm.artifact":"[{\"kind\":\"resource\",\"identity\":{\"name\":\"multi-implicit\",\"version\":\"v2\"}},{\"kind\":\"resource\",\"identity\":{\"name\":\"multi-explicit\",\"version\":\"v2\"}},{\"kind\":\"source\",\"identity\":{\"name\":\"multi-implicit\",\"version\":\"v2\"}},{\"kind\":\"source\",\"identity\":{\"name\":\"multi-explicit\",\"version\":\"v2\"}}]"}}],"annotations":{"software.ocm.componentversion":"github.com/mandelsoft/test1:1.0.0","software.ocm.creator":"OCM Go Library 0.18.0"}}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
test data
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
extended test data
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{"componentDescriptorLayer":{"mediaType":"application/vnd.ocm.software.component-descriptor.v2+yaml+tar","digest":"sha256:a4b84597729cfa026c51a47e903f449ad7b4a9ec9897f2084b982a66fcdbfb5f","size":4608}}
Binary file not shown.

0 comments on commit 3e5f69b

Please sign in to comment.