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

V1.5 dev resourceReferenceChoice ref clarifications #251

Conversation

jkowalleck
Copy link
Member

@jkowalleck jkowalleck commented Jun 17, 2023

@mrutkows could you help improve the schema in terms of documentation?
the resourceReferenceChoice.ref is intended to link to a bom-ref.

Question: Must this link to a bom-ref in the same document, or would it also be okay to link to a bom-ref in another document aswell, using a BOM-Link?
depending on your answer, I might need to modify the content of this PR.

see comments below ...


there was some documentation enhancement brought into the schema to make this much clearer.
followup of #236 & #222
caused by #136 and #217

Signed-off-by: Jan Kowalleck <jan.kowalleck@gmail.com>
@jkowalleck jkowalleck added this to the 1.5 milestone Jun 17, 2023
@jkowalleck jkowalleck requested review from mrutkows and a team June 17, 2023 17:49
@jkowalleck jkowalleck changed the base branch from master to v1.5-dev June 17, 2023 17:50
@jkowalleck jkowalleck marked this pull request as draft June 17, 2023 17:51
@jkowalleck jkowalleck changed the title V1.5 dev resourceReferenceChoice ref clarifications [WIP] V1.5 dev resourceReferenceChoice ref clarifications Jun 17, 2023
@jkowalleck
Copy link
Member Author

current proposal was designed to document that the linked bom-ref must be in the same document. it is possible to link to external documents only, instead. or allow link to same AND external documents.

Need an answer: what should it be? Either any document or same document only or external document only?

Will modify this PR depending on the answer.

@mrutkows
Copy link
Contributor

mrutkows commented Jun 19, 2023

@jkowalleck You have it correct in its intent. That is, for workflows (formulation) it was intended to link to resources (components/services) in the same BOM. The "choice" allows you to use an externalReference (which is what was used for a bom-link) although I have seen hints of discussions a couple weeks ago about making an independent object for BOMLink. If indeed there is a movement away from externalReference for bom-link (that is, it will be deprecated, then that new option could possibly be added to the "choice" object.

The whole reason choice is exists is to link to resources locally, via bom-ref or externally via externalReference (as these were the only two options I was aware of at time of authoring).

@jkowalleck
Copy link
Member Author

@mrutkows
current draft is "either bomRef in current document, or externalReference".
And keep in mind: "externalReference" was already modified to be "either URI or BOM-Link." see https://github.com/CycloneDX/specification/blob/v1.5-dev/schema/bom-1.5.xsd#L1405

@jkowalleck jkowalleck changed the title [WIP] V1.5 dev resourceReferenceChoice ref clarifications V1.5 dev resourceReferenceChoice ref clarifications Jun 19, 2023
@jkowalleck jkowalleck marked this pull request as ready for review June 19, 2023 12:59
@jkowalleck jkowalleck marked this pull request as draft June 19, 2023 13:00
@jkowalleck
Copy link
Member Author

as it was intended to link to components/services in current or external docuemnts (#251 (comment))
i wil also add the dedicated element for it.

@mrutkows
Copy link
Contributor

mrutkows commented Jun 19, 2023

@jkowalleck hehe, I do not "remember" (as I only saw some hints in emails) as this was never discussed in either the Formulation, Attestations or ML work groups I attended ;) and really have no idea what that means in terms of invalidating the use of externalReference.

Also, it appears from your link to the XSD that it is an addition (i.e., "union") which now includes a new explicit type... which is fine the way I read it (guessing from the XSD terminology). That is, it is just a new way to use externalReference to link to another resource's BOM which is fine.

@jkowalleck
Copy link
Member Author

jkowalleck commented Jun 19, 2023

@mrutkows

Unfortunately, there was no hint in the docs, how a BOM-link could be detected or where it was allowed to be used.
Just some pamphlet and texts existed, some questions, discussions and issues in GitHub mentioned details.
Rules of thump from @stevespringett was: every URI could be a BOM-Link, if it had the expected format, every value of "ref" could be a BOM-Link if it had the expected format .

I do not "remember" (as I only saw some hints in emails) as this was never discussed in either the Formulation, Attestations or ML work groups I attended ;) and really have no idea what that means in terms of invalidating the use of externalReference.

changes were purely documentations, no need for any working group, as @stevespringett told me. Strings are still strings, URIs are still URIs.

externalReference is still valid and unchanged; externalReference.url is still an iri-reference/anyURI.
the only thing is that via BOM-link it was agreed that a URI(iri-reference/anyURI) in the format urn:cdx:<document-id>/<document-version>[#<bom-ref-value>] had a special meaning.
read https://cyclonedx.org/capabilities/bomlink/

To improve the situation, a new type for BOM-Link was introduced in the schema, which is just another iri-reference/anyURI with some documentation and a regex constraint.
All existing iri-reference/anyURI were reviewed and some were extended, so that they could be either the general URI(just like before), or be a special URI in the format of a BOM-Link.
Since old type and new type are just the same, nothing actually changed in the matter of data formats, but it now is clear from the schema, where a BOM-link could be used, and which kind of BOM-Link was acceptable.

That is, it is just a new way to use externalReference to link to another resource's BOM which is fine.

exactly. No format changes, just documentation and helpers for machine readability.
previously we had things being arbitrary string, now they are arbitrary string or another special-format of string.
previously we had things being arbitrary URI, now they are an arbitrary URI or another special-format of URI.

Signed-off-by: Jan Kowalleck <jan.kowalleck@gmail.com>
@jkowalleck jkowalleck marked this pull request as ready for review June 19, 2023 13:38
@jkowalleck
Copy link
Member Author

jkowalleck commented Jun 19, 2023

@mrutkows could you review the PR again, and mark it as approved/rejected ?

changes since your previous comment:

  • resourceReferenceChoice.ref could also be a BOM-Link, not only any string
    (implications: no change. type was a string already, now it is either an arbitrary string or a special formatted string)

@@ -3645,7 +3645,10 @@
"ref": {
"title": "BOM Reference",
"description": "References an object by its bom-ref attribute",
"$ref": "#/definitions/refType"
Copy link
Member Author

Choose a reason for hiding this comment

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

was any string

@@ -3645,7 +3645,10 @@
"ref": {
"title": "BOM Reference",
"description": "References an object by its bom-ref attribute",
"$ref": "#/definitions/refType"
"anyOf": [
{"$ref": "#/definitions/refLinkType"},
Copy link
Member Author

Choose a reason for hiding this comment

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

is any string

"$ref": "#/definitions/refType"
"anyOf": [
{"$ref": "#/definitions/refLinkType"},
{"$ref": "#/definitions/bomLinkElementType"}
Copy link
Member Author

Choose a reason for hiding this comment

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

is any string with iri-reference format and a certain regex/format/pattern for BOM-Link

@@ -4356,12 +4356,15 @@ limitations under the License.
<xs:complexType name="resourceReferenceType">
<xs:sequence>
<xs:choice>
<xs:element name="ref" type="bom:refType" minOccurs="1" maxOccurs="1">
Copy link
Member Author

Choose a reason for hiding this comment

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

was any string

<xs:annotation>
<xs:documentation>
References an object by its bom-ref attribute
</xs:documentation>
</xs:annotation>
<xs:simpleType>
<xs:union memberTypes="bom:refLinkType bom:bomLinkElementType"/>
Copy link
Member Author

Choose a reason for hiding this comment

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

is any of:

  • bom:refLinkType: any string
  • bom:bomLinkElementType: anyURI string with a certain pattern/format for BOM-Link

@mrutkows
Copy link
Contributor

@jkowalleck Thanks for educating me on these new definitions. It appears the important part, relative to workflow, is whereever we reference the resourceReferenceChoice type which is more expansive to include the latest (2) ways to link to a traditional "bom-ref" (i.e., refLinkType and bomLinkElementType ) and now has this definition:

  "properties": {
    "ref": {
      "title": "BOM Reference",
      "description": "References an object by its bom-ref attribute",
      "anyOf": [
        {"$ref": "#/definitions/refLinkType"},
        {"$ref": "#/definitions/bomLinkElementType"}
      ]
    },
    "externalReference": {
      "title": "External reference",
      "description": "Reference to an externally accessible resource.",
      "$ref": "#/definitions/externalReference"
    }
  },

It looks quite good and understand the rationale (sorry I was not aware before now). I approve this change, but please know I will not have time to "test" this change anytime soon. Looking purely at the schema, it should work for the original "bom-ref" case (as part of the "choice"). Thanks again.

Copy link
Contributor

@mrutkows mrutkows left a comment

Choose a reason for hiding this comment

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

Please see my summary comments above.

@stevespringett
Copy link
Member

LGTM @jkowalleck. If it's passing the tests, please merge this in.

@stevespringett stevespringett merged commit 2003555 into CycloneDX:v1.5-dev Jun 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants