Merge pull request #10713 from thockin/no-localhost-endpoints

Check loopback and link-local multicast endpoints

api/swagger-spec/api.json

@@ -1,7 +1,7 @@
 {
   "swaggerVersion": "1.2",
   "apiVersion": "",
-  "basePath": "https://127.0.0.1:6443",
+  "basePath": "https://10.10.10.10:6443",
   "resourcePath": "/api",
   "apis": [
    {

api/swagger-spec/v1.json

@@ -1,7 +1,7 @@
 {
   "swaggerVersion": "1.2",
   "apiVersion": "v1",
-  "basePath": "https://127.0.0.1:6443",
+  "basePath": "https://10.10.10.10:6443",
   "resourcePath": "/api/v1",
   "apis": [
    {
@@ -11230,7 +11230,7 @@
     "properties": {
      "ip": {
       "type": "string",
-      "description": "IP address of the endpoint"
+      "description": "IP address of the endpoint; may not be loopback (127.0.0.0/8), link-local (169.254.0.0/16), or link-local multicast ((224.0.0.0/24)"
      },
      "targetRef": {
       "$ref": "v1.ObjectReference",

api/swagger-spec/version.json

@@ -1,7 +1,7 @@
 {
   "swaggerVersion": "1.2",
   "apiVersion": "",
-  "basePath": "https://127.0.0.1:6443",
+  "basePath": "https://10.10.10.10:6443",
   "resourcePath": "/version",
   "apis": [
    {

cmd/kube-apiserver/app/server.go

@@ -270,8 +270,9 @@ func (s *APIServer) Run(_ []string) error {
 	s.verifyClusterIPFlags()
 
 	// If advertise-address is not specified, use bind-address. If bind-address
-	// is also unset (or 0.0.0.0), setDefaults() in pkg/master/master.go will
+	// is not usable (unset, 0.0.0.0, or loopback), setDefaults() in
-	// do the right thing and use the host's default interface.
+	// pkg/master/master.go will do the right thing and use the host's default
+	// interface.
 	if s.AdvertiseAddress == nil || s.AdvertiseAddress.IsUnspecified() {
 		s.AdvertiseAddress = s.BindAddress
 	}

docs/user-guide/services.md

@@ -195,6 +195,9 @@ created. You can manually map the service to your own specific endpoints:
 }
 ```
 
+NOTE: Endpoint IPs may not be loopback (127.0.0.0/8), link-local
+(169.254.0.0/16), or link-local multicast ((224.0.0.0/24).
+
 Accessing a `Service` without a selector works the same as if it had selector.
 The traffic will be routed to endpoints defined by the user (`1.2.3.4:80` in
 this example).

hack/after-build/update-swagger-spec.sh

@@ -56,6 +56,7 @@ KUBE_API_VERSIONS="v1" "${KUBE_OUTPUT_HOSTBIN}/kube-apiserver" \
   --port="${API_PORT}" \
   --etcd-servers="http://${ETCD_HOST}:${ETCD_PORT}" \
   --public-address-override="127.0.0.1" \
+  --advertise-address="10.10.10.10" \
   --kubelet-port=${KUBELET_PORT} \
   --runtime-config=api/v1 \
   --service-cluster-ip-range="10.0.0.0/24" >/dev/null 2>&1 &

pkg/api/v1/types.go

@@ -1273,7 +1273,7 @@ type EndpointSubset struct {
 type EndpointAddress struct {
 	// The IP of this endpoint.
 	// TODO: This should allow hostname or IP, see #4447.
-	IP string `json:"ip" description:"IP address of the endpoint"`
+	IP string `json:"ip" description:"IP address of the endpoint; may not be loopback (127.0.0.0/8), link-local (169.254.0.0/16), or link-local multicast ((224.0.0.0/24)"`
 
 	// Optional: The kubernetes object related to the entry point.
 	TargetRef *ObjectReference `json:"targetRef,omitempty" description:"reference to object providing the endpoint"`

pkg/api/validation/validation.go

@@ -1734,24 +1734,25 @@ func validateEndpointSubsets(subsets []api.EndpointSubset) errs.ValidationErrorL
 	return allErrs
 }
 
-var linkLocalNet *net.IPNet
-
 func validateEndpointAddress(address *api.EndpointAddress) errs.ValidationErrorList {
-	if linkLocalNet == nil {
-		var err error
-		_, linkLocalNet, err = net.ParseCIDR("169.254.0.0/16")
-		if err != nil {
-			glog.Errorf("Failed to parse link-local CIDR: %v", err)
-		}
-	}
-
 	allErrs := errs.ValidationErrorList{}
 	if !util.IsValidIPv4(address.IP) {
 		allErrs = append(allErrs, errs.NewFieldInvalid("ip", address.IP, "invalid IPv4 address"))
+		return allErrs
 	}
-	if linkLocalNet.Contains(net.ParseIP(address.IP)) {
+	// 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)
+	if ip.IsLoopback() {
+		allErrs = append(allErrs, errs.NewFieldInvalid("ip", address.IP, "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)"))
 	}
+	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)"))
+	}
 	return allErrs
 }
 

pkg/api/validation/validation_test.go

@@ -3948,6 +3948,19 @@ func TestValidateEndpoints(t *testing.T) {
 			},
 			errorType: "FieldValueRequired",
 		},
+		"Address is loopback": {
+			endpoints: api.Endpoints{
+				ObjectMeta: api.ObjectMeta{Name: "mysvc", Namespace: "namespace"},
+				Subsets: []api.EndpointSubset{
+					{
+						Addresses: []api.EndpointAddress{{IP: "127.0.0.1"}},
+						Ports:     []api.EndpointPort{{Name: "p", Port: 93, Protocol: "TCP"}},
+					},
+				},
+			},
+			errorType:   "FieldValueInvalid",
+			errorDetail: "loopback",
+		},
 		"Address is link-local": {
 			endpoints: api.Endpoints{
 				ObjectMeta: api.ObjectMeta{Name: "mysvc", Namespace: "namespace"},
@@ -3961,6 +3974,19 @@ func TestValidateEndpoints(t *testing.T) {
 			errorType:   "FieldValueInvalid",
 			errorDetail: "link-local",
 		},
+		"Address is link-local multicast": {
+			endpoints: api.Endpoints{
+				ObjectMeta: api.ObjectMeta{Name: "mysvc", Namespace: "namespace"},
+				Subsets: []api.EndpointSubset{
+					{
+						Addresses: []api.EndpointAddress{{IP: "224.0.0.1"}},
+						Ports:     []api.EndpointPort{{Name: "p", Port: 93, Protocol: "TCP"}},
+					},
+				},
+			},
+			errorType:   "FieldValueInvalid",
+			errorDetail: "link-local multicast",
+		},
 	}
 
 	for k, v := range errorCases {

pkg/master/master.go

@@ -286,7 +286,7 @@ func setDefaults(c *Config) {
 	if c.CacheTimeout == 0 {
 		c.CacheTimeout = 5 * time.Second
 	}
-	for c.PublicAddress == nil || c.PublicAddress.IsUnspecified() {
+	for c.PublicAddress == nil || c.PublicAddress.IsUnspecified() || c.PublicAddress.IsLoopback() {
 		// TODO: This should be done in the caller and just require a
 		// valid value to be passed in.
 		hostIP, err := util.ChooseHostInterface()