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

Generate wildcard router certificate #3226

Closed
wants to merge 1 commit into from

Conversation

cben
Copy link
Contributor

@cben cben commented Jan 30, 2017

[issue #3981, got implemeted in #3821, backported to 1.5 in #4120]

Proof of concept for generating default router cert valid for
*.{{openshift_master_default_subdomain}}.

Motivation: make it effortless to rely on openshift-signer (trusting just its root cert) for master API, hosted services (I'm particularly intersted in hawkular metrics) and other app routes.
Currently hawkular-metrics route by default doesn't define certificate, falls back on router default cert which by default only issued by openshift-service-serving-signer for router.default.svc, router.default.svc.cluster.local domains — useless from outside.
It's already possible to pass externally generated hosted metrics and/or router certs. But doing so requires users to generate their own CA root and the needed certs themselves, and then to trust both their root and openshift-signer — or go to even more trouble rooting all certs at their CA. (That makes sense if you already have an organization CA, but should not be required for I-just-want-a-demo-openshift scenarios.)

I see hosted registry & logging already have code to generate a cert.
(As does openshift_metrics role. What's the difference between openshift_metrics and openshift_hosted_metrics?)
So another option would be to add similar code to openshift_hosted_metrics. But this approach has a major benefit of covering app routes!

  • Worked for me, with byo playbook and this inventory: https://gist.github.com/cben/5396f531b17115db94ccc380997ac785
    curl --cacert OPENSHIFT_SIGNER_ROOT.ca.crt happily connects to both hawkular-metrics.10.35.48.131.xip.io and an example app nodejs-ex-red-dragon.10.35.48.131.xip.io (internal ips, don't promise to keep them up).

    • TODO: The chain I'm seeing the router present has openshift-signer self-signed root twice at the end.

    • New cert isn't valid for router.default.svc, router.default.svc.cluster.local. Should I include them? Can this (or the fact it's not signed by openshift-service-serving-signer) break something for internal connections to router?

  • Conditioned on openshift_hosted_router_create_certificate=true. The way I did the conditions is probably wrong. I hope this could (eventually) become the default behavior when router cert is not explicitly specified.

  • No idea how this behaves wrt. caching & re-deploying certs...

cc @jcantrill @bbguimaraes @simon3z

Conditioned on openshift_hosted_router_create_certificate=true.
Covers *.{{openshift_master_default_subdomain}},
in particular makes hawkular-metrics route have good cert out of the box.
@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

@jcantrill
Copy link
Contributor

@cben Also to consider is this role is only valid for 1.4 or 3.4. Future metrics ansible work is part of the openshift_metrics role

@cben
Copy link
Contributor Author

cben commented Feb 22, 2017

Thanks @jcantrill. I'd love some direction here. (and I'm not familiar with how this project handles branches/versions/backport)

  1. I think a generated wildcard router cert is useful for apps, regardless of metrics.

    • Do you want this at all?
    • Could this be default behavior when router cert is not explicitly specified? Or does it need a flag?
  2. Cool, so 3.5 metrics should be covered (openshift_metrics role already has a cert).
    I also care about metrics in older openshifts (to reduce user temptation to disable cert validation).

    • Should I also do PR to openshift_hosted_metrics role, independent of router cert?

--hostnames='*.{{ openshift_master_default_subdomain }}'
--cert='{{ openshift_master_config_dir }}/openshift-router.crt'
--key='{{ openshift_master_config_dir }}/openshift-router.key'
when: (openshift_hosted_router_create_certificate | default(false)) and (not '{{ openshift_master_config_dir }}/openshift-router.key' | exists)
Copy link
Contributor

Choose a reason for hiding this comment

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

You should be able to use the module being added in this PR: #3378

@detiber
Copy link
Contributor

detiber commented Feb 22, 2017

Thanks @jcantrill. I'd love some direction here. (and I'm not familiar with how this project handles branches/versions/backport)

In general we try to backport as much as we feasibly can. We cannot backport easily to 3.1, though because it is incompatible with Ansible 2.x

I think a generated wildcard router cert is useful for apps, regardless of metrics.

Indeed, I think the module in the PR I linked would help with this, because it would allow for using the same task in all the roles and will only create the certificate a single time.

Do you want this at all?

Yes.

Could this be default behavior when router cert is not explicitly specified? Or does it need a flag?

I think having it be the default is fine.

Should I also do PR to openshift_hosted_metrics role, independent of router cert?

@sdodson thoughts?

@detiber
Copy link
Contributor

detiber commented Feb 22, 2017

@mtnbikenc might be good to look at incorporating this into your router/registry PR here: #3423

@mtnbikenc
Copy link
Member

@detiber @cben We have significantly changed the logic in the #3423 PR. This is good stuff and we can look at merging this in after we get the rest of that PR working.

@mtnbikenc
Copy link
Member

@cben We've merged in the changes to this role converting everything over to using modules. Could you rebase this PR and update the tasks to use the modules? You can see the usage of the modules in the code and can follow that example. There are also plans to split this role into two, one for router, one for registry so it would be good to get this merged in before we refactor again. Thanks!

@cben
Copy link
Contributor Author

cben commented Feb 27, 2017 via email

@openshift-bot
Copy link

OpenShift Ansible Action Required: Pull request cannot be automatically merged, please rebase your branch from latest HEAD and push again

@openshift-bot openshift-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 5, 2017
@cben
Copy link
Contributor Author

cben commented Mar 23, 2017

Closing in favor of #3749 that uses the new module (thanks @enoodle!)

@cben cben closed this Mar 23, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants