Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JSON schemas should be strict validatable #780

Closed
o-l-a-v opened this issue Oct 16, 2024 · 6 comments
Closed

JSON schemas should be strict validatable #780

o-l-a-v opened this issue Oct 16, 2024 · 6 comments
Assignees
Labels
bug Something isn't working

Comments

@o-l-a-v
Copy link
Contributor

o-l-a-v commented Oct 16, 2024

Describe the bug

All EPAC JSON schemas should be strict validatable, to ensure definitions does not have unwanted properties that might break build and deploy.

I'd like to avoid writing our own linter for this, hopefully this is something that can be implemented in EPAC itself.

I'm no JSON schema expert, maybe adding "additionalProperties": false as a "root" value isn't enough? Seems like that's what Scoop does:

Currently, none of the EPAC schemas have this:

To Reproduce

See issue comment: #777 (comment)

Expected behavior

Non-valid JSON / JSONC should not go through EPAC.

Screenshots

None.

EPAC Version

Not relevant, JSON schema issue AFAIK.

@o-l-a-v o-l-a-v added the bug Something isn't working label Oct 16, 2024
@o-l-a-v o-l-a-v changed the title JSON schemas should all have "additionalProperties": false for strict validation JSON schemas should be strict validatable Oct 16, 2024
@o-l-a-v
Copy link
Contributor Author

o-l-a-v commented Oct 16, 2024

Maybe it'd be smart to use a JSON schema version VSCode actually supports? 🤔

PowerShell seems to have problem with 2019 and newer too:

@anwather
Copy link
Collaborator

Looks like additionalProperties might work.

@anwather
Copy link
Collaborator

Updated schema to use this value - it seems to not like the additional property of id in your example but then doesn't have a problem with type so not sure what it going on there.

@anwather
Copy link
Collaborator

See comment in #777 re why the schema is provided - we can still look at adding additionalProperties but I don't believe that we'll be able to use the schema to test things like removed or invalid parameters - can do that with Pester and the module provided.

@o-l-a-v
Copy link
Contributor Author

o-l-a-v commented Oct 17, 2024

Thanks @anwather. We'll take a look at AzPolicyTest. We've linted wanted "root values" for EPAC policyDefinitions with something like this for now:

$AllDefinitions.Where{
    $(
        [string[]](
            Compare-Object -PassThru -CaseSensitive -ReferenceObject (
                (
                     ConvertFrom-Json -InputObject (Get-Content -Raw -Path $_.'Path')
                ).'PSObject'.'Properties'.Where{$_.'MemberType' -eq 'NoteProperty'}.'Name'
            ) -DifferenceObject '$schema','name','properties'
        )
    ).'Count' -gt 0
}

@anwather
Copy link
Collaborator

Closing as discussed with dev team - the purpose of the schema is to provide some level of Intellisense, not to support validation of policy files.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants