Skip to content

Support comments in user secrets#7507

Merged
davidfowl merged 1 commit intomicrosoft:mainfrom
Cethric:secretcomments
Feb 11, 2025
Merged

Support comments in user secrets#7507
davidfowl merged 1 commit intomicrosoft:mainfrom
Cethric:secretcomments

Conversation

@Cethric
Copy link
Copy Markdown
Contributor

@Cethric Cethric commented Feb 10, 2025

Description

This solution takes inspiration from JsonConfigurationFileParser.cs in runtime to skip comments in the user secrets json file.
Completing this as a good first issue before trying to work on something larger.

Fixes #3284

Checklist

  • Is this feature complete?
    • Yes. Ready to ship.
    • No. Follow-up changes expected.
  • Are you including unit tests for the changes and scenario tests if relevant?
    • Yes
    • No
  • Did you add public API?
    • Yes
      • If yes, did you have an API Review for it?
        • Yes
        • No
      • Did you add <remarks /> and <code /> elements on your triple slash comments?
        • Yes
        • No
    • No
  • Does the change make any security assumptions or guarantees?
    • Yes
      • If yes, have you done a threat model and had a security review?
        • Yes
        • No
    • No
  • Does the change require an update in our Aspire docs?

Signed-off-by: Cethric <7356786+cethric@users.noreply.github.com>
@dotnet-policy-service dotnet-policy-service Bot added the community-contribution Indicates that the PR has been added by a community member label Feb 10, 2025

private static async Task<JsonObject> GetUserSecretsAsync(string? userSecretsPath, CancellationToken cancellationToken)
{
var jsonDocumentOptions = new JsonDocumentOptions
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I know that JsonSerializerOptions should be static, maybe this one should be too. At least if it can it wouldn't hurt.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree

Copy link
Copy Markdown
Member

@eerhardt eerhardt Feb 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JsonDocumentOptions is a struct and only contains an int, a bool, and an enum. It doesn't cache things like JsonSerializerOptions does.

@davidfowl davidfowl merged commit d6f0f03 into microsoft:main Feb 11, 2025
@github-actions github-actions Bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Mar 10, 2025
@github-actions github-actions Bot locked and limited conversation to collaborators Apr 9, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

community-contribution Indicates that the PR has been added by a community member needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners

Projects

None yet

Development

Successfully merging this pull request may close these issues.

AzureProvisioner user secrets parsing doesn't support comments

4 participants