#214 - Restore output-type to cfn_nag #215
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
byronic
added a commit
that referenced
this pull request
May 30, 2019
Complies with Jesse's PR #215, adding output format option to cfn_nag
ghost
reviewed
May 31, 2019
bin/cfn_nag
Outdated
| end | ||
|
|
||
| cfn_nag.render_results(aggregate_results: aggregate_results, |
There was a problem hiding this comment.
doesn't this change the contract on the results?
There was a problem hiding this comment.
given we just bumped to 0.4.x, i guess it's not horrible to change the contract though.... and in the case of processing multiple explicit files via ARGF.... a good enhancement?
There was a problem hiding this comment.
@erickascic This does change the result but towards consistency. I think it is a 👍
ghost
pushed a commit
that referenced
this pull request
May 31, 2019
* Refactor CLI Interface, Add Scan fail-on-warnings [Finishes #165899506]; [Finishes #165899494]; factors out command line arguments reading; adds fail-on-warnings support to cfn_nag_scan; removes repeated code across cfn_nag_scan and cfn_nag by refactoring into an executor. Post-refactor, rubocop and testing required the following included supplmental changes: CfnNagExecutor is too large if read_cli_options is included, so factors that function out to its own file; CfnNag class is too large with the added fail_on_warnings option, refactors initialize parameters as a separate CfnNagConfig class; refactors the cfn_nag integration tests to use the new signatures * Fix Missing validate_options Call Fixes a missing call to validate_options in the executor; refactors to stay within the 15 line per function bounds of rubocop * Executor Tests, Name Fixes Adds naming corrections per @erickascic 's review; adds tests for CfnNagExecutor class at 88% * Test Coverage Bump Adds additional tests to get cfn_nag_executor and other introduced files to 100% coverage * Keep cli_options Backwards Compatible Updates cli_options to be backwards compatible -- removes alphabetical ordering, splits into two options categories for cfn_nag and cfn_nag_scan * Factory Pattern, cleanup CfnNagExecutor arguments Refactors cli_options and argf_supplier into a factory-pattern from lambdas; changes the CfnNagExecutor to have no initializer arguments and scan to properly have a cfn_nag_scan argument instead, for clarity * Test Coverage Raises Argf and Options test coverage to 100% * Resolve Test Issues, Incorporate Eric's Changeset Incorporates Eric's changeset from Slack; removes a test that is both no longer necessary and not working under the new changes; resolves rubocop issues * No @ Allowed In Trollop.options Removes @ items used in Trollop.options block -- those are unsupported and cause nil class exceptions * Resolve Failing e2e Tests - Extraneous puts An extraneous puts options call in cfn_nag_executor was causing failures in the e2e tests; removes * Add output format option to cfn_nag Complies with Jesse's PR #215, adding output format option to cfn_nag * Rename execute_single_scan, options type, deprecated argf class Per Eric Kascic's review, removes the Argf class (which was unused after prior changes); renames execute_single_scan execute_file_or_piped_scan since the argument can be an explicit file or piped input; changes the options type value for file/piped scans to 'file' from 'cli'
added 3 commits
June 3, 2019 06:53
jesseadams
force-pushed
the
cfn-nag-output-format
branch
from
6377250
to
f859093
Compare
byronic
approved these changes
Jun 3, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #214
Syncs up cfn_nag and cfn_nag_scan so they use the same results renderers and have consistent output.