Skip to content

Add SSL support to local mirrored registry #856

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.

Sign up for GitHub

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

Jump to bottom
Merged
hardys merged 2 commits into from
Dec 23, 2019
Merged

Add SSL support to local mirrored registry #856

hardys merged 2 commits into from
Dec 23, 2019

Conversation

@yprokule
Copy link
Contributor

@yprokule yprokule commented Nov 25, 2019 •
edited
Loading

Add SSL support for local mirror.

There are issues while using local mirror that doesn't support SSL.
Current change addresses this limitation.

  • create self-signed ssl cert for local mirror
  • create pull-secret for local mirror
  • combine local pull secret with global
  • add custom ssl-cert to install-config.yml
  • extract the installation program from the mirrored content
  • use local pull secret for deployment

@yprokule
Copy link
Contributor Author

yprokule commented Nov 25, 2019

/cc @hardys @stbenjam @honza

@yprokule yprokule changed the title Mirror registry local Add SSL support to local mirrored registry Nov 25, 2019
@yprokule yprokule changed the title Add SSL support to local mirrored registry WIP: Add SSL support to local mirrored registry Nov 25, 2019
@hardys hardys mentioned this pull request Nov 25, 2019
Closed
@yprokule yprokule force-pushed the mirror-registry-local branch 3 times, most recently from 0b43543 to cdbdac4 Compare November 25, 2019 14:33
hardys
hardys reviewed Nov 25, 2019
View reviewed changes
@hardys
Copy link

hardys commented Nov 25, 2019

Looks good, but can we avoid running two registry containers, seems like just one should be enough for both MIRROR_IMAGES and *_LOCAL_IMAGE cases?

@yprokule yprokule force-pushed the mirror-registry-local branch 3 times, most recently from 20f1355 to 6a45fa6 Compare November 25, 2019 15:42
honza
honza reviewed Nov 25, 2019
View reviewed changes
@yprokule yprokule force-pushed the mirror-registry-local branch 4 times, most recently from 7f12c7f to ba56210 Compare November 25, 2019 21:17
honza
honza reviewed Nov 25, 2019
View reviewed changes
01_install_requirements.sh Outdated
-key ${REGISTRY_DIR}/certs/registry.key \
-out ${REGISTRY_DIR}/certs/registry.crt \
-days 365 \
-addext "$SSL_EXT" \
Copy link
Member

@honza honza Nov 25, 2019

Choose a reason for hiding this comment

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

The -addext flag isn't supported by the version of openssl that is installed via dev-scripts. It was introduced in 1.1.1.

Copy link
Contributor Author

@yprokule yprokule Nov 26, 2019

Choose a reason for hiding this comment

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

@honza which one do U have? I'm trying on RHEL-8 which has openssl-1.1.1-8

Copy link
Member

@honza honza Nov 26, 2019

Choose a reason for hiding this comment

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

OpenSSL 1.0.2k-fips on centos 7.7.1908

Copy link

@mazzystr mazzystr Dec 13, 2019

Choose a reason for hiding this comment

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

and for me on RHEL 8.1...

# rpm -q openssl
openssl-1.1.1c-2.el8.x86_64

# openssl version
OpenSSL 1.1.1c FIPS  28 May 2019

If dev-scripts is shipping openssl then that needs to stop. Dev-scripts should necessary args nothing more.

@yprokule yprokule force-pushed the mirror-registry-local branch from ba56210 to 9a27415 Compare November 26, 2019 07:38
honza
honza reviewed Nov 26, 2019
View reviewed changes
@yprokule yprokule force-pushed the mirror-registry-local branch 3 times, most recently from d72e902 to cb49716 Compare November 28, 2019 12:49
@honza
Copy link
Member

honza commented Nov 28, 2019

Should we run this through CI at this point?

@honza
Copy link
Member

honza commented Nov 28, 2019

The image content sources are still a bit off.

This is the oc adm release mirror output:

imageContentSources:
- mirrors:
  - 192.168.111.1:5000/localimages/local-release-image
  source: registry.svc.ci.openshift.org/ocp/4.4-2019-11-28-120853
- mirrors:
  - 192.168.111.1:5000/localimages/local-release-image
  source: registry.svc.ci.openshift.org/ocp/release

But this is what gets added to install-config:

imageContentSources:
- mirrors:
    - 192.168.111.15000/localimages/local-release-image
  source: registry.svc.ci.openshift.org/ocp/release
- mirrors:
    - 192.168.111.1:5000/localimages/local-release-image
  source: registry.svc.ci.openshift.org/ocp/4.4.0-0.ci-2019-11-28-120853

@yprokule yprokule force-pushed the mirror-registry-local branch 3 times, most recently from f298163 to 05c9e29 Compare December 4, 2019 16:08
@hardys hardys added the CI check this PR with CI label Dec 4, 2019
@metal3ci
Copy link

metal3ci commented Dec 4, 2019

Build SUCCESS, see build http://10.8.144.11:8080/job/dev-tools/1348/

@yprokule
Copy link
Contributor Author

yprokule commented Dec 13, 2019

Could we fix this then so the existing registry is removed if it doesn’t have ssl? Nearly everyone will have an existing registry who’s tested local images. We don’t clean it up typically.

Now ssl key and cert are generated if they don't exist. If they do get md5 hash of cert and compare it with cert's hash used by registry container. If they mismatch (or cert isn't used) or container is not running - new registry is started

mazzystr
mazzystr suggested changes Dec 13, 2019
View reviewed changes
@hardys hardys mentioned this pull request Dec 16, 2019
Closed
@yprokule yprokule force-pushed the mirror-registry-local branch from 2307729 to 5f6810b Compare December 17, 2019 10:34
@metal3ci
Copy link

metal3ci commented Dec 17, 2019

Build FAILURE, see build http://10.8.144.11:8080/job/dev-tools/1373/

@yprokule yprokule force-pushed the mirror-registry-local branch from 5f6810b to 90a73d3 Compare December 17, 2019 10:52
@derekhiggins derekhiggins mentioned this pull request Dec 17, 2019
Closed
@metal3ci
Copy link

metal3ci commented Dec 17, 2019

Build SUCCESS, see build http://10.8.144.11:8080/job/dev-tools/1374/

@yprokule yprokule requested review from hardys and honza December 17, 2019 14:08
@yprokule yprokule force-pushed the mirror-registry-local branch from 90a73d3 to 1983c1f Compare December 17, 2019 14:52
@metal3ci
Copy link

metal3ci commented Dec 17, 2019

Build SUCCESS, see build http://10.8.144.11:8080/job/dev-tools/1376/

@yprokule yprokule force-pushed the mirror-registry-local branch from 1983c1f to 65d5825 Compare December 18, 2019 06:21
@metal3ci
Copy link

metal3ci commented Dec 18, 2019

Build FAILURE, see build http://10.8.144.11:8080/job/dev-tools/1382/

@yprokule yprokule force-pushed the mirror-registry-local branch from 65d5825 to ae47b0e Compare December 18, 2019 11:29
@metal3ci
Copy link

metal3ci commented Dec 18, 2019

Build FAILURE, see build http://10.8.144.11:8080/job/dev-tools/1385/

@yprokule yprokule force-pushed the mirror-registry-local branch from ae47b0e to 5e046a2 Compare December 19, 2019 07:42
@metal3ci
Copy link

metal3ci commented Dec 19, 2019

Build FAILURE, see build http://10.8.144.11:8080/job/dev-tools/1387/

@honza @yprokule
Add option to mirror openshift images
8f48ad9 
We add a new `MIRROR_IMAGES` option.  When non-empty, it will use `oc
adm` to mirror openshift images to our local registry.  It will also
modify `install-config.yaml` to add the new mirror.
@yprokule yprokule force-pushed the mirror-registry-local branch from 5e046a2 to a97f4b1 Compare December 20, 2019 09:29
@metal3ci
Copy link

metal3ci commented Dec 20, 2019

Build SUCCESS, see build http://10.8.144.11:8080/job/dev-tools/1389/

@hardys hardys mentioned this pull request Dec 20, 2019
Closed
hardys
hardys reviewed Dec 23, 2019
View reviewed changes
@yprokule
Add SSL support for local mirror.
842b8d5 
There are issues while using local mirror that doesn't support SSL.
Current change addresses this limitation.

- [x] create self-signed ssl cert for local mirror
- [x] create pull-secret for local mirror
- [x] combine locall pull secret with global
- [x] add custom ssl-cert to install-config.yml
- [x] extract the installation program from the mirrored content
- [x] use local pull secret for deployment
@yprokule yprokule force-pushed the mirror-registry-local branch from a97f4b1 to 842b8d5 Compare December 23, 2019 10:23
@hardys
Copy link

hardys commented Dec 23, 2019

Ok this works well for me locally, and it passed CI - @derekhiggins had some comments about removing some things related to the local image registry which I don't think are yet fully resolved but perhaps we should take care of those via a follow-up?

@hardys
Copy link

hardys commented Dec 23, 2019

Spoke with @derekhiggins and he agreed to push a PR removing the old path as a follow-up, so I'll go ahead and merge this

@hardys hardys merged commit 3a0b957 into openshift-metal3:master Dec 23, 2019
@metal3ci
Copy link

metal3ci commented Dec 23, 2019

Build SUCCESS, see build http://10.8.144.11:8080/job/dev-tools/1394/

@yprokule yprokule deleted the mirror-registry-local branch December 23, 2019 14:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@derekhiggins derekhiggins derekhiggins left review comments

@honza honza Awaiting requested review from honza

+2 more reviewers

@hardys hardys hardys left review comments

@mazzystr mazzystr mazzystr requested changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

CI check this PR with CI

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@yprokule @hardys @honza @metal3ci @stbenjam @derekhiggins @mazzystr