Skip to content
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

Refactor openshift_hosted's docker-registry route setup #4254

Merged
merged 1 commit into from
Aug 21, 2017

Conversation

dmsimard
Copy link
Contributor

We have identified an issue where a docker-registry service set up
as 'reencrypt' with a provided certificate and a self-signed certificate
on the pod does not authorize users to push images.
If the docker-registry service is set up as 'passthrough' with the
same provided certificate, everything works. This is filed as an
Origin bug here: openshift/origin#14249

In light of this, this commit essentially adds support for configuring
provided certificates with a passthrough route while maintaining backwards
compatibility with the other use cases.
The default remains 'passthrough' with self-generated certificates.

Other miscellaneous changes include:

  • Move fact setup that were only used in secure.yml there
  • Omit the hostname for the route if there are none to configure,
    oc_route takes care of handling the default
  • Replace hardcoded /etc/origin/master by openshift_master_config_dir

@openshift-bot
Copy link

Can one of the admins verify this patch?
I understand the following commands:

  • bot, add author to whitelist
  • bot, test pull request
  • bot, test pull request once

@dmsimard
Copy link
Contributor Author

@abutcher @aweiteka FYI.

I have personally tested this in as many scenarios as I can:

  1. full default (no openshift_hosted_registry_routehost or certificates) with openshift_hosted_registry_routetermination properly defaulting to passthrough
  2. specified openshift_hosted_registry_routehost without certificates with openshift_hosted_registry_routetermination properly defaulting to passthrough
  3. specified openshift_hosted_registry_routehost with certificates with openshift_hosted_registry_routetermination explicitely set to passthrough
  4. specified openshift_hosted_registry_routehost with certificates and openshift_hosted_registry_routetermination set to reencrypt (reproduces origin issue mentioned in PR description)

The size of the patch is larger than I had anticipated but it turned out to be a bit troublesome to maintain full backwards compatibility with the same default while adding support for the scenario that actually works (custom host, custom certs and passthrough).

@openshift-bot
Copy link

Can one of the admins verify this patch?
I understand the following commands:

  • bot, add author to whitelist
  • bot, test pull request
  • bot, test pull request once

We have identified an issue where a docker-registry service set up
as 'reencrypt' with a provided certificate and a self-signed certificate
on the pod does not authorize users to push images.
If the docker-registry service is set up as 'passthrough' with the
same provided certificate, everything works.

In light of this, this commit essentially adds support for configuring
provided certificates with a passthrough route while maintaining backwards
compatibility with the other use cases.
The default remains 'passthrough' with self-generated certificates.

Other miscellaneous changes include:
- Move fact setup that were only used in secure.yml there
- Omit the hostname for the route if there are none to configure,
  oc_route takes care of handling the default
- Replace hardcoded /etc/origin/master by openshift_master_config_dir
@dmsimard
Copy link
Contributor Author

Rebased on top of current master.

@sdodson
Copy link
Member

sdodson commented Jul 24, 2017

aos-ci-test

@openshift-bot
Copy link

success: "aos-ci-jenkins/OS_3.6_NOT_containerized, aos-ci-jenkins/OS_3.6_NOT_containerized_e2e_tests" for d7d9796 (logs)

@openshift-bot
Copy link

success: "aos-ci-jenkins/OS_3.6_containerized, aos-ci-jenkins/OS_3.6_containerized_e2e_tests" for d7d9796 (logs)

@mjudeikis
Copy link
Contributor

@dmsimard Can you please include this one to your change and I will close mine.
#5133

@sdodson
Copy link
Member

sdodson commented Aug 18, 2017

[merge]

@sdodson
Copy link
Member

sdodson commented Aug 18, 2017

Lets just get this merged and then we can evaluate the additional change. This PR has been languishing way too long.

@openshift-bot
Copy link

openshift-bot commented Aug 18, 2017

continuous-integration/openshift-jenkins/merge Waiting: You are in the build queue at position: 1

@openshift-bot
Copy link

Evaluated for openshift ansible merge up to d7d9796

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.

4 participants