-
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
Add automate reporter #2902
Add automate reporter #2902
Conversation
2d89d30
to
fe38949
Compare
Signed-off-by: Jared Quick <jquick@chef.io>
fe38949
to
644ac27
Compare
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.
Short and simple 👍 Thank you @jquick
# This hashes the passed string into SHA1. | ||
# Then it downgrades the 160bit SHA1 to a 128bit | ||
# then we format it as a valid UUIDv5. | ||
def uuid_from_string(string) |
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.
Interesting idea 👍
lib/inspec/reporters/automate.rb
Outdated
Net::HTTP.start(uri.hostname, | ||
uri.port, | ||
use_ssl: uri.scheme == 'https', | ||
verify_mode: OpenSSL::SSL::VERIFY_NONE) do |http| |
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.
Can we make this a config option? I would like to go to a world where we verify it
Signed-off-by: Jared Quick <jquick@chef.io>
5b8f9f9
to
90c5d2a
Compare
lib/inspec/reporters/automate.rb
Outdated
super(config) | ||
|
||
# default to using no ssl for sending reports | ||
@config['use_ssl'] = @config['use_ssl'] || false |
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.
Would @config['use_ssl'] ||= false
work here?
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.
I think use_ssl
is not precise, because we use ssl in any case. The only difference is that in one case we validate the certificate and in the other we don't. Maybe @config['verify_ssl'] = @config['verify_ssl'] || false
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.
Looks good to me, thanks @jquick
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.
Great work @jquick
@backend.platform[field] | ||
rescue Train::Error => e | ||
Inspec::Log.error(e.message) | ||
nil |
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.
I think we should document, why it is okay to ignore the error here
lib/inspec/reporters/automate.rb
Outdated
super(config) | ||
|
||
# default to using no ssl for sending reports | ||
@config['use_ssl'] = @config['use_ssl'] || false |
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.
I think use_ssl
is not precise, because we use ssl in any case. The only difference is that in one case we validate the certificate and in the other we don't. Maybe @config['verify_ssl'] = @config['verify_ssl'] || false
Signed-off-by: Jared Quick <jquick@chef.io>
Adds the automate reporter to send directly to automate. Currently this report is only usable via the
--json-config
as it requires additional metadata.Fixes #2547
Signed-off-by: Jared Quick jquick@chef.io