Bug 1686589
| Summary: | Cincinnati should return digests, not tags in the graph | ||
|---|---|---|---|
| Product: | OpenShift Container Platform | Reporter: | Clayton Coleman <ccoleman> |
| Component: | Cluster Version Operator | Assignee: | Stefan Junker <sjunker> |
| Status: | CLOSED CURRENTRELEASE | QA Contact: | liujia <jiajliu> |
| Severity: | high | Docs Contact: | |
| Priority: | unspecified | ||
| Version: | 4.1.0 | CC: | agarcial, aos-bugs, aos-cloud, bleanhar, dgoodwin, erich, evb, jhou, jokerman, mmccomas, paji, sjunker |
| Target Milestone: | --- | ||
| Target Release: | 4.3.0 | ||
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
| Whiteboard: | |||
| Fixed In Version: | Doc Type: | Enhancement | |
| Doc Text: |
Feature: Deterministic payload references
Reason: Payload references aren't deterministic. Applying the same release of the same graph at different points in time might yield different results.
Result: Payload references contain the SHA of the image the release was found at, if the container registry provides the manifestref.
|
Story Points: | --- |
| Clone Of: | 1685225 | Environment: | |
| Last Closed: | 2019-11-08 16:26:05 UTC | Type: | --- |
| Regression: | --- | Mount Type: | --- |
| Documentation: | --- | CRM: | |
| Verified Versions: | Category: | --- | |
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
| Cloudforms Team: | --- | Target Upstream Version: | |
| Embargoed: | |||
In a recent version of Cincinnati a node in the graph looks as follows:
...
{
"version": "4.0.0-0.5",
"payload": "quay.io/openshift-release-dev/ocp-release:4.0.0-0.5",
"metadata": {
"com.openshift.upgrades.graph.release.manifestref": "sha256:6bfad9b32e8f4f94ba08424db4242da03b4dd14c7a65e216d69f5f13508882cd"
}
},
...
Is that fine or do you need the information in "payload"?
The requested change is currently waiting for a review at https://github.com/openshift/cincinnati/pull/75. For a discussion in the review process I would like to get input from the reporter. Is it acceptable that in environments where the container registry does not provide the manifestref to Cincinnati it will continue to return tag instead of digest? I have encountered Satellite to not provide the manifestref during integration experiments and have therefore made that codepath depend on the availability. So in most cases https://github.com/openshift/cincinnati/pull/75 will deliver the digest, but in some it might not. The alternative to this would be to make this a requirement towards container registries and treat it as an error if it's failed to be provided. https://github.com/openshift/cincinnati/pull/75 has been merged and is running at staging. The result from there contains: ... { "version": "4.0.0-5", "payload": "quay.io/openshift-release-dev/ocp-release@sha256:ed45d79377a8f36aa5db7be63c1278ae864031ec3d50b80fdd9902c6be4517ba", "metadata": { "com.openshift.upgrades.graph.release.manifestref": "sha256:ed45d79377a8f36aa5db7be63c1278ae864031ec3d50b80fdd9902c6be4517ba" } ... Clayton, would you be able to answer Stefan's question from Comment #1? Just adding a note to Brenton's request. The question in comment 1 is obsolete with regards to the api.openshift.com instance of Cincinnati, but it might still be valid for other environments. Therefore please consider comment 3 when answering. Supporting SHAs should be a requirement of 4.x in general - I'd like to dig in why Satellite is not returning that. We need the payload pointing to the SHA I requested feedback on this from the Satellite team there will be a meeting on this tomorrow. Specifically requesting info from Partha for this ticket, thanks! It looks like we're not going to have a quick fix for this in Satellite. In order to remain compatible with Satellite for test setups, and keeping in mind that this bug is already fixed for the api.openshift.com deployments, the two options are: 1. Introduce a flag to Cincinnati which controls if the digest is required from the registry, which would default to 'true' and would need to be set to 'false' for Satellite test setups 2. Move this ticket to 4.2 and wait it out Choosing option (2) I'm assuming this is not a blocker or it would have been resolved long ago. Stefan, would you mind providing an update here? After digging for upstream changes and repeating the experiments with Satellite I can confirm that the digests are now reported to Cincinnati. For those interested, this is the relevant upstream change: https://github.com/Katello/katello/pull/8070/files . There are still a few issues to resolve to make Satellite fully compatible but they are unrelated to this bug. The last step I see is to enforce getting the digest from the registry in Cincinnati, and treat the absence as a hard error. The PR which requires the manifest digest sent by the registry is online: https://github.com/openshift/cincinnati/pull/158 This has by now landed [1] in Cincinnati production. [1]: https://gitlab.cee.redhat.com/service/saas-cincinnati/merge_requests/8 |
In order to guarantee reproducibility, Cincinnati should return digests instead of tags: { "nodes":[ { "version":"4.0.0-5", "payload":"quay.io/openshift-release-dev/ocp-release:4.0.0-5", "metadata":{} }, ... Payload should be quay.io/openshift-release-dev/ocp-release@sha256:xxxxxxxx Prevents attackers from seizing control of our tags and redirecting to a bad release image, and also prevents lack of clarity about what version a cluster is going to use.