Skip to content

Commit 227b200

Browse files
jwrookiestarsz
andauthored
fix: create upstream error when pass host is node and nodes without port (#2421)
Co-authored-by: Peter Zhu <[email protected]>
1 parent 92f46d9 commit 227b200

File tree

6 files changed

+220
-7
lines changed

6 files changed

+220
-7
lines changed

api/go.sum

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -271,6 +271,7 @@ github.com/hashicorp/logutils v1.0.0/go.mod h1:QIAnNjmIWmVIIkWDTG1z5v++HQmx9WQRO
271271
github.com/hashicorp/mdns v1.0.0/go.mod h1:tL+uN++7HEJ6SQLQ2/p+z2pH24WQKWjBPkE0mNTz8vQ=
272272
github.com/hashicorp/memberlist v0.1.3/go.mod h1:ajVTdAv/9Im8oMAAj5G31PhhMCZJV2pPBoIllUwCN7I=
273273
github.com/hashicorp/serf v0.8.2/go.mod h1:6hOLApaqBFA1NXqRQAsxw9QxuDEvNxSQRwA/JwenrHc=
274+
github.com/hpcloud/tail v1.0.0 h1:nfCOvKYfkgYP8hkirhJocXT2+zOD8yUNjXaWfTlyFKI=
274275
github.com/hpcloud/tail v1.0.0/go.mod h1:ab1qPbhIpdTxEkNHXyeSf5vhxWSCs/tWer42PpOxQnU=
275276
github.com/hudl/fargo v1.3.0/go.mod h1:y3CKSmjA+wD2gak7sUSXTAoopbhU08POFhmITJgmKTg=
276277
github.com/ianlancetaylor/demangle v0.0.0-20181102032728-5e5cf60278f6/go.mod h1:aSSvb/t6k1mPoxDqO4vJh6VOCGPwU4O0C2/Eqndh1Sc=
@@ -357,7 +358,9 @@ github.com/oklog/oklog v0.3.2/go.mod h1:FCV+B7mhrz4o+ueLpx+KqkyXRGMWOYEvfiXtdGtb
357358
github.com/oklog/run v1.0.0/go.mod h1:dlhp/R75TPv97u0XWUtDeV/lRKWPKSdTuV0TZvrmrQA=
358359
github.com/olekukonko/tablewriter v0.0.0-20170122224234-a0225b3f23b5/go.mod h1:vsDQFd/mU46D+Z4whnwzcISnGGzXWMclvtLoiIKAKIo=
359360
github.com/onsi/ginkgo v1.6.0/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+WWjE=
361+
github.com/onsi/ginkgo v1.7.0 h1:WSHQ+IS43OoUrWtD1/bbclrwK8TTH5hzp+umCiuxHgs=
360362
github.com/onsi/ginkgo v1.7.0/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+WWjE=
363+
github.com/onsi/gomega v1.4.3 h1:RE1xgDvH7imwFD45h+u2SgIfERHlS2yNG4DObb5BSKU=
361364
github.com/onsi/gomega v1.4.3/go.mod h1:ex+gbHU/CVuBBDIJjb2X0qEXbFg53c61hWP/1CpauHY=
362365
github.com/op/go-logging v0.0.0-20160315200505-970db520ece7/go.mod h1:HzydrMdWErDVzsI23lYNej1Htcns9BCg93Dk0bBINWk=
363366
github.com/opentracing-contrib/go-observer v0.0.0-20170622124052-a52f23424492/go.mod h1:Ngi6UdF0k5OKD5t5wlmGhe/EDKPoUM3BXZSSfIuJbis=
@@ -878,13 +881,15 @@ gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15 h1:YR8cESwS4TdDjEe65xsg0ogR
878881
gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
879882
gopkg.in/cheggaaa/pb.v1 v1.0.25/go.mod h1:V/YB90LKu/1FcN3WVnfiiE5oMCibMjukxqG/qStrOgw=
880883
gopkg.in/errgo.v2 v2.1.0/go.mod h1:hNsd1EY+bozCKY1Ytp96fpM3vjJbqLJn88ws8XvfDNI=
884+
gopkg.in/fsnotify.v1 v1.4.7 h1:xOHLXZwVvI9hhs+cLKq5+I5onOuwQLhQwiu63xxlHs4=
881885
gopkg.in/fsnotify.v1 v1.4.7/go.mod h1:Tz8NjZHkW78fSQdbUxIjBTcgA1z1m8ZHf0WmKUhAMys=
882886
gopkg.in/gcfg.v1 v1.2.3/go.mod h1:yesOnuUOFQAhST5vPY4nbZsb/huCgGGXlipJsBn0b3o=
883887
gopkg.in/go-playground/assert.v1 v1.2.1/go.mod h1:9RXL0bg/zibRAgZUYszZSwO/z8Y/a8bDuhia5mkpMnE=
884888
gopkg.in/go-playground/validator.v9 v9.29.1/go.mod h1:+c9/zcJMFNgbLvly1L1V+PpxWdVbfP1avr/N00E2vyQ=
885889
gopkg.in/ini.v1 v1.62.0 h1:duBzk771uxoUuOlyRLkHsygud9+5lrlGjdFBb4mSKDU=
886890
gopkg.in/ini.v1 v1.62.0/go.mod h1:pNLf8WUiyNEtQjuu5G5vTm06TEv9tsIgeAvK8hOrP4k=
887891
gopkg.in/resty.v1 v1.12.0/go.mod h1:mDo4pnntr5jdWRML875a/NmxYqAlA73dVijT2AXvQQo=
892+
gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7 h1:uRGJdciOHaEIrze2W8Q3AKkepLTh2hOroT7a+7czfdQ=
888893
gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7/go.mod h1:dt/ZhP58zS4L8KSrWDmTeBkI65Dw0HsyUHuEVlX15mw=
889894
gopkg.in/warnings.v0 v0.1.2/go.mod h1:jksf8JmL6Qr/oQM2OXTHunEvvTAsrWBLb6OOjuVWRNI=
890895
gopkg.in/yaml.v2 v2.0.0-20170812160011-eb3733d160e7/go.mod h1:JAlM8MvJe8wmxCU4Bli9HhUf9+ttbYbLASfIpnQbh74=

api/internal/core/entity/format.go

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,17 +17,23 @@
1717
package entity
1818

1919
import (
20-
"net"
20+
"errors"
2121
"strconv"
22+
"strings"
2223

2324
"github.com/apisix/manager-api/internal/log"
2425
)
2526

2627
func mapKV2Node(key string, val float64) (*Node, error) {
27-
host, port, err := net.SplitHostPort(key)
28-
if err != nil {
29-
log.Errorf("split host port fail: %s", err)
30-
return nil, err
28+
hp := strings.Split(key, ":")
29+
host := hp[0]
30+
// according to APISIX upstream nodes policy, port is optional
31+
port := "0"
32+
33+
if len(hp) > 2 {
34+
return nil, errors.New("invalid upstream node")
35+
} else if len(hp) == 2 {
36+
port = hp[1]
3137
}
3238

3339
portInt, err := strconv.Atoi(port)

api/internal/core/entity/format_test.go

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,17 @@ func TestNodesFormat_no_nodes(t *testing.T) {
180180
assert.Contains(t, jsonStr, `null`)
181181
}
182182

183+
func TestNodesFormat_nodes_without_port(t *testing.T) {
184+
nodes := map[string]float64{"127.0.0.1": 0}
185+
// nodes format
186+
formattedNodes := NodesFormat(nodes)
187+
188+
// json encode for client
189+
res, err := json.Marshal(formattedNodes)
190+
assert.Nil(t, err)
191+
assert.Equal(t, res, []byte(`[{"host":"127.0.0.1","weight":0}]`))
192+
}
193+
183194
func Test_Idle_Timeout_nil_and_zero(t *testing.T) {
184195
ukp0 := UpstreamKeepalivePool{}
185196
// Unmarshal from zero value
@@ -229,3 +240,56 @@ func TestUpstream_nil_and_zero_retries(t *testing.T) {
229240
assert.Nil(t, err)
230241
assert.Equal(t, string(marshaledNull), `{}`)
231242
}
243+
244+
func TestMapKV2Node(t *testing.T) {
245+
testCases := []struct {
246+
name string
247+
key string
248+
value float64
249+
wantErr bool
250+
errMessage string
251+
wantRes *Node
252+
}{
253+
{
254+
name: "invalid upstream node",
255+
key: "127.0.0.1:0:0",
256+
wantErr: true,
257+
errMessage: "invalid upstream node",
258+
},
259+
{
260+
name: "when address contains port convert should succeed",
261+
key: "127.0.0.1:8080",
262+
value: 100,
263+
wantErr: false,
264+
wantRes: &Node{
265+
Host: "127.0.0.1",
266+
Port: 8080,
267+
Weight: 100,
268+
},
269+
},
270+
{
271+
name: "when address without port convert should succeed",
272+
key: "127.0.0.1",
273+
wantErr: false,
274+
wantRes: &Node{
275+
Host: "127.0.0.1",
276+
Port: 0,
277+
Weight: 0,
278+
},
279+
},
280+
}
281+
282+
for _, tc := range testCases {
283+
t.Run(tc.name, func(t *testing.T) {
284+
got, err := mapKV2Node(tc.key, tc.value)
285+
if tc.wantErr {
286+
assert.NotNil(t, err)
287+
assert.Contains(t, err.Error(), tc.errMessage)
288+
return
289+
}
290+
291+
assert.Nil(t, err)
292+
assert.Equal(t, tc.wantRes, got)
293+
})
294+
}
295+
}

api/internal/core/store/validate.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -166,8 +166,11 @@ func checkUpstream(upstream *entity.UpstreamDef) error {
166166
}
167167

168168
if upstream.PassHost == "node" && upstream.Nodes != nil {
169-
if nodes := entity.NodesFormat(upstream.Nodes); len(nodes.([]*entity.Node)) != 1 {
170-
return fmt.Errorf("only support single node for `node` mode currently")
169+
nodes, ok := entity.NodesFormat(upstream.Nodes).([]*entity.Node)
170+
if !ok {
171+
return fmt.Errorf("upstrams nodes not support value %v when `pass_host` is `node`", nodes)
172+
} else if len(nodes) != 1 {
173+
return fmt.Errorf("only support single node for `node` mode currentlywhen `pass_host` is `node`")
171174
}
172175
}
173176

api/internal/handler/upstream/upstream_test.go

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -533,6 +533,75 @@ func TestUpstream_Create(t *testing.T) {
533533
},
534534
wantErr: nil,
535535
},
536+
{
537+
caseDesc: "when nodes address without port and pass host is node, create should succeed",
538+
getCalled: true,
539+
giveInput: &entity.Upstream{
540+
BaseInfo: entity.BaseInfo{
541+
ID: "u1",
542+
},
543+
UpstreamDef: entity.UpstreamDef{
544+
Name: "upstream1",
545+
Timeout: &entity.Timeout{
546+
Connect: 15,
547+
Send: 15,
548+
Read: 15,
549+
},
550+
Key: "server_addr",
551+
Nodes: map[string]float64{"127.0.0.1": 100},
552+
PassHost: "node",
553+
},
554+
},
555+
giveRet: &entity.Upstream{
556+
BaseInfo: entity.BaseInfo{
557+
ID: "u1",
558+
},
559+
UpstreamDef: entity.UpstreamDef{
560+
Name: "upstream1",
561+
Timeout: &entity.Timeout{
562+
Connect: 15,
563+
Send: 15,
564+
Read: 15,
565+
},
566+
Key: "server_addr",
567+
Nodes: map[string]float64{"127.0.0.1": 100},
568+
PassHost: "node",
569+
},
570+
},
571+
wantInput: &entity.Upstream{
572+
BaseInfo: entity.BaseInfo{
573+
ID: "u1",
574+
},
575+
UpstreamDef: entity.UpstreamDef{
576+
Name: "upstream1",
577+
Timeout: &entity.Timeout{
578+
Connect: 15,
579+
Send: 15,
580+
Read: 15,
581+
},
582+
Key: "server_addr",
583+
Nodes: map[string]float64{"127.0.0.1": 100},
584+
PassHost: "node",
585+
},
586+
},
587+
wantRet: &entity.Upstream{
588+
BaseInfo: entity.BaseInfo{
589+
ID: "u1",
590+
},
591+
UpstreamDef: entity.UpstreamDef{
592+
Name: "upstream1",
593+
Timeout: &entity.Timeout{
594+
Connect: 15,
595+
Send: 15,
596+
Read: 15,
597+
},
598+
Key: "server_addr",
599+
Nodes: map[string]float64{"127.0.0.1": 100},
600+
PassHost: "node",
601+
},
602+
},
603+
wantErr: nil,
604+
},
536605
{
537606
caseDesc: "create failed, create return error",
538607
getCalled: true,

api/test/e2enew/upstream/upstream_test.go

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,63 @@ var _ = ginkgo.Describe("Upstream", func() {
8888
ExpectStatus: http.StatusOK,
8989
})
9090
})
91+
ginkgo.It("create upstream3 success when pass host is 'node' and nodes without port", func() {
92+
ginkgo.By("create upstream3", func() {
93+
createUpstreamBody := make(map[string]interface{})
94+
createUpstreamBody["name"] = "upstream3"
95+
createUpstreamBody["nodes"] = map[string]float64{base.UpstreamIp: 100}
96+
createUpstreamBody["type"] = "roundrobin"
97+
createUpstreamBody["pass_host"] = "node"
98+
99+
_createUpstreamBody, err := json.Marshal(createUpstreamBody)
100+
gomega.Expect(err).To(gomega.BeNil())
101+
base.RunTestCase(base.HttpTestCase{
102+
Object: base.ManagerApiExpect(),
103+
Method: http.MethodPut,
104+
Path: "/apisix/admin/upstreams/3",
105+
Body: string(_createUpstreamBody),
106+
Headers: map[string]string{"Authorization": base.GetToken()},
107+
ExpectStatus: http.StatusOK,
108+
})
109+
})
110+
111+
ginkgo.By("create route using the upstream3", func() {
112+
base.RunTestCase(base.HttpTestCase{
113+
Object: base.ManagerApiExpect(),
114+
Method: http.MethodPut,
115+
Path: "/apisix/admin/routes/1",
116+
Body: `{
117+
"name": "route1",
118+
"uri": "/hello",
119+
"upstream_id": "3"
120+
}`,
121+
Headers: map[string]string{"Authorization": base.GetToken()},
122+
ExpectStatus: http.StatusOK,
123+
Sleep: base.SleepTime,
124+
})
125+
})
126+
127+
ginkgo.By("hit the route just created", func() {
128+
base.RunTestCase(base.HttpTestCase{
129+
Object: base.APISIXExpect(),
130+
Method: http.MethodGet,
131+
Path: "/hello",
132+
ExpectStatus: http.StatusOK,
133+
ExpectBody: "hello",
134+
Sleep: base.SleepTime,
135+
})
136+
})
137+
138+
ginkgo.By("delete route", func() {
139+
base.RunTestCase(base.HttpTestCase{
140+
Object: base.ManagerApiExpect(),
141+
Method: http.MethodDelete,
142+
Path: "/apisix/admin/routes/1",
143+
Headers: map[string]string{"Authorization": base.GetToken()},
144+
ExpectStatus: http.StatusOK,
145+
})
146+
})
147+
})
91148
ginkgo.It("create upstream failed, name existed", func() {
92149
createUpstreamBody := make(map[string]interface{})
93150
createUpstreamBody["name"] = "upstream2"
@@ -252,6 +309,15 @@ var _ = ginkgo.Describe("Upstream", func() {
252309
ExpectStatus: http.StatusOK,
253310
})
254311
})
312+
ginkgo.It("delete upstream3", func() {
313+
base.RunTestCase(base.HttpTestCase{
314+
Object: base.ManagerApiExpect(),
315+
Method: http.MethodDelete,
316+
Path: "/apisix/admin/upstreams/3",
317+
Headers: map[string]string{"Authorization": base.GetToken()},
318+
ExpectStatus: http.StatusOK,
319+
})
320+
})
255321
ginkgo.It("hit the route just deleted", func() {
256322
base.RunTestCase(base.HttpTestCase{
257323
Object: base.APISIXExpect(),

0 commit comments

Comments
 (0)