Validate port protocol case strictly

[liggitt]

This tightens validation on port protocol allowed values.

Currently, port protocol validation allows case-insensitive matching of "tcp" or "udp". However, other parts of the system rely on exact matches to the API constants:

pkg/api/validation/validation.go:
 1085   if service.Spec.Type == api.ServiceTypeLoadBalancer {
 1086       for i := range service.Spec.Ports {
 1087:          if service.Spec.Ports[i].Protocol != api.ProtocolTCP {
 1088               allErrs = append(allErrs, errs.NewFieldInvalid(fmt.Sprintf("spec.ports[%d].protocol", i), service.Spec.Ports[i].Protocol, "cannot create an external load balancer with non-TCP ports"))
 1089           }

pkg/cloudprovider/servicecontroller/servicecontroller.go:
  461       // it's supported.
  462       sp := &service.Spec.Ports[i]
  463:      if sp.Protocol != api.ProtocolTCP {
  464           return nil, fmt.Errorf("external load balancers for non TCP services are not currently supported.")
  465       }

pkg/master/controller.go:
  241   }
  242   p := &sub.Ports[0]
  243:  if p.Port != port || p.Protocol != api.ProtocolTCP {
  244       return false, false
  245   }

Other places where exact comparisons are expected between objects:

contrib/mesos/pkg/service/endpoints_controller.go:
400:                if port.Name == name && port.Protocol == svcPort.Protocol {
401:                    return findMappedPortName(pod, port.Protocol, name)

contrib/mesos/pkg/service/endpoints_controller.go:
414:                if port.ContainerPort == p && port.Protocol == svcPort.Protocol {
415:                    return findMappedPort(pod, port.Protocol, p)

pkg/service/endpoints_controller.go:
406:                if port.Name == name && port.Protocol == svcPort.Protocol {

Allowing a service or endpoint to be created with loose protocol validation, only to fail other parts of the system with strict checking isn't good.

[k8s-bot]

GCE e2e build/test passed for commit ee4cd97.

• Build Log
• Test Artifacts
• Internal Jenkins Results

[saad-ali]

Assigning @bgrant0607
v1.0 triage needed

[thockin]

We don't allow caseless compare anywhere else. If we wanted to allow it (I don't think we do) we should normalize case during defaulting, and that just sounds tedious and error prone.

[bgrant0607]

No, we shouldn't normalize case during defaulting. That would violate one of my API rules, which is that we don't munge field values provided by the user.

I agree this is an annoying quirk, but I don't know that we can fix it at this late stage.

[liggitt]

+1 on not mutating. Is it even possible for lowercase values to work? From what I saw, they would fail equality checks and not actually drive behavior

[thockin]

Yeah, we should just do it strictly.

[thockin]

LGTM
I think we should take this.