Repository: qiniu/goc
Pull request #286 (2022-06-30 02:02:44)

fix:fix server address error when running in k8s
when goc server running  in k8s
client register uri while be transform to is proxy address
in fact,there is no need to transform

add address uri check,make sure address parm is a complete uri


Patch:
diff --git a/cmd/server.go b/cmd/server.go
index 36d5b5e..7e2a6e5 100644
--- a/cmd/server.go
+++ b/cmd/server.go
@@ -17,9 +17,10 @@
 package cmd
 
 import (
+	"log"
+
 	"github.com/qiniu/goc/pkg/cover"
 	"github.com/spf13/cobra"
-	"log"
 )
 
 var serverCmd = &cobra.Command{
@@ -41,14 +42,17 @@ goc server --port=localhost:8080
 		if err != nil {
 			log.Fatalf("New file based server failed, err: %v", err)
 		}
+		server.IPRevise = iprevise
 		server.Run(port)
 	},
 }
 
 var port, localPersistence string
+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)")
 	rootCmd.AddCommand(serverCmd)
 }
diff --git a/pkg/cover/server.go b/pkg/cover/server.go
index 142961f..6954e6a 100644
--- a/pkg/cover/server.go
+++ b/pkg/cover/server.go
@@ -26,6 +26,7 @@ import (
 	"net/url"
 	"os"
 	"regexp"
+	"strconv"
 
 	"github.com/gin-gonic/gin"
 	log "github.com/sirupsen/logrus"
@@ -38,6 +39,7 @@ const LogFile = "goc.log"
 
 type server struct {
 	PersistenceFile string
+	IPRevise        bool // whether to do ip revise during registering
 	Store           Store
 }
 
@@ -98,8 +100,9 @@ func (s *server) Route(w io.Writer) *gin.Engine {
 
 // ServiceUnderTest is a entry under being tested
 type ServiceUnderTest struct {
-	Name    string `form:"name" json:"name" binding:"required"`
-	Address string `form:"address" json:"address" binding:"required"`
+	Name     string `form:"name" json:"name" binding:"required"`
+	Address  string `form:"address" json:"address" binding:"required"`
+	IPRevise string `form:"iprevise" json:"-"`
 }
 
 // ProfileParam is param of profile API
@@ -123,24 +126,57 @@ 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()})
 		return
 	}
-	host, port, err := net.SplitHostPort(u.Host)
-	if err != nil {
-		c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()})
+	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"})
 		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"})
+			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)
+		if err != nil {
+			c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()})
+			return
+		}
 
-	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("http://%s:%s", realIP, port)
+		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)
+		}
 	}
 
 	address := s.Store.Get(service.Name)
@@ -152,7 +188,6 @@ func (s *server) registerService(c *gin.Context) {
 	}
 
 	c.JSON(http.StatusOK, gin.H{"result": "success"})
-	return
 }
 
 // profile API examples:


Test patch:
diff --git a/pkg/cover/server_test.go b/pkg/cover/server_test.go
index 3896486..ce025ac 100644
--- a/pkg/cover/server_test.go
+++ b/pkg/cover/server_test.go
@@ -9,6 +9,7 @@ import (
 	"net/url"
 	"os"
 	"reflect"
+	"strconv"
 	"strings"
 	"testing"
 
@@ -139,7 +140,7 @@ func TestRegisterService(t *testing.T) {
 
 	assert.Equal(t, http.StatusBadRequest, w.Code)
 
-	// register with invalid service.Address
+	// register with invalid service.Address(bad uri)
 	data := url.Values{}
 	data.Set("name", "aaa")
 	data.Set("address", "&%%")
@@ -151,22 +152,104 @@ func TestRegisterService(t *testing.T) {
 	assert.Equal(t, http.StatusBadRequest, w.Code)
 	assert.Contains(t, w.Body.String(), "invalid URL escape")
 
-	// register with host but no port
+	// regist with invalid service.Address(schema)
 	data = url.Values{}
 	data.Set("name", "aaa")
-	data.Set("address", "http://127.0.0.1")
+	data.Set("address", "fpt://127.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(), "unsupport schema")
 
+	// regist with unempty path
+	data = url.Values{}
+	data.Set("name", "aaa")
+	data.Set("address", "http://127.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(), "missing port in address")
+	assert.Contains(t, w.Body.String(), "uri path must empty")
+
+	// regist with empty host(in direct mode,empty must fail,default is success)
+	// in default,use request realhost as host,use 80 as port
+	server.IPRevise = false
+	data = url.Values{}
+	data.Set("name", "aaa")
+	data.Set("address", "http://")
+	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(), "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")
+
+	data = url.Values{}
+	data.Set("name", "aaa")
+	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")
+	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")
+	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)
+
+	server.IPRevise = true //use clientip and default port(80)
+	data = url.Values{}
+	data.Set("name", "aaa")
+	data.Set("address", "http://")
+	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)
+
+	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.StatusOK, w.Code)
+
+	data = url.Values{}
+	data.Set("name", "aaa")
+	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")
+	router.ServeHTTP(w, req)
+	assert.Equal(t, http.StatusOK, w.Code)
 
 	// 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
+		Name:     "foo",
+		Address:  "http://:64444", // the real IP is empty in unittest, so server will get a empty one
+		IPRevise: strconv.FormatBool(server.IPRevise),
 	}
 	testObj := new(MockStore)
 	testObj.On("Get", "foo").Return([]string{"http://127.0.0.1:66666"})
@@ -177,6 +260,7 @@ func TestRegisterService(t *testing.T) {
 	w = httptest.NewRecorder()
 	data.Set("name", expectedS.Name)
 	data.Set("address", expectedS.Address)
+	data.Set("network", "")
 	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)
diff --git a/pkg/cover/store_test.go b/pkg/cover/store_test.go
index bf3201f..a4898d8 100644
--- a/pkg/cover/store_test.go
+++ b/pkg/cover/store_test.go
@@ -18,12 +18,14 @@ package cover
 
 import (
 	"fmt"
+	"os"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 )
 
 func TestLocalStore(t *testing.T) {
+	os.Remove("_svrs_address.txt") //remove _svrs_address.txt file，make sure no data effect this unit test
 	localStore, err := NewFileStore("_svrs_address.txt")
 	assert.NoError(t, err)
 	var tc1 = ServiceUnderTest{
diff --git a/tests/register.bats b/tests/register.bats
index 6f5d089..da40d76 100755
--- a/tests/register.bats
+++ b/tests/register.bats
@@ -59,7 +59,7 @@ teardown_file() {
     run gocc register --center=http://127.0.0.1:60001 --name=xyz --address=http://137.0.0.1 --debug --debugcisyncfile ci-sync.bak;
     info register2 output: $output
     [ "$status" -eq 0 ]
-    [[ "$output" == *"missing port"* ]]
+    [[ "$output" != *"missing port"* ]]
 
     wait $profile_pid
 }