default certificate support

[pweil-]

This pr contains the ability to specify a default certificate for a router in order to support wild card certificates.

Included:

1. new command flag for the router --default-cert=/path/to/file
2. haproxy template update that uses the crt directive to add the default cert if available. It will still use the dummy pem file in the non-sni backend if no default cert is found. In the SNI end it will resort to the configured directory
3. refactor of the template since we are passing more than just routes as the data. There is now a wrapper object with DefaultCertificate and State
4. the shard is now passed to validation. If validation sees that there are no certificates set for edge/reencrypt termination it will see if the host ends in the DNS shard configuration

Issue: for updates we don't allocate the shard. Right now I short circuit for a nil shard so an error condition exists for an update where tls is removed, term is still set to edge/reencrypt. I assume at some point we'll want to store the shard configuration somewhere and be able to retrieve it via an annotation on the route.

If this looks good I'll wrap up the unit tests tomorrow.

@ramr @rajatchopra @smarterclayton PTAL

[pweil-]

/cc @abhgupta

[pweil-]

rebased. Looking for feedback on the approach since there is a known issue with update validation (or if I should just go ahead and throw an annotation on the route in this pr if that is the correct course of action)

[smarterclayton]

Assigned shard would be a field on route. before we do that we have to decide whether we want to bind routes to multiple shards or follow the pod model and require you to create multiple.

As far as how the router stores and uses certs, we probably want to move (in time) to using a KUBECONFIG file or some other more formal config for the router. It's fine for now what you've done.

[pweil-]

ok, I will clean this up and prep it for merge then. Thank you.

[pweil-]

unit tests are in.

For reviewers: the unit tests were in a non-table based implementation for TLS...coded by a 6 months younger and less wise pweil-. All the additions/deletions were from converting them to a single, table based test. The relevant test cases for wildcard certs are the last 4 in the table.

[smarterclayton]

LGTM but would like someone else to do sign off.

[abhgupta]

@smarterclayton @pweil- Taking a look at it now

[abhgupta]

LGTM - just had a minor question related to route validation during updates. I will try to ping you tomorrow for that.

[pweil-]

snag that I didn't catch, pkg/api/validation/validation.go ValidateObject (used by the SwaggerSchema from what my search turned up) calls a validate generically for all objects so it won't have the shard. I set it to nil, but I'm worried that I'm just digging a hole on that one. Should I just throw an annotation on the route? If I do I probably also need to refactor the rest implementation to use a strategy, it is still in the old format.

[smarterclayton]

On Apr 22, 2015, at 12:00 PM, Paul [email protected] wrote:

snag that I didn't catch, pkg/api/validation/validation.go ValidateObject (used by the SwaggerSchema from what my search turned up) calls a validate generically for all objects so it won't have the shard. I set it to nil, but I'm worried that I'm just digging a hole on that one. Should I just throw an annotation on the route? If I do I probably also need to refactor the rest implementation to use a strategy, it is still in the old format.

For what?
—
Reply to this email directly or view it on GitHub.

[pweil-]

I'm over thinking it. I can just do a fetch/carry over the annotation/update in the existing structure without moving to generic etcd. That allows me to correctly validate the update and set the validateRoute back to just taking a route.

[smarterclayton]

Quick question - why are we not using generic etcd here? :)

On Apr 22, 2015, at 1:02 PM, Paul [email protected] wrote:

I'm over thinking it. I can just do a fetch/carry over the annotation/update in the existing structure without moving to generic etcd. That allows me to correctly validate the update and set the validateRoute back to just taking a route.

—
Reply to this email directly or view it on GitHub.

[pweil-]

it is still in the original implementation format from ... September 2014. It has not been refactored. We should be using generic etd.

[smarterclayton]

Open a follow up then.

----- Original Message -----

it is still in the original implementation format from ... September 2014.
It has not been refactored. We should be using generic etd.

---

Reply to this email directly or view it on GitHub:
#1779 (comment)

[smarterclayton]

A route should not require a shard to exist.

[smarterclayton]

Users will create a route that won't have a shard bound to it, then a shard will be bound.

[pweil-]

Right, what we need to do is move to a status/spec setup where a route can be failed post validation and maintain information about allocation. Then validation can be relaxed for the certificates and the shard allocation mechanism can figure out if it supports the route as a wildcard or not. I thought that would be a post 3.0 item though and we wanted wildcard certs before then. In that case, with the current setup, I can tell if a route is attempting to be a wild card route up front in the allocation (since we use a generic allocator 100% of the time).

Is this a hold off item or are we just discussing future changes?

Will open the refactor issue.

[pweil-]

#1870

[pweil-]

Updated and [test] for good measure

[openshift-bot]

continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_openshift3/1899/)

[smarterclayton]

----- Original Message -----

Right, what we need to do is move to a status/spec setup where a route can be
failed post validation and maintain information about allocation. Then
validation can be relaxed for the certificates and the shard allocation
mechanism can figure out if it supports the route as a wildcard or not. I
thought that would be a post 3.0 item though and we wanted wildcard certs
before then. In that case, with the current setup, I can tell if a route is
attempting to be a wild card route up front in the allocation (since we use
a generic allocator 100% of the time).

Is this a hold off item or are we just discussing future changes?

Validation can't fail because shard hasn't been set. Validation can do more work if shard is set. But it can't fail.

Will open the refactor issue.

---

Reply to this email directly or view it on GitHub:
#1779 (comment)

[pweil-]

But right now a shard is always set: https://github.com/openshift/origin/blob/master/pkg/route/registry/route/rest.go#L83

The current validation is: if you have a route with TLS term type of edge or reencrypt then it must have cert/key/cacert

In this PR the validation would be: if you have a route with a TLS term type of edge or reencrypt that has no certificates set check to see if the shard suffix is the host suffix. If so, good route.

I can remove the validation and add something to the plugin to filter routes when writing config, logging the reasons why. The plugin itself would know what the default cert is, perhaps it could parse and load it to get the hosts, check for a * and run the suffix check from there. At that point the dns suffix in the shard seems redundant. That does tie the plugin to PEM data, but we discussed that on the validation PR.

If that's not good, what would be the preferred implementation? I feel like the more we move into the shard configuration stories the closer routers become to being resources that the server can interact with since the configuration of routes relies on the configuration of the router. Not saying they have to be, just saying they're closely tied when it comes to name generation and wild card certificates.

[smarterclayton]

I'm saying, the validation should not require a shard. If a particular validation needs a shard, make it conditional on the shard being set.

On Apr 22, 2015, at 3:41 PM, Paul [email protected] wrote:

But right now a shard is always set: https://github.com/openshift/origin/blob/master/pkg/route/registry/route/rest.go#L83

The current validation is: if you have a route with TLS term type of edge or reencrypt then it must have cert/key/cacert

In this PR the validation would be: if you have a route with a TLS term type of edge or reencrypt that has no certificates set check to see if the shard suffix is the host suffix. If so, good route.

I can remove the validation and add something to the plugin to filter routes when writing config, logging the reasons why. The plugin itself would know what the default cert is, perhaps it could parse and load it to get the hosts, check for a * and run the suffix check from there. At that point the dns suffix in the shard seems redundant. That does tie the plugin to PEM data, but we discussed that on the validation PR.

If that's not good, what would be the preferred implementation? I feel like the more we move into the shard configuration stories the closer routers become to being resources that the server can interact with since the configuration of routes relies on the configuration of the router. Not saying they have to be, just saying they're closely tied when it comes to name generation and wild card certificates.

—
Reply to this email directly or view it on GitHub.

[pweil-]

heh then I think we may be agreeing. Seriously not trying to be a pain in your ass, I promise 😈

for edge/reencrypt we check that you either have all the certs defined or are a wildcard: https://github.com/pweil-/origin/blob/router-wildcard-cert/pkg/route/api/validation/validation.go#L82

if you have any of the certs defined you should have them all: https://github.com/pweil-/origin/blob/router-wildcard-cert/pkg/route/api/validation/validation.go#L82

otherwise check if you may be a wild card route, if no shard is set you can't be a wildcard because we don't know the dns suffix that is supported by the shard: https://github.com/pweil-/origin/blob/router-wildcard-cert/pkg/route/api/validation/validation.go#L82

So you get an extra chance at being valid IF the shard is set

[smarterclayton]

otherwise check if you may be a wild card route, if no shard is set you can't be a wildcard because we don't know the dns suffix that is supported by the shard

A shard may not be set yet. Validate should allow us to set values that get resolved later for correctness. So validate shouldn't reject a configuration that might become valid once shard is set.

[pweil-]

Ok, well then here is how I see our options pre-spec/status implementation

1. allow edge/reencrypt routes with no certs - router ignores them by comparing the host against a wildcard domain config and logs why
2. allow edge/reencrypt routes with no certs - router writes them and users get served a default cert if it exists and receive a browser warning that the cert and host do not match

Post spec/status implementation the route will be in Pending state, the shard allocation mechanism can validate and set the state to Allocated or Failed, the router only cares about Allocated routes

Which option is suitable for now or is there another one I'm not thinking of?

[smarterclayton]

On Apr 24, 2015, at 9:37 AM, Paul [email protected] wrote:

Ok, well then here is how I see our options pre-spec/status implementation

allow edge/reencrypt routes with no certs - router ignores them by comparing the host against a wildcard domain config and logs why
allow edge/reencrypt routes with no certs - router writes them and users get served a default cert if it exists and receive a browser warning that the cert and host do not match
Post spec/status implementation the route will be in Pending state, the shard allocation mechanism can validate and set the state to Allocated or Failed, the router only cares about Allocated routes

2 sounds more friendly, but you should still log
Which option is suitable for now or is there another one I'm not thinking of?

—
Reply to this email directly or view it on GitHub.

[pweil-]

ok, on it. Thank you.

[pweil-]

[test]

[pweil-]

I've removed the validation that required the ca/cert/key for edge and reencrypt routes. The router will now do a check to see if those certificates exist. If the router is configured with a default certificate then a V4 log message will be produced. If the router was not configured with a default cert then a Warning log message will be produced:

W0426 16:46:55.477666 1 router.go:352] a reencrypt terminated route with host hello-nginx-secure-default.router.default.local does not have the required certificates. The route will still be created but no certificates will be written

Two other changes:

1. Added some init logging
2. Ensured the pull policy on the ex router command is PullIfNotPresent

@smarterclayton @abhgupta - Please take another pass

[smarterclayton]

LGTM [merge]

[openshift-bot]

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_requests_openshift3/1692/) (Image: devenv-fedora_1374)

[openshift-bot]

Evaluated for origin up to fb7b0c0