fix: oras pull error `empty response Docker-Content-Digest` #237

nima · 2022-07-11T16:28:21Z

See #225 for details, but here's the tldr:

% oras pull 080565210187.dkr.ecr.us-west-2.amazonaws.com/my.repo:my.tag
Error: GET "https://080565210187.dkr.ecr.us-west-2.amazonaws.com/v2/my.repo/manifests/my.tag": empty response Docker-Content-Digest

The expected outcome (what this PR claims to fix):

% oras pull 080565210187.dkr.ecr.us-west-2.amazonaws.com/my.repo:my.tag
Downloading 3cd22ae067a3
Downloaded empty artifact
Pulled 080565210187.dkr.ecr.us-west-2.amazonaws.com/my.repo:my.tag
Digest: sha256:32aef68c20b6f25d69ecbc2f3f98e28f6f6fdacade75ee879459b61f610583f0

Resolves #225

nima · 2022-07-12T20:40:08Z

repro prior to the change:

➜  DoodleContainers git:(mainline) ✗ make setup start main
#------------------------------------------------------------------------------- oci:/home/ntd/docker/config.json
aws --profile ntd@ecr --region us-west-2 ecr get-login-password | docker login --username AWS --password-stdin 080565210187.dkr.ecr.us-west-2.amazonaws.com
WARNING! Your password will be stored unencrypted in /home/ntd/.docker/config.json.
Configure a credential helper to remove this warning. See
https://docs.docker.com/engine/reference/commandline/login/#credentials-store

Login Succeeded

#------------------------------------------------------------------------------- oci:/home/ntd/docker/config.json
aws --profile ntd@ecr --region us-west-2 ecr get-login-password | oras login --username AWS --password-stdin 080565210187.dkr.ecr.us-west-2.amazonaws.com
Login Succeeded

#------------------------------------------------------------------------------- oci:setup
aws --profile ntd@ecr --region us-west-2 ecr create-repository --repository-name my.repo --image-scanning-configuration scanOnPush=true | jq .
{
  "repository": {
    "repositoryUri": "080565210187.dkr.ecr.us-west-2.amazonaws.com/my.repo",
    "imageScanningConfiguration": {
      "scanOnPush": true
    },
    "encryptionConfiguration": {
      "encryptionType": "AES256"
    },
    "registryId": "080565210187",
    "imageTagMutability": "MUTABLE",
    "repositoryArn": "arn:aws:ecr:us-west-2:080565210187:repository/my.repo",
    "repositoryName": "my.repo",
    "createdAt": 1657835189
  }
}

#------------------------------------------------------------------------------- oci:setup
docker image build --tag my.repo:my.tag .
Sending build context to Docker daemon  371.2kB
Step 1/8 : FROM alpine:latest
 ---> e66264b98777
Step 2/8 : RUN mkdir -p /docked
 ---> Running in d2183399b150
Removing intermediate container d2183399b150
 ---> 56d7cf077766
Step 3/8 : WORKDIR /docked
 ---> Running in 36bac997e4ab
Removing intermediate container 36bac997e4ab
 ---> 8c7179532383
Step 4/8 : RUN apk add --no-cache socat
 ---> Running in 902e75a3a394
fetch https://dl-cdn.alpinelinux.org/alpine/v3.16/main/x86_64/APKINDEX.tar.gz
fetch https://dl-cdn.alpinelinux.org/alpine/v3.16/community/x86_64/APKINDEX.tar.gz
(1/4) Installing ncurses-terminfo-base (6.3_p20220521-r0)
(2/4) Installing ncurses-libs (6.3_p20220521-r0)
(3/4) Installing readline (8.1.2-r0)
(4/4) Installing socat (1.7.4.3-r0)
Executing busybox-1.35.0-r13.trigger
OK: 7 MiB in 18 packages
Removing intermediate container 902e75a3a394
 ---> 671c5e9f24a1
Step 5/8 : EXPOSE 1234
 ---> Running in fec0ca2cc0aa
Removing intermediate container fec0ca2cc0aa
 ---> 6480cf48acca
Step 6/8 : EXPOSE 4321
 ---> Running in 99961c605283
Removing intermediate container 99961c605283
 ---> 8ca79f8cdfcd
Step 7/8 : ENTRYPOINT [ "socat" ]
 ---> Running in 11b11566b05a
Removing intermediate container 11b11566b05a
 ---> 134e5e444ce0
Step 8/8 : CMD [ "TCP4-LISTEN:1234", "TCP4-LISTEN:4321" ]
 ---> Running in afb7185f1033
Removing intermediate container afb7185f1033
 ---> 4b8f7040c20e
Successfully built 4b8f7040c20e
Successfully tagged my.repo:my.tag

#------------------------------------------------------------------------------- oci:setup
docker image tag my.repo:my.tag 080565210187.dkr.ecr.us-west-2.amazonaws.com/my.repo:my.tag

#------------------------------------------------------------------------------- oci:setup
docker image push 080565210187.dkr.ecr.us-west-2.amazonaws.com/my.repo:my.tag
The push refers to repository [080565210187.dkr.ecr.us-west-2.amazonaws.com/my.repo]
8e2e4e6d1ed9: Pushed
980e95506b86: Pushed
24302eb7d908: Pushed
my.tag: digest: sha256:f8f0e94c24b562c0a5059a6c5d1c23f3f15c16381e912798531522d32d496df4 size: 945

#------------------------------------------------------------------------------- oci:start
docker container run --detach --rm --name my.container --publish 1234:1234 --publish 4321:4321 my.repo:my.tag
2e9752460dc4862f0dafbbf1f1b29fe1521ce771cd58ae9208e82a1ca6ecde72

#------------------------------------------------------------------------------- oci:main
oras pull 080565210187.dkr.ecr.us-west-2.amazonaws.com/my.repo:my.tag
Error: GET "https://080565210187.dkr.ecr.us-west-2.amazonaws.com/v2/my.repo/manifests/my.tag": empty response Docker-Content-Digest

make: *** [main] Error 1
➜  DoodleContainers git:(mainline) ✗

run test again...

➜  DoodleContainers git:(mainline) ✗ make
################################################################################
#------------------------------------------------------------------------------- oci:/home/nima/docker/config.json
aws --profile nima@ecr --region us-west-2 ecr get-login-password | docker login --username AWS --password-stdin 12345678901.dkr.ecr.us-west-2.amazonaws.com
WARNING! Your password will be stored unencrypted in /home/nima/.docker/config.json.
Configure a credential helper to remove this warning. See
https://docs.docker.com/engine/reference/commandline/login/#credentials-store

Login Succeeded

#------------------------------------------------------------------------------- oci:/home/nima/docker/config.json
aws --profile nima@ecr --region us-west-2 ecr get-login-password | oras login --username AWS --password-stdin 12345678901.dkr.ecr.us-west-2.amazonaws.com
Login Succeeded

#------------------------------------------------------------------------------- oci:setup
aws --profile nima@ecr --region us-west-2 ecr create-repository --repository-name my.repo --image-scanning-configuration scanOnPush=true | jq .
{
  "repository": {
    "repositoryUri": "12345678901.dkr.ecr.us-west-2.amazonaws.com/my.repo",
    "imageScanningConfiguration": {
      "scanOnPush": true
    },
    "encryptionConfiguration": {
      "encryptionType": "AES256"
    },
    "registryId": "12345678901",
    "imageTagMutability": "MUTABLE",
    "repositoryArn": "arn:aws:ecr:us-west-2:12345678901:repository/my.repo",
    "repositoryName": "my.repo",
    "createdAt": 1657595026
  }
}

#------------------------------------------------------------------------------- oci:setup
docker image build --tag my.repo:my.tag .
Sending build context to Docker daemon    276kB
Step 1/8 : FROM alpine:latest
 ---> e66264b98777
Step 2/8 : RUN mkdir -p /docked
 ---> Using cache
 ---> b70ff30822c5
Step 3/8 : WORKDIR /docked
 ---> Using cache
 ---> adbf13e74342
Step 4/8 : RUN apk add --no-cache socat
 ---> Using cache
 ---> 10673949eac4
Step 5/8 : EXPOSE 1234
 ---> Using cache
 ---> fe6e2db39608
Step 6/8 : EXPOSE 4321
 ---> Using cache
 ---> 69b73391ecf3
Step 7/8 : ENTRYPOINT [ "socat" ]
 ---> Using cache
 ---> 7ed365f9b91e
Step 8/8 : CMD [ "TCP4-LISTEN:1234", "TCP4-LISTEN:4321" ]
 ---> Using cache
 ---> cffeefdfa844
Successfully built cffeefdfa844
Successfully tagged my.repo:my.tag

#------------------------------------------------------------------------------- oci:setup
docker image tag my.repo:my.tag 12345678901.dkr.ecr.us-west-2.amazonaws.com/my.repo:my.tag

#------------------------------------------------------------------------------- oci:setup
docker image push 12345678901.dkr.ecr.us-west-2.amazonaws.com/my.repo:my.tag
The push refers to repository [12345678901.dkr.ecr.us-west-2.amazonaws.com/my.repo]
336abb34c67c: Pushed
26a6b0d4e0cf: Pushed
24302eb7d908: Pushed
my.tag: digest: sha256:b9b2c380a97b8b4ab2acfd3d88850bc3ce2b95e803af33ac382e002b6683fbfd size: 945

#------------------------------------------------------------------------------- oci:main
docker image rm my.repo:my.tag
Untagged: my.repo:my.tag

#------------------------------------------------------------------------------- oci:main
oras pull 12345678901.dkr.ecr.us-west-2.amazonaws.com/my.repo:my.tag
Downloaded empty artifact
Pulled 12345678901.dkr.ecr.us-west-2.amazonaws.com/my.repo:my.tag
Digest:

#------------------------------------------------------------------------------- oci:teardown
docker image rm my.repo:my.tag
Error: No such image: my.repo:my.tag

make: [oci.teardown] Error 1 (ignored)
#------------------------------------------------------------------------------- oci:teardown
docker image rm 12345678901.dkr.ecr.us-west-2.amazonaws.com/my.repo:my.tag
Error response from daemon: conflict: unable to remove repository reference "12345678901.dkr.ecr.us-west-2.amazonaws.com/my.repo:my.tag" (must force) - container 799004d623e6 is using its referenced image cffeefdfa844

make: [oci.teardown] Error 1 (ignored)
#------------------------------------------------------------------------------- oci:teardown
aws --profile nima@ecr --region us-west-2 ecr batch-delete-image --repository-name my.repo --image-ids imageTag=my.tag | jq .
{
  "failures": [],
  "imageIds": [
    {
      "imageTag": "my.tag",
      "imageDigest": "sha256:b9b2c380a97b8b4ab2acfd3d88850bc3ce2b95e803af33ac382e002b6683fbfd"
    }
  ]
}

#------------------------------------------------------------------------------- oci:teardown
aws --profile nima@ecr --region us-west-2 ecr delete-repository --repository-name my.repo | jq .
{
  "repository": {
    "repositoryUri": "12345678901.dkr.ecr.us-west-2.amazonaws.com/my.repo",
    "registryId": "12345678901",
    "imageTagMutability": "MUTABLE",
    "repositoryArn": "arn:aws:ecr:us-west-2:12345678901:repository/my.repo",
    "repositoryName": "my.repo",
    "createdAt": 1657595026
  }
}

GUIDE            : https://www.freecodecamp.org/news/the-docker-handbook/
DEMO             : oci
TARGET           : oci
ORAS_CLIENT_PATH : /home/nima/bin/oras
ORAS_CLIENT      : 0.13.0+unreleased
DOCKER_CLIENT    : 20.10.7
DOCKER_SERVER    : 20.10.7
################################################################################
➜  DoodleContainers git:(mainline) ✗

internal/ioutil/io.go

Wwwsylvia · 2022-07-15T10:37:03Z

Thanks @nima for the PR! Please let us know if this is ready for review. 😀

nima · 2022-07-16T06:48:55Z

Thanks @nima for the PR! Please let us know if this is ready for review. 😀

Thanks for taking a look @Wwwsylvia -- I have another PR out now, it's not intended to be approved, only to be inspected because I'm still not sure how to deal with the current issue. I'm maintaining a log of my thoughts in a thread in the #oras slack channel, and also in the git commit message body.

nima · 2022-08-18T02:50:06Z

[INFO] Current slack thread for this round.

nima · 2022-08-18T03:34:55Z

I've set this to R4R despite the failure to expose the PR. The failure is due to the fact that one of the dependencies of this package, go-digest, has also been modified as part of this change. Locally, I've been using thereplace directive in go.mod to point it to the locally modified version, but I'm not sure how to go about it here, other than get that change separately approved and merged in first, then continue with this.

shizhMSFT · 2022-08-18T04:00:54Z

Few general comments to make this PR better.

Please follow conventional commits for PR title and body.
Please link this PR to an issue.
It is better to have a ruler at 80 chars for comment blocks so that people with small screens can read the code easily.
Try not to modify the code of dependencies. If it is really needed, try proposing and merging the required changes to the upstream first.

FeynmanZhou · 2022-08-18T04:33:50Z

Few general comments to make this PR better.

Please follow conventional commits for PR title and body.

Please link this PR to an issue.

It is better to have a ruler at 80 chars for comment blocks so that people with small screens can read the code easily.

Try not to modify the code of dependencies. If it is really needed, try proposing and merging the required changes to the upstream first.

The ORAS community also has a simple version of Git Conventions for references.

registry/remote/repository.go

That this issue existed is a side-effect of an earlier chronic issue. At least in the pull workflow, no where does the library (on behalf of the client) attempt to verify the digest returned by the register/server. By not doing so, it also has taken an unhealthy dependency on a precariously OPTIONAL response header, namely, `Docker-Content-Digest`. All this has lead to issue oras-project#225. Discussion thread on this has been opened up in Slack: https://cloud-native.slack.com/archives/CJ1KHJM5Z/p1657935407555609 Signed-off-by: Nima Talebi <github@nima.id.au>

Also drop dependency on go-digest changes, making this PR stand-alone. Signed-off-by: Nima Talebi <github@nima.id.au>

Signed-off-by: Nima Talebi <github@nima.id.au>

nima

Okay I think I've addressed all comments so far. Also expanded on what happens when expected is not length-checked.

registry/remote/repository.go

registry/remote/repository_test.go

registry/remote/repository.go

Signed-off-by: Nima Talebi <github@nima.id.au>

shizhMSFT

LGTM

Wwwsylvia

LGTM with s minor suggestion

registry/remote/repository.go

nima · 2022-08-25T06:13:56Z

And a final double-check...

% oras pull 080565210187.dkr.ecr.us-west-2.amazonaws.com/my.repo:my.tag
Downloaded empty artifact
Pulled 080565210187.dkr.ecr.us-west-2.amazonaws.com/my.repo:my.tag
Digest: sha256:410bc64c7dbbc3286211561a0f6ca43affdca5c92c568f838a37e34640f5f76e

Signed-off-by: Nima Talebi <github@nima.id.au>

Wwwsylvia

LGTM!

gokarnm · 2022-08-25T15:27:40Z

Thanks @nima for chasing this issue down, and @shizhMSFT @Wwwsylvia for the reviews!
@shizhMSFT @FeynmanZhou do we plan to release this fix with a patch release?

shizhMSFT · 2022-08-26T15:30:37Z

We plan to release this in the next rc release.

michaelb990 · 2022-10-11T07:34:33Z

Nope! That’s my Java showing 😇… carry on.

…

On Thu, Aug 18, 2022 at 17:25 Nima Talebi ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In registry/remote/repository.go <#237 (comment)>: > return fmt.Errorf("%s: mismatch digest: %s", expected, contentDigest) } return nil } +// verifyContentDigests is a superset of verifyContentDigest; it takes in the actual (calculated) digest, and +// verifies against that, the other two digests, both of which may or may not be set to the empty digest. If empty, +// there is no validation error, however is not empty, it ensures that the given digest is valid (matches the actual +// digest) +func verifyContentDigests(actualDigest digest.Digest, digests map[string]digest.Digest) error { + for src, d := range digests { + if len(d.String()) == 0 { Because you can't set a string to a nil; it's not a *digest.Digest (pointer). Am I missing something? — Reply to this email directly, view it on GitHub <#237 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAIDST5SMDJGM3IW22JF4CDVZ3IBBANCNFSM53IDEHXQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>

nima force-pushed the issues/225 branch 2 times, most recently from 0fba8f2 to f43a780 Compare July 11, 2022 16:35

nima marked this pull request as draft July 11, 2022 16:47

nima force-pushed the issues/225 branch from f43a780 to 22b211d Compare July 12, 2022 03:03

qweeah reviewed Jul 15, 2022

View reviewed changes

internal/ioutil/io.go Outdated Show resolved Hide resolved

nima force-pushed the issues/225 branch 2 times, most recently from e9abfb6 to 2e04701 Compare July 15, 2022 07:49

nima mentioned this pull request Jul 15, 2022

[SUPERCEDED] Standardize and disambiguate the return from digest.Parse opencontainers/go-digest#76

Closed

nima force-pushed the issues/225 branch from 2e04701 to f8e52ec Compare July 15, 2022 07:58

nima force-pushed the issues/225 branch from f8e52ec to be03d3b Compare July 16, 2022 03:59

nima force-pushed the issues/225 branch 8 times, most recently from bfe9480 to 1d4fbdb Compare July 19, 2022 07:22

nima force-pushed the issues/225 branch 3 times, most recently from 3521041 to 2fa1eb0 Compare August 18, 2022 02:45

nima marked this pull request as ready for review August 18, 2022 03:32

shizhMSFT linked an issue Aug 18, 2022 that may be closed by this pull request

oras pull error: empty response Docker-Content-Digest #225

Closed

nima force-pushed the issues/225 branch from 677c41f to 7e8b69a Compare August 25, 2022 03:20

shizhMSFT reviewed Aug 25, 2022

View reviewed changes

registry/remote/repository.go Outdated Show resolved Hide resolved

registry/remote/repository.go Outdated Show resolved Hide resolved

registry/remote/repository.go Outdated Show resolved Hide resolved

registry/remote/repository.go Outdated Show resolved Hide resolved

nima added 8 commits August 24, 2022 20:49

fix: Address PR comments

44e2c34

Also drop dependency on go-digest changes, making this PR stand-alone. Signed-off-by: Nima Talebi <github@nima.id.au>

fix: Add, test for, and implement the truth table

d19c896

Signed-off-by: Nima Talebi <github@nima.id.au>

refactor: Convert `generateDescriptor' methods to functions

408d31d

Signed-off-by: Nima Talebi <github@nima.id.au>

fix: Add digest validation back

9d4bdc9

Signed-off-by: Nima Talebi <github@nima.id.au>

fix: Address PR comments

4af3568

Signed-off-by: Nima Talebi <github@nima.id.au>

fix: Address PR comments

ec7c06d

Signed-off-by: Nima Talebi <github@nima.id.au>

test: Add generateBlobDescriptor tests & truth table

59a8bab

Signed-off-by: Nima Talebi <github@nima.id.au>

nima commented Aug 25, 2022

View reviewed changes

nima force-pushed the issues/225 branch from 7e8b69a to e7ad951 Compare August 25, 2022 04:15

shizhMSFT reviewed Aug 25, 2022

View reviewed changes

registry/remote/repository.go Outdated Show resolved Hide resolved

registry/remote/repository.go Outdated Show resolved Hide resolved

registry/remote/repository.go Outdated Show resolved Hide resolved

Wwwsylvia reviewed Aug 25, 2022

View reviewed changes

registry/remote/repository.go Outdated Show resolved Hide resolved

registry/remote/repository_test.go Outdated Show resolved Hide resolved

shizhMSFT reviewed Aug 25, 2022

View reviewed changes

registry/remote/repository.go Outdated Show resolved Hide resolved

registry/remote/repository.go Show resolved Hide resolved

fix: Address PR comments

9dc69ee

Signed-off-by: Nima Talebi <github@nima.id.au>

nima force-pushed the issues/225 branch 3 times, most recently from 9e662e6 to bcfd66f Compare August 25, 2022 05:33

shizhMSFT approved these changes Aug 25, 2022

View reviewed changes

Wwwsylvia approved these changes Aug 25, 2022

View reviewed changes

registry/remote/repository.go Outdated Show resolved Hide resolved

test: Remove redundant tests

83a8779

Signed-off-by: Nima Talebi <github@nima.id.au>

nima force-pushed the issues/225 branch from bcfd66f to 83a8779 Compare August 25, 2022 06:14

Wwwsylvia approved these changes Aug 25, 2022

View reviewed changes

Wwwsylvia merged commit 98e72a2 into oras-project:main Aug 25, 2022

nima deleted the issues/225 branch August 25, 2022 06:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: oras pull error `empty response Docker-Content-Digest` #237

fix: oras pull error `empty response Docker-Content-Digest` #237

nima commented Jul 11, 2022 •

edited

Loading

nima commented Jul 12, 2022 •

edited

Loading

Wwwsylvia commented Jul 15, 2022

nima commented Jul 16, 2022

nima commented Aug 18, 2022

nima commented Aug 18, 2022 •

edited

Loading

shizhMSFT commented Aug 18, 2022

FeynmanZhou commented Aug 18, 2022

nima left a comment

shizhMSFT left a comment

Wwwsylvia left a comment

nima commented Aug 25, 2022

Wwwsylvia left a comment

gokarnm commented Aug 25, 2022

shizhMSFT commented Aug 26, 2022

michaelb990 commented Oct 11, 2022 via email

fix: oras pull error empty response Docker-Content-Digest #237

fix: oras pull error empty response Docker-Content-Digest #237

Conversation

nima commented Jul 11, 2022 • edited Loading

nima commented Jul 12, 2022 • edited Loading

repro prior to the change:

run test again...

Wwwsylvia commented Jul 15, 2022

nima commented Jul 16, 2022

nima commented Aug 18, 2022

nima commented Aug 18, 2022 • edited Loading

shizhMSFT commented Aug 18, 2022

FeynmanZhou commented Aug 18, 2022

nima left a comment

Choose a reason for hiding this comment

shizhMSFT left a comment

Choose a reason for hiding this comment

Wwwsylvia left a comment

Choose a reason for hiding this comment

nima commented Aug 25, 2022

Wwwsylvia left a comment

Choose a reason for hiding this comment

gokarnm commented Aug 25, 2022

shizhMSFT commented Aug 26, 2022

michaelb990 commented Oct 11, 2022 via email

fix: oras pull error `empty response Docker-Content-Digest` #237

fix: oras pull error `empty response Docker-Content-Digest` #237

nima commented Jul 11, 2022 •

edited

Loading

nima commented Jul 12, 2022 •

edited

Loading

nima commented Aug 18, 2022 •

edited

Loading