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

Provide better error message when inspec.yml is invalid #1552

Merged
merged 1 commit into from
Mar 9, 2017

Conversation

adamleff
Copy link
Contributor

@adamleff adamleff commented Mar 8, 2017

Currently, if the inspec.yml for a profile is invalid (such as including
an improperly-defined multi-line string), InSpec will throw an exception
from the YAML parser that does not given a clear indication that the
issue was encountered while parsing the inspec.yml file.

This change introduces a better exception message to clue the user into
where the problem actually lies.

Fixes #1549.

@chris-rock
Copy link
Contributor

@adamleff Awesome. That is so helpful for users.

$ inspec exec examples/profile
Unable to parse inspec.yml: line 8, could not find expected ':' while scanning a simple key

I was playing with the idea for InSpec check:

$ inspec check examples/profile
/Users/chartmann/Development/compliance/inspec/lib/source_readers/inspec.rb:47:in `rescue in load_metadata': Unable to parse inspec.yml: line 8, could not find expected ':' while scanning a simple key (RuntimeError)
	from /Users/chartmann/Development/compliance/inspec/lib/source_readers/inspec.rb:42:in `load_metadata'
	from /Users/chartmann/Development/compliance/inspec/lib/source_readers/inspec.rb:34:in `initialize'
	from /Users/chartmann/Development/compliance/inspec/lib/source_readers/inspec.rb:14:in `new'
	from /Users/chartmann/Development/compliance/inspec/lib/source_readers/inspec.rb:14:in `resolve'
	from /Users/chartmann/Development/compliance/inspec/lib/utils/plugin_registry.rb:18:in `block in resolve'
	from /Users/chartmann/Development/compliance/inspec/lib/utils/plugin_registry.rb:17:in `each'
	from /Users/chartmann/Development/compliance/inspec/lib/utils/plugin_registry.rb:17:in `resolve'
	from /Users/chartmann/Development/compliance/inspec/lib/inspec/source_reader.rb:14:in `resolve'
	from /Users/chartmann/Development/compliance/inspec/lib/inspec/profile.rb:58:in `for_path'
	from /Users/chartmann/Development/compliance/inspec/lib/inspec/profile.rb:69:in `for_fetcher'
	from /Users/chartmann/Development/compliance/inspec/lib/inspec/profile.rb:75:in `for_target'
	from /Users/chartmann/Development/compliance/inspec/lib/inspec/cli.rb:67:in `check'
	from /Users/chartmann/.rvm/gems/ruby-2.3.3/gems/thor-0.19.4/lib/thor/command.rb:27:in `run'
	from /Users/chartmann/.rvm/gems/ruby-2.3.3/gems/thor-0.19.4/lib/thor/invocation.rb:126:in `invoke_command'
	from /Users/chartmann/.rvm/gems/ruby-2.3.3/gems/thor-0.19.4/lib/thor.rb:369:in `dispatch'
	from /Users/chartmann/.rvm/gems/ruby-2.3.3/gems/thor-0.19.4/lib/thor/base.rb:444:in `start'
	from /Users/chartmann/Development/compliance/inspec/bin/inspec:12:in `<top (required)>'
	from /Users/chartmann/.rvm/gems/ruby-2.3.3/bin/inspec:22:in `load'
	from /Users/chartmann/.rvm/gems/ruby-2.3.3/bin/inspec:22:in `<main>'
	from /Users/chartmann/.rvm/gems/ruby-2.3.3/bin/ruby_executable_hooks:15:in `eval'
	from /Users/chartmann/.rvm/gems/ruby-2.3.3/bin/ruby_executable_hooks:15:in `<main>'

Instead of showing a parse error, we should return a response with the error that the yml is not correct. What do you think. Should we do that in another PR?

@adamleff
Copy link
Contributor Author

adamleff commented Mar 9, 2017

@chris-rock If you're talking about changing the output of inspec check to simply a be a one-line message indicating the error and eliminate the stacktrace, I'm totally good with that too and happy to do that in this PR.

If not though, can you restate your suggestion because I'm possibly not understanding it :)

@chris-rock
Copy link
Contributor

As discussed offline, we will do inspec check as a separate ticket.

Copy link
Contributor

@chris-rock chris-rock left a comment

Choose a reason for hiding this comment

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

It is so great to make the UX better @adamleff Thank you @graham1228 for reporting that issue.

Currently, if the inspec.yml for a profile is invalid (such as including
an improperly-defined multi-line string), InSpec will throw an exception
from the YAML parser that does not given a clear indication that the
issue was encountered while parsing the inspec.yml file.

This change introduces a better exception message to clue the user into
where the problem actually lies.

Signed-off-by: Adam Leff <adam@leff.co>
@chris-rock chris-rock force-pushed the adamleff/better-inspec-yml-errors branch from 225fc7c to dfce561 Compare March 9, 2017 17:03
@adamleff adamleff merged commit 1c87db9 into master Mar 9, 2017
@adamleff adamleff deleted the adamleff/better-inspec-yml-errors branch March 9, 2017 17:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants