Skip to content
This repository has been archived by the owner on Mar 21, 2022. It is now read-only.

Add authentication X-Registry-Config to build api for SSL support on private repository #171

Merged
merged 6 commits into from
Jun 15, 2015

Conversation

kevtaylor
Copy link
Contributor

Introduce a wrapper class to pass auth values and return formatted X-Registry-Config string.
The user/password/email and repository are base64 encoded and passed on the request header.

Notes - in order to get a local build, a couple of issues were found:
a). Found that the ant version 1.8.2 is transitively pulled which causes findbugs to fail with a class not found exception - explicitly added 1.9.4 in the pom to correct this
b). Although exec has been added to the API in 1.15, exec inspect didn't get added until 1.16 so this causes failures in docker 1.3.2 (API V1.15)

@kevtaylor
Copy link
Contributor Author

Fixes issue: #157

* "serveraddress":"myserverAddress"}}}
*/
public String toConfig() {
StringBuilder sb = new StringBuilder();
Copy link
Contributor

Choose a reason for hiding this comment

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

We should use the appropriate jackson stuff rather than building JSON strings by concatenation. The latter could end poorly depending on the values supplied by the user.

@rohansingh
Copy link
Contributor

Thanks for the PR! I made some comments inline. Could you please also squash your commits into either a single commit, or rebase into a single commit per semantic change.

For example, you could just have one commit that has the same title as this PR. Alternatively, you could have 3 commits, one for adding the auth stuff, one for ant stuff, and one for fixing the exec test.

Either of these approaches help keep the project's commit history clean & useful.

@kevtaylor
Copy link
Contributor Author

@rohansingh All changes applied as discussed.

@kevtaylor
Copy link
Contributor Author

Would somebody be able to update us on whether this pull request can be approved and merged?
We are currently building a local patched version and there are a couple of other interested parties from the plugin forum who would benefit from this fix, so it would be nice to get it released.
Thanks for any input. Kevin

@davidxia
Copy link
Contributor

@rohansingh Other than your comment above, is this good to go? I can review it as well if you'd like.

@kevtaylor
Copy link
Contributor Author

@davidxia @rohansingh Hi. Do you have any update on our pull request?

rgrunber and others added 2 commits June 6, 2015 18:35
ApacheUnixSocket provides API for a client to communicate with an
existing unix socket, so the API related to the server-side 'bind'
command isn't necessary. They can simply be made to return whichever
values indicate an unbound state.

Fixes #146
@rohansingh
Copy link
Contributor

Apologies guys, dropped the ball on this. @kevtaylor If you want to rebase on latest master, I'm happy to merge.

@kevtaylor
Copy link
Contributor Author

@rohansingh @davidxia Rebase completed - tests all good.

rohansingh added a commit that referenced this pull request Jun 15, 2015
Add authentication X-Registry-Config to build api for SSL support on private repository
@rohansingh rohansingh merged commit 0b57555 into spotify:master Jun 15, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants