-
Notifications
You must be signed in to change notification settings - Fork 389
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
Proposed fix for ACR caching while providing guidance for GitLab users. #1628
base: main
Are you sure you want to change the base?
Changes from all commits
1e1784d
3924ef9
8d7649d
e22eec1
409ed4c
da8ae55
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -169,7 +169,15 @@ async def test_get_token(): | |
test_handle = {"username": username, "password": password} | ||
app = Application( | ||
[ | ||
(r"/token", MockTokenHandler, {"test_handle": test_handle}), | ||
( | ||
r"/token", | ||
MockTokenHandler, | ||
{ | ||
"test_handle": test_handle, | ||
"service": "service.1", | ||
"scope": "scope.2", | ||
}, | ||
), | ||
] | ||
) | ||
ip = "127.0.0.1" | ||
|
@@ -196,6 +204,49 @@ async def test_get_token(): | |
assert token == test_handle["token"] | ||
|
||
|
||
async def test_get_token_with_config_supplied_service(): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It looks like this is a copy of |
||
# regression coverage for https://github.com/jupyterhub/binderhub/issues/1620 | ||
|
||
username = "user" | ||
password = "pass" | ||
test_handle = {"username": username, "password": password} | ||
app = Application( | ||
[ | ||
( | ||
r"/token", | ||
MockTokenHandler, | ||
{ | ||
"test_handle": test_handle, | ||
"service": "service.1", | ||
"scope": "scope.2", | ||
}, | ||
), | ||
] | ||
) | ||
ip = "127.0.0.1" | ||
port = randint(10000, 65535) | ||
app.listen(port, ip) | ||
|
||
registry = DockerRegistry( | ||
url="https://example.org", username=username, password=password | ||
) | ||
|
||
assert registry.url == "https://example.org" | ||
assert registry.auth_config_url == "https://example.org" | ||
# token_url should be unset, since it should be determined by the caller from a | ||
# WWW-Authenticate header | ||
assert registry.token_url == "" | ||
assert registry.username == username | ||
assert registry.password == password | ||
token = await registry._get_token( | ||
httpclient.AsyncHTTPClient(), | ||
f"http://{ip}:{port}/token?service=service.1", | ||
None, | ||
"scope.2", | ||
) | ||
assert token == test_handle["token"] | ||
|
||
|
||
@pytest.mark.parametrize("token_url_known", [True, False]) | ||
async def test_get_image_manifest(tmpdir, token_url_known): | ||
username = "asdf" | ||
|
@@ -230,7 +281,7 @@ async def test_get_image_manifest(tmpdir, token_url_known): | |
f, | ||
) | ||
if token_url_known: | ||
token_url = url + "/token" | ||
token_url = url + "/token?service=container_registry" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this needed? If it's testing another scenario can you use |
||
else: | ||
token_url = "" | ||
registry = DockerRegistry( | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this change needed?