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

feat(normalisation): jsonNormalisation/v3 and fixes to jsonNormalisation/v1 as well as jsonNormalisation/v2 #1218

Conversation

jakobmoellerdev
Copy link
Contributor

@jakobmoellerdev jakobmoellerdev commented Jan 3, 2025

What this PR does / why we need it

Package jsonv3 provides a normalization which is completely based on the abstract (internal) version of the component descriptor and is therefore agnostic of the final serialization format. Signatures using this algorithm can be transferred among different schema versions, as long as is able to handle the complete information using for the normalization. jsonv2 is the predecessor of this version but had internal defaulting logic that is no longer included as part of this normalization. Thus v3 should be preferred over v2. Note that between v2 and v3 differences can occur mainly if the "extra identity" field is not unique, in which case the v2 normalization opinionated on how to differentiate these items. This no longer happens in v3, meaning the component descriptor is normalized as is.

v2 and v1 were adjusted to accomodate the old(but new because forgotten) legacy behavior in legacy.go. Without this, old signatures would not work. This means this should be (at least partially) back-ported to the last minor versioned released without this correction.

Which issue(s) this PR fixes

This issue fixes #1214 and supercedes #1217 as a better solution longterm (by getting rid of the old normalization) and shortterm (by achieving full backwards compatibility + introducing a simple test case)

Note that this changes the default normalization algorithm to be jsonNormalisation/v3 instead of jsonNormalisation/v1 as it is important for users to migrate as soon as possible.

@jakobmoellerdev jakobmoellerdev requested a review from a team as a code owner January 3, 2025 16:34
@github-actions github-actions bot added kind/feature new feature, enhancement, improvement, extension area/documentation Documentation related component/ocm-cli OCM Command Line Interface component/github-actions Changes on GitHub Actions or within `.github/` directory size/m Medium labels Jan 3, 2025
@jakobmoellerdev jakobmoellerdev force-pushed the feat/jsonNormalisation/v3 branch 3 times, most recently from 4c85f74 to 0a1ccc5 Compare January 3, 2025 17:20
Package jsonv3 provides a normalization which is completely based on the
abstract (internal) version of the component descriptor and is therefore
agnostic of the final serialization format. Signatures using this algorithm
can be transferred among different schema versions, as long as is able to
handle the complete information using for the normalization.
jsonv2 is the predecessor of this version but had internal defaulting logic
that is no longer included as part of this normalization. Thus v3 should be preferred over v2.
Note that between v2 and v3 differences can occur mainly if the "extra identity" field is not unique,
in which case the v2 normalization opinionated on how to differentiate these items. This no longer
happens in v3, meaning the component descriptor is normalized as is.

v2 and v1 were adjusted to accomodate the old(but new because forgotten) legacy behavior in legacy.go. Without this, old signatures would not work
@jakobmoellerdev jakobmoellerdev force-pushed the feat/jsonNormalisation/v3 branch from 0a1ccc5 to dc977e1 Compare January 3, 2025 18:07
Copy link
Contributor

@Skarlso Skarlso left a comment

Choose a reason for hiding this comment

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

One question, otherwise, lovely job Jakob! :)

Copy link
Contributor

@mandelsoft mandelsoft left a comment

Choose a reason for hiding this comment

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

I'm not sure, why you created a new normalization v3, which is basically exactly the same as v2, before, and changed v2 to modify the component descriptor.

Normalization should never change anything.

The issue stated, that the problem arises after a transport, therefore it cannot be caused by the normalization, but by the transport handling of the extra identity (as you correctly identified in your comment in the issue).

So, it is caused by the defaulting done in the SetResource and SetSource methods of a CV. If modify mode is on, it defaults the extra identity according to the new rules.
This has been intended for the creation of a component version, but this should never happen during a transport.

Therefore, we have to fix the transport behavior, ere. This is only possible by introducing a new modification option for those methods, which instruct the adding to keep the original method data and do not the defaulting. The transport tool then has to give this option. (see #1223)

@jakobmoellerdev
Copy link
Contributor Author

I'm not sure, why you created a new normalization v3, which is basically exactly the same as v2, before, and changed v2 to modify the component descriptor.

Normalization should never change anything.

I changed it because your change broke existing logic for normalization. Before, it didnt matter if the component versions contained extra identities or not, now it does. V3 simply makes this break obvious.

The issue stated, that the problem arises after a transport, therefore it cannot be caused by the normalization, but by the transport handling of the extra identity (as you correctly identified in your comment in the issue).

So, it is caused by the defaulting done in the SetResource and SetSource methods of a CV. If modify mode is on, it defaults the extra identity according to the new rules. This has been intended for the creation of a component version, but this should never happen during a transport.

This is true, but at the same time, the normalization is not allowed to change. I still think the change you submitted is relevant, but the normalization must stay the same.

Therefore, we have to fix the transport behavior, ere. This is only possible by introducing a new modification option for those methods, which instruct the adding to keep the original method data and do not the defaulting. The transport tool then has to give this option. (see #1223)

Also true, but existing (already transferred) component versions have to be verifiable as well, even with the transport having changed the extra identity

@jakobmoellerdev jakobmoellerdev merged commit a93ecca into open-component-model:main Jan 8, 2025
24 of 25 checks passed
jakobmoellerdev added a commit to jakobmoellerdev/ocm that referenced this pull request Jan 8, 2025
…ion/v1 as well as jsonNormalisation/v2 (open-component-model#1218)

<!-- markdownlint-disable MD041 -->
#### What this PR does / why we need it

Package jsonv3 provides a normalization which is completely based on the
abstract (internal) version of the component descriptor and is therefore
agnostic of the final serialization format. Signatures using this
algorithm can be transferred among different schema versions, as long as
is able to handle the complete information using for the normalization.
jsonv2 is the predecessor of this version but had internal defaulting
logic that is no longer included as part of this normalization. Thus v3
should be preferred over v2. Note that between v2 and v3 differences can
occur mainly if the "extra identity" field is not unique, in which case
the v2 normalization opinionated on how to differentiate these items.
This no longer happens in v3, meaning the component descriptor is
normalized as is.

v2 and v1 were adjusted to accomodate the old(but new because forgotten)
legacy behavior in legacy.go. Without this, old signatures would not
work. This means this should be (at least partially) back-ported to the
last minor versioned released without this correction.

#### Which issue(s) this PR fixes
<!--
Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.
-->
This issue fixes open-component-model#1214
and supercedes open-component-model#1217 as
a better solution longterm (by getting rid of the old normalization) and
shortterm (by achieving full backwards compatibility + introducing a
simple test case)

Note that this changes the default normalization algorithm to be
`jsonNormalisation/v3` instead of `jsonNormalisation/v1` as it is
important for users to migrate as soon as possible.
ikhandamirov pushed a commit that referenced this pull request Jan 8, 2025
…ion/v1 as well as jsonNormalisation/v2 (#1218) (#1230)

Note that this introduces a json normalization algorithm v3 which
technically will break a minor release. however this is needed as we
currently broke hashing and need to offer a working alternative. thus it
should have been part of v0.19.0 already:
#1214

<!-- markdownlint-disable MD041 -->
#### What this PR does / why we need it

Package jsonv3 provides a normalization which is completely based on the
abstract (internal) version of the component descriptor and is therefore
agnostic of the final serialization format. Signatures using this
algorithm can be transferred among different schema versions, as long as
is able to handle the complete information using for the normalization.
jsonv2 is the predecessor of this version but had internal defaulting
logic that is no longer included as part of this normalization. Thus v3
should be preferred over v2. Note that between v2 and v3 differences can
occur mainly if the "extra identity" field is not unique, in which case
the v2 normalization opinionated on how to differentiate these items.
This no longer happens in v3, meaning the component descriptor is
normalized as is.

v2 and v1 were adjusted to accomodate the old(but new because forgotten)
legacy behavior in legacy.go. Without this, old signatures would not
work. This means this should be (at least partially) back-ported to the
last minor versioned released without this correction.

#### Which issue(s) this PR fixes
<!--
Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`. -->
This issue fixes #1214
and supercedes #1217 as
a better solution longterm (by getting rid of the old normalization) and
shortterm (by achieving full backwards compatibility + introducing a
simple test case)

Note that this changes the default normalization algorithm to be
`jsonNormalisation/v3` instead of `jsonNormalisation/v1` as it is
important for users to migrate as soon as possible.

This is a backport of
a93ecca
in #1218
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/documentation Documentation related component/github-actions Changes on GitHub Actions or within `.github/` directory component/ocm-cli OCM Command Line Interface kind/feature new feature, enhancement, improvement, extension size/m Medium
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incompatible change of OCM-Component-Descriptor-Normalisation between OCM-CLI v0.18.0 vs v0.19.0
3 participants