Feature/idp 1028 treat warning as error flag (#40)

* Add option to Treat Warning As Error (Breaking Changes)
* Add option to opt out

---------

Co-authored-by: Mathieu Gamache <mathieu.gamache@workleap.com>
Co-authored-by: Anthony Simmon <anthony.simmon@gsoft.com>
Co-authored-by: heqianwang <158102624+heqianwang@users.noreply.github.com>

Run-SystemTest.ps1

@@ -5,6 +5,26 @@ Begin {
 }
 
 Process {
+    function BuildProject {
+        param (
+            [string]$openApiMsBuildSource,
+            [string]$projectPath,
+            [bool]$isFailureExpected,
+            [string]$extraArgs
+        )
+        Push-Location $projectPath
+
+        Exec { & dotnet add package Workleap.OpenApi.MSBuild --prerelease --source $openApiMsBuildSource }
+
+        $buildProcess = Start-Process -FilePath "dotnet" -ArgumentList "build -c Release $extraArgs" -NoNewWindow -PassThru -Wait
+
+        if ($isFailureExpected -and $buildProcess.ExitCode -eq 0 ) {
+            Write-Error "The build for project $projectPath was expected to fail, but it succeeded."
+        } elseif (!$isFailureExpected -and $buildProcess.ExitCode -ne 0) {
+            Write-Error "The build for project $projectPath was expected to succeed, but it failed."
+        }
+    }
+
     function Exec([scriptblock]$Command) {
         & $Command
         if ($LASTEXITCODE -ne 0) {
@@ -14,24 +34,24 @@ Process {
 
     $workingDir = Join-Path $PSScriptRoot "src"
     $outputDir = Join-Path $PSScriptRoot ".output"
+
     $contractFirstSysTestDir = Join-Path $PSScriptRoot "src/tests/WebApi.MsBuild.SystemTest.ContractFirst"
     $codeFirstSysTestDir = Join-Path $PSScriptRoot "src/tests/WebApi.MsBuild.SystemTest.CodeFirst"
+    $oasDiffErrorSysTestDir = Join-Path $PSScriptRoot "src/tests/WebApi.MsBuild.SystemTest.OasDiffError"
+    $spectralErrorSysTestDir = Join-Path $PSScriptRoot "src/tests/WebApi.MsBuild.SystemTest.SpectralError"
 
     try {
         Push-Location $workingDir
 
+        # Build the OpenApi.MSBuild package to be used in the system tests
         Exec { & dotnet pack -c Release -o "$outputDir" }
 
-        Push-Location $contractFirstSysTestDir
-
-        Exec { & dotnet add package Workleap.OpenApi.MSBuild --prerelease --source $outputDir }
-        Exec { & dotnet build -c Release -warnaserror }
-
-        Push-Location $codeFirstSysTestDir
-
-        Exec { & dotnet add package Workleap.OpenApi.MSBuild --prerelease --source $outputDir }
-        Exec { & dotnet build -c Release -warnaserror }
-
+        BuildProject -openApiMsBuildSource $outputDir -projectPath $contractFirstSysTestDir -isFailureExpected $false
+        BuildProject -openApiMsBuildSource $outputDir -projectPath $codeFirstSysTestDir -isFailureExpected $false
+        BuildProject -openApiMsBuildSource $outputDir -projectPath $oasDiffErrorSysTestDir -isFailureExpected $true
+        BuildProject -openApiMsBuildSource $outputDir -projectPath $spectralErrorSysTestDir -isFailureExpected $true
+        BuildProject -openApiMsBuildSource $outputDir -projectPath $oasDiffErrorSysTestDir -isFailureExpected $false -extraArgs "/p:OpenApiTreatWarningsAsErrors=false"
+        BuildProject -openApiMsBuildSource $outputDir -projectPath $spectralErrorSysTestDir -isFailureExpected $false -extraArgs "/p:OpenApiTreatWarningsAsErrors=false"
     }
     finally {
         Pop-Location

src/Workleap.OpenApi.MSBuild.sln

@@ -23,6 +23,10 @@ Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "WebApi.MsBuild.SystemTest.C
 EndProject
 Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "WebApi.MsBuild.SystemTest.CodeFirst", "tests\WebApi.MsBuild.SystemTest.CodeFirst\WebApi.MsBuild.SystemTest.CodeFirst.csproj", "{575E14D8-52E8-4B5B-A93C-D3E86E30BD3C}"
 EndProject
+Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "WebApi.MsBuild.SystemTest.OasDiffError", "tests\WebApi.MsBuild.SystemTest.OasDiffError\WebApi.MsBuild.SystemTest.OasDiffError.csproj", "{3909D38E-7584-4206-9CDC-3E56203BE6DB}"
+EndProject
+Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "WebApi.MsBuild.SystemTest.SpectralError", "tests\WebApi.MsBuild.SystemTest.SpectralError\WebApi.MsBuild.SystemTest.SpectralError.csproj", "{C42C2836-4997-49D3-9BC6-E6A0E1B8C472}"
+EndProject
 Global
 	GlobalSection(SolutionConfigurationPlatforms) = preSolution
 		Debug|Any CPU = Debug|Any CPU
@@ -52,10 +56,20 @@ Global
 		{575E14D8-52E8-4B5B-A93C-D3E86E30BD3C}.Debug|Any CPU.Build.0 = Debug|Any CPU
 		{575E14D8-52E8-4B5B-A93C-D3E86E30BD3C}.Release|Any CPU.ActiveCfg = Release|Any CPU
 		{575E14D8-52E8-4B5B-A93C-D3E86E30BD3C}.Release|Any CPU.Build.0 = Release|Any CPU
+		{3909D38E-7584-4206-9CDC-3E56203BE6DB}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
+		{3909D38E-7584-4206-9CDC-3E56203BE6DB}.Debug|Any CPU.Build.0 = Debug|Any CPU
+		{3909D38E-7584-4206-9CDC-3E56203BE6DB}.Release|Any CPU.ActiveCfg = Release|Any CPU
+		{3909D38E-7584-4206-9CDC-3E56203BE6DB}.Release|Any CPU.Build.0 = Release|Any CPU
+		{C42C2836-4997-49D3-9BC6-E6A0E1B8C472}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
+		{C42C2836-4997-49D3-9BC6-E6A0E1B8C472}.Debug|Any CPU.Build.0 = Debug|Any CPU
+		{C42C2836-4997-49D3-9BC6-E6A0E1B8C472}.Release|Any CPU.ActiveCfg = Release|Any CPU
+		{C42C2836-4997-49D3-9BC6-E6A0E1B8C472}.Release|Any CPU.Build.0 = Release|Any CPU
 	EndGlobalSection
 	GlobalSection(NestedProjects) = preSolution
 		{A4C90BE2-889F-46BB-8B4D-2219B1084695} = {07F66FC2-73B9-44C7-843C-36C13905AE9B}
 		{B8A81C76-574B-40FD-B34B-FA893D6C1423} = {4DDE83BF-D190-4CC9-AD36-E9250DABB27D}
 		{575E14D8-52E8-4B5B-A93C-D3E86E30BD3C} = {4DDE83BF-D190-4CC9-AD36-E9250DABB27D}
+		{3909D38E-7584-4206-9CDC-3E56203BE6DB} = {4DDE83BF-D190-4CC9-AD36-E9250DABB27D}
+		{C42C2836-4997-49D3-9BC6-E6A0E1B8C472} = {4DDE83BF-D190-4CC9-AD36-E9250DABB27D}
 	EndGlobalSection
 EndGlobal

src/Workleap.OpenApi.MSBuild/LoggerWrapper.cs

@@ -5,13 +5,30 @@ namespace Workleap.OpenApi.MSBuild;
 
 internal sealed class LoggerWrapper : ILoggerWrapper
 {
-    private readonly TaskLoggingHelper _taskLoggingHelper;
+    private readonly TaskLoggingHelper _helper;
+    private readonly bool _treatWarningAsError;
 
-    public LoggerWrapper(TaskLoggingHelper helper) => this._taskLoggingHelper = helper;
+    public LoggerWrapper(TaskLoggingHelper helper, bool treatWarningAsError)
+    {
+        this._helper = helper;
+        this._treatWarningAsError = treatWarningAsError;
+    }
+
+    public void LogMessage(string message, MessageImportance importance = MessageImportance.Low, params object[] messageArgs)
+        => this._helper.LogMessage(importance, message, messageArgs);
 
     public void LogWarning(string message, params object[] messageArgs)
-        => this._taskLoggingHelper.LogWarning(message, messageArgs);
+    {
+        if (this._treatWarningAsError)
+        {
+            this._helper.LogError(message, messageArgs);
+        }
+        else
+        {
+            this._helper.LogWarning(message, messageArgs);
+        }
+    }
 
-    public void LogMessage(string message, MessageImportance importance = MessageImportance.Low, params object[] messageArgs)
-        => this._taskLoggingHelper.LogMessage(importance, message, messageArgs);
+    public void LogError(string message, params object[] messageArgs)
+        => this._helper.LogError(message, messageArgs);
 }

src/Workleap.OpenApi.MSBuild/OasdiffManager.cs

@@ -5,7 +5,7 @@ namespace Workleap.OpenApi.MSBuild;
 
 internal sealed class OasdiffManager : IOasdiffManager
 {
-    private const string OasdiffVersion = "1.9.6";
+    private const string OasdiffVersion = "1.10.11";
     private const string OasdiffDownloadUrlFormat = "https://github.com/Tufin/oasdiff/releases/download/v{0}/{1}";
 
     private readonly ILoggerWrapper _loggerWrapper;
@@ -59,9 +59,19 @@ public async Task RunOasdiffAsync(IReadOnlyCollection<string> openApiSpecFiles,
             this._loggerWrapper.LogMessage("- Specification file path: {0}", MessageImportance.High, baseSpecFile);
             this._loggerWrapper.LogMessage("- Specification generated from code path: {0} \n", MessageImportance.High, generatedSpecFilePath);
 
-            var result = await this._processWrapper.RunProcessAsync(oasdiffExecutePath, new[] { "diff", baseSpecFile, generatedSpecFilePath, "--exclude-elements", "description,examples,title,summary", "-f", "text", "-o" }, cancellationToken);
-            this._loggerWrapper.LogMessage(result.StandardOutput, MessageImportance.High);
-            if (!string.IsNullOrEmpty(result.StandardError))
+            var result = await this._processWrapper.RunProcessAsync(oasdiffExecutePath, new[] { "diff", baseSpecFile, generatedSpecFilePath, "--exclude-elements", "description,examples,title,summary", "-o" }, cancellationToken);
+            if (string.IsNullOrEmpty(result.StandardError))
+            {
+                var isChangesDetected = result.ExitCode != 0;
+                this._loggerWrapper.LogMessage("Oasdiff returned: {0}", MessageImportance.Normal, result.ExitCode);
+                if (isChangesDetected)
+                {
+                    this._loggerWrapper.LogWarning($"Your web API does not respect the following OpenAPI specification: {fileName}. Please review the logs below for details.");
+                }
+
+                this._loggerWrapper.LogMessage(result.StandardOutput, MessageImportance.High);
+            }
+            else
             {
                 this._loggerWrapper.LogWarning(result.StandardError);
             }

src/Workleap.OpenApi.MSBuild/ValidateOpenApiTask.cs

@@ -1,4 +1,3 @@
-using System.Diagnostics;
 using Microsoft.Build.Framework;
 
 namespace Workleap.OpenApi.MSBuild;
@@ -7,23 +6,23 @@ public sealed class ValidateOpenApiTask : CancelableAsyncTask
 {
     private const string CodeFirst = "CodeFirst";
     private const string ContractFirst = "ContractFirst";
-    
+
     /// <summary>
-    /// 2 supported modes:
+    ///     2 supported modes:
     ///     - CodeFirst: Generate the OpenAPI specification files from the code
     ///     - ContractFirst: Will use the OpenAPI specification files provided
     /// </summary>
     [Required]
     public string OpenApiDevelopmentMode { get; set; } = string.Empty;
 
-    /// <summary>When Development mode is Contract first, will validate if the specification match the code.</summary>  
+    /// <summary>When Development mode is Contract first, will validate if the specification match the code.</summary>
     [Required]
     public bool OpenApiCompareCodeAgainstSpecFile { get; set; } = false;
-    
+
     /// <summary>The path of the ASP.NET Core project startup assembly directory.</summary>
     [Required]
     public string StartupAssemblyPath { get; set; } = string.Empty;
-    
+
     /// <summary>The path of the ASP.NET Core project being built.</summary>
     [Required]
     public string OpenApiWebApiAssemblyPath { get; set; } = string.Empty;
@@ -44,35 +43,40 @@ public sealed class ValidateOpenApiTask : CancelableAsyncTask
     [Required]
     public string[] OpenApiSpecificationFiles { get; set; } = Array.Empty<string>();
 
+    /// <summary>If warnings should be logged as errors instead.</summary>
+    public bool OpenApiTreatWarningsAsErrors { get; set; }
+
     protected override async Task<bool> ExecuteAsync(CancellationToken cancellationToken)
     {
-        this.Log.LogMessage(MessageImportance.Normal, "\n******** Starting {0} ********\n", nameof(ValidateOpenApiTask));
-
+        var loggerWrapper = new LoggerWrapper(this.Log, this.OpenApiTreatWarningsAsErrors);
+
+        loggerWrapper.LogMessage("\n******** Starting {0} ********\n", MessageImportance.Normal, nameof(ValidateOpenApiTask));
+
         var reportsPath = Path.Combine(this.OpenApiToolsDirectoryPath, "reports");
-        var loggerWrapper = new LoggerWrapper(this.Log);
         var processWrapper = new ProcessWrapper(this.StartupAssemblyPath);
         var swaggerManager = new SwaggerManager(loggerWrapper, processWrapper, this.OpenApiToolsDirectoryPath, this.OpenApiWebApiAssemblyPath);
 
         var httpClientWrapper = new HttpClientWrapper();
 
         var spectralManager = new SpectralManager(loggerWrapper, processWrapper, this.OpenApiToolsDirectoryPath, reportsPath, httpClientWrapper);
         var oasdiffManager = new OasdiffManager(loggerWrapper, processWrapper, this.OpenApiToolsDirectoryPath, httpClientWrapper);
-        var specGeneratorManager = new SpecGeneratorManager(loggerWrapper); 
+        var specGeneratorManager = new SpecGeneratorManager(loggerWrapper);
 
         var codeFirstProcess = new CodeFirstProcess(loggerWrapper, spectralManager, swaggerManager, specGeneratorManager, oasdiffManager);
         var contractFirstProcess = new ContractFirstProcess(loggerWrapper, spectralManager, swaggerManager, oasdiffManager);
 
-        this.Log.LogMessage(MessageImportance.Normal, "{0} = '{1}'", nameof(this.OpenApiDevelopmentMode), this.OpenApiDevelopmentMode);
-        this.Log.LogMessage(MessageImportance.Normal, "{0} = '{1}'", nameof(this.OpenApiCompareCodeAgainstSpecFile), this.OpenApiCompareCodeAgainstSpecFile);
-        this.Log.LogMessage(MessageImportance.Low, "{0} = '{1}'", nameof(this.OpenApiWebApiAssemblyPath), this.OpenApiWebApiAssemblyPath);
-        this.Log.LogMessage(MessageImportance.Low, "{0} = '{1}'", nameof(this.OpenApiToolsDirectoryPath), this.OpenApiToolsDirectoryPath);
-        this.Log.LogMessage(MessageImportance.Low, "{0} = '{1}'", nameof(this.OpenApiSpectralRulesetUrl), this.OpenApiSpectralRulesetUrl);
-        this.Log.LogMessage(MessageImportance.Low, "{0} = '{1}'", nameof(this.OpenApiSwaggerDocumentNames), string.Join(", ", this.OpenApiSwaggerDocumentNames));
-        this.Log.LogMessage(MessageImportance.Low, "{0} = '{1}'", nameof(this.OpenApiSpecificationFiles), string.Join(", ", this.OpenApiSpecificationFiles));
+        loggerWrapper.LogMessage("{0} = '{1}'", MessageImportance.Normal, nameof(this.OpenApiDevelopmentMode), this.OpenApiDevelopmentMode);
+        loggerWrapper.LogMessage("{0} = '{1}'", MessageImportance.Normal, nameof(this.OpenApiCompareCodeAgainstSpecFile), this.OpenApiCompareCodeAgainstSpecFile);
+        loggerWrapper.LogMessage("{0} = '{1}'", MessageImportance.Low, nameof(this.OpenApiTreatWarningsAsErrors), this.OpenApiTreatWarningsAsErrors);
+        loggerWrapper.LogMessage("{0} = '{1}'", MessageImportance.Low, nameof(this.OpenApiWebApiAssemblyPath), this.OpenApiWebApiAssemblyPath);
+        loggerWrapper.LogMessage("{0} = '{1}'", MessageImportance.Low, nameof(this.OpenApiToolsDirectoryPath), this.OpenApiToolsDirectoryPath);
+        loggerWrapper.LogMessage("{0} = '{1}'", MessageImportance.Low, nameof(this.OpenApiSpectralRulesetUrl), this.OpenApiSpectralRulesetUrl);
+        loggerWrapper.LogMessage("{0} = '{1}'", MessageImportance.Low, nameof(this.OpenApiSwaggerDocumentNames), string.Join(", ", this.OpenApiSwaggerDocumentNames));
+        loggerWrapper.LogMessage("{0} = '{1}'", MessageImportance.Low, nameof(this.OpenApiSpecificationFiles), string.Join(", ", this.OpenApiSpecificationFiles));
 
         if (this.OpenApiSpecificationFiles.Length != this.OpenApiSwaggerDocumentNames.Length)
         {
-            this.Log.LogWarning("You must provide the same amount of open api specification file names and swagger document file names.");
+            loggerWrapper.LogWarning("You must provide the same amount of open api specification file names and swagger document file names.");
 
             return false;
         }
@@ -85,17 +89,17 @@ protected override async Task<bool> ExecuteAsync(CancellationToken cancellationT
             switch (this.OpenApiDevelopmentMode)
             {
                 case CodeFirst:
-                    this.Log.LogMessage(MessageImportance.Normal, "\nStarting code first...");
+                    loggerWrapper.LogMessage("\nStarting code first process...", MessageImportance.Normal);
                     await codeFirstProcess.Execute(
                         this.OpenApiSpecificationFiles,
                         this.OpenApiSwaggerDocumentNames,
                         this.OpenApiSpectralRulesetUrl,
                         this.OpenApiCompareCodeAgainstSpecFile ? CodeFirstProcess.CodeFirstMode.SpecComparison : CodeFirstProcess.CodeFirstMode.SpecGeneration,
                         cancellationToken);
                     break;
-                
+
                 case ContractFirst:
-                    this.Log.LogMessage(MessageImportance.Normal, "\nStarting contract first...");
+                    loggerWrapper.LogMessage("\nStarting contract first process...", MessageImportance.Normal);
                     var isSuccess = await contractFirstProcess.Execute(
                         this.OpenApiSpecificationFiles,
                         this.OpenApiToolsDirectoryPath,
@@ -110,15 +114,15 @@ await codeFirstProcess.Execute(
                     }
 
                     break;
-                
+
                 default:
-                    this.Log.LogError("Invalid value of '{0}' for {1}. Allowed values are '{2}' or '{3}'", this.OpenApiDevelopmentMode, nameof(ValidateOpenApiTask.OpenApiDevelopmentMode), ContractFirst, CodeFirst);
+                    loggerWrapper.LogError("Invalid value of '{0}' for {1}. Allowed values are '{2}' or '{3}'", this.OpenApiDevelopmentMode, nameof(this.OpenApiDevelopmentMode), ContractFirst, CodeFirst);
                     return false;
             }
         }
         catch (OpenApiTaskFailedException e)
         {
-            this.Log.LogWarning("An error occurred while validating the OpenAPI specification: {0}", e.Message);
+            loggerWrapper.LogWarning("An error occurred while validating the OpenAPI specification: {0}", e.Message);
         }
 
         return true;

src/Workleap.OpenApi.MSBuild/msbuild/tools/Workleap.OpenApi.MSBuild.targets

@@ -38,6 +38,10 @@
       <!-- The names of the Swagger documents to generate OpenAPI specifications for -->
       <!-- "v1" is the default Swagger document name. Users can specify multiple values separated by semicolons -->
       <OpenApiSwaggerDocumentNames Condition="'$(OpenApiSwaggerDocumentNames)' == ''">v1</OpenApiSwaggerDocumentNames>
+
+      <!-- Use TreatWarningsAsErrors property unless it is opt-out with OpenApiTreatWarningsAsErrors, fallback to false if both are unset -->
+      <OpenApiTreatWarningsAsErrors Condition="'$(OpenApiTreatWarningsAsErrors)' == ''">$(TreatWarningsAsErrors)</OpenApiTreatWarningsAsErrors>
+      <OpenApiTreatWarningsAsErrors Condition="'$(OpenApiTreatWarningsAsErrors)' == ''">false</OpenApiTreatWarningsAsErrors>
     </PropertyGroup>
 
     <ItemGroup>
@@ -64,6 +68,7 @@
       OpenApiSpectralRulesetUrl="$(OpenApiSpectralRulesetUrl)"
       OpenApiSwaggerDocumentNames="$(OpenApiSwaggerDocumentNames)"
       OpenApiSpecificationFiles="$(OpenApiSpecificationFiles)"
+      OpenApiTreatWarningsAsErrors="$(OpenApiTreatWarningsAsErrors)"
     />
   </Target>
 </Project>

src/tests/WebApi.MsBuild.SystemTest.CodeFirst/WebApi.MsBuild.SystemTest.CodeFirst.csproj

@@ -11,6 +11,7 @@
     <PropertyGroup>
       <OpenApiDevelopmentMode>CodeFirst</OpenApiDevelopmentMode>
       <OpenApiSwaggerDocumentNames>v1;v1-management</OpenApiSwaggerDocumentNames>
+      <TreatWarningsAsErrors>true</TreatWarningsAsErrors>
     </PropertyGroup>
 
     <ItemGroup>

src/tests/WebApi.MsBuild.SystemTest.ContractFirst/WebApi.MsBuild.SystemTest.ContractFirst.csproj

@@ -14,6 +14,7 @@
       <OpenApiCompareCodeAgainstSpecFile>true</OpenApiCompareCodeAgainstSpecFile>
       <OpenApiSwaggerDocumentNames>v1;v1-management</OpenApiSwaggerDocumentNames>
       <OpenApiSpectralRulesetUrl>./custom.spectral.yaml</OpenApiSpectralRulesetUrl>
+      <TreatWarningsAsErrors>true</TreatWarningsAsErrors>
     </PropertyGroup>
 
     <ItemGroup>

src/tests/WebApi.MsBuild.SystemTest.OasDiffError/Controllers/WeatherForecastController.cs

@@ -0,0 +1,36 @@
+using Microsoft.AspNetCore.Mvc;
+
+namespace WebApi.MsBuild.SystemTest.Controllers;
+
+[ApiController]
+[Route("[controller]")]
+[Produces("application/json")]
+[ApiExplorerSettings(GroupName = "v1")]
+public class WeatherForecastController : ControllerBase
+{
+    private static readonly string[] Summaries = new[]
+    {
+        "Freezing", "Bracing", "Chilly", "Cool", "Mild", "Warm", "Balmy", "Hot", "Sweltering", "Scorching",
+    };
+
+    private readonly ILogger<WeatherForecastController> _logger;
+
+    public WeatherForecastController(ILogger<WeatherForecastController> logger)
+    {
+        this._logger = logger;
+    }
+
+    [HttpGet(Name = "GetWeatherForecast")]
+    [ProducesResponseType(StatusCodes.Status200OK)]
+    [ProducesResponseType(StatusCodes.Status403Forbidden)]
+    public IEnumerable<WeatherForecast> Get()
+    {
+        return Enumerable.Range(1, 5).Select(index => new WeatherForecast
+            {
+                Date = DateTime.Now.AddDays(index),
+                TemperatureC = Random.Shared.Next(-20, 55),
+                Summary = Summaries[Random.Shared.Next(Summaries.Length)],
+            })
+            .ToArray();
+    }
+}