diff --git a/cmd/register.go b/cmd/register.go index ab0518be..9f5559b5 100644 --- a/cmd/register.go +++ b/cmd/register.go @@ -34,8 +34,9 @@ goc register [flags] `, Run: func(cmd *cobra.Command, args []string) { s := cover.ServiceUnderTest{ - Name: name, - Address: address, + Name: name, + Address: address, + IPRevise: ipRevise, } res, err := cover.NewWorker(center).RegisterService(s) if err != nil { @@ -46,14 +47,16 @@ goc register [flags] } var ( - name string - address string + name string + address string + ipRevise string ) func init() { registerCmd.Flags().StringVarP(¢er, "center", "", "http://127.0.0.1:7777", "cover profile host center") registerCmd.Flags().StringVarP(&name, "name", "n", "", "service name") registerCmd.Flags().StringVarP(&address, "address", "a", "", "service address") + registerCmd.Flags().StringVarP(&ipRevise, "ip_revise", "", "true", "whether to do ip revise during registering") registerCmd.MarkFlagRequired("name") registerCmd.MarkFlagRequired("address") rootCmd.AddCommand(registerCmd) diff --git a/cmd/server.go b/cmd/server.go index 7e2a6e59..9f7a4cd0 100644 --- a/cmd/server.go +++ b/cmd/server.go @@ -42,17 +42,17 @@ goc server --port=localhost:8080 if err != nil { log.Fatalf("New file based server failed, err: %v", err) } - server.IPRevise = iprevise + server.IPRevise = IPRevise server.Run(port) }, } var port, localPersistence string -var iprevise bool +var IPRevise bool func init() { serverCmd.Flags().StringVarP(&port, "port", "", ":7777", "listen port to start a coverage host center") serverCmd.Flags().StringVarP(&localPersistence, "local-persistence", "", "_svrs_address.txt", "the file to save services address information") - serverCmd.Flags().BoolVarP(&iprevise, "ip_revise", "", true, "setting the network type(default:regist server use proxy or under nat、same network ect,direct:use register request parm)") + serverCmd.Flags().BoolVarP(&IPRevise, "ip_revise", "", true, "whether to do ip revise during registering. Recommend to set this as false if under NAT or Proxy environment") rootCmd.AddCommand(serverCmd) } diff --git a/pkg/cover/client.go b/pkg/cover/client.go index 827c7c82..33ade26e 100644 --- a/pkg/cover/client.go +++ b/pkg/cover/client.go @@ -79,7 +79,7 @@ func (c *client) RegisterService(srv ServiceUnderTest) ([]byte, error) { if strings.TrimSpace(srv.Name) == "" { return nil, fmt.Errorf("invalid service name") } - u := fmt.Sprintf("%s%s?name=%s&address=%s", c.Host, CoverRegisterServiceAPI, srv.Name, srv.Address) + u := fmt.Sprintf("%s%s?name=%s&address=%s&ip_revise=%s", c.Host, CoverRegisterServiceAPI, srv.Name, srv.Address, srv.IPRevise) _, res, err := c.do("POST", u, "", nil) return res, err } diff --git a/pkg/cover/server.go b/pkg/cover/server.go index 6954e6a0..2c3f36db 100644 --- a/pkg/cover/server.go +++ b/pkg/cover/server.go @@ -27,6 +27,7 @@ import ( "os" "regexp" "strconv" + "strings" "github.com/gin-gonic/gin" log "github.com/sirupsen/logrus" @@ -102,7 +103,7 @@ func (s *server) Route(w io.Writer) *gin.Engine { type ServiceUnderTest struct { Name string `form:"name" json:"name" binding:"required"` Address string `form:"address" json:"address" binding:"required"` - IPRevise string `form:"iprevise" json:"-"` + IPRevise string `form:"ip_revise" json:"ip_revise" binding:"-"` // whether to do ip revise during registering } // ProfileParam is param of profile API @@ -126,59 +127,58 @@ func (s *server) registerService(c *gin.Context) { c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()}) return } - if service.IPRevise == "" { - service.IPRevise = strconv.FormatBool(s.IPRevise) - } - isrevise, err := strconv.ParseBool(service.IPRevise) - if err != nil { - isrevise = s.IPRevise - } + u, err := url.Parse(service.Address) if err != nil { - c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()}) + c.JSON(http.StatusBadRequest, gin.H{"error": fmt.Sprintf("url.Parse %s failed: %s", service.Address, err.Error())}) return } if u.Scheme != "https" && u.Scheme != "http" { c.JSON(http.StatusBadRequest, gin.H{"error": "unsupport schema"}) return - } else if u.Path != "" { - c.JSON(http.StatusBadRequest, gin.H{"error": "uri path must empty"}) + } + if u.Host == "" { + c.JSON(http.StatusBadRequest, gin.H{"error": "empty host"}) return } - if !isrevise { - if u.Host == "" { - c.JSON(http.StatusBadRequest, gin.H{"error": "address is empty"}) - return - } - if u.Hostname() == "" { - c.JSON(http.StatusBadRequest, gin.H{"error": "empty host name"}) + host, port, err := net.SplitHostPort(u.Host) + if err != nil { + if strings.Contains(err.Error(), "missing port in address") { + // valid scenario, keep going + host = u.Host + } else { + c.JSON(http.StatusBadRequest, gin.H{"error": fmt.Sprintf("net.SplitHostPort %s failed: %s", u.Host, err.Error())}) return } - } else { - host := u.Hostname() - port := u.Port() - if host == "" { - host = c.ClientIP() - } - if port == "" { - port = "80" - } - u.Host = fmt.Sprintf("%s:%s", host, port) - host, port, err := net.SplitHostPort(u.Host) + } + + var doIPRevise bool + // Prefer user's decision first. + if service.IPRevise != "" { + doIPRevise, err = strconv.ParseBool(service.IPRevise) if err != nil { - c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()}) + c.JSON(http.StatusBadRequest, gin.H{"error": fmt.Sprintf("strconv.ParseBool %s failed: %s", service.IPRevise, err.Error())}) return } + } else { + doIPRevise = s.IPRevise + } + if doIPRevise { realIP := c.ClientIP() // only for IPV4 // refer: https://github.com/qiniu/goc/issues/177 if net.ParseIP(realIP).To4() != nil && host != realIP { log.Printf("the registered host %s of service %s is different with the real one %s, here we choose the real one", service.Name, host, realIP) - service.Address = fmt.Sprintf("%s://%s:%s", u.Scheme, realIP, port) + host = realIP } } + service.Address = fmt.Sprintf("%s://%s", u.Scheme, host) + if port != "" { + service.Address = fmt.Sprintf("%s:%s", service.Address, port) + } + address := s.Store.Get(service.Name) if !contains(address, service.Address) { if err := s.Store.Add(service); err != nil && err != ErrServiceAlreadyRegistered { diff --git a/pkg/cover/server_test.go b/pkg/cover/server_test.go index ce025ace..84709612 100644 --- a/pkg/cover/server_test.go +++ b/pkg/cover/server_test.go @@ -9,7 +9,6 @@ import ( "net/url" "os" "reflect" - "strconv" "strings" "testing" @@ -166,13 +165,12 @@ func TestRegisterService(t *testing.T) { // regist with unempty path data = url.Values{} data.Set("name", "aaa") - data.Set("address", "http://127.0.0.1:21/") + data.Set("address", "http://127.0.0.1:21/") // valid scenario, the final stored address will be http://127.0.0.0.1:21 w = httptest.NewRecorder() req, _ = http.NewRequest("POST", "/v1/cover/register", strings.NewReader(data.Encode())) req.Header.Set("Content-Type", "application/x-www-form-urlencoded") router.ServeHTTP(w, req) - assert.Equal(t, http.StatusBadRequest, w.Code) - assert.Contains(t, w.Body.String(), "uri path must empty") + assert.Equal(t, http.StatusOK, w.Code) // regist with empty host(in direct mode,empty must fail,default is success) // in default,use request realhost as host,use 80 as port @@ -185,32 +183,20 @@ func TestRegisterService(t *testing.T) { req.Header.Set("Content-Type", "application/x-www-form-urlencoded") router.ServeHTTP(w, req) assert.Equal(t, http.StatusBadRequest, w.Code) - assert.Contains(t, w.Body.String(), "address is empty") - - data = url.Values{} - data.Set("name", "aaa") - data.Set("address", "http://:8080") - w = httptest.NewRecorder() - req, _ = http.NewRequest("POST", "/v1/cover/register", strings.NewReader(data.Encode())) - req.Header.Set("Content-Type", "application/x-www-form-urlencoded") - router.ServeHTTP(w, req) - assert.Equal(t, http.StatusBadRequest, w.Code) - assert.Contains(t, w.Body.String(), "empty host name") + assert.Contains(t, w.Body.String(), "empty host") data = url.Values{} data.Set("name", "aaa") - data.Set("address", "http://127.0.0.1") + data.Set("address", "http://:8080") //valid scenario, be consistent with curl w = httptest.NewRecorder() req, _ = http.NewRequest("POST", "/v1/cover/register", strings.NewReader(data.Encode())) req.Header.Set("Content-Type", "application/x-www-form-urlencoded") router.ServeHTTP(w, req) assert.Equal(t, http.StatusOK, w.Code) - //change network type data = url.Values{} data.Set("name", "aaa") - data.Set("address", "http://") - data.Set("iprevise", "true") + data.Set("address", "http://127.0.0.1") w = httptest.NewRecorder() req, _ = http.NewRequest("POST", "/v1/cover/register", strings.NewReader(data.Encode())) req.Header.Set("Content-Type", "application/x-www-form-urlencoded") @@ -225,7 +211,7 @@ func TestRegisterService(t *testing.T) { req, _ = http.NewRequest("POST", "/v1/cover/register", strings.NewReader(data.Encode())) req.Header.Set("Content-Type", "application/x-www-form-urlencoded") router.ServeHTTP(w, req) - assert.Equal(t, http.StatusOK, w.Code) + assert.Equal(t, http.StatusBadRequest, w.Code) data = url.Values{} data.Set("name", "aaa") @@ -248,8 +234,8 @@ func TestRegisterService(t *testing.T) { // register with store failure expectedS := ServiceUnderTest{ Name: "foo", - Address: "http://:64444", // the real IP is empty in unittest, so server will get a empty one - IPRevise: strconv.FormatBool(server.IPRevise), + Address: "http://:8080", + IPRevise: "true", } testObj := new(MockStore) testObj.On("Get", "foo").Return([]string{"http://127.0.0.1:66666"}) @@ -260,7 +246,7 @@ func TestRegisterService(t *testing.T) { w = httptest.NewRecorder() data.Set("name", expectedS.Name) data.Set("address", expectedS.Address) - data.Set("network", "") + data.Set("ip_revise", expectedS.IPRevise) req, _ = http.NewRequest("POST", "/v1/cover/register", strings.NewReader(data.Encode())) req.Header.Set("Content-Type", "application/x-www-form-urlencoded") router.ServeHTTP(w, req)