-
Notifications
You must be signed in to change notification settings - Fork 502
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
Final fixes for initial 2-SBOM release #2156
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: puerco The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just two nits, otherwise LGTM
pkg/anago/anago.go
Outdated
@@ -45,6 +45,11 @@ const ( | |||
|
|||
// releaseNotesJSONFile is the file containing the release notes in json format | |||
releaseNotesJSONFile = workspaceDir + "/src/release-notes.json" | |||
|
|||
ReleaseDownloadURL = "https://storage.googleapis.com/kubernetes-release/release/" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://storage.googleapis.com/
should be a const somewhere since it is used in various places:
> rg -tgo storage.googleapis.com
pkg/release/release.go
254: urlPrefix := fmt.Sprintf("https://storage.googleapis.com/%s", bucket)
pkg/kubepkg/kubepkg_test.go
525: expected: "https://storage.googleapis.com/k8s-artifacts-cni/release/v0.8.6/cni-plugins-linux-amd64-v0.8.6.tgz",
pkg/kubepkg/kubepkg.go
629: return fmt.Sprintf("https://storage.googleapis.com/k8s-artifacts-cni/release/v%s/cni-plugins-linux-%s-v%s.tgz", version, arch, version), nil
pkg/testgrid/testgrid.go
30:const testgridConfigURL = "https://storage.googleapis.com/k8s-testgrid/config"
cmd/krel/cmd/testgrid.go
206: testgridJobs[i].GCSLocation = fmt.Sprintf("https://storage.googleapis.com/%s", gcsPath)
And then we can construct it as another const, because "kubernetes-release" is already defined in release.ProductionBucket
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree completely, I'll refactor this value in a fallow up. But for now, I removed the ReleaseDownloadURL
constant from anago since I could not use it in the SBOM.
pkg/release/release.go
Outdated
// WorkspaceDir is the global directory where the stage and release process | ||
// happens. | ||
WorkspaceDir = "/workspace" | ||
|
||
// gitRoot is the local repository root of k/k. | ||
GitRoot = WorkspaceDir + "/src/k8s.io/kubernetes" | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not a fan of re-defining those here again, let's pass the GitRoot
used in the functions in pkg/release/workspace.go
as argument.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wanted to have the functions as easy as possible in order to avoid rethinking the paths when using them. But now that I changed them, it turns out that passing the git root is better as I can write a test.
I plan to replace them once we have the supported platforms code bit for now they will take the gitRoot as arg.
OK, done
This commit changes the stated license as Apache-2.0 in both SBOMs. Signed-off-by: Adolfo García Veytia (Puerco) <[email protected]>
This commit adds the capability to the spdx package to reference external documents. This is achieved by a new type ExternalDocumentRef which gets rendered in the document. This commit also modifies the YAML configuration support to enable the definition of external document references: ```yaml external-docs: - id: kubernetes-source uri: http://gcr.io/k8s.spdx checksums: {sha256: 5167fec9c11112ee8ea3f2b21345df18d4a4aec5a5dcbeb03d0d2c8c438eada7} ``` A test for the correct rendering of the new type is included. Signed-off-by: Adolfo García Veytia (Puerco) <[email protected]>
This commit adds three functions to workspace.go to get the artifacts produced for a given version: ListBuildImages: Returns a list if the image archives ListBuildTarballs: Returns the various tarballs before staging ListBuildBinaries: Returns a struct of binaries, organized by arch and platform Signed-off-by: Adolfo García Veytia (Puerco) <[email protected]>
This commit modifies GenerateVersionArtifactsBOM to add binaries and workspace files independently to list the bucket paths that will hold them after release. Previously, the SBOM listed binaries in in their staging paths and tarballs where missing. Signed-off-by: Adolfo García Veytia (Puerco) <[email protected]>
Signed-off-by: Adolfo García Veytia (Puerco) <[email protected]>
When generating a conatiner image PSDX package, we now generate the SPDX IDs for the layers incorporating the image ref. This avoids ID clashes when two images containt the same layer (ie the same base image). Signed-off-by: Adolfo García Veytia (Puerco) <[email protected]>
This commit links the artifacts sbom with the artifacts sbom using a SPDX ExternalDocumentRef. To do this, we now add a new function to the ExternalDocumentRef type to calculate the sha1 checksum from a local file. A test is included for the new function. Signed-off-by: Adolfo García Veytia (Puerco) <[email protected]>
This commit improves the handling of the SPDX relationships rendering by catching potential inconsistencies. A new test for the function is included. Signed-off-by: Adolfo García Veytia (Puerco) <[email protected]>
Nits are addressed, PTAL. I gave it a final test run: https://console.cloud.google.com/cloud-build/builds;region=global/b76652b1-e95f-4d83-9ba7-8b588a05e9c2?project=kubernetes-release-test |
This commit introduces a change to correctly handle relationships with remote SPDX entities. Before this change, only local objects could be referenced in the SBOM. Signed-off-by: Adolfo García Veytia (Puerco) <[email protected]>
I've pushed two extra commits to address a SPDX validation issue with remote document references in the release sbom. |
This PR corrects the external reference linking the kubernetes source sbom and the release sbom now that the SPDX package supports external references in relationships. Signed-off-by: Adolfo García Veytia (Puerco) <[email protected]>
Final stage run after last commit: https://console.cloud.google.com/cloud-build/builds;region=global/1086028c-ce0f-4776-8668-7bad65b3fae1?project=kubernetes-release-test |
/lgtm |
What type of PR is this?
/kind bug
/kind cleanup
/kind feature
What this PR does / why we need it:
In preparation for the v1.22.0-beta.1 prerelease, this PR adds the final touches for the 2 SBOM structure. The main focus of these changes is stability, adding detail to the SBOMs and linking the two produces documents together.
Which issue(s) this PR fixes:
Part of #1837
Special notes for your reviewer:
Changes are listed in the release notes, more details about each modification can be found in the commit messages.
A stage test run of these commits can be seen here: https://console.cloud.google.com/cloud-build/builds;region=global/57e940cb-c65c-4a8a-a3f2-e6e33043b512?project=kubernetes-release-test
Does this PR introduce a user-facing change?