Skip to content

disa-alignment: Upload ARF results to artifacts as well in case of failure - #656

Draft
ggbecker wants to merge 1 commit into
RHSecurityCompliance:mainfrom
ggbecker:disa-alignment-arf-results
Draft

ggbecker wants to merge 1 commit into
RHSecurityCompliance:mainfrom
ggbecker:disa-alignment-arf-results

Conversation

@ggbecker

Copy link
Copy Markdown
Contributor

I'm not sure this logic of uploading a log artifact only when the test fails makes sense. From what I see there are no cases like this across the project.

ARF files can take some space and I think they would only be good when there is actually a failure to investigate.

The HTML reports already have good OVAL results and provide the source of the failure, so it's rather complementing.

Please advise and/or feel free to reject.


disa-alignment: Upload ARF results to artifacts as well in case of test failure.

These files can be more easily processed locally by AI for example and can help reviewing more in depth the differences between the two sources of results.

In case of test failure.

These files can be more easily processed locally by AI for example and
can help reviewing more in depth the differences between the two sources
of results.
@comps

comps commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

The typical policy of Contest is to always upload all logs, even on pass, and potentially filter them out on a higher level, e.g. when post-processing results.

The reason is that - sometimes - passing logs are very useful for comparison - just this week, I was able to compare ansible-playbook outputs and find out that recent bootc issues are applicability based, because the passing test did the remediation, and failing test skipped it.

But if ARF XMLs of passing tests are not useful in any possible scenario, maybe we could skip the upload?

#545 is very related - a similar deviation from the upload-on-pass policy, for performance reasons.

Maybe we could finally fix that issue + this PR, perhaps by something like

  1. Adding a function that returns True if extra logs should be uploaded
    • This would check the failure counts + take into account some env var (if it is set to 1, logs are always uploaded, even for pass)
  2. Making this code + per-rule use it

@matusmarhefka WDYT?

@matusmarhefka

Copy link
Copy Markdown
Contributor

The typical policy of Contest is to always upload all logs, even on pass, and potentially filter them out on a higher level, e.g. when post-processing results.

The reason is that - sometimes - passing logs are very useful for comparison - just this week, I was able to compare ansible-playbook outputs and find out that recent bootc issues are applicability based, because the passing test did the remediation, and failing test skipped it.

But if ARF XMLs of passing tests are not useful in any possible scenario, maybe we could skip the upload?

#545 is very related - a similar deviation from the upload-on-pass policy, for performance reasons.

Maybe we could finally fix that issue + this PR, perhaps by something like

1. Adding a function that returns True if extra logs should be uploaded
   
   * This would check the failure counts + take into account some env var (if it is set to 1, logs are always uploaded, even for pass)

2. Making this code + per-rule use it

@matusmarhefka WDYT?

@comps Yes, this proposal sounds good to me.

@ggbecker If you don't want to implement what @comps suggested within this PR we can file an issue for it and merge this PR as is.

@ggbecker

Copy link
Copy Markdown
Contributor Author

I would only open the issue and do not merge this PR. I think the current disa alignment test already provides good information so we are not in a hurry to have the arf results. So I would file the issue instead to implement this logic properly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants