Skip to content

Conversation

@matlaj
Copy link
Contributor

@matlaj matlaj commented Dec 11, 2025

What this PR does / why we need it:

Since CPOv2, Portieris will crash if it can't write to ./trust. This fix mounts an empty dir to this path to allow Portieris to write its files inside its ReadOnlyRootFileSystem container.

Which issue(s) this PR fixes:

Fixes OCPBUGS-67224

Special notes for your reviewer:

None

Checklist:

  • Subject and description added to both, commit and PR.
  • Relevant issues have been referenced.
  • This change includes docs.
  • This change includes unit tests.

@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 11, 2025
@openshift-ci-robot openshift-ci-robot added jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. labels Dec 11, 2025
@openshift-ci-robot
Copy link

@matlaj: This pull request references Jira Issue OCPBUGS-67224, which is valid. The bug has been moved to the POST state.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target version (4.21.0) matches configured target version for branch (4.21.0)
  • bug is in the state New, which is one of the valid states (NEW, ASSIGNED, POST)

The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

What this PR does / why we need it:

Since CPOv2, Portieris will crash if it can't write to ./trust. This fix mounts an empty dir to this path to allow Portieris to write its files inside its ReadOnlyRootFileSystem container.

Which issue(s) this PR fixes:

Fixes OCPBUGS-67224

Special notes for your reviewer:

Checklist:

  • Subject and description added to both, commit and PR.
  • Relevant issues have been referenced.
  • This change includes docs.
  • This change includes unit tests.

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 openshift-eng/jira-lifecycle-plugin repository.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 11, 2025

Walkthrough

Modifies the KAS Portieris controller to add an in-memory trust data volume. Introduces a new EmptyDir volume mounted at /run/.trust in the Portieris container and adds a corresponding volume-building function to the Pod specification.

Changes

Cohort / File(s) Summary
Portieris trust volume configuration
control-plane-operator/controllers/hostedcontrolplane/v2/kas/portieries.go
Adds in-memory trust data volume (portieris-trust-data) with EmptyDir storage. Introduces buildKASPortierisTrustVolume function and extends volume mounts map to include new volume mounted at /run/.trust for Portieris container.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Verify that the EmptyDir configuration is correctly specified for ephemeral trust storage
  • Confirm the volume mount path /run/.trust aligns with Portieris expectations and does not conflict with existing mount points
  • Validate that the volume is properly integrated into the Pod spec and volume mounts map
✨ Finishing touches
  • 📝 Generate docstrings
🧪 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

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

📥 Commits

Reviewing files that changed from the base of the PR and between 8e41a55 and 73d2bd5.

📒 Files selected for processing (1)
  • control-plane-operator/controllers/hostedcontrolplane/v2/kas/portieries.go (2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**

⚙️ CodeRabbit configuration file

-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.

Files:

  • control-plane-operator/controllers/hostedcontrolplane/v2/kas/portieries.go
🔇 Additional comments (4)
control-plane-operator/controllers/hostedcontrolplane/v2/kas/portieries.go (4)

18-18: LGTM!

Constant naming follows established conventions and clearly identifies the trust data volume.


33-33: LGTM!

Volume configuration correctly appends both required volumes to the Pod specification.


91-101: LGTM!

The EmptyDir volume with memory-backed storage is the appropriate solution for providing writable space in a read-only root filesystem container. The implementation follows established patterns.


26-26: Verify that the mount path matches Portieris's actual trust path expectations.

The code mounts a volume at /run/.trust (absolute path). However, without access to the PR description and Portieris's implementation, the review comment's concern cannot be resolved: does Portieris write to /run/.trust directly, or does it expect a working directory set to /run and write to ./trust relative to it? The container spec has no explicit WorkingDir configured (lines 36-78).


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

@matlaj
Copy link
Contributor Author

matlaj commented Dec 11, 2025

/auto-cc

@openshift-ci openshift-ci bot added area/control-plane-operator Indicates the PR includes changes for the control plane operator - in an OCP release needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. and removed do-not-merge/needs-area labels Dec 11, 2025
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Dec 11, 2025

Hi @matlaj. Thanks for your PR.

I'm waiting for a github.com member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

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.

@openshift-ci openshift-ci bot requested review from bryan-cox and sjenning December 11, 2025 15:09
@openshift-ci-robot
Copy link

@matlaj: This pull request references Jira Issue OCPBUGS-67224, which is valid.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target version (4.21.0) matches configured target version for branch (4.21.0)
  • bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)
Details

In response to this:

What this PR does / why we need it:

Since CPOv2, Portieris will crash if it can't write to ./trust. This fix mounts an empty dir to this path to allow Portieris to write its files inside its ReadOnlyRootFileSystem container.

Which issue(s) this PR fixes:

Fixes OCPBUGS-67224

Special notes for your reviewer:

None

Checklist:

  • Subject and description added to both, commit and PR.
  • Relevant issues have been referenced.
  • This change includes docs.
  • This change includes unit tests.

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 openshift-eng/jira-lifecycle-plugin repository.

@TwoDCube
Copy link
Member

/ok-to-test

@openshift-ci openshift-ci bot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Dec 11, 2025
@TwoDCube
Copy link
Member

/cherry-pick release-4.20

@openshift-cherrypick-robot

@TwoDCube: once the present PR merges, I will cherry-pick it on top of release-4.20 in a new PR and assign it to you.

Details

In response to this:

/cherry-pick release-4.20

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.

@TwoDCube
Copy link
Member

/ok-to-test

@TwoDCube
Copy link
Member

/retest

Since CPOv2, Portieris will crash if it can't write to ./trust.
This fix mounts an empty dir to this path to allow Portieris to write its files inside its ReadOnlyRootFileSystem container.
@matlaj matlaj force-pushed the matlaj/portieris_workdir branch from a4f0e80 to 73d2bd5 Compare December 12, 2025 10:14
@TwoDCube
Copy link
Member

/ok-to-test

@TwoDCube
Copy link
Member

/retest

1 similar comment
@TwoDCube
Copy link
Member

/retest

@rtheis
Copy link
Contributor

rtheis commented Dec 15, 2025

/retest-required

1 similar comment
@rtheis
Copy link
Contributor

rtheis commented Dec 16, 2025

/retest-required

@matlaj matlaj marked this pull request as ready for review December 16, 2025 13:22
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 16, 2025
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Dec 18, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: matlaj, rtheis, sjenning

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

The pull request process is described 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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 18, 2025
@rtheis
Copy link
Contributor

rtheis commented Dec 19, 2025

/ok-to-test

@rtheis
Copy link
Contributor

rtheis commented Dec 19, 2025

/cherry-pick release-4.21 release-4.20

@openshift-cherrypick-robot

@rtheis: once the present PR merges, I will cherry-pick it on top of release-4.21 in a new PR and assign it to you.

Details

In response to this:

/cherry-pick release-4.21 release-4.20

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.

@rtheis
Copy link
Contributor

rtheis commented Dec 19, 2025

/retest-required

1 similar comment
@rtheis
Copy link
Contributor

rtheis commented Dec 19, 2025

/retest-required

@openshift-ci-robot
Copy link

/retest-required

Remaining retests: 0 against base HEAD 56af5d5 and 2 for PR HEAD 73d2bd5 in total

@rtheis
Copy link
Contributor

rtheis commented Dec 19, 2025

/retest-required

@openshift-ci-robot
Copy link

/retest-required

Remaining retests: 0 against base HEAD 780f88b and 1 for PR HEAD 73d2bd5 in total

@rtheis
Copy link
Contributor

rtheis commented Dec 19, 2025

/retest-required

1 similar comment
@rtheis
Copy link
Contributor

rtheis commented Dec 19, 2025

/retest-required

@rtheis
Copy link
Contributor

rtheis commented Dec 20, 2025

/override ci/prow/okd-scos-images

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Dec 20, 2025

@rtheis: rtheis unauthorized: /override is restricted to Repo administrators, approvers in top level OWNERS file, and the following github teams:openshift: openshift-release-oversight openshift-staff-engineers openshift-sustaining-engineers.

Details

In response to this:

/override ci/prow/okd-scos-images

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.

@openshift-ci-robot
Copy link

/retest-required

Remaining retests: 0 against base HEAD 351f6ae and 0 for PR HEAD 73d2bd5 in total

@openshift-ci-robot
Copy link

/hold

Revision 73d2bd5 was retested 3 times: holding

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 22, 2025
@rtheis
Copy link
Contributor

rtheis commented Dec 22, 2025

/ok-to-test
/remove-hold

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 22, 2025
@openshift-ci-robot
Copy link

/retest-required

Remaining retests: 0 against base HEAD 351f6ae and 2 for PR HEAD 73d2bd5 in total

@openshift-ci-robot
Copy link

/retest-required

Remaining retests: 0 against base HEAD da2f23d and 1 for PR HEAD 73d2bd5 in total

@TwoDCube
Copy link
Member

/retest-required

1 similar comment
@TwoDCube
Copy link
Member

/retest-required

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Dec 24, 2025

@matlaj: all tests passed!

Full PR test history. Your PR dashboard.

Details

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.

@openshift-merge-bot openshift-merge-bot bot merged commit 9c9bda3 into openshift:main Dec 24, 2025
22 checks passed
@openshift-ci-robot
Copy link

@matlaj: Jira Issue Verification Checks: Jira Issue OCPBUGS-67224
✔️ This pull request was pre-merge verified.
✔️ All associated pull requests have merged.
✔️ All associated, merged pull requests were pre-merge verified.

Jira Issue OCPBUGS-67224 has been moved to the MODIFIED state and will move to the VERIFIED state when the change is available in an accepted nightly payload. 🕓

Details

In response to this:

What this PR does / why we need it:

Since CPOv2, Portieris will crash if it can't write to ./trust. This fix mounts an empty dir to this path to allow Portieris to write its files inside its ReadOnlyRootFileSystem container.

Which issue(s) this PR fixes:

Fixes OCPBUGS-67224

Special notes for your reviewer:

None

Checklist:

  • Subject and description added to both, commit and PR.
  • Relevant issues have been referenced.
  • This change includes docs.
  • This change includes unit tests.

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 openshift-eng/jira-lifecycle-plugin repository.

@openshift-cherrypick-robot

@TwoDCube: new pull request created: #7421

Details

In response to this:

/cherry-pick release-4.20

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.

@openshift-cherrypick-robot

@rtheis: new pull request created: #7422

Details

In response to this:

/cherry-pick release-4.21 release-4.20

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.

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

Labels

acknowledge-critical-fixes-only Indicates if the issuer of the label is OK with the policy. approved Indicates a PR has been approved by an approver from all required OWNERS files. area/control-plane-operator Indicates the PR includes changes for the control plane operator - in an OCP release jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. verified Signifies that the PR passed pre-merge verification criteria

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants