Merge pull request #10974 from brendandburns/lakesong

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")...)

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 {

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()

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)
+		}
+	}
+}

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)
 }

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() {