Skip to content

Commit

Permalink
Fix update and hint (#583)
Browse files Browse the repository at this point in the history
* Call Update() in ocm contexts GetResolver() method

Without calling update, the configuration is not necessarily applied
before returning the resolvers. Therefore, resolvers would frequently
return nil even though they are set in the configuration.

* Fix passing wrong variable

Since the wrong variable was passed here, the type prefix
CommonTransportFormat:: was attempted to be mapped to a file format,
which does not work.

* Rework the Type handling for mapping CLI String Notations to repository specs.

---------

Co-authored-by: Uwe Krueger <uwe.krueger@sap.com>
  • Loading branch information
fabianburth and mandelsoft authored Nov 27, 2023
1 parent 21a0d66 commit 6738019
Show file tree
Hide file tree
Showing 16 changed files with 214 additions and 45 deletions.
15 changes: 11 additions & 4 deletions pkg/common/accessio/format.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ const (
FormatTar FileFormat = "tar"
FormatTGZ FileFormat = "tgz"
FormatDirectory FileFormat = "directory"
FormatNone FileFormat = ""
)

var suffixes = map[FileFormat]string{
Expand Down Expand Up @@ -77,18 +78,24 @@ func GetFormatsFor[T any](fileFormats map[FileFormat]T) []string {
return list
}

func FileFormatForType(t string) FileFormat {
// FileFormatForTypeSpec returns the format hint provided
// by a type specification.The format hint is an optional
// suffix separated by a +.
func FileFormatForTypeSpec(t string) FileFormat {
i := strings.Index(t, "+")
if i < 0 {
return FileFormat(t)
return ""
}
return FileFormat(t[i+1:])
}

func TypeForType(t string) string {
// TypeForTypeSpec returns the pure type info provided
// by a type specification.The format hint is an optional
// suffix separated by a +.
func TypeForTypeSpec(t string) string {
i := strings.Index(t, "+")
if i < 0 {
return ""
return t
}
return t[:i]
}
Expand Down
27 changes: 27 additions & 0 deletions pkg/common/accessobj/format.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,3 +149,30 @@ func DefaultCreateOptsFileHandling(kind string, info AccessObjectInfo, path stri

return NewAccessObject(info, ACC_CREATE, opts.GetRepresentation(), nil, CloserFunction(func(obj *AccessObject) error { return handler.Write(obj, path, opts, mode) }), DirMode)
}

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

// MapType maps a given type name to an effective type and a format.
func MapType(hint string, efftyp string, deffmt accessio.FileFormat, useFormats bool, alt ...string) (string, accessio.FileFormat) {
typ := accessio.TypeForTypeSpec(hint)
f := accessio.FileFormatForTypeSpec(hint)
if f != "" {
deffmt = f
}
if typ == efftyp {
return efftyp, deffmt
}
for _, t := range alt {
if typ == t {
return efftyp, deffmt
}
}
if useFormats {
for _, f := range accessio.GetFormats() {
if hint == f {
return efftyp, accessio.FileFormat(f)
}
}
}
return "", ""
}
10 changes: 10 additions & 0 deletions pkg/contexts/oci/grammar/grammar.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
package grammar

import (
"strings"

. "github.com/open-component-model/ocm/pkg/regex"
)

Expand Down Expand Up @@ -217,3 +219,11 @@ var (
// identifier value, anchored at start and end of string.
AnchoredIdentifierRegexp = Anchored(IdentifierRegexp)
)

func SplitTypeSpec(t string) (string, string) {
i := strings.Index(t, "+")
if i < 0 {
return t, ""
}
return t[:i], t[i+1:]
}
10 changes: 8 additions & 2 deletions pkg/contexts/oci/ref.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,20 @@ func ParseRepo(ref string) (UniformRepositorySpec, error) {
if match == nil {
return UniformRepositorySpec{}, errors.ErrInvalid(KIND_OCI_REFERENCE, ref)
}
h := string(match[1])
t, _ := grammar.SplitTypeSpec(h)
return UniformRepositorySpec{
Type: string(match[1]),
Type: t,
TypeHint: h,
Info: string(match[2]),
CreateIfMissing: create,
}, nil
}
h := string(match[1])
t, _ := grammar.SplitTypeSpec(h)
return UniformRepositorySpec{
Type: string(match[1]),
Type: t,
TypeHint: h,
Scheme: string(match[2]),
Host: string(match[3]),
CreateIfMissing: create,
Expand Down
5 changes: 3 additions & 2 deletions pkg/contexts/oci/ref_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -204,8 +204,9 @@ var _ = Describe("ref parsing", func() {
Info: "alias",
})
CheckRepo("tar::a/b.tar", &oci.UniformRepositorySpec{
Type: "tar",
Info: "a/b.tar",
Type: "tar",
Info: "a/b.tar",
TypeHint: "tar",
})
CheckRepo("a/b.tar", &oci.UniformRepositorySpec{
Info: "a/b.tar",
Expand Down
13 changes: 6 additions & 7 deletions pkg/contexts/oci/repositories/artifactset/uniform.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,6 @@ func init() {
h := &repospechandler{}
cpi.RegisterRepositorySpecHandler(h, "")
cpi.RegisterRepositorySpecHandler(h, Type)
for _, f := range SupportedFormats() {
cpi.RegisterRepositorySpecHandler(h, string(f))
}
}

type repospechandler struct{}
Expand All @@ -32,23 +29,25 @@ func (h *repospechandler) MapReference(ctx cpi.Context, u *cpi.UniformRepository
}
fs := vfsattr.Get(ctx)

hint := u.TypeHint
hint, f := accessobj.MapType(u.TypeHint, Type, accessio.FormatDirectory, false)
if !u.CreateIfMissing {
hint = ""
}

create, ok, err := accessobj.CheckFile(Type, hint, accessio.TypeForType(u.Type) == Type, path, fs, ArtifactSetDescriptorFileName)
create, ok, err := accessobj.CheckFile(Type, hint, accessio.TypeForTypeSpec(u.Type) == Type, path, fs, ArtifactSetDescriptorFileName)
if err == nil && !ok {
create, ok, err = accessobj.CheckFile(Type, hint, accessio.TypeForType(u.Type) == Type, path, fs, OCIArtifactSetDescriptorFileName)
create, ok, err = accessobj.CheckFile(Type, hint, accessio.TypeForTypeSpec(u.Type) == Type, path, fs, OCIArtifactSetDescriptorFileName)
}

if !ok || err != nil {
return nil, err
}

mode := accessobj.ACC_WRITABLE
createHint := accessio.FormatNone
if create {
mode |= accessobj.ACC_CREATE
createHint = f
}
return NewRepositorySpec(mode, path, accessio.FileFormatForType(u.Type), accessio.PathFileSystem(fs))
return NewRepositorySpec(mode, path, createHint, accessio.PathFileSystem(fs))
}
13 changes: 9 additions & 4 deletions pkg/contexts/oci/repositories/ctf/uniform.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import (
"github.com/open-component-model/ocm/pkg/contexts/oci/cpi"
)

const AltType = "ctf"

func init() {
h := &repospechandler{}
cpi.RegisterRepositorySpecHandler(h, "")
Expand All @@ -36,12 +38,13 @@ func MapReference(ctx cpi.Context, u *cpi.UniformRepositorySpec) (cpi.Repository
}
fs := vfsattr.Get(ctx)

hint := u.TypeHint
typ, _ := accessobj.MapType(u.Type, Type, accessio.FormatNone, true, AltType)
hint, f := accessobj.MapType(u.TypeHint, Type, accessio.FormatDirectory, true, AltType)
if !u.CreateIfMissing {
hint = ""
}
create, ok, err := accessobj.CheckFile(Type, hint, accessio.TypeForType(u.Type) != "", path, fs, ArtifactIndexFileName)
if !ok || err != nil {
create, ok, err := accessobj.CheckFile(Type, hint, accessio.TypeForTypeSpec(typ) == Type, path, fs, ArtifactIndexFileName)
if !ok || (err != nil && typ == "") {
if err != nil {
return nil, err
}
Expand All @@ -50,8 +53,10 @@ func MapReference(ctx cpi.Context, u *cpi.UniformRepositorySpec) (cpi.Repository
}
}
mode := accessobj.ACC_WRITABLE
createHint := accessio.FormatNone
if create {
mode |= accessobj.ACC_CREATE
createHint = f
}
return NewRepositorySpec(mode, path, accessio.FileFormatForType(u.Type), accessio.PathFileSystem(fs))
return NewRepositorySpec(mode, path, createHint, accessio.PathFileSystem(fs))
}
4 changes: 4 additions & 0 deletions pkg/contexts/ocm/grammar/grammar.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,3 +67,7 @@ var (
Optional(Literal(VersionSeparator), Capture(VersionRegexp))),
)
)

func SplitTypeSpec(t string) (string, string) {
return grammar.SplitTypeSpec(t)
}
1 change: 1 addition & 0 deletions pkg/contexts/ocm/internal/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,7 @@ func (c *_context) SetAlias(name string, spec RepositorySpec) {
}

func (c *_context) GetResolver() ComponentVersionResolver {
c.Update()
if len(c.resolver.rules) == 0 {
return nil
}
Expand Down
26 changes: 21 additions & 5 deletions pkg/contexts/ocm/ref.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"github.com/open-component-model/ocm/pkg/contexts/ocm/cpi"
"github.com/open-component-model/ocm/pkg/contexts/ocm/grammar"
"github.com/open-component-model/ocm/pkg/errors"
"github.com/open-component-model/ocm/pkg/utils"
)

const (
Expand All @@ -37,25 +38,34 @@ func ParseRepo(ref string) (UniformRepositorySpec, error) {
if match == nil {
return UniformRepositorySpec{}, errors.ErrInvalid(KIND_OCM_REFERENCE, ref)
}
h := string(match[1])
t, _ := grammar.SplitTypeSpec(h)
return cpi.HandleRef(UniformRepositorySpec{
Type: string(match[1]),
Type: t,
TypeHint: h,
Info: string(match[2]),
CreateIfMissing: create,
})
}
h := string(match[1])
t, _ := grammar.SplitTypeSpec(h)
return cpi.HandleRef(UniformRepositorySpec{
Type: string(match[1]),
Type: t,
TypeHint: h,
Host: string(match[2]),
SubPath: string(match[3]),
CreateIfMissing: create,
})
}

func ParseRepoToSpec(ctx Context, ref string) (RepositorySpec, error) {
func ParseRepoToSpec(ctx Context, ref string, create ...bool) (RepositorySpec, error) {
uni, err := ParseRepo(ref)
if err != nil {
return nil, errors.ErrInvalidWrap(err, KIND_REPOSITORYSPEC, ref)
}
if !uni.CreateIfMissing {
uni.CreateIfMissing = utils.Optional(create...)
}
repoSpec, err := ctx.MapUniformRepositorySpec(&uni)
if err != nil {
return nil, errors.ErrInvalidWrap(err, KIND_REPOSITORYSPEC, ref)
Expand Down Expand Up @@ -86,9 +96,12 @@ func ParseRef(ref string) (RefSpec, error) {
return RefSpec{}, errors.ErrInvalid(KIND_OCM_REFERENCE, ref)
}
v = string(match[4])
h := string(match[1])
t, _ := grammar.SplitTypeSpec(h)
spec = RefSpec{
UniformRepositorySpec{
Type: string(match[1]),
Type: t,
TypeHint: h,
Info: string(match[2]),
CreateIfMissing: create,
},
Expand All @@ -99,9 +112,12 @@ func ParseRef(ref string) (RefSpec, error) {
}
} else {
v = string(match[5])
h := string(match[1])
t, _ := grammar.SplitTypeSpec(h)
spec = RefSpec{
UniformRepositorySpec{
Type: string(match[1]),
Type: t,
TypeHint: h,
Host: string(match[2]),
SubPath: string(match[3]),
CreateIfMissing: create,
Expand Down
13 changes: 9 additions & 4 deletions pkg/contexts/ocm/ref_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (

"github.com/open-component-model/ocm/pkg/contexts/ocm"
"github.com/open-component-model/ocm/pkg/contexts/ocm/repositories/ocireg"
"github.com/open-component-model/ocm/pkg/utils"
)

func Type(t string) string {
Expand All @@ -34,11 +35,14 @@ func Vers(t string) string {
return ":" + t
}

func CheckRef(ref, ut, h, us, c, uv, i string) {
func CheckRef(ref, ut, h, us, c, uv, i string, th ...string) {
var v *string
if uv != "" {
v = &uv
}
if len(th) == 0 && ut != "" {
th = []string{ut}
}
spec, err := ocm.ParseRef(ref)
Expect(err).WithOffset(1).To(Succeed())
Expect(spec).WithOffset(1).To(Equal(ocm.RefSpec{
Expand All @@ -47,6 +51,7 @@ func CheckRef(ref, ut, h, us, c, uv, i string) {
Host: h,
SubPath: us,
Info: i,
TypeHint: utils.Optional(th...),
CreateIfMissing: ref[0] == '+',
},
CompSpec: ocm.CompSpec{
Expand Down Expand Up @@ -101,8 +106,8 @@ var _ = Describe("ref parsing", func() {
})

It("dir ref", func() {
CheckRef("+ctf+directory::./file//bla.blob/comp", "ctf+directory", "", "", "bla.blob/comp", "", "./file")
CheckRef("ctf+directory::./file//bla.blob/comp", "ctf+directory", "", "", "bla.blob/comp", "", "./file")
CheckRef("+ctf+directory::./file//bla.blob/comp", "ctf", "", "", "bla.blob/comp", "", "./file", "ctf+directory")
CheckRef("ctf+directory::./file//bla.blob/comp", "ctf", "", "", "bla.blob/comp", "", "./file", "ctf+directory")
CheckRef("directory::./file//bla.blob/comp", "directory", "", "", "bla.blob/comp", "", "./file")
CheckRef("directory::file//bla.blob/comp", "directory", "", "", "bla.blob/comp", "", "file")
CheckRef("directory::./file.io//bla.blob/comp", "directory", "", "", "bla.blob/comp", "", "./file.io")
Expand All @@ -120,7 +125,7 @@ var _ = Describe("ref parsing", func() {
SubPath: "dev/v1",
Info: "",
CreateIfMissing: false,
TypeHint: "",
TypeHint: "OCIRegistry",
},
CompSpec: ocm.CompSpec{
Component: "github.wdf.sap.corp/kubernetes/landscape-setup-dependencies",
Expand Down
18 changes: 8 additions & 10 deletions pkg/contexts/ocm/repositories/comparch/uniform.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"github.com/open-component-model/ocm/pkg/common/accessio"
"github.com/open-component-model/ocm/pkg/common/accessobj"
"github.com/open-component-model/ocm/pkg/contexts/datacontext/attrs/vfsattr"
"github.com/open-component-model/ocm/pkg/contexts/oci/repositories/ctf"
"github.com/open-component-model/ocm/pkg/contexts/ocm/cpi"
)

Expand All @@ -17,11 +16,6 @@ func init() {
cpi.RegisterRepositorySpecHandler(h, "")
cpi.RegisterRepositorySpecHandler(h, Type)
cpi.RegisterRepositorySpecHandler(h, "ca")
for _, f := range ctf.SupportedFormats() {
cpi.RegisterRepositorySpecHandler(h, string(f))
cpi.RegisterRepositorySpecHandler(h, "ca+"+string(f))
cpi.RegisterRepositorySpecHandler(h, Type+"+"+string(f))
}
}

type repospechandler struct{}
Expand All @@ -35,12 +29,14 @@ func (h *repospechandler) MapReference(ctx cpi.Context, u *cpi.UniformRepository
path = u.Host
}
fs := vfsattr.Get(ctx)
hint := u.TypeHint

typ, _ := accessobj.MapType(u.Type, Type, accessio.FormatNone, true, "ca")
hint, f := accessobj.MapType(u.TypeHint, Type, accessio.FormatDirectory, false, "ca")
if !u.CreateIfMissing {
hint = ""
}
create, ok, err := accessobj.CheckFile(Type, hint, accessio.TypeForType(u.Type) == Type, path, fs, ComponentDescriptorFileName)
if !ok || err != nil {
create, ok, err := accessobj.CheckFile(Type, hint, accessio.TypeForTypeSpec(typ) == Type, path, fs, ComponentDescriptorFileName)
if !ok || (err != nil && typ == "") {
if err != nil {
return nil, err
}
Expand All @@ -49,8 +45,10 @@ func (h *repospechandler) MapReference(ctx cpi.Context, u *cpi.UniformRepository
}
}
mode := accessobj.ACC_WRITABLE
createHint := accessio.FormatNone
if create {
mode |= accessobj.ACC_CREATE
createHint = f
}
return NewRepositorySpec(mode, path, accessio.FileFormatForType(u.Type), accessio.PathFileSystem(fs))
return NewRepositorySpec(mode, path, createHint, accessio.PathFileSystem(fs))
}
Loading

0 comments on commit 6738019

Please sign in to comment.