From 952c6184413763dec07f1b6429566c46910be49e Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Wed, 12 Apr 2017 09:22:35 -0700 Subject: [PATCH] server: Fix a regression in printing startup banner. (#4100) Octect based sorting was lost in the previous commit de204a0a52e6b13fec2d25b73102239891e6de35 This PR fixes a regression - fixes #4099 --- cmd/net.go | 55 ++++++++++++++++++++++++++++++++++-- cmd/net_test.go | 59 +++++++++++++++++++++++++++++++++++++++ cmd/server-startup-msg.go | 1 + 3 files changed, 112 insertions(+), 3 deletions(-) diff --git a/cmd/net.go b/cmd/net.go index e8b58a57c..9f8d6021c 100644 --- a/cmd/net.go +++ b/cmd/net.go @@ -77,18 +77,67 @@ func getHostIP4(host string) (ipList set.StringSet, err error) { return ipList, err } +// byLastOctetValue implements sort.Interface used in sorting a list +// of ip address by their last octet value in descending order. +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 { + // This case is needed when all ips in the list + // have same last octets, Following just ensures that + // 127.0.0.1 is moved to the end of the list. + if n[i].String() == "127.0.0.1" { + return false + } + if n[j].String() == "127.0.0.1" { + return true + } + return []byte(n[i].To4())[3] > []byte(n[j].To4())[3] +} + +// sortIPs - sort ips based on higher octects. +// 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 sortIPs(ipList []string) []string { + if len(ipList) == 1 { + return ipList + } + + var ipV4s []net.IP + var nonIPs []string + for _, ip := range ipList { + nip := net.ParseIP(ip) + if nip != nil { + ipV4s = append(ipV4s, nip) + } else { + nonIPs = append(nonIPs, ip) + } + } + + sort.Sort(byLastOctetValue(ipV4s)) + + var ips []string + for _, ip := range ipV4s { + ips = append(ips, ip.String()) + } + + return append(nonIPs, ips...) +} + func getAPIEndpoints(serverAddr string) (apiEndpoints []string) { host, port := mustSplitHostPort(serverAddr) var ipList []string if host == "" { - ipList = localIP4.ToSlice() + ipList = sortIPs(localIP4.ToSlice()) } else { ipList = []string{host} } - sort.Strings(ipList) - scheme := httpScheme if globalIsSSL { scheme = httpsScheme diff --git a/cmd/net_test.go b/cmd/net_test.go index 966a66b0e..e95e81b9d 100644 --- a/cmd/net_test.go +++ b/cmd/net_test.go @@ -19,6 +19,7 @@ package cmd import ( "fmt" "net" + "reflect" "runtime" "testing" @@ -52,6 +53,64 @@ func TestMustSplitHostPort(t *testing.T) { } } +func TestSortIPs(t *testing.T) { + testCases := []struct { + ipList []string + sortedIPList []string + }{ + // Default case of two ips one with higher octet moves + // to the beginning of the list. + { + ipList: []string{"127.0.0.1", "10.0.0.13"}, + sortedIPList: []string{"10.0.0.13", "127.0.0.1"}, + }, + // With multiple types of octet, chooses a higher octet. + { + ipList: []string{"127.0.0.1", "172.0.21.1", "192.168.1.106"}, + sortedIPList: []string{"192.168.1.106", "172.0.21.1", "127.0.0.1"}, + }, + // With different ip along with localhost. + { + ipList: []string{"127.0.0.1", "192.168.1.106"}, + sortedIPList: []string{"192.168.1.106", "127.0.0.1"}, + }, + // With a list of only one element nothing to sort. + { + ipList: []string{"hostname"}, + sortedIPList: []string{"hostname"}, + }, + // With a list of only one element nothing to sort. + { + ipList: []string{"127.0.0.1"}, + sortedIPList: []string{"127.0.0.1"}, + }, + // Non parsable ip is assumed to be hostame and gets preserved + // as the left most elements, regardless of IP based sorting. + { + ipList: []string{"hostname", "127.0.0.1", "192.168.1.106"}, + sortedIPList: []string{"hostname", "192.168.1.106", "127.0.0.1"}, + }, + // Non parsable ip is assumed to be hostname, with a mixed input of ip and hostname. + // gets preserved and moved into left most elements, regardless of + // IP based sorting. + { + ipList: []string{"hostname1", "10.0.0.13", "hostname2", "127.0.0.1", "192.168.1.106"}, + sortedIPList: []string{"hostname1", "hostname2", "192.168.1.106", "10.0.0.13", "127.0.0.1"}, + }, + // With same higher octets, preferentially move the localhost. + { + ipList: []string{"127.0.0.1", "10.0.0.1", "192.168.0.1"}, + sortedIPList: []string{"10.0.0.1", "192.168.0.1", "127.0.0.1"}, + }, + } + for i, testCase := range testCases { + gotIPList := sortIPs(testCase.ipList) + if !reflect.DeepEqual(testCase.sortedIPList, gotIPList) { + t.Errorf("Test %d: Expected %s, got %s", i+1, testCase.sortedIPList, gotIPList) + } + } +} + func TestMustGetLocalIP4(t *testing.T) { testCases := []struct { expectedIPList set.StringSet diff --git a/cmd/server-startup-msg.go b/cmd/server-startup-msg.go index f634ac929..a9298ce65 100644 --- a/cmd/server-startup-msg.go +++ b/cmd/server-startup-msg.go @@ -42,6 +42,7 @@ func getFormatStr(strLen int, padding int) string { // Prints the formatted startup message. func printStartupMessage(apiEndPoints []string) { + // Prints credential, region and browser access. printServerCommonMsg(apiEndPoints)