Update docker_registry2 #7578

NautiluX · 2023-07-18T08:35:40Z

Fixes specs that break after 1.15
Bump to 1.17 to allow pagination on registry.access.redhat.com based images

Closes #7562.

docker/Gemfile

jurre · 2023-07-18T09:03:10Z

Could you provide a little bit of context on the update, it looks like the minor update contained some breaking changes? Thanks!

NautiluX · 2023-07-18T09:06:21Z

Yes.
Prior art in PR #7289
I added a fix to docker_registry2 which will come with the next tag. The tag is not yet created, hence I started updating to the current latest tag to adjust to the changes in docker_registry2, so pulling in the next update is hopefully a quick thing.

NautiluX · 2023-07-18T13:45:32Z

Tag 1.17 has been pushed in the meantime, so I added updating to 1.17 in this PR.

NautiluX · 2023-07-18T14:01:12Z

Not sure the docker smoke test is related, can you have a look @jurre?

jurre · 2023-07-18T14:57:09Z

@NautiluX yeah that does look related, seems like dockerhub is requiring us to auth for these requests now? Let me re-run them to make sure it wasn't just a hiccup

NautiluX · 2023-07-18T15:06:16Z

right so these requests:

  proxy | 2023/07/18 14:58:43 [016] GET https://registry.hub.docker.com:443/v2/library/ubuntu/manifests/latest
  proxy | 2023/07/18 14:58:44 [016] 401 https://registry.hub.docker.com:443/v2/library/ubuntu/manifests/latest

used to be HEAD requests before 1.15, that could explain the difference.
There are some authorized requests to /list, probably we can do the same with GET.

NautiluX · 2023-07-19T11:34:28Z

I tried reproducing the behaviour with a minimal example, doing the same calls that dependabot would do:

require "docker_registry2"

client = DockerRegistry2.connect(
            "https://registry.hub.docker.com:443",
            user: nil,
            password: nil,
            read_timeout: 10,
            http_options: { proxy: "https://localhost:8080",verify_ssl: false }
            
          )
client.tags("library/ubuntu", auto_paginate: true).fetch("tags").each do |t|
    print t
end
print client.digest("library/ubuntu", "latest")

Interjected the requests with a proxy and got similar requests than the smoketest:

[13:32:03.281] HTTP(S) proxy listening at *:8080.
127.0.0.1:51575: GET https://registry.hub.docker.com/v2/library/ubuntu/tags/list
 << 401 Unauthorized 157b
127.0.0.1:51577: GET https://auth.docker.io/token?service=registry.docker.io&scope=repository%3Alibrary%2Fubuntu%3Apull&account
127.0.0.1:51579: GET https://registry.hub.docker.com/v2/library/ubuntu/tags/list
 << 200 OK 9.6k
127.0.0.1:51581: GET https://registry.hub.docker.com/v2/library/ubuntu/manifests/latest
 << 401 Unauthorized 157b
127.0.0.1:51583: GET https://auth.docker.io/token?service=registry.docker.io&scope=repository%3Alibrary%2Fubuntu%3Apull&account
 << 200 OK 4.3k
127.0.0.1:51585: GET https://registry.hub.docker.com/v2/library/ubuntu/manifests/latest
 << 200 OK 1.1k

However, the second GET got a 200 as response, not a 401 as in the smoketest.

NautiluX · 2023-07-19T13:49:08Z

managed to get the test running locally, it's succeeding as well. Not sure what's still different to the environment where they run in the GitHub action.

  proxy | 2023/07/19 13:46:41 [010] GET https://registry.hub.docker.com:443/v2/library/ubuntu/tags/list
  proxy | 2023/07/19 13:46:41 [010] 401 https://registry.hub.docker.com:443/v2/library/ubuntu/tags/list
  proxy | 2023/07/19 13:46:41 [012] GET https://auth.docker.io:443/token?service=registry.docker.io&scope=repository%3Alibrary%2Fubuntu%3Apull&account
  proxy | 2023/07/19 13:46:41 [012] 200 https://auth.docker.io:443/token?service=registry.docker.io&scope=repository%3Alibrary%2Fubuntu%3Apull&account
  proxy | 2023/07/19 13:46:41 [014] GET https://registry.hub.docker.com:443/v2/library/ubuntu/tags/list
  proxy | 2023/07/19 13:46:42 [014] 200 https://registry.hub.docker.com:443/v2/library/ubuntu/tags/list
  proxy | 2023/07/19 13:46:42 [016] GET https://registry.hub.docker.com:443/v2/library/ubuntu/manifests/latest
  proxy | 2023/07/19 13:46:42 [016] 401 https://registry.hub.docker.com:443/v2/library/ubuntu/manifests/latest
  proxy | 2023/07/19 13:46:42 [018] GET https://auth.docker.io:443/token?service=registry.docker.io&scope=repository%3Alibrary%2Fubuntu%3Apull&account
  proxy | 2023/07/19 13:46:42 [018] 200 https://auth.docker.io:443/token?service=registry.docker.io&scope=repository%3Alibrary%2Fubuntu%3Apull&account
  proxy | 2023/07/19 13:46:42 [020] GET https://registry.hub.docker.com:443/v2/library/ubuntu/manifests/latest
  proxy | 2023/07/19 13:46:42 [020] 200 https://registry.hub.docker.com:443/v2/library/ubuntu/manifests/latest
  proxy | 2023/07/19 13:46:42 [022] GET https://registry.hub.docker.com:443/v2/library/ubuntu/manifests/23.10
  proxy | 2023/07/19 13:46:42 [022] 401 https://registry.hub.docker.com:443/v2/library/ubuntu/manifests/23.10
  proxy | 2023/07/19 13:46:42 [024] GET https://auth.docker.io:443/token?service=registry.docker.io&scope=repository%3Alibrary%2Fubuntu%3Apull&account
  proxy | 2023/07/19 13:46:42 [024] 200 https://auth.docker.io:443/token?service=registry.docker.io&scope=repository%3Alibrary%2Fubuntu%3Apull&account
  proxy | 2023/07/19 13:46:42 [026] GET https://registry.hub.docker.com:443/v2/library/ubuntu/manifests/23.10
  proxy | 2023/07/19 13:46:42 [026] 200 https://registry.hub.docker.com:443/v2/library/ubuntu/manifests/23.10
  proxy | 2023/07/19 13:46:42 [028] GET https://registry.hub.docker.com:443/v2/library/ubuntu/manifests/23.04
  proxy | 2023/07/19 13:46:42 [028] 401 https://registry.hub.docker.com:443/v2/library/ubuntu/manifests/23.04
  proxy | 2023/07/19 13:46:42 [030] GET https://auth.docker.io:443/token?service=registry.docker.io&scope=repository%3Alibrary%2Fubuntu%3Apull&account
  proxy | 2023/07/19 13:46:42 [030] 200 https://auth.docker.io:443/token?service=registry.docker.io&scope=repository%3Alibrary%2Fubuntu%3Apull&account
  proxy | 2023/07/19 13:46:42 [032] GET https://registry.hub.docker.com:443/v2/library/ubuntu/manifests/23.04
  proxy | 2023/07/19 13:46:42 [032] 200 https://registry.hub.docker.com:443/v2/library/ubuntu/manifests/23.04
  proxy | 2023/07/19 13:46:42 [034] GET https://registry.hub.docker.com:443/v2/library/ubuntu/manifests/22.10
  proxy | 2023/07/19 13:46:42 [034] 401 https://registry.hub.docker.com:443/v2/library/ubuntu/manifests/22.10
  proxy | 2023/07/19 13:46:43 [036] GET https://auth.docker.io:443/token?service=registry.docker.io&scope=repository%3Alibrary%2Fubuntu%3Apull&account
  proxy | 2023/07/19 13:46:43 [036] 200 https://auth.docker.io:443/token?service=registry.docker.io&scope=repository%3Alibrary%2Fubuntu%3Apull&account
  proxy | 2023/07/19 13:46:43 [038] GET https://registry.hub.docker.com:443/v2/library/ubuntu/manifests/22.10
  proxy | 2023/07/19 13:46:43 [038] 200 https://registry.hub.docker.com:443/v2/library/ubuntu/manifests/22.10
  proxy | 2023/07/19 13:46:43 [040] GET https://registry.hub.docker.com:443/v2/library/ubuntu/manifests/22.04
  proxy | 2023/07/19 13:46:43 [040] 401 https://registry.hub.docker.com:443/v2/library/ubuntu/manifests/22.04
  proxy | 2023/07/19 13:46:43 [042] GET https://auth.docker.io:443/token?service=registry.docker.io&scope=repository%3Alibrary%2Fubuntu%3Apull&account
  proxy | 2023/07/19 13:46:43 [042] 200 https://auth.docker.io:443/token?service=registry.docker.io&scope=repository%3Alibrary%2Fubuntu%3Apull&account
  proxy | 2023/07/19 13:46:43 [044] GET https://registry.hub.docker.com:443/v2/library/ubuntu/manifests/22.04
  proxy | 2023/07/19 13:46:43 [044] 200 https://registry.hub.docker.com:443/v2/library/ubuntu/manifests/22.04

NautiluX · 2023-07-19T13:58:48Z

@jurre I think I found the problem. When I download the cache and run against it, the test run fails for me locally as well. Looking at the test output in the github action, it says:

proxy | 2023/07/18 14:58:44 7/9 calls cached (77%)

My understanding is, for some reason, it uses cached HTTP responses for the GET calls and in the cache there is the 401 response.
Any idea how to fix that? Is there a way to disable or clear that cache?

NautiluX · 2023-07-19T14:12:39Z

If I understand how the smoke-tests work correctly, I don't think recaching before merging will help since this PR is responsible for the changed responses right?
I tried disabling the cache for the docker smoke test to verify my assumption.

jurre · 2023-07-19T14:15:06Z

Ah, I think that's something about how our e2e tests work, let me ask someone internally

.github/workflows/smoke.yml

* Fixes specs that break after 1.15 * Bump to 1.17 to allow paging registry.access.redhat.com based images

* digest API has changed in docker_registry2 with 1.15 * preserving current behavior by extracting the digest header as docker_registry2 <1.15 did

NautiluX · 2023-07-20T07:56:11Z

@jurre I verified the docker e2e test works without cache locally, so the PR should be ready to get merged from my perspective. I had to update the digest querying so it behaves as it did with docker_registry2 <1.15.

jurre · 2023-07-20T10:06:19Z

Thanks for this fix @NautiluX 🙇 I'll try to get the change out in a bit

jurre · 2023-07-20T13:23:58Z

Unfortunately, it seems that with these changes we're now seeing a lot of rate limiting errors, I'm going to revert so we can dig into it more

jurre · 2023-07-20T14:35:41Z

docker/lib/dependabot/docker/update_checker.rb

@@ -245,7 +245,8 @@ def digest_of(tag)
      end

      def fetch_digest_of(tag)
-        docker_registry_client.digest(docker_repo_name, tag)&.delete_prefix("sha256:")
+        manifest = docker_registry_client.manifest(docker_repo_name, tag)


So it seems like the GET request to this endpoint ends up counting towards rate-limits whereas the HEAD request didn't, and since we make a lot of requests from a relatively small set of IP addresses, a lot of our updates ended up failing when I ran this in production :(

NautiluX · 2023-07-20T14:56:17Z

Trying to reintroduce the old functionality in docker_registry2 lib: deitch/docker_registry2#95

yeikel · 2023-09-11T21:15:39Z

Commenting to add that depending on this new method goes against the spec. See deitch/docker_registry2#98

yeikel · 2023-09-11T21:26:17Z

I created #8010 to open this for discussion

Update docker_registry2

NautiluX requested a review from a team as a code owner July 18, 2023 08:35

github-actions bot added the L: docker Docker containers label Jul 18, 2023

NautiluX force-pushed the update-dockerregistry branch from 40bf586 to 4147bcd Compare July 18, 2023 08:43

jurre reviewed Jul 18, 2023

View reviewed changes

docker/Gemfile Outdated Show resolved Hide resolved

NautiluX force-pushed the update-dockerregistry branch from 4147bcd to 15ae2aa Compare July 18, 2023 09:03

NautiluX force-pushed the update-dockerregistry branch 3 times, most recently from bd98523 to 14e32ed Compare July 18, 2023 13:23

NautiluX changed the title ~~Update dockerregistry~~ Update docker_registry2 Jul 18, 2023

NautiluX force-pushed the update-dockerregistry branch from 14e32ed to ae3fc02 Compare July 18, 2023 13:43

NautiluX requested a review from jurre July 18, 2023 14:14

NautiluX force-pushed the update-dockerregistry branch from ae3fc02 to 65df421 Compare July 19, 2023 11:57

deivid-rodriguez mentioned this pull request Jul 19, 2023

Bump docker_registry2 from 1.14.0 to 1.17.0 in /updater #7587

Closed

jurre force-pushed the update-dockerregistry branch from 88ca7df to d598a9e Compare July 19, 2023 17:16

jurre reviewed Jul 19, 2023

View reviewed changes

.github/workflows/smoke.yml Outdated Show resolved Hide resolved

Manuel Dewald added 2 commits July 20, 2023 09:19

Update docker_registry2

9a3082a

* Fixes specs that break after 1.15 * Bump to 1.17 to allow paging registry.access.redhat.com based images

Use manifest API to get image digest

a33efb0

* digest API has changed in docker_registry2 with 1.15 * preserving current behavior by extracting the digest header as docker_registry2 <1.15 did

NautiluX force-pushed the update-dockerregistry branch from d598a9e to a33efb0 Compare July 20, 2023 07:31

NautiluX mentioned this pull request Jul 20, 2023

Not getting expected update on docker image #7562

Closed

1 task

jurre approved these changes Jul 20, 2023

View reviewed changes

jurre merged commit b3103d4 into dependabot:main Jul 20, 2023
117 of 118 checks passed

jurre mentioned this pull request Jul 20, 2023

Revert "Update docker_registry2" #7601

Merged

jurre reviewed Jul 20, 2023

View reviewed changes

NautiluX mentioned this pull request Jul 20, 2023

Reintroduce old digest functionality deitch/docker_registry2#95

Merged

yeikel mentioned this pull request Sep 11, 2023

Remove dependency on docker_content_digest digest headers for docker updates #8010

Open

brettfo pushed a commit to brettfo/dependabot-core that referenced this pull request Oct 11, 2023

Merge pull request dependabot#7578 from NautiluX/update-dockerregistry

9704b7f

Update docker_registry2

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update docker_registry2 #7578

Update docker_registry2 #7578

NautiluX commented Jul 18, 2023 •

edited by deivid-rodriguez

Loading

jurre commented Jul 18, 2023

NautiluX commented Jul 18, 2023

NautiluX commented Jul 18, 2023

NautiluX commented Jul 18, 2023

jurre commented Jul 18, 2023

NautiluX commented Jul 18, 2023 •

edited

Loading

NautiluX commented Jul 19, 2023

NautiluX commented Jul 19, 2023

NautiluX commented Jul 19, 2023

NautiluX commented Jul 19, 2023

jurre commented Jul 19, 2023

NautiluX commented Jul 20, 2023

jurre commented Jul 20, 2023

jurre commented Jul 20, 2023

jurre Jul 20, 2023

NautiluX commented Jul 20, 2023

yeikel commented Sep 11, 2023

yeikel commented Sep 11, 2023

Update docker_registry2 #7578

Update docker_registry2 #7578

Conversation

NautiluX commented Jul 18, 2023 • edited by deivid-rodriguez Loading

jurre commented Jul 18, 2023

NautiluX commented Jul 18, 2023

NautiluX commented Jul 18, 2023

NautiluX commented Jul 18, 2023

jurre commented Jul 18, 2023

NautiluX commented Jul 18, 2023 • edited Loading

NautiluX commented Jul 19, 2023

NautiluX commented Jul 19, 2023

NautiluX commented Jul 19, 2023

NautiluX commented Jul 19, 2023

jurre commented Jul 19, 2023

NautiluX commented Jul 20, 2023

jurre commented Jul 20, 2023

jurre commented Jul 20, 2023

jurre Jul 20, 2023

Choose a reason for hiding this comment

NautiluX commented Jul 20, 2023

yeikel commented Sep 11, 2023

yeikel commented Sep 11, 2023

NautiluX commented Jul 18, 2023 •

edited by deivid-rodriguez

Loading

NautiluX commented Jul 18, 2023 •

edited

Loading