fix: missing Oci-Subject header pushing index with subject #1586
Conversation
jdolitsky
requested review from
eusebiu-constantin-petu-dbk,
rchincha and
andaaron
as code owners
|
@jdolitsky some sanity checks in CI "fix: missing OCI-Subject header ..." Also pls sign your commit. |
jdolitsky
force-pushed
the
fix-index-header
branch
from
3d07c6c to
18f9c3f
Compare
jdolitsky
changed the title
|
related: opencontainers/distribution-spec#436 |
Codecov Report
@@ Coverage Diff @@
## main #1586 +/- ##
==========================================
- Coverage 91.29% 91.26% -0.03%
==========================================
Files 117 117
Lines 24293 24315 +22
==========================================
+ Hits 22179 22192 +13
- Misses 1576 1582 +6
- Partials 538 541 +3
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
There was a problem hiding this comment.
Merge this as-is or wait on opencontainers/distribution-spec#436 to be merged first and change the branch?
|
Unfortunately opencontainers/distribution-spec#436 will not pass CI/CD until there is a new zot release available (zot container image). My vote would be to get this change into an RC, then we update the OCI PR to point to new image cc @sudo-bmitch |
|
I can also drop the changes here related to conformance and you all can add them back later. Just wanted a way to test this change |
Also, run OCI conformance tests from HEAD (vs. 1.0.0) Signed-off-by: Josh Dolitsky <[email protected]>
jdolitsky
force-pushed
the
fix-index-header
branch
from
3ccc90c to
9c105ee
Compare
I did this in #1589 just in case |
|
|
||
| err := json.Unmarshal(body, &index) | ||
| if err != nil { | ||
| return "", "", err |
There was a problem hiding this comment.
Note this line cannot be covered by tests because the blob is already unmarshalled/validated at line 374.
|
|
||
| err := json.Unmarshal(body, &index) | ||
| if err != nil { | ||
| return "", "", err |
There was a problem hiding this comment.
Note this line cannot be covered by tests because the blob is already unmarshalled/validated at line 463
| } | ||
|
|
||
| if index.Subject != nil { | ||
| subjectDigest = index.Subject.Digest |
There was a problem hiding this comment.
I'm working on a test to cover this.
|
@andaaron - feel free to push over my changes here |
|
I updated https://github.com/project-zot/zot/pull/1589/files. This PR would conflict with #1559 on enabling the conformance code. |
|
Closed in favor of #1589 |
Also, run OCI conformance tests from HEAD (vs. 1.0.0)
What type of PR is this?
Which issue does this PR fix:
What does this PR do / Why do we need it:
If an issue # is not available please add repro steps and logs showing the issue:
Testing done on this change:
Automation added to e2e:
Will this break upgrades or downgrades?
Does this PR introduce any user-facing change?:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.