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

Resource equivalence contract broken in version v0.6.0 #29

Closed
Tomasz-Smelcerz-SAP opened this issue Feb 5, 2024 · 6 comments
Closed
Assignees
Labels
area/ipcei Important Project of Common European Interest kind/bugfix Bug

Comments

@Tomasz-Smelcerz-SAP
Copy link

Tomasz-Smelcerz-SAP commented Feb 5, 2024

What happened:
We use the OCM library for creating and distributing component archives in the Kyma-CLI project.
When upgrading the version from v0.4.0 to v0.6.0 I noticed that the pkg/contexts/ocm/compdesc.Resource.IsEquivalent() method is gone.
It looks like it's been replaced by a more sophisticated Equivalent(e ElementMetaAccessor) equivalent.EqualState method.
The new method has a different contract though, and I think there may be a bug there.
First, two identical Resource object copies (without optional Digest) are not considered "equivalent" anymore.
Second, two different Resource object (without optional Digest) are considered "localHashEqual".
None of the available checks work for me.

What you expected to happen:
Equality check allows to determine if two Resource objects without Digests are equal/equivalent.

How to reproduce it (as minimally and precisely as possible):
I have provided a little demo with unit tests:

Anything else we need to know:
We test resources for equality/equivalence to avoid unnecessary repository pushes (when the content doesn't change).

Environment:
n/a

@Tomasz-Smelcerz-SAP Tomasz-Smelcerz-SAP changed the title Resource equivalence contract broken in version 0.6.0 Resource equivalence contract broken in version v0.6.0 Feb 5, 2024
@morri-son
Copy link
Contributor

HI @mandelsoft , can you please have a look at this?

@morri-son morri-son added the area/ipcei Important Project of Common European Interest label Feb 6, 2024
@mandelsoft
Copy link

mandelsoft commented Feb 13, 2024

The new equivalence behaviour has been introduced to better control transports and delta transports.
Together with this, now resources digests are generated directly together with the CV creation.
The digests are required to decide whether two resources are available because the access method may be
different. So, if no digests are found, it is not possible to decide,whether source and target of a transport are
identical. If digests are not present, IsArtifactDetectable() returns false.

May be this could be improved by involving the access method, if no digest is found.

But, the digests should now always be available, because they are always created when adding a resource
(if this in not explicitly disabled by an option)

@robertwol
Copy link

Hi @Tomasz-Smelcerz-SAP, does the above answer from @mandelsoft help?

@Tomasz-Smelcerz-SAP
Copy link
Author

Hi, Thanks for reply.
Yes, that answers my question: I understand that digests are now expected to be present, so I have to adjust our code.
I was misled by this comment (in v0.6.0): https://github.com/open-component-model/ocm/blob/dfcf27e5f22d83feac7869e0f303c6e6e66f1e8d/pkg/contexts/ocm/compdesc/componentdescriptor.go#L671
Perhaps the comment could be improved somehow?

@morri-son
Copy link
Contributor

@mandelsoft, can you please check Tomasz comment?

@morri-son morri-son transferred this issue from open-component-model/ocm Mar 14, 2024
@mandelsoft
Copy link

Yes, the digest is optional. It is still possible to generate the digests on-the-fly, when they are required (during the signing process). But if the digests are not present, delta transports are not really possible, the overwrite is required.
By default, the composition calls in the CLI and the library now always calculate the digest (but you can specify options to skip this step).

@morri-son morri-son added this to the 2024-Q1 milestone Mar 25, 2024
@Skarlso Skarlso moved this from 🍺 Done to 🔒Closed in OCM Backlog Board Apr 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ipcei Important Project of Common European Interest kind/bugfix Bug
Projects
Status: 🔒Closed
Development

No branches or pull requests

4 participants