validate name is set

[pweil-]

Validate a name is set on the route and add a test case for all route.* field validation (excludes tls, there are separate test cases for those scenarios).

[smarterclayton]

Whitespace is off

[pweil-]

Updated to use upstream validation method. I reused ValidateServiceName as the name validation function. If there is a more appropriate func (ValidatePodName, ValidateReplicationControllerName, or ValidateNodeName) or if we should create our own specifically for routes then say the word and it shall be so.

[smarterclayton]

it depends on how we're fixing the unique name problem.

[pweil-]

The unique host name (different from route meta name) issue has been a bit tricky. In order to validate the uniqueness of a host we need access to storage and do a query using the all namespace. However, accessing storage from within validation (ie. passing the registry) creates a circular dependency.

So right now my thinking is that it belongs closer to REST which is where validation is called anyway. In terms of actual implementation I am thinking I will get all the routes and iterate through them validating that:

1. You cannot create the same host in two different namespaces
2. There can only be 2 routes with the same host. Of those two routes, one must be unsecure and the other must have a tls termination definition.

[smarterclayton]

I think we really can move this problem to router binding. That is the only way the check will scale anyway. The act of binding is a reservation process that can succeed or fail. It's really not different than pod binding.

On Feb 13, 2015, at 9:38 AM, Paul [email protected] wrote:

The unique host name (different from route meta name) issue has been a bit tricky. In order to validate the uniqueness of a host we need access to storage and do a query using the all namespace. However, accessing storage from within validation (ie. passing the registry) creates a circular dependency.

So right now my thinking is that it belongs closer to REST which is where validation is called anyway. In terms of actual implementation I am thinking I will get all the routes and iterate through them validating that:

You cannot create the same host in two different namespaces
There can only be 2 routes with the same host. Of those two routes, one must be unsecure and the other must have a tls termination definition.
—
Reply to this email directly or view it on GitHub.

[pweil-]

Interesting, I gave that some thought but abandoned it to allow create-time failure. I can add a status to the route which would be good anyway. I'll head that direction.

[smarterclayton]

You can definitely throw out any name that can't be DNS. Name uniqueness within a namespace is then checked by the save. But the fact that someone might want multiple dns names, or we want to limit to only the first one to register, or want to change those on the fly (create/recreate route?) is really a different use case.

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

Interesting, I gave that some thought but abandoned it to allow create-time
failure. I can add a status to the route which would be good anyway. I'll
head that direction.

---

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

[pweil-]

To be clear since we're talking about 2 things on this PR:

1. This PR is specifically to address the issue of not requiring ObjectMeta.Name to be set. The upstream function ValidateObjectMeta requires a name validation function to be passed. I chose to reuse an existing function that is used for service names.
2. What we're discussing in this thread now is about validation of the Host field on a route. I will add a DNS validation to the field at create time and add a bind time validation for Host uniqueness. Uniqueness is defined as no more than 1 secure and 1 unsecure version of that Host. Having multiple Hosts for a service is currently possible by using multiple routes but that can also be improved by making Host a slice if we want.

[pweil-]

Summary of recent discussion:

Definitions

Host: the dns name of the route like www.example.com
Name: the resource name of the route object withing the OpenShift system

Route Summary

1. Route that simply asks for a Host to be assigned. The generated Host would include the namespace
2. Route request when the customer already has a domain they would like to use as the Host

Route Characteristics

1. Route hosts are not guaranteed to be unique
2. Secure and unsecure versions of the same route utilize the same host
3. Applications performing a no-downtime blue/green deployment will want the same route to exist multiple times
4. Route hosts should be unique across router shards
5. Uniqueness still allows for secure/non-secure versions of the same route to exist in the same shard
6. In the sharded environment the first route to hit the shard reserves the right to exist there indefinitely. Even across restarts
7. Routes should be parkable
8. There are a couple of ways this can be done: commands or pointing to an empty service
9. A parked route still exists in the system and should be able to display an informational message/page
10. Routes do not need a Host to be defined, users should be able to add the information late

Route Permissions

1. In the course of determining uniqueness of routes it must be easily grokable to an administrator when a route Host is already taken within a shard or exists elsewhere in the system.
2. It is a valid use case for the same route to exist across multiple projects in the case of a blue/green deployment. Permissions must be flexible enough to support this or the creation of a route must be verbose enough to communicate potential issues.

API TODOs

1. Route bindings
2. Adding a createdAt date/time to a resource to allow ordering

Open Items To Be Addressed in This PR

1. Remove the host requirement on a route. Routes with no hosts are already skipped by the template, this is a validation update
2. Add DNS validation to the Host field if it exists
3. The Name validation does not need to use kval.ValidateServiceName (but is still a required field)

[pweil-]

I have updated this PR to reflect the open items we discussed above.

1. Host is no longer required.
2. If it is set it will be validated with upstream's IsDNSSubdomain
3. ObjectMeta is validated with upstream's ValidateObjectMeta. The name function required is an always true placeholder since there are no special requirements (as opposed to reusing the pod/service/etc func that already exist)
4. Unit tests have been updated to reflect the new validations

[smarterclayton]

You should use pod name as the name for routes.

[pweil-]

updated to use the upstream ValidatePodName function in ValidateObjectMeta

[smarterclayton]

LGTM [merge]

[openshift-bot]

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

[openshift-bot]

Evaluated for origin up to 68f1bf5