From 7436fc6261968443f6e86069bb86934b7b8b1343 Mon Sep 17 00:00:00 2001 From: Prashanth Balasubramanian Date: Mon, 28 Mar 2016 10:29:31 -0700 Subject: [PATCH] Default firewall port to TCP when unspecified. --- pkg/cloudprovider/providers/gce/gce.go | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/pkg/cloudprovider/providers/gce/gce.go b/pkg/cloudprovider/providers/gce/gce.go index 295a6530a1f..c23da8ee3de 100644 --- a/pkg/cloudprovider/providers/gce/gce.go +++ b/pkg/cloudprovider/providers/gce/gce.go @@ -924,6 +924,11 @@ func (gce *GCECloud) firewallObject(name, region, desc string, sourceRanges nets TargetTags: hostTags, Allowed: []*compute.FirewallAllowed{ { + // TODO: Make this more generic. Currently this method is only + // used to create firewall rules for loadbalancers, which have + // exactly one protocol, so we can never end up with a list of + // mixed TCP and UDP ports. It should be possible to use a + // single firewall rule for both a TCP and UDP lb. IPProtocol: strings.ToLower(string(ports[0].Protocol)), Ports: allowedPorts, }, @@ -1237,8 +1242,13 @@ func (gce *GCECloud) CreateFirewall(name, desc string, sourceRanges netsets.IPNe // TODO: This completely breaks modularity in the cloudprovider but the methods // shared with the TCPLoadBalancer take api.ServicePorts. svcPorts := []api.ServicePort{} + // TODO: Currently the only consumer of this method is the GCE L7 + // loadbalancer controller, which never needs a protocol other than TCP. + // We should pipe through a mapping of port:protocol and default to TCP + // if UDP ports are required. This means the method signature will change + // forcing downstream clients to refactor interfaces. for _, p := range ports { - svcPorts = append(svcPorts, api.ServicePort{Port: int(p)}) + svcPorts = append(svcPorts, api.ServicePort{Port: int(p), Protocol: api.ProtocolTCP}) } hosts, err := gce.getInstancesByNames(hostNames) if err != nil { @@ -1266,8 +1276,13 @@ func (gce *GCECloud) UpdateFirewall(name, desc string, sourceRanges netsets.IPNe // TODO: This completely breaks modularity in the cloudprovider but the methods // shared with the TCPLoadBalancer take api.ServicePorts. svcPorts := []api.ServicePort{} + // TODO: Currently the only consumer of this method is the GCE L7 + // loadbalancer controller, which never needs a protocol other than TCP. + // We should pipe through a mapping of port:protocol and default to TCP + // if UDP ports are required. This means the method signature will change, + // forcing downstream clients to refactor interfaces. for _, p := range ports { - svcPorts = append(svcPorts, api.ServicePort{Port: int(p)}) + svcPorts = append(svcPorts, api.ServicePort{Port: int(p), Protocol: api.ProtocolTCP}) } hosts, err := gce.getInstancesByNames(hostNames) if err != nil {