Skip to content

Commit 54e0190

Browse files
fabianburthfabianburth
authored
fix!: unwrap standard transfer handler and fix spiff handler (#1501) (#1617)
<!-- markdownlint-disable MD041 --> #### What this PR does / why we need it - Make `ocm transfer {cv|ctf}` respect `--omit-access-types` (as originally intended). - Unwrap the standard transfer handler so omit logic is applied consistently across handlers. - Fix spiff handler so it no longer reinstates access types when omit is requested. #### Which issue(s) this PR fixes <!-- Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`. --> Previously, users might have used the flag `--omit-access-types` with a transfer and would have changed behavior now (flag was ignored). After this change, `--omit-access-types` takes effect for any transfer as originally intended. **Users expecting resources to be transferred even though they were mentioned with their access type in `--omit-access-types` must likely drop `--omit-access-types` for that access type if they used it before and expected the resources to still be present.** Fixes #1500 --------- (cherry picked from commit 479f138) Signed-off-by: Fabian Burth <fabian.burth@sap.com>
1 parent ae2b1d0 commit 54e0190

3 files changed

Lines changed: 110 additions & 4 deletions

File tree

‎api/ocm/tools/transfer/transferhandler/spiff/handler.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,9 @@ func (h *Handler) TransferResource(src ocm.ComponentVersionAccess, a ocm.AccessS
112112
if !h.opts.IsResourcesByValue() {
113113
return false, nil
114114
}
115+
if h.opts.IsAccessTypeOmitted(a.GetType()) {
116+
return false, nil
117+
}
115118
if h.opts.GetScript() == nil {
116119
return true, nil
117120
}
@@ -123,6 +126,9 @@ func (h *Handler) TransferSource(src ocm.ComponentVersionAccess, a ocm.AccessSpe
123126
if !h.opts.IsSourcesByValue() {
124127
return false, nil
125128
}
129+
if h.opts.IsAccessTypeOmitted(a.GetType()) {
130+
return false, nil
131+
}
126132
if h.opts.GetScript() == nil {
127133
return true, nil
128134
}

‎api/ocm/tools/transfer/transferhandler/spiff/handler_test.go‎

Lines changed: 97 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,21 +4,20 @@ import (
44
"encoding/json"
55
"fmt"
66

7+
"github.com/mandelsoft/goutils/testutils"
78
. "github.com/onsi/ginkgo/v2"
89
. "github.com/onsi/gomega"
910
. "ocm.software/ocm/api/helper/builder"
1011
. "ocm.software/ocm/api/oci/testhelper"
1112

12-
"github.com/mandelsoft/goutils/testutils"
13-
1413
"ocm.software/ocm/api/oci"
1514
"ocm.software/ocm/api/oci/artdesc"
1615
"ocm.software/ocm/api/oci/extensions/repositories/artifactset"
1716
metav1 "ocm.software/ocm/api/ocm/compdesc/meta/v1"
1817
"ocm.software/ocm/api/ocm/extensions/accessmethods/ociartifact"
1918
resourcetypes "ocm.software/ocm/api/ocm/extensions/artifacttypes"
2019
"ocm.software/ocm/api/ocm/extensions/repositories/ctf"
21-
ocmutils "ocm.software/ocm/api/ocm/ocmutils"
20+
"ocm.software/ocm/api/ocm/ocmutils"
2221
"ocm.software/ocm/api/ocm/tools/transfer"
2322
"ocm.software/ocm/api/ocm/tools/transfer/transferhandler/spiff"
2423
"ocm.software/ocm/api/ocm/tools/transfer/transferhandler/standard"
@@ -295,4 +294,99 @@ process: (( (*(rules[mode] || rules.default)).process ))
295294
Expect(err).To(Succeed())
296295
})
297296
})
297+
298+
Context("test omit access type option", func() {
299+
var env *Builder
300+
testOciAccess := ociartifact.New(oci.StandardOCIRef(OCIHOST+".alias", OCINAMESPACE2, OCIVERSION))
301+
302+
BeforeEach(func() {
303+
env = NewBuilder()
304+
305+
FakeOCIRepo(env, OCIPATH, OCIHOST)
306+
307+
env.OCICommonTransport(OCIPATH, accessio.FormatDirectory, func() {
308+
OCIManifest2(env)
309+
})
310+
311+
env.OCMCommonTransport(ARCH, accessio.FormatDirectory, func() {
312+
env.Component(COMPONENT, func() {
313+
env.Version(VERSION, func() {
314+
env.Provider(PROVIDER)
315+
env.Resource("ref", "", resourcetypes.OCI_IMAGE, metav1.LocalRelation, func() {
316+
env.Access(
317+
testOciAccess,
318+
)
319+
})
320+
})
321+
})
322+
})
323+
324+
env.OCMCommonTransport(ARCH2, accessio.FormatDirectory, func() {
325+
env.Component(COMPONENT2, func() {
326+
env.Version(VERSION, func() {
327+
env.Reference("ref", COMPONENT, VERSION)
328+
env.Provider(PROVIDER)
329+
})
330+
})
331+
})
332+
})
333+
334+
AfterEach(func() {
335+
env.Cleanup()
336+
})
337+
338+
It("should omit oci access", func() {
339+
src, err := ctf.Open(env.OCMContext(), accessobj.ACC_READONLY, ARCH, 0, env)
340+
Expect(err).To(Succeed())
341+
cv, err := src.LookupComponentVersion(COMPONENT, VERSION)
342+
Expect(err).To(Succeed())
343+
tgt, err := ctf.Create(env.OCMContext(), accessobj.ACC_WRITABLE|accessobj.ACC_CREATE, OUT, 0o700, accessio.FormatDirectory, env)
344+
Expect(err).To(Succeed())
345+
defer tgt.Close()
346+
347+
handler, err := spiff.New(standard.ResourcesByValue(), standard.AddOmittedAccessTypes(ociartifact.Type))
348+
Expect(err).To(Succeed())
349+
err = transfer.TransferVersion(nil, nil, cv, tgt, handler)
350+
Expect(err).To(Succeed())
351+
Expect(env.DirExists(OUT)).To(BeTrue())
352+
353+
list, err := tgt.ComponentLister().GetComponents("", true)
354+
Expect(err).To(Succeed())
355+
Expect(list).To(Equal([]string{COMPONENT}))
356+
comp, err := tgt.LookupComponentVersion(COMPONENT, VERSION)
357+
Expect(err).To(Succeed())
358+
Expect(len(comp.GetDescriptor().Resources)).To(Equal(1))
359+
360+
acc := comp.GetDescriptor().Resources[0].Access
361+
Expect(acc).To(testutils.YAMLEqual(testOciAccess))
362+
})
363+
364+
It("should not omit oci access", func() {
365+
src, err := ctf.Open(env.OCMContext(), accessobj.ACC_READONLY, ARCH, 0, env)
366+
Expect(err).To(Succeed())
367+
cv, err := src.LookupComponentVersion(COMPONENT, VERSION)
368+
Expect(err).To(Succeed())
369+
tgt, err := ctf.Create(env.OCMContext(), accessobj.ACC_WRITABLE|accessobj.ACC_CREATE, OUT, 0o700, accessio.FormatDirectory, env)
370+
Expect(err).To(Succeed())
371+
defer tgt.Close()
372+
373+
handler, err := spiff.New(standard.ResourcesByValue())
374+
Expect(err).To(Succeed())
375+
err = transfer.TransferVersion(nil, nil, cv, tgt, handler)
376+
Expect(err).To(Succeed())
377+
Expect(env.DirExists(OUT)).To(BeTrue())
378+
379+
list, err := tgt.ComponentLister().GetComponents("", true)
380+
Expect(err).To(Succeed())
381+
Expect(list).To(Equal([]string{COMPONENT}))
382+
comp, err := tgt.LookupComponentVersion(COMPONENT, VERSION)
383+
Expect(err).To(Succeed())
384+
Expect(len(comp.GetDescriptor().Resources)).To(Equal(1))
385+
386+
data, err := json.Marshal(comp.GetDescriptor().Resources[0].Access)
387+
Expect(err).To(Succeed())
388+
hash := HashManifest2(artifactset.DefaultArtifactSetDescriptorFileName)
389+
Expect(string(data)).To(testutils.StringEqualWithContext("{\"localReference\":\"" + hash + "\",\"mediaType\":\"application/vnd.oci.image.manifest.v1+tar+gzip\",\"referenceName\":\"" + OCINAMESPACE2 + ":" + OCIVERSION + "\",\"type\":\"localBlob\"}"))
390+
})
391+
})
298392
})

‎cmds/ocm/commands/ocmcmds/components/transfer/cmd.go‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212

1313
clictx "ocm.software/ocm/api/cli"
1414
"ocm.software/ocm/api/ocm"
15+
"ocm.software/ocm/api/ocm/compdesc"
1516
"ocm.software/ocm/api/ocm/tools/transfer"
1617
"ocm.software/ocm/api/ocm/tools/transfer/transferhandler"
1718
"ocm.software/ocm/api/ocm/tools/transfer/transferhandler/spiff"
@@ -173,7 +174,12 @@ func (a *action) Add(e interface{}) error {
173174
if !ok {
174175
return fmt.Errorf("object of type %T is not a valid comphdlr.Object", e)
175176
}
176-
err := transfer.TransferVersion(a.printer, a.closure, o.ComponentVersion, a.target, a.handler)
177+
sub, h, err := a.handler.TransferVersion(o.Repository, nil, compdesc.NewComponentReference("", o.ComponentVersion.GetName(), o.ComponentVersion.GetVersion(), nil), a.target)
178+
if err != nil {
179+
return errors.Wrapf(err, "cannot transfer component version %s/%s", o.ComponentVersion.GetName(), o.ComponentVersion.GetVersion())
180+
}
181+
err = transfer.TransferVersion(a.printer, a.closure, sub, a.target, h)
182+
sub.Close()
177183
a.errors.Add(err)
178184
if err != nil {
179185
a.printer.Printf("Error: %s\n", err)

0 commit comments

Comments
 (0)