From a8f02e54721f4f22abb38c1056dcb620282839d3 Mon Sep 17 00:00:00 2001 From: Brendan Burns Date: Wed, 8 Jul 2015 22:02:10 -0700 Subject: [PATCH] Automatically open a firewall when creating a GCE load balancer. --- pkg/api/validation/validation.go | 8 ++++ pkg/api/validation/validation_test.go | 10 +++++ pkg/cloudprovider/gce/gce.go | 56 ++++++++++++++++++++++++++- pkg/cloudprovider/gce/gce_test.go | 24 ++++++++++++ pkg/kubectl/cmd/expose.go | 10 ----- test/e2e/service.go | 6 ++- 6 files changed, 102 insertions(+), 12 deletions(-) diff --git a/pkg/api/validation/validation.go b/pkg/api/validation/validation.go index 0436a7edb890a..3328c46fb117e 100644 --- a/pkg/api/validation/validation.go +++ b/pkg/api/validation/validation.go @@ -1039,6 +1039,14 @@ func ValidateService(service *api.Service) errs.ValidationErrorList { if len(service.Spec.Ports) == 0 { allErrs = append(allErrs, errs.NewFieldRequired("spec.ports")) } + if service.Spec.Type == api.ServiceTypeLoadBalancer { + for ix := range service.Spec.Ports { + port := &service.Spec.Ports[ix] + if port.Port == 10250 { + allErrs = append(allErrs, errs.NewFieldInvalid(fmt.Sprintf("spec.ports[%d].port", ix), port.Port, "can not expose port 10250 externally since it is used by kubelet")) + } + } + } allPortNames := util.StringSet{} for i := range service.Spec.Ports { allErrs = append(allErrs, validateServicePort(&service.Spec.Ports[i], len(service.Spec.Ports) > 1, &allPortNames).PrefixIndex(i).Prefix("spec.ports")...) diff --git a/pkg/api/validation/validation_test.go b/pkg/api/validation/validation_test.go index 82e2703a21465..cad441b91cfdd 100644 --- a/pkg/api/validation/validation_test.go +++ b/pkg/api/validation/validation_test.go @@ -1833,6 +1833,16 @@ func TestValidateService(t *testing.T) { }, numErrs: 0, }, + { + // For now we open firewalls, and its insecure if we open 10250, remove this + // when we have better protections in place. + name: "invalid port type=LoadBalancer", + tweakSvc: func(s *api.Service) { + s.Spec.Type = api.ServiceTypeLoadBalancer + s.Spec.Ports = append(s.Spec.Ports, api.ServicePort{Name: "kubelet", Port: 10250, Protocol: "TCP", TargetPort: util.NewIntOrStringFromInt(12345)}) + }, + numErrs: 1, + }, } for _, tc := range testCases { diff --git a/pkg/cloudprovider/gce/gce.go b/pkg/cloudprovider/gce/gce.go index e18b4ca5902a4..d2a0752fec80b 100644 --- a/pkg/cloudprovider/gce/gce.go +++ b/pkg/cloudprovider/gce/gce.go @@ -333,6 +333,10 @@ func translateAffinityType(affinityType api.ServiceAffinity) GCEAffinityType { } } +func makeFirewallName(name string) string { + return fmt.Sprintf("k8s-fw-%s", name) +} + // CreateTCPLoadBalancer is an implementation of TCPLoadBalancer.CreateTCPLoadBalancer. // TODO(a-robinson): Don't just ignore specified IP addresses. Check if they're // owned by the project and available to be used, and use them if they are. @@ -379,11 +383,44 @@ func (gce *GCECloud) CreateTCPLoadBalancer(name, region string, externalIP net.I return nil, err } + allowedPorts := make([]string, len(ports)) + for ix := range ports { + allowedPorts[ix] = strconv.Itoa(ports[ix].Port) + } + + hostTag := gce.computeHostTag(hosts[0]) + + firewall := &compute.Firewall{ + Name: makeFirewallName(name), + Description: fmt.Sprintf("KubernetesAutoGenerated_OnlyAllowTrafficForDestinationIP_%s", fwd.IPAddress), + Network: gce.gceNetworkName(), + SourceRanges: []string{"0.0.0.0/0"}, + TargetTags: []string{hostTag}, + Allowed: []*compute.FirewallAllowed{ + { + IPProtocol: "tcp", + Ports: allowedPorts, + }, + }, + } + if op, err = gce.service.Firewalls.Insert(gce.projectID, firewall).Do(); err != nil && !isHTTPErrorCode(err, http.StatusConflict) { + return nil, err + } + if err = gce.waitForGlobalOp(op); err != nil && !isHTTPErrorCode(err, http.StatusConflict) { + return nil, err + } + status := &api.LoadBalancerStatus{} status.Ingress = []api.LoadBalancerIngress{{IP: fwd.IPAddress}} return status, nil } +// This is kind of hacky, but the managed instance group adds 4 random chars and a hyphen +// to the base name. +func (gce *GCECloud) computeHostTag(host string) string { + return host[:len(host)-5] +} + // UpdateTCPLoadBalancer is an implementation of TCPLoadBalancer.UpdateTCPLoadBalancer. func (gce *GCECloud) UpdateTCPLoadBalancer(name, region string, hosts []string) error { pool, err := gce.service.TargetPools.Get(gce.projectID, region, name).Do() @@ -456,6 +493,19 @@ func (gce *GCECloud) EnsureTCPLoadBalancerDeleted(name, region string) error { if err != nil { glog.Warningf("Failed waiting for Target Pool %s to be deleted: got error %s.", name, err.Error()) } + fwName := makeFirewallName(name) + op, err = gce.service.Firewalls.Delete(gce.projectID, fwName).Do() + if err != nil && isHTTPErrorCode(err, http.StatusNotFound) { + glog.Infof("Firewall doesn't exist, moving on to deleting target pool.") + } else if err != nil { + glog.Warningf("Failed to delete firewall %s, got error %v", fwName, err) + return err + } else { + if err = gce.waitForGlobalOp(op); err != nil { + glog.Warningf("Failed waiting for Firewall %s to be deleted. Got error: %v", fwName, err) + return err + } + } return err } @@ -670,6 +720,10 @@ func (gce *GCECloud) ListRoutes(clusterName string) ([]*cloudprovider.Route, err return routes, nil } +func (gce *GCECloud) gceNetworkName() string { + return fmt.Sprintf("global/networks/%s", gce.networkName) +} + func (gce *GCECloud) CreateRoute(clusterName string, nameHint string, route *cloudprovider.Route) error { routeName := truncateClusterName(clusterName) + "-" + nameHint @@ -678,7 +732,7 @@ func (gce *GCECloud) CreateRoute(clusterName string, nameHint string, route *clo Name: routeName, DestRange: route.DestinationCIDR, NextHopInstance: fmt.Sprintf("zones/%s/instances/%s", gce.zone, instanceName), - Network: fmt.Sprintf("global/networks/%s", gce.networkName), + Network: gce.gceNetworkName(), Priority: 1000, Description: k8sNodeRouteTag, }).Do() diff --git a/pkg/cloudprovider/gce/gce_test.go b/pkg/cloudprovider/gce/gce_test.go index 798175d9a0b37..6164b2cf6e7a0 100644 --- a/pkg/cloudprovider/gce/gce_test.go +++ b/pkg/cloudprovider/gce/gce_test.go @@ -36,3 +36,27 @@ func TestGetRegion(t *testing.T) { t.Errorf("Unexpected region: %s", zone.Region) } } + +func TestGetHostTag(t *testing.T) { + tests := []struct { + host string + expected string + }{ + { + host: "kubernetes-minion-559o", + expected: "kubernetes-minion", + }, + { + host: "gke-test-ea6e8c80-node-8ytk", + expected: "gke-test-ea6e8c80-node", + }, + } + + gce := &GCECloud{} + for _, test := range tests { + hostTag := gce.computeHostTag(test.host) + if hostTag != test.expected { + t.Errorf("expected: %s, saw: %s for %s", test.expected, hostTag, test.host) + } + } +} diff --git a/pkg/kubectl/cmd/expose.go b/pkg/kubectl/cmd/expose.go index 3ac5680a76caa..53d8f17c792af 100644 --- a/pkg/kubectl/cmd/expose.go +++ b/pkg/kubectl/cmd/expose.go @@ -201,15 +201,5 @@ func RunExpose(f *cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []str } } - if cmdutil.GetFlagBool(cmd, "create-external-load-balancer") { - msg := fmt.Sprintf(` - An external load-balanced service was created. On many platforms (e.g. Google Compute Engine), - you will also need to explicitly open a firewall rule for the service port (%d) to serve traffic. - - See https://github.com/GoogleCloudPlatform/kubernetes/tree/master/docs/services-firewall.md for more details. - `, cmdutil.GetFlagInt(cmd, "port")) - out.Write([]byte(msg)) - } - return f.PrintObject(cmd, object, out) } diff --git a/test/e2e/service.go b/test/e2e/service.go index 7d751e6bc9cae..295c151497f73 100644 --- a/test/e2e/service.go +++ b/test/e2e/service.go @@ -245,8 +245,12 @@ var _ = Describe("Services", func() { } }() + inboundPort := 3000 + service := t.BuildServiceSpec() service.Spec.Type = api.ServiceTypeLoadBalancer + service.Spec.Ports[0].Port = inboundPort + service.Spec.Ports[0].TargetPort = util.NewIntOrStringFromInt(80) By("creating service " + serviceName + " with external load balancer in namespace " + ns) result, err := t.CreateService(service) @@ -278,7 +282,7 @@ var _ = Describe("Services", func() { testReachable(pickMinionIP(c), port.NodePort) By("hitting the pod through the service's external load balancer") - testLoadBalancerReachable(ingress, 80) + testLoadBalancerReachable(ingress, inboundPort) }) It("should be able to create a functioning NodePort service", func() {