kube-bench
kube-bench copied to clipboard
Add --status flag
Fixes #918
This change adds the --status
flag as described on the referenced issue, so that kube-bench
will only log tests results with a check.State
contained in the --status
flag. If the flag is not passed, all messages are logged.
This does not affect the summary output.
Hi! :wave: I'm looking at this issue in the context of Hacktoberfest 2021.
If you think this PR has a meaningful change, and it might not be merged/approved before the end of the month, could you please label it with hacktoberfest-accepted
?
Thank you!
π I've just noticed I did not do any sort of error handling/input check on the --status flag. I'll handle that as well, although it would be great if I could get some feedback before that, to ensure I'm on the right track. π
Codecov Report
Merging #1030 (5ed6c78) into main (dd68e85) will increase coverage by
0.18%
. The diff coverage is71.69%
.
@@ Coverage Diff @@
## main #1030 +/- ##
==========================================
+ Coverage 63.56% 63.75% +0.18%
==========================================
Files 14 14
Lines 1905 1948 +43
==========================================
+ Hits 1211 1242 +31
- Misses 633 645 +12
Partials 61 61
Impacted Files | Coverage Ξ | |
---|---|---|
check/controls.go | 75.11% <0.00%> (-4.49%) |
:arrow_down: |
cmd/common.go | 57.58% <92.50%> (+3.24%) |
:arrow_up: |
cmd/root.go | 34.50% <100.00%> (+0.46%) |
:arrow_up: |
Continue to review full report at Codecov.
Legend - Click here to learn more
Ξ = absolute <relative> (impact)
,ΓΈ = not affected
,? = missing data
Powered by Codecov. Last update dd68e85...5ed6c78. Read the comment docs.
Thanks for contributing, First I will check about the hacktoberfest.
Secondly we would like --status to change the complete output, because the use of it will be to better know what tests in what status especially when we would support more statuses, for example if I would like to see what I need to fix in my system and thats it I will run:
kube-bench --status="FAIL"
And would expect only the failed tests to be outputted with the remediation and the summary of the different parts.
(About the json I'm reluctant from changing the output since it is very easy to run manipulation on json data so maybe we should keep it as informative as possible)
Hi @yoavrotems ! π
I'm not sure I understood what you meant with your comment on what the output should be, so I'll try to clarify it. π
Do you mean that:
- when the --status flag is set, all of the stdout should be filtered, including summary and report;
- Or do you mean that other methods of outputting should also be affected, with the exception of JSON, as you've mentioned?
Thanks for the clarification!
To make this more explicit and hopefully easier to communicate, here is the current output I get from integration tests when I modify them to pass --status=FAIL
: https://gist.github.com/mtpereira/12e0acb97a209b2ae72389d3a640ca28
As you can see, the whole output is only about the checks that FAIL
ed.
Is this not what we want?
Hey you were right in the first part
Hi @yoavrotems ! π
I'm not sure I understood what you meant with your comment on what the output should be, so I'll try to clarify it. π
Do you mean that:
- when the --status flag is set, all of the stdout should be filtered, including summary and report;
- Or do you mean that other methods of outputting should also be affected, with the exception of JSON, as you've mentioned?
Thanks for the clarification!
About the second part all of the output manipulation flags --noremediations --nosummary --noresults effects the stdout only and not the json. And yes it should all co-exist I don't see a reason why not using --noremediations and --status=FAIL
I saw the output you shared status wise it seems good, but I would skip printing remediation for not printed tests like you have with 3.1.1 3.2.1 3.2.2 those tests all are WARN so it not printed but the remediation do.
If someone wanted to see all fails tests the it remediation I guess that WARN tests remediation is not relevant here. if so you should run --status="FAIL,WARN"
wdyt?
@yoavrotems Got it, that makes sense! I'll give it a look. :ok_hand:
Hello @yoavrotems :wave:
How is this looking now? https://gist.github.com/mtpereira/12e0acb97a209b2ae72389d3a640ca28/revisions
:memo: The code needs some refactoring, it is still not ready, but tests are :green_circle: .
Hey looks better the output that not including remediation when its not printing the test is great! This PR is great and part from a bigger change that we want to see, I think you should take in consideration this PR #1035
Hey looks better the output that not including remediation when its not printing the test is great!
This PR is great and part from a bigger change that we want to see, I think you should take in consideration this PR #1035
Hi! π
Oh this might be tricky to handle! π How do you suggest we go about this? Shall we merge one PR first and then proceed with the other? I'm fine either way. π
Let me check it and return to you on how we are going to accept the other PR because we might take as a flag to not break old behaviors
Hey @mtpereira We would accept the #1035 PR but we will do it as a flag for now, the default will be using all of the statuses, but with flag --enhancedoutput=false
to allow backward compatibility, so I think that this PR will need to support filtering all of the statuses.
One thing we need to decide is the behavior when printing unused flag, for example:
./kube-bench --enhancedoutput=false --status="WARN,MANU"
I think that we can do in one of the two ways,
-
When use flag
status
check what filtering is applying and if using new status withenhancedoutput
=false, raise error and not running. -->kube-bench can't output non-supported status, either remove $STATUS from --status, or set --enhancedoutput=true
P.S should also support for something like this one trying none exist status. -
When trying to filter out none exist flag just not print it since there isn't any $STATUS in the output, same as I would run --status="FAIL" and none of the tests failed.
I think that the first option is the best but open to suggestions. WDYT?
Hello @yoavrotems ! :wave:
Option 1 seems better to me as well! I'll give it a look once I have some spare time. :bow: