Skip to content

Conversation

@jbalonso
Copy link

We maintain a private docker registry without SSL (it isn't necessary). This PR adds an -i option to fig pull that enables insecure mode.

@thaJeztah
Copy link
Member

I wonder if this should be a short ( -i) flag, or be more explicit (--insecure)?

@dnephin
Copy link

dnephin commented Sep 22, 2014

+1, I hit this as well, I'd be fine with either short or long option

@bfirsh
Copy link

bfirsh commented Sep 23, 2014

+1 long flag. And it should be --allow-insecure-ssl: https://github.com/docker/docker/pull/2687/files

@jbalonso
Copy link
Author

Ok. A commit with --alow-insecure-ssl is forthcoming. Also, sign-offs, and using insecure_registry as the keyword argument instead of insecure for consistency with docker-py.

@bfirsh
Copy link

bfirsh commented Sep 23, 2014

Great, thanks! Original commit still needs signing off though. You can squash all of this with git rebase -i master and signoff the single commit with git commit --amend --signoff.

@jbalonso
Copy link
Author

I took care of the signoff in the squash, and I believe the ball is back in your court.

@bfirsh
Copy link

bfirsh commented Sep 23, 2014

Great stuff, thanks!

@dnephin
Copy link

dnephin commented Sep 24, 2014

I just realized that this kwarg (insecure_registry) was only added to docker-py in 0.5.0, but right now in setup.py the minimum version is 0.3.2. I think for this change to be safe you need to change the docker-py requirement to:
docker-py >= 0.5, < 0.6

@dnephin
Copy link

dnephin commented Sep 24, 2014

Not sure why the travis build hasn't run on this branch, but I'm also getting a couple flake8 errors

@bfirsh
Copy link

bfirsh commented Sep 24, 2014

Oops.

We've replaced Travis with Wercker, so this needs rebasing to get CI run on it.

@jbalonso
Copy link
Author

I've fixed the docker-py dependency. I'll worry about rebasing (where should I rebase to? I see an "All is well -- build finished" on the PR) and figuring out the flakes tomorrow.

@bfirsh
Copy link

bfirsh commented Sep 25, 2014

We only recently added the file that makes the build work. If you rebase on to the current master, it'll get the wercker.yml file to make the build work. You can run nosetests and flake8 fig locally to verify the things the build runs.

@jbalonso
Copy link
Author

I've rebased, but I'm still cleaning up the branch.

Jason Bernardino Alonso added 2 commits September 26, 2014 16:36
Signed-off-by: Jason Bernardino Alonso <jalonso@luminoso.com>
Signed-off-by: Jason Bernardino Alonso <jalonso@luminoso.com>
@jbalonso
Copy link
Author

Ok. A third force-push later, I believe I have cleaned up this PR.

@bfirsh
Copy link

bfirsh commented Sep 30, 2014

LGTM

@bfirsh
Copy link

bfirsh commented Sep 30, 2014

Don't think it's easy to test this without mocking out docker-py. :/

@dnephin
Copy link

dnephin commented Sep 30, 2014

At the very least a simple unit test that asserts the right docker.client function was called wouldn't hurt. ServiceTest already creates a mock_client

@bfirsh
Copy link

bfirsh commented Sep 30, 2014

Oh cool, forgot about that. @jbalonso – would be great to get a test!

@bfirsh bfirsh added this to the 1.0.0 milestone Sep 30, 2014
Signed-off-by: Moss Collum <mcollum@luminoso.com>
@moss
Copy link

moss commented Oct 1, 2014

I've added the missing unit test to ServiceTest. Please let me know if it needs any cleanup.

@bfirsh
Copy link

bfirsh commented Oct 1, 2014

LGTM

aanand added a commit that referenced this pull request Oct 1, 2014
Allow pulls from an insecure registry
@aanand aanand merged commit 431fdaa into docker:master Oct 1, 2014
@jbalonso jbalonso deleted the insecure-pull branch October 9, 2014 19:49
@tjrivera
Copy link

Hey guys, it seems like Fig will also attempt to pull needed images when running fig up -- not just when fig pull is run explicitly. The insecure flag in this case would have to be also passed at Project.up then through here: https://github.com/docker/fig/blob/master/fig/project.py#L175 and ultimately out to here: https://github.com/docker/fig/blob/master/fig/service.py#L182 where pull is being run again. I can work on a PR if this makes sense.

@jbalonso
Copy link
Author

I kinda feel responsible for not catching that in my PR. I'll mention that I'm out of bandwidth to work on it myself at the moment.

@dnephin
Copy link

dnephin commented Oct 15, 2014

I've run into this as well. I got around it for now by always doing a fig pull --allow-insecure-ssl first but it would be nice to not have to do that.

I think your solution sounds appropriate.

@torbjornvatn
Copy link

I've also run into this, and I really would like to call fig pull --allow-insecure-ssl directly. Are you working on a PR @tjrivera or should I try to fix this at work tomorrow?

@tjrivera
Copy link

@torbjornvatn I was planning on submitting a PR tomorrow morning

@torbjornvatn
Copy link

@tjrivera Nice!

yuval-k pushed a commit to yuval-k/compose that referenced this pull request Apr 10, 2015
Allow pulls from an insecure registry
Signed-off-by: Yuval Kohavi <yuval.kohavi@gmail.com>
yuval-k pushed a commit to yuval-k/compose that referenced this pull request Apr 10, 2015
PR docker#490 Provides the ability to pull from an insecure registry by passing --allow-insecure-ssl. This commit extends the work done in docker#490 and adds the ability to pass --allow-insecure-ssl to the up and run commands which will attempt to pull dependent images if they do not exist.

Signed-off-by: Tyler Rivera <riverat2@email.chop.edu>

Signed-off-by: Yuval Kohavi <yuval.kohavi@gmail.com>
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.

8 participants