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

duplicate detection for add commands #564

Merged
merged 2 commits into from
Nov 8, 2023
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
13 changes: 13 additions & 0 deletions cmds/ocm/commands/ocmcmds/common/addhdlrs/comp/components.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,19 @@ import (
)

func ProcessComponents(ctx clictx.Context, ictx inputs.Context, repo ocm.Repository, complete ocm.ComponentVersionResolver, thdlr transferhandler.TransferHandler, h *ResourceSpecHandler, elems []addhdlrs.Element) (err error) {
list := errors.ErrorList{}

for _, elem := range elems {
if r, ok := elem.Spec().(*ResourceSpec); ok {
list.Add(addhdlrs.ValidateElementSpecIdentities("resource", elem.Source().String(), generics.ConvertSliceTo[addhdlrs.ElementSpec](r.Resources)))
list.Add(addhdlrs.ValidateElementSpecIdentities("source", elem.Source().String(), generics.ConvertSliceTo[addhdlrs.ElementSpec](r.Sources)))
list.Add(addhdlrs.ValidateElementSpecIdentities("reference", elem.Source().String(), generics.ConvertSliceTo[addhdlrs.ElementSpec](r.References)))
}
}
if err := list.Result(); err != nil {
return err
}

index := generics.Set[common.NameVersion]{}
for _, elem := range elems {
if r, ok := elem.Spec().(*ResourceSpec); ok {
Expand Down
4 changes: 4 additions & 0 deletions cmds/ocm/commands/ocmcmds/common/addhdlrs/comp/elements.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,10 @@ type ResourceSpec struct {

var _ addhdlrs.ElementSpec = (*ResourceSpec)(nil)

func (r *ResourceSpec) GetRawIdentity() metav1.Identity {
return metav1.NewIdentity(r.Name, metav1.SystemIdentityVersion, r.Version)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this constant called SystemIdentityVersion?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SystemIdentityVersion and SystemIdentityName are the two predefined identity properties use to identify elements (resources, sources, referebces) in CDs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added docu to their const definitions.

}

func (r *ResourceSpec) Info() string {
return fmt.Sprintf("component %s:%s", r.Name, r.Version)
}
Expand Down
23 changes: 23 additions & 0 deletions cmds/ocm/commands/ocmcmds/common/addhdlrs/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,25 +10,34 @@ import (

"github.com/open-component-model/ocm/cmds/ocm/commands/ocmcmds/common/inputs"
"github.com/open-component-model/ocm/pkg/contexts/clictx"
metav1 "github.com/open-component-model/ocm/pkg/contexts/ocm/compdesc/meta/v1"
"github.com/open-component-model/ocm/pkg/contexts/ocm/cpi"
"github.com/open-component-model/ocm/pkg/errors"
"github.com/open-component-model/ocm/pkg/generics"
)

// ResourceInput describe the source for the content of
// a content based element (sources or resources).
// It is either an input or access specification.
type ResourceInput struct {
Access *cpi.GenericAccessSpec `json:"access"`
// Input *inputs.BlobInput `json:"input,omitempty"`
Input *inputs.GenericInputSpec `json:"input,omitempty"`
}

// ElementSpecHandler is the interface for a handler
// responsible to handle a dedicated kind of element specification.
type ElementSpecHandler interface {
Key() string
RequireInputs() bool
Decode(data []byte) (ElementSpec, error)
}

type ElementSource interface {
// Origin provides access to the source
// specification used to provide elements.
Origin() SourceInfo
// Get provides access to the content of the element source.
Get() (string, error)
}

Expand Down Expand Up @@ -75,19 +84,33 @@ func (s *sourceInfo) Id() string {
return id
}

// ElementSpec is the specification of
// the model element. It provides access to
// common attributes, like the identity.
type ElementSpec interface {
GetName() string
GetVersion() string
SetVersion(string)
GetRawIdentity() metav1.Identity
Info() string
Validate(ctx clictx.Context, input *ResourceInput) error
}

// Element is the abstraction over model elements handled by
// the add handler, for example, resources, sources, references or complete
// component versions.
type Element interface {
// Source provides info about the source the element has been
// derived from. (for example a component.yaml or resources.yaml).
Source() SourceInfo
// Spec provides access to the element specification.
Spec() ElementSpec
// Type is used for types elements, like sources and resources.
Type() string
// Data provides access to the element descriptor representation.
Data() []byte
// Input provides access to the underlying data specification.
// It is either an access specification or an input specification.
Input() *ResourceInput
}

Expand Down
5 changes: 5 additions & 0 deletions cmds/ocm/commands/ocmcmds/common/addhdlrs/refs/elements.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"github.com/open-component-model/ocm/pkg/contexts/clictx"
"github.com/open-component-model/ocm/pkg/contexts/ocm"
"github.com/open-component-model/ocm/pkg/contexts/ocm/compdesc"
metav1 "github.com/open-component-model/ocm/pkg/contexts/ocm/compdesc/meta/v1"
compdescv2 "github.com/open-component-model/ocm/pkg/contexts/ocm/compdesc/versions/v2"
"github.com/open-component-model/ocm/pkg/runtime"
)
Expand Down Expand Up @@ -70,6 +71,10 @@ type ResourceSpec struct {

var _ addhdlrs.ElementSpec = (*ResourceSpec)(nil)

func (r *ResourceSpec) GetRawIdentity() metav1.Identity {
return r.ElementMeta.GetRawIdentity()
}

func (r *ResourceSpec) Info() string {
return fmt.Sprintf("reference %s: %s", r.ComponentName, r.GetRawIdentity())
}
Expand Down
4 changes: 4 additions & 0 deletions cmds/ocm/commands/ocmcmds/common/addhdlrs/rscs/elements.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,10 @@ type ResourceSpec struct {

var _ addhdlrs.ElementSpec = (*ResourceSpec)(nil)

func (r *ResourceSpec) GetRawIdentity() metav1.Identity {
return r.ElementMeta.GetRawIdentity()
}

func (r *ResourceSpec) Info() string {
return fmt.Sprintf("resource %s: %s", r.Type, r.GetRawIdentity())
}
Expand Down
5 changes: 5 additions & 0 deletions cmds/ocm/commands/ocmcmds/common/addhdlrs/srcs/elements.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"github.com/open-component-model/ocm/pkg/contexts/clictx"
"github.com/open-component-model/ocm/pkg/contexts/ocm"
"github.com/open-component-model/ocm/pkg/contexts/ocm/compdesc"
metav1 "github.com/open-component-model/ocm/pkg/contexts/ocm/compdesc/meta/v1"
compdescv2 "github.com/open-component-model/ocm/pkg/contexts/ocm/compdesc/versions/v2"
"github.com/open-component-model/ocm/pkg/runtime"
)
Expand Down Expand Up @@ -70,6 +71,10 @@ type ResourceSpec struct {

var _ addhdlrs.ElementSpec = (*ResourceSpec)(nil)

func (r *ResourceSpec) GetRawIdentity() metav1.Identity {
return r.ElementMeta.GetRawIdentity()
}

func (r *ResourceSpec) Info() string {
return fmt.Sprintf("source %s: %s", r.Type, r.GetRawIdentity())
}
Expand Down
41 changes: 41 additions & 0 deletions cmds/ocm/commands/ocmcmds/common/addhdlrs/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ func ProcessDescriptions(ctx clictx.Context, printer common2.Printer, templ temp
}
elems = append(elems, tmp...)
}
err := ValidateElementIdentities(h.Key(), elems)
if err != nil {
return nil, nil, err
}
ictx.Printf("found %d %s\n", len(elems), utils.Plural(h.Key(), len(elems)))
return elems, ictx, nil
}
Expand Down Expand Up @@ -279,6 +283,43 @@ func Validate(r *ResourceInput, ctx inputs.Context, inputFilePath string) error
return allErrs.ToAggregate()
}

func ValidateElementIdentities(kind string, elems []Element) error {
list := errors.ErrList()
ids := map[string]SourceInfo{}
for _, r := range elems {
var i interface{}
err := runtime.DefaultYAMLEncoding.Unmarshal(r.Data(), &i)
if err != nil {
return errors.Wrapf(err, "cannot eval data %q", string(r.Data()))
}
id := r.Spec().GetRawIdentity()
dig := id.Digest()
if s, ok := ids[string(dig)]; ok {
list.Add(fmt.Errorf("duplicate %s identity %s (%s and %s)", kind, id, r.Source(), s))
}
ids[string(dig)] = r.Source()
}
return list.Result()
}

// ValidateElementSpecIdentities validate the element specifications
// taken from some source (for example a resources.yaml or components.yaml).
// The parameter src somehow identifies the element source, for example
// the path of the parsed file.
func ValidateElementSpecIdentities(kind string, src string, elems []ElementSpec) error {
list := errors.ErrList()
ids := map[string]int{}
for i, r := range elems {
id := r.GetRawIdentity()
dig := id.Digest()
if s, ok := ids[string(dig)]; ok {
list.Add(fmt.Errorf("duplicate %s identity %s (%s index %d and %d)", kind, id, src, i+1, s+1))
}
ids[string(dig)] = i
}
return list.Result()
}

func PrintElements(p common2.Printer, elems []Element, outfile string, fss ...vfs.FileSystem) error {
if outfile != "" && outfile != "-" {
f, err := accessio.FileSystem(fss...).OpenFile(outfile, vfs.O_TRUNC|vfs.O_CREATE|vfs.O_WRONLY, 0o644)
Expand Down
23 changes: 23 additions & 0 deletions cmds/ocm/commands/ocmcmds/components/add/cmd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,29 @@ var _ = Describe("Test Environment", func() {
CheckComponent(env, nil)
})

Context("failures", func() {
It("rejects adding duplicate components", func() {
ExpectError(env.Execute("add", "c", "-fc", "--file", ARCH, "--version", "1.0.0", "testdata/components-dup.yaml")).To(
MatchError(`duplicate component identity "name"="ocm.software/demo/test","version"="1.0.0" (testdata/components-dup.yaml[1][2] and testdata/components-dup.yaml[1][1])`),
)
})
It("rejects adding duplicate resources", func() {
ExpectError(env.Execute("add", "c", "-fc", "--file", ARCH, "--version", "1.0.0", "testdata/component-dup-res.yaml")).To(
MatchError(`duplicate resource identity "name"="text" (testdata/component-dup-res.yaml[1][1] index 3 and 1)`),
)
})
It("rejects adding duplicate source", func() {
ExpectError(env.Execute("add", "c", "-fc", "--file", ARCH, "--version", "1.0.0", "testdata/component-dup-src.yaml")).To(
MatchError(`duplicate source identity "name"="source" (testdata/component-dup-src.yaml[1][1] index 2 and 1)`),
)
})
It("rejects adding duplicate reference", func() {
ExpectError(env.Execute("add", "c", "-fc", "--file", ARCH, "--version", "1.0.0", "testdata/component-dup-ref.yaml")).To(
MatchError(`duplicate reference identity "name"="ref","version"="v1" (testdata/component-dup-ref.yaml[1][1] index 2 and 1)`),
)
})
})

Context("with completion", func() {
var ldesc *artdesc.Descriptor

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
name: ocm.software/demo/test
version: 1.0.0
provider:
name: ocm.software
labels:
- name: city
value: Karlsruhe
labels:
- name: purpose
value: test

resources:
- name: text
type: PlainText
labels:
- name: city
value: Karlsruhe
merge:
algorithm: default
config:
overwrite: inbound
input:
type: file
path: testdata
- name: data
type: PlainText
input:
type: binary
data: IXN0cmluZ2RhdGE=

componentReferences:
- name: ref
version: v1
componentName: github.com/mandelsoft/test2
- name: ref
version: v1
componentName: github.com/mandelsoft/test3

Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
name: ocm.software/demo/test
version: 1.0.0
provider:
name: ocm.software
labels:
- name: city
value: Karlsruhe
labels:
- name: purpose
value: test

resources:
- name: text
type: PlainText
labels:
- name: city
value: Karlsruhe
merge:
algorithm: default
config:
overwrite: inbound
input:
type: file
path: testdata
- name: data
type: PlainText
input:
type: binary
data: IXN0cmluZ2RhdGE=
- name: text
type: PlainText
input:
type: binary
data: IXN0cmluZ2RhdGE=

componentReferences:
- name: ref
version: v1
componentName: github.com/mandelsoft/test2
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
name: ocm.software/demo/test
version: 1.0.0
provider:
name: ocm.software
labels:
- name: city
value: Karlsruhe
labels:
- name: purpose
value: test

sources:
- name: source
type: DirectoryTree
input:
type: binary
data: IXN0cmluZ2RhdGE=
- name: source
type: DirectoryTree
input:
type: binary
data: IXN0cmluZ2RhdGE=

resources:
- name: text
type: PlainText
labels:
- name: city
value: Karlsruhe
merge:
algorithm: default
config:
overwrite: inbound
input:
type: file
path: testdata
- name: data
type: PlainText
input:
type: binary
data: IXN0cmluZ2RhdGE=

componentReferences:
- name: ref
version: v1
componentName: github.com/mandelsoft/test2
Loading