diff --git a/api/swagger-spec/v1.json b/api/swagger-spec/v1.json index 3d57c43823d..e03d8aab10e 100644 --- a/api/swagger-spec/v1.json +++ b/api/swagger-spec/v1.json @@ -13430,12 +13430,12 @@ "type": "string", "description": "type of this service; must be ClusterIP, NodePort, or LoadBalancer; defaults to ClusterIP; see http://releases.k8s.io/HEAD/docs/user-guide/services.md#external-services" }, - "deprecatedPublicIPs": { + "externalIPs": { "type": "array", "items": { "type": "string" }, - "description": "deprecated. externally visible IPs (e.g. load balancers) that should be proxied to this service" + "description": "externally visible IPs (e.g. load balancers) that should be proxied to this service" }, "sessionAffinity": { "type": "string", diff --git a/contrib/completions/bash/kubectl b/contrib/completions/bash/kubectl index b3174e1ea97..46084270960 100644 --- a/contrib/completions/bash/kubectl +++ b/contrib/completions/bash/kubectl @@ -749,6 +749,7 @@ _kubectl_expose() flags+=("--container-port=") flags+=("--create-external-load-balancer") flags+=("--dry-run") + flags+=("--external-ip=") flags+=("--filename=") flags_with_completion+=("--filename") flags_completion+=("__handle_filename_extension_flag json|yaml|yml") @@ -768,7 +769,6 @@ _kubectl_expose() flags+=("--overrides=") flags+=("--port=") flags+=("--protocol=") - flags+=("--public-ip=") flags+=("--selector=") flags+=("--session-affinity=") flags+=("--show-all") diff --git a/docs/man/man1/kubectl-expose.1 b/docs/man/man1/kubectl-expose.1 index b756c84d31c..9ce2652a2ac 100644 --- a/docs/man/man1/kubectl-expose.1 +++ b/docs/man/man1/kubectl-expose.1 @@ -34,6 +34,10 @@ re\-use the labels from the resource it exposes. \fB\-\-dry\-run\fP=false If true, only print the object that would be sent, without creating it. +.PP +\fB\-\-external\-ip\fP="" + External IP address to set for the service. The service can be accessed by this IP in addition to its generated service IP. + .PP \fB\-f\fP, \fB\-\-filename\fP=[] Filename, directory, or URL to a file identifying the resource to expose a service @@ -78,10 +82,6 @@ re\-use the labels from the resource it exposes. \fB\-\-protocol\fP="TCP" The network protocol for the service to be created. Default is 'tcp'. -.PP -\fB\-\-public\-ip\fP="" - Name of a public IP address to set for the service. The service will be assigned this IP in addition to its generated service IP. - .PP \fB\-\-selector\fP="" A label selector to use for this service. If empty (the default) infer the selector from the replication controller. diff --git a/docs/user-guide/kubectl/kubectl_expose.md b/docs/user-guide/kubectl/kubectl_expose.md index 39ac2ba5fed..2cee689f971 100644 --- a/docs/user-guide/kubectl/kubectl_expose.md +++ b/docs/user-guide/kubectl/kubectl_expose.md @@ -45,7 +45,7 @@ selector for a new Service on the specified port. If no labels are specified, th re-use the labels from the resource it exposes. ``` -kubectl expose (-f FILENAME | TYPE NAME) --port=port [--protocol=TCP|UDP] [--target-port=number-or-name] [--name=name] [--public-ip=ip] [--type=type] +kubectl expose (-f FILENAME | TYPE NAME) --port=port [--protocol=TCP|UDP] [--target-port=number-or-name] [--name=name] [----external-ip=external-ip-of-service] [--type=type] ``` ### Examples @@ -70,6 +70,7 @@ $ kubectl expose rc streamer --port=4100 --protocol=udp --name=video-stream --container-port="": Synonym for --target-port --create-external-load-balancer[=false]: If true, create an external load balancer for this service (trumped by --type). Implementation is cloud provider dependent. Default is 'false'. --dry-run[=false]: If true, only print the object that would be sent, without creating it. + --external-ip="": External IP address to set for the service. The service can be accessed by this IP in addition to its generated service IP. -f, --filename=[]: Filename, directory, or URL to a file identifying the resource to expose a service --generator="service/v2": The name of the API generator to use. There are 2 generators: 'service/v1' and 'service/v2'. The only difference between them is that service port in v1 is named 'default', while it is left unnamed in v2. Default is 'service/v2'. -h, --help[=false]: help for expose @@ -81,7 +82,6 @@ $ kubectl expose rc streamer --port=4100 --protocol=udp --name=video-stream --overrides="": An inline JSON override for the generated object. If this is non-empty, it is used to override the generated object. Requires that the object supply a valid apiVersion field. --port=-1: The port that the service should serve on. Copied from the resource being exposed, if unspecified --protocol="TCP": The network protocol for the service to be created. Default is 'tcp'. - --public-ip="": Name of a public IP address to set for the service. The service will be assigned this IP in addition to its generated service IP. --selector="": A label selector to use for this service. If empty (the default) infer the selector from the replication controller. --session-affinity="": If non-empty, set the session affinity for the service to this; legal values: 'None', 'ClientIP' -a, --show-all[=false]: When printing, show all resources (default hide terminated pods.) @@ -124,7 +124,7 @@ $ kubectl expose rc streamer --port=4100 --protocol=udp --name=video-stream * [kubectl](kubectl.md) - kubectl controls the Kubernetes cluster manager -###### Auto generated by spf13/cobra at 2015-08-20 22:01:12.478645014 +0000 UTC +###### Auto generated by spf13/cobra at 2015-08-20 23:09:42.260392956 +0000 UTC [![Analytics](https://kubernetes-site.appspot.com/UA-36037335-10/GitHub/docs/user-guide/kubectl/kubectl_expose.md?pixel)]() diff --git a/hack/verify-flags/known-flags.txt b/hack/verify-flags/known-flags.txt index ac0fe93c9ed..42063b5ef8a 100644 --- a/hack/verify-flags/known-flags.txt +++ b/hack/verify-flags/known-flags.txt @@ -77,6 +77,7 @@ executor-suicide-timeout experimental-keystone-url experimental-prefix external-hostname +external-ip failover-timeout file-check-frequency file-suffix @@ -180,7 +181,6 @@ proxy-bindall proxy-logv proxy-port-range public-address-override -public-ip pvclaimbinder-sync-period read-only-port really-crash-for-testing diff --git a/pkg/api/deep_copy_generated.go b/pkg/api/deep_copy_generated.go index 2a88c4cda36..d7fb8678a46 100644 --- a/pkg/api/deep_copy_generated.go +++ b/pkg/api/deep_copy_generated.go @@ -1949,13 +1949,13 @@ func deepCopy_api_ServiceSpec(in ServiceSpec, out *ServiceSpec, c *conversion.Cl } out.ClusterIP = in.ClusterIP out.Type = in.Type - if in.DeprecatedPublicIPs != nil { - out.DeprecatedPublicIPs = make([]string, len(in.DeprecatedPublicIPs)) - for i := range in.DeprecatedPublicIPs { - out.DeprecatedPublicIPs[i] = in.DeprecatedPublicIPs[i] + if in.ExternalIPs != nil { + out.ExternalIPs = make([]string, len(in.ExternalIPs)) + for i := range in.ExternalIPs { + out.ExternalIPs[i] = in.ExternalIPs[i] } } else { - out.DeprecatedPublicIPs = nil + out.ExternalIPs = nil } out.SessionAffinity = in.SessionAffinity return nil diff --git a/pkg/api/types.go b/pkg/api/types.go index 1698cda3872..a1a6c5eb4dc 100644 --- a/pkg/api/types.go +++ b/pkg/api/types.go @@ -1190,10 +1190,9 @@ type ServiceSpec struct { // Type determines how the service will be exposed. Valid options: ClusterIP, NodePort, LoadBalancer Type ServiceType `json:"type,omitempty"` - // DeprecatedPublicIPs are deprecated and silently ignored. - // Old behaviour: PublicIPs are used by external load balancers, or can be set by + // ExternalIPs are used by external load balancers, or can be set by // users to handle external traffic that arrives at a node. - DeprecatedPublicIPs []string `json:"deprecatedPublicIPs,omitempty"` + ExternalIPs []string `json:"externalIPs,omitempty"` // Required: Supports "ClientIP" and "None". Used to maintain session affinity. SessionAffinity ServiceAffinity `json:"sessionAffinity,omitempty"` diff --git a/pkg/api/v1/conversion_generated.go b/pkg/api/v1/conversion_generated.go index c0736192e24..2013941b7e2 100644 --- a/pkg/api/v1/conversion_generated.go +++ b/pkg/api/v1/conversion_generated.go @@ -2166,13 +2166,13 @@ func convert_api_ServiceSpec_To_v1_ServiceSpec(in *api.ServiceSpec, out *Service } out.ClusterIP = in.ClusterIP out.Type = ServiceType(in.Type) - if in.DeprecatedPublicIPs != nil { - out.DeprecatedPublicIPs = make([]string, len(in.DeprecatedPublicIPs)) - for i := range in.DeprecatedPublicIPs { - out.DeprecatedPublicIPs[i] = in.DeprecatedPublicIPs[i] + if in.ExternalIPs != nil { + out.ExternalIPs = make([]string, len(in.ExternalIPs)) + for i := range in.ExternalIPs { + out.ExternalIPs[i] = in.ExternalIPs[i] } } else { - out.DeprecatedPublicIPs = nil + out.ExternalIPs = nil } out.SessionAffinity = ServiceAffinity(in.SessionAffinity) return nil @@ -4581,13 +4581,13 @@ func convert_v1_ServiceSpec_To_api_ServiceSpec(in *ServiceSpec, out *api.Service } out.ClusterIP = in.ClusterIP out.Type = api.ServiceType(in.Type) - if in.DeprecatedPublicIPs != nil { - out.DeprecatedPublicIPs = make([]string, len(in.DeprecatedPublicIPs)) - for i := range in.DeprecatedPublicIPs { - out.DeprecatedPublicIPs[i] = in.DeprecatedPublicIPs[i] + if in.ExternalIPs != nil { + out.ExternalIPs = make([]string, len(in.ExternalIPs)) + for i := range in.ExternalIPs { + out.ExternalIPs[i] = in.ExternalIPs[i] } } else { - out.DeprecatedPublicIPs = nil + out.ExternalIPs = nil } out.SessionAffinity = api.ServiceAffinity(in.SessionAffinity) return nil diff --git a/pkg/api/v1/deep_copy_generated.go b/pkg/api/v1/deep_copy_generated.go index 24a3acdd9c5..9a850049631 100644 --- a/pkg/api/v1/deep_copy_generated.go +++ b/pkg/api/v1/deep_copy_generated.go @@ -1954,13 +1954,13 @@ func deepCopy_v1_ServiceSpec(in ServiceSpec, out *ServiceSpec, c *conversion.Clo } out.ClusterIP = in.ClusterIP out.Type = in.Type - if in.DeprecatedPublicIPs != nil { - out.DeprecatedPublicIPs = make([]string, len(in.DeprecatedPublicIPs)) - for i := range in.DeprecatedPublicIPs { - out.DeprecatedPublicIPs[i] = in.DeprecatedPublicIPs[i] + if in.ExternalIPs != nil { + out.ExternalIPs = make([]string, len(in.ExternalIPs)) + for i := range in.ExternalIPs { + out.ExternalIPs[i] = in.ExternalIPs[i] } } else { - out.DeprecatedPublicIPs = nil + out.ExternalIPs = nil } out.SessionAffinity = in.SessionAffinity return nil diff --git a/pkg/api/v1/types.go b/pkg/api/v1/types.go index 2ef351f9f21..29194fac952 100644 --- a/pkg/api/v1/types.go +++ b/pkg/api/v1/types.go @@ -1150,7 +1150,7 @@ type ServiceSpec struct { // Deprecated. PublicIPs are used by external load balancers, or can be set by // users to handle external traffic that arrives at a node. - DeprecatedPublicIPs []string `json:"deprecatedPublicIPs,omitempty" description:"deprecated. externally visible IPs (e.g. load balancers) that should be proxied to this service"` + ExternalIPs []string `json:"externalIPs,omitempty" description:"externally visible IPs (e.g. load balancers) that should be proxied to this service"` // Optional: Supports "ClientIP" and "None". Used to maintain session affinity. SessionAffinity ServiceAffinity `json:"sessionAffinity,omitempty" description:"enable client IP based session affinity; must be ClientIP or None; defaults to None; see http://releases.k8s.io/HEAD/docs/user-guide/services.md#virtual-ips-and-service-proxies"` diff --git a/pkg/api/validation/validation.go b/pkg/api/validation/validation.go index 14d707f3755..2703009b9c1 100644 --- a/pkg/api/validation/validation.go +++ b/pkg/api/validation/validation.go @@ -1090,12 +1090,11 @@ func ValidateService(service *api.Service) errs.ValidationErrorList { } } - for _, ip := range service.Spec.DeprecatedPublicIPs { + for _, ip := range service.Spec.ExternalIPs { if ip == "0.0.0.0" { - allErrs = append(allErrs, errs.NewFieldInvalid("spec.publicIPs", ip, "is not an IP address")) - } else if util.IsValidIPv4(ip) && net.ParseIP(ip).IsLoopback() { - allErrs = append(allErrs, errs.NewFieldInvalid("spec.publicIPs", ip, "publicIP cannot be a loopback")) + allErrs = append(allErrs, errs.NewFieldInvalid("spec.externalIPs", ip, "is not an IP address")) } + allErrs = append(allErrs, validateIpIsNotLinkLocalOrLoopback(ip, "spec.externalIPs")...) } if service.Spec.Type == "" { @@ -1740,18 +1739,26 @@ func validateEndpointAddress(address *api.EndpointAddress) errs.ValidationErrorL allErrs = append(allErrs, errs.NewFieldInvalid("ip", address.IP, "invalid IPv4 address")) return allErrs } - // We disallow some IPs as endpoints. Specifically, loopback addresses are - // nonsensical and link-local addresses tend to be used for node-centric - // purposes (e.g. metadata service). - ip := net.ParseIP(address.IP) + return validateIpIsNotLinkLocalOrLoopback(address.IP, "ip") +} + +func validateIpIsNotLinkLocalOrLoopback(ipAddress, fieldName string) errs.ValidationErrorList { + // We disallow some IPs as endpoints or external-ips. Specifically, loopback addresses are + // nonsensical and link-local addresses tend to be used for node-centric purposes (e.g. metadata service). + allErrs := errs.ValidationErrorList{} + ip := net.ParseIP(ipAddress) + if ip == nil { + allErrs = append(allErrs, errs.NewFieldInvalid(fieldName, ipAddress, "not a valid IP address")) + return allErrs + } if ip.IsLoopback() { - allErrs = append(allErrs, errs.NewFieldInvalid("ip", address.IP, "may not be in the loopback range (127.0.0.0/8)")) + allErrs = append(allErrs, errs.NewFieldInvalid(fieldName, ipAddress, "may not be in the loopback range (127.0.0.0/8)")) } if ip.IsLinkLocalUnicast() { - allErrs = append(allErrs, errs.NewFieldInvalid("ip", address.IP, "may not be in the link-local range (169.254.0.0/16)")) + allErrs = append(allErrs, errs.NewFieldInvalid(fieldName, ipAddress, "may not be in the link-local range (169.254.0.0/16)")) } if ip.IsLinkLocalMulticast() { - allErrs = append(allErrs, errs.NewFieldInvalid("ip", address.IP, "may not be in the link-local multicast range (224.0.0.0/24)")) + allErrs = append(allErrs, errs.NewFieldInvalid(fieldName, ipAddress, "may not be in the link-local multicast range (224.0.0.0/24)")) } return allErrs } diff --git a/pkg/api/validation/validation_test.go b/pkg/api/validation/validation_test.go index b30e61457a9..bc0e9d7c740 100644 --- a/pkg/api/validation/validation_test.go +++ b/pkg/api/validation/validation_test.go @@ -1721,23 +1721,23 @@ func TestValidateService(t *testing.T) { { name: "invalid publicIPs localhost", tweakSvc: func(s *api.Service) { - s.Spec.DeprecatedPublicIPs = []string{"127.0.0.1"} + s.Spec.ExternalIPs = []string{"127.0.0.1"} }, numErrs: 1, }, { name: "invalid publicIPs", tweakSvc: func(s *api.Service) { - s.Spec.DeprecatedPublicIPs = []string{"0.0.0.0"} + s.Spec.ExternalIPs = []string{"0.0.0.0"} }, numErrs: 1, }, { - name: "valid publicIPs host", + name: "invalid publicIPs host", tweakSvc: func(s *api.Service) { - s.Spec.DeprecatedPublicIPs = []string{"myhost.mydomain"} + s.Spec.ExternalIPs = []string{"myhost.mydomain"} }, - numErrs: 0, + numErrs: 1, }, { name: "dup port name", diff --git a/pkg/controller/service/servicecontroller.go b/pkg/controller/service/servicecontroller.go index 4159edca834..e92e4b2f270 100644 --- a/pkg/controller/service/servicecontroller.go +++ b/pkg/controller/service/servicecontroller.go @@ -378,8 +378,8 @@ func (s *ServiceController) createExternalLoadBalancer(service *api.Service) err return err } name := s.loadBalancerName(service) - if len(service.Spec.DeprecatedPublicIPs) > 0 { - for _, publicIP := range service.Spec.DeprecatedPublicIPs { + if len(service.Spec.ExternalIPs) > 0 { + for _, publicIP := range service.Spec.ExternalIPs { // TODO: Make this actually work for multiple IPs by using different // names for each. For now, we'll just create the first and break. status, err := s.balancer.EnsureTCPLoadBalancer(name, s.zone.Region, net.ParseIP(publicIP), @@ -477,11 +477,11 @@ func needsUpdate(oldService *api.Service, newService *api.Service) bool { if !portsEqualForLB(oldService, newService) || oldService.Spec.SessionAffinity != newService.Spec.SessionAffinity { return true } - if len(oldService.Spec.DeprecatedPublicIPs) != len(newService.Spec.DeprecatedPublicIPs) { + if len(oldService.Spec.ExternalIPs) != len(newService.Spec.ExternalIPs) { return true } - for i := range oldService.Spec.DeprecatedPublicIPs { - if oldService.Spec.DeprecatedPublicIPs[i] != newService.Spec.DeprecatedPublicIPs[i] { + for i := range oldService.Spec.ExternalIPs { + if oldService.Spec.ExternalIPs[i] != newService.Spec.ExternalIPs[i] { return true } } diff --git a/pkg/kubectl/cmd/expose.go b/pkg/kubectl/cmd/expose.go index 46f9fa784dd..2c4c8b8ae30 100644 --- a/pkg/kubectl/cmd/expose.go +++ b/pkg/kubectl/cmd/expose.go @@ -49,7 +49,7 @@ $ kubectl expose rc streamer --port=4100 --protocol=udp --name=video-stream` func NewCmdExposeService(f *cmdutil.Factory, out io.Writer) *cobra.Command { cmd := &cobra.Command{ - Use: "expose (-f FILENAME | TYPE NAME) --port=port [--protocol=TCP|UDP] [--target-port=number-or-name] [--name=name] [--public-ip=ip] [--type=type]", + Use: "expose (-f FILENAME | TYPE NAME) --port=port [--protocol=TCP|UDP] [--target-port=number-or-name] [--name=name] [----external-ip=external-ip-of-service] [--type=type]", Short: "Take a replicated application and expose it as Kubernetes Service", Long: expose_long, Example: expose_example, @@ -70,7 +70,7 @@ func NewCmdExposeService(f *cmdutil.Factory, out io.Writer) *cobra.Command { cmd.Flags().Bool("dry-run", false, "If true, only print the object that would be sent, without creating it.") cmd.Flags().String("container-port", "", "Synonym for --target-port") cmd.Flags().String("target-port", "", "Name or number for the port on the container that the service should direct traffic to. Optional.") - cmd.Flags().String("public-ip", "", "Name of a public IP address to set for the service. The service will be assigned this IP in addition to its generated service IP.") + cmd.Flags().String("external-ip", "", "External IP address to set for the service. The service can be accessed by this IP in addition to its generated service IP.") cmd.Flags().String("overrides", "", "An inline JSON override for the generated object. If this is non-empty, it is used to override the generated object. Requires that the object supply a valid apiVersion field.") cmd.Flags().String("name", "", "The name for the newly created object.") cmd.Flags().String("session-affinity", "", "If non-empty, set the session affinity for the service to this; legal values: 'None', 'ClientIP'") diff --git a/pkg/kubectl/resource_printer.go b/pkg/kubectl/resource_printer.go index 971eb0425a0..9f3473213a4 100644 --- a/pkg/kubectl/resource_printer.go +++ b/pkg/kubectl/resource_printer.go @@ -585,8 +585,14 @@ func printReplicationControllerList(list *api.ReplicationControllerList, w io.Wr func getServiceExternalIP(svc *api.Service) string { switch svc.Spec.Type { case api.ServiceTypeClusterIP: + if len(svc.Spec.ExternalIPs) > 0 { + return strings.Join(svc.Spec.ExternalIPs, ",") + } return "" case api.ServiceTypeNodePort: + if len(svc.Spec.ExternalIPs) > 0 { + return strings.Join(svc.Spec.ExternalIPs, ",") + } return "nodes" case api.ServiceTypeLoadBalancer: ingress := svc.Status.LoadBalancer.Ingress @@ -596,6 +602,9 @@ func getServiceExternalIP(svc *api.Service) string { result = append(result, ingress[i].IP) } } + if len(svc.Spec.ExternalIPs) > 0 { + result = append(result, svc.Spec.ExternalIPs...) + } return strings.Join(result, ",") } return "unknown" diff --git a/pkg/kubectl/service.go b/pkg/kubectl/service.go index cbf03a0427c..927fa1e820a 100644 --- a/pkg/kubectl/service.go +++ b/pkg/kubectl/service.go @@ -18,11 +18,10 @@ package kubectl import ( "fmt" - "strconv" - "k8s.io/kubernetes/pkg/api" "k8s.io/kubernetes/pkg/runtime" "k8s.io/kubernetes/pkg/util" + "strconv" ) // The only difference between ServiceGeneratorV1 and V2 is that the service port is named "default" in V1, while it is left unnamed in V2. @@ -54,7 +53,7 @@ func paramNames() []GeneratorParam { {"selector", true}, {"port", true}, {"labels", false}, - {"public-ip", false}, + {"external-ip", false}, {"create-external-load-balancer", false}, {"type", false}, {"protocol", false}, @@ -144,8 +143,8 @@ func generate(genericParams map[string]interface{}) (runtime.Object, error) { if params["create-external-load-balancer"] == "true" { service.Spec.Type = api.ServiceTypeLoadBalancer } - if len(params["public-ip"]) != 0 { - service.Spec.DeprecatedPublicIPs = []string{params["public-ip"]} + if len(params["external-ip"]) > 0 { + service.Spec.ExternalIPs = []string{params["external-ip"]} } if len(params["type"]) != 0 { service.Spec.Type = api.ServiceType(params["type"]) diff --git a/pkg/kubectl/service_test.go b/pkg/kubectl/service_test.go index 6d8208014d1..5345bdf70e8 100644 --- a/pkg/kubectl/service_test.go +++ b/pkg/kubectl/service_test.go @@ -128,7 +128,7 @@ func TestGenerateService(t *testing.T) { "port": "80", "protocol": "UDP", "container-port": "foobar", - "public-ip": "1.2.3.4", + "external-ip": "1.2.3.4", }, expected: api.Service{ ObjectMeta: api.ObjectMeta{ @@ -146,7 +146,7 @@ func TestGenerateService(t *testing.T) { TargetPort: util.NewIntOrStringFromString("foobar"), }, }, - DeprecatedPublicIPs: []string{"1.2.3.4"}, + ExternalIPs: []string{"1.2.3.4"}, }, }, }, @@ -158,7 +158,7 @@ func TestGenerateService(t *testing.T) { "port": "80", "protocol": "UDP", "container-port": "foobar", - "public-ip": "1.2.3.4", + "external-ip": "1.2.3.4", "create-external-load-balancer": "true", }, expected: api.Service{ @@ -177,8 +177,8 @@ func TestGenerateService(t *testing.T) { TargetPort: util.NewIntOrStringFromString("foobar"), }, }, - Type: api.ServiceTypeLoadBalancer, - DeprecatedPublicIPs: []string{"1.2.3.4"}, + Type: api.ServiceTypeLoadBalancer, + ExternalIPs: []string{"1.2.3.4"}, }, }, }, diff --git a/pkg/proxy/iptables/proxier.go b/pkg/proxy/iptables/proxier.go index d3d800b8d42..caf93065140 100644 --- a/pkg/proxy/iptables/proxier.go +++ b/pkg/proxy/iptables/proxier.go @@ -130,7 +130,7 @@ type serviceInfo struct { stickyMaxAgeSeconds int endpoints []string // Deprecated, but required for back-compat (including e2e) - deprecatedPublicIPs []string + externalIPs []string } // returns a new serviceInfo struct @@ -236,7 +236,7 @@ func (proxier *Proxier) sameConfig(info *serviceInfo, service *api.Service, port if !info.clusterIP.Equal(net.ParseIP(service.Spec.ClusterIP)) { return false } - if !ipsEqual(info.deprecatedPublicIPs, service.Spec.DeprecatedPublicIPs) { + if !ipsEqual(info.externalIPs, service.Spec.ExternalIPs) { return false } if !api.LoadBalancerStatusEqual(&info.loadBalancerStatus, &service.Status.LoadBalancer) { @@ -318,7 +318,7 @@ func (proxier *Proxier) OnServiceUpdate(allServices []api.Service) { info.port = servicePort.Port info.protocol = servicePort.Protocol info.nodePort = servicePort.NodePort - info.deprecatedPublicIPs = service.Spec.DeprecatedPublicIPs + info.externalIPs = service.Spec.ExternalIPs // Deep-copy in case the service instance changes info.loadBalancerStatus = *api.LoadBalancerStatusDeepCopy(&service.Status.LoadBalancer) info.sessionAffinityType = service.Spec.SessionAffinity @@ -556,7 +556,7 @@ func (proxier *Proxier) syncProxyRules() error { "-j", string(svcChain)) // Capture externalIPs. - for _, externalIP := range info.deprecatedPublicIPs { + for _, externalIP := range info.externalIPs { args := []string{ "-A", string(iptablesServicesChain), "-m", "comment", "--comment", fmt.Sprintf("\"%s external IP\"", name.String()), @@ -564,10 +564,23 @@ func (proxier *Proxier) syncProxyRules() error { "-d", fmt.Sprintf("%s/32", externalIP), "--dport", fmt.Sprintf("%d", info.port), } - // We have to SNAT packets from external IPs. + // We have to SNAT packets to external IPs. writeLine(rulesLines, append(args, "-j", "MARK", "--set-xmark", fmt.Sprintf("%s/0xffffffff", iptablesMasqueradeMark))...) - writeLine(rulesLines, append(args, + + // Allow traffic for external IPs that does not come from a bridge (i.e. not from a container) + // nor from a local process to be forwarded to the service. + // This rule roughly translates to "all traffic from off-machine". + // This is imperfect in the face of network plugins that might not use a bridge, but we can revisit that later. + externalTrafficOnlyArgs := append(args, + "-m", "physdev", "!", "--physdev-is-in", + "-m", "addrtype", "!", "--src-type", "LOCAL") + writeLine(rulesLines, append(externalTrafficOnlyArgs, + "-j", string(svcChain))...) + dstLocalOnlyArgs := append(args, "-m", "addrtype", "--dst-type", "LOCAL") + // Allow traffic bound for external IPs that happen to be recognized as local IPs to stay local. + // This covers cases like GCE load-balancers which get added to the local routing table. + writeLine(rulesLines, append(dstLocalOnlyArgs, "-j", string(svcChain))...) } diff --git a/pkg/proxy/userspace/proxier.go b/pkg/proxy/userspace/proxier.go index d888570f212..8084a01c162 100644 --- a/pkg/proxy/userspace/proxier.go +++ b/pkg/proxy/userspace/proxier.go @@ -35,8 +35,9 @@ import ( ) type portal struct { - ip net.IP - port int + ip net.IP + port int + isExternal bool } type serviceInfo struct { @@ -51,7 +52,7 @@ type serviceInfo struct { sessionAffinityType api.ServiceAffinity stickyMaxAgeMinutes int // Deprecated, but required for back-compat (including e2e) - deprecatedPublicIPs []string + externalIPs []string } func logTimeout(err error) bool { @@ -330,7 +331,7 @@ func (proxier *Proxier) OnServiceUpdate(services []api.Service) { } info.portal.ip = serviceIP info.portal.port = servicePort.Port - info.deprecatedPublicIPs = service.Spec.DeprecatedPublicIPs + info.externalIPs = service.Spec.ExternalIPs // Deep-copy in case the service instance changes info.loadBalancerStatus = *api.LoadBalancerStatusDeepCopy(&service.Status.LoadBalancer) info.nodePort = servicePort.NodePort @@ -368,7 +369,7 @@ func sameConfig(info *serviceInfo, service *api.Service, port *api.ServicePort) if !info.portal.ip.Equal(net.ParseIP(service.Spec.ClusterIP)) { return false } - if !ipsEqual(info.deprecatedPublicIPs, service.Spec.DeprecatedPublicIPs) { + if !ipsEqual(info.externalIPs, service.Spec.ExternalIPs) { return false } if !api.LoadBalancerStatusEqual(&info.loadBalancerStatus, &service.Status.LoadBalancer) { @@ -397,15 +398,15 @@ func (proxier *Proxier) openPortal(service proxy.ServicePortName, info *serviceI if err != nil { return err } - for _, publicIP := range info.deprecatedPublicIPs { - err = proxier.openOnePortal(portal{net.ParseIP(publicIP), info.portal.port}, info.protocol, proxier.listenIP, info.proxyPort, service) + for _, publicIP := range info.externalIPs { + err = proxier.openOnePortal(portal{net.ParseIP(publicIP), info.portal.port, true}, info.protocol, proxier.listenIP, info.proxyPort, service) if err != nil { return err } } for _, ingress := range info.loadBalancerStatus.Ingress { if ingress.IP != "" { - err = proxier.openOnePortal(portal{net.ParseIP(ingress.IP), info.portal.port}, info.protocol, proxier.listenIP, info.proxyPort, service) + err = proxier.openOnePortal(portal{net.ParseIP(ingress.IP), info.portal.port, false}, info.protocol, proxier.listenIP, info.proxyPort, service) if err != nil { return err } @@ -422,18 +423,40 @@ func (proxier *Proxier) openPortal(service proxy.ServicePortName, info *serviceI func (proxier *Proxier) openOnePortal(portal portal, protocol api.Protocol, proxyIP net.IP, proxyPort int, name proxy.ServicePortName) error { // Handle traffic from containers. - args := proxier.iptablesContainerPortalArgs(portal.ip, portal.port, protocol, proxyIP, proxyPort, name) + args := proxier.iptablesContainerPortalArgs(portal.ip, portal.isExternal, false, portal.port, protocol, proxyIP, proxyPort, name) existed, err := proxier.iptables.EnsureRule(iptables.Append, iptables.TableNAT, iptablesContainerPortalChain, args...) if err != nil { - glog.Errorf("Failed to install iptables %s rule for service %q", iptablesContainerPortalChain, name) + glog.Errorf("Failed to install iptables %s rule for service %q, args:%v", iptablesContainerPortalChain, name, args) return err } if !existed { glog.V(3).Infof("Opened iptables from-containers portal for service %q on %s %s:%d", name, protocol, portal.ip, portal.port) } + if portal.isExternal { + args := proxier.iptablesContainerPortalArgs(portal.ip, false, true, portal.port, protocol, proxyIP, proxyPort, name) + existed, err := proxier.iptables.EnsureRule(iptables.Append, iptables.TableNAT, iptablesContainerPortalChain, args...) + if err != nil { + glog.Errorf("Failed to install iptables %s rule that opens service %q for local traffic, args:%v", iptablesContainerPortalChain, name, args) + return err + } + if !existed { + glog.V(3).Infof("Opened iptables from-containers portal for service %q on %s %s:%d for local traffic", name, protocol, portal.ip, portal.port) + } + + args = proxier.iptablesHostPortalArgs(portal.ip, true, portal.port, protocol, proxyIP, proxyPort, name) + existed, err = proxier.iptables.EnsureRule(iptables.Append, iptables.TableNAT, iptablesHostPortalChain, args...) + if err != nil { + glog.Errorf("Failed to install iptables %s rule for service %q for dst-local traffic", iptablesHostPortalChain, name) + return err + } + if !existed { + glog.V(3).Infof("Opened iptables from-host portal for service %q on %s %s:%d for dst-local traffic", name, protocol, portal.ip, portal.port) + } + return nil + } // Handle traffic from the host. - args = proxier.iptablesHostPortalArgs(portal.ip, portal.port, protocol, proxyIP, proxyPort, name) + args = proxier.iptablesHostPortalArgs(portal.ip, false, portal.port, protocol, proxyIP, proxyPort, name) existed, err = proxier.iptables.EnsureRule(iptables.Append, iptables.TableNAT, iptablesHostPortalChain, args...) if err != nil { glog.Errorf("Failed to install iptables %s rule for service %q", iptablesHostPortalChain, name) @@ -522,12 +545,12 @@ func (proxier *Proxier) openNodePort(nodePort int, protocol api.Protocol, proxyI func (proxier *Proxier) closePortal(service proxy.ServicePortName, info *serviceInfo) error { // Collect errors and report them all at the end. el := proxier.closeOnePortal(info.portal, info.protocol, proxier.listenIP, info.proxyPort, service) - for _, publicIP := range info.deprecatedPublicIPs { - el = append(el, proxier.closeOnePortal(portal{net.ParseIP(publicIP), info.portal.port}, info.protocol, proxier.listenIP, info.proxyPort, service)...) + for _, publicIP := range info.externalIPs { + el = append(el, proxier.closeOnePortal(portal{net.ParseIP(publicIP), info.portal.port, true}, info.protocol, proxier.listenIP, info.proxyPort, service)...) } for _, ingress := range info.loadBalancerStatus.Ingress { if ingress.IP != "" { - el = append(el, proxier.closeOnePortal(portal{net.ParseIP(ingress.IP), info.portal.port}, info.protocol, proxier.listenIP, info.proxyPort, service)...) + el = append(el, proxier.closeOnePortal(portal{net.ParseIP(ingress.IP), info.portal.port, false}, info.protocol, proxier.listenIP, info.proxyPort, service)...) } } if info.nodePort != 0 { @@ -545,14 +568,29 @@ func (proxier *Proxier) closeOnePortal(portal portal, protocol api.Protocol, pro el := []error{} // Handle traffic from containers. - args := proxier.iptablesContainerPortalArgs(portal.ip, portal.port, protocol, proxyIP, proxyPort, name) + args := proxier.iptablesContainerPortalArgs(portal.ip, portal.isExternal, false, portal.port, protocol, proxyIP, proxyPort, name) if err := proxier.iptables.DeleteRule(iptables.TableNAT, iptablesContainerPortalChain, args...); err != nil { glog.Errorf("Failed to delete iptables %s rule for service %q", iptablesContainerPortalChain, name) el = append(el, err) } - // Handle traffic from the host. - args = proxier.iptablesHostPortalArgs(portal.ip, portal.port, protocol, proxyIP, proxyPort, name) + if portal.isExternal { + args := proxier.iptablesContainerPortalArgs(portal.ip, false, true, portal.port, protocol, proxyIP, proxyPort, name) + if err := proxier.iptables.DeleteRule(iptables.TableNAT, iptablesContainerPortalChain, args...); err != nil { + glog.Errorf("Failed to delete iptables %s rule for service %q", iptablesContainerPortalChain, name) + el = append(el, err) + } + + args = proxier.iptablesHostPortalArgs(portal.ip, true, portal.port, protocol, proxyIP, proxyPort, name) + if err := proxier.iptables.DeleteRule(iptables.TableNAT, iptablesHostPortalChain, args...); err != nil { + glog.Errorf("Failed to delete iptables %s rule for service %q", iptablesHostPortalChain, name) + el = append(el, err) + } + return el + } + + // Handle traffic from the host (portalIP is not external). + args = proxier.iptablesHostPortalArgs(portal.ip, false, portal.port, protocol, proxyIP, proxyPort, name) if err := proxier.iptables.DeleteRule(iptables.TableNAT, iptablesHostPortalChain, args...); err != nil { glog.Errorf("Failed to delete iptables %s rule for service %q", iptablesHostPortalChain, name) el = append(el, err) @@ -681,7 +719,7 @@ var zeroIPv6 = net.ParseIP("::0") var localhostIPv6 = net.ParseIP("::1") // Build a slice of iptables args that are common to from-container and from-host portal rules. -func iptablesCommonPortalArgs(destIP net.IP, destPort int, protocol api.Protocol, service proxy.ServicePortName) []string { +func iptablesCommonPortalArgs(destIP net.IP, addPhysicalInterfaceMatch bool, addDstLocalMatch bool, destPort int, protocol api.Protocol, service proxy.ServicePortName) []string { // This list needs to include all fields as they are eventually spit out // by iptables-save. This is because some systems do not support the // 'iptables -C' arg, and so fall back on parsing iptables-save output. @@ -702,12 +740,20 @@ func iptablesCommonPortalArgs(destIP net.IP, destPort int, protocol api.Protocol args = append(args, "-d", fmt.Sprintf("%s/32", destIP.String())) } + if addPhysicalInterfaceMatch { + args = append(args, "-m", "physdev", "!", "--physdev-is-in") + } + + if addDstLocalMatch { + args = append(args, "-m", "addrtype", "--dst-type", "LOCAL") + } + return args } // Build a slice of iptables args for a from-container portal rule. -func (proxier *Proxier) iptablesContainerPortalArgs(destIP net.IP, destPort int, protocol api.Protocol, proxyIP net.IP, proxyPort int, service proxy.ServicePortName) []string { - args := iptablesCommonPortalArgs(destIP, destPort, protocol, service) +func (proxier *Proxier) iptablesContainerPortalArgs(destIP net.IP, addPhysicalInterfaceMatch bool, addDstLocalMatch bool, destPort int, protocol api.Protocol, proxyIP net.IP, proxyPort int, service proxy.ServicePortName) []string { + args := iptablesCommonPortalArgs(destIP, addPhysicalInterfaceMatch, addDstLocalMatch, destPort, protocol, service) // This is tricky. // @@ -753,8 +799,8 @@ func (proxier *Proxier) iptablesContainerPortalArgs(destIP net.IP, destPort int, } // Build a slice of iptables args for a from-host portal rule. -func (proxier *Proxier) iptablesHostPortalArgs(destIP net.IP, destPort int, protocol api.Protocol, proxyIP net.IP, proxyPort int, service proxy.ServicePortName) []string { - args := iptablesCommonPortalArgs(destIP, destPort, protocol, service) +func (proxier *Proxier) iptablesHostPortalArgs(destIP net.IP, addDstLocalMatch bool, destPort int, protocol api.Protocol, proxyIP net.IP, proxyPort int, service proxy.ServicePortName) []string { + args := iptablesCommonPortalArgs(destIP, false, addDstLocalMatch, destPort, protocol, service) // This is tricky. // @@ -789,7 +835,7 @@ func (proxier *Proxier) iptablesHostPortalArgs(destIP net.IP, destPort int, prot // See iptablesContainerPortalArgs // TODO: Should we just reuse iptablesContainerPortalArgs? func (proxier *Proxier) iptablesContainerNodePortArgs(nodePort int, protocol api.Protocol, proxyIP net.IP, proxyPort int, service proxy.ServicePortName) []string { - args := iptablesCommonPortalArgs(nil, nodePort, protocol, service) + args := iptablesCommonPortalArgs(nil, false, false, nodePort, protocol, service) if proxyIP.Equal(zeroIPv4) || proxyIP.Equal(zeroIPv6) { // TODO: Can we REDIRECT with IPv6? @@ -806,7 +852,7 @@ func (proxier *Proxier) iptablesContainerNodePortArgs(nodePort int, protocol api // See iptablesHostPortalArgs // TODO: Should we just reuse iptablesHostPortalArgs? func (proxier *Proxier) iptablesHostNodePortArgs(nodePort int, protocol api.Protocol, proxyIP net.IP, proxyPort int, service proxy.ServicePortName) []string { - args := iptablesCommonPortalArgs(nil, nodePort, protocol, service) + args := iptablesCommonPortalArgs(nil, false, false, nodePort, protocol, service) if proxyIP.Equal(zeroIPv4) || proxyIP.Equal(zeroIPv6) { proxyIP = proxier.hostIP diff --git a/pkg/proxy/userspace/proxier_test.go b/pkg/proxy/userspace/proxier_test.go index c36d6bdf0af..87704b67c5c 100644 --- a/pkg/proxy/userspace/proxier_test.go +++ b/pkg/proxy/userspace/proxier_test.go @@ -787,8 +787,8 @@ func TestProxyUpdatePublicIPs(t *testing.T) { Port: svcInfo.portal.port, Protocol: "TCP", }}, - ClusterIP: svcInfo.portal.ip.String(), - DeprecatedPublicIPs: []string{"4.3.2.1"}, + ClusterIP: svcInfo.portal.ip.String(), + ExternalIPs: []string{"4.3.2.1"}, }, }}) // Wait for the socket to actually get free.