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

fix: disable oras tag fallback to tag schema when tagging a referrer #1435

Merged

Conversation

njucjc
Copy link
Contributor

@njucjc njucjc commented Jun 28, 2024

What this PR does / why we need it:

  • disable oras tag fallback to tag schema when tagging a referrer

Please check the following list:

  • Does the affected code have corresponding tests, e.g. unit test, E2E test?
  • Does this change require a documentation update?
  • Does this introduce breaking changes that would require an announcement or bumping the major version?
  • Do all new files have an appropriate license header?

Signed-off-by: njucjc <njucjc@gmail.com>
cmd/oras/root/tag.go Outdated Show resolved Hide resolved
@qweeah
Copy link
Contributor

qweeah commented Jun 28, 2024

Hi @njucjc, thanks for contributing. Can you help explain why --distribution-spec flag need to be added to oras tag?

@njucjc
Copy link
Contributor Author

njucjc commented Jun 28, 2024

Hi @njucjc, thanks for contributing. Can you help explain why --distribution-spec flag need to be added to oras tag?

@qweeah I want to tag a referrer, but now if not specify --distribution-spec v1.1-referrers-api, it will fallback to --distribution-spec v1.1-referrers-tag method. The result is that an unexpected addition of a sha256 subject tag in addition to the one I specified.

@qweeah
Copy link
Contributor

qweeah commented Jun 28, 2024

I want to tag a referrer, but now if not specify --distribution-spec v1.1-referrers-api, it will fallback to --distribution-spec v1.1-referrers-tag method.

That's weird because --distribution-spec should only work for oras attach when adding a referrer. Can you kindly share the error debug log and which registry you are using?

@njucjc
Copy link
Contributor Author

njucjc commented Jun 28, 2024

That's weird because --distribution-spec should only work for oras attach when adding a referrer. Can you kindly share the error debug log and which registry you are using?

@qweeah

DEBU[0000] Request #0
> Request method: "GET"
> Request headers:
   "Accept": "application/vnd.docker.distribution.manifest.v2+json, application/vnd.docker.distribution.manifest.list.v2+json, application/vnd.oci.image.manifest.v1+json, application/vnd.oci.image.index.v1+json, application/vnd.oci.artifact.manifest.v1+json"
   "User-Agent": "oras/1.2.0"
DEBU[0000] Response #0
< Response Status: "401 Unauthorized"
< Response headers:
   "Content-Length": "157"
   "Docker-Distribution-Api-Version": "registry/2.0"
   "Date": "Fri, 28 Jun 2024 14:31:27 GMT"
   "Content-Type": "application/json; charset=utf-8"
DEBU[0000] Request #1
> Request method: "GET"
> Request headers:
   "Authorization": "*****"
   "User-Agent": "oras/1.2.0"
DEBU[0000] Response #1
< Response Status: "200 OK"
< Response headers:
   "X-Xss-Protection": "1; mode=block"
   "Cache-Control": "no-cache, no-store, max-age=0, must-revalidate"
   "Pragma": "no-cache"
   "Timing-Allow-Origin": "*"
   "Server": "Tengine"
   "Vary": "Accept-Encoding"
   "X-Content-Type-Options": "nosniff"
   "Date": "Fri, 28 Jun 2024 14:31:27 GMT"
   "Strict-Transport-Security": "max-age=31536000"
   "Eagleeye-Traceid": "0a06dff517195850875481284ea292"
   "Content-Type": "application/json;charset=UTF-8"
   "Set-Cookie": "*****"
   "Expires": "0"
   "X-Frame-Options": "DENY"
DEBU[0000] Request #2
> Request URL: "https:/xxxx/manifests/sha256:bd6e6c6fc03d1f15108de17531082501997335eacfa7ec8cb805430e35fc57eb"
> Request method: "GET"
> Request headers:
   "Accept": "application/vnd.docker.distribution.manifest.v2+json, application/vnd.docker.distribution.manifest.list.v2+json, application/vnd.oci.image.manifest.v1+json, application/vnd.oci.image.index.v1+json, application/vnd.oci.artifact.manifest.v1+json"
   "Authorization": "*****"
   "User-Agent": "oras/1.2.0"
DEBU[0000] Response #2
< Response Status: "200 OK"
< Response headers:
   "Date": "Fri, 28 Jun 2024 14:31:27 GMT"
   "Content-Type": "application/vnd.oci.image.index.v1+json"
   "Content-Length": "736"
   "Docker-Content-Digest": "sha256:bd6e6c6fc03d1f15108de17531082501997335eacfa7ec8cb805430e35fc57eb"
   "Docker-Distribution-Api-Version": "registry/2.0"
   "Etag": "\"sha256:bd6e6c6fc03d1f15108de17531082501997335eacfa7ec8cb805430e35fc57eb\""
Tagging [registry] xxxxx/library/test-2@sha256:bd6e6c6fc03d1f15108de17531082501997335eacfa7ec8cb805430e35fc57eb
DEBU[0000] Request #3
> Request URL: "https://xxxxx/v2/library/test-2/manifests/test"
> Request method: "PUT"
> Request headers:
   "Content-Type": "application/vnd.oci.image.index.v1+json"
   "User-Agent": "oras/1.2.0"
DEBU[0000] Response #3
< Response Status: "401 Unauthorized"
< Response headers:
   "Content-Length": "230"
   "Docker-Distribution-Api-Version": "registry/2.0"
   "Www-Authenticate": "Bearer realm=\
   "Date": "Fri, 28 Jun 2024 14:31:27 GMT"
   "Content-Type": "application/json; charset=utf-8"
DEBU[0000] Request #4
> Request URL: 
> Request method: "GET"
> Request headers:
   "Authorization": "*****"
   "User-Agent": "oras/1.2.0"
DEBU[0001] Response #4
< Response Status: "200 OK"
< Response headers:
   "Date": "Fri, 28 Jun 2024 14:31:28 GMT"
   "X-Frame-Options": "DENY"
   "Strict-Transport-Security": "max-age=31536000"
   "Set-Cookie": "*****"
   "X-Xss-Protection": "1; mode=block"
   "Cache-Control": "no-cache, no-store, max-age=0, must-revalidate"
   "Pragma": "no-cache"
   "Expires": "0"
   "Eagleeye-Traceid": "0a06dff517195850879751310ea292"
   "Timing-Allow-Origin": "*"
   "Server": "Tengine"
   "Content-Type": "application/json;charset=UTF-8"
   "Vary": "Accept-Encoding"
   "X-Content-Type-Options": "nosniff"
DEBU[0001] Request #5
> Request URL: "https://xxxx/v2/library/test-2/manifests/test"
> Request method: "PUT"
> Request headers:
   "Content-Type": "application/vnd.oci.image.index.v1+json"
   "Authorization": "*****"
   "User-Agent": "oras/1.2.0"
DEBU[0001] Response #5
< Response Status: "201 Created"
< Response headers:
   "Date": "Fri, 28 Jun 2024 14:31:28 GMT"
   "Content-Length": "0"
   "Location": "https://xxxx/v2/library/test-2/manifests/sha256:bd6e6c6fc03d1f15108de17531082501997335eacfa7ec8cb805430e35fc57eb"
   "Docker-Content-Digest": "sha256:bd6e6c6fc03d1f15108de17531082501997335eacfa7ec8cb805430e35fc57eb"
   "Docker-Distribution-Api-Version": "registry/2.0"
DEBU[0001] Request #6
> Request URL: "https://xxxxxxx/v2/library/test-2/manifests/sha256-9eaf39d0c368f3ea629401810641cb677c412953e9d0f28e8b26f6cd8653cfde"
> Request method: "GET"
> Request headers:
   "User-Agent": "oras/1.2.0"
   "Accept": "application/vnd.docker.distribution.manifest.v2+json, application/vnd.docker.distribution.manifest.list.v2+json, application/vnd.oci.image.manifest.v1+json, application/vnd.oci.image.index.v1+json, application/vnd.oci.artifact.manifest.v1+json"
   "Authorization": "*****"
DEBU[0001] Response #6
< Response Status: "200 OK"
< Response headers:
   "Date": "Fri, 28 Jun 2024 14:31:28 GMT"
   "Content-Type": "application/vnd.oci.image.index.v1+json"
   "Content-Length": "302"
   "Docker-Content-Digest": "sha256:ce9946afdcd5298cc6da3940e2ebee9583e1c87a71d9ad2e5e0d2792f59626d5"
   "Docker-Distribution-Api-Version": "registry/2.0"
   "Etag": "\"sha256:ce9946afdcd5298cc6da3940e2ebee9583e1c87a71d9ad2e5e0d2792f59626d5\""
Tagged test

the registry (support referrers api as distribution-spec v1.1) appears two tags: test and sha256-9eaf39d0c368f3ea629401810641cb677c412953e9d0f28e8b26f6cd8653cfde

By the way,oras cp subcommand will also use --from-distribution-spec and --to-distribution-spec flag to indicate that the remote registries support the Referrers API.

Copy link

codecov bot commented Jun 28, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.22%. Comparing base (ce758bc) to head (8bd4797).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1435      +/-   ##
==========================================
- Coverage   85.27%   85.22%   -0.05%     
==========================================
  Files         106      106              
  Lines        3796     3798       +2     
==========================================
  Hits         3237     3237              
- Misses        334      336       +2     
  Partials      225      225              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@qweeah
Copy link
Contributor

qweeah commented Jun 28, 2024

the registry (support referrers api as distribution-spec v1.1) appears two tags: test and sha256-9eaf39d0c368f3ea629401810641cb677c412953e9d0f28e8b26f6cd8653cfde

Just to confirm, so you are trying to avoid generating sha256-9eaf39d0c368f3ea629401810641cb677c412953e9d0f28e8b26f6cd8653cfde and oras tag --distribution-spec v1.1-referrers-api is the desired behavior in your case, right?

@shizhMSFT
Copy link
Contributor

Although tagging a referrer artifact manifest is discouraged, it is still doable. However, re-tagging is implemented as pushing the exact manifest again to the remote server. According to the distribution-spec, if the remote server does not support referrers API, it will go for the referrers tag schema case. However, it is not necessary since the manifest is already there, we are just adding a new tag not really pushing a new manifest.

In summary, tagging a referrers manifest should never go for the tag schema even if the server does not support referrers API since the referrer manifest is already there on the server.

@njucjc
Copy link
Contributor Author

njucjc commented Jun 28, 2024

the registry (support referrers api as distribution-spec v1.1) appears two tags: test and sha256-9eaf39d0c368f3ea629401810641cb677c412953e9d0f28e8b26f6cd8653cfde

Just to confirm, so you are trying to avoid generating sha256-9eaf39d0c368f3ea629401810641cb677c412953e9d0f28e8b26f6cd8653cfde and oras tag --distribution-spec v1.1-referrers-api is the desired behavior in your case, right?

@qweeah Yes!

@njucjc
Copy link
Contributor Author

njucjc commented Jun 28, 2024

Although tagging a referrer artifact manifest is discouraged, it is still doable. However, re-tagging is implemented as pushing the exact manifest again to the remote server. According to the distribution-spec, if the remote server does not support referrers API, it will go for the referrers tag schema case. However, it is not necessary since the manifest is already there, we are just adding a new tag not really pushing a new manifest.

In summary, tagging a referrers manifest should never go for the tag schema even if the server does not support referrers API since the referrer manifest is already there on the server.

@shizhMSFT @qweeah Is there any suggestions to fix it? The simplest way to disable the tag schema is set referrersAPI = true inside the registry client in oras tag subcommand.

…rrers

Signed-off-by: Billy Zha <jinzha1@microsoft.com>
@qweeah
Copy link
Contributor

qweeah commented Jun 29, 2024

@shizhMSFT @qweeah Is there any suggestions to fix it? The simplest way to disable the tag schema is set referrersAPI = true inside the registry client in oras tag subcommand.

@njucjc Yes, we should disable tag schema without adding --distribution-spec to oras tag. If you are interested in fixing it, you may achieve that using https://pkg.go.dev/oras.land/oras-go/v2@v2.5.0/registry/remote#Repository.SetReferrersCapability.

qweeah added 3 commits June 29, 2024 04:24
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
@qweeah
Copy link
Contributor

qweeah commented Jun 29, 2024

I have added two test specs, one of them ensures that tag schema is not used after tagging to an OCI v1.0 registry (which doesn't support referrers API). @njucjc With a proper fix in oras tag the test specs should pass.

@njucjc njucjc changed the title feat: support oras tag enable distribution spec flag fix: disable oras tag fallback to tag schema when tagging a referrer Jul 1, 2024
@njucjc
Copy link
Contributor Author

njucjc commented Jul 1, 2024

I have added two test specs, one of them ensures that tag schema is not used after tagging to an OCI v1.0 registry (which doesn't support referrers API). @njucjc With a proper fix in oras tag the test specs should pass.

@qweeah PTAL

@njucjc njucjc force-pushed the oras-tag-support-distribution-spec-flag branch from 7599361 to e19fdea Compare July 1, 2024 02:05
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Copy link
Contributor

@qweeah qweeah left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @njucjc

Copy link
Contributor

@qweeah qweeah left a comment

Choose a reason for hiding this comment

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

Oh one required change I have missed: the type assertion should be done after the error handling.

cmd/oras/root/tag.go Outdated Show resolved Hide resolved
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
@njucjc njucjc force-pushed the oras-tag-support-distribution-spec-flag branch from 46b1785 to 8b7d60e Compare July 1, 2024 05:49
Copy link
Contributor

@qweeah qweeah left a comment

Choose a reason for hiding this comment

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

LGTM

qweeah added 3 commits July 1, 2024 08:36
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Copy link
Contributor

@shizhMSFT shizhMSFT left a comment

Choose a reason for hiding this comment

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

LGTM

@qweeah qweeah merged commit ecb7878 into oras-project:main Jul 1, 2024
8 checks passed
@FeynmanZhou FeynmanZhou linked an issue Jul 2, 2024 that may be closed by this pull request
qweeah added a commit to qweeah/oras that referenced this pull request Dec 5, 2024
…ras-project#1435)

Signed-off-by: njucjc <njucjc@gmail.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Co-authored-by: Billy Zha <jinzha1@microsoft.com>
qweeah added a commit to qweeah/oras that referenced this pull request Dec 5, 2024
…ras-project#1435)

Signed-off-by: njucjc <njucjc@gmail.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Co-authored-by: Billy Zha <jinzha1@microsoft.com>
qweeah added a commit to qweeah/oras that referenced this pull request Dec 5, 2024
…ras-project#1435)

Signed-off-by: njucjc <njucjc@gmail.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Co-authored-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

oras tag should not create referrers tags
4 participants