Skip to content

Allow pushing a manifest to a registry with self-signed TLS certificate #1952

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

wndhydrnt
Copy link

- What I did
This change enables iterating over all available registry endpoints when pushing a manifest. Before this change, only one endpoint was used. The scheme of that endpoint was set to HTTP if the registry was marked as insecure even if the registry was available via HTTPS. This resulted in an error because a HTTP request was sent to a HTTPS endpoint.

Fixes #1951
- How I did it
The change re-uses the function client.iterateEndpoints() that was already available. Because that function was only used for reading previously, another parameter lookup of type func has been added to it. lookup is used to get the endpoints to query.

- How to verify it

  1. Start a Docker registry using a self-signed certificate. Mine is running at 127.0.0.1:52854.
  2. Use docker manifest create to create a new manifest
  3. Push to the registry, e.g. docker manifest push 127.0.0.1:52854/debian:stretch-slim-latest

- Description for the changelog
Allow pushing a manifest to a registry with self-signed TLS certificate

Signed-off-by: wndhydrnt <hydrantanderwand@gmail.com>
@GordonTheTurtle
Copy link

Please sign your commits following these rules:
https://github.com/moby/moby/blob/master/CONTRIBUTING.md#sign-your-work
The easiest way to do this is to amend the last commit:

$ git clone -b "fix-manifest-push-insecure-https" git@github.com:wndhydrnt/cli.git somewhere
$ cd somewhere
$ git rebase -i HEAD~842359016240
editor opens
change each 'pick' to 'edit'
save the file and quit
$ git commit --amend -s --no-edit
$ git rebase --continue # and repeat the amend for each commit
$ git push -f

Amending updates the existing PR. You DO NOT need to open a new one.

@codecov-io
Copy link

Codecov Report

Merging #1952 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1952   +/-   ##
=======================================
  Coverage   56.72%   56.72%           
=======================================
  Files         310      310           
  Lines       21800    21800           
=======================================
  Hits        12367    12367           
  Misses       8518     8518           
  Partials      915      915

Signed-off-by: wndhydrnt <hydrantanderwand@gmail.com>
@wndhydrnt wndhydrnt force-pushed the fix-manifest-push-insecure-https branch from 3b4a8b9 to c02ae8f Compare June 16, 2019 16:21
wndhydrnt added a commit to imagespy/api that referenced this pull request Jun 20, 2019
Cannot add a e2e test because docker/cli#1952 needs to be
merged first.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to push manifest to an insecure registry with self-signed certificate
5 participants