[MSBUILD SDK] Add basic function app validation #3214
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds basic function app validation to the MSBuild SDK by implementing validation targets, error logging infrastructure, and associated tests. The validation ensures proper Azure Functions version usage, target framework compatibility, and prevents legacy SDK conflicts.
Key changes:
- Introduces a structured error/warning logging system with standardized AZFW error codes (0100-0199 range)
- Implements validation targets for Functions version, target framework, and legacy SDK detection
- Adds comprehensive test coverage for the logging infrastructure
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Azure.Functions.Sdk/LogMessage.cs | Defines LogMessage struct with error code parsing and static message definitions |
| src/Azure.Functions.Sdk/Tasks/MSBuildLogExtensions.cs | Provides MSBuild logging extensions for structured error/warning output |
| src/Azure.Functions.Sdk/Tasks/FuncSdkLog.cs | Implements MSBuild task for logging messages from resource strings |
| src/Azure.Functions.Sdk/Targets/Azure.Functions.Sdk.targets | Adds validation targets for Functions version, target framework, and legacy SDK detection |
| src/Azure.Functions.Sdk/Strings.resx | Contains localized error/warning messages with AZFW error codes |
| src/Azure.Functions.Sdk/Throw.cs | Utility class for standardized exception throwing (shared code) |
| test/Azure.Functions.Sdk.Tests/LogMessageTests.cs | Comprehensive test suite for LogMessage parsing and validation |
| test/Azure.Functions.Sdk.Tests/Azure.Functions.Sdk.Tests.csproj | Test project configuration |
| src/Azure.Functions.Sdk/Azure.Functions.Sdk.csproj | Updated with dependencies and build configuration |
| src/Azure.Functions.Sdk/Sdk/Sdk.targets | Adds task namespace property |
| src/Azure.Functions.Sdk/Properties/AssemblyInfo.cs | Exposes internals to test assembly |
| Directory.Build.props | Adds SrcRoot property for test project references |
| DotNetWorker.sln | Adds test project to solution |
Comments suppressed due to low confidence (1)
src/Azure.Functions.Sdk/Targets/Azure.Functions.Sdk.targets:1
- The error message has swapped parameter order. The message format shows '{0}' as unsupported framework and {1} as minimum, but the Arguments attribute passes them as '$(_MinimumSupportedTfmInFunctions);$(TargetFramework)', which is reversed.
<!--
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| private static void LogNoCode(this TaskLoggingHelper log, LogMessage logCode, params string[] messageArgs) | ||
| { | ||
| log.TaskResources = Strings.ResourceManager;switch (logCode.Level) |
There was a problem hiding this comment.
Missing space after semicolon. Should be: log.TaskResources = Strings.ResourceManager; switch (logCode.Level)
| <FuncSdkLog Condition="'%(_SelectedFunctionVersion.InSupport)' == 'false'" Resource="Warning_EndOfLifeFunctionsVersion" Arguments="$(_AzureFunctionsVersionStandardized)" /> | ||
| <FuncSdkLog Condition="'@(_SelectedFunctionVersion)' == ''" Resource="Error_UnknownFunctionsVersion" Arguments="$(_AzureFunctionsVersionStandardized)" /> | ||
| <FuncSdkLog Condition="'$(TargetFrameworkIdentifier)' == '.NETCoreApp' AND !$([MSBuild]::IsTargetFrameworkCompatible('$(TargetFramework)', '$(_MinimumSupportedTfmInFunctions)'))" | ||
| Resource="Error_UnsupportedTargetFramework" Arguments="$(_MinimumSupportedTfmInFunctions);$(TargetFramework)" /> |
There was a problem hiding this comment.
The Arguments order should be '$(TargetFramework);$(_MinimumSupportedTfmInFunctions)' to match the error message format in Strings.resx line 142, which expects the unsupported framework first ('{0}') and minimum framework second ('{1}').
| Resource="Error_UnsupportedTargetFramework" Arguments="$(_MinimumSupportedTfmInFunctions);$(TargetFramework)" /> | |
| Resource="Error_UnsupportedTargetFramework" Arguments="$(TargetFramework);$(_MinimumSupportedTfmInFunctions)" /> |
| <PropertyGroup> | ||
| <_AzureFunctionsVersionStandardized>$(AzureFunctionsVersion.ToLowerInvariant().Split('-')[0])</_AzureFunctionsVersionStandardized> | ||
| <_FunctionsRuntimeMajorVersion>$(_AzureFunctionsVersionStandardized.TrimStart('vV'))</_FunctionsRuntimeMajorVersion> | ||
| <_MinimumSupportedTfmInFunctions>net8.0</_MinimumSupportedTfmInFunctions> |
There was a problem hiding this comment.
Why is this the minumum? What happens if someone is running on .net 6?
| <value>Failed to poll '{0}'. HTTP Status: {1}. Retry count {2}/{3}</value> | ||
| </data> | ||
| <data name="Deploy_Status" xml:space="preserve"> | ||
| <value>Deployment status is {0}.</value> |
There was a problem hiding this comment.
Do we have any deploy actions? How do these errors work? Is this supposed to work the with az cli/ core tools?
There was a problem hiding this comment.
We have a publish zipdeploy action. Targets are not yet ported over to this SDK
| <value>Successfully published {0} to {1}. Status: {2}</value> | ||
| </data> | ||
| <data name="Deploy_Failed" xml:space="preserve"> | ||
| <value>Zip deployment to {0} has failed.\n {1}\n HTTP Status: {2}\n Deployment Status: {3}</value> |
There was a problem hiding this comment.
| <value>Zip deployment to {0} has failed.\n {1}\n HTTP Status: {2}\n Deployment Status: {3}</value> | |
| <value>Zip deployment to '{0}' has failed.\n {1}\n HTTP Status: {2}\n Deployment Status: {3}</value> |
| <value>The AzureFunctionsVersion '{0}' is unknown or not supported.</value> | ||
| </data> | ||
| <data name="AZFW0107_Error_UnsupportedTargetFramework" xml:space="preserve"> | ||
| <value>Target framework '{0}' is not supported by Azure Functions. Minimum target framework is {1}</value> |
There was a problem hiding this comment.
This is just being pedantic tbh so feel free to ignore. But feels weird to put one tfm in ' ' and not the other
| <value>Target framework '{0}' is not supported by Azure Functions. Minimum target framework is {1}</value> | |
| <value>Target framework '{0}' is not supported by Azure Functions. Minimum target framework is '{1}'</value> |
|
Going to address comments in future PR. |
* Add basic function app validation * Fix rebase
Issue describing the changes in this PR
Part of #3131
Pull request checklist
release_notes.mdAdditional information
Adds targets to validate basic state of a function app. This is primarily a port from the existing SDK, with some key changes:
_ToolingSuffixis calculated (so we don't need to have a case for each TFM)AZFWin the0100-0199range. These will be reserved for build/sdk errors. By using MSBuild code logging, we can participate in theNoWarn,TreatWarningsAsErrors, etc semantics of msbuild.