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

#181: Fix bug when authenticating to private registry #220

Conversation

jdavisonc
Copy link

#181

Fix bug when authenticating to private registry, add "serveraddress" to authentication header.

Base on the following documentation, "serveraddress" is required by Docker for authenticate. Not adding this part on header generates "Authentication required." error.

computerlyrik and others added 6 commits July 18, 2015 15:46
Signed-off-by: Christian Fischer <sw-dev@computerlyrik.de>
- introduced 'BuildService'
- cleaned up plexus components
- use mockito for testing

Signed-off-by: Jae Gangemi <jgangemi@gmail.com>
- url builder refactoring

Signed-off-by: Jae Gangemi <jgangemi@gmail.com>
@rhuss
Copy link
Collaborator

rhuss commented Jul 21, 2015

Thanks !

'will review this PR ASAP.

jgangemi and others added 6 commits July 22, 2015 09:58
- fixed model classes to only return first 12 of container id
- doc update / re-arrange sub-elements into alphabetical order

Signed-off-by: Jae Gangemi <jgangemi@gmail.com>
Cleaned up quite a bit, removed classes and made things more consistent. I.e. UrlBuilder is now homogenous. Removed also argument classes, since as long as we have so small argument list they only add to complexity (instead of clarifying things).

Fixes fabric8io#230.
…ons.

The http client was not correctly used which was leaving connections open and not being able to reuse, which caused at some point to reach the max connections per route. We faced this particular issue when trying to build 2 images with 2 tags each: since the default limit of connections per route host is 2 when we tried to build the images the plugin was getting stuck trying to get more connections.
The change consists mainly in changing execute with returns of result with proper ResponseHandlers so http client library automatically handles response automatic close to release open connections.

Signed-off-by: Roger Abelenda <rabelenda@gmail.com>
@rhuss
Copy link
Collaborator

rhuss commented Jul 24, 2015

Could you please sign-off the the PR as described in here ? Thanks a lot !

@jdavisonc jdavisonc force-pushed the private-registry-auth-add-serveraddres branch 2 times, most recently from e01f0f9 to bcced1e Compare July 24, 2015 15:28
rhuss and others added 2 commits July 24, 2015 12:28
…ess" to authentication header. Signed-off-by: Jorge Davison <jdavisonc@gmail.com>
@jdavisonc jdavisonc force-pushed the private-registry-auth-add-serveraddres branch from bcced1e to c28007f Compare July 24, 2015 15:29
@jdavisonc
Copy link
Author

Done! If you require any further information, let me know.

@rhuss
Copy link
Collaborator

rhuss commented Jul 25, 2015

Thanks ;-)

I just tried out the patch and it prevented me to push to index.docker.io (which still works with 0.13.2) with an Authentication failed. I still didn't dive into this PR yet (sorry), but will check this of course.

Does pushing to docker.io works for you ?

@jdavisonc
Copy link
Author

I only test the pull/push against a private repository V1, not the docker.io ones. Maybe is a error with that version only.

@rhuss
Copy link
Collaborator

rhuss commented Aug 13, 2015

Did you have a chance to check it with the official repository ?

I wil postpone this PR for a later release since I'm going to cut 0.13.3 today.

@rhuss
Copy link
Collaborator

rhuss commented Dec 1, 2015

Is authentication still an issue for you ?

Sorry for the long delay. It seems thaat serveraddress is really required for registry API V1 but not for V2 anymore. It is also somewhat redundant since the registry address is already part of the pull/push. V1 also doesn't seem to be much in use anymore, so if it is not absolutely necessary I would like drop support for this and close this PR.

Authentication will be generally be polished via #147.

@jdavisonc
Copy link
Author

I still using V1, but anyway I resolve the problem with other solution. Thanks anyway, I'll close this PR.

@jdavisonc jdavisonc closed this Dec 27, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants