Skip to content

Switch to send full AuthConfig object for build action #682

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Jul 22, 2015
Merged

Switch to send full AuthConfig object for build action #682

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ebcfab0
Switch to send full AuthConfig object for build action
moutten Jul 17, 2015
7430eb8
Merge branch 'fix-build-auth' of https://github.com/moutten/docker-py…
shin- Jul 20, 2015
4be2286
Make build auth work with API versions < 1.19 too
shin- Jul 20, 2015
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
6 changes: 0 additions & 6 deletions docker/auth/auth.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,12 +98,6 @@ def encode_header(auth):
return base64.b64encode(auth_json)


def encode_full_header(auth):
""" Returns the given auth block encoded for the X-Registry-Config header.
"""
return encode_header({'configs': auth})


def parse_auth(entries):
"""
Parses authentication entries
Expand Down
11 changes: 8 additions & 3 deletions docker/client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,9 +140,14 @@ def build(self, path=None, tag=None, quiet=False, fileobj=None,
if self._auth_configs:
if headers is None:
headers = {}
headers['X-Registry-Config'] = auth.encode_full_header(
self._auth_configs
)
if utils.compare_version('1.19', self._version) >= 0:
Copy link
Contributor

Choose a reason for hiding this comment

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

The only issue that this causes, for my particular situation, is that docker-compose would still be broke since it registers the client API version as 1.18. I don't know really how to detect what version the docker server is expecting. I'm not sure that an API version was changed when this authentication header was changed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ping @aanand :3

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like the change went out in Docker 1.7.1, according to the milestone on this PR: moby/moby#14139

I don't see any switching being done on the API version, so it's possible that backwards compatibility is broken. I've asked for clarification: moby/moby#14139 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From Josh's answer, seems like the change only affects 1.7.0 and was reverted in 1.7.1 - making this PR unnecessary. Users of 1.7.0 should upgrade to 1.7.1.

@moutten Can you confirm that this change is unnecessary with Docker 1.7.1 ?

Copy link
Contributor

Choose a reason for hiding this comment

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

This change was still necessary with 1.7.1. You'll see that the previous code was including a top level key of configs. Removing the top level key of configs works with 1.7.1. Based on Josh's comment, I'm not sure that was previously necessary. I haven't tried testing to see if removing the configs still works against older versions of docker (prior to 1.7).

Copy link
Contributor

Choose a reason for hiding this comment

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

I tested my setup using docker version 1.6.2, docker-compose 1.3.3, along with a patched version of docker-py, and I can verify it did work correctly with the change in this pull request. This looks like the cleanest way for docker-py to implement this change. It looks like I'm going to have to wait for docker-compose to change the supported API to 1.19.

headers['X-Registry-Config'] = auth.encode_header(
self._auth_configs
)
else:
headers['X-Registry-Config'] = auth.encode_header({
'configs': self._auth_configs
})

response = self._post(
u,
Expand Down