-
Notifications
You must be signed in to change notification settings - Fork 84
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
DO NOT MERGE: Generate OCI Image Indexes by default #776
base: main
Are you sure you want to change the base?
Conversation
a56a17a
to
8942590
Compare
Quality Gate passedKudos, no new issues were introduced! 0 New issues |
@@ -341,6 +347,15 @@ spec: | |||
buildah config -a org.opencontainers.image.base.name=${base_image_name} -a org.opencontainers.image.base.digest=${base_image_digest} $container | |||
buildah commit $container $IMAGE | |||
|
|||
IMAGE_INDEX="$IMAGE-index" | |||
if [ -n "${DISABLE_IMAGE_INDEX}" ]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we just make sure the build-image-manifest
task is included in the Pipeline?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would have been another option to achieve this functionality, but it would result in additional tasks in users pipelines. I chose the alternative of always producing Image Indexes while ensuring that the build-image-manifest
task supports the case (I just realized that the naming is inconsistent)...
@lcarva , you mention that Chains should sign and attest the Image Index and the Image Manifest. That isn't going to happen with this PR, but how can we introduce that ability? Would we need to use two tasks always and this PR would change to enabling/disabling that second image generation? |
For signing, make sure the TaskRun emits a result for each ref, e.g. For attestation, we need to either "promote" all those results to Pipeline results, or enable deep inspection in Chains. |
From an implementation perspective of this PR and its desire to be minimally invasive to users' configured PLRs, the preference for me would be for Chains to be able to deep inspect. Is there an issue for that already? |
Not that I know of. |
8942590
to
c55caa0
Compare
In order to better support the mix of single-architecture and multi-architecture container images, we should put OCI Image Manifests within OCI Image Indexes by default. Acknowledging that some use cases might not want an Image Index, it is possible to disable their generation. Signed-off-by: arewm <arewm@users.noreply.github.com>
c55caa0
to
0258dc6
Compare
Quality Gate passedIssues Measures |
While this PR is a small change, we need to effectively support multi-arch test and release before it can be merged. Therefore, work needs to happen for https://issues.redhat.com/browse/KONFLUX-387 first.
In order to better support the mix of single-architecture and multi-architecture container images, we should put OCI Image Manifests within OCI Image Indexes by default.
Acknowledging that some use cases might not want an Image Index, it is possible to disable their generation.
Before you complete this pull request ...
Look for any open pull requests in the repository with the title "e2e-tests update" and
see if there are recent e2e-tests updates that will be applicable to your change.