Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RHOAIENG-16517: chore(tests): add sandboxing so that Dockerfile builds can only access files we know they access #803

Merged
merged 1 commit into from
Dec 11, 2024

Conversation

jiridanek
Copy link
Member

@jiridanek jiridanek commented Dec 11, 2024

https://issues.redhat.com/browse/RHOAIENG-16517

Description

This(will be) important for OCP-CI and for Konflux, because there it is not possible to run the changed files detection up front, but instead the rules must be written into a .tekton directory (konflux) or in openshift/release (ocp-ci).

We must keep track of what files go into each image build, otherwise we cannot determine affected subset of images when testing a PR and would have to build everything.

How Has This Been Tested?

Merge criteria:

  • The commits are squashed in a cohesive manner and have meaningful messages.
  • Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
  • The developer has manually tested the changes and verified that the changes work

@jiridanek jiridanek self-assigned this Dec 11, 2024
@openshift-ci openshift-ci bot requested review from atheo89 and jstourac December 11, 2024 11:24
@jiridanek jiridanek added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Dec 11, 2024
@jiridanek jiridanek force-pushed the jd_sandbox branch 2 times, most recently from 67f8357 to 6be9c2d Compare December 11, 2024 12:26
@jstourac
Copy link
Member

Okay, so basically - if our script bin/buildinputs if faulty, this will fail... we should be aware of this as this may cause us trouble in some cases. But effectively, we probably want to be aware of that, so good. It's just that this brings another step in our build workflow that can break somehow... but yeah, it's a good approach in general, I guess. Only workaround would be to make this configurable somehow (enabled by default) in cases we want to workaround something. What do you think?

/lgtm

@jiridanek
Copy link
Member Author

/approve
this also increases disk space on /, so we want this sooner rather than later

Copy link
Contributor

openshift-ci bot commented Dec 11, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jiridanek

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

@jiridanek
Copy link
Member Author

/override ci/prow/images

Copy link
Contributor

openshift-ci bot commented Dec 11, 2024

@jiridanek: Overrode contexts on behalf of jiridanek: ci/prow/images

In response to this:

/override ci/prow/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.

@jiridanek
Copy link
Member Author

@jstourac I like the override idea

@openshift-merge-bot openshift-merge-bot bot merged commit 33f95dd into opendatahub-io:main Dec 11, 2024
7 checks passed
@jiridanek jiridanek deleted the jd_sandbox branch December 12, 2024 10:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved lgtm tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants