little-cui closed pull request #279: SCB-336 1.add ut about find instance with
tag 2.optimize find api
URL: https://github.com/apache/incubator-servicecomb-service-center/pull/279
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/pkg/rest/client.go b/pkg/rest/client.go
index 2cd68abe..f5b6c009 100644
--- a/pkg/rest/client.go
+++ b/pkg/rest/client.go
@@ -229,10 +229,10 @@ func (client *HttpClient) httpDo(method string, url
string, headers map[string]s
var bodyReader io.Reader = nil
if body != nil {
if headers == nil || len(headers["Content-Type"]) == 0 {
- //
????????Conent-Type????json???????????json????????headers??????
+ //
????????Content-Type????json???????????json????????headers??????
bodyBytes, err = json.Marshal(body)
if err != nil {
- util.Logger().Errorf(err, "mashal object
failed.")
+ util.Logger().Errorf(err, "marshal object
failed.")
return status, result
}
} else {
@@ -293,7 +293,7 @@ func (client *HttpClient) HttpDo(method string, url string,
headers map[string]s
//
????????Conent-Type????json???????????json????????headers??????
bodyBytes, err = json.Marshal(body)
if err != nil {
- util.Logger().Errorf(err, "mashal object
failed.")
+ util.Logger().Errorf(err, "marshal object
failed.")
return nil, err
}
} else {
diff --git a/server/service/instances.go b/server/service/instances.go
index a4529672..19f77a69 100644
--- a/server/service/instances.go
+++ b/server/service/instances.go
@@ -567,13 +567,12 @@ func (s *InstanceService) Find(ctx context.Context, in
*pb.FindInstancesRequest)
if apt.IsShared(provider) {
// it means the shared micro-services must be the same env with
SC.
provider.Environment = apt.Service.Environment
- findFlag += "(shared services in " + provider.Environment + "
environment)"
+ findFlag += "(provider is shared service in " +
provider.Environment + " environment)"
} else {
// provider is not a shared micro-service,
// only allow shared micro-service instances found in different
domains.
util.SetTargetDomainProject(ctx, util.ParseDomain(ctx),
util.ParseProject(ctx))
provider.Tenant = util.ParseTargetDomainProject(ctx)
- findFlag += "('" + provider.Environment + "' services of the
same domain)"
}
// ????
@@ -585,7 +584,7 @@ func (s *InstanceService) Find(ctx context.Context, in
*pb.FindInstancesRequest)
}, err
}
if len(ids) == 0 {
- mes := fmt.Sprintf("no provider matched, %s", findFlag)
+ mes := fmt.Sprintf("provider not exist, %s", findFlag)
util.Logger().Errorf(nil, "find instance failed, %s", mes)
return &pb.FindInstancesResponse{
Response: pb.CreateResponse(scerr.ErrServiceNotExists,
mes),
@@ -617,7 +616,7 @@ func (s *InstanceService) Find(ctx context.Context, in
*pb.FindInstancesRequest)
//??version???,service name ????????????
providerService, err := serviceUtil.GetService(ctx, provider.Tenant,
ids[0])
if providerService == nil {
- util.Logger().Errorf(err, "find instance failed, %s: no
provider matched.", findFlag)
+ util.Logger().Errorf(err, "find instance failed, %s: provider
%s not exist.", findFlag, ids[0])
return &pb.FindInstancesResponse{
Response: pb.CreateResponse(scerr.ErrServiceNotExists,
"No provider matched."),
}, nil
diff --git a/server/service/tag_test.go b/server/service/tag_test.go
index 7a85869e..267635cd 100644
--- a/server/service/tag_test.go
+++ b/server/service/tag_test.go
@@ -191,7 +191,7 @@ var _ = Describe("'Tag' service", func() {
})
- Describe("execute 'update' operartion", func() {
+ Describe("execute 'update' operation", func() {
var (
serviceId string
)
@@ -283,9 +283,123 @@ var _ = Describe("'Tag' service", func() {
})
+ Context("find instance, contain tag", func() {
+ It("should pass", func() {
+ By("create consumer")
+ resp, err :=
serviceResource.Create(getContext(), &pb.CreateServiceRequest{
+ Service: &pb.MicroService{
+ AppId:
"find_inst_tag_group",
+ ServiceName:
"find_inst_tag_consumer",
+ Version: "1.0.0",
+ Level: "FRONT",
+ Status: pb.MS_UP,
+ },
+ })
+ Expect(err).To(BeNil())
+
Expect(resp.Response.Code).To(Equal(pb.Response_SUCCESS))
+ consumerId := resp.ServiceId
+
+ By("create provider")
+ resp, err =
serviceResource.Create(getContext(), &pb.CreateServiceRequest{
+ Service: &pb.MicroService{
+ AppId:
"find_inst_tag_group",
+ ServiceName:
"find_inst_tag_provider",
+ Version: "1.0.1",
+ Level: "FRONT",
+ Status: pb.MS_UP,
+ },
+ })
+ Expect(err).To(BeNil())
+
Expect(resp.Response.Code).To(Equal(pb.Response_SUCCESS))
+ providerId := resp.ServiceId
+
+ addTagResp, err :=
serviceResource.AddTags(getContext(), &pb.AddServiceTagsRequest{
+ ServiceId: providerId,
+ Tags:
map[string]string{"filter_tag": "filter"},
+ })
+ Expect(err).To(BeNil())
+
Expect(addTagResp.Response.Code).To(Equal(pb.Response_SUCCESS))
+
+ instanceResp, err :=
instanceResource.Register(getContext(), &pb.RegisterInstanceRequest{
+ Instance: &pb.MicroServiceInstance{
+ ServiceId: providerId,
+ Endpoints: []string{
+
"findInstanceForTagFilter:127.0.0.1:8080",
+ },
+ HostName: "UT-HOST",
+ Status: pb.MSI_UP,
+ },
+ })
+ Expect(err).To(BeNil())
+
Expect(instanceResp.Response.Code).To(Equal(pb.Response_SUCCESS))
+
+ findResp, err :=
instanceResource.Find(getContext(), &pb.FindInstancesRequest{
+ ConsumerServiceId: consumerId,
+ AppId: "find_inst_tag_group",
+ ServiceName: "find_inst_tag_provider",
+ VersionRule: "1.0.0+",
+ Tags: []string{"not-exist-tag"},
+ })
+
Expect(findResp.Response.Code).To(Equal(pb.Response_SUCCESS))
+ Expect(len(findResp.Instances)).To(Equal(0))
+
+ findResp, err =
instanceResource.Find(getContext(), &pb.FindInstancesRequest{
+ ConsumerServiceId: consumerId,
+ AppId: "find_inst_tag_group",
+ ServiceName: "find_inst_tag_provider",
+ VersionRule: "1.0.0+",
+ Tags: []string{"filter_tag"},
+ })
+ Expect(err).To(BeNil())
+
Expect(findResp.Response.Code).To(Equal(pb.Response_SUCCESS))
+
Expect(findResp.Instances[0].InstanceId).To(Equal(instanceResp.InstanceId))
+
+ respAddRule, err :=
serviceResource.AddRule(getContext(), &pb.AddServiceRulesRequest{
+ ServiceId: providerId,
+ Rules: []*pb.AddOrUpdateServiceRule{
+ &pb.AddOrUpdateServiceRule{
+ RuleType: "WHITE",
+ Attribute:
"tag_filter_tag",
+ Pattern: "f*",
+ Description: "test
white",
+ },
+ },
+ })
+ Expect(err).To(BeNil())
+
Expect(respAddRule.Response.Code).To(Equal(pb.Response_SUCCESS))
+
+ findResp, err =
instanceResource.Find(getContext(), &pb.FindInstancesRequest{
+ ConsumerServiceId: consumerId,
+ AppId: "find_inst_tag_group",
+ ServiceName: "find_inst_tag_provider",
+ VersionRule: "1.0.0+",
+ Tags: []string{"filter_tag"},
+ })
+
Expect(findResp.Response.Code).To(Equal(pb.Response_SUCCESS))
+ Expect(len(findResp.Instances)).To(Equal(0))
+
+ addTagResp, err =
serviceResource.AddTags(getContext(), &pb.AddServiceTagsRequest{
+ ServiceId: consumerId,
+ Tags:
map[string]string{"filter_tag": "filter"},
+ })
+ Expect(err).To(BeNil())
+
Expect(addTagResp.Response.Code).To(Equal(pb.Response_SUCCESS))
+
+ findResp, err =
instanceResource.Find(getContext(), &pb.FindInstancesRequest{
+ ConsumerServiceId: consumerId,
+ AppId: "find_inst_tag_group",
+ ServiceName: "find_inst_tag_provider",
+ VersionRule: "1.0.0+",
+ Tags: []string{"filter_tag"},
+ })
+
Expect(findResp.Response.Code).To(Equal(pb.Response_SUCCESS))
+
Expect(findResp.Instances[0].InstanceId).To(Equal(instanceResp.InstanceId))
+ })
+ })
+
})
- Describe("execute 'delete' operartion", func() {
+ Describe("execute 'delete' operation", func() {
var (
serviceId string
)
diff --git a/server/service/util/rule_util.go b/server/service/util/rule_util.go
index b8b07744..f7a3dee1 100644
--- a/server/service/util/rule_util.go
+++ b/server/service/util/rule_util.go
@@ -31,12 +31,6 @@ import (
"strings"
)
-var tagRegEx *regexp.Regexp
-
-func init() {
- tagRegEx, _ = regexp.Compile("tag_(.*)")
-}
-
type RuleFilter struct {
DomainProject string
Provider *pb.MicroService
@@ -162,54 +156,84 @@ func AllowAcrossDimension(ctx context.Context,
providerService *pb.MicroService,
return nil
}
-func MatchRules(rules []*pb.ServiceRule, service *pb.MicroService, serviceTags
map[string]string) *scerr.Error {
- if service == nil {
- return scerr.NewError(scerr.ErrInvalidParams, "service is nil")
+func MatchRules(rulesOfProvider []*pb.ServiceRule, consumer *pb.MicroService,
tagsOfConsumer map[string]string) *scerr.Error {
+ if consumer == nil {
+ return scerr.NewError(scerr.ErrInvalidParams, "consumer is nil")
}
- v := reflect.Indirect(reflect.ValueOf(service))
+ isWhite := false
+ if len(rulesOfProvider) <= 0 {
+ return nil
+ }
+ if rulesOfProvider[0].RuleType == "WHITE" {
+ isWhite = true
+ }
+ if isWhite {
+ return patternWhiteList(rulesOfProvider, tagsOfConsumer,
consumer)
+ }
+ return patternBlackList(rulesOfProvider, tagsOfConsumer, consumer)
+}
- hasWhite := false
- for _, rule := range rules {
- var value string
- if tagRegEx.MatchString(rule.Attribute) {
- key := tagRegEx.FindStringSubmatch(rule.Attribute)[1]
- value = serviceTags[key]
- if len(value) == 0 {
- util.Logger().Infof("can not find service %s
tag '%s'", service.ServiceId, key)
- continue
- }
- } else {
- key := v.FieldByName(rule.Attribute)
- if !key.IsValid() {
- util.Logger().Errorf(nil, "can not find service
%s field '%s', rule %s",
- service.ServiceId, rule.Attribute,
rule.RuleId)
- return scerr.NewError(scerr.ErrInternal,
fmt.Sprintf("Can not find field '%s'", rule.Attribute))
- }
- value = key.String()
+func patternWhiteList(rulesOfProvider []*pb.ServiceRule, tagsOfConsumer
map[string]string, consumer *pb.MicroService) *scerr.Error {
+ v := reflect.Indirect(reflect.ValueOf(consumer))
+ consumerId := consumer.ServiceId
+ for _, rule := range rulesOfProvider {
+ value, err := parsePattern(v, rule, tagsOfConsumer, consumerId)
+ if err != nil {
+ return err
+ }
+ if len(value) == 0 {
+ continue
+ }
+
+ match, _ := regexp.MatchString(rule.Pattern, value)
+ if match {
+ util.Logger().Infof("consumer %s match white list,
rule.Pattern is %s, value is %s",
+ consumerId, rule.Pattern, value)
+ return nil
}
+ }
+ return scerr.NewError(scerr.ErrPermissionDeny, "Not found in white
list")
+}
- switch rule.RuleType {
- case "WHITE":
- hasWhite = true
- match, _ := regexp.MatchString(rule.Pattern, value)
- if match {
- util.Logger().Infof("service %s match white
list, rule.Pattern is %s, value is %s",
- service.ServiceId, rule.Pattern, value)
- return nil
- }
- case "BLACK":
- match, _ := regexp.MatchString(rule.Pattern, value)
- if match {
- util.Logger().Infof("service %s match black
list, rule.Pattern is %s, value is %s",
- service.ServiceId, rule.Pattern, value)
- return scerr.NewError(scerr.ErrPermissionDeny,
"Found in black list")
- }
+func parsePattern(v reflect.Value, rule *pb.ServiceRule, tagsOfConsumer
map[string]string, consumerId string) (string, *scerr.Error) {
+ if strings.HasPrefix(rule.Attribute, "tag_") {
+ key := rule.Attribute[4:]
+ value := tagsOfConsumer[key]
+ if len(value) == 0 {
+ util.Logger().Infof("can not find service %s tag '%s'",
consumerId, key)
}
+ return value, nil
+ }
+ key := v.FieldByName(rule.Attribute)
+ if !key.IsValid() {
+ util.Logger().Errorf(nil, "can not find service %s field '%s',
rule %s",
+ consumerId, rule.Attribute, rule.RuleId)
+ return "", scerr.NewError(scerr.ErrInternal, fmt.Sprintf("Can
not find field '%s'", rule.Attribute))
}
- if hasWhite {
- util.Logger().Infof("service %s do not match white list",
service.ServiceId)
- return scerr.NewError(scerr.ErrPermissionDeny, "Not found in
white list")
+ return key.String(), nil
+
+}
+
+func patternBlackList(rulesOfProvider []*pb.ServiceRule, tagsOfConsumer
map[string]string, consumer *pb.MicroService) *scerr.Error {
+ v := reflect.Indirect(reflect.ValueOf(consumer))
+ consumerId := consumer.ServiceId
+ for _, rule := range rulesOfProvider {
+ var value string
+ value, err := parsePattern(v, rule, tagsOfConsumer, consumerId)
+ if err != nil {
+ return err
+ }
+ if len(value) == 0 {
+ continue
+ }
+
+ match, _ := regexp.MatchString(rule.Pattern, value)
+ if match {
+ util.Logger().Infof("no permission to access, consumer
%s match black list, rule.Pattern is %s, value is %s",
+ consumerId, rule.Pattern, value)
+ return scerr.NewError(scerr.ErrPermissionDeny, "Found
in black list")
+ }
}
return nil
}
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
