From aa2a211699adb459806610f555d9f6351ef06414 Mon Sep 17 00:00:00 2001 From: dongjianhui Date: Tue, 15 Jun 2021 22:17:00 +0800 Subject: [PATCH 1/4] make the package v3router/judger test coverage rate reach 80% --- .../judger/attachment_match_judger.go | 59 +++++++-------- .../judger/attachment_match_judger_test.go | 15 ++-- .../judger/list_string_match_judger_test.go | 34 +++++++++ .../v3router/judger/method_match_judger.go | 1 + .../judger/method_match_judger_test.go | 72 +++++++++++++++++++ .../judger/url_label_match_judge_test.go | 35 +++++++++ cluster/router/v3router/router_chain.go | 6 ++ metadata/report/etcd/report.go | 4 +- 8 files changed, 184 insertions(+), 42 deletions(-) create mode 100644 cluster/router/v3router/judger/list_string_match_judger_test.go create mode 100644 cluster/router/v3router/judger/method_match_judger_test.go create mode 100644 cluster/router/v3router/judger/url_label_match_judge_test.go diff --git a/cluster/router/v3router/judger/attachment_match_judger.go b/cluster/router/v3router/judger/attachment_match_judger.go index cb4d3eaa95..1d215375d4 100644 --- a/cluster/router/v3router/judger/attachment_match_judger.go +++ b/cluster/router/v3router/judger/attachment_match_judger.go @@ -29,45 +29,34 @@ type AttachmentMatchJudger struct { // nolint func (amj *AttachmentMatchJudger) Judge(invocation protocol.Invocation) bool { invAttaMap := invocation.Attachments() - if amj.EagleeyeContext != nil { - for k, v := range amj.EagleeyeContext { - invAttaValue, ok := invAttaMap[k] - if !ok { - if v.Empty == "" { - return false - } - continue - } - // exist this key - str, ok := invAttaValue.(string) - if !ok { - return false - } - strJudger := NewStringMatchJudger(v) - if !strJudger.Judge(str) { - return false - } - } + if amj.EagleeyeContext != nil && !judge(amj.EagleeyeContext, invAttaMap) { + return false } - if amj.DubboContext != nil { - for k, v := range amj.DubboContext { - invAttaValue, ok := invAttaMap[k] - if !ok { - if v.Empty == "" { - return false - } - continue - } - // exist this key - str, ok := invAttaValue.(string) - if !ok { - return false - } - strJudger := NewStringMatchJudger(v) - if !strJudger.Judge(str) { + if amj.DubboContext != nil && !judge(amj.DubboContext, invAttaMap) { + return false + } + + return true +} + +func judge(condition map[string]*config.StringMatch, invAttaMap map[string]interface{}) bool { + for k, v := range condition { + invAttaValue, ok := invAttaMap[k] + if !ok { + if v.Empty == "" { return false } + continue + } + // exist this key + str, ok := invAttaValue.(string) + if !ok { + return false + } + strJudger := NewStringMatchJudger(v) + if !strJudger.Judge(str) { + return false } } diff --git a/cluster/router/v3router/judger/attachment_match_judger_test.go b/cluster/router/v3router/judger/attachment_match_judger_test.go index 7e6b324947..26a8d5ed59 100644 --- a/cluster/router/v3router/judger/attachment_match_judger_test.go +++ b/cluster/router/v3router/judger/attachment_match_judger_test.go @@ -31,19 +31,24 @@ import ( ) func TestAttachmentMatchJudger(t *testing.T) { - dubboCtxMap := make(map[string]*config.StringMatch) + conditionMap := make(map[string]*config.StringMatch) dubboIvkMap := make(map[string]interface{}) - dubboCtxMap["test-key"] = &config.StringMatch{ + conditionMap["test-key"] = &config.StringMatch{ Exact: "abc", } dubboIvkMap["test-key"] = "abc" assert.True(t, NewAttachmentMatchJudger(&config.DubboAttachmentMatch{ - DubboContext: dubboCtxMap, + DubboContext: conditionMap, + }).Judge(invocation.NewRPCInvocation("method", nil, dubboIvkMap))) + assert.True(t, NewAttachmentMatchJudger(&config.DubboAttachmentMatch{ + EagleeyeContext: conditionMap, }).Judge(invocation.NewRPCInvocation("method", nil, dubboIvkMap))) dubboIvkMap["test-key"] = "abd" assert.False(t, NewAttachmentMatchJudger(&config.DubboAttachmentMatch{ - DubboContext: dubboCtxMap, + DubboContext: conditionMap, + }).Judge(invocation.NewRPCInvocation("method", nil, dubboIvkMap))) + assert.False(t, NewAttachmentMatchJudger(&config.DubboAttachmentMatch{ + EagleeyeContext: conditionMap, }).Judge(invocation.NewRPCInvocation("method", nil, dubboIvkMap))) - } diff --git a/cluster/router/v3router/judger/list_string_match_judger_test.go b/cluster/router/v3router/judger/list_string_match_judger_test.go new file mode 100644 index 0000000000..12144cfc19 --- /dev/null +++ b/cluster/router/v3router/judger/list_string_match_judger_test.go @@ -0,0 +1,34 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package judger + +import ( + "dubbo.apache.org/dubbo-go/v3/config" + "github.com/stretchr/testify/assert" + "testing" +) + +func TestListStringMatchJudger(t *testing.T) { + assert.True(t, newListStringMatchJudger(&config.ListStringMatch{ + Oneof: []*config.StringMatch{{Exact: "abd"}}, + }).Judge("abd")) + + assert.False(t, newListStringMatchJudger(&config.ListStringMatch{ + Oneof: []*config.StringMatch{{Exact: "abc"}}, + }).Judge("abd")) +} diff --git a/cluster/router/v3router/judger/method_match_judger.go b/cluster/router/v3router/judger/method_match_judger.go index edf8fa0b4b..e069ef9cca 100644 --- a/cluster/router/v3router/judger/method_match_judger.go +++ b/cluster/router/v3router/judger/method_match_judger.go @@ -57,6 +57,7 @@ func (mmj *MethodMatchJudger) Judge(invocation protocol.Invocation) bool { if !newListStringMatchJudger(v.StrValue).Judge(value.String()) { return false } + // FIXME int invoke Float may cause panic case "float", "int": // todo now numbers Must not be zero, else it will ignore this match if !newListDoubleMatchJudger(v.NumValue).Judge(value.Float()) { diff --git a/cluster/router/v3router/judger/method_match_judger_test.go b/cluster/router/v3router/judger/method_match_judger_test.go new file mode 100644 index 0000000000..8e7c72a878 --- /dev/null +++ b/cluster/router/v3router/judger/method_match_judger_test.go @@ -0,0 +1,72 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package judger + +import ( + "dubbo.apache.org/dubbo-go/v3/config" + "dubbo.apache.org/dubbo-go/v3/protocol/invocation" + "github.com/stretchr/testify/assert" + "reflect" + "testing" +) + +func TestMethodMatchJudger(t *testing.T) { + + methodArgs := make([]*config.DubboMethodArg, 0) + methodArgs = append(methodArgs, &config.DubboMethodArg{ + Index: 1, + Type: "string", + StrValue: &config.ListStringMatch{Oneof: []*config.StringMatch{{Exact: "hello world"}}}, + NumValue: nil, + BoolValue: nil, + }) + methodArgs = append(methodArgs, &config.DubboMethodArg{ + Index: 2, + Type: "bool", + StrValue: nil, + NumValue: nil, + BoolValue: &config.BoolMatch{Exact: true}, + }) + methodArgs = append(methodArgs, &config.DubboMethodArg{ + Index: 3, + Type: "float64", + StrValue: nil, + NumValue: &config.ListDoubleMatch{Oneof: []*config.DoubleMatch{{Exact: 10}}}, + BoolValue: nil, + }) + + methodMatch := &config.DubboMethodMatch{ + NameMatch: &config.StringMatch{Exact: "Greet"}, + Argc: 3, + Args: methodArgs, + Argp: nil, + Headers: nil, + } + + stringValue := reflect.ValueOf("hello world") + boolValue := reflect.ValueOf(true) + numValue := reflect.ValueOf(10.0) + ivc := invocation.NewRPCInvocationWithOptions( + invocation.WithMethodName("Greet"), + invocation.WithParameterValues([]reflect.Value{stringValue, boolValue, numValue}), + ) + + assert.False(t, NewMethodMatchJudger(&config.DubboMethodMatch{NameMatch: &config.StringMatch{Exact: "Great"}}).Judge(ivc)) + assert.False(t, NewMethodMatchJudger(&config.DubboMethodMatch{NameMatch: &config.StringMatch{Exact: "Greet"}, Argc: 1}).Judge(ivc)) + assert.True(t, NewMethodMatchJudger(methodMatch).Judge(ivc)) +} diff --git a/cluster/router/v3router/judger/url_label_match_judge_test.go b/cluster/router/v3router/judger/url_label_match_judge_test.go new file mode 100644 index 0000000000..eb27b1e5c8 --- /dev/null +++ b/cluster/router/v3router/judger/url_label_match_judge_test.go @@ -0,0 +1,35 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package judger + +import ( + "dubbo.apache.org/dubbo-go/v3/common" + "github.com/stretchr/testify/assert" + "testing" +) + +func TestJudgeUrlLabel(t *testing.T) { + url := common.NewURLWithOptions(common.WithParamsValue("a", "A")) + + labels := make(map[string]string) + labels["a"] = "A" + assert.True(t, JudgeUrlLabel(url, labels)) + + labels["a"] = "B" + assert.False(t, JudgeUrlLabel(url, labels)) +} diff --git a/cluster/router/v3router/router_chain.go b/cluster/router/v3router/router_chain.go index 1ae21304ff..599b5b595b 100644 --- a/cluster/router/v3router/router_chain.go +++ b/cluster/router/v3router/router_chain.go @@ -180,6 +180,7 @@ func parseFromConfigToRouters(virtualServiceConfig, destinationRuleConfig []byte vsDecoder := yaml.NewDecoder(strings.NewReader(string(virtualServiceConfig))) drDecoder := yaml.NewDecoder(strings.NewReader(string(destinationRuleConfig))) + // parse virtual service for { virtualServiceCfg := &config.VirtualServiceConfig{} @@ -195,6 +196,7 @@ func parseFromConfigToRouters(virtualServiceConfig, destinationRuleConfig []byte virtualServiceConfigList = append(virtualServiceConfigList, virtualServiceCfg) } + // parse destination rule for { destRuleCfg := &config.DestinationRuleConfig{} err := drDecoder.Decode(destRuleCfg) @@ -205,10 +207,14 @@ func parseFromConfigToRouters(virtualServiceConfig, destinationRuleConfig []byte logger.Error("parseFromConfigTo destination rule err = ", err) return nil, err } + + // name -> labels destRuleCfgMap := make(map[string]map[string]string) for _, v := range destRuleCfg.Spec.SubSets { destRuleCfgMap[v.Name] = v.Labels } + + // host -> name -> labels destRuleConfigsMap[destRuleCfg.Spec.Host] = destRuleCfgMap } diff --git a/metadata/report/etcd/report.go b/metadata/report/etcd/report.go index bec551c843..836b43b329 100644 --- a/metadata/report/etcd/report.go +++ b/metadata/report/etcd/report.go @@ -58,7 +58,7 @@ func (e *etcdMetadataReport) GetAppMetadata(metadataIdentifier *identifier.Subsc if err != nil { return nil, err } - + info := &common.MetadataInfo{} return info, json.Unmarshal([]byte(data), info) } @@ -70,7 +70,7 @@ func (e *etcdMetadataReport) PublishAppMetadata(metadataIdentifier *identifier.S if err == nil { err = e.client.Put(key, string(value)) } - + return err } From 73e97b1687d1a668fbbfb50ec92af664df42ae2e Mon Sep 17 00:00:00 2001 From: Dong Jianhui <978007503@qq.com> Date: Wed, 16 Jun 2021 23:38:50 +0800 Subject: [PATCH 2/4] add router_chain unit test --- cluster/router/v3router/router_chain.go | 4 +- cluster/router/v3router/router_chain_test.go | 49 ++++++++++++++++++++ config/uniform_router_config.go | 2 +- 3 files changed, 52 insertions(+), 3 deletions(-) diff --git a/cluster/router/v3router/router_chain.go b/cluster/router/v3router/router_chain.go index 599b5b595b..dc14c7325d 100644 --- a/cluster/router/v3router/router_chain.go +++ b/cluster/router/v3router/router_chain.go @@ -95,7 +95,7 @@ func (r *RouterChain) Process(event *config_center.ConfigChangeEvent) { logger.Error("newVSValue.ObjectMeta.Annotations has no key named kubectl.kubernetes.io/last-applied-configuration") return } - logger.Debugf("json file = %v\n", newVSJsonValue) + logger.Debugf("new virtual service json value = \n%v\n", newVSJsonValue) newVirtualServiceConfig := &config.VirtualServiceConfig{} if err := json.Unmarshal([]byte(newVSJsonValue), newVirtualServiceConfig); err != nil { logger.Error("on process json data unmarshal error = ", err) @@ -148,7 +148,7 @@ func (r *RouterChain) Process(event *config_center.ConfigChangeEvent) { return } default: - logger.Error("unknow unsupported event key:", event.Key) + logger.Error("unknown unsupported event key:", event.Key) } } diff --git a/cluster/router/v3router/router_chain_test.go b/cluster/router/v3router/router_chain_test.go index 7293963d1c..f3e1b7b211 100644 --- a/cluster/router/v3router/router_chain_test.go +++ b/cluster/router/v3router/router_chain_test.go @@ -20,6 +20,12 @@ package v3router import ( "fmt" "testing" + + "dubbo.apache.org/dubbo-go/v3/cluster/router/v3router/k8s_api" + "dubbo.apache.org/dubbo-go/v3/config" + "dubbo.apache.org/dubbo-go/v3/config_center" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) import ( @@ -206,3 +212,46 @@ func TestRouterChain_Route(t *testing.T) { assert.Equal(t, 0, len(result)) //todo test find target invoker } + +func TestRouterChain_Process(t *testing.T) { + vsJson := `{"apiVersion":"service.dubbo.apache.org/v1alpha2", "kind":"VirtualService", "name":"demo-route"}` + + rc := &RouterChain{} + mockVirtualServiceConfig := &config.VirtualServiceConfig{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "kubectl.kubernetes.io/last-applied-configuration": vsJson, + }, + }, + } + + // test virtual service config chage event + mockVirtualServiceChangeEvent := &config_center.ConfigChangeEvent{ + Key: k8s_api.VirtualServiceEventKey, + Value: mockVirtualServiceConfig, + ConfigType: 0, + } + rc.Process(mockVirtualServiceChangeEvent) + + // test destination rule config chage event + destJson := `{"apiVersion":"service.dubbo.apache.org/v1alpha2", "kind":"VirtualService", "name":"demo-route"}` + mockDestinationRuleConfig := &config.DestinationRuleConfig{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "kubectl.kubernetes.io/last-applied-configuration": destJson, + }, + }, + } + mockDestinationRuleChangeEvent := &config_center.ConfigChangeEvent{ + Key: k8s_api.DestinationRuleEventKey, + Value: mockDestinationRuleConfig, + ConfigType: 0, + } + rc.Process(mockDestinationRuleChangeEvent) + + // test unknown event type + mockUnsupportedEvent := &config_center.ConfigChangeEvent{ + Key: "unknown", + } + rc.Process(mockUnsupportedEvent) +} diff --git a/config/uniform_router_config.go b/config/uniform_router_config.go index e18c07c082..fa5d02db19 100644 --- a/config/uniform_router_config.go +++ b/config/uniform_router_config.go @@ -25,7 +25,7 @@ import ( // nolint type MetaDataStruct struct { - Name string `yaml:"name"` + Name string `yaml:"name" json:"name"` } // VirtualService Config Definition From 49a28707a4c522c453e84b9441fb4be5b140031b Mon Sep 17 00:00:00 2001 From: Dong Jianhui <978007503@qq.com> Date: Wed, 23 Jun 2021 20:38:38 +0800 Subject: [PATCH 3/4] refactor imports and some code --- .../v3router/judger/attachment_match_judger.go | 6 +----- .../v3router/judger/list_string_match_judger_test.go | 10 ++++++++-- .../v3router/judger/method_match_judger_test.go | 12 +++++++++--- .../v3router/judger/url_label_match_judge_test.go | 10 ++++++++-- cluster/router/v3router/router_chain_test.go | 8 ++++++-- 5 files changed, 32 insertions(+), 14 deletions(-) diff --git a/cluster/router/v3router/judger/attachment_match_judger.go b/cluster/router/v3router/judger/attachment_match_judger.go index 1d215375d4..31f46e05cf 100644 --- a/cluster/router/v3router/judger/attachment_match_judger.go +++ b/cluster/router/v3router/judger/attachment_match_judger.go @@ -33,11 +33,7 @@ func (amj *AttachmentMatchJudger) Judge(invocation protocol.Invocation) bool { return false } - if amj.DubboContext != nil && !judge(amj.DubboContext, invAttaMap) { - return false - } - - return true + return amj.DubboContext == nil || judge(amj.DubboContext, invAttaMap) } func judge(condition map[string]*config.StringMatch, invAttaMap map[string]interface{}) bool { diff --git a/cluster/router/v3router/judger/list_string_match_judger_test.go b/cluster/router/v3router/judger/list_string_match_judger_test.go index 12144cfc19..1816b9503f 100644 --- a/cluster/router/v3router/judger/list_string_match_judger_test.go +++ b/cluster/router/v3router/judger/list_string_match_judger_test.go @@ -18,11 +18,17 @@ package judger import ( - "dubbo.apache.org/dubbo-go/v3/config" - "github.com/stretchr/testify/assert" "testing" ) +import ( + "github.com/stretchr/testify/assert" +) + +import ( + "dubbo.apache.org/dubbo-go/v3/config" +) + func TestListStringMatchJudger(t *testing.T) { assert.True(t, newListStringMatchJudger(&config.ListStringMatch{ Oneof: []*config.StringMatch{{Exact: "abd"}}, diff --git a/cluster/router/v3router/judger/method_match_judger_test.go b/cluster/router/v3router/judger/method_match_judger_test.go index 8e7c72a878..9d361e5f82 100644 --- a/cluster/router/v3router/judger/method_match_judger_test.go +++ b/cluster/router/v3router/judger/method_match_judger_test.go @@ -18,13 +18,19 @@ package judger import ( - "dubbo.apache.org/dubbo-go/v3/config" - "dubbo.apache.org/dubbo-go/v3/protocol/invocation" - "github.com/stretchr/testify/assert" "reflect" "testing" ) +import ( + "github.com/stretchr/testify/assert" +) + +import ( + "dubbo.apache.org/dubbo-go/v3/config" + "dubbo.apache.org/dubbo-go/v3/protocol/invocation" +) + func TestMethodMatchJudger(t *testing.T) { methodArgs := make([]*config.DubboMethodArg, 0) diff --git a/cluster/router/v3router/judger/url_label_match_judge_test.go b/cluster/router/v3router/judger/url_label_match_judge_test.go index eb27b1e5c8..bb4b4679dd 100644 --- a/cluster/router/v3router/judger/url_label_match_judge_test.go +++ b/cluster/router/v3router/judger/url_label_match_judge_test.go @@ -18,11 +18,17 @@ package judger import ( - "dubbo.apache.org/dubbo-go/v3/common" - "github.com/stretchr/testify/assert" "testing" ) +import ( + "github.com/stretchr/testify/assert" +) + +import ( + "dubbo.apache.org/dubbo-go/v3/common" +) + func TestJudgeUrlLabel(t *testing.T) { url := common.NewURLWithOptions(common.WithParamsValue("a", "A")) diff --git a/cluster/router/v3router/router_chain_test.go b/cluster/router/v3router/router_chain_test.go index f3e1b7b211..4ce4eef3b0 100644 --- a/cluster/router/v3router/router_chain_test.go +++ b/cluster/router/v3router/router_chain_test.go @@ -20,12 +20,16 @@ package v3router import ( "fmt" "testing" +) + +import ( + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) +import ( "dubbo.apache.org/dubbo-go/v3/cluster/router/v3router/k8s_api" "dubbo.apache.org/dubbo-go/v3/config" "dubbo.apache.org/dubbo-go/v3/config_center" - - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) import ( From e0007fc893ce8fa0adf607e4723ab3aaa2b03ccd Mon Sep 17 00:00:00 2001 From: Dong Jianhui <978007503@qq.com> Date: Wed, 23 Jun 2021 22:07:32 +0800 Subject: [PATCH 4/4] remove blank lines --- cluster/router/v3router/judger/method_match_judger_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/cluster/router/v3router/judger/method_match_judger_test.go b/cluster/router/v3router/judger/method_match_judger_test.go index 9d361e5f82..71e90aacb5 100644 --- a/cluster/router/v3router/judger/method_match_judger_test.go +++ b/cluster/router/v3router/judger/method_match_judger_test.go @@ -32,7 +32,6 @@ import ( ) func TestMethodMatchJudger(t *testing.T) { - methodArgs := make([]*config.DubboMethodArg, 0) methodArgs = append(methodArgs, &config.DubboMethodArg{ Index: 1,