-
Notifications
You must be signed in to change notification settings - Fork 63
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
Nvcrfix #238
base: master
Are you sure you want to change the base?
Nvcrfix #238
Conversation
imagegw/shifter_imagegw/dockerv2.py
Outdated
if creds and self.username is not None and self.password is not None: | ||
if self.username=='$oauthtoken': | ||
self.private = True | ||
headers['Authoriation'] = 'Bearer %s' % (self.password) |
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.
authoriation is probably a typo
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.
the password is just sent plaintext? not even the base64 encoded to assure special characters don't derail the whole thing?
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.
yeah it is just a bearer token of some sort. Good catch on the misspelling. I'm confused as to how this worked now. Let me test this again.
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.
It winds up the typo was part of why it worked. I need to figure out what is going on here.
@scanon any news on this PR? Pulling from nvcr doesn't work for us currently. |
@uvNikita Sorry. This fell of my radar. Let me push on it again. |
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.
I've added changes suggestion that I think fixed the issue for us. If I recall it correctly, the problem was that the scope parameter was not parsed properly.
@@ -372,7 +372,7 @@ def do_token_auth(self, auth_loc_str, creds=False): | |||
# TODO, figure out what mode was for | |||
(_, auth_data_str) = auth_loc_str.split(' ', 2) | |||
|
|||
auth_data = {} | |||
auth_data = {'service':'', 'scope':'pull'} | |||
for item in filter(None, re.split(r'(\w+=".*?"),', auth_data_str)): | |||
(key, val) = item.split('=', 2) |
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.
(key, val) = item.split('=', 2) | |
(key, val) = item.split('=', 1) |
@@ -372,7 +372,7 @@ def do_token_auth(self, auth_loc_str, creds=False): | |||
# TODO, figure out what mode was for | |||
(_, auth_data_str) = auth_loc_str.split(' ', 2) | |||
|
|||
auth_data = {} | |||
auth_data = {'service':'', 'scope':'pull'} | |||
for item in filter(None, re.split(r'(\w+=".*?"),', auth_data_str)): | |||
(key, val) = item.split('=', 2) | |||
auth_data[key] = val.replace('"', '') |
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.
auth_data[key] = val.replace('"', '') | |
auth_data[key] = val.replace('"', '') | |
if '?scope=' in auth_data['realm']: | |
auth_data['realm'], auth_data['scope'] = auth_data['realm'].split('?scope=', 1) |
Pulls from Nvidia's registry didn't work for a few reasons. This addresses those issues. One thing I had to do was remove the check for application/json because the Nvidia registry sets it to text.