#3053 Fix by using custom bool parsing that if a string is found then…

[simonsabin]

… it uses the environment replacement and looks for a true false 1 or 0.

Why make this change?

Fixes #3053

Boolean values can't be set using environment variables. This allows that

What is this change?

Using custom JsonConverter for bools that if a string is detected it uses the string serialiser that uses the environment replacement rules.

How was this tested?

• Integration Tests
• Unit Tests

[Copilot]

Pull request overview

This pull request fixes issue #3053 by enabling boolean configuration values to be set using environment variables. Previously, boolean values could not use environment variable substitution like string values could.

Changes:

• Added a custom BoolJsonConverter that handles boolean deserialization from both native JSON boolean tokens and string tokens (which enables environment variable substitution)
• Registered the new converter in the JSON serialization pipeline
• Added unit tests to validate boolean values can be set through environment variables

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 8 comments.

File	Description
src/Config/Converters/BooleanJsonConverterFactory.cs	New custom JSON converter that deserializes boolean values from both JSON booleans and strings, enabling environment variable substitution
src/Config/RuntimeConfigLoader.cs	Registers the BoolJsonConverter in the serialization options
src/Service.Tests/UnitTests/RuntimeConfigLoaderJsonDeserializerTests.cs	Adds test coverage for boolean environment variable replacement

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Aniruddh25]

Additional testing for double quoted value boolean value to test the string deserialization.

[simonsabin]

Additional testing for double quoted value boolean value to test the string deserialization.

I think this is now catered for it you mean where the json is the following.

{
 .... 
  "enabled": "true"
  ...
}

and also

{
 .... 
   "enabled": true
  ...
}

[JerryNixon]

I am not sure this resolves JSON Schema constraints/editing errors, does it?

[simonsabin]

I am not sure this resolves JSON Schema constraints/editing errors, does it?

@JerryNixon It doesn't but I've just pushed change to the schema that would.

If thats an acceptable approach I can apply to other booleans.

Interestinlythere does seem to be a mix of json schema validation jsonconverter validation. The jsonconverter stuff looks overly complicated. for some simple serialisation.

[simonsabin]

Added tests to validate the schema approach. for key enabled flags.

[Aniruddh25]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 6 pipeline(s).

[simonsabin]

forgot formatting, try again

[JerryNixon]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 6 pipeline(s).

[Aniruddh25]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 6 pipeline(s).

[Aniruddh25]

wouldnt doing .{3} allow any pattern other than env or akv?

If yes, I think we should retain the previous pattern.

[simonsabin]

This was @JerryNixon 's suggestion. Assume it was a bit of forward planning.

[Aniruddh25]

Looks good, except for the pattern match for @env/@akv.

[simonsabin]

Looks good, except for the pattern match for @env/@akv.

Can you discuss this with @JerryNixon and decide

[simonsabin]

Integration tests aren't passing, is that an issue?

[Aniruddh25]

Integration tests aren't passing, is that an issue?

that's not. rerunning should fix it.

[Aniruddh25]

Looks good, except for the pattern match for @env/@akv.

Can you discuss this with @JerryNixon and decide

checked with Jerry. your previous pattern is what we both recommend.