Skip to content

Conversation

@lilyLuLiu
Copy link
Contributor

@lilyLuLiu lilyLuLiu commented Oct 17, 2025

Description

Fixes:
https://github.com/crc-org/crc/actions/runs/18542762776/job/52896312693#step:6:143
Error message when provisioning Windows with mapt: open /workspace/adminusername: permission denied

Relates to: redhat-developer/mapt#587

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change
  • Chore (non-breaking change which doesn't affect codebase;
    test, version modification, documentation, etc.)

Proposed changes

Testing

Contribution Checklist

  • I Keep It Small and Simple: The smaller the PR is, the easier it is to review and have it merged
  • I have performed a self-review of my code
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Which platform have you tested the code changes on?
    • Linux
    • Windows
    • MacOS

Summary by CodeRabbit

  • Chores
    • Updated workflow configuration to enhance workspace permissions handling during instance creation processes.

@openshift-ci openshift-ci bot requested review from albfan and cfergeau October 17, 2025 02:08
@coderabbitai
Copy link

coderabbitai bot commented Oct 17, 2025

Walkthrough

A GitHub Actions workflow is modified to add a permission change step that recursively sets workspace permissions to 777 before executing a podman run command for Windows instance creation, ensuring subsequent steps have adequate write access to the workspace directory.

Changes

Cohort / File(s) Summary
GitHub Actions Workflow Permission Fix
\.github/workflows/windows-qe-tpl.yml
Adds sudo chmod -R 777 $GITHUB_WORKSPACE step before the podman run command to recursively set workspace permissions, ensuring the workspace is writable for subsequent steps in the Windows instance creation workflow.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Suggested reviewers

  • gbraad
  • cfergeau

Poem

🐰 A hop through permissions, so grand,
Rights set at 777 across the land,
Before the container springs to life,
No more access strife,
The workflow runs smooth, just as planned! 🔐

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The pull request description is largely incomplete and does not adequately follow the template structure. While the author has provided a description section header and identified the problem (permission denied error when provisioning Windows with mapt) along with a related upstream issue link, several critical sections are missing content. The "Type of change" section has no checkbox selected, the "Proposed changes" section is entirely empty without listing the modifications made, and the "Testing" section lacks any testing description or pseudo-code. Additionally, the description does not include a plain English explanation of how the permission issue was solved, which is essential information per the template guidelines. The author should complete the pull request description by: selecting an appropriate checkbox under "Type of change" (this appears to be a bug fix based on context), providing a detailed list of proposed changes (such as adding the sudo chmod command and where it was added), including a testing section with specific steps or assertions to verify the permission fix works, and adding a clear plain-English explanation of how the fix addresses the permission denied error. Ideally, the "Fixes:" field should reference a specific issue number in the format "Fixes: #N" rather than just a CI run URL.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title "[QE]fix permission issue of provision windows with mapt" directly relates to the main change in the pull request, which adds a permission-setting step to the Windows instance workflow. The title is specific and clear—it indicates this is a permission-related bug fix for Windows provisioning with mapt, which someone reviewing the history would immediately understand. The title is appropriately concise at eight words and accurately summarizes the primary intent of the change.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c73186a and d60f41e.

📒 Files selected for processing (1)
  • .github/workflows/windows-qe-tpl.yml (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (18)
  • GitHub Check: build-qe (linux, arm64)
  • GitHub Check: build-qe (linux, amd64)
  • GitHub Check: build-qe (darwin, amd64)
  • GitHub Check: build-qe (windows, amd64)
  • GitHub Check: build-qe (darwin, arm64)
  • GitHub Check: build-installer (windows-2022, 1.24)
  • GitHub Check: verify-devcontainer
  • GitHub Check: build (ubuntu-latest, 1.24)
  • GitHub Check: build (macOS-13, 1.24)
  • GitHub Check: build (macOS-14, 1.24)
  • GitHub Check: build (ubuntu-22.04, 1.24)
  • GitHub Check: build (macOS-14, 1.24)
  • GitHub Check: build (macOS-13, 1.24)
  • GitHub Check: build (ubuntu-latest, 1.24)
  • GitHub Check: build (windows-2022, 1.24)
  • GitHub Check: Run OKD bundle with crc (1.24)
  • GitHub Check: build (windows-2022, 1.24)
  • GitHub Check: Konflux kflux-prd-rh02 / crc-binary-on-pull-request
🔇 Additional comments (1)
.github/workflows/windows-qe-tpl.yml (1)

91-114: Verify the upstream issue context and consider targeted permission fixes.

The review comment's security concerns are valid: sudo chmod -R 777 $GITHUB_WORKSPACE (line 93) applies blanket world-writable permissions to the entire workspace, which violates the principle of least privilege. Analysis shows this is an outlier—other uses of chmod in the codebase are targeted (e.g., chmod +x on specific binaries), not blanket directory permissions.

However, verification of the upstream recommendation is blocked:

  • The cited redhat-developer/mapt issue Fix make lint issues on Windows #587 could not be accessed publicly
  • Cannot confirm whether 777 is the official mapt maintainers' recommendation or a temporary workaround
  • Cannot determine if alternative solutions (container user mapping, SELinux context tuning, or umask settings) exist

Recommended next steps:

  1. Confirm the upstream issue URL and verify the 777 recommendation directly with mapt maintainers
  2. If 777 is a temporary workaround, document the planned remediation and upstream fix tracking
  3. Consider scoped alternatives: apply chmod only to specific output subdirectories rather than the entire workspace

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.

❤️ Share

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

@openshift-ci
Copy link

openshift-ci bot commented Oct 17, 2025

@lilyLuLiu: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/security d60f41e link false /test security
ci/prow/e2e-crc d60f41e link true /test e2e-crc

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

@lilyLuLiu lilyLuLiu self-assigned this Oct 23, 2025
@openshift-ci openshift-ci bot added the lgtm label Oct 23, 2025
@anjannath anjannath merged commit be1f29e into crc-org:main Oct 23, 2025
28 of 38 checks passed
@openshift-ci
Copy link

openshift-ci bot commented Oct 23, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: anjannath

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

The pull request process is described here

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

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants