Skip to content

Conversation

hasanawad94
Copy link
Contributor

Each container gets its own isolated emptyDir volume mounted at "/shp-writeable-home", to remove writes to the container's rootfs. Should be isolated volumes since when step 1 runs as user A but step 2 as user B, there will permission issues if this directory is shared. For Git SSH, it would actually mean that we put a private key on disk which is then unnecessarily visible

Changes

  • Added logic creating and mounting a volume to be used as a home directory for the build containers.
  • Mounted a volume and set HOME env value for source,build, image-processing containers.

Submitter Checklist

  • Includes tests if functionality changed/was added
  • Includes docs if changes are user-facing
  • Set a kind label on this PR
  • Release notes block has been filled in, or marked NONE

See the contributor guide
for details on coding conventions, github and prow interactions, and the code review process.

Release Notes

Mounting a volume to be used as a home directory for `source`,`build`, `image-processing` containers.

@openshift-ci openshift-ci bot added the release-note Label for when a PR has specified a release note label Sep 10, 2025
@pull-request-size pull-request-size bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Sep 10, 2025
Copy link
Contributor

openshift-ci bot commented Sep 10, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

The full list of commands accepted by this bot can be found 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

@hasanawad94 hasanawad94 marked this pull request as draft September 10, 2025 07:34
@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 Sep 10, 2025
@hasanawad94 hasanawad94 marked this pull request as ready for review September 10, 2025 09:19
@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 Sep 10, 2025
@hasanawad94
Copy link
Contributor Author

Part of #1969

@openshift-merge-robot openshift-merge-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Sep 16, 2025
@rxinui
Copy link

rxinui commented Sep 17, 2025

The changes looks clean and simple, it seems that it relies on previous changes (ie. AppendWriteableVolumes(taskSpec, &bundleStep))

Nothing to comment on, but would be better to have a second opinion.
/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Sep 17, 2025
@openshift-ci openshift-ci bot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. and removed do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Sep 24, 2025
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Oct 4, 2025
Copy link
Contributor

openshift-ci bot commented Oct 4, 2025

New changes are detected. LGTM label has been removed.

Each container gets its own isolated emptyDir volume mounted at "/shp-writeable-home", to remove writes to the container's rootfs.
Should be isolated volumes since when step 1 runs as user A but step 2 as user B, there will permission issues if this directory is shared.
For Git SSH, it would actually mean that we put a private key on disk which is then unnecessarily visible

Signed-off-by: Hasan Awad <[email protected]>
@pull-request-size pull-request-size bot removed the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Oct 5, 2025
@pull-request-size pull-request-size bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Oct 5, 2025
@hasanawad94
Copy link
Contributor Author

@adambkaplan @SaschaSchwarze0 is it ok if we merge this ? This is one pr away from us closing #1969

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

Labels

release-note Label for when a PR has specified a release note size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

4 participants