ci: Use generalized action to test against different distributions#239
Conversation
Reviewer's GuideConsolidates multiple OS-specific Ansible CI workflows into a single matrix-based workflow that runs the same Ansible role tests across various container images using a shared GitHub Action. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The
runs-on: ubuntu-latestline appears twice in thetestjob; drop the duplicate to avoid confusion. - The
actions/checkoutreference is set to@v6, which does not currently exist; align this with a valid, pinned major version (e.g.@v4). - Consider pinning
Jakuje/check-ansible-actionto a specific tagged version or commit instead of@mainto make the workflow more stable and predictable.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `runs-on: ubuntu-latest` line appears twice in the `test` job; drop the duplicate to avoid confusion.
- The `actions/checkout` reference is set to `@v6`, which does not currently exist; align this with a valid, pinned major version (e.g. `@v4`).
- Consider pinning `Jakuje/check-ansible-action` to a specific tagged version or commit instead of `@main` to make the workflow more stable and predictable.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Fixed!
It indeed exists! https://github.com/actions/checkout/releases/tag/v6
Its still quite fast evolving so once it will stabilize a bit, I will pin it. |
Jakuje
force-pushed
the
ci-general
branch
8 times, most recently
from
f2285cd to
a47d000
Compare
|
Hi @Jakuje , I am testing a new ai pr reviewer tool on these changes in spetrosi#2, comment 2 about openssh-client sounds reasonable but might be a hallucination, and comment 3 sound reasonable too. |
|
Thank you!
I am ok to do that, but over last days, I was pushing some more workarounds and fixes to the action so I would prefer, at least for the beginning, to keep it on
I think this is hallucination/misunderstanding of what the "code" is doing.
This one is right. I will fix it. |
As check-ansible-action is your action it's fine to use main |
|
@richm any chance to get this reviewed? |
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
📝 WalkthroughWalkthroughThis PR extends the SSH role with two new SSH configuration directives (RefuseConnection and WarnWeakCrypto), adds Alpine Linux and Ubuntu 26 platform support with appropriate defaults and test adjustments, and consolidates OS-specific CI workflows into a single matrix-based approach for more maintainable testing. ChangesSSH Role Feature and Platform Expansion
🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ansible-check.yml:
- Around line 30-31: The checkout step using actions/checkout@v6 currently lacks
the persist-credentials setting; update the GitHub Actions step that references
actions/checkout@v6 to include a with block setting persist-credentials: false
(i.e., add "with: persist-credentials: false" under the uses:
actions/checkout@v6 step) so credentials are not persisted to subsequent steps.
- Around line 31-34: Update the workflow to pin third-party actions to immutable
commit SHAs: replace the floating tags "actions/checkout@v6" and
"Jakuje/check-ansible-action@main" with their respective full commit SHAs (e.g.,
actions/checkout@<sha> and Jakuje/check-ansible-action@<sha>) so the runner uses
exact commits; locate the two `uses:` lines in the ansible-check.yml file (the
entries referencing actions/checkout and Jakuje/check-ansible-action) and swap
the tags for the corresponding verified commit SHAs from each repository.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 4ebeda0b-9a28-4d67-a526-d8389424ad85
📒 Files selected for processing (13)
.dev-tools/options_body.github/workflows/ansible-centos.yml.github/workflows/ansible-check.yml.github/workflows/ansible-debian.yml.github/workflows/ansible-fedora.yml.github/workflows/ansible-ubuntu.ymltemplates/ssh_config.j2tests/tasks/setup.ymltests/tests_additional_packages.ymltests/tests_all_options.ymltests/tests_no_skip_defaults.ymlvars/Alpine.ymlvars/Ubuntu_26.yml
💤 Files with no reviewable changes (4)
- .github/workflows/ansible-ubuntu.yml
- .github/workflows/ansible-debian.yml
- .github/workflows/ansible-centos.yml
- .github/workflows/ansible-fedora.yml
| - name: Checkout code | ||
| uses: actions/checkout@v6 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify checkout steps explicitly disable credential persistence
rg -nP --type=yaml 'uses:\s*actions/checkout@' .github/workflows -C2
rg -nP --type=yaml 'persist-credentials:\s*false' .github/workflowsRepository: linux-system-roles/ssh
Length of output: 5617
Add with: persist-credentials: false to the actions/checkout@v6 step in ansible-check.yml.
.github/workflows/ansible-check.yml’s checkout step only has uses: actions/checkout@v6 (no with:), and there are no persist-credentials: false occurrences in .github/workflows, so credentials can persist into later steps/actions.
🧰 Tools
🪛 zizmor (1.25.2)
[warning] 30-31: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 31-31: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/ansible-check.yml around lines 30 - 31, The checkout step
using actions/checkout@v6 currently lacks the persist-credentials setting;
update the GitHub Actions step that references actions/checkout@v6 to include a
with block setting persist-credentials: false (i.e., add "with:
persist-credentials: false" under the uses: actions/checkout@v6 step) so
credentials are not persisted to subsequent steps.
There was a problem hiding this comment.
The .github/workflow/*.yml files are generated from the common source in https://github.com/linux-system-roles/.github/tree/main/playbooks/templates/.github/workflows
If there are changes to be made, please create a github issue in that repo with the suggested issues and fixes.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
gh issue create \
--repo linux-system-roles/.github \
--title "ci: Add persist-credentials: false and pin actions/checkout to a hash in workflow templates" \
--body "## Summary
The \`.github/workflows/\` YAML files generated from the templates in this repository have two security-related findings flagged by [zizmor](https://github.com/woodruffw/zizmor) in [linux-system-roles/ssh#239](https://github.com/linux-system-roles/ssh/pull/239).
These findings apply to the template source used to generate the workflow files, so fixes should be made here.
---
## Finding 1 — Credential persistence (artipacked)
**Severity:** Warning
**Rule:** \`artipacked\`
The \`actions/checkout\` step does not set \`persist-credentials: false\`. This means GitHub credentials (the \`GITHUB_TOKEN\`) are written to the local git config and remain available to all subsequent steps and any action invoked after checkout, which is a wider-than-necessary credential scope.
**Suggested fix:**
\`\`\`yaml
- name: Checkout code
uses: actions/checkout@<hash>
with:
persist-credentials: false
\`\`\`
---
## Finding 2 — Unpinned action reference (unpinned-uses)
**Severity:** Error
**Rule:** \`unpinned-uses\`
\`actions/checkout@v6\` is referenced by a mutable tag rather than an immutable commit hash. A compromised or altered tag could introduce unexpected code into CI.
**Suggested fix:** Pin to the full commit SHA of the desired release, e.g.:
\`\`\`yaml
uses: actions/checkout@<full-sha> # v6
\`\`\`
---
## References
- PR where this was flagged: https://github.com/linux-system-roles/ssh/pull/239#discussion_r3320114000
- Raised by: `@richm`
"Length of output: 1770
@richm, I've created an issue in the upstream template repository with both findings (credential persistence and unpinned action reference):
linux-system-roles/.github#170
✏️ Learnings added
Learnt from: richm
Repo: linux-system-roles/ssh PR: 239
File: .github/workflows/ansible-check.yml:30-31
Timestamp: 2026-05-28T19:23:44.331Z
Learning: In the linux-system-roles/ssh repository, the `.github/workflows/*.yml` files are generated from common template sources located at https://github.com/linux-system-roles/.github/tree/main/playbooks/templates/.github/workflows. Any suggested changes to these workflow files should be filed as issues in the `linux-system-roles/.github` repository rather than directly editing the generated files.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| uses: actions/checkout@v6 | ||
|
|
||
| - name: Test Ansible SSH Role | ||
| uses: Jakuje/check-ansible-action@main |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify all workflow `uses:` refs are SHA-pinned
rg -nP --type=yaml '^\s*uses:\s*[^@]+@(?!(?:[0-9a-fA-F]{40})\b).+$' .github/workflowsRepository: linux-system-roles/ssh
Length of output: 2927
Pin third-party GitHub Actions to immutable commit SHAs in ansible-check.yml
In .github/workflows/ansible-check.yml, uses: actions/checkout@v6 (line 31) and uses: Jakuje/check-ansible-action@main (line 34) are not SHA-pinned, so upstream updates can change CI behavior without notice.
- Pin both
uses:entries to full commit SHAs.
Suggested hardening diff
- - name: Checkout code
- uses: actions/checkout@v6
+ - name: Checkout code
+ uses: actions/checkout@<full-commit-sha-for-v6>
- - name: Test Ansible SSH Role
- uses: Jakuje/check-ansible-action@main
+ - name: Test Ansible SSH Role
+ uses: Jakuje/check-ansible-action@<full-commit-sha>🧰 Tools
🪛 zizmor (1.25.2)
[error] 31-31: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 34-34: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/ansible-check.yml around lines 31 - 34, Update the
workflow to pin third-party actions to immutable commit SHAs: replace the
floating tags "actions/checkout@v6" and "Jakuje/check-ansible-action@main" with
their respective full commit SHAs (e.g., actions/checkout@<sha> and
Jakuje/check-ansible-action@<sha>) so the runner uses exact commits; locate the
two `uses:` lines in the ansible-check.yml file (the entries referencing
actions/checkout and Jakuje/check-ansible-action) and swap the tags for the
corresponding verified commit SHAs from each repository.
There was a problem hiding this comment.
The .github/workflow/*.yml files are generated from the common source in https://github.com/linux-system-roles/.github/tree/main/playbooks/templates/.github/workflows
If there are changes to be made, please create a github issue in that repo with the suggested issues and fixes.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
gh issue create \
--repo linux-system-roles/.github \
--title "Pin GitHub Actions to immutable commit SHAs in workflow templates" \
--body "## Summary
The workflow templates under \`playbooks/templates/.github/workflows/\` (which generate \`.github/workflows/ansible-check.yml\` and related files in downstream roles such as [linux-system-roles/ssh#239](https://github.com/linux-system-roles/ssh/pull/239)) currently reference GitHub Actions using floating tags rather than immutable commit SHAs.
## Examples flagged
\`\`\`yaml
uses: actions/checkout@v6
uses: Jakuje/check-ansible-action@main
\`\`\`
## Risk
Floating tags (e.g., \`@v6\`, \`@main\`) can be moved upstream at any time, meaning CI behavior can change — or be hijacked — without any change to this repository. Pinning to full commit SHAs provides a stable, auditable reference.
## Suggested fix
Replace floating tag references with the corresponding full 40-character commit SHAs, for example:
\`\`\`yaml
uses: actions/checkout@<full-sha-for-v6>
uses: Jakuje/check-ansible-action@<full-sha-for-current-main>
\`\`\`
Tools such as [Dependabot](https://docs.github.com/en/code-security/dependabot/working-with-dependabot/keeping-your-actions-up-to-date-with-dependabot) or [zizmor](https://github.com/woodruffw/zizmor) can help automate this.
## References
- Flagged by CodeRabbit static analysis (zizmor rule \`unpinned-uses\`) in [linux-system-roles/ssh#239 (comment)](https://github.com/linux-system-roles/ssh/pull/239#discussion_r3320114005)
- Requested by `@richm`
"Length of output: 1660
@richm, the issue has been filed at https://github.com/linux-system-repos/.github/issues/171 — it covers pinning both actions/checkout@v6 and Jakuje/check-ansible-action@main to full commit SHAs in the upstream workflow templates.
✏️ Learnings added
Learnt from: richm
Repo: linux-system-roles/ssh PR: 239
File: .github/workflows/ansible-check.yml:31-34
Timestamp: 2026-05-28T19:25:11.306Z
Learning: In the linux-system-roles/ssh repository, the `.github/workflows/*.yml` files are generated from upstream templates located at https://github.com/linux-system-roles/.github/tree/main/playbooks/templates/.github/workflows. Any suggested changes to these workflow files should be filed as issues in the `linux-system-roles/.github` repository rather than made directly in the ssh repository.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
|
@HVSharma12 any idea why the |
|
@richm The leap failure is unrelated to the pr, its caused by the test image openssh is newer than openssh-helpers the mirror currently serves, needs downgrade and zypper aborts, this should self heal when the mirror catches up in next runs. A quick fix would be to replace 'openssh-helpers' to 'sshpass' in the additional packages test - https://github.com/linux-system-roles/ssh/blob/main/tests/tests_additional_packages.yml#L12 |
Ok - whatever you think is best - this can wait for the mirror to catch up |
4 participants
Enhancement: Use single GH action to test on container images
Reason: The fleet of GH actions testing against different container images was not maintainable.
Result: On this role side, all code is in one workflow file.
Issue Tracker Tickets (Jira or BZ if any): -
Summary by Sourcery
Consolidate Ansible role CI into a single GitHub Actions workflow that runs tests across multiple container distributions.
CI:
Summary by CodeRabbit
New Features
RefuseConnectionandWarnWeakCryptoSSH configuration options.Tests