Merge pull request #7 from baiyutang/main

fix(*): incorrect handling of svc tag

.github/workflows/push-check.yml

@@ -21,7 +21,7 @@ jobs:
             ${{ runner.os }}-go-
 
       - name: Check License Header
-        uses: apache/skywalking-eyes@main
+        uses: apache/skywalking-eyes/[email protected]
         env:
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
 

.licenserc.yaml

@@ -5,5 +5,8 @@ header:
 
   paths:
     - '**/*.go'
+
+  paths-ignore:
+    - example/hello/**
 
   comment: on-failure

Makefile

@@ -1,3 +1,6 @@
 prepare:
 	docker pull consul:latest
-	docker run -d --name=dev-consul -e CONSUL_BIND_INTERFACE=eth0 -p 8500:8500 consul:latest
+	docker run -d --rm --name=dev-consul -e CONSUL_BIND_INTERFACE=eth0 -p 8500:8500 consul:latest
+
+stop:
+	docker stop dev-consul

consul_registry.go

@@ -1,37 +1,44 @@
-// Copyright 2021 CloudWeGo Authors.
-//
-// Licensed 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.
+/*
+ * Copyright 2021 CloudWeGo Authors
+ *
+ * Licensed 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 consul
 
 import (
 	"errors"
 	"fmt"
+	"strings"
 
 	"github.com/cloudwego/kitex/pkg/registry"
 	"github.com/hashicorp/consul/api"
 )
 
+type options struct {
+	check *api.AgentServiceCheck
+}
+
 type consulRegistry struct {
 	consulClient *api.Client
 	opts         options
 }
 
+const kvJoinChar = ":"
+
 var _ registry.Registry = (*consulRegistry)(nil)
 
-type options struct {
-	check *api.AgentServiceCheck
-}
+var errIllegalTagChar = errors.New("illegal tag character")
 
 // Option is consul option.
 type Option func(o *options)
@@ -72,6 +79,7 @@ func NewConsulRegisterWithConfig(config *api.Config) (*consulRegistry, error) {
 }
 
 // Register register a service to consul.
+// Note: the tag map of the service can not contain the `:` character.
 func (c *consulRegistry) Register(info *registry.Info) error {
 	if err := validateRegistryInfo(info); err != nil {
 		return err
@@ -81,16 +89,23 @@ func (c *consulRegistry) Register(info *registry.Info) error {
 	if err != nil {
 		return err
 	}
+
 	svcID, err := getServiceId(info)
 	if err != nil {
 		return err
 	}
+
+	tagSlice, err := convTagMapToSlice(info.Tags)
+	if err != nil {
+		return err
+	}
+
 	svcInfo := &api.AgentServiceRegistration{
 		ID:      svcID,
 		Address: host,
 		Port:    port,
 		Name:    info.ServiceName,
-		Meta:    info.Tags,
+		Tags:    tagSlice,
 		Weights: &api.AgentWeights{
 			Passing: info.Weight,
 			Warning: info.Weight,
@@ -133,3 +148,16 @@ func defaultCheck() *api.AgentServiceCheck {
 
 	return check
 }
+
+// convTagMapToSlice Tags map be convert to slice.
+// Keys must not contain `:`.
+func convTagMapToSlice(tagMap map[string]string) ([]string, error) {
+	svcTags := make([]string, 0, len(tagMap))
+	for k, v := range tagMap {
+		if strings.Contains(k, kvJoinChar) {
+			return svcTags, errIllegalTagChar
+		}
+		svcTags = append(svcTags, fmt.Sprintf("%s%s%s", k, kvJoinChar, v))
+	}
+	return svcTags, nil
+}

consul_resolver.go

@@ -1,24 +1,26 @@
-// Copyright 2021 CloudWeGo Authors.
-//
-// Licensed 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.
+/*
+ * Copyright 2021 CloudWeGo Authors
+ *
+ * Licensed 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 consul
 
 import (
 	"context"
 	"errors"
 	"fmt"
-	"log"
+	"strings"
 
 	"github.com/cloudwego/kitex/pkg/discovery"
 	"github.com/cloudwego/kitex/pkg/rpcinfo"
@@ -67,22 +69,23 @@ func (c *consulResolver) Resolve(_ context.Context, desc string) (discovery.Resu
 	var eps []discovery.Instance
 	agentServiceList, _, err := c.consulClient.Health().Service(desc, "", true, nil)
 	if err != nil {
-		log.Printf("err:%v", err)
 		return discovery.Result{}, err
 	}
 	if len(agentServiceList) == 0 {
 		return discovery.Result{}, errors.New("no service found")
 	}
+
 	for _, i := range agentServiceList {
 		svc := i.Service
 		if svc == nil || svc.Address == "" {
 			continue
 		}
+
 		eps = append(eps, discovery.NewInstance(
 			defaultNetwork,
 			fmt.Sprint(svc.Address, ":", svc.Port),
 			svc.Weights.Passing,
-			svc.Meta,
+			splitTags(svc.Tags),
 		))
 	}
 
@@ -102,3 +105,25 @@ func (c *consulResolver) Diff(cacheKey string, prev, next discovery.Result) (dis
 func (c *consulResolver) Name() string {
 	return "consul"
 }
+
+// splitTags Tags characters be separated to map.
+func splitTags(tags []string) map[string]string {
+	n := len(tags)
+	tagMap := make(map[string]string, n)
+	if n == 0 {
+		return tagMap
+	}
+
+	for _, tag := range tags {
+		if tag == "" {
+			continue
+		}
+		strArr := strings.SplitN(tag, kvJoinChar, 2)
+		if len(strArr) == 2 {
+			key := strArr[0]
+			tagMap[key] = strArr[1]
+		}
+	}
+
+	return tagMap
+}

consul_resolver_test.go

@@ -0,0 +1,62 @@
+/*
+ * Copyright 2021 CloudWeGo Authors
+ *
+ * Licensed 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 consul
+
+import (
+	"reflect"
+	"testing"
+)
+
+func TestSplitTags(t *testing.T) {
+	type args struct {
+		tags []string
+	}
+	tests := []struct {
+		name string
+		args args
+		want map[string]string
+	}{
+		{
+			name: "Regular tags",
+			args: args{
+				tags: []string{"k1:v1", "k2:v2"},
+			},
+			want: map[string]string{"k1": "v1", "k2": "v2"},
+		},
+		{
+			name: "Some tags no values",
+			args: args{
+				tags: []string{"k1:", "k2:v2"},
+			},
+			want: map[string]string{"k1": "", "k2": "v2"},
+		},
+		{
+			name: "Tags char splited,two elements be handled correctly",
+			args: args{
+				tags: []string{"k1:v1:vv1", "k2:v2"},
+			},
+			want: map[string]string{"k1": "v1:vv1", "k2": "v2"},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := splitTags(tt.args.tags); !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("splitTags() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}