Fix Get-AzWebApp deserialization failure for Azure Storage mount type "FileShare" - #29981
Fix Get-AzWebApp deserialization failure for Azure Storage mount type "FileShare"#29981Aditya Pujara (a0x1ab) with Copilot wants to merge 4 commits into
Conversation
There was a problem hiding this comment.
Copilot wasn't able to review any files in this pull request.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
Live test skipped⏭️ Skipping the live test for this revision because no changed test file was found under a The live-test pipeline runs only the scenario/xUnit test files a PR changes, so there is nothing to execute for this commit. This is informational — a regression test is encouraged where it makes sense, but not required. If a test file is added in a later commit, the live test will run automatically. Posted by agent-assist (autonomous bug-fix pipeline). |
…are" Co-authored-by: a0x1ab <59631311+a0x1ab@users.noreply.github.com>
Co-authored-by: a0x1ab <59631311+a0x1ab@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Suppressed comments (2)
src/Websites/Websites.Test/ScenarioTests/AzureStorageTypeJsonConverterTests.cs:21
- The test file's namespace uses
WebAppseven though the Websites test project root namespace and other ScenarioTests useMicrosoft.Azure.Commands.Websites.Test.ScenarioTests. This inconsistency makes navigation/search harder and deviates from the established naming used across the project.
namespace Microsoft.Azure.Commands.WebApps.Test.ScenarioTests
src/Websites/Websites/Utilities/AzureStorageTypeJsonConverter.cs:51
Enum.TryParsewill successfully parse numeric strings (and may accept values not actually defined inAzureStorageType), which conflicts with the intent of treating unknown values as null. Consider validating the parsed enum value is one of the defined members (and treating whitespace-only values as null) so truly unknown values don't get surfaced as an undefined enum value.
string value = reader.Value?.ToString();
AzureStorageType result;
if (!string.IsNullOrEmpty(value) && Enum.TryParse(value, ignoreCase: true, result: out result))
{
return result;
Live test results — TestFx
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Suppressed comments (3)
src/Websites/Websites/Utilities/AzureStorageTypeJsonConverter.cs:52
- ReadJson uses Enum.TryParse on the enum member name, which may not match the SDK's StringEnumConverter behavior (e.g., EnumMember values, naming strategies). To preserve SDK-compatible parsing while still being lenient for unknown values, delegate to StringEnumConverter.ReadJson and swallow the known exceptions, returning null for unrecognized values.
string value = reader.Value?.ToString();
AzureStorageType result;
if (!string.IsNullOrEmpty(value) && Enum.TryParse(value, ignoreCase: true, result: out result))
{
return result;
src/Websites/Websites/Utilities/AzureStorageTypeJsonConverter.cs:80
- ResolveContractConverter only checks for AzureStorageType, but the contract resolver can be asked about Nullable depending on how the contract is built. Using the same nullable-unwrapping logic as CanConvert makes this more robust and avoids reintroducing the original failure if the resolver receives a nullable type.
protected override JsonConverter ResolveContractConverter(Type objectType)
{
if (objectType == typeof(AzureStorageType))
{
return null;
src/Websites/Websites.Test/ScenarioTests/AzureStorageTypeJsonConverterTests.cs:21
- This test file's namespace uses "WebApps" but the rest of Websites.Test scenario tests use the "Websites" namespace prefix. Aligning the namespace keeps test organization consistent and makes it easier to locate alongside other Websites scenario tests.
namespace Microsoft.Azure.Commands.WebApps.Test.ScenarioTests
There was a problem hiding this comment.
Automated CI + Live-Test Review — PR #29981
CI checks: ✅ all passed (2/2).
Live test (TestFx Record): ❌ FAILED
- Test project:
src/Websites/Websites.Test - Filter:
FullyQualifiedName~AzureStorageTypeJsonConverterTests - Changed test file:
src/Websites/Websites.Test/ScenarioTests/AzureStorageTypeJsonConverterTests.cs - Workflow run: https://github.com/Azure/issue-sentinel/actions/runs/31360062058
The dotnet test run for AzureStorageTypeJsonConverterTests exited non-zero
(no captured stdout/log was available from the runner, but the job step
explicitly reported TestFx tests failed). This test exercises the
AzureStorageTypeJsonConverter used to deserialize Get-AzWebApp mount
configuration, which is exactly what this PR (fix for issue #29979,
Get-AzWebApp deserialization failure for Azure Storage mount type
FileShare) is meant to fix. Please:
- Re-run
src/Websites/Websites.Test/ScenarioTests/AzureStorageTypeJsonConverterTests.cs
locally and inspect the actual assertion failure/exception. - Verify the
FileSharemount type fix in the converter/deserialization code
handles all enum cases and any recorded HTTP session fixtures the test
relies on (check for a matching.jsonsession record under
src/Websites/Websites.Test/SessionRecords/if the test uses TestFx
playback). - Push a fix so this test passes, then this PR will be re-reviewed
automatically.
Posted by agent-assist (autonomous bug-fix pipeline).
|
Started a Copilot task using Posted by agent-assist (autonomous bug-fix pipeline). |
Co-authored-by: a0x1ab <59631311+a0x1ab@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Suppressed comments (1)
src/Websites/Websites/Utilities/AzureStorageTypeJsonConverter.cs:80
- AzureStorageTypeContractResolver only bypasses the enum’s type-level JsonConverter when ResolveContractConverter is called with objectType == typeof(AzureStorageType). In the original failure stack trace, Json.NET was converting to System.Nullable, so ResolveContractConverter may be asked about the nullable wrapper type as well. Handling the nullable case defensively avoids regressions where the SDK’s StringEnumConverter attribute would still take precedence and throw on unknown values.
protected override JsonConverter ResolveContractConverter(Type objectType)
{
if (objectType == typeof(AzureStorageType))
{
return null;
}
return base.ResolveContractConverter(objectType);
}
Live test results — TestFx
|
There was a problem hiding this comment.
Automated Review Summary
PR: Fix Get-AzWebApp deserialization failure for Azure Storage mount type "FileShare" (fixes #29979)
Tester (live-test-powershell.yml): ✅ Passed — TestFx Record run completed successfully for the changed test file(s).
CI checks: ✅ 2/2 checks passed, no failures, nothing pending.
Everything is green. No further action needed from the automated pipeline; ready for human/maintainer review and merge decision.
Posted by agent-assist (autonomous bug-fix pipeline).
Get-AzWebAppthrowsMicrosoft.Rest.SerializationExceptionwhen a web app'sazureStorageAccountsconfiguration contains a mounttypevalue the SDK'sAzureStorageTypeenum doesn't define (e.g."FileShare", returned for Logic App Standard "File System" connections backed by an SMB share).Root cause
Microsoft.Azure.Management.WebSites.Models.AzureStorageTypeonly definesAzureFiles/AzureBlob. The enum carries a type-level[JsonConverter(typeof(StringEnumConverter))]attribute, so any unrecognized string value throws instead of degrading gracefully — this attribute-level converter takes precedence over anything registered inJsonSerializerSettings.Converters, so simply adding a converter isn't enough to intercept it.Fix
AzureStorageTypeJsonConverter: a lenientJsonConverterforAzureStorageTypethat returnsnullfor unrecognized values instead of throwing, while still parsing known values normally.WriteJsondelegates toStringEnumConverterto preserve correctEnumMemberserialization.AzureStorageTypeContractResolver: extendsReadOnlyJsonContractResolver(the resolver the generated client uses by default) and overridesResolveContractConverterto bypass the type-level attribute specifically forAzureStorageType, letting the lenient converter above actually take effect.WebsitesClient'sDeserializationSettingsso all reads through the client tolerate unknown storage mount types.{ "azureStorageAccounts": { "FileSystem": { "type": "FileShare", "accountName": "..." } } }now deserializes with
Typeleftnullfor the unrecognizedFileSharevalue, instead of failing the entireGet-AzWebAppcall.Tests
Added unit tests covering unknown, known, and null
typevalues.