Skip to content

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

Open
wndhydrnt wants to merge 2 commits into
docker:masterfrom
wndhydrnt:fix-manifest-push-insecure-https
Open

Allow pushing a manifest to a registry with self-signed TLS certificate#1952
wndhydrnt wants to merge 2 commits into
docker:masterfrom
wndhydrnt:fix-manifest-push-insecure-https

Conversation

@wndhydrnt

Copy link
Copy Markdown

- 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
Copy Markdown

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
Copy Markdown

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.
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

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

6 participants