fix: allow validate input to handle multiple source groups #1815
Conversation
Signed-off-by: Mark Bestavros <mbestavr@redhat.com>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1815 +/- ##
==========================================
- Coverage 80.74% 80.73% -0.01%
==========================================
Files 68 68
Lines 4954 4963 +9
==========================================
+ Hits 4000 4007 +7
- Misses 954 956 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
|
| for _, e := range p.Evaluators { | ||
| results, _, err := e.Evaluate(ctx, evaluator.EvaluationTarget{Inputs: inputFiles}) | ||
| if err != nil { | ||
| log.Debug("Problem running conftest policy check!") |
| log.Debug("\n\nRunning conftest policy check\n\n") | ||
|
|
||
| if err != nil { | ||
| log.Debug("Problem running conftest policy check!") |
| log.Debug("Problem running conftest policy check!") | ||
| return nil, err | ||
| } | ||
| log.Debug("\n\nRunning conftest policy check\n\n") |
There was a problem hiding this comment.
A big unusual to include \n in the log message.
There was a problem hiding this comment.
Yeah, let's remove the extra blanks lines.
There was a problem hiding this comment.
I agree, but just for the sake of completeness, I pulled some of this code from ec validate image and that logging was there too.
| defFunc: mockNewPipelineDefinitionFile, | ||
| }, | ||
| { | ||
| name: "validation succeeds with yaml input", | ||
| fpath: "kind: task", | ||
| err: nil, | ||
| output: &output.Output{PolicyCheck: []evaluator.Outcome{}}, | ||
| output: &output.Output{PolicyCheck: []evaluator.Outcome(nil)}, |
There was a problem hiding this comment.
I'm curious about this diff, is it just a style change?
There was a problem hiding this comment.
It was necessary to allow tests to pass. AFAICT it's just a different type of "nothing" since before it was just an empty list.
|
I created #1858 which includes the changes from the PR and addresses the comments. |
|
#1858 was merged. Let's close this. |
Resolves #1688.
Turns out that the
NewInputfunction only returned the final source group in the policy. This patch makes the function return ALL source groups and changes the caller to evaluate each one, then append all to the output.