Skip to content

Fix QEMU permission denied for agent-based installs#86

Open
dhensel-rh wants to merge 1 commit into
openshift-eng:mainfrom
dhensel-rh:fix/agent-acl-path
Open

Fix QEMU permission denied for agent-based installs#86
dhensel-rh wants to merge 1 commit into
openshift-eng:mainfrom
dhensel-rh:fix/agent-acl-path

Conversation

@dhensel-rh

@dhensel-rh dhensel-rh commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Summary

  • The ACL task in create.yml hardcoded /root, but dev-scripts is deployed under /home/ec2-user
  • QEMU needs rx on the actual home directory to traverse the path to the agent ISO
  • Replaced hardcoded /root with {{ ansible_env.HOME }} and {{ dev_scripts_path }}
  • Only affects agent method installs (task has when: method == "agent")
  • IPI installs (fencing-ipi, arbiter-ipi) are unaffected — this task is skipped entirely

Fixes #85

Test plan

  • Deploy cluster with method=agent and verify QEMU starts VMs without permission errors
  • Deploy cluster with method=ipi and verify no regression

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Updated the file access permissions used during agent installation to grant the qemu user the required rx access to the correct home-based directories.
    • This improves reliability by ensuring permissions are applied to the environment-appropriate locations rather than fixed paths, helping avoid install failures caused by insufficient filesystem access.

@openshift-ci openshift-ci Bot requested review from qJkee and slintes June 26, 2026 13:26
@openshift-ci

openshift-ci Bot commented Jun 26, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: dhensel-rh
Once this PR has been reviewed and has the lgtm label, please assign fonta-rh for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@coderabbitai

coderabbitai Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: 21f67ed5-86c6-4824-8a3d-a8cace82b3f3

📥 Commits

Reviewing files that changed from the base of the PR and between ab556ec and 18dd4ae.

📒 Files selected for processing (1)
  • deploy/openshift-clusters/roles/dev-scripts/install-dev/tasks/create.yml
🚧 Files skipped from review as they are similar to previous changes (1)
  • deploy/openshift-clusters/roles/dev-scripts/install-dev/tasks/create.yml

Walkthrough

The agent-install ACL task now grants qemu read/execute access to the user’s home directory and the dev-scripts path instead of . and /root.

Changes

Agent install ACL paths

Layer / File(s) Summary
Update qemu ACL targets
deploy/openshift-clusters/roles/dev-scripts/install-dev/tasks/create.yml
The agent-install ACL loop now uses ansible_env.HOME and ansible_env.HOME/{{ dev_scripts_path }} for qemu rx permissions.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Suggested labels

ready-for-human-review

🚥 Pre-merge checks | ✅ 10 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Ai-Attribution ⚠️ Warning AI use is explicit, but the commit uses Co-Authored-By instead of Red Hat Assisted-by/Generated-by trailers. Replace the AI co-author trailer with a Red Hat-compliant Assisted-by or Generated-by trailer in the commit or PR metadata.
✅ Passed checks (10 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: fixing QEMU permission errors for agent-based installs.
Linked Issues check ✅ Passed The ACL paths were updated from hardcoded /root to the home directory and dev_scripts_path, matching issue #85.
Out of Scope Changes check ✅ Passed The PR only adjusts the QEMU ACL task in create.yml and introduces no unrelated changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
No-Weak-Crypto ✅ Passed The only changed task is an ACL path update; it contains no weak crypto, custom crypto, or secret-comparison logic.
Container-Privileges ✅ Passed The only touched file is an Ansible task; no container/K8s privilege settings like privileged, hostPID, SYS_ADMIN, or allowPrivilegeEscalation are present.
No-Sensitive-Data-In-Logs ✅ Passed The only change is an ACL path update in create.yml; no new log/debug output or sensitive data exposure was introduced.
No-Hardcoded-Secrets ✅ Passed Touched file only changes ACL paths; no secret literals, base64 blobs, or credentialed URLs are present.
No-Injection-Vectors ✅ Passed The changed ACL task only uses Ansible modules and Jinja path vars; it adds no shell, eval, pickle, YAML load, SQL, or unsafe HTML sink.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot added the ready-for-human-review Indicates a PR has been reviewed by automated tools and is ready for human review label Jun 26, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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 `@deploy/openshift-clusters/roles/dev-scripts/install-dev/tasks/create.yml`:
- Around line 26-27: `dev_scripts_path` is being used directly as an ACL target
even though its default value is relative, so the path may not resolve to the
actual directory under `ansible_env.HOME`. Update the ACL setup in `create.yml`
to normalize `dev_scripts_path` before passing it to the ACL task, using the
same resolved path consistently with `ansible_env.HOME` and the
`dev_scripts_path` variable so the QEMU process gets the intended permissions.
🪄 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: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: de414b6e-307d-45a9-8a1b-016a7e2891ad

📥 Commits

Reviewing files that changed from the base of the PR and between 903e071 and ab556ec.

📒 Files selected for processing (1)
  • deploy/openshift-clusters/roles/dev-scripts/install-dev/tasks/create.yml

Comment on lines +26 to +27
- "{{ ansible_env.HOME }}"
- "{{ dev_scripts_path }}"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Normalize dev_scripts_path before using it as an ACL target.

dev_scripts_path is relative by default (openshift-metal3/dev-scripts in defaults/main.yml), so Line 27 still does not point at the real directory under {{ ansible_env.HOME }} unless inventories override it with an absolute path. That means the default agent-install path can still miss the ACL the QEMU process needs.

Suggested fix
   loop:
     - "{{ ansible_env.HOME }}"
-    - "{{ dev_scripts_path }}"
+    - "{{ dev_scripts_path if dev_scripts_path.startswith('/') else ansible_env.HOME ~ '/' ~ dev_scripts_path }}"
📝 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.

Suggested change
- "{{ ansible_env.HOME }}"
- "{{ dev_scripts_path }}"
- "{{ ansible_env.HOME }}"
- "{{ dev_scripts_path if dev_scripts_path.startswith('/') else ansible_env.HOME ~ '/' ~ dev_scripts_path }}"
🤖 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 `@deploy/openshift-clusters/roles/dev-scripts/install-dev/tasks/create.yml`
around lines 26 - 27, `dev_scripts_path` is being used directly as an ACL target
even though its default value is relative, so the path may not resolve to the
actual directory under `ansible_env.HOME`. Update the ACL setup in `create.yml`
to normalize `dev_scripts_path` before passing it to the ACL task, using the
same resolved path consistently with `ansible_env.HOME` and the
`dev_scripts_path` variable so the QEMU process gets the intended permissions.

The ACL task hardcoded /root, but dev-scripts is deployed under
/home/ec2-user. QEMU needs rx on the actual home directory to
traverse the path to the agent ISO.

Fixes openshift-eng#85

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@dhensel-rh dhensel-rh force-pushed the fix/agent-acl-path branch from ab556ec to 18dd4ae Compare June 26, 2026 13:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-human-review Indicates a PR has been reviewed by automated tools and is ready for human review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Agent install fails: QEMU cannot read ISO due to incorrect ACL path

1 participant