Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds support for new OCI registries #658

Merged
merged 5 commits into from
Apr 22, 2022
Merged

Conversation

nmasse-itix
Copy link
Contributor

Adds support for new OCI registries

Fix #605

  • add the "application/vnd.oci.image.index.v1+json" content type in the Accept headers of the requests made to the registries
  • process the "application/vnd.oci.image.index.v1+json" responses just like "application/vnd.docker.distribution.manifest.list.v2+json" ones
  • remove support for "application/vnd.docker.distribution.manifest.v2+json"
  • update the quay.io unit test case to be an OCI unit test case

Test

To test this pull request, you can run the following commands:

go test -v ./pkg/plugins/utils/docker/dockerregistry

I also made sure to test a real pipeline such as:

title: Miniflux
sources:
  miniflux:
    name: Miniflux Release
    kind: githubRelease
    spec:
      owner: miniflux
      repository: v2
      token: REDACTED
      versionFilter:
        kind: semver
        pattern: ~2
conditions:
  miniflux_docker:
    name: Container image of Miniflux is available on Docker
    kind: dockerImage
    sourceID: miniflux
    spec:
      hostname: ghcr.io
      image: docker.io/miniflux/miniflux
      architecture: amd64
targets:
  miniflux:
    name: Update YAML
    kind: yaml
    sourceID: miniflux
    spec:
      file: tests/out.yaml
      key: miniflux.docker

I tested such a pipeline with the following images:

  • docker.io/miniflux/miniflux
  • ghcr.io/miniflux/miniflux
  • quay.io/itix/tic-tsdb
  • quay.io/itix/photo-bot

@olblak olblak requested review from dduportal and olblak April 19, 2022 11:54
@olblak
Copy link
Member

olblak commented Apr 19, 2022

Thanks for opening the pullrequest.
I just did some quick testing and while this PR is a bit improvement since it add supprot to OCI registry.
I don't think that it fixed #605

I did some testing with `quay.io/ansible/ansible-runner:latest

ERROR: ✗ Unsupported response type from the registry quay.io: Content-Type "application/vnd.docker.distribution.manifest.v1+json" is either deprecated or unknown by updatecli.

@nmasse-itix
Copy link
Contributor Author

Hi @olblak !

Thanks for the review. I tried to reproduced the issue with the following pipeline:

title: Ansible runner
sources:
  ansible-runner:
    name: Ansible Runner
    kind: dockerDigest
    spec:
      hostname: quay.io
      image: quay.io/ansible/ansible-runner
      tag: latest
targets:
  ansible-runner:
    name: Update YAML
    kind: yaml
    sourceID: ansible-runner
    spec:
      file: tests/out.yaml
      key: ansible-runner

And it seems that I'm getting the same error as you ERROR: ✗ Unsupported response type from the registry quay.io: Content-Type "application/vnd.docker.distribution.manifest.v1+json" is either deprecated or unknown by updatecli..

I will investigate. 👍

@olblak
Copy link
Member

olblak commented Apr 20, 2022

While investigate this issue, someone shared with me containerd/containerd#6471
I am still trying to find someone knowledgeable on this

@nmasse-itix
Copy link
Contributor Author

I read the OCI registry specs
And added support for standalone image manifests : application/vnd.oci.image.manifest.v1+json

Indeed, when dealing with standalone image manifests, the correct way to get a pullable digest is be reading the "Docker-Content-Digest" HTTP header.

The Docker-Content-Digest header, if present on the response, returns the canonical digest of the uploaded blob

But earlier, the spec says:

Because of the origins this specification, the client MAY encounter Docker-specific headers, such as Docker-Content-Digest, or Docker-Distribution-API-Version. These headers are OPTIONAL and clients SHOULD NOT depend on them.

Note: I left the original behavior where the architecture is not checked for standalone manifests. We can retrieve the architecture from the configuration layer at the expense of an extra HTTP request. If needed, I can implement it in another PR.

@olblak
Copy link
Member

olblak commented Apr 20, 2022

I did some experimentation and it seems to introduce a regression for dockerhub

For example jenkins/jenkins:latest would return

ERROR: ✗ Unsupported response type from the registry registry-1.docker.io: Content-Type "application/vnd.docker.distribution.manifest.v1+prettyjws" is either deprecated or unknown by updatecli.

But if I put back

--- a/pkg/plugins/utils/docker/dockerregistry/main.go
+++ b/pkg/plugins/utils/docker/dockerregistry/main.go
@@ -71,6 +71,7 @@ func (dgr DockerGenericRegistry) Digest(image dockerimage.Image) (string, error)
        //  * Docker Registry V2 manifest list
        //  * List of OCI manifests (multiple architectures provided by the registry)
        //  * Standalone OCI manifest (only one architecture provided by the registry)
+       req.Header.Add("Accept", "application/vnd.docker.distribution.manifest.v2+json")
        req.Header.Add("Accept", "application/vnd.docker.distribution.manifest.list.v2+json")
        req.Header.Add("Accept", "application/vnd.oci.image.index.v1+json")
        req.Header.Add("Accept", "application/vnd.oci.image.manifest.v1+json")
@@ -119,6 +120,10 @@ func (dgr DockerGenericRegistry) Digest(image dockerimage.Image) (string, error)
        }
 
        switch res.Header.Get("content-type") {
+       // Case of images not having a multi-architecture manifest, or wrapping a v1 API answer
+       case "application/vnd.docker.distribution.manifest.v2+json":
+               return strings.TrimPrefix(res.Header.Get("Docker-Content-Digest"), "sha256:"), nil
+
        // Standalone OCI manifest (only one architecture provided by the registry)
        case "application/vnd.oci.image.manifest.v1+json":
                // Note that there are no check against the image's architecture

Then both work quay.io and dockerhub

My understanding is application/vnd.oci.image.index.v1+json is equivalent to application/vnd.docker.distribution.manifest.v2+json but dockerhub only understand the later one

@olblak
Copy link
Member

olblak commented Apr 20, 2022

@olblak
Copy link
Member

olblak commented Apr 20, 2022

I need more investigation.
For sharing purpose, I am testing using this manifest

@dduportal
Copy link
Contributor

I did some experimentation and it seems to introduce a regression for dockerhub

For example jenkins/jenkins:latest would return

ERROR: ✗ Unsupported response type from the registry registry-1.docker.io: Content-Type "application/vnd.docker.distribution.manifest.v1+prettyjws" is either deprecated or unknown by updatecli.

But if I put back

--- a/pkg/plugins/utils/docker/dockerregistry/main.go
+++ b/pkg/plugins/utils/docker/dockerregistry/main.go
@@ -71,6 +71,7 @@ func (dgr DockerGenericRegistry) Digest(image dockerimage.Image) (string, error)
        //  * Docker Registry V2 manifest list
        //  * List of OCI manifests (multiple architectures provided by the registry)
        //  * Standalone OCI manifest (only one architecture provided by the registry)
+       req.Header.Add("Accept", "application/vnd.docker.distribution.manifest.v2+json")
        req.Header.Add("Accept", "application/vnd.docker.distribution.manifest.list.v2+json")
        req.Header.Add("Accept", "application/vnd.oci.image.index.v1+json")
        req.Header.Add("Accept", "application/vnd.oci.image.manifest.v1+json")
@@ -119,6 +120,10 @@ func (dgr DockerGenericRegistry) Digest(image dockerimage.Image) (string, error)
        }
 
        switch res.Header.Get("content-type") {
+       // Case of images not having a multi-architecture manifest, or wrapping a v1 API answer
+       case "application/vnd.docker.distribution.manifest.v2+json":
+               return strings.TrimPrefix(res.Header.Get("Docker-Content-Digest"), "sha256:"), nil
+
        // Standalone OCI manifest (only one architecture provided by the registry)
        case "application/vnd.oci.image.manifest.v1+json":
                // Note that there are no check against the image's architecture

Then both work quay.io and dockerhub

My understanding is application/vnd.oci.image.index.v1+json is equivalent to application/vnd.docker.distribution.manifest.v2+json but dockerhub only understand the later one

Try another image than jenkins/jenkins because we are dealing with an issue in its publication right now: jenkins-infra/helpdesk#2890

Signed-off-by: Olblak <me@olblak.com>
@olblak
Copy link
Member

olblak commented Apr 21, 2022

@nmasse-itix I opened this PR nmasse-itix#1 on your fork.

@dduportal
Copy link
Contributor

@nmasse-itix @olblak you're on fire folks, many thanks for taking care of this! (sorry I'm really busy these days did not have enough time to review/test)

Add dockerhub registry v2 support back
@olblak
Copy link
Member

olblak commented Apr 21, 2022

For information, the linter issues appears because I recently updated the golangci-linter version, in this pullrquest https://github.com/updatecli/updatecli/pull/652/files

I'll fix the warning in a different pullrequest

@olblak olblak added resource-dockerdigest Resource of kind dockerDigest enhancement New feature or request labels Apr 21, 2022
@olblak olblak modified the milestone: 0.24.0 Apr 21, 2022
@olblak
Copy link
Member

olblak commented Apr 21, 2022

I successfully tested this pullrequest and I get the following result.
From my point of view, it's ready to be merged so we can have it in release 0.24.0

+++++++++++
+ PREPARE +
+++++++++++

Loading Pipeline "examples/updatecli.generic/dockerdigest.yaml"

SCM repository retrieved: 0


++++++++++++
+ PIPELINE +
++++++++++++



############################################################
# SHOW A SET OF DOCKERIMAGE RESOURCES AS A GENERIC EXAMPLE #
############################################################


SOURCES
=======

jenkinsDockerHub
----------------
✔ Digest "545a48328e879154de809212c2c86273142961ec0583c5bd4c731230e9228fa9" found for the docker image registry-1.docker.io/jenkins/jenkins:lts.
	Remark: Do not forget to add @sha256 after your the docker image name
	Example: jenkins/jenkins@sha256:545a48328e879154de809212c2c86273142961ec0583c5bd4c731230e9228fa9

jenkinsAnonymousDockerHub
-------------------------
✔ Digest "9405b917836c1d4a073e41a3413808fd5bdb6ac230da84fd4739424830d7c32f" found for the docker image registry-1.docker.io/jenkins/jenkins:latest.
	Remark: Do not forget to add @sha256 after your the docker image name
	Example: jenkins/jenkins@sha256:9405b917836c1d4a073e41a3413808fd5bdb6ac230da84fd4739424830d7c32f

digestOfLatestUpdateCliOnGhcr
-----------------------------
✔ Digest "63d9c31ee6c27c104c8c6d381739c3d1634ae1a8bf2020cd167b4f56327044f6" found for the docker image ghcr.io/updatecli/updatecli:latest.
	Remark: Do not forget to add @sha256 after your the docker image name
	Example: ghcr.io/updatecli/updatecli@sha256:63d9c31ee6c27c104c8c6d381739c3d1634ae1a8bf2020cd167b4f56327044f6

digestOfLatestQuayAnsibleRunner
-------------------------------
WARNING: No username and/or password specified: trying to retrieve a token as anonymous user might not be supported or could impact the rate limiting on your network.
✔ Digest "e5d73a079780d28d3c2b86c67a594603ccb6621384ed0820f7f6d5d8b7b4589f" found for the docker image quay.io/ansible/ansible-runner:latest.
	Remark: Do not forget to add @sha256 after your the docker image name
	Example: quay.io/ansible/ansible-runner@sha256:e5d73a079780d28d3c2b86c67a594603ccb6621384ed0820f7f6d5d8b7b4589f

digestOfLatestUpdateCliARMDockerHub
-----------------------------------
✔ Digest "7d4ac50f1049e0d9db6b0547980b53be06ede8958d6aa7cdef0476657955613f" found for the docker image registry-1.docker.io/updatecli/updatecli:latest.
	Remark: Do not forget to add @sha256 after your the docker image name
	Example: updatecli/updatecli@sha256:7d4ac50f1049e0d9db6b0547980b53be06ede8958d6aa7cdef0476657955613f

nginxOfficialDockerHub
----------------------
✔ Digest "efc09388b15fb423c402f0b8b28ca70c7fd20fe31f8d7531ae1896bbb4944999" found for the docker image registry-1.docker.io/library/nginx:alpine.
	Remark: Do not forget to add @sha256 after your the docker image name
	Example: nginx@sha256:efc09388b15fb423c402f0b8b28ca70c7fd20fe31f8d7531ae1896bbb4944999

=============================

REPORTS:


- DOCKERDIGEST.YAML:
	Sources:
		✔ [digestOfLatestQuayAnsibleRunner] Get digest of the latest quay.io/ansible/ansible-runner image on Quay.io (kind: dockerDigest)
		✔ [digestOfLatestUpdateCliARMDockerHub] Get digest of the latest updatecli/updatecli image on the DockerHub (kind: dockerDigest)
		✔ [digestOfLatestUpdateCliOnGhcr] Get digest of the latest updatecli/updatecli image on ghcr.io (kind: dockerDigest)
		✔ [jenkinsAnonymousDockerHub] Get digest of the latest jenkins/jenkins image on the DockerHub (kind: dockerDigest)
		✔ [jenkinsDockerHub] Get digest of the latest jenkins/jenkins:lts image on the DockerHub (kind: dockerDigest)
		✔ [nginxOfficialDockerHub] Get digest of the latest nginx:alpine image on the DockerHub (kind: dockerDigest)
	Target:



Run Summary
===========
Pipeline(s) run:
  * Changed:	0
  * Failed:	0
  * Skipped:	1
  * Succeeded:	0
  * Total:	1

Copy link
Member

@olblak olblak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot @nmasse-itix for the pull request.

@olblak olblak closed this Apr 22, 2022
@olblak olblak reopened this Apr 22, 2022
@olblak olblak merged commit caf6dcd into updatecli:main Apr 22, 2022
@nmasse-itix nmasse-itix deleted the oci-registry branch April 22, 2022 11:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request resource-dockerdigest Resource of kind dockerDigest
Projects
None yet
3 participants