Azure / Azure/bicep-types

Fix serializer in `TypeSerializer` to better handle modern scope properties

Open
#736 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
13
Forks
25
Avg merge
5h 14m
Merged PRs (30d)
16

Description

[Here is a temporary bandaid (Bicep repo)](https://github.com/Azure/bicep/blob/3f2dad6414e29caebeb39f0d42a5d8073beca74a/src/Bicep.Core/TypeSystem/Providers/Extensibility/ExtensionResourceTypeFactory.cs#L253C13-L257C14) to handle 3 failing tests that I couldn't get to pass. Here is what I found so far:

In the ExtensionResourceTypeHelper.cs, we are serializing Resource types but upon serialization the type definitions that are generated have both legacy and modern properties. Even though readableScopes and writableScopes are set to ScopeType.All, this is serialized to JSON that includes both modern properties (readableScopes: 31, writableScopes: 31) AND legacy properties (scopeType: null, readOnlyScopes: null).

The problem is when this gets deserialized, the deserializer prioritizes the legacy null values and converts them to ScopeType.None, completely ignoring the correct modern values. This causes extension resources like request@v1 to be incorrectly flagged as ReadOnly, triggering `BCP245 errors: "Resource type 'request@v1' can only be used with the 'existing' keyword"` in LocalDeployCommand tests.

Potential solution:
I think we may need to add [DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull](https://learn.microsoft.com/en-us/dotnet/standard/serialization/system-text-json/ignore-properties#ignore-all-null-value-properties) to https://github.com/Azure/bicep-types/blob/fc9ae27d9f4aa5a6962aeb2bc09216702fc51adf/src/Bicep.Types/Serialization/TypeSerializer.cs#L18-L27 (Bicep-types repo)

AFAICT no other consumer of Bicep.Types is using the C# code to serialize types (only to deserialize them).

Things to do once serialization is fixed:
- Rip off [bandaid](https://github.com/Azure/bicep/blob/3f2dad6414e29caebeb39f0d42a5d8073beca74a/src/Bicep.Core/TypeSystem/Providers/Extensibility/ExtensionResourceTypeFactory.cs#L253C13-L257C14)
- Ensure all tests specifically the LocalDeployCommand tests pass

Contributor guide

No contributing guide indexed for this repository

Research direction

Start in Bicep.Types/src/Bicep.Types/Serialization/TypeSerializer.cs and inspect how null legacy scope properties are serialized alongside modern properties. Then review the bandaid in Bicep.Core/TypeSystem/Providers/Extensibility/ExtensionResourceTypeFactory.cs and run the LocalDeployCommand tests. Done means the serializer preserves modern scope values without the bandaid and the affected tests pass.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
tooling
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.