Skip to content

Commit

Permalink
fix field name for srcRefs (#651)
Browse files Browse the repository at this point in the history
## Description

There is an inconsistency between the json schemas and the json field
names for the component descriptor
versions. The original spec as well as the schemas use the name
`srcRefs` for the list of source references
in am entry of the resource list.

In the ocm code base the field name `srcRef` is used for the Go structs
as well as the json annotation.

Unfortunately the scheme allows for additional fields, therefore the
mismatch never provided an error.

Therefore it might have happened, that wrong component descriptors using
the singular form have
been created.

Because of the differentiation of the internal and external
representation it is easily possible
to accept both versions for deserialization, but the serialization
provides only the correct form, now.

## What type of PR is this? (check all applicable)

- [ ] 🍕 Feature
- [x] 🐛 Bug Fix
- [x] 📝 Documentation Update
- [ ] 🎨 Style
- [ ] 🧑‍💻 Code Refactor
- [ ] 🔥 Performance Improvements
- [x] ✅ Test
- [ ] 🤖 Build
- [ ] 🔁 CI
- [ ] 📦 Chore (Release)
- [ ] ⏩ Revert

## Related Tickets & Documents

<!-- 
Please use this format link issue numbers: Fixes #123

https://docs.github.com/en/free-pro-team@latest/github/managing-your-work-on-github/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword
-->
- Related Issue # (issue)
- Closes # (issue)
- Fixes # (issue)
> Remove if not applicable

## Screenshots

<!-- Visual changes require screenshots -->


## Added tests?

- [x] 👍 yes
- [ ] 🙅 no, because they aren't needed
- [ ] 🙋 no, because I need help
- [ ] Separate ticket for tests # (issue/pr)

Please describe the tests that you ran to verify your changes. Provide
instructions so we can reproduce. Please also list any relevant details
for your test configuration


## Added to documentation?

- [ ] 📜 README.md
- [ ] 🙅 no documentation needed

## Checklist:

- [ ] My code follows the style guidelines of this project
- [ ] I have performed a self-review of my code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [ ] My changes generate no new warnings
- [ ] I have added tests that prove my fix is effective or that my
feature works
- [ ] New and existing unit tests pass locally with my changes
- [ ] Any dependent changes have been merged and published in downstream
modules
  • Loading branch information
mandelsoft authored Feb 16, 2024
1 parent ed906a3 commit 34e6d22
Show file tree
Hide file tree
Showing 23 changed files with 198 additions and 47 deletions.
15 changes: 8 additions & 7 deletions cmds/ocm/commands/ocmcmds/common/addhdlrs/rscs/elements.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,9 +101,9 @@ func (h ResourceSpecHandler) Set(v ocm.ComponentVersionAccess, r addhdlrs.Elemen
ExtraIdentity: spec.ExtraIdentity,
Labels: spec.Labels,
},
Type: spec.Type,
Relation: spec.Relation,
SourceRef: compdescv2.ConvertSourcerefsTo(spec.SourceRef),
Type: spec.Type,
Relation: spec.Relation,
SourceRefs: compdescv2.ConvertSourcerefsTo(spec.SourceRefs),
}
opts := h.getModOpts()
if ocm.IsIntermediate(v.Repository().GetSpecification()) {
Expand All @@ -124,9 +124,10 @@ type ResourceSpec struct {
// Can be a local or external resource
Relation metav1.ResourceRelation `json:"relation,omitempty"`

// SourceRef defines a list of source names.
// These names reference the sources defines in `component.sources`.
SourceRef []compdescv2.SourceRef `json:"srcRef"`
// SourceRefs defines a list of source names.
// These entries reference the sources defined in the
// component.sources.
SourceRefs []compdescv2.SourceRef `json:"srcRefs"`

addhdlrs.ResourceInput `json:",inline"`
}
Expand Down Expand Up @@ -160,7 +161,7 @@ func (r *ResourceSpec) Validate(ctx clictx.Context, input *addhdlrs.ResourceInpu
ElementMeta: r.ElementMeta,
Type: r.Type,
Relation: r.Relation,
SourceRef: r.SourceRef,
SourceRefs: r.SourceRefs,
}
if err := compdescv2.ValidateResource(fldPath, rsc, false); err != nil {
allErrs = append(allErrs, err...)
Expand Down
4 changes: 2 additions & 2 deletions docs/ocm/model.md
Original file line number Diff line number Diff line change
Expand Up @@ -346,9 +346,9 @@ A resource uses the following additional formal fields:
the resource description. This is required because there might be different
digest and resource normalization algorithms.

- **`srcRef`** (optional) *struct*
- **`srcRefs`** (optional) *[]struct*

This field is used to describe the sources used to generate the resource.
This field describes a list of sources used to generate the resource.
The selection is done by the following two fields:

- **`identitySelector`** *map[string]string*
Expand Down
11 changes: 6 additions & 5 deletions pkg/contexts/ocm/compdesc/componentdescriptor.go
Original file line number Diff line number Diff line change
Expand Up @@ -635,7 +635,7 @@ func (r *Resource) Equivalent(e ElementMetaAccessor) equivalent.EqualState {
return state
} else {
// not delegated to ResourceMeta, because the significance of digests can only be determined at the Resource level.
state := equivalent.StateLocalHashEqual(r.Type == o.Type && r.Relation == o.Relation && reflect.DeepEqual(r.SourceRef, o.SourceRef))
state := equivalent.StateLocalHashEqual(r.Type == o.Type && r.Relation == o.Relation && reflect.DeepEqual(r.SourceRefs, o.SourceRefs))

if !IsNoneAccess(r.Access) || !IsNoneAccess(o.Access) {
state = state.Apply(r.Digest.Equivalent(o.Digest))
Expand Down Expand Up @@ -664,9 +664,10 @@ type ResourceMeta struct {
// Can be a local or external resource
Relation metav1.ResourceRelation `json:"relation,omitempty"`

// SourceRef defines a list of source names.
// These names reference the sources defines in `component.sources`.
SourceRef SourceRefs `json:"srcRef,omitempty"`
// SourceRefs defines a list of source names.
// These entries reference the sources defined in the
// component.sources.
SourceRefs SourceRefs `json:"srcRefs,omitempty"`

// Digest is the optional digest of the referenced resource.
// +optional
Expand Down Expand Up @@ -704,7 +705,7 @@ func (o *ResourceMeta) Copy() *ResourceMeta {
ElementMeta: *o.ElementMeta.Copy(),
Type: o.Type,
Relation: o.Relation,
SourceRef: o.SourceRef.Copy(),
SourceRefs: o.SourceRefs.Copy(),
Digest: o.Digest.Copy(),
}
return r
Expand Down
6 changes: 3 additions & 3 deletions pkg/contexts/ocm/compdesc/normalizations/jsonv2/norm.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,9 @@ var CDExcludes = signing.MapExcludes{
"resources": signing.DynamicArrayExcludes{
ValueMapper: rules.MapResourcesWithNoneAccess,
Continue: signing.MapExcludes{
"access": nil,
"srcRef": nil,
"labels": rules.LabelExcludes,
"access": nil,
"srcRefs": nil,
"labels": rules.LabelExcludes,
},
},
"sources": signing.ArrayExcludes{
Expand Down
2 changes: 1 addition & 1 deletion pkg/contexts/ocm/compdesc/testutils/compnametest.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ func TestCompName(dataBytes []byte, err error) {
var scheme map[string]interface{}
Expect(runtime.DefaultYAMLEncoding.Unmarshal(dataBytes, &scheme)).To(Succeed())

pattern := scheme["definitions"].(map[string]interface{})["componentName"].(map[string]interface{})["pattern"].(string)
pattern := scheme["$defs"].(map[string]interface{})["componentName"].(map[string]interface{})["pattern"].(string)

logrus.Infof("pattern=%s", pattern)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -236,8 +236,12 @@ type Resource struct {
// Can be a local or external resource
Relation metav1.ResourceRelation `json:"relation,omitempty"`

// SourceRef defines a list of source names.
// These names reference the sources defines in `component.sources`.
// SourceRefs defines a list of source names.
// These entries reference the sources defined in the
// component.sources.
SourceRefs []SourceRef `json:"srcRefs,omitempty"`
// SourceRef is for deserialization compatibility, only.
// The usage of this field in external formats is deprecated.
SourceRef []SourceRef `json:"srcRef,omitempty"`

// Access describes the type specific method to
Expand Down

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,9 @@ var CDExcludes = signing.MapExcludes{
"resources": signing.DynamicArrayExcludes{
ValueMapper: rules.MapResourcesWithNoneAccess,
Continue: signing.MapExcludes{
"access": nil,
"srcRef": nil,
"labels": rules.LabelExcludes,
"access": nil,
"srcRefs": nil,
"labels": rules.LabelExcludes,
},
},
"sources": signing.ArrayExcludes{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ func ValidateResources(fldPath *field.Path, resources Resources, componentVersio
localPath := fldPath.Index(i)
allErrs = append(allErrs, ValidateResource(localPath, res, true)...)

if err := ValidateSourceRefs(localPath.Child("sourceRef"), res.SourceRef); err != nil {
if err := ValidateSourceRefs(localPath.Child("sourceRef"), res.SourceRefs); err != nil {
allErrs = append(allErrs, err...)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,12 +141,16 @@ func convertElementmetaTo(in ElementMeta) compdesc.ElementMeta {
}

func convertResourceTo(in Resource) compdesc.Resource {
srcRefs := ConvertSourcerefsTo(in.SourceRefs)
if srcRefs == nil {
srcRefs = ConvertSourcerefsTo(in.SourceRef)
}
return compdesc.Resource{
ResourceMeta: compdesc.ResourceMeta{
ElementMeta: convertElementmetaTo(in.ElementMeta),
Type: in.Type,
Relation: in.Relation,
SourceRef: ConvertSourcerefsTo(in.SourceRef),
SourceRefs: srcRefs,
Digest: in.Digest.Copy(),
},
Access: compdesc.GenericAccessSpec(in.Access),
Expand Down Expand Up @@ -273,7 +277,7 @@ func convertResourceFrom(in compdesc.Resource) Resource {
ElementMeta: convertElementmetaFrom(in.ElementMeta),
Type: in.Type,
Relation: in.Relation,
SourceRef: convertSourcerefsFrom(in.SourceRef),
SourceRefs: convertSourcerefsFrom(in.SourceRefs),
Access: acc,
Digest: in.Digest.Copy(),
}
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 6 additions & 2 deletions pkg/contexts/ocm/compdesc/versions/v2/componentdescriptor.go
Original file line number Diff line number Diff line change
Expand Up @@ -293,8 +293,12 @@ type Resource struct {
// Can be a local or external resource
Relation metav1.ResourceRelation `json:"relation,omitempty"`

// SourceRef defines a list of source names.
// These names reference the sources defines in `component.sources`.
// SourceRefs defines a list of source names.
// These entries reference the sources defined in the
// component.sources.
SourceRefs []SourceRef `json:"srcRefs,omitempty"`
// SourceRef is for deserialization compatibility, only.
// The usage of this field in external formats is deprecated.
SourceRef []SourceRef `json:"srcRef,omitempty"`

// Access describes the type specific method to
Expand Down

Large diffs are not rendered by default.

6 changes: 3 additions & 3 deletions pkg/contexts/ocm/compdesc/versions/v2/signing.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,9 @@ var CDExcludes = signing.MapExcludes{
Next: signing.DynamicArrayExcludes{
ValueMapper: rules.MapResourcesWithNoneAccess,
Continue: signing.MapExcludes{
"access": nil,
"srcRef": nil,
"labels": rules.LabelExcludes,
"access": nil,
"srcRefs": nil,
"labels": rules.LabelExcludes,
},
},
}.EnforceNull("extraIdentity"),
Expand Down
2 changes: 1 addition & 1 deletion pkg/contexts/ocm/compdesc/versions/v2/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ func ValidateResources(fldPath *field.Path, resources Resources, componentVersio
localPath := fldPath.Index(i)
allErrs = append(allErrs, ValidateResource(localPath, res, true)...)

if err := ValidateSourceRefs(localPath.Child("sourceRef"), res.SourceRef); err != nil {
if err := ValidateSourceRefs(localPath.Child("sourceRef"), res.SourceRefs); err != nil {
allErrs = append(allErrs, err...)
}

Expand Down
40 changes: 40 additions & 0 deletions pkg/contexts/ocm/compdesc/versions/v2/validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,17 @@
package v2_test

import (
"encoding/json"
"fmt"
"strings"
"testing"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
. "github.com/onsi/gomega/gstruct"
"github.com/open-component-model/ocm/pkg/contexts/ocm/compdesc"
. "github.com/open-component-model/ocm/pkg/contexts/ocm/compdesc/versions/v2"
. "github.com/open-component-model/ocm/pkg/testutils"

"k8s.io/apimachinery/pkg/util/validation/field"

Expand Down Expand Up @@ -252,6 +257,41 @@ var _ = Describe("Validation", func() {
})

Context("#Resources", func() {
It("should handle srcRefs", func() {
comp.Name = "acme.org/test"
comp.RepositoryContexts = []*runtime.UnstructuredTypedObject{}
comp.ComponentReferences = ComponentReferences{}
comp.Sources = Sources{}
comp.Resources = []Resource{
{
ElementMeta: ElementMeta{
Name: "test",
Version: "v1",
},
Type: "test",
Relation: meta.LocalRelation,
Access: runtime.NewEmptyUnstructured("access"),
SourceRefs: []SourceRef{
{
IdentitySelector: map[string]string{
"name": "test",
},
Labels: nil,
},
},
},
}
v := &DescriptorVersion{}
data := Must(json.Marshal(comp))
fmt.Printf("%s\n", string(data))
r := Must(v.Decode(data, &compdesc.DecodeOptions{Codec: compdesc.DefaultYAMLCodec}))
Expect(r).To(DeepEqual(comp))

old := strings.Replace(string(data), "srcRefs", "srcRef", -1)
r = Must(v.Decode([]byte(old), &compdesc.DecodeOptions{Codec: compdesc.DefaultYAMLCodec}))
Expect(v.ConvertTo(r)).To(DeepEqual(Must(v.ConvertTo(comp))))
})

It("should forbid if a resource name contains invalid characters", func() {
comp.Resources = []Resource{
{
Expand Down
8 changes: 6 additions & 2 deletions pkg/contexts/ocm/compdesc/versions/v2/version.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,12 +150,16 @@ func convertElementMetaTo(in ElementMeta) compdesc.ElementMeta {
}

func convertResourceTo(in Resource) compdesc.Resource {
srcRefs := ConvertSourcerefsTo(in.SourceRefs)
if srcRefs == nil {
srcRefs = ConvertSourcerefsTo(in.SourceRef)
}
return compdesc.Resource{
ResourceMeta: compdesc.ResourceMeta{
ElementMeta: convertElementMetaTo(in.ElementMeta),
Type: in.Type,
Relation: in.Relation,
SourceRef: ConvertSourcerefsTo(in.SourceRef),
SourceRefs: srcRefs,
Digest: in.Digest.Copy(),
},
Access: compdesc.GenericAccessSpec(in.Access),
Expand Down Expand Up @@ -295,7 +299,7 @@ func convertResourceFrom(in compdesc.Resource) Resource {
ElementMeta: convertElementMetaFrom(in.ElementMeta),
Type: in.Type,
Relation: in.Relation,
SourceRef: convertSourceRefsFrom(in.SourceRef),
SourceRefs: convertSourceRefsFrom(in.SourceRefs),
Access: acc,
Digest: in.Digest.Copy(),
}
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion pkg/contexts/ocm/elements/resources.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func (o *srcref) ApplyToResourceMeta(m *compdesc.ResourceMeta) error {
if err := o.errlist.Result(); err != nil {
return err
}
m.SourceRef = append(m.SourceRef, compdesc.SourceRef{IdentitySelector: o.ref.Copy(), Labels: o.labels.Copy()})
m.SourceRefs = append(m.SourceRefs, compdesc.SourceRef{IdentitySelector: o.ref.Copy(), Labels: o.labels.Copy()})
return nil
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/contexts/ocm/elements/resources_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ labels:
value:
field: value
signing: true
srcRef:
srcRefs:
- identitySelector:
name: image
labels:
Expand Down
Loading

0 comments on commit 34e6d22

Please sign in to comment.