Add new options to dump json on failure and enable formatting #86
Conversation
| @@ -4,6 +4,9 @@ This project adheres to [Semantic Versioning](http://semver.org/). | |||
| This CHANGELOG follows the format listed at [Keep A Changelog](http://keepachangelog.com/) | |||
|
|
|||
| ## [Unreleased] | |||
| ### Added - 2018-02-08 | |||
There was a problem hiding this comment.
generally we date it after it has been accepted
There was a problem hiding this comment.
Sorry about that, I got a little ahead of myself following the template. I can update that if you'd like if/when it gets accepted
There was a problem hiding this comment.
no problem I will merge, fixup, and release this hopefully tonight.
| end | ||
|
|
||
| ok message | ||
| rescue => e | ||
| critical "key check failed: #{e}" | ||
| if config[:dump_json] | ||
| json_response = config[:pretty] ? JSON.pretty_generate(json) : json |
There was a problem hiding this comment.
if someone is specifying authentication this is an opportunity for it to leak out. Can we add some redaction for basic auth headers if they exist?
There was a problem hiding this comment.
This should only dump the parsed json from the body of the response after it's been validated, and we're trying to match a specific key/value (https://github.com/jplindquist/sensu-plugins-http/blob/fec20b0bac993a02fabdeedbce67fd70ed6d7bc4/bin/check-http-json.rb#L181-L183). It should not include any headers of the request or response at all as far as I know.
I'll be happy to update it if you want with additional checks if auth headers are set, but otherwise this is just a straight dump of the parsed json body after the request is made and we've received a valid json response
There was a problem hiding this comment.
you are correct on closer inspection.
| end | ||
|
|
||
| ok message | ||
| rescue => e | ||
| critical "key check failed: #{e}" | ||
| if config[:dump_json] | ||
| json_response = config[:pretty] ? JSON.pretty_generate(json) : json |
There was a problem hiding this comment.
you are correct on closer inspection.
| @@ -4,6 +4,9 @@ This project adheres to [Semantic Versioning](http://semver.org/). | |||
| This CHANGELOG follows the format listed at [Keep A Changelog](http://keepachangelog.com/) | |||
|
|
|||
| ## [Unreleased] | |||
| ### Added - 2018-02-08 | |||
There was a problem hiding this comment.
no problem I will merge, fixup, and release this hopefully tonight.
|
I will fixup the dep issue on ruby 2.2 prior to merging. |
|
Awesome, thanks so much for the review @majormoses . Let me know if I can do anything else to help |
Add 2 new options
check-http-json.rb:--dump-json, which defaults to false will dump the json response on failureand formatting
--pretty, which defaults to false will format the json output in a pretty format if--dump-jsonis enabledPull Request Checklist
Is this in reference to an existing issue? Possibly? Not sure if this relates to #85 or not honestly
General
Update Changelog following the conventions laid out on Keep A Changelog
Update README with any necessary configuration snippets
Binstubs are created if needed
RuboCop passes
Existing tests pass
New Plugins
Tests
Add the plugin to the README
Does it have a complete header as outlined here
Purpose
Allow users to include the json response in the result on failure, and format it if they so choose
Known Compatibility Issues
None, existing functionality remains exactly the same, just adding 2 new options