Skip to content

Commit 50b62f0

Browse files
CarlJiCarlJi
authored
feat: enhance about ip revise logic (#290)
* feat: a little refactor about ip revise logic * feat: goc register support ip revise parameter * feat: adjust unit test
1 parent 5ccfc73 commit 50b62f0

File tree

5 files changed

+51
-62
lines changed

5 files changed

+51
-62
lines changed

‎cmd/register.go‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,9 @@ goc register [flags]
3434
`,
3535
Run: func(cmd *cobra.Command, args []string) {
3636
s := cover.ServiceUnderTest{
37-
Name: name,
38-
Address: address,
37+
Name: name,
38+
Address: address,
39+
IPRevise: ipRevise,
3940
}
4041
res, err := cover.NewWorker(center).RegisterService(s)
4142
if err != nil {
@@ -46,14 +47,16 @@ goc register [flags]
4647
}
4748

4849
var (
49-
name string
50-
address string
50+
name string
51+
address string
52+
ipRevise string
5153
)
5254

5355
func init() {
5456
registerCmd.Flags().StringVarP(&center, "center", "", "http://127.0.0.1:7777", "cover profile host center")
5557
registerCmd.Flags().StringVarP(&name, "name", "n", "", "service name")
5658
registerCmd.Flags().StringVarP(&address, "address", "a", "", "service address")
59+
registerCmd.Flags().StringVarP(&ipRevise, "ip_revise", "", "true", "whether to do ip revise during registering")
5760
registerCmd.MarkFlagRequired("name")
5861
registerCmd.MarkFlagRequired("address")
5962
rootCmd.AddCommand(registerCmd)

‎cmd/server.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,17 +42,17 @@ goc server --port=localhost:8080
4242
if err != nil {
4343
log.Fatalf("New file based server failed, err: %v", err)
4444
}
45-
server.IPRevise = iprevise
45+
server.IPRevise = IPRevise
4646
server.Run(port)
4747
},
4848
}
4949

5050
var port, localPersistence string
51-
var iprevise bool
51+
var IPRevise bool
5252

5353
func init() {
5454
serverCmd.Flags().StringVarP(&port, "port", "", ":7777", "listen port to start a coverage host center")
5555
serverCmd.Flags().StringVarP(&localPersistence, "local-persistence", "", "_svrs_address.txt", "the file to save services address information")
56-
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)")
56+
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")
5757
rootCmd.AddCommand(serverCmd)
5858
}

‎pkg/cover/client.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ func (c *client) RegisterService(srv ServiceUnderTest) ([]byte, error) {
7979
if strings.TrimSpace(srv.Name) == "" {
8080
return nil, fmt.Errorf("invalid service name")
8181
}
82-
u := fmt.Sprintf("%s%s?name=%s&address=%s", c.Host, CoverRegisterServiceAPI, srv.Name, srv.Address)
82+
u := fmt.Sprintf("%s%s?name=%s&address=%s&ip_revise=%s", c.Host, CoverRegisterServiceAPI, srv.Name, srv.Address, srv.IPRevise)
8383
_, res, err := c.do("POST", u, "", nil)
8484
return res, err
8585
}

‎pkg/cover/server.go‎

Lines changed: 31 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import (
2727
"os"
2828
"regexp"
2929
"strconv"
30+
"strings"
3031

3132
"github.com/gin-gonic/gin"
3233
log "github.com/sirupsen/logrus"
@@ -102,7 +103,7 @@ func (s *server) Route(w io.Writer) *gin.Engine {
102103
type ServiceUnderTest struct {
103104
Name string `form:"name" json:"name" binding:"required"`
104105
Address string `form:"address" json:"address" binding:"required"`
105-
IPRevise string `form:"iprevise" json:"-"`
106+
IPRevise string `form:"ip_revise" json:"ip_revise" binding:"-"` // whether to do ip revise during registering
106107
}
107108

108109
// ProfileParam is param of profile API
@@ -126,59 +127,58 @@ func (s *server) registerService(c *gin.Context) {
126127
c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()})
127128
return
128129
}
129-
if service.IPRevise == "" {
130-
service.IPRevise = strconv.FormatBool(s.IPRevise)
131-
}
132-
isrevise, err := strconv.ParseBool(service.IPRevise)
133-
if err != nil {
134-
isrevise = s.IPRevise
135-
}
130+
136131
u, err := url.Parse(service.Address)
137132
if err != nil {
138-
c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()})
133+
c.JSON(http.StatusBadRequest, gin.H{"error": fmt.Sprintf("url.Parse %s failed: %s", service.Address, err.Error())})
139134
return
140135
}
141136
if u.Scheme != "https" && u.Scheme != "http" {
142137
c.JSON(http.StatusBadRequest, gin.H{"error": "unsupport schema"})
143138
return
144-
} else if u.Path != "" {
145-
c.JSON(http.StatusBadRequest, gin.H{"error": "uri path must empty"})
139+
}
140+
if u.Host == "" {
141+
c.JSON(http.StatusBadRequest, gin.H{"error": "empty host"})
146142
return
147143
}
148-
if !isrevise {
149-
if u.Host == "" {
150-
c.JSON(http.StatusBadRequest, gin.H{"error": "address is empty"})
151-
return
152-
}
153-
if u.Hostname() == "" {
154-
c.JSON(http.StatusBadRequest, gin.H{"error": "empty host name"})
144+
host, port, err := net.SplitHostPort(u.Host)
145+
if err != nil {
146+
if strings.Contains(err.Error(), "missing port in address") {
147+
// valid scenario, keep going
148+
host = u.Host
149+
} else {
150+
c.JSON(http.StatusBadRequest, gin.H{"error": fmt.Sprintf("net.SplitHostPort %s failed: %s", u.Host, err.Error())})
155151
return
156152
}
157-
} else {
158-
host := u.Hostname()
159-
port := u.Port()
160-
if host == "" {
161-
host = c.ClientIP()
162-
}
163-
if port == "" {
164-
port = "80"
165-
}
166-
u.Host = fmt.Sprintf("%s:%s", host, port)
167-
host, port, err := net.SplitHostPort(u.Host)
153+
}
154+
155+
var doIPRevise bool
156+
// Prefer user's decision first.
157+
if service.IPRevise != "" {
158+
doIPRevise, err = strconv.ParseBool(service.IPRevise)
168159
if err != nil {
169-
c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()})
160+
c.JSON(http.StatusBadRequest, gin.H{"error": fmt.Sprintf("strconv.ParseBool %s failed: %s", service.IPRevise, err.Error())})
170161
return
171162
}
163+
} else {
164+
doIPRevise = s.IPRevise
165+
}
172166

167+
if doIPRevise {
173168
realIP := c.ClientIP()
174169
// only for IPV4
175170
// refer: https://github.com/qiniu/goc/issues/177
176171
if net.ParseIP(realIP).To4() != nil && host != realIP {
177172
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)
178-
service.Address = fmt.Sprintf("%s://%s:%s", u.Scheme, realIP, port)
173+
host = realIP
179174
}
180175
}
181176

177+
service.Address = fmt.Sprintf("%s://%s", u.Scheme, host)
178+
if port != "" {
179+
service.Address = fmt.Sprintf("%s:%s", service.Address, port)
180+
}
181+
182182
address := s.Store.Get(service.Name)
183183
if !contains(address, service.Address) {
184184
if err := s.Store.Add(service); err != nil && err != ErrServiceAlreadyRegistered {

‎pkg/cover/server_test.go‎

Lines changed: 9 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import (
99
"net/url"
1010
"os"
1111
"reflect"
12-
"strconv"
1312
"strings"
1413
"testing"
1514

@@ -166,13 +165,12 @@ func TestRegisterService(t *testing.T) {
166165
// regist with unempty path
167166
data = url.Values{}
168167
data.Set("name", "aaa")
169-
data.Set("address", "http://127.0.0.1:21/")
168+
data.Set("address", "http://127.0.0.1:21/") // valid scenario, the final stored address will be http://127.0.0.0.1:21
170169
w = httptest.NewRecorder()
171170
req, _ = http.NewRequest("POST", "/v1/cover/register", strings.NewReader(data.Encode()))
172171
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
173172
router.ServeHTTP(w, req)
174-
assert.Equal(t, http.StatusBadRequest, w.Code)
175-
assert.Contains(t, w.Body.String(), "uri path must empty")
173+
assert.Equal(t, http.StatusOK, w.Code)
176174

177175
// regist with empty host(in direct mode,empty must fail,default is success)
178176
// in default,use request realhost as host,use 80 as port
@@ -185,32 +183,20 @@ func TestRegisterService(t *testing.T) {
185183
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
186184
router.ServeHTTP(w, req)
187185
assert.Equal(t, http.StatusBadRequest, w.Code)
188-
assert.Contains(t, w.Body.String(), "address is empty")
189-
190-
data = url.Values{}
191-
data.Set("name", "aaa")
192-
data.Set("address", "http://:8080")
193-
w = httptest.NewRecorder()
194-
req, _ = http.NewRequest("POST", "/v1/cover/register", strings.NewReader(data.Encode()))
195-
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
196-
router.ServeHTTP(w, req)
197-
assert.Equal(t, http.StatusBadRequest, w.Code)
198-
assert.Contains(t, w.Body.String(), "empty host name")
186+
assert.Contains(t, w.Body.String(), "empty host")
199187

200188
data = url.Values{}
201189
data.Set("name", "aaa")
202-
data.Set("address", "http://127.0.0.1")
190+
data.Set("address", "http://:8080") //valid scenario, be consistent with curl
203191
w = httptest.NewRecorder()
204192
req, _ = http.NewRequest("POST", "/v1/cover/register", strings.NewReader(data.Encode()))
205193
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
206194
router.ServeHTTP(w, req)
207195
assert.Equal(t, http.StatusOK, w.Code)
208196

209-
//change network type
210197
data = url.Values{}
211198
data.Set("name", "aaa")
212-
data.Set("address", "http://")
213-
data.Set("iprevise", "true")
199+
data.Set("address", "http://127.0.0.1")
214200
w = httptest.NewRecorder()
215201
req, _ = http.NewRequest("POST", "/v1/cover/register", strings.NewReader(data.Encode()))
216202
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
@@ -225,7 +211,7 @@ func TestRegisterService(t *testing.T) {
225211
req, _ = http.NewRequest("POST", "/v1/cover/register", strings.NewReader(data.Encode()))
226212
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
227213
router.ServeHTTP(w, req)
228-
assert.Equal(t, http.StatusOK, w.Code)
214+
assert.Equal(t, http.StatusBadRequest, w.Code)
229215

230216
data = url.Values{}
231217
data.Set("name", "aaa")
@@ -248,8 +234,8 @@ func TestRegisterService(t *testing.T) {
248234
// register with store failure
249235
expectedS := ServiceUnderTest{
250236
Name: "foo",
251-
Address: "http://:64444", // the real IP is empty in unittest, so server will get a empty one
252-
IPRevise: strconv.FormatBool(server.IPRevise),
237+
Address: "http://:8080",
238+
IPRevise: "true",
253239
}
254240
testObj := new(MockStore)
255241
testObj.On("Get", "foo").Return([]string{"http://127.0.0.1:66666"})
@@ -260,7 +246,7 @@ func TestRegisterService(t *testing.T) {
260246
w = httptest.NewRecorder()
261247
data.Set("name", expectedS.Name)
262248
data.Set("address", expectedS.Address)
263-
data.Set("network", "")
249+
data.Set("ip_revise", expectedS.IPRevise)
264250
req, _ = http.NewRequest("POST", "/v1/cover/register", strings.NewReader(data.Encode()))
265251
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
266252
router.ServeHTTP(w, req)

0 commit comments

Comments
 (0)