From 1d0dc08a6414ad5293a5947623351d9a169f649e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcelo=20Lynch=20=F0=9F=A7=89?= Date: Thu, 6 Jan 2022 01:28:25 +0000 Subject: [PATCH] Merged PR 643482: Fix runtime failures in DropDaemon's GetSbomPackages - Remove the `__ENABLE_SBOM_PACKAGE_CONVERSION` env variable. This was meant to guard against using this feature while the library had a bug that is already resolved, so not necessary anymore - Microsoft.SBOM.Adapters and some other packages reference Newtonsoft.Json 13.0.1, while we were using 12.0.3 throughout. This was causing a failure when the runtime tried to load the assembly after calling the library. Version 13.0.1 is incompatible with earlier ones so a downgrading binding redirect is not possible. Instead, we deploy the drop daemon with Newtonsoft.Json version 13.0.1 and use "forward" binding redirects. - Add some extra logging With these changes, an end-to-end build where CG is run in the build runner and the packages are retrieved and added to the SPDX SBOM was successful: https://cbtest.microsoft.com/build/1d85303e-322e-4ddf-af1f-585ecce96079 --- Public/Src/Tools/DropDaemon/DropDaemon.cs | 109 ++++++++---------- .../Src/Tools/DropDaemon/Tool.DropDaemon.dsc | 10 +- .../DropDaemon/Tool.DropDaemonRunner.dsc | 1 - .../Test.Tool.DropDaemon.dll.config | 2 +- cg/nuget/cgmanifest.json | 9 ++ config.microsoftInternal.dsc | 4 +- config.nuget.dotnetcore.dsc | 1 + 7 files changed, 73 insertions(+), 63 deletions(-) diff --git a/Public/Src/Tools/DropDaemon/DropDaemon.cs b/Public/Src/Tools/DropDaemon/DropDaemon.cs index ff98a8901..09b550be0 100644 --- a/Public/Src/Tools/DropDaemon/DropDaemon.cs +++ b/Public/Src/Tools/DropDaemon/DropDaemon.cs @@ -71,10 +71,6 @@ namespace Tool.DropDaemon private readonly ISBOMGenerator m_sbomGenerator; private BsiMetadataExtractor m_bsiMetadataExtractor; private readonly string m_sbomGenerationOutputDirectory; - /// - /// If set to 1, SBOMPackages will be added from component detection data to the SBOM. - /// - private const string m_enableSBOMPackageConversion = "__ENABLE_SBOM_PACKAGE_CONVERSION"; // This field should be removed once this SBOM format is deprecated // Related work item: #1895958. @@ -884,63 +880,60 @@ namespace Tool.DropDaemon /// private IEnumerable GetSbomPackages() { - var shouldConvertPackages = Environment.GetEnvironmentVariable(m_enableSBOMPackageConversion); - if (shouldConvertPackages != null && shouldConvertPackages == "1") + // Read Path for bcde output from environment, this should already be set by Cloudbuild + var bcdeOutputJsonPath = Environment.GetEnvironmentVariable(Constants.ComponentGovernanceBCDEOutputFilePath); + + if (string.IsNullOrWhiteSpace(bcdeOutputJsonPath)) { - // Read Path for bcde output from environment, this should already be set by Cloudbuild - var bcdeOutputJsonPath = Environment.GetEnvironmentVariable(Constants.ComponentGovernanceBCDEOutputFilePath); - - if (string.IsNullOrWhiteSpace(bcdeOutputJsonPath)) - { - // This shouldn't happen, but SBOM creation can still happen without it a set of packages. So, log it and return an empty set. - // TODO [pgunasekara]: Change this to a Warning. Currently this is only Info level until CB changes are fully rolled out to avoid generating warnings unnecessarily. - Logger.Info($"The '{Constants.ComponentGovernanceBCDEOutputFilePath}' environment variable was not found. Component detection data will not be included in build manifest."); - return new List(); - } - else if (!System.IO.File.Exists(bcdeOutputJsonPath)) - { - Logger.Warning($"Component detection output file not found at path '{bcdeOutputJsonPath}'. Component detection data will not be included in build manifest."); - return new List(); - } - - - var (adapterReport, packages) = new ComponentDetectionToSBOMPackageAdapter().TryConvert(bcdeOutputJsonPath); - - foreach (var reportItem in adapterReport.Report) - { - switch (reportItem.Type) - { - case AdapterReportItemType.Success: - { - if (!string.IsNullOrEmpty(reportItem.Details)) - { - Logger.Info("[ComponentDetectionToSBOMPackageAdapter] " + reportItem.Details); - } - break; - } - case AdapterReportItemType.Warning: - { - if (!string.IsNullOrEmpty(reportItem.Details)) - { - Logger.Warning("[ComponentDetectionToSBOMPackageAdapter] " + reportItem.Details); - } - break; - } - case AdapterReportItemType.Failure: - { - if (!string.IsNullOrEmpty(reportItem.Details)) - { - Logger.Error("[ComponentDetectionToSBOMPackageAdapter] " + reportItem.Details); - } - break; - } - } - } - - return packages ?? new List(); + // This shouldn't happen, but SBOM creation can still happen without it a set of packages. So, log it and return an empty set. + // TODO [pgunasekara]: Change this to a Warning. Currently this is only Info level until CB changes are fully rolled out to avoid generating warnings unnecessarily. + Logger.Info($"[GetSbomPackages] The '{Constants.ComponentGovernanceBCDEOutputFilePath}' environment variable was not found. Component detection data will not be included in build manifest."); + return new List(); + } + else if (!System.IO.File.Exists(bcdeOutputJsonPath)) + { + Logger.Warning($"[GetSbomPackages] Component detection output file not found at path '{bcdeOutputJsonPath}'. Component detection data will not be included in build manifest."); + return new List(); } - return new List(); + Logger.Info($"[GetSbomPackages] Retrieving component detection package list from file at {bcdeOutputJsonPath}"); + + var (adapterReport, packages) = new ComponentDetectionToSBOMPackageAdapter().TryConvert(bcdeOutputJsonPath); + + foreach (var reportItem in adapterReport.Report) + { + switch (reportItem.Type) + { + case AdapterReportItemType.Success: + { + if (!string.IsNullOrEmpty(reportItem.Details)) + { + Logger.Info("[ComponentDetectionToSBOMPackageAdapter] " + reportItem.Details); + } + break; + } + case AdapterReportItemType.Warning: + { + if (!string.IsNullOrEmpty(reportItem.Details)) + { + Logger.Warning("[ComponentDetectionToSBOMPackageAdapter] " + reportItem.Details); + } + break; + } + case AdapterReportItemType.Failure: + { + if (!string.IsNullOrEmpty(reportItem.Details)) + { + Logger.Error("[ComponentDetectionToSBOMPackageAdapter] " + reportItem.Details); + } + break; + } + } + } + + var result = packages ?? new List(); + Logger.Verbose($"[GetSbomPackages] Retrieved {result.Count()} packages"); + return result; } /// diff --git a/Public/Src/Tools/DropDaemon/Tool.DropDaemon.dsc b/Public/Src/Tools/DropDaemon/Tool.DropDaemon.dsc index 2491488e9..3b2a36b03 100644 --- a/Public/Src/Tools/DropDaemon/Tool.DropDaemon.dsc +++ b/Public/Src/Tools/DropDaemon/Tool.DropDaemon.dsc @@ -45,7 +45,6 @@ export namespace DropDaemon { importFrom("Microsoft.VisualStudio.Services.BlobStore.Client").pkg, importFrom("Microsoft.VisualStudio.Services.Client").pkg, importFrom("Microsoft.VisualStudio.Services.InteractiveClient").pkg, - importFrom("Newtonsoft.Json").pkg, importFrom("WindowsAzure.Storage").pkg, importFrom("Microsoft.Azure.Storage.Common").pkg, importFrom("Microsoft.Extensions.Logging.Abstractions.v6.0.0").pkg, @@ -64,6 +63,7 @@ export namespace DropDaemon { ), importFrom("Microsoft.SBOM.Adapters").withQualifier({ targetFramework: "netstandard2.0" }).pkg, importFrom("System.Text.Json.v5.0.0").pkg, + importFrom("Newtonsoft.Json.v13.0.1").pkg, importFrom("System.Text.Encodings.Web.v5.0.1").pkg, ], internalsVisibleTo: [ @@ -120,6 +120,13 @@ export namespace DropDaemon { export function dropDaemonBindingRedirects() { return [ ...BuildXLSdk.cacheBindingRedirects(), + { + name: "Newtonsoft.Json", + publicKeyToken: "30ad4fe6b2a6aeed", + culture: "neutral", + oldVersion: "0.0.0.0-13.0.0.0", + newVersion: "13.0.0.0", // Corresponds to { id: "Newtonsoft.Json", version: "13.0.1", alias: "Newtonsoft.Json.v13.0.1" } + }, { name: "System.Text.Json", publicKeyToken: "cc7b13ffcd2ddd51", @@ -141,6 +148,7 @@ export namespace DropDaemon { export function dropDaemonRuntimeContentToSkip() { return [ importFrom("System.Text.Json").withQualifier({ targetFramework: "netstandard2.0" }).pkg, + importFrom("Newtonsoft.Json").pkg, importFrom("System.Text.Encodings.Web").withQualifier({ targetFramework: "netstandard2.0" }).pkg, importFrom("Microsoft.Extensions.Logging.Abstractions").pkg, ]; diff --git a/Public/Src/Tools/DropDaemon/Tool.DropDaemonRunner.dsc b/Public/Src/Tools/DropDaemon/Tool.DropDaemonRunner.dsc index af1ab3d9f..9b236757d 100644 --- a/Public/Src/Tools/DropDaemon/Tool.DropDaemonRunner.dsc +++ b/Public/Src/Tools/DropDaemon/Tool.DropDaemonRunner.dsc @@ -549,7 +549,6 @@ export namespace DropDaemonRunner { "__CLOUDBUILD_AUTH_HELPER_CONFIG__", "QAUTHMATERIALROOT", // Auth material for low-privilege build. "AZURE_ARTIFACTS_CREDENTIALPROVIDERS_PATH", // Cloudbuild auth helper executable path for build cache, symbol, and drop - "__ENABLE_SBOM_PACKAGE_CONVERSION", ...cloudBuildVarsPointingToDirs]; /** * Sets the values of the 'forwardEnvironmentVars' diff --git a/Public/Src/Tools/UnitTests/DropDaemon/Test.Tool.DropDaemon.dll.config b/Public/Src/Tools/UnitTests/DropDaemon/Test.Tool.DropDaemon.dll.config index ebee1d5d0..6ec31e8a6 100644 --- a/Public/Src/Tools/UnitTests/DropDaemon/Test.Tool.DropDaemon.dll.config +++ b/Public/Src/Tools/UnitTests/DropDaemon/Test.Tool.DropDaemon.dll.config @@ -47,7 +47,7 @@ - + diff --git a/cg/nuget/cgmanifest.json b/cg/nuget/cgmanifest.json index f2e46bb4a..a83a53fb8 100644 --- a/cg/nuget/cgmanifest.json +++ b/cg/nuget/cgmanifest.json @@ -2755,6 +2755,15 @@ } } }, + { + "Component": { + "Type": "NuGet", + "NuGet": { + "Name": "Newtonsoft.Json", + "Version": "13.0.1" + } + } + }, { "Component": { "Type": "NuGet", diff --git a/config.microsoftInternal.dsc b/config.microsoftInternal.dsc index 0b1df7910..8d99f1104 100644 --- a/config.microsoftInternal.dsc +++ b/config.microsoftInternal.dsc @@ -76,8 +76,8 @@ export const pkgs = isMicrosoftInternal ? [ { id: "Microsoft.SBOMCore", version: sbomApiVersion, dependentPackageIdsToSkip: ["Microsoft.Extensions.Logging.Abstractions"] }, { id: "Microsoft.Parsers.ManifestGenerator", version: sbomApiVersion, dependentPackageIdsToSkip: ["Newtonsoft.Json"]}, { id: "Microsoft.Parsers.SPDX22SBOMParser", version: sbomApiVersion }, - { id: "Microsoft.SBOM.Adapters", version: sbomApiVersion }, - { id: "Microsoft.ComponentDetection.Contracts", version: "1.0.8" }, + { id: "Microsoft.SBOM.Adapters", version: sbomApiVersion, dependentPackageIdsToSkip : ["Newtonsoft.Json"] }, + { id: "Microsoft.ComponentDetection.Contracts", version: "1.0.8", dependentPackageIdsToSkip: ["Newtonsoft.Json"] }, { id: "Microsoft.ManifestInterface", version: sbomApiVersion, dependentPackageIdsToSkip: ["System.Text.Json"] }, { id: "Microsoft.Sbom.Contracts", version: sbomApiVersion }, { id: "Microsoft.Bcl.HashCode", version: "1.1.1" }, diff --git a/config.nuget.dotnetcore.dsc b/config.nuget.dotnetcore.dsc index 499c0056d..be89d6f7d 100644 --- a/config.nuget.dotnetcore.dsc +++ b/config.nuget.dotnetcore.dsc @@ -259,6 +259,7 @@ export const pkgs = [ dependentPackageIdsToSkip: ["System.Memory", "System.Buffers", "System.ValueTuple", "System.Runtime.CompilerServices.Unsafe", "System.Numerics.Vectors", "System.Threading.Tasks.Extensions", "Microsoft.Bcl.AsyncInterfaces", "System.Text.Encodings.Web"], alias: "System.Text.Json.v5.0.0" }, + { id: "Newtonsoft.Json", version: "13.0.1", alias: "Newtonsoft.Json.v13.0.1" }, { id: "System.Threading.AccessControl", version: pkgVersionNext }, { id: "System.IO.FileSystem.AccessControl", version: pkgVersion6Preview, alias: "System.IO.FileSystem.AccessControl.v6.0.0" },