Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a helper for saving command output in a file #3286

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

happz
Copy link
Collaborator

@happz happz commented Oct 14, 2024

This will be a very needed functionality: the plugin runs a command, and its output needs to be saved in a dedicated file, usually because the output represents some interesting, standalone piece of information. For example, an AVC denial report generated by ausearch.

If it were the only such actor, it would be fine to keep the implementation in the avc plugin. But, with the advent of saving results of the prepare and finish phases, the question is, if I get results.yaml for a prepare step, would it even contain anything besides pass or error? Well, it can contain logs of commands like ansible-playbook or shell scripts. And suddenly we have several plugins that may need to save some walls of text, usually produced by a command, in files, and then link those files to results and present them to a user.

And because of that, it makes sense to provide a helper function that would produce unified output for all of them. Why should avc separate stdout and stderr by two lines and prepare/shell with just one? Not on my watch.

Pull Request Checklist

  • implement the feature

@happz happz added the code | style Code style changes not affecting functionality label Oct 14, 2024
@happz happz added this to the 1.38 milestone Oct 14, 2024
@happz happz added the ci | full test Pull request is ready for the full test execution label Oct 14, 2024
@happz happz force-pushed the report-file-command-helper branch 2 times, most recently from 99f2692 to 4b6e215 Compare October 17, 2024 10:25
@happz happz marked this pull request as ready for review October 17, 2024 10:26
@happz happz force-pushed the report-file-command-helper branch 2 times, most recently from b3f7c66 to 8eff501 Compare October 22, 2024 07:16
@thrix thrix modified the milestones: 1.38, 1.39 Oct 25, 2024
@happz happz added the status | ready for merge The only missing piece is to do the rebase the current 'main' and let the CI finish. label Oct 31, 2024
@psss
Copy link
Collaborator

psss commented Nov 1, 2024

Some problem on TF side?

Guest couldn't be provisioned: Artemis resource ended

Let's reschedule?

Copy link
Collaborator

@psss psss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

yield from output_lines
yield OUTPUT_WIDTH * '~'
yield f'{comment_sign} {name} ({line_summary} lines)'
yield comment_sign + (OUTPUT_WIDTH - 1) * '~'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
yield comment_sign + (OUTPUT_WIDTH - 1) * '~'
yield f'{comment_sign} ' + (OUTPUT_WIDTH - 2) * '~'

Would you be ok with this as well? I find it a little bit more pleasing to the eye...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't mind, just need to fix tests after such a change.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would be nice, thanks!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in 21f1896

This will be a very needed functionality: plugin runs a command, and its
output needs to be saved in a dedicated file, usualy because the output
represents some interesting, standalone piece of information. For
example, an AVC denial report generated by `ausearch`.

If it would be the only such actor, it would be fine to keep the
implementation in the `avc` plugin. But, with the advent of saving
results of `prepare` and `finish` phases, the question is, if I get
`results.yaml` for a `prepare` step, would it even contain anything
besides `pass` or `error`? Well, it can contain logs of commands like
`ansible-playbook` or shell scripts. And suddenly we have several
plugins that may need to save some walls of texts, usually produced by a
command, in files, and then link those files to results and present them
to user.

And because of that, it makes sense to provide a helper function that
would produce unified output for all of them? Why should `avc` separate
stdout and stderr by two lines and `prepare/shell` with just one? Not on
my watch.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci | full test Pull request is ready for the full test execution code | style Code style changes not affecting functionality status | ready for merge The only missing piece is to do the rebase the current 'main' and let the CI finish.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants