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

Minor fixes and support for Subject #89

Merged
merged 5 commits into from
Jul 26, 2023

Conversation

dev-zero
Copy link
Contributor

  • registry: disable cookies
  • schema: the subject element consists is an object (if present)
  • provider: add argument to pass a subject

@vsoch
Copy link
Contributor

vsoch commented Jun 28, 2023

@dev-zero this looks good - a few tweaks:

  • you'll need black black-23.3.0 for pre-commit to run locally and fix the formatting errors
  • please bump the version in oras/version.py
  • add a corresponding note about the changes in CHANGELOG.md

@dev-zero
Copy link
Contributor Author

@vsoch do you mind if I "fix" pre-commit to install the tools isolated rather than trying to run locally?
flake8 fails here as well due to typing issues and may need to be pinned to an older version.

@dev-zero
Copy link
Contributor Author

... and while being at it, switch from flake8 to ruff?

@vsoch
Copy link
Contributor

vsoch commented Jun 29, 2023

I think I’d prefer keeping the current linting for now - at least for this PR. I’m open to changes but it’s out of scope for the work here.

@dev-zero
Copy link
Contributor Author

done

@dev-zero
Copy link
Contributor Author

I found yet another issue when working on/with Subject: the referrer API gives back digests for the referenced manifests, but manifest_uri() did not consider digests.

@vsoch
Copy link
Contributor

vsoch commented Jun 29, 2023

I found yet another issue when working on/with Subject: the referrer API gives back digests for the referenced manifests, but manifest_uri() did not consider digests.

The specs are changing and the registries differ in implementations, so definitely there could be missing pieces. Shall we finish up the work here and work on this in another scoped PR? Either way, you'll still need to bump the version and update the CHANGELOG.md as I mentioned earlier. Thanks!

@dev-zero dev-zero force-pushed the bugfix/subject branch 2 times, most recently from e82608a to d681083 Compare July 11, 2023 17:59
@dev-zero
Copy link
Contributor Author

done, @vsoch

Copy link
Contributor

@vsoch vsoch left a comment

Choose a reason for hiding this comment

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

One tiny tweak for the CHANGELOG then LGTM!

CHANGELOG.md Outdated Show resolved Hide resolved
@vsoch
Copy link
Contributor

vsoch commented Jul 11, 2023

Also - if formatting keeps failing I'd be happy to push a commit for you (just let me know).

Required to properly interact with some registries like Harbor which are otherwise trying to set and validate CSRF headers which are not passed along against hidden cookie values. This chainreaction starts with oras-py accepting the sid cookie.

Signed-off-by: Tiziano Müller <tiziano.mueller@hpe.com>
Signed-off-by: Tiziano Müller <tiziano.mueller@hpe.com>
Signed-off-by: Tiziano Müller <tiziano.mueller@hpe.com>
Not doing so makes it impossible to pull() an artifact via digest (and simply passing in a digest as a tag will confuse the regex parser up to the point it will try to fetch the blobs from docker.io

Signed-off-by: Tiziano Müller <tiziano.mueller@hpe.com>
Signed-off-by: Tiziano Müller <tiziano.mueller@hpe.com>
@dev-zero
Copy link
Contributor Author

Ok, merged and rebased @vsoch

@vsoch vsoch merged commit d6bdef5 into oras-project:main Jul 26, 2023
5 checks passed
@dev-zero dev-zero deleted the bugfix/subject branch July 26, 2023 12:55
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.

None yet

2 participants