bootup-validation: Allow port configuration only using --address option. (#3166)

cmd/object-common_test.go

@@ -101,6 +101,7 @@ func TestHouseKeeping(t *testing.T) {
 
 // Test getPath() - the path that needs to be passed to newPosix()
 func TestGetPath(t *testing.T) {
+	globalMinioHost = ""
 	var testCases []struct {
 		epStr string
 		path  string

cmd/server-main.go

@@ -118,8 +118,31 @@ func parseStorageEndpoints(eps []string) (endpoints []*url.URL, err error) {
 		if err != nil {
 			return nil, err
 		}
-		if u.Host != "" && globalMinioHost == "" {
+		if u.Host != "" {
+			_, port, err := net.SplitHostPort(u.Host)
+			// Ignore the missing port error as the default port can be globalMinioPort.
+			if err != nil && !strings.Contains(err.Error(), "missing port in address") {
+				return nil, err
+			}
+
+			if globalMinioHost == "" {
+				// For ex.: minio server host1:port1 host2:port2...
+				// we return error as port is configurable only
+				// using "--address :port"
+				if port != "" {
+					errorIf(fmt.Errorf("Invalid argument %s, port configurable using --address :<port>", u.Host), "")
+					return nil, errInvalidArgument
+				}
 				u.Host = net.JoinHostPort(u.Host, globalMinioPort)
+			} else {
+				// For ex.: minio server --address host:port host1:port1 host2:port2...
+				// i.e if "--address host:port" is specified
+				// port info in u.Host is mandatory else return error.
+				if port == "" {
+					errorIf(fmt.Errorf("Invalid argument %s, port mandatory when --address <host>:<port> is used", u.Host), "")
+					return nil, errInvalidArgument
+				}
+			}
 		}
 		endpoints = append(endpoints, u)
 	}

cmd/server-main_test.go

@@ -177,6 +177,28 @@ func TestCheckSufficientDisks(t *testing.T) {
 	}
 }
 
+func TestParseStorageEndpoints(t *testing.T) {
+	testCases := []struct {
+		globalMinioHost string
+		host            string
+		expectedErr     error
+	}{
+		{"", "http://localhost/export", nil},
+		{"testhost", "http://localhost/export", errInvalidArgument},
+		{"", "http://localhost:9000/export", errInvalidArgument},
+		{"testhost", "http://localhost:9000/export", nil},
+	}
+	for i, test := range testCases {
+		globalMinioHost = test.globalMinioHost
+		_, err := parseStorageEndpoints([]string{test.host})
+		if err != test.expectedErr {
+			t.Errorf("Test %d : got %v, expected %v", i+1, err, test.expectedErr)
+		}
+	}
+	// Should be reset back to "" so that we don't affect other tests.
+	globalMinioHost = ""
+}
+
 func TestCheckEndpointsSyntax(t *testing.T) {
 	var testCases []string
 	if runtime.GOOS == "windows" {
@@ -255,9 +277,8 @@ func TestIsDistributedSetup(t *testing.T) {
 			disks  []string
 			result bool
 		}{
-			{[]string{`http://4.4.4.4:80/c:\mnt\disk1`, `http://4.4.4.4:80/c:\mnt\disk2`}, true},
-			{[]string{`http://4.4.4.4:9000/c:\mnt\disk1`, `http://127.0.0.1:9000/c:\mnt\disk2`}, true},
-			{[]string{`http://127.0.0.1:9000/c:\mnt\disk1`, `http://127.0.0.1:9001/c:\mnt\disk2`}, true},
+			{[]string{`http://4.4.4.4/c:\mnt\disk1`, `http://4.4.4.4/c:\mnt\disk2`}, true},
+			{[]string{`http://4.4.4.4/c:\mnt\disk1`, `http://127.0.0.1/c:\mnt\disk2`}, true},
 			{[]string{`c:\mnt\disk1`, `c:\mnt\disk2`}, false},
 		}
 	} else {
@@ -265,23 +286,44 @@ func TestIsDistributedSetup(t *testing.T) {
 			disks  []string
 			result bool
 		}{
-			{[]string{"http://4.4.4.4:9000/mnt/disk1", "http://4.4.4.4:9000/mnt/disk2"}, true},
-			{[]string{"http://4.4.4.4:9000/mnt/disk1", "http://127.0.0.1:9000/mnt/disk2"}, true},
-			{[]string{"http://127.0.0.1:9000/mnt/disk1", "http://127.0.0.1:9000/mnt/disk2"}, true},
+			{[]string{"http://4.4.4.4/mnt/disk1", "http://4.4.4.4/mnt/disk2"}, true},
+			{[]string{"http://4.4.4.4/mnt/disk1", "http://127.0.0.1/mnt/disk2"}, true},
 			{[]string{"/mnt/disk1", "/mnt/disk2"}, false},
 		}
+	}
+	for i, test := range testCases {
+		endpoints, err := parseStorageEndpoints(test.disks)
+		if err != nil {
+			t.Fatalf("Test %d: Unexpected error: %s", i+1, err)
+		}
+		res := isDistributedSetup(endpoints)
+		if res != test.result {
+			t.Errorf("Test %d: expected result %t but received %t", i+1, test.result, res)
+		}
+	}
 
+	// Test cases when globalMinioHost is set
+	globalMinioHost = "testhost"
+	testCases = []struct {
+		disks  []string
+		result bool
+	}{
+		{[]string{"http://127.0.0.1:9001/mnt/disk1", "http://127.0.0.1:9002/mnt/disk2", "http://127.0.0.1:9003/mnt/disk3", "http://127.0.0.1:9004/mnt/disk4"}, true},
+		{[]string{"/mnt/disk1", "/mnt/disk2"}, false},
 	}
+
 	for i, test := range testCases {
 		endpoints, err := parseStorageEndpoints(test.disks)
 		if err != nil {
-			t.Fatalf("Unexpected error %s", err)
+			t.Fatalf("Test %d: Unexpected error: %s", i+1, err)
 		}
 		res := isDistributedSetup(endpoints)
 		if res != test.result {
 			t.Errorf("Test %d: expected result %t but received %t", i+1, test.result, res)
 		}
 	}
+	// Reset so that we don't affect other tests.
+	globalMinioHost = ""
 }
 
 func TestInitServerConfig(t *testing.T) {