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: add generated SPDX file on bottling #16594

Merged
merged 1 commit into from
May 7, 2024

Conversation

SMillerDev
Copy link
Member

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same change?
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes? Here's an example.
  • Have you successfully run brew style with your changes locally?
  • Have you successfully run brew typecheck with your changes locally?
  • Have you successfully run brew tests with your changes locally?

Attached is an example SBOM.
spdx.sbom.json

This should allow us to have some more tracking of what goes into our bottles, but also allow others to have some insight into it.

Copy link
Member

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

  • feels a bit weird this living on Tab when it's got little in common there
  • do we have requests for these files anywhere you can link to?

Library/Homebrew/dev-cmd/bottle.rb Outdated Show resolved Hide resolved
Library/Homebrew/tab.rb Outdated Show resolved Hide resolved
Library/Homebrew/tab.rb Outdated Show resolved Hide resolved
@@ -407,4 +430,151 @@ def to_s
end
s.join(" ")
end

sig { returns(Hash) }
def to_spdx_sbom
Copy link
Member

Choose a reason for hiding this comment

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

Is this an officially defined format somewhere? If so: we should be using some sort of validator here ideally both at brew bottle and brew tests times.

Copy link
Member

Choose a reason for hiding this comment

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

I believe the SPDX spec is defined here: https://spdx.github.io/spdx-spec/v2.3/

In terms of a Ruby validator, I'm not sure. I can look into this 🙂

Copy link

@jkowalleck jkowalleck Mar 23, 2024

Choose a reason for hiding this comment

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

full ack. get the results validated. the current implementation produces invalid data.
see #16594 (comment)

@SMillerDev
Copy link
Member Author

feels a bit weird this living on Tab when it's got little in common there

Yeah, I was considering splitting it out of tab, but it is sort of the same thing so wanted to get it public first.

do we have requests for these files anywhere you can link to?

No, but I can see the tooling being available in the larger ecosystem be useful. And I chatted with some people about this and they seemed interested.
@gdams for example.

@MikeMcQuaid
Copy link
Member

Yeah, I was considering splitting it out of tab, but it is sort of the same thing so wanted to get it public first.

Cool, all good, as long as done before merged 👍🏻

No, but I can see the tooling being available in the larger ecosystem be useful. And I chatted with some people about this and they seemed interested.
@gdams for example.

I think this is the sort of thing I'd like to see some more requests for before we consider integration here.

@gdams
Copy link
Contributor

gdams commented Feb 12, 2024

Yeah this is certainly something that I see to be useful in homebrew. With the significant pressure companies/projects are being put under to provide SBOMs it would be useful for projects to be able to easily determine the exact set of deps in homebrew formulas.

Copy link

github-actions bot commented Mar 5, 2024

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@github-actions github-actions bot added the stale No recent activity label Mar 5, 2024
Library/Homebrew/tab.rb Outdated Show resolved Hide resolved
Library/Homebrew/tab.rb Outdated Show resolved Hide resolved
@github-actions github-actions bot removed the stale No recent activity label Mar 6, 2024
@SMillerDev SMillerDev force-pushed the feat/sbom/install_spdx branch from 0e3db6e to 8c63729 Compare March 23, 2024 19:36
@jkowalleck
Copy link

jkowalleck commented Mar 23, 2024

Attached is an example SBOM.
spdx.sbom.json

The resulting data structure seams invalid to the SPDX 2.3 JSON schema.
Tested with https://www.jsonschemavalidator.net/ and https://www.liquid-technologies.com/online-json-schema-validator

@SMillerDev SMillerDev force-pushed the feat/sbom/install_spdx branch 2 times, most recently from 2166dde to af7e6fe Compare March 24, 2024 09:59
@SMillerDev
Copy link
Member Author

New generated SBOM:
sbom.spdx.json

That should pass validation and have some more information than the previous one.

@SMillerDev SMillerDev force-pushed the feat/sbom/install_spdx branch from af7e6fe to 8a52c56 Compare March 24, 2024 10:44
Copy link
Member

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

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

Looking better!

Blockers needed before merge:

  • tests, ideally with 100% coverage of sbom.rb
  • uses JSONSchemer.schema (like github_packages.rb) to do schema validation on write

@SMillerDev
Copy link
Member Author

uses JSONSchemer.schema (like github_packages.rb) to do schema validation on write

Didn't know we had that, it sounds awesome. Do we use that for the API already? Otherwise I'm adding that to my list.

@MikeMcQuaid
Copy link
Member

Didn't know we had that, it sounds awesome.

It is good for catching problems for sure.

Do we use that for the API already?

No(t yet). We'd need to create and publish a schema, too. Might want to sync up with @apainintheneck and save this for API v3 rather than create a v2 schema that won't be around in a year.

@apainintheneck
Copy link
Contributor

I sent @SMillerDev some info but honestly API v3 still seems a ways off since the way we handle dependencies for formulae is still undecided and cask v3 is currently blocked by potential scope creep. Either we hold off on validating API v2 for now or we add validation knowing it might get removed in a few months.

@SMillerDev SMillerDev force-pushed the feat/sbom/install_spdx branch from 8a52c56 to 18ae7dc Compare March 30, 2024 18:40
@SMillerDev
Copy link
Member Author

Does anyone know where stdlib information comes from in the install tab etc? Or is that only for Linux and that's why I can't find it?

@SMillerDev SMillerDev force-pushed the feat/sbom/install_spdx branch 3 times, most recently from 4cf1dd2 to fd8dad2 Compare March 30, 2024 19:04
@SMillerDev SMillerDev force-pushed the feat/sbom/install_spdx branch 2 times, most recently from f55bd13 to 919d77e Compare April 18, 2024 16:36
@SMillerDev
Copy link
Member Author

Any suggestions to fix this?

  Error: bottling failed
  Error: cannot load such file -- json_schemer

@SMillerDev SMillerDev marked this pull request as ready for review April 21, 2024 11:23
# typed: true
# frozen_string_literal: true

require "json_schemer"
Copy link
Member

Choose a reason for hiding this comment

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

@SMillerDev This will only be available for dev-cmd so may need to get moved into a method call instead. I suspect that's needed to fix CI failures.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure what you mean by a method call, do you mean to make a command to generate an SBOM?

Copy link
Member

Choose a reason for hiding this comment

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

Move the require inside a def something

@SMillerDev SMillerDev force-pushed the feat/sbom/install_spdx branch from 72365d6 to 2ad8e6b Compare April 25, 2024 15:13
@Bo98
Copy link
Member

Bo98 commented Apr 25, 2024

We cannot vendor it for all as it unfortunately depends on a native extension (via simpleidn -> unf -> unf_ext).

We already have rexml in multiple groups so it isn't really a problem having it in potentially many groups.

@SMillerDev SMillerDev force-pushed the feat/sbom/install_spdx branch from 2ad8e6b to 7b040c1 Compare April 25, 2024 15:26
@SMillerDev
Copy link
Member Author

I think/hope this is ready for a final review

Copy link
Member

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

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

Great work so far @SMillerDev, think this is pretty close.

Library/Homebrew/dev-cmd/bottle.rb Outdated Show resolved Hide resolved
Library/Homebrew/sbom.rb Outdated Show resolved Hide resolved
Library/Homebrew/sbom.rb Outdated Show resolved Hide resolved
Library/Homebrew/sbom.rb Outdated Show resolved Hide resolved
Library/Homebrew/sbom.rb Outdated Show resolved Hide resolved
Library/Homebrew/sbom.rb Outdated Show resolved Hide resolved
Library/Homebrew/sbom.rb Outdated Show resolved Hide resolved
Library/Homebrew/sbom.rb Outdated Show resolved Hide resolved
Library/Homebrew/test/sbom_spec.rb Outdated Show resolved Hide resolved
Library/Homebrew/test/sbom_spec.rb Show resolved Hide resolved
Library/Homebrew/sbom.rb Outdated Show resolved Hide resolved
Library/Homebrew/sbom.rb Outdated Show resolved Hide resolved
Library/Homebrew/sbom.rb Outdated Show resolved Hide resolved
Library/Homebrew/sbom.rb Outdated Show resolved Hide resolved
@SMillerDev SMillerDev force-pushed the feat/sbom/install_spdx branch from 6ad273d to 44c4ca3 Compare May 3, 2024 12:27
@SMillerDev
Copy link
Member Author

Not sure how to resolve this

Library/Homebrew/sbom.rb:79:5: C: Homebrew/InstallBundlerGems: Only use Homebrew.install_bundler_gems! in dev-cmd.
    Homebrew.install_bundler_gems!(groups: ["bottle"])
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

@MikeMcQuaid
Copy link
Member

Not sure how to resolve this

Need to move that call to the relevant dev-cmd instead e.g. bottle.

@SMillerDev SMillerDev force-pushed the feat/sbom/install_spdx branch 6 times, most recently from 8b64aeb to 5b5eda0 Compare May 6, 2024 18:09
@SMillerDev
Copy link
Member Author

Okay, after my latest changes it definitely hits those paths, but I guess codecov isn't based on the online run?

@MikeMcQuaid
Copy link
Member

Okay, after my latest changes it definitely hits those paths, but I guess codecov isn't based on the online run?

It should be: https://github.com/Homebrew/brew/actions/runs/8973658512/job/24644407661?pr=16594#step:13:52

We don't run online tests on macOS in case that's it?

CC @Bo98 for ideas.

@SMillerDev SMillerDev force-pushed the feat/sbom/install_spdx branch from 5b5eda0 to e9bb075 Compare May 7, 2024 18:28
@SMillerDev SMillerDev force-pushed the feat/sbom/install_spdx branch from e9bb075 to a43b746 Compare May 7, 2024 18:33
@SMillerDev
Copy link
Member Author

Checked the patches and simplified the test a little bit since I think it ran as generic (and would never take those patches). I think we're good to merge now.

Copy link
Member

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

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

Thanks again @SMillerDev!

@MikeMcQuaid MikeMcQuaid enabled auto-merge May 7, 2024 18:40
@MikeMcQuaid MikeMcQuaid merged commit 0d1bebc into Homebrew:master May 7, 2024
24 checks passed
@github-actions github-actions bot added the outdated PR was locked due to age label Jun 7, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated PR was locked due to age
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants