containers/image: remove signatures when copying in to OCI layout #1664
Conversation
This does not affect signature validation, and we do not need to preserve signatures _after_ validation because we will never need to propagate those signatures to another image transport/destination. Signed-off-by: Joe Lanford <[email protected]>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: joelanford 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 |
openshift-ci
bot
added
the
approved
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1664 +/- ##
==========================================
+ Coverage 55.15% 55.17% +0.01%
==========================================
Files 136 136
Lines 15911 15918 +7
==========================================
+ Hits 8776 8782 +6
- Misses 5982 5983 +1
Partials 1153 1153 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| // so we remove the source signatures when copying. | ||
| // Signature validation will still be performed | ||
| // accordingly to a provided policy context. | ||
| RemoveSignatures: true, |
There was a problem hiding this comment.
Maybe we also want
PreserveDigests: true,It doesn't impact this particular operation, but it also seems a good option which doesn't default on.
I verified that this works with upstream (unsigned) and downstream (signed) example catalogs.
There was a problem hiding this comment.
GoDoc for that field is:
// Preserve digests, and fail if we cannot.
I'm hesitant about the "fail if we cannot" part. We have similar code in operator-controller that does not set this value, so I don't think it is necessary, and likely not for this bug fix specifically.
There was a problem hiding this comment.
I agree that it's not necessary for resolving this bug. But the bug arose because we didn't consider other fields that might've been pertinent when we introduced the new containers/image registry support, so it felt like we should at least discuss any other possible missed configuration.
I think we can agree that mutating digests across our local copy would be an undesirable side effect.
There was a problem hiding this comment.
On second thought, even skopeo has this option as a non-main-path flag, which suggests that there are conditions under which this is desirable, but not in general.
If we wanted this, we should plumb through a controlling flag. Which I don't think we need to do, at this time.
|
/lgtm |
openshift-merge-bot
bot
merged commit eac1504
into
operator-framework:master
The OPM 1.53.0 has problem, once there will be new version with the fix we can bump to latest. operator-framework/operator-registry#1664 Ref: https://redhat-internal.slack.com/archives/C074JM28DTP/p1746458556603619 Signed-off-by: Martin Necas <[email protected]>
The OPM 1.53.0 has problem, once there will be new version with the fix we can bump to latest. operator-framework/operator-registry#1664 Ref: https://redhat-internal.slack.com/archives/C074JM28DTP/p1746458556603619 --------- Signed-off-by: Martin Necas <[email protected]>
The OPM 1.53.0 has problem, once there will be new version with the fix we can bump to latest. operator-framework/operator-registry#1664 Ref: https://redhat-internal.slack.com/archives/C074JM28DTP/p1746458556603619 --------- Signed-off-by: Martin Necas <[email protected]>
The OPM 1.53.0 has problem, once there will be new version with the fix we can bump to latest. operator-framework/operator-registry#1664 Ref: https://redhat-internal.slack.com/archives/C074JM28DTP/p1746458556603619 --------- Signed-off-by: Martin Necas <[email protected]>
This does not affect signature validation, and we do not need to preserve signatures after validation because we will never need to propagate those signatures to another image transport/destination.
Description of the change:
When using the containers/image registry client implementation, remove signatures when copying in to OCI layout
Motivation for the change:
The OCI layout destination does not support copying signatures. This causes a bug (render/migrate fail) when using a policy that requires signature validation.
This change does not affect signature validation, and we do not need to preserve signatures after validation because we will never need to propagate those signatures to another image transport/destination.
Reviewer Checklist
/docs