-
Notifications
You must be signed in to change notification settings - Fork 680
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
implement JSON schema for inspec exec
json outputs
#1564
Conversation
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.
This is a huge improvement. Thank you @arlimus. I really like that we have a clear specification now. The only thing that I was confused with is the location of the files in lib/utils/schema
. Do we intent do use those in core inspec as well or are they for testing purposes only? If the latter, should we move the directory to test/unit/schema
?
lib/utils/schema/schema.rb
Outdated
'statistics' => statistics, | ||
'version' => { 'type' => 'string' }, | ||
|
||
# DEPRECATED PROPERTIES!! These will be removed with the next major version bump |
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.
What would be a good way do communicate this?
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.
We could put it into the output of the generator
I don't like the location either; |
Just discussed with @arlimus offline and we came to the conclusion that the current location is the best one, since we want to use the schema to verify reports too in future. |
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.
Left a few comments, and it looks like you have some lint errors to contend with.
@@ -0,0 +1 @@ | |||
{"type":"object","additionalProperties":false,"properties":{"profiles":{"type":"array","items":{"type":"object","additionalProperties":false,"properties":{"name":{"type":"string"},"version":{"type":"string","optional":true},"title":{"type":"string","optional":true},"maintainer":{"type":"string","optional":true},"copyright":{"type":"string","optional":true},"copyright_email":{"type":"string","optional":true},"license":{"type":"string","optional":true},"summary":{"type":"string","optional":true},"supports":{"type":"array","items":{"type":"object","additionalProperties":false,"properties":{"os-family":{"type":"string","optional":true}}},"optional":true},"controls":{"type":"array","items":{"type":"object","additionalProperties":false,"properties":{"id":{"type":"string"},"title":{"type":["string","null"]},"desc":{"type":["string","null"]},"impact":{"type":"number"},"refs":{"type":"array","items":{"type":"object","additionalProperties":false,"properties":{"ref":{"type":"string"},"uri":{"type":"string","optional":true},"url":{"type":"string","optional":true}}}},"tags":{"type":"object"},"code":{"type":"string"},"source_location":{"type":"object","properties":{"ref":{"type":"string"},"line":{"type":"number"}}},"results":{"type":"array","items":{"type":"object","additionalProperties":false,"properties":{"status":{"type":"string"},"code_desc":{"type":"string"},"run_time":{"type":"number"},"start_time":{"type":"string"},"skip_message":{"type":"string","optional":true},"resource":{"type":"string","optional":true}}}}}}},"groups":{"type":"array","items":{"type":"object","additionalProperties":false,"properties":{"id":{"type":"string"},"title":{"type":"string","optional":true},"controls":{"type":"array","items":{"type":"string"}}}}},"attributes":{"type":"array"}}}},"statistics":{"type":"object","additionalProperties":false,"properties":{"duration":{"type":"number"}}},"version":{"type":"string"},"controls":"array","other_checks":"array"}} |
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.
Is there any harm in prettyifying this to make it easier for people to see? I realize the output out of the tool isn't "pretty" (and perhaps we should change that) but for purposes of comprehension, it'd be good to see this formatted nicely for humans.
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.
We certainly can :) I left it compressed initially to make sure this is just the minified schema. Reasoning should be done via the schema.rb
file, changes too; These schemas get complex rather quicly where the schema.rb
is easy to understand and follow. By making it more human-readable we give more access to it, but may also invite people to try to change the json file directly.
What do you think?
@@ -0,0 +1 @@ | |||
{"type":"object","additionalProperties":false,"properties":{"statistics":{"type":"object","additionalProperties":false,"properties":{"duration":{"type":"number"}}},"version":{"type":"string"},"controls":{"type":"array","items":{"type":"object","additionalProperties":false,"properties":{"id":{"type":"string"},"profile_id":{"type":["string","null"]},"status":{"type":"string"},"code_desc":{"type":"string"},"skip_message":{"type":"string","optional":true},"resource":{"type":"string","optional":true}}}}}} |
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.
Same comment as inspec.exec.full.json - would love to see this structured so humans can grok it.
lib/utils/schema/schema.rb
Outdated
@@ -0,0 +1,164 @@ | |||
#!/usr/bin/env ruby |
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.
For safety/paranoia purposes, should we wrap these methods in a module? I'd hate for something to accidentally require
this and have some methods overridden.
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.
Sure, good point! Also valid since it's still in lib/utils
instead of some extras
folder, so I fully 👍 this argument
2837a80
to
b981b13
Compare
We had 2 more suggestions to make this part of the inspec CLI. While I hesitate to bloat the CLI calls, Adam also mentioned to make it a hidden CLI command. I like this approach, as it is only needed in m2m cases and should not bother regular users. That way the location of |
@chris-rock @adamleff updated to |
Signed-off-by: Dominik Richter <dominik.richter@gmail.com>
This is a current draft to implement JSON schema verification for InSpec
json
andjson-min
formatters.It would also have prevented this #884 (comment) 😄
Fixes #884
This is a low-priority item, no rush.