pipelines-as-code icon indicating copy to clipboard operation
pipelines-as-code copied to clipboard

Enable knowledge of standardized results in PaC to support decisions on non-failing PipelineTasks

Open arewm opened this issue 2 years ago • 2 comments

In situations where there are many related tasks in a single PipelineRun, I want to be able to have all tasks run "successfully" so that the entire pipeline will be run even if some tasks are not strictly successful. While it makes sense to have some PipelineRuns actually fail (for example, if an artifact build fails), there are other types of tasks that might successfully run while also reporting issues (for example, test or vulnerability scans). In addition to formalizing on a result name, we can also formalize on the minimum data reported in these results so that PaC would be able to return consistent data.

While there would be a standard definition for PaC, Tekton/PaC should not strictly enforce the fields to enable further extension of the result types by tasks as needed.

Proposal for standardizing on test-type tasks

Tekton result

TEKTON_TEST_RESULT

Required fields

  • result - ENUM containing values SUCCESS, FAILURE, WARNING, SKIPPED or ERROR
  • successes - the number of successes returned in a test
  • failures - the number of failures returned in a test
  • warnings - the number of warnings returned in a test

Expected handling

result value(s) GitHub status conclusion value
SUCCESS success
SKIPPED skipped
FAILURE, ERROR failure
WARNING neutral

Sample output

{
    "result": "SUCCESS",
    "timestamp": "1649843611",
    "successes": 16,
    "note": "An example of an extra field ignored by PaC",
    "failures": 0,
    "warnings": 0
}

Proposal for standardizing on vulnerability scan-type results

Tekton result

TEKTON_SCAN_RESULT

Required fields

Following the convention of security ratings: https://access.redhat.com/security/updates/classification/, the following results should be supported

  • critical
  • high
  • medium
  • low

Expected handling

PaC should return the result of the most severe non-zero result in the Checks table.

In order to enable users to change the Checks conclusion based on the presence of vulnerabilities, PaC can enable a customization for a minimum_allowed_scan_result. If this parameter is specified, then a maximum result at or below the configured value would correspond to a conclusion of success and any value above would correspond to a conclusion of failure. If no value is set, all results would return a conclusion of neutral.

Sample output

{
  "critical": 0,
  "high": 1,
  "medium": 1,
  "low":0
}

Would be handled with a maximum result of high.

arewm avatar Apr 13 '23 14:04 arewm

If I understand correctly, you are looking for a more detailed status report on GitHub for your Pipelineruns, beyond just a binary "Success/Failure" status. It seems to me that you are approaching this from a security perspective, specific to RHT. However, I believe that a more generic solution would be more appropriate.

What if we added an annotation to the Pipelinerun that allows users to control the GitHub checks status? For example:

pipelinesascode.tekton.dev/results-status: "github-status-results"

This annotation would enable PAC to look into the Pipelinerun result value of github-status-results and map that status to the VCS status on GitHub (i.e. success, skipped, failure, neutral). The decision-making power would then be held by a final task controlled by the user to output the GitHub status.

On your end, you can implement the mapping as you described and tweak it as needed, without having PAC do it for you.

We could perhaps as well include a custom message to pass through in the checks tab for additional information.

chmouel avatar Apr 14 '23 11:04 chmouel

That is roughly what I am trying to do, yes. I understand the desire to generalize the approach from a PAC perspective. With this approach, would PAC be performing the mapping back to GitHub statuses or would that happen by the task itself as part of the identified status result? If PAC is performing this mapping, what will the mapping look like? Would there be an expected key in a JSON result to indicate whether there is a pass/fail? Looking again at the original proposal, PAC would only need to be concerned about the result key:

result - ENUM containing values SUCCESS, FAILURE, WARNING, SKIPPED, or ERROR

The other proposed ones would be unnecessary.

As Pipelines are composed of heterogenous task definitions, all tasks might not have the same "types" of results like I indicated in the original post. If I ant to adhere to a specific API for task result formats to improve interoperability, I might need multiple types of results. The success/test-like and quality/scan-like results are only the two immediate cases that come to mind, but I imagine that there can be others.

One possible way to reduce this previous concern from PAC's perspective is to enable an annotation as suggested to be a list where any of the configured results can be used to report the status and if multiple are reported, the first on the list would take precedence. If none are detected then the fallback would be for the pipeline/task's ultimate status (or the current behavior).

arewm avatar Apr 17 '23 19:04 arewm