-
Notifications
You must be signed in to change notification settings - Fork 2.7k
refactor(misconf): migrate from custom Azure JSON parser #9222
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: nikpivkin <[email protected]>
Signed-off-by: nikpivkin <[email protected]>
Signed-off-by: nikpivkin <[email protected]>
9da60be
to
308f3d1
Compare
Signed-off-by: nikpivkin <[email protected]>
Signed-off-by: nikpivkin <[email protected]>
Signed-off-by: nikpivkin <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors the Azure ARM parser to replace a custom JSON parser with the standardized github.com/go-json-experiment/json
library. This modernization simplifies the codebase significantly by removing approximately 1,500 lines of custom parsing logic while maintaining equivalent functionality.
- Replaced custom armjson parsing library with go-json-experiment/json
- Introduced a hook system for capturing metadata during JSON unmarshaling
- Updated Value struct to use pointer-based metadata management
Reviewed Changes
Copilot reviewed 42 out of 42 changed files in this pull request and generated 4 comments.
Show a summary per file
File | Description |
---|---|
pkg/x/json/json.go | Enhanced with generic hooks and location tracking capabilities |
pkg/iac/scanners/azure/value.go | Refactored to implement json.UnmarshalerFrom interface |
pkg/iac/scanners/azure/arm/parser/template.go | New ParseTemplate function with metadata collection system |
pkg/iac/scanners/azure/arm/parser/parser.go | Simplified to use new ParseTemplate function |
Multiple armjson files | Removed entire custom JSON parser implementation |
Comments suppressed due to low confidence (1)
pkg/iac/scanners/azure/arm/parser/template.go:69
- Field name 'Loc' is inconsistent with other fields and unclear. It should be renamed to 'Location' to match the original field name and improve readability.
Loc azure.Value `json:"location"`
@nikpivkin although somewhat unrelated - I recall us not being able to save positional arguments for helm charts. Could we use a similar hook based approach for that too? |
Are you referring to the positions in the original Helm templates? I don't think that's possible. I left a comment in this discussion. |
Signed-off-by: nikpivkin <[email protected]>
Signed-off-by: nikpivkin <[email protected]>
Signed-off-by: nikpivkin <[email protected]>
Signed-off-by: nikpivkin <[email protected]>
Location azure.Value `json:"location"` | ||
Loc azure.Value `json:"location"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason to change the field? Location seems self explanatory.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, just left one small nit.
Description
This PR replaces the custom JSON parser with
github.com/go-json-experiment/json
for parsing ARM templates. This significantly simplifies the codebase and reduces maintenance overhead.Instead of manually parsing the JSON structure, and positional information, the unmarshaler from
github.com/go-json-experiment/json
and some helper functions fromx/json
are now used.The only non-trivial part of the migration was correctly initializing the metadata object with:
• file system (fs.FS),
• file path,
• range (position of the node in the source file),
• reference to the corresponding JSON node in the tree.
To support populating this metadata during decoding, a hook system was introduced. Currently, only the
After
hook is implemented. It is triggered after decoding each object and provides access to:• decoder,
• decoded object,
• source location of the JSON node.
Related issues
Remove this section if you don't have related PRs.
Checklist