Add use of key for shutdown - #83062
Conversation
|
/pj-rehearse periodic-ci-openshift-kni-eco-ci-cd-main-cnf-ran-ibu-4.20-cnf-ran-ztp-tests |
WalkthroughStep 3 now uses the mounted spoke master SSH key for the seed node poweroff playbook. It stores the key in a temporary mode 600 file, passes the file explicitly to Ansible, and removes the file after execution. ChangesSeed node poweroff
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: shaior The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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
`@ci-operator/step-registry/telcov10n/functional/cnf-ran/ibu-eco-gotests-seed/telcov10n-functional-cnf-ran-ibu-eco-gotests-seed-commands.sh`:
- Around line 152-157: Update the private-key setup block before the cat command
to create a unique temporary file and register an EXIT trap that removes it on
every exit path. Use that temporary-file variable for chmod, ansible-playbook’s
--private-key argument, and the existing explicit cleanup, preserving removal on
successful completion.
🪄 Autofix
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: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 3815c96f-5b11-499c-bb74-5bbd6b9d3773
📒 Files selected for processing (1)
ci-operator/step-registry/telcov10n/functional/cnf-ran/ibu-eco-gotests-seed/telcov10n-functional-cnf-ran-ibu-eco-gotests-seed-commands.sh
| cat "${MOUNTED_SPOKE_INVENTORY}/ansible_ssh_private_key" > /tmp/spoke-master-ssh-key | ||
| chmod 600 /tmp/spoke-master-ssh-key | ||
| ansible-playbook playbooks/ran/ibu-poweroff-seed-spoke.yml \ | ||
| -i "${OCP_DEPLOYMENT_INVENTORY_PATH}/build-inventory.py" | ||
| -i "${OCP_DEPLOYMENT_INVENTORY_PATH}/build-inventory.py" \ | ||
| --private-key=/tmp/spoke-master-ssh-key | ||
| rm -f /tmp/spoke-master-ssh-key |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Clean up the private key on every exit path.
Based on the surrounding file context, set -e is active before this block. A non-zero ansible-playbook or chmod therefore exits before Line 157, leaving the private key in /tmp. (github.com)
Create a unique temporary file and register an EXIT trap before copying the key. Keep the explicit removal for the successful path.
Proposed fix
- cat "${MOUNTED_SPOKE_INVENTORY}/ansible_ssh_private_key" > /tmp/spoke-master-ssh-key
- chmod 600 /tmp/spoke-master-ssh-key
+ spoke_master_ssh_key="$(mktemp)"
+ trap 'rm -f -- "$spoke_master_ssh_key"' EXIT
+ chmod 600 "$spoke_master_ssh_key"
+ cat "${MOUNTED_SPOKE_INVENTORY}/ansible_ssh_private_key" > "$spoke_master_ssh_key"
ansible-playbook playbooks/ran/ibu-poweroff-seed-spoke.yml \
-i "${OCP_DEPLOYMENT_INVENTORY_PATH}/build-inventory.py" \
- --private-key=/tmp/spoke-master-ssh-key
- rm -f /tmp/spoke-master-ssh-key
+ --private-key="$spoke_master_ssh_key"
+ rm -f -- "$spoke_master_ssh_key"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| cat "${MOUNTED_SPOKE_INVENTORY}/ansible_ssh_private_key" > /tmp/spoke-master-ssh-key | |
| chmod 600 /tmp/spoke-master-ssh-key | |
| ansible-playbook playbooks/ran/ibu-poweroff-seed-spoke.yml \ | |
| -i "${OCP_DEPLOYMENT_INVENTORY_PATH}/build-inventory.py" | |
| -i "${OCP_DEPLOYMENT_INVENTORY_PATH}/build-inventory.py" \ | |
| --private-key=/tmp/spoke-master-ssh-key | |
| rm -f /tmp/spoke-master-ssh-key | |
| spoke_master_ssh_key="$(mktemp)" | |
| trap 'rm -f -- "$spoke_master_ssh_key"' EXIT | |
| chmod 600 "$spoke_master_ssh_key" | |
| cat "${MOUNTED_SPOKE_INVENTORY}/ansible_ssh_private_key" > "$spoke_master_ssh_key" | |
| ansible-playbook playbooks/ran/ibu-poweroff-seed-spoke.yml \ | |
| -i "${OCP_DEPLOYMENT_INVENTORY_PATH}/build-inventory.py" \ | |
| --private-key="$spoke_master_ssh_key" | |
| rm -f -- "$spoke_master_ssh_key" |
🧰 Tools
🪛 ast-grep (0.45.0)
[warning] 152-152: Writing to or reading from a hardcoded, predictable path under /tmp is vulnerable to symlink and TOCTOU attacks: a local attacker can pre-create the file (or a symlink pointing elsewhere) and hijack or corrupt the contents. Generate a unique, unpredictable temporary file with mktemp instead, e.g. tmpfile="$(mktemp)" (or mktemp -d for directories) and reference "$tmpfile".
Context: /tmp/spoke-master-ssh-key
Note: [CWE-377] Insecure Temporary File.
(predictable-tmp-file-bash)
[warning] 156-156: Writing to or reading from a hardcoded, predictable path under /tmp is vulnerable to symlink and TOCTOU attacks: a local attacker can pre-create the file (or a symlink pointing elsewhere) and hijack or corrupt the contents. Generate a unique, unpredictable temporary file with mktemp instead, e.g. tmpfile="$(mktemp)" (or mktemp -d for directories) and reference "$tmpfile".
Context: /tmp/spoke-master-ssh-key
Note: [CWE-377] Insecure Temporary File.
(predictable-tmp-file-bash)
🤖 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
`@ci-operator/step-registry/telcov10n/functional/cnf-ran/ibu-eco-gotests-seed/telcov10n-functional-cnf-ran-ibu-eco-gotests-seed-commands.sh`
around lines 152 - 157, Update the private-key setup block before the cat
command to create a unique temporary file and register an EXIT trap that removes
it on every exit path. Use that temporary-file variable for chmod,
ansible-playbook’s --private-key argument, and the existing explicit cleanup,
preserving removal on successful completion.
Source: MCP tools
|
@shaior: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
[REHEARSALNOTIFIER]
Prior to this PR being merged, you will need to either run and acknowledge or opt to skip these rehearsals. Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
1 participant
Summary by CodeRabbit
The telcov10n CNF-RAN IBU ECO GoTests seed workflow now uses the SSH key from the mounted spoke inventory when it powers off the seed spoke. The workflow creates a temporary key file with restricted permissions, passes it to the Ansible playbook, and removes it after execution.