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

Add option to refresh (current behaviour) or keep previous headers when fetching manifests #123

Merged
merged 3 commits into from
Jan 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ and **Merged pull requests**. Critical items to know are:
The versions coincide with releases on pip. Only major versions will be released as tags on Github.

## [0.0.x](https://github.com/oras-project/oras-py/tree/main) (0.0.x)
- Introduce the option to not refresh headers when fetching manifests when pulling artifacts (0.1.27)
- To make it available for more OCI registries, the value of config used when `manifest_config` is not specified in `client.push()` has been changed from a pure empty string to `{}` (0.1.26)
- refactor tests using fixtures and rework pre-commit configuration (0.1.25)
- eliminate the additional subdirectory creation while pulling an image to a custom output directory (0.1.24)
Expand Down
23 changes: 19 additions & 4 deletions oras/provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -757,8 +757,10 @@ def push(self, *args, **kwargs) -> requests.Response:

# Config is just another layer blob!
logger.debug(f"Preparing config {conf}")
with temporary_empty_config() if config_file is None else nullcontext(
config_file
with (
temporary_empty_config()
if config_file is None
else nullcontext(config_file)
) as config_file:
response = self.upload_blob(config_file, container, conf)

Expand All @@ -780,6 +782,8 @@ def pull(self, *args, **kwargs) -> List[str]:
:type allowed_media_type: list or None
:param overwrite: if output file exists, overwrite
:type overwrite: bool
:param refresh_headers: if true, headers are refreshed when fetching manifests
:type refresh_headers: bool
:param manifest_config_ref: save manifest config to this file
:type manifest_config_ref: str
:param outdir: output directory path
Expand All @@ -788,9 +792,12 @@ def pull(self, *args, **kwargs) -> List[str]:
:type target: str
"""
allowed_media_type = kwargs.get("allowed_media_type")
refresh_headers = kwargs.get("refresh_headers")
if refresh_headers is None:
refresh_headers = True
container = self.get_container(kwargs["target"])
self.load_configs(container, configs=kwargs.get("config_path"))
manifest = self.get_manifest(container, allowed_media_type)
manifest = self.get_manifest(container, allowed_media_type, refresh_headers)
Copy link

Choose a reason for hiding this comment

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

This is a breaking change that has broken depscan. It shouldn't have been released as a patch.

Copy link
Contributor

Choose a reason for hiding this comment

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

Would you care to elaborate on that? This change adds no new dependencies, and (without specifying anything) the default behavior is the same as before.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah I see, you maintain a project, depscan https://github.com/owasp-dep-scan/dep-scan/actions/runs/7695237445/job/20967667418#step:7:44 and you have a custom class that reimplements the function https://github.com/owasp-dep-scan/dep-scan/blob/db4988ca72e6a6c28befec378191f62b2ab340bc/depscan/lib/orasclient.py#L25. You just need to add an additional argument there.

We can't always anticipate how a user of the library is going to customize a class for their needs. For your implementation it looks like it's a mirror copy but you removed validation. My suggestion would be to not re-implement the function but request a parameter that might more easily turn off the validation if you don't want it.

outdir = kwargs.get("outdir") or oras.utils.get_tmpdir()
overwrite = kwargs.get("overwrite", True)

Expand Down Expand Up @@ -830,7 +837,10 @@ def pull(self, *args, **kwargs) -> List[str]:

@decorator.ensure_container
def get_manifest(
self, container: container_type, allowed_media_type: Optional[list] = None
self,
container: container_type,
allowed_media_type: Optional[list] = None,
refresh_headers: bool = True,
) -> dict:
"""
Retrieve a manifest for a package.
Expand All @@ -839,11 +849,16 @@ def get_manifest(
:type container: oras.container.Container or str
:param allowed_media_type: one or more allowed media types
:type allowed_media_type: str
:param refresh_headers: if true, headers are refreshed
:type refresh_headers: bool
"""
if not allowed_media_type:
allowed_media_type = [oras.defaults.default_manifest_media_type]
headers = {"Accept": ";".join(allowed_media_type)}

if not refresh_headers:
headers.update(self.headers)

get_manifest = f"{self.prefix}://{container.manifest_url()}" # type: ignore
response = self.do_request(get_manifest, "GET", headers=headers)
self._check_200_response(response)
Expand Down
2 changes: 1 addition & 1 deletion oras/version.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
__copyright__ = "Copyright The ORAS Authors."
__license__ = "Apache-2.0"

__version__ = "0.1.26"
__version__ = "0.1.27"
AUTHOR = "Vanessa Sochat"
EMAIL = "[email protected]"
NAME = "oras"
Expand Down
Loading