Skip to content

Commit

Permalink
fix: search labels with the same key (#1151)
Browse files Browse the repository at this point in the history 
* fix: search labels with the same key (#1130)
  • Loading branch information
@starsz
starsz authored Dec 31, 2020
1 parent 2dea025 commit c0241ee
Show file tree
Hide file tree
Showing 6 changed files with 145 additions and 42 deletions.
9 changes: 6 additions & 3 deletions api/internal/handler/label/label.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,15 +78,18 @@ type ListInput struct {
Label string `auto_read:"label,query"`
}

func subsetOf(reqLabels, labels map[string]string) map[string]string {
func subsetOf(reqLabels map[string]struct{}, labels map[string]string) map[string]string {
if len(reqLabels) == 0 {
return labels
}

var res = make(map[string]string)
for k, v := range labels {
l, exist := reqLabels[k]
if exist && ((l == "") || v == l) {
if _, exist := reqLabels[k]; exist {
res[k] = v
}

if _, exist := reqLabels[k+":"+v]; exist {
res[k] = v
}
}
Expand Down
69 changes: 47 additions & 22 deletions api/internal/handler/label/label_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -221,30 +221,43 @@ func TestLabel(t *testing.T) {
}
}

var testCases []*testCase

expect := []interface{}{
Pair{"label1", "value1"},
Pair{"label1", "value2"},
Pair{"label2", "value2"},
}
case1 := newCase(giveData, expect)
case1.giveInput.Type = typ
tc := newCase(giveData, expect)
tc.giveInput.Type = typ
testCases = append(testCases, tc)

expect = []interface{}{
Pair{"label1", "value1"},
Pair{"label1", "value2"},
}
case2 := newCase(giveData, expect)
case2.giveInput.Type = typ
case2.giveInput.Label = "label1"
tc = newCase(giveData, expect)
tc.giveInput.Type = typ
tc.giveInput.Label = "label1"
testCases = append(testCases, tc)

expect = []interface{}{
Pair{"label1", "value2"},
}
tc = newCase(giveData, expect)
tc.giveInput.Type = typ
tc.giveInput.Label = "label1:value2"
testCases = append(testCases, tc)

expect = []interface{}{
Pair{"label1", "value1"},
Pair{"label1", "value2"},
}
case3 := newCase(giveData, expect)
case3.giveInput.Type = typ
case3.giveInput.Label = "label1:value2"
tc = newCase(giveData, expect)
tc.giveInput.Type = typ
tc.giveInput.Label = "label1:value1,label1:value2"
testCases = append(testCases, tc)

testCases := []*testCase{case1, case2, case3}
handler := Handler{}
for _, tc := range testCases {
switch typ {
Expand Down Expand Up @@ -290,6 +303,8 @@ func TestLabel(t *testing.T) {
serviceStore: genMockStore(t, []interface{}{genService(m5)}),
}

var testCases []*testCase

expect := []interface{}{
Pair{"label1", "value1"},
Pair{"label1", "value2"},
Expand All @@ -298,35 +313,45 @@ func TestLabel(t *testing.T) {
Pair{"label4", "value4"},
Pair{"label5", "value5"},
}
case1 := newCase(nil, expect)
case1.giveInput.Type = "all"
tc := newCase(nil, expect)
tc.giveInput.Type = "all"
testCases = append(testCases, tc)

expect = []interface{}{
Pair{"label1", "value1"},
Pair{"label1", "value2"},
}
case2 := newCase(nil, expect)
case2.giveInput.Type = "all"
case2.giveInput.Label = "label1"
tc = newCase(nil, expect)
tc.giveInput.Type = "all"
tc.giveInput.Label = "label1"
testCases = append(testCases, tc)

expect = []interface{}{
Pair{"label1", "value2"},
}
case3 := newCase(nil, expect)
case3.giveInput.Type = "all"
case3.giveInput.Label = "label1:value2"
tc = newCase(nil, expect)
tc.giveInput.Type = "all"
tc.giveInput.Label = "label1:value2"
testCases = append(testCases, tc)

expect = []interface{}{
Pair{"label1", "value1"},
Pair{"label1", "value2"},
Pair{"label5", "value5"},
}
case4 := newCase(nil, expect)
case4.giveInput.Type = "all"
case4.giveInput.Label = "label1,label5:value5"
tc = newCase(nil, expect)
tc.giveInput.Type = "all"
tc.giveInput.Label = "label1,label5:value5"

expect = []interface{}{
Pair{"label1", "value1"},
Pair{"label1", "value2"},
}
tc = newCase(nil, expect)
tc.giveInput.Type = "all"
tc.giveInput.Label = "label1=value1,label1=value2"

testcase := []*testCase{case1, case2, case3, case4}
for _, tc := range testcase {
for _, tc := range testCases {
ctx := droplet.NewContext()
ctx.SetInput(tc.giveInput)
ret, err := handler.List(ctx)
Expand Down
21 changes: 14 additions & 7 deletions api/internal/utils/utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,9 +109,9 @@ func ObjectClone(origin, copy interface{}) error {
return err
}

func GenLabelMap(label string) (map[string]string, error) {
func GenLabelMap(label string) (map[string]struct{}, error) {
var err = errors.New("malformed label")
mp := make(map[string]string)
mp := make(map[string]struct{})

if label == "" {
return mp, nil
Expand All @@ -125,13 +125,15 @@ func GenLabelMap(label string) (map[string]string, error) {
return nil, err
}

mp[kv[0]] = kv[1]
// Because the labels may contain the same key, like this: label=version:v1,version:v2
// we need to combine them as a map's key
mp[l] = struct{}{}
} else if len(kv) == 1 {
if kv[0] == "" {
return nil, err
}

mp[kv[0]] = ""
mp[kv[0]] = struct{}{}
} else {
return nil, err
}
Expand All @@ -140,14 +142,19 @@ func GenLabelMap(label string) (map[string]string, error) {
return mp, nil
}

func LabelContains(labels, reqLabels map[string]string) bool {
func LabelContains(labels map[string]string, reqLabels map[string]struct{}) bool {
if len(reqLabels) == 0 {
return true
}

for k, v := range labels {
l, exist := reqLabels[k]
if exist && ((l == "") || v == l) {
// first check the key
if _, exist := reqLabels[k]; exist {
return true
}

// second check the key:value
if _, exist := reqLabels[k+":"+v]; exist {
return true
}
}
Expand Down
37 changes: 27 additions & 10 deletions api/internal/utils/utils_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,12 +66,17 @@ func TestGenLabelMap(t *testing.T) {
expectedErr := errors.New("malformed label")
mp, err := GenLabelMap("l1")
assert.Nil(t, err)
assert.Equal(t, mp["l1"], "")
assert.Equal(t, mp["l1"], struct{}{})

mp, err = GenLabelMap("l1,l2:v2")
assert.Nil(t, err)
assert.Equal(t, mp["l1"], "")
assert.Equal(t, mp["l2"], "v2")
assert.Equal(t, mp["l1"], struct{}{})
assert.Equal(t, mp["l2:v2"], struct{}{})

mp, err = GenLabelMap("l1:v1,l1:v2")
assert.Nil(t, err)
assert.Equal(t, mp["l1:v1"], struct{}{})
assert.Equal(t, mp["l1:v2"], struct{}{})

mp, err = GenLabelMap(",")
assert.Equal(t, expectedErr, err)
Expand All @@ -83,20 +88,32 @@ func TestGenLabelMap(t *testing.T) {
}

func TestLabelContains(t *testing.T) {
mp1, _ := GenLabelMap("l1,l2:v2")
mp2 := map[string]string{
reqMap, _ := GenLabelMap("l1,l2:v2")
mp := map[string]string{
"l1": "v1",
}
assert.True(t, LabelContains(mp2, mp1))
assert.True(t, LabelContains(mp, reqMap))

mp3 := map[string]string{
mp = map[string]string{
"l1": "v1",
"l2": "v3",
}
assert.True(t, LabelContains(mp3, mp1))
assert.True(t, LabelContains(mp, reqMap))

mp4 := map[string]string{
mp = map[string]string{
"l2": "v3",
}
assert.False(t, LabelContains(mp4, mp1))
assert.False(t, LabelContains(mp, reqMap))

reqMap, _ = GenLabelMap("l1:v1,l1:v2")
mp = map[string]string{
"l1": "v1",
}
assert.True(t, LabelContains(mp, reqMap))

reqMap, _ = GenLabelMap("l1:v1,l1:v2")
mp = map[string]string{
"l1": "v2",
}
assert.True(t, LabelContains(mp, reqMap))
}
40 changes: 40 additions & 0 deletions api/test/e2e/label_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,16 @@ func TestLabel(t *testing.T) {
ExpectStatus: http.StatusOK,
ExpectBody: "{\"build\":\"16\"},{\"build\":\"17\"}",
},
{
Desc: "get labels with the same key (key = build)",
Object: ManagerApiExpect(t),
Method: http.MethodGet,
Headers: map[string]string{"Authorization": token},
Query: "label=build:16,build:17",
Path: "/apisix/admin/labels/all",
ExpectStatus: http.StatusOK,
ExpectBody: "{\"build\":\"16\"},{\"build\":\"17\"}",
},
{
Desc: "get labels (key = build) with page",
Object: ManagerApiExpect(t),
Expand All @@ -210,6 +220,26 @@ func TestLabel(t *testing.T) {
ExpectStatus: http.StatusOK,
ExpectBody: "{\"build\":\"17\"}",
},
{
Desc: "get labels with same key (key = build) and page",
Object: ManagerApiExpect(t),
Method: http.MethodGet,
Headers: map[string]string{"Authorization": token},
Query: "label=build:16,build:17&page=1&page_size=2",
Path: "/apisix/admin/labels/all",
ExpectStatus: http.StatusOK,
ExpectBody: "{\"build\":\"16\"},{\"build\":\"17\"}",
},
{
Desc: "get labels with same key (key = build) and page",
Object: ManagerApiExpect(t),
Method: http.MethodGet,
Headers: map[string]string{"Authorization": token},
Query: "label=build:16,build:17&page=2&page_size=1",
Path: "/apisix/admin/labels/all",
ExpectStatus: http.StatusOK,
ExpectBody: "{\"build\":\"17\"}",
},
{
Desc: "get labels (key = build && env = production)",
Object: ManagerApiExpect(t),
Expand All @@ -220,6 +250,16 @@ func TestLabel(t *testing.T) {
ExpectStatus: http.StatusOK,
ExpectBody: "{\"build\":\"16\"},{\"build\":\"17\"},{\"env\":\"production\"}",
},
{
Desc: "get labels (build=16 | 17 and env = production)",
Object: ManagerApiExpect(t),
Method: http.MethodGet,
Headers: map[string]string{"Authorization": token},
Query: "label=build:16,build:17,env:production",
Path: "/apisix/admin/labels/all",
ExpectStatus: http.StatusOK,
ExpectBody: "{\"build\":\"16\"},{\"build\":\"17\"},{\"env\":\"production\"}",
},
{
Desc: "get labels (key = build && env = production) with page",
Object: ManagerApiExpect(t),
Expand Down
11 changes: 11 additions & 0 deletions api/test/e2e/route_with_management_fileds_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,17 @@ func TestRoute_search_by_label(t *testing.T) {
ExpectBody: "\"total_size\":2",
Sleep: sleepTime,
},
{
Desc: "search the route by label (combination)",
Object: ManagerApiExpect(t),
Path: "/apisix/admin/routes",
Query: "label=build:16,build:17",
Method: http.MethodGet,
Headers: map[string]string{"Authorization": token},
ExpectStatus: http.StatusOK,
ExpectBody: "\"total_size\":2",
Sleep: sleepTime,
},
{
Desc: "delete the route (r1)",
Object: ManagerApiExpect(t),
Expand Down

0 comments on commit c0241ee

Please sign in to comment.