Skip to content

Conversation

vivekpatani
Copy link
Contributor

@vivekpatani vivekpatani commented Jul 7, 2025

@k8s-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: vivekpatani
Once this PR has been reviewed and has the lgtm label, please assign spzala for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot
Copy link

Hi @vivekpatani. Thanks for your PR.

I'm waiting for a etcd-io member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@vivekpatani
Copy link
Contributor Author

vivekpatani commented Jul 7, 2025

Sample output from a local run

> PASSES="modern_sbom" ./scripts/test.sh
Running with --race
Starting at: Mon Jun 30 13:20:59 PDT 2025

'modern_sbom' started at Mon Jun 30 13:20:59 PDT 2025
validating modern SBOM generation...
syft is already installed: 1.27.1
sbomasm is already installed locally
generating modern SBOM files for version dev
generating SPDX SBOM...
 ✔ Indexed file system                                                                                                                                   . 
 ✔ Cataloged contents                                                                     19be9340167c940e8dc3cbae0b85682c19973b77c6f3a30def20c6976b478d0b 
   ├── ✔ Packages                        [912 packages]  
   ├── ✔ Executables                     [4 executables]  
   ├── ✔ File metadata                   [22 locations]  
   └── ✔ File digests                    [22 files]  
SPDX SBOM generated: /var/folders/6h/5jv5cfs54v72cvvc_pvysdqw0000gn/T/tmp.EUNXl5rXEh/sbom.spdx.json
generating CycloneDX SBOM...
 ✔ Indexed file system                                                                                                                                   . 
 ✔ Cataloged contents                                                                     19be9340167c940e8dc3cbae0b85682c19973b77c6f3a30def20c6976b478d0b 
   ├── ✔ Packages                        [912 packages]  
   ├── ✔ Executables                     [4 executables]  
   ├── ✔ File metadata                   [22 locations]  
   └── ✔ File digests                    [22 files]  
CycloneDX SBOM generated: /var/folders/6h/5jv5cfs54v72cvvc_pvysdqw0000gn/T/tmp.EUNXl5rXEh/sbom.cdx.json
augmenting SBOM files with enhanced metadata...
augmenting SPDX SBOM...
{"level":"info","ts":1751314862.2400372,"caller":"edit/spdx_edit.go:82","msg":"SPDX error updating supplier: not supported"}
{"level":"info","ts":1751314862.2400792,"caller":"edit/spdx_edit.go:82","msg":"SPDX error updating repository: not supported"}
SPDX SBOM augmented successfully
augmenting CycloneDX SBOM...
CycloneDX SBOM augmented successfully
SBOM augmentation completed successfully
modern SBOM files generated successfully
files created:
  - /var/folders/6h/5jv5cfs54v72cvvc_pvysdqw0000gn/T/tmp.EUNXl5rXEh/sbom.spdx.json
  - /var/folders/6h/5jv5cfs54v72cvvc_pvysdqw0000gn/T/tmp.EUNXl5rXEh/sbom.cdx.json
validating generated SBOM files...
Validated: /var/folders/6h/5jv5cfs54v72cvvc_pvysdqw0000gn/T/tmp.EUNXl5rXEh/sbom.spdx.json
Validated: /var/folders/6h/5jv5cfs54v72cvvc_pvysdqw0000gn/T/tmp.EUNXl5rXEh/sbom.cdx.json
all SBOM files validated successfully
modern SBOM generation validation PASSED
'modern_sbom' PASSED and completed at Mon Jun 30 13:21:02 PDT 2025
SUCCESS

scripts/test.sh Outdated
@@ -57,7 +57,7 @@ if [ -n "${OUTPUT_FILE}" ]; then
exec > >(tee -a "${OUTPUT_FILE}") 2>&1
fi

PASSES=${PASSES:-"gofmt bom dep build unit"}
PASSES=${PASSES:-"gofmt bom modern_sbom dep build unit"}
Copy link
Member

Choose a reason for hiding this comment

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

Please don't integrate here, but create a makefile target.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ack.

Updated PR. Added the following to Makefile:

# Static analysis
.PHONY: verify
verify: verify-gofmt verify-bom verify-modern-sbom verify-lint verify-dep verify-shellcheck verify-goword \
	verify-govet verify-license-header verify-mod-tidy \
	verify-shellws verify-proto-annotations verify-genproto verify-yamllint \
	verify-govet-shadow verify-markdown-marker verify-go-versions

.PHONY: verify-modern-sbom
verify-modern-sbom:
	PASSES="modern_sbom" ./scripts/test.sh

@serathius

- Syft to generate CycloneDX/SPDX SBOM format
- sbomasm to augment SBOM for missing information
- add to testing suite
- address: etcd-io#18902
- add Makefile target

Signed-off-by: vivekpatani <[email protected]>
Copy link
Member

@jmhbnz jmhbnz left a comment

Choose a reason for hiding this comment

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

Awesome progress @vivekpatani - thanks for drafting this!

Comment on lines +74 to +82
log_callout "augmenting SPDX SBOM..."
if "${sbomasm_cmd}" edit --append --subject Document \
--author 'etcd maintainers (https://github.com/etcd-io/etcd/blob/main/MAINTAINERS)' \
--supplier 'etcd project (https://etcd.io/)' \
--repository 'https://github.com/etcd-io/etcd' \
--license 'Apache-2.0 (https://raw.githubusercontent.com/etcd-io/etcd/main/LICENSE)' \
"${output_dir}/sbom.spdx.json" > "${output_dir}/sbom.spdx.json.tmp"; then
mv "${output_dir}/sbom.spdx.json.tmp" "${output_dir}/sbom.spdx.json"
log_success "SPDX SBOM augmented successfully"
Copy link
Member

Choose a reason for hiding this comment

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

When running make verify locally on this pr I see:

augmenting SBOM files with enhanced metadata...                                                                                                               
augmenting SPDX SBOM...                                    
{"level":"info","ts":1752054550.5096443,"caller":"edit/spdx_edit.go:82","msg":"SPDX error updating supplier: not supported"}
{"level":"info","ts":1752054550.5097733,"caller":"edit/spdx_edit.go:82","msg":"SPDX error updating repository: not supported"}

Are the errors on updating repository and supplier expected?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are the errors on updating repository and supplier expected?

My understanding is the sbomasm tool intentionally does not support --supplier and --repository flags for SPDX format in v1.0.4 (which is the current latest). However, the newest main has some diff in the supplier version which addresses the issue. I'd prefer sticking to a release version, rather than build main manually.

https://github.com/interlynk-io/sbomasm/blob/a0d5adcfacfb1a822642edfc143d54c3e0cc4b4a/pkg/edit/spdx_edit.go#L127C1-L150C2 vs https://github.com/interlynk-io/sbomasm/blob/a23a22f1438c75c972902f428e66c52b6667f363/pkg/edit/spdx_edit.go#L144-L186

Obviously, I'm open to feedback, and can adjust.

@jmhbnz thoughts?

Copy link
Member

Choose a reason for hiding this comment

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

In this case, I would hold the pull request until they release the new version (assuming we have verified that this addresses the issue).

Choose a reason for hiding this comment

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

Hey, basically SPDX at document level doesn't support these fields supplier and repository. Instead it has author which could be a person or organization. However these fields supplier and repository are supported at package level, which are referenced via PackageSupplier and PackageDownloadLocation.

That's why sbomam throws an error that SPDX doesn't support these fields, whereas CycloneDX do support these fields at document level. If you try the same same using CycloneDX SBOM it would work fine.

One thing we can do here, under Author as a person, add this part: 'etcd maintainers (https://github.com/etcd-io/etcd/blob/main/MAINTAINERS)', which represents list of all authors as a person, whereas this part: 'etcd project (https://etcd.io/)', which represents "organization" and it's "website" can be added under "Author" as a organization, instead of supplier.

Currently, sbomasm do support adding of author as person, but not for organization. The issue is already there for the same to add support for adding suthor as an organization, hopefully in next release will fix it.

@jmhbnz
Copy link
Member

jmhbnz commented Jul 13, 2025

/ok-to-test

Copy link

codecov bot commented Jul 13, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 69.11%. Comparing base (233021b) to head (464df34).
⚠️ Report is 282 commits behind head on main.

Additional details and impacted files

see 24 files with indirect coverage changes

@@            Coverage Diff             @@
##             main   #20303      +/-   ##
==========================================
+ Coverage   68.94%   69.11%   +0.17%     
==========================================
  Files         415      415              
  Lines       34590    34617      +27     
==========================================
+ Hits        23847    23925      +78     
+ Misses       9348     9304      -44     
+ Partials     1395     1388       -7     

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 233021b...464df34. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@vivekpatani vivekpatani marked this pull request as ready for review July 15, 2025 18:28
@vivekpatani
Copy link
Contributor Author

Hello team, gentle bump @jmhbnz @ivanvc @ahrtr @serathius

Copy link
Member

@ivanvc ivanvc 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 putting this together, @vivekpatani, great job :)

I ran scripts/generate-modern-sbom.sh v3.6.1 locally, but the output file shows UNKNOWN for our local packages both in sbom.cdx.json and sbom.spdx.json. And in sbom.spdx.json, all the dependencies show NOASSERTION for licenseDeclared and copyrightText (among other fields). I assume this is not the expected behavior, right? Unless I executed them incorrectly.

# augment SPDX SBOM with etcd-specific metadata
log_callout "augmenting SPDX SBOM..."
if "${sbomasm_cmd}" edit --append --subject Document \
--author 'etcd maintainers (https://github.com/etcd-io/etcd/blob/main/MAINTAINERS)' \
Copy link
Member

Choose a reason for hiding this comment

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

This should point to the OWNERS file:

Suggested change
--author 'etcd maintainers (https://github.com/etcd-io/etcd/blob/main/MAINTAINERS)' \
--author 'etcd maintainers (https://github.com/etcd-io/etcd/blob/main/OWNERS)' \

if ! command -v syft >/dev/null && [ ! -f "${syft_bin}" ]; then
log_callout "installing syft to local bin directory..."
mkdir -p ./bin
curl -sSfL https://raw.githubusercontent.com/anchore/syft/main/install.sh | sh -s -- -b ./bin
Copy link
Member

Choose a reason for hiding this comment

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

I think that because Syft and SBOMAS are both Go projects, we could add their dependencies to tools/mod and maintain the version through Dependabot, rather than relying on their installation scripts or pulling the latest release. For example, if there's a breaking change, we could lock to a specific version while we update the code to make it compatible with the latest.


function install_syft() {
local syft_bin="./bin/syft"
if ! command -v syft >/dev/null && [ ! -f "${syft_bin}" ]; then
Copy link
Member

Choose a reason for hiding this comment

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

I would simplify this and use only our provided binaries to avoid mismatches if the user is running a different version locally.

Copy link
Member

Choose a reason for hiding this comment

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

I'm not a specialist in SBOMs. Is this the correct way to distribute them (by placing them as released files, rather than in the repository)?

FWIW, I like the idea.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

6 participants