Update headers in do_request method, provider.py #129

my5cents · 2024-04-03T16:15:19Z

I work with remote registry, available via http URL. I set basic auth and tried to do some operations within ORAS. I was able to get tags, manifest and make pull, but push operation failed. Debugging showed that during the push operation POST and PUT methods loose provided auth headers. It is crucial for remote registry, since further re-auth logic tries to reach registry locally. Below is my log and truncated stack trace (it's too long):

2024-04-03 18:19:09.422       oras_manager:40  INFO: ['0.0.1', '0.1.1', 'latest']
2024-04-03 18:19:11.818             logger:246 INFO: Retrying in 3 seconds - error: HTTPConnectionPool(host='docker-registry-realm', port=80): Max retries exceeded with url: / (Caused by NameResolutionError("<urllib3.connection.HTTPConnection object at 0x00000168BBEDA960>: Failed to resolve 'docker-registry-realm' ([Errno 11001] getaddrinfo failed)"))
2024-04-03 18:19:15.878             logger:246 INFO: Retrying in 5 seconds - error: HTTPConnectionPool(host='docker-registry-realm', port=80): Max retries exceeded with url: / (Caused by NameResolutionError("<urllib3.connection.HTTPConnection object at 0x00000168BBED93D0>: Failed to resolve 'docker-registry-realm' ([Errno 11001] getaddrinfo failed)"))
2024-04-03 18:19:21.954             logger:246 INFO: Retrying in 11 seconds - error: HTTPConnectionPool(host='docker-registry-realm', port=80): Max retries exceeded with url: / (Caused by NameResolutionError("<urllib3.connection.HTTPConnection object at 0x00000168BBEDAED0>: Failed to resolve 'docker-registry-realm' ([Errno 11001] getaddrinfo failed)"))
2024-04-03 18:19:34.029             logger:246 INFO: Retrying in 29 seconds - error: HTTPConnectionPool(host='docker-registry-realm', port=80): Max retries exceeded with url: / (Caused by NameResolutionError("<urllib3.connection.HTTPConnection object at 0x00000168BBEDB950>: Failed to resolve 'docker-registry-realm' ([Errno 11001] getaddrinfo failed)"))
2024-04-03 18:20:04.108             logger:246 INFO: Retrying in 83 seconds - error: HTTPConnectionPool(host='docker-registry-realm', port=80): Max retries exceeded with url: / (Caused by NameResolutionError("<urllib3.connection.HTTPConnection object at 0x00000168BBF28290>: Failed to resolve 'docker-registry-realm' ([Errno 11001] getaddrinfo failed)"))
FAILED                                                                   [100%]Retrying in 3 seconds - error: HTTPConnectionPool(host='docker-registry-realm', port=80): Max retries exceeded with url: / (Caused by NameResolutionError("<urllib3.connection.HTTPConnection object at 0x00000168BBEDA960>: Failed to resolve 'docker-registry-realm' ([Errno 11001] getaddrinfo failed)"))
Retrying in 5 seconds - error: HTTPConnectionPool(host='docker-registry-realm', port=80): Max retries exceeded with url: / (Caused by NameResolutionError("<urllib3.connection.HTTPConnection object at 0x00000168BBED93D0>: Failed to resolve 'docker-registry-realm' ([Errno 11001] getaddrinfo failed)"))
Retrying in 11 seconds - error: HTTPConnectionPool(host='docker-registry-realm', port=80): Max retries exceeded with url: / (Caused by NameResolutionError("<urllib3.connection.HTTPConnection object at 0x00000168BBEDAED0>: Failed to resolve 'docker-registry-realm' ([Errno 11001] getaddrinfo failed)"))
Retrying in 29 seconds - error: HTTPConnectionPool(host='docker-registry-realm', port=80): Max retries exceeded with url: / (Caused by NameResolutionError("<urllib3.connection.HTTPConnection object at 0x00000168BBEDB950>: Failed to resolve 'docker-registry-realm' ([Errno 11001] getaddrinfo failed)"))
Retrying in 83 seconds - error: HTTPConnectionPool(host='docker-registry-realm', port=80): Max retries exceeded with url: / (Caused by NameResolutionError("<urllib3.connection.HTTPConnection object at 0x00000168BBF28290>: Failed to resolve 'docker-registry-realm' ([Errno 11001] getaddrinfo failed)"))

.....

tests\smoke_tests\test_registry_smoke.py:32: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
libs\oras\oras_manager.py:47: in __init__
    res = registry.get_manifest('busybox')
..\..\AppData\Local\Programs\Python\Python312\Lib\site-packages\oras\decorator.py:35: in __call__
    return self.func(cls, *args, **kwargs)
..\..\AppData\Local\Programs\Python\Python312\Lib\site-packages\oras\provider.py:864: in get_manifest
    response = self.do_request(get_manifest, "GET", headers=headers)
..\..\AppData\Local\Programs\Python\Python312\Lib\site-packages\oras\decorator.py:60: in __call__
    return self.func(cls, *args, **kwargs)
..\..\AppData\Local\Programs\Python\Python312\Lib\site-packages\oras\provider.py:914: in do_request
    if self.authenticate_request(response):
..\..\AppData\Local\Programs\Python\Python312\Lib\site-packages\oras\provider.py:1004: in authenticate_request
    authResponse = self.session.get(h.realm, headers=headers, params=params)  # type: ignore
..\..\AppData\Local\Programs\Python\Python312\Lib\site-packages\requests\sessions.py:602: in get
    return self.request("GET", url, **kwargs)
..\..\AppData\Local\Programs\Python\Python312\Lib\site-packages\requests\sessions.py:589: in request
    resp = self.send(prep, **send_kwargs)
..\..\AppData\Local\Programs\Python\Python312\Lib\site-packages\requests\sessions.py:703: in send
    r = adapter.send(request, **kwargs)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <requests.adapters.HTTPAdapter object at 0x00000168BBED9250>
request = <PreparedRequest [GET]>, stream = False
timeout = Timeout(connect=None, read=None, total=None), verify = True
cert = None, proxies = OrderedDict()

    def send(
        self, request, stream=False, timeout=None, verify=True, cert=None, proxies=None
    ):

.....
    
>           raise ConnectionError(e, request=request)
E           requests.exceptions.ConnectionError: HTTPConnectionPool(host='docker-registry-realm', port=80): Max retries exceeded with url: / (Caused by NameResolutionError("<urllib3.connection.HTTPConnection object at 0x00000168BBF28BC0>: Failed to resolve 'docker-registry-realm' ([Errno 11001] getaddrinfo failed)"))

..\..\AppData\Local\Programs\Python\Python312\Lib\site-packages\requests\adapters.py:519: ConnectionError

Process finished with exit code 1

If do_request method is updated to include provided 'Authorization' header, push works fine as well as other operations:

2024-04-03 18:26:18.777       oras_manager:40  INFO: ['0.0.1', '0.1.1', 'latest']
2024-04-03 18:26:18.866             logger:252 DEBUG: Preparing layer {'mediaType': 'application/vnd.oci.image.layer.v1.tar+gzip', 'size': 2939, 'digest': 'sha256:1532aa4a0fb892d0091e7791d31e438f344d4b62dec6f8cb5f21afd8eee05849', 'annotations': {'org.opencontainers.image.title': 'busybox'}}
2024-04-03 18:26:19.443             logger:252 DEBUG: Preparing config {'mediaType': 'application/vnd.unknown.config.v1+json', 'size': 2, 'digest': 'sha256:44136fa355b3678a1146ad16f7e8649e94fb4fc21fe77e8310c060f61caaff8a'}
2024-04-03 18:26:20.086       oras_manager:50  INFO: Push response: <Response [201]>
2024-04-03 18:26:20.252             logger:246 INFO: Successfully pulled C:\Users\my5cents\AppData\Local\Temp\oras-tmp.i0s7_bfz\busybox.
2024-04-03 18:26:20.252       oras_manager:54  INFO: Pull response: ['C:\\Users\\my5cents\\AppData\\Local\\Temp\\oras-tmp.i0s7_bfz\\busybox']
PASSED                                                                   [100%]Preparing layer {'mediaType': 'application/vnd.oci.image.layer.v1.tar+gzip', 'size': 2939, 'digest': 'sha256:1532aa4a0fb892d0091e7791d31e438f344d4b62dec6f8cb5f21afd8eee05849', 'annotations': {'org.opencontainers.image.title': 'busybox'}}
Preparing config {'mediaType': 'application/vnd.unknown.config.v1+json', 'size': 2, 'digest': 'sha256:44136fa355b3678a1146ad16f7e8649e94fb4fc21fe77e8310c060f61caaff8a'}
Successfully pushed my.remote.registry.domain.edu/busybox:latest
Successfully pulled C:\Users\my5cents\AppData\Local\Temp\oras-tmp.i0s7_bfz\busybox.

Process finished with exit code 0

During the push operation POST and PUT methods loose provided auth headers. It is crucial for remote registry, since further re-auth logic tries to reach registry locally. do_request method is updated to include always 'Authorization' header if it is provided. Signed-off-by: my5cents <[email protected]>

vsoch · 2024-04-03T16:46:30Z

oras/provider.py

@@ -893,6 +893,7 @@ def do_request(
        :type stream: bool
        """
        headers = headers or {}
+        headers.update(self.headers)


This should likely be tried after an initial failure (e.g., see below where it updates after the auth response.

I think the maintain the current behavior we might want to have a boolean that determines if the headers should be re-used and persist (or not).

Thanks for quick reply!

After initial failure it tries to get token from realm returned in 'www-authenticate' header, but remote registry does not expose it externally, and all the following re-auth attempts fail since URL 'https://docker-registry-realm' is not available.

We can pass refresh_headers flag with False value to keep self.headers in request, like it is done in the pull method. But my concerns here are:

it is necessary to pass it through long chain (push -> upload_blob -> put_upload/chunked_upload);

if user performed explicit login/set_basic_auth operation, then he meant to use the credentials until he calls logout operation

What do you think?

The normal flow I think is to expect that auth flow to work, although now that I think of it the auth isn't standardized. The basic auth is usually there to then request the token (the flow we have).

I understand your use case I think, and perhaps we need another set credential function that sets something permanently and does not try the auth flow?

self.headers property seems quite permanent :) It is accessible from all Registry methods although is not actually used in each request...

If you have a set method for more static credentials, that would add those credentials to the headers, and given the client knows that set method was used, it would not touch or try to update headers.

That was my initial point, basic auth (username and pass) are static unlike the token and could be used in all requests until the explicit logout (like in ORAS CLI).

But that function is used to encode in the initial request for the token. Saying to take the basic auth and not use it for a token (just provide as is) is a different flow I think.

I see. Fixed in following commit to propagate refresh_headers flag like in pull flow.

I guess, refresh_headers flag may be turned later to the internal property, and set automatically within set_token_auth method to True and within the set_basic_auth method to False.

Added refresh_headers flag to allow user to keep basic auth in headers during the push flow Signed-off-by: my5cents <[email protected]>

vsoch · 2024-04-04T07:17:47Z

This looks good - you are exposing this variable to more functions. Could you please bump the version (in version.py) and add a line to CHANGELOG.md? For the formatting/linting errors you can see the issue in the GitHub action output and run the same versions (e.g., black) locally to fix.

Signed-off-by: my5cents <[email protected]>

my5cents · 2024-04-04T10:48:47Z

Could you please bump the version (in version.py) and add a line to CHANGELOG.md? For the formatting/linting errors you can see the issue in the GitHub action output and run the same versions (e.g., black) locally to fix.

Done

vsoch · 2024-04-04T20:01:18Z

@my5cents looks like you just need one more run of black and we're good (take note of the version).

Signed-off-by: Yuriy Novostavskiy <[email protected]>

This reverts commit 598f6b0. Signed-off-by: Yuriy Novostavskiy <[email protected]>

Signed-off-by: Yuriy Novostavskiy <[email protected]>

yurnov · 2024-04-05T17:08:27Z

I reformatted remaining part, it should be fine

my5cents requested review from vsoch and SteveLasker as code owners April 3, 2024 16:15

vsoch reviewed Apr 3, 2024

View reviewed changes

Added refresh_headers flag to the push flow

85db3d4

Added refresh_headers flag to allow user to keep basic auth in headers during the push flow Signed-off-by: my5cents <[email protected]>

my5cents requested a review from vsoch April 4, 2024 05:52

my5cents added 3 commits April 4, 2024 13:36

fixed pep8 error

01f5e69

Signed-off-by: my5cents <[email protected]>

Bumped version in version.py

deee70f

Signed-off-by: my5cents <[email protected]>

Update CHANGELOG.md

7372a1a

Signed-off-by: my5cents <[email protected]>

11reformat with black

598f6b0

Signed-off-by: Yuriy Novostavskiy <[email protected]>

yurnov force-pushed the patch-1 branch from b49f1de to 598f6b0 Compare April 5, 2024 16:33

yurnov added 2 commits April 5, 2024 16:40

Revert "11reformat with black"

fdb66a1

This reverts commit 598f6b0. Signed-off-by: Yuriy Novostavskiy <[email protected]>

reformatted with black

51b4377

Signed-off-by: Yuriy Novostavskiy <[email protected]>

vsoch approved these changes Apr 5, 2024

View reviewed changes

vsoch merged commit 545b16f into oras-project:main Apr 5, 2024
5 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update headers in do_request method, provider.py #129

Update headers in do_request method, provider.py #129

my5cents commented Apr 3, 2024

vsoch Apr 3, 2024

my5cents Apr 3, 2024 •

edited

Loading

vsoch Apr 3, 2024

my5cents Apr 3, 2024

vsoch Apr 3, 2024

my5cents Apr 3, 2024

vsoch Apr 3, 2024

my5cents Apr 3, 2024

vsoch commented Apr 4, 2024

my5cents commented Apr 4, 2024

vsoch commented Apr 4, 2024

yurnov commented Apr 5, 2024

Update headers in do_request method, provider.py #129

Update headers in do_request method, provider.py #129

Conversation

my5cents commented Apr 3, 2024

vsoch Apr 3, 2024

Choose a reason for hiding this comment

my5cents Apr 3, 2024 • edited Loading

Choose a reason for hiding this comment

vsoch Apr 3, 2024

Choose a reason for hiding this comment

my5cents Apr 3, 2024

Choose a reason for hiding this comment

vsoch Apr 3, 2024

Choose a reason for hiding this comment

my5cents Apr 3, 2024

Choose a reason for hiding this comment

vsoch Apr 3, 2024

Choose a reason for hiding this comment

my5cents Apr 3, 2024

Choose a reason for hiding this comment

vsoch commented Apr 4, 2024

my5cents commented Apr 4, 2024

vsoch commented Apr 4, 2024

yurnov commented Apr 5, 2024

my5cents Apr 3, 2024 •

edited

Loading