-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Add an implicit reference to the Generate SBOM task #43151
base: main
Are you sure you want to change the base?
Add an implicit reference to the Generate SBOM task #43151
Conversation
fix Target syntax and introduce version for package that dependabot could update
Stamp the version of the SBOM tools package at SDK construction time.
@dotnet-policy-service agree company="Microsoft" |
eng/Versions.props
Outdated
@@ -89,6 +89,7 @@ | |||
<MicrosoftWindowsCsWin32PackageVersion>0.3.49-beta</MicrosoftWindowsCsWin32PackageVersion> | |||
<!-- This is the version of the zip archive for the WiX toolset and is different from the NuGet package version format. --> | |||
<WixVersion>3.14.1.8722</WixVersion> | |||
<MicrosoftSbomTargetsPackageVersion>0.0.0-preview.0</MicrosoftSbomTargetsPackageVersion> |
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.
This will be updated once we publish an official Nuget package.
- Update this with the official version.
@baronfel is going to provide some guidance on how to make the package reference work for CPM and for non-CPM (currently this will fail if the customer has CPM enabled). |
@@ -253,6 +253,12 @@ Copyright (c) .NET Foundation. All rights reserved. | |||
Condition="'@(PackageReference->AnyHaveMetadataValue('Identity', 'Microsoft.Net.Compilers.Toolset.Framework'))' == 'true'" /> | |||
</Target> | |||
|
|||
<Target Name="_AddGenerateSbomTask" BeforeTargets="CollectPackageReferences"> | |||
<ItemGroup Condition=" '$(GenerateSbom)' == 'true' "> | |||
<PackageReference Include="Microsoft.Sbom.Targets" Version="$(MicrosoftSbomTargetsPackageVersion)" IsImplicitlyDefined="true" /> |
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.
Ok, so this will be a problem when users have opted into NuGet Central Package Version Management, because CPM requires that users separate the declaration of PackageReferences and their versions.
This is a good opportunity to develop a pattern. Perhaps something like the following:
<ItemGroup Condition=" '$(GenerateSbom)' == 'true' ">
<PackageReference Include="Microsoft.Sbom.Targets" Version="$(MicrosoftSbomTargetsPackageVersion)" IsImplicitlyDefined="true" Condition="'$(ManagePackageVersionsCentrally)' != 'true' " />
<!-- If using CPM create the PackageReference and PackageVersion -->
<PackageReference Include="Microsoft.Sbom.Targets" IsImplicitlyDefined="true" Condition="'$(ManagePackageVersionsCentrally)' == 'true' " />
<PackageVersion Include="Microsoft.Sbom.Targets" Version="$(MicrosoftSbomTargetsPackageVersion)" Condition="'$(ManagePackageVersionsCentrally)' == 'true' "/>
</ItemGroup>
cc @JonDouglas / @nkolev92 for feedback on how we should manage SDK-delivered CPM-able dependencies. This will be a Big Deal when we turn on SBOM generation by default for 10.
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.
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.
You don't need to handle CPM. IsImplicitlyDefined="true"
will tell NuGet that this package is not managed by the customer, and therefore is expected to explicitly have the version set on the PackageReference
.
It also means that in VS we won't show it available for updates, and I think that maybe we disable the install/upgrade button even if you search for the package in the browse tab. I'm not yet aware of partners engaging with us about how to allow customers to upgrade implicitly defined packages, hence there's no feature for that at this time.
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.
Ok. What about users using lock files? We currently have a really bad story there with implicit packages and the SDK, and this is one more instance of that pain.
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.
🤷
lock files basically have two purposes:
- to ensure that the same package version is restored
- the package contents (hash) are the same.
If different versions of the SDK pull in different versions of the package, yes, it'll be a bad time, but it's working precisely as intended. One way customers can mitigate this is by using a global.json to pin to a specific .NET SDK, and that generally necessitates using dotnet-install.[ps1|sh]
to install the pinned SDK, rather than using the CI agent installed.
Perhaps our teams need to get together and talk more about lock file experiences and how we can make implicit packages work better. But at the same time lock files are used by customers who want deterministic builds, and different versions of the SDK brining in different package versions conflicts with that goal.
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.
Yeah, let's set something up soon to talk about it - maybe a Monday partner sync Monday after next or something? cc @dsplaisted
eng/Versions.props
Outdated
@@ -89,6 +89,7 @@ | |||
<MicrosoftWindowsCsWin32PackageVersion>0.3.49-beta</MicrosoftWindowsCsWin32PackageVersion> | |||
<!-- This is the version of the zip archive for the WiX toolset and is different from the NuGet package version format. --> | |||
<WixVersion>3.14.1.8722</WixVersion> | |||
<MicrosoftSbomTargetsPackageVersion>2.2.8</MicrosoftSbomTargetsPackageVersion> |
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've mirrored this package over (the job is running now)
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.
PR lgtm
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.
We need
- Provide sbom-tool tests with a base set of scenarios #43582
- to triage and address which pieces of feedback from .NET integration follow-up considerations microsoft/sbom-tool#693 need to be done in V1 of the integration. Some of those seem quite impactful.
Here's a SPDX generated for autofac's repo: https://github.com/autofac/Autofac Some quick feedback I saw: License information seems missing for all components. although defined as
|
Please don't.
|
Noted. I was referring to just a simple scenario here where no License information is being generated due to properties still reflecting as |
The SBOM task also spews all over the console output - that is not good MSBuild behavior. Any output from the task needs to happen through MSBuild's Logging APIs in the Task. chusk@Chet-Desktop E:\....\sbom-tests> dotnet pack -bl
Restore succeeded with 1 warning(s) in 3.6s
E:\Code\Scratch\sbom-tests\sbom-tests.csproj : warning NU1604: Project dependency Microsoft.Sbom.Targets does not contain an inclusive lower bound. Include a lower bound in the dependency version to ensure consistent restore results.
You are using a preview version of .NET. See: https://aka.ms/dotnet-support-policy
sbom-tests net9.0 succeeded with 1 warning(s) (0.1s)
E:\Code\Scratch\sbom-tests\sbom-tests.csproj : warning NU1604: Project dependency Microsoft.Sbom.Targets does not contain an inclusive lower bound. Include a lower bound in the dependency version to ensure consistent restore results.
sbom-tests GenerateSbomTarget (9.5s)
##[information]Finding components... (9.9s)
##[information]No instructions received to scan docker images.
sbom-tests GenerateSbomTarget (10.0s)
##[information]Enumerated 39 files and 22 directories in 00:00:00.0231810 (10.2s)
##[information]PipReport: No pip found on system. Pip installation report detection will not run. (10.8s)
Detection Summary
┌────────────────────────────────┬────────────────────────────────┬────────────────────────────────┬────────────────────────────────┐
│ Component Detector Id │ Detection Time │ # Components Found │ # Explicitly Referenced │
├────────────────────────────────┼────────────────────────────────┼────────────────────────────────┼────────────────────────────────┤
│ CocoaPods │ 0.1 seconds │ 0 │ 0 │
│ ConanLock │ 0.096 seconds │ 0 │ 0 │
│ Go │ 0.098 seconds │ 0 │ 0 │
│ GoWithReplace (Beta) │ 0.098 seconds │ 0 │ 0 │
│ Gradle │ 0.096 seconds │ 0 │ 0 │
│ Ivy (Beta) │ 0.17 seconds │ 0 │ 0 │
│ Linux │ 0.0077 seconds │ 0 │ 0 │
│ MvnCli │ 0.19 seconds │ 0 │ 0 │
│ Npm │ 0.096 seconds │ 0 │ 0 │
│ NpmLockfile3 │ 0.096 seconds │ 0 │ 0 │
│ NpmWithRoots │ 0.096 seconds │ 0 │ 0 │
│ NuGet │ 0.098 seconds │ 1 │ 0 │
│ NuGetPackagesConfig │ 0.096 seconds │ 0 │ 0 │
│ NuGetProjectCentric │ 0.14 seconds │ 2 │ 2 │
│ Pip │ 0.72 seconds │ 0 │ 0 │
│ PipReport (Beta) │ 0.24 seconds │ 0 │ 0 │
│ Pnpm │ 0.096 seconds │ 0 │ 0 │
│ Poetry (Beta) │ 0.096 seconds │ 0 │ 0 │
│ Ruby │ 0.096 seconds │ 0 │ 0 │
│ RustCli │ 0.096 seconds │ 0 │ 0 │
│ RustCrateDetector │ 0.096 seconds │ 0 │ 0 │
│ SPDX22SBOM │ 0.096 seconds │ 0 │ 0 │
│ Vcpkg │ 0.096 seconds │ 0 │ 0 │
│ Yarn │ 0.096 seconds │ 0 │ 0 │
│ ────────────────────────────── │ ────────────────────────────── │ ────────────────────────────── │ ────────────────────────────── │
│ Total │ 1.1 seconds │ 3 │ 2 │
└────────────────────────────────┴────────────────────────────────┴────────────────────────────────┴────────────────────────────────┘
##[information]""
##[information]""
##[information]Detection time: 1.1199817 seconds. (10.8s)
##[information]Scan Manifest file: "C:\\Users\\chusk\\AppData\\Local\\Temp\\ScanManifest_20240918143541642.json"(11.0s)
##[information]Finished execution of the Generate workflow SBOMTelemetry {Result=Success, Errors=ErrorContainer`1 {Count=0, Errors=[]}, Parameters=Configuration {BuildDropPath=ConfigurationSetting`1 {Value="E:\\Code\\Scratch\\sbom-tests\\bin\\Release\\sbom-tests.1.0.0.1edb7ea3.temp", Source=SBOMApi, IsDefaultSource=False}, BuildComponentPath=ConfigurationSetting`1 {Value="E:\\Code\\Scratch\\sbom-tests", Source=SBOMApi, IsDefaultSource=False}, BuildListFile=null, ManifestPath=null, ManifestDirPath=ConfigurationSetting`1 {Value="E:\\Code\\Scratch\\sbom-tests\\bin\\Release\\sbom-tests.1.0.0.1edb7ea3.temp\\_manifest", Source=Default, IsDefaultSource=True}, OutputPath=null, Parallelism=ConfigurationSetting`1 {Value=8, Source=SBOMApi, IsDefaultSource=False}, Verbosity=ConfigurationSetting`1 {Value=Information, Source=SBOMApi, IsDefaultSource=False}, ConfigFilePath=null, ManifestInfo=ConfigurationSetting`1 {Value=[ManifestInfo {Name="SPDX", Version="2.2"}], Source=SBOMApi, IsDefaultSource=False}, HashAlgorithm=null, RootPathFilter=null, CatalogFilePath=null, ValidateSignature=null, IgnoreMissing=null, ManifestToolAction=Generate, PackageName=ConfigurationSetting`1 {Value="sbom-tests", Source=SBOMApi, IsDefaultSource=False}, PackageVersion=ConfigurationSetting`1 {Value="1.0.0", Source=SBOMApi, IsDefaultSource=False}, PackageSupplier=ConfigurationSetting`1 {Value="sbom-tests", Source=SBOMApi, IsDefaultSource=False}, FilesList=null, PackagesList=null, TelemetryFilePath=null, DockerImagesToScan=null, ExternalDocumentReferenceListFile=null, AdditionalComponentDetectorArgs=null, NamespaceUriUni(11.0s)=ConfigurationSetting`1 {Value=null, Source=SBOMApi, IsDefaultSource
ConfigurationSetting`1 {Value="http://spdx.org/spdxdocs/sbom-tests", Source=SBOMApi, IsDefaultSource=False}, GenerationTimestamp=ConfigurationSetting`1 {Value=null, Source=SBOMApi, IsDefaultSource=False}, FollowSymlinks=ConfigurationSetting`1 {Value=True, Source=SBOMApi, IsDefaultSource=False}, DeleteManifestDirIfPresent=ConfigurationSetting`1 {Value=True, Source=SBOMApi, IsDefaultSource=False}, FailIfNoPackages=null, FetchLicenseInformation=null, EnablePackageMetadataParsing=null, SbomPath=null, SbomDir=null}, SBOMFormatsUsed=[SBOMFile {SbomFormatName=ManifestInfo {Name="SPDX", Version="2.2"}, SbomFilePath="E:\\Code\\Scratch\\sbom-tests\\bin\\Release\\sbom-tests.1.0.0.1edb7ea3.temp\\_manifest\\spdx_2.2\\manifest.spdx.json", FileSizeInBytes=7717, TotalNumberOfPackages=4}], Timings=[Timing {EventName="Metadata build time for SPDX:2.2 format", TimeSpan="00:00:00.0096792"}, Timing {EventName="External document reference generation time", TimeSpan="00:00:00.0027449"}, Timing {EventName="Packages generation time", TimeSpan="00:00:00.0318228"}, Timing {EventName="Relationships generation time", TimeSpan="00:00:00.0043172"}, Timing {EventName="Files generation time", TimeSpan="00:00:01.4718744"}, Timing {EventName="Total generation time", TimeSpan="00:00:01.5549652"}], Switches={}, Exceptions={} sbom-tests succeeded with 1 warning(s) (11.0s) → bin\Release\net9.0\sbom-tests.dll
E:\Code\Scratch\sbom-tests\sbom-tests.csproj : warning NU1604: Project dependency Microsoft.Sbom.Targets does not contain an inclusive lower bound. Include a lower bound in the dependency version to ensure consistent restore results.
Build succeeded with 3 warning(s) in 15.1s |
@dsplaisted / @marcpopMSFT / @joeloff - since this is a change to GenerateBundledVersions.props, how do we get that change to be reflected on the redist SDK? Right now for simple usage (build.cmd, eng/dogfood.ps1, |
@baronfel in the past, we've had to merge installer first and then update stage 0. In the future, we won't have the overlay sdk and so should use the live bundled versions. At the moment, I am not sure. this may require two PRs to do it right. |
Thanks @marcpopMSFT - so for testing purposes folks using this should add a property to their |
@@ -89,6 +89,7 @@ | |||
<MicrosoftWindowsCsWin32PackageVersion>0.3.49-beta</MicrosoftWindowsCsWin32PackageVersion> | |||
<!-- This is the version of the zip archive for the WiX toolset and is different from the NuGet package version format. --> | |||
<WixVersion>3.14.1.8722</WixVersion> | |||
<MicrosoftSbomTargetsPackageVersion>2.2.9</MicrosoftSbomTargetsPackageVersion> |
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.
new version was released, so I've mirrored it and bumped this
Added #43582 for tracking some tests. Also added proper list of future items in issues here: microsoft/sbom-tool#693 Please feel free to add/remove to them as we track. |
The way it currently works is that the redist project runs an Another way would be to use the output of the |
Depends on microsoft/sbom-tool#674 to actually create the Microsoft.Sbom.Targets package.