fix(trait):define the default trait name

Make the default trait name not the same as the component name

Fixes crossplane#66

Signed-off-by: zhuhuijun <[email protected]>

go.mod

@@ -4,6 +4,7 @@ go 1.13
 
 require (
 	github.com/crossplane/crossplane-runtime v0.8.0
+	github.com/davecgh/go-spew v1.1.1
 	github.com/gertd/go-pluralize v0.1.7
 	github.com/go-logr/logr v0.1.0
 	github.com/google/go-cmp v0.4.0
@@ -13,6 +14,7 @@ require (
 	github.com/rs/xid v1.2.1
 	github.com/stretchr/testify v1.4.0
 	golang.org/x/tools v0.0.0-20200630223951-c138986dd9b9 // indirect
+	gopkg.in/go-playground/assert.v1 v1.2.1 // indirect
 	k8s.io/api v0.18.5
 	k8s.io/apiextensions-apiserver v0.18.2
 	k8s.io/apimachinery v0.18.5

go.sum

@@ -570,6 +570,8 @@ gopkg.in/cheggaaa/pb.v1 v1.0.25/go.mod h1:V/YB90LKu/1FcN3WVnfiiE5oMCibMjukxqG/qS
 gopkg.in/cheggaaa/pb.v1 v1.0.27/go.mod h1:V/YB90LKu/1FcN3WVnfiiE5oMCibMjukxqG/qStrOgw=
 gopkg.in/fsnotify.v1 v1.4.7 h1:xOHLXZwVvI9hhs+cLKq5+I5onOuwQLhQwiu63xxlHs4=
 gopkg.in/fsnotify.v1 v1.4.7/go.mod h1:Tz8NjZHkW78fSQdbUxIjBTcgA1z1m8ZHf0WmKUhAMys=
+gopkg.in/go-playground/assert.v1 v1.2.1 h1:xoYuJVE7KT85PYWrN730RguIQO0ePzVRfFMXadIrXTM=
+gopkg.in/go-playground/assert.v1 v1.2.1/go.mod h1:9RXL0bg/zibRAgZUYszZSwO/z8Y/a8bDuhia5mkpMnE=
 gopkg.in/inf.v0 v0.9.0/go.mod h1:cWUDdTG/fYaXco+Dcufb5Vnc6Gp2YChqWtbxRZE0mXw=
 gopkg.in/inf.v0 v0.9.1 h1:73M5CoZyi3ZLMOyDlQh031Cx6N9NDJ2Vvfl76EDAgDc=
 gopkg.in/inf.v0 v0.9.1/go.mod h1:cWUDdTG/fYaXco+Dcufb5Vnc6Gp2YChqWtbxRZE0mXw=

pkg/controller/v1alpha2/applicationconfiguration/render.go

@@ -165,7 +165,7 @@ func (r *components) renderComponent(ctx context.Context, acc v1alpha2.Applicati
 }
 
 func (r *components) renderTrait(ctx context.Context, ct v1alpha2.ComponentTrait, namespace, componentName string, ref *metav1.OwnerReference, dag *dependency.DAG) (*unstructured.Unstructured, *v1alpha2.TraitDefinition, error) {
-	traitName := util.GenTraitName(componentName)
+	traitName := util.GenTraitName(componentName, ct.DeepCopy(), nil)
 	t, err := r.trait.Render(ct.Trait.Raw)
 	if err != nil {
 		return nil, nil, errors.Wrapf(err, errFmtRenderTrait, traitName)

pkg/controller/v1alpha2/applicationconfiguration/render_test.go

@@ -173,7 +173,7 @@ func TestRenderComponents(t *testing.T) {
 			},
 			args: args{ac: ac},
 			want: want{
-				err: errors.Wrapf(errBoom, errFmtRenderTrait, componentName),
+				err: errors.Wrapf(errBoom, errFmtRenderTrait, util.GenTraitName(componentName, ac.Spec.Components[0].Traits[0].DeepCopy(), nil)),
 			},
 		},
 		"Success": {
@@ -670,7 +670,7 @@ func TestRenderTraitWithoutMetadataName(t *testing.T) {
 		t.Run(name, func(t *testing.T) {
 			r := &components{tc.fields.client, nil, tc.fields.params, tc.fields.workload, tc.fields.trait}
 			got, _ := r.Render(tc.args.ctx, tc.args.ac)
-			if len(got) == 0 || len(got[0].Traits) == 0 || got[0].Traits[0].GetName() != componentName {
+			if len(got) == 0 || len(got[0].Traits) == 0 || got[0].Traits[0].GetName() != util.GenTraitName(componentName, ac.Spec.Components[0].Traits[0].DeepCopy(), nil) {
 				t.Errorf("\n%s\nr.Render(...): -want error, +got error:\n%s\n", tc.reason, "Trait name is NOT"+
 					"automatically set.")
 			}

pkg/oam/util/helper.go

@@ -2,12 +2,17 @@ package util
 
 import (
 	"context"
+	"encoding/binary"
 	"encoding/json"
 	"fmt"
+	"hash"
+	"hash/fnv"
 	"reflect"
 	"strings"
 	"time"
 
+	"github.com/davecgh/go-spew/spew"
+
 	cpv1alpha1 "github.com/crossplane/crossplane-runtime/apis/core/v1alpha1"
 	plur "github.com/gertd/go-pluralize"
 	"github.com/go-logr/logr"
@@ -16,6 +21,7 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/apimachinery/pkg/util/rand"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
@@ -204,6 +210,37 @@ func Object2Map(obj interface{}) (map[string]interface{}, error) {
 }
 
 // GenTraitName generate trait name
-func GenTraitName(componentName string) string {
-	return fmt.Sprintf("%s-%s", TraitPrefixKey, componentName)
+func GenTraitName(componentName string, ct *v1alpha2.ComponentTrait, collisionCount *int32) string {
+	return fmt.Sprintf("%s-%s-%s", componentName, TraitPrefixKey, ComputeHash(ct, collisionCount))
+}
+
+// ComputeHash returns a hash value calculated from pod template and
+// a collisionCount to avoid hash collision. The hash will be safe encoded to
+// avoid bad words.
+func ComputeHash(trait *v1alpha2.ComponentTrait, collisionCount *int32) string {
+	componentTraitHasher := fnv.New32a()
+	DeepHashObject(componentTraitHasher, *trait)
+
+	// Add collisionCount in the hash if it exists.
+	if collisionCount != nil {
+		collisionCountBytes := make([]byte, 8)
+		binary.LittleEndian.PutUint32(collisionCountBytes, uint32(*collisionCount))
+		_, _ = componentTraitHasher.Write(collisionCountBytes)
+	}
+
+	return rand.SafeEncodeString(fmt.Sprint(componentTraitHasher.Sum32()))
+}
+
+// DeepHashObject writes specified object to hash using the spew library
+// which follows pointers and prints actual values of the nested objects
+// ensuring the hash does not change when a pointer changes.
+func DeepHashObject(hasher hash.Hash, objectToWrite interface{}) {
+	hasher.Reset()
+	printer := spew.ConfigState{
+		Indent:         " ",
+		SortKeys:       true,
+		DisableMethods: true,
+		SpewKeys:       true,
+	}
+	_, _ = printer.Fprintf(hasher, "%#v", objectToWrite)
 }

pkg/oam/util/helper_test.go

@@ -3,6 +3,7 @@ package util_test
 import (
 	"context"
 	"fmt"
+	"math"
 
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
@@ -292,3 +293,61 @@ var _ = Describe("Test unstructured related helper utils", func() {
 		}
 	})
 })
+
+var _ = Describe("Test GenTraitName helper utils", func() {
+	It("Test generate trait name", func() {
+
+		collisionCount := int32(1)
+		otherCollisionCount := int32(2)
+		maxCollisionCount := int32(math.MaxInt32)
+
+		tests := []struct {
+			name                string
+			template            *v1alpha2.ComponentTrait
+			collisionCount      *int32
+			otherCollisionCount *int32
+		}{
+			{
+				name:                "simple",
+				template:            &v1alpha2.ComponentTrait{},
+				collisionCount:      &collisionCount,
+				otherCollisionCount: &otherCollisionCount,
+			},
+			{
+				name:                "using math.MaxInt64",
+				template:            &v1alpha2.ComponentTrait{},
+				collisionCount:      nil,
+				otherCollisionCount: &maxCollisionCount,
+			},
+		}
+
+		for _, test := range tests {
+			hash := util.ComputeHash(test.template, test.collisionCount)
+			otherHash := util.ComputeHash(test.template, test.otherCollisionCount)
+			Expect(hash).ShouldNot(Equal(otherHash))
+		}
+
+		test2s := []struct {
+			name                string
+			template            *v1alpha2.ComponentTrait
+			collisionCount      *int32
+			otherCollisionCount *int32
+		}{
+			{
+				name:                "simple",
+				template:            &v1alpha2.ComponentTrait{},
+				collisionCount:      &collisionCount,
+				otherCollisionCount: &collisionCount,
+			},
+		}
+		for _, test := range test2s {
+			hash := util.ComputeHash(test.template, test.collisionCount)
+			name := fmt.Sprintf("%s-%s-%s", test.name, util.TraitPrefixKey, util.ComputeHash(test.template, &collisionCount))
+			otherHash := util.ComputeHash(test.template, test.otherCollisionCount)
+			Expect(hash).Should(Equal(otherHash))
+			traitName := util.GenTraitName(test.name, test.template, test.collisionCount)
+			Expect(traitName).Should(Equal(name))
+		}
+
+	})
+})