Use net.ParseCIDR instead of custom-built parsers (#3055)

Removes avoidable conversion to and from net.IP to string.

cmd/host-to-ip.go

@@ -1,51 +0,0 @@
-/*
- * Minio Cloud Storage, (C) 2016 Minio, Inc.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package cmd
-
-import (
-	"fmt"
-	"net"
-	"sort"
-)
-
-// byLastOctet implements sort.Interface used in sorting a list
-// of ip address by their last octet value.
-type byLastOctet []net.IP
-
-func (n byLastOctet) Len() int      { return len(n) }
-func (n byLastOctet) Swap(i, j int) { n[i], n[j] = n[j], n[i] }
-func (n byLastOctet) Less(i, j int) bool {
-	return []byte(n[i].To4())[3] < []byte(n[j].To4())[3]
-}
-
-// sortIPsByOctet - returns a reverse sorted list of hosts based on the last octet value.
-func sortIPsByOctet(ips []string) error {
-	var nips []net.IP
-	for _, ip := range ips {
-		nip := net.ParseIP(ip)
-		if nip == nil {
-			return fmt.Errorf("Unable to parse invalid ip %s", ip)
-		}
-		nips = append(nips, nip)
-	}
-	// Reverse sort ips by their last octet.
-	sort.Sort(sort.Reverse(byLastOctet(nips)))
-	for i, nip := range nips {
-		ips[i] = nip.String()
-	}
-	return nil
-}

cmd/host-to-ip_test.go

@@ -1,85 +0,0 @@
-/*
- * Minio Cloud Storage, (C) 2016 Minio, Inc.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package cmd
-
-import (
-	"fmt"
-	"testing"
-)
-
-// Tests sorted list generated from hosts to ip.
-func TestHostToIP(t *testing.T) {
-	// Collection of test cases to validate last octet sorting.
-	testCases := []struct {
-		ips        []string
-		sortedIPs  []string
-		err        error
-		shouldPass bool
-	}{
-		{
-			// List of ip addresses that need to be sorted.
-			ips: []string{
-				"129.95.30.40",
-				"5.24.69.2",
-				"19.20.203.5",
-				"1.2.3.4",
-				"127.0.0.1",
-				"19.20.21.22",
-				"5.220.100.50",
-			},
-			// Numerical sorting result based on the last octet.
-			sortedIPs: []string{
-				"5.220.100.50",
-				"129.95.30.40",
-				"19.20.21.22",
-				"19.20.203.5",
-				"1.2.3.4",
-				"5.24.69.2",
-				"127.0.0.1",
-			},
-			err:        nil,
-			shouldPass: true,
-		},
-		{
-			ips: []string{
-				"localhost",
-			},
-			sortedIPs:  []string{},
-			err:        fmt.Errorf("Unable to parse invalid ip localhost"),
-			shouldPass: false,
-		},
-	}
-
-	// Tests the correct sorting behavior of getIPsFromHosts.
-	for j, testCase := range testCases {
-		err := sortIPsByOctet(testCase.ips)
-		if !testCase.shouldPass && testCase.err.Error() != err.Error() {
-			t.Fatalf("Test %d: Expected error %s, got %s", j+1, testCase.err, err)
-		}
-		if testCase.shouldPass && err != nil {
-			t.Fatalf("Test %d: Expected error %s", j+1, err)
-		}
-		if testCase.shouldPass {
-			for i, ip := range testCase.ips {
-				if ip == testCase.sortedIPs[i] {
-					continue
-				}
-				t.Errorf("Test %d expected to pass but failed. Wanted ip %s, but got %s", j+1, testCase.sortedIPs[i], ip)
-			}
-		}
-	}
-}

cmd/interface-ips.go

@@ -0,0 +1,69 @@
+/*
+ * Minio Cloud Storage, (C) 2016 Minio, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package cmd
+
+import (
+	"fmt"
+	"net"
+	"sort"
+)
+
+// byLastOctetValue implements sort.Interface used in sorting a list
+// of ip address by their last octet value.
+type byLastOctetValue []net.IP
+
+func (n byLastOctetValue) Len() int      { return len(n) }
+func (n byLastOctetValue) Swap(i, j int) { n[i], n[j] = n[j], n[i] }
+func (n byLastOctetValue) Less(i, j int) bool {
+	return []byte(n[i].To4())[3] < []byte(n[j].To4())[3]
+}
+
+// getInterfaceIPv4s is synonymous to net.InterfaceAddrs()
+// returns net.IP IPv4 only representation of the net.Addr.
+// Additionally the returned list is sorted by their last
+// octet value.
+//
+// [The logic to sort by last octet is implemented to
+// prefer CIDRs with higher octects, this in-turn skips the
+// localhost/loopback address to be not preferred as the
+// first ip on the list. Subsequently this list helps us print
+// a user friendly message with appropriate values].
+func getInterfaceIPv4s() ([]net.IP, error) {
+	addrs, err := net.InterfaceAddrs()
+	if err != nil {
+		return nil, fmt.Errorf("Unable to determine network interface address. %s", err)
+	}
+	// Go through each return network address and collate IPv4 addresses.
+	var nips []net.IP
+	for _, addr := range addrs {
+		if addr.Network() == "ip+net" {
+			var nip net.IP
+			// Attempt to parse the addr through CIDR.
+			nip, _, err = net.ParseCIDR(addr.String())
+			if err != nil {
+				return nil, fmt.Errorf("Unable to parse addrss %s, error %s", addr, err)
+			}
+			// Collect only IPv4 addrs.
+			if nip.To4() != nil {
+				nips = append(nips, nip)
+			}
+		}
+	}
+	// Sort the list of IPs by their last octet value.
+	sort.Sort(sort.Reverse(byLastOctetValue(nips)))
+	return nips, nil
+}

cmd/interface-ips_test.go

@@ -0,0 +1,31 @@
+/*
+ * Minio Cloud Storage, (C) 2016 Minio, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package cmd
+
+import "testing"
+
+func TestGetInterfaceIPv4s(t *testing.T) {
+	ipv4s, err := getInterfaceIPv4s()
+	if err != nil {
+		t.Fatalf("Unexpected error %s", err)
+	}
+	for _, ip := range ipv4s {
+		if ip.To4() == nil {
+			t.Fatalf("Unexpected expecting only IPv4 addresses only %s", ip)
+		}
+	}
+}

cmd/server-main.go

@@ -222,23 +222,14 @@ func getListenIPs(httpServerConf *http.Server) (hosts []string, port string, err
 		return nil, port, fmt.Errorf("Unable to parse host address %s", err)
 	}
 	if host == "" {
-		var addrs []net.Addr
+		var ipv4s []net.IP
-		addrs, err = net.InterfaceAddrs()
+		ipv4s, err = getInterfaceIPv4s()
-		if err != nil {
-			return nil, port, fmt.Errorf("Unable to determine network interface address. %s", err)
-		}
-		for _, addr := range addrs {
-			if addr.Network() == "ip+net" {
-				hostname := strings.Split(addr.String(), "/")[0]
-				if ip := net.ParseIP(hostname); ip.To4() != nil {
-					hosts = append(hosts, hostname)
-				}
-			}
-		}
-		err = sortIPsByOctet(hosts)
 		if err != nil {
 			return nil, port, fmt.Errorf("Unable reverse sorted ips from hosts %s", err)
 		}
+		for _, ip := range ipv4s {
+			hosts = append(hosts, ip.String())
+		}
 		return hosts, port, nil
 	} // if host != "" {
 	// Proceed to append itself, since user requested a specific endpoint.