Simplified virtual node name users need to define in the CRDs

Dockerfile

@@ -1,4 +1,4 @@
-FROM golang:1.11.5-stretch as builder
+FROM golang:1.12-stretch as builder
 WORKDIR /go/src/github.com/aws/aws-app-mesh-controller-for-k8s
 
 # Force the go compiler to use modules.

docs/design.md

@@ -34,6 +34,55 @@ When you want to add an application to their service mesh, create a VirtualNode
 
 Next (though order is not enforced), you can create a VirtualService custom resource, which contains route configuration, allowing requests from one application in the mesh to be routed to a number of virtual nodes that make up a service.  In App Mesh, this results in the creation of a virtual service, virtual router, and one or more routes.
 
-The controller is meant to be used with aws-app-mesh-inject, a webhook which performs envoy sidecar injection, to reduce the amount of manual configuration that you have to perform. The webhook will inject the sidecar into any namespace that is labelled with `appmesh.k8s.aws/sidecarInjectorWebhook=enabled`. See the [aws-app-mesh-inject](https://github.com/aws/aws-app-mesh-inject) repository for more details.
-
-Each virtual node uses either DNS or Cloud Map service discovery.  If DNS is being used for a given virtual node, then the hostname provided must resolve.  The easiest way to configure that is to create a [service](https://kubernetes.io/docs/concepts/services-networking/service/) that corresponds to the virtual node.  
+It is highly recommended, though not required, to use the controller together with aws-app-mesh-inject, a webhook which performs envoy sidecar injection, to reduce the amount of manual configuration that you have to perform. The webhook will inject the sidecar into any namespace that is labelled with `appmesh.k8s.aws/sidecarInjectorWebhook=enabled`. See the [aws-app-mesh-inject](https://github.com/aws/aws-app-mesh-inject) repository for more details.
+
+Each virtual node uses either DNS or Cloud Map service discovery.  If DNS is being used for a given virtual node, then the hostname provided must resolve.  The easiest way to configure that is to create a [service](https://kubernetes.io/docs/concepts/services-networking/service/) that corresponds to the virtual node.  
+
+## Virtual node naming convention
+To support the use case of using the same application as virtual node across namespaces, the controller creates the virtual node name in AppMesh backend by appending the name of the `VirtualNode` CRD with the namespace. The naming format is "VirtualNodeName-Namespace" 
+
+For example, in the VirtualNode definition below, 
+```
+kind: VirtualNode
+metadata:
+  name: colorteller-black
+  namespace: appmesh-demo
+
+```
+the virtual node name created in AppMesh backend will be "colorteller-black-appmesh-demo".
+
+However, if the VirtualNode name in the CRD contains `.`, i.e. `foo.bar`, the controller will assume namespace is `bar` by kubernetes convention. It will replace all `.` with `-` when creating the AppMesh virtual node because `.` isn't a valid character in AppMesh name.  
+
+For example, in the VirtualNode definition below, 
+```
+kind: VirtualNode
+metadata:
+  name: colorteller-black.appmesh-demo
+  namespace: appmesh-demo
+```
+the virtual node name created in AppMesh backend will be "colorteller-black-appmesh-demo".
+
+Similarly, any references to VirtualNode name in other CRDs follow the same naming convention. 
+
+For example, in the example below, 
+
+```
+kind: VirtualService
+metadata:
+  name: colorteller.appmesh-demo.svc.cluster.local
+  namespace: appmesh-demo
+spec:
+  routes:
+  - http:
+      action:
+        weightedTargets:
+        - virtualNodeName: colorteller.appmesh-demo
+          weight: 0
+        - virtualNodeName: colorteller-blue
+          weight: 3
+        - virtualNodeName: colorteller-black.appmesh-demo
+          weight: 3
+```
+the corresponding virtual node names in the AppMesh backend are, `colorteller-appmesh-demo`, `colorteller-blue-appmesh-demo`, and `colorteller-black-appmesh-demo` respectively. 
+
+Check out the [example](docs/example.md) application for more details. 

examples/color.yaml

@@ -17,7 +17,7 @@ spec:
 apiVersion: appmesh.k8s.aws/v1alpha1
 kind: VirtualNode
 metadata:
-  name: colorgateway-appmesh-demo
+  name: colorgateway
   namespace: appmesh-demo
 spec:
   meshName: color-mesh
@@ -35,7 +35,7 @@ spec:
 apiVersion: appmesh.k8s.aws/v1alpha1
 kind: VirtualNode
 metadata:
-  name: colorteller-appmesh-demo
+  name: colorteller
   namespace: appmesh-demo
 spec:
   meshName: color-mesh
@@ -50,7 +50,7 @@ spec:
 apiVersion: appmesh.k8s.aws/v1alpha1
 kind: VirtualNode
 metadata:
-  name: colorteller-black-appmesh-demo
+  name: colorteller-black
   namespace: appmesh-demo
 spec:
   meshName: color-mesh
@@ -65,7 +65,7 @@ spec:
 apiVersion: appmesh.k8s.aws/v1alpha1
 kind: VirtualNode
 metadata:
-  name: colorteller-blue-appmesh-demo
+  name: colorteller-blue
   namespace: appmesh-demo
 spec:
   meshName: color-mesh
@@ -80,7 +80,7 @@ spec:
 apiVersion: appmesh.k8s.aws/v1alpha1
 kind: VirtualNode
 metadata:
-  name: colorteller-red-appmesh-demo
+  name: colorteller-red
   namespace: appmesh-demo
 spec:
   meshName: color-mesh
@@ -108,11 +108,11 @@ spec:
           prefix: /
         action:
           weightedTargets:
-            - virtualNodeName: colorteller-appmesh-demo
+            - virtualNodeName: colorteller.appmesh-demo
               weight: 1
-            - virtualNodeName: colorteller-blue-appmesh-demo
+            - virtualNodeName: colorteller-blue
               weight: 1
-            - virtualNodeName: colorteller-black-appmesh-demo
+            - virtualNodeName: colorteller-black.appmesh-demo
               weight: 1
 ---
 apiVersion: appmesh.k8s.aws/v1alpha1
@@ -131,7 +131,7 @@ spec:
           prefix: /color
         action:
           weightedTargets:
-            - virtualNodeName: colorgateway-appmesh-demo
+            - virtualNodeName: colorgateway
               weight: 1
 ---
 apiVersion: v1

go.mod

@@ -2,7 +2,7 @@ module github.com/aws/aws-app-mesh-controller-for-k8s
 
 require (
 	github.com/BurntSushi/toml v0.3.1 // indirect
-	github.com/aws/aws-sdk-go v1.17.13
+	github.com/aws/aws-sdk-go v1.19.0
 	github.com/deckarep/golang-set v1.7.1
 	github.com/ghodss/yaml v1.0.0 // indirect
 	github.com/gogo/protobuf v1.2.1 // indirect

go.sum

@@ -4,6 +4,8 @@ github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03
 github.com/armon/consul-api v0.0.0-20180202201655-eb2c6b5be1b6/go.mod h1:grANhF5doyWs3UAsr3K4I6qtAmlQcZDesFNEHPZAzj8=
 github.com/aws/aws-sdk-go v1.17.13 h1:MJZwZoDI/UGp/6l0+cfZOQlWCqKmrHTuewhxfd5uQeE=
 github.com/aws/aws-sdk-go v1.17.13/go.mod h1:KmX6BPdI08NWTb3/sm4ZGu5ShLoqVDhKgpiN924inxo=
+github.com/aws/aws-sdk-go v1.19.0 h1:3d9Htr/dl/+8xJYx/fpjEifvfpabZB1YUu61i/WX87Q=
+github.com/aws/aws-sdk-go v1.19.0/go.mod h1:KmX6BPdI08NWTb3/sm4ZGu5ShLoqVDhKgpiN924inxo=
 github.com/beorn7/perks v0.0.0-20180321164747-3a771d992973 h1:xJ4a3vCFaGF/jqvzLMYoU8P317H5OQ+Via4RmuPwCS0=
 github.com/beorn7/perks v0.0.0-20180321164747-3a771d992973/go.mod h1:Dwedo/Wpr24TaqPxmxbtue+5NUziq4I4S80YR8gNf3Q=
 github.com/coreos/etcd v3.3.10+incompatible/go.mod h1:uF7uidLiAD3TWHmW31ZFd/JWoc32PjwdhPthX9715RE=

pkg/aws/appmesh.go

@@ -7,6 +7,7 @@ import (
 	"time"
 
 	appmeshv1alpha1 "github.com/aws/aws-app-mesh-controller-for-k8s/pkg/apis/appmesh/v1alpha1"
+
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/service/appmesh"
 	set "github.com/deckarep/golang-set"
@@ -39,10 +40,10 @@ type AppMeshAPI interface {
 	GetMesh(context.Context, string) (*Mesh, error)
 	CreateMesh(context.Context, *appmeshv1alpha1.Mesh) (*Mesh, error)
 	DeleteMesh(context.Context, string) (*Mesh, error)
-	GetVirtualNode(context.Context, string, string) (*VirtualNode, error)
+	GetVirtualNode(context.Context, string, string, string) (*VirtualNode, error)
 	CreateVirtualNode(context.Context, *appmeshv1alpha1.VirtualNode) (*VirtualNode, error)
 	UpdateVirtualNode(context.Context, *appmeshv1alpha1.VirtualNode) (*VirtualNode, error)
-	DeleteVirtualNode(context.Context, string, string) (*VirtualNode, error)
+	DeleteVirtualNode(context.Context, string, string, string) (*VirtualNode, error)
 	GetVirtualService(context.Context, string, string) (*VirtualService, error)
 	CreateVirtualService(context.Context, *appmeshv1alpha1.VirtualService) (*VirtualService, error)
 	UpdateVirtualService(context.Context, *appmeshv1alpha1.VirtualService) (*VirtualService, error)
@@ -51,8 +52,8 @@ type AppMeshAPI interface {
 	CreateVirtualRouter(context.Context, *appmeshv1alpha1.VirtualRouter, string) (*VirtualRouter, error)
 	DeleteVirtualRouter(context.Context, string, string) (*VirtualRouter, error)
 	GetRoute(context.Context, string, string, string) (*Route, error)
-	CreateRoute(context.Context, *appmeshv1alpha1.Route, string, string) (*Route, error)
-	UpdateRoute(context.Context, *appmeshv1alpha1.Route, string, string) (*Route, error)
+	CreateRoute(context.Context, *appmeshv1alpha1.Route, string, string, string) (*Route, error)
+	UpdateRoute(context.Context, *appmeshv1alpha1.Route, string, string, string) (*Route, error)
 	GetRoutesForVirtualRouter(context.Context, string, string) (Routes, error)
 	DeleteRoute(context.Context, string, string, string) (*Route, error)
 }
@@ -208,13 +209,13 @@ func (v *VirtualNode) BackendsSet() set.Set {
 }
 
 // CreateVirtualNode calls describe virtual node.
-func (c *Cloud) GetVirtualNode(ctx context.Context, name string, meshName string) (*VirtualNode, error) {
+func (c *Cloud) GetVirtualNode(ctx context.Context, name string, namespace string, meshName string) (*VirtualNode, error) {
 	ctx, cancel := context.WithTimeout(ctx, time.Second*DescribeVirtualNodeTimeout)
 	defer cancel()
 
 	input := &appmesh.DescribeVirtualNodeInput{
 		MeshName:        aws.String(meshName),
-		VirtualNodeName: aws.String(name),
+		VirtualNodeName: aws.String(ConstructAppMeshVNodeNameFromCRD(name, namespace)),
 	}
 
 	if output, err := c.appmesh.DescribeVirtualNodeWithContext(ctx, input); err != nil {
@@ -235,7 +236,7 @@ func (c *Cloud) CreateVirtualNode(ctx context.Context, vnode *appmeshv1alpha1.Vi
 	defer cancel()
 
 	input := &appmesh.CreateVirtualNodeInput{
-		VirtualNodeName: aws.String(vnode.Name),
+		VirtualNodeName: aws.String(ConstructAppMeshVNodeNameFromCRD(vnode.Name, vnode.Namespace)),
 		MeshName:        aws.String(vnode.Spec.MeshName),
 		Spec:            &appmesh.VirtualNodeSpec{},
 	}
@@ -276,7 +277,7 @@ func (c *Cloud) CreateVirtualNode(ctx context.Context, vnode *appmeshv1alpha1.Vi
 		} else if vnode.Spec.ServiceDiscovery.CloudMap != nil {
 			// TODO(nic) add CloudMap Service Discovery when SDK supports it
 		} else {
-			klog.Warning("No service discovery set for virtual node %s", vnode.Name)
+			klog.Warningf("No service discovery set for virtual node %s", vnode.Name)
 		}
 	}
 
@@ -298,7 +299,7 @@ func (c *Cloud) UpdateVirtualNode(ctx context.Context, vnode *appmeshv1alpha1.Vi
 	defer cancel()
 
 	input := &appmesh.UpdateVirtualNodeInput{
-		VirtualNodeName: aws.String(vnode.Name),
+		VirtualNodeName: aws.String(ConstructAppMeshVNodeNameFromCRD(vnode.Name, vnode.Namespace)),
 		MeshName:        aws.String(vnode.Spec.MeshName),
 		Spec:            &appmesh.VirtualNodeSpec{},
 	}
@@ -339,7 +340,7 @@ func (c *Cloud) UpdateVirtualNode(ctx context.Context, vnode *appmeshv1alpha1.Vi
 		} else if vnode.Spec.ServiceDiscovery.CloudMap != nil {
 			// TODO(nic) add CloudMap Service Discovery when SDK supports it
 		} else {
-			klog.Warning("No service discovery set for virtual node %s", vnode.Name)
+			klog.Warningf("No service discovery set for virtual node %s", vnode.Name)
 		}
 	}
 
@@ -354,13 +355,13 @@ func (c *Cloud) UpdateVirtualNode(ctx context.Context, vnode *appmeshv1alpha1.Vi
 	}
 }
 
-func (c *Cloud) DeleteVirtualNode(ctx context.Context, name string, meshName string) (*VirtualNode, error) {
+func (c *Cloud) DeleteVirtualNode(ctx context.Context, name string, namespace string, meshName string) (*VirtualNode, error) {
 	ctx, cancel := context.WithTimeout(ctx, time.Second*DeleteVirtualNodeTimeout)
 	defer cancel()
 
 	input := &appmesh.DeleteVirtualNodeInput{
 		MeshName:        aws.String(meshName),
-		VirtualNodeName: aws.String(name),
+		VirtualNodeName: aws.String(ConstructAppMeshVNodeNameFromCRD(name, namespace)),
 	}
 
 	if output, err := c.appmesh.DeleteVirtualNodeWithContext(ctx, input); err != nil {
@@ -622,33 +623,6 @@ func (r *Route) Prefix() string {
 	return ""
 }
 
-// Route converts into our API type
-func (r *Route) Route() appmeshv1alpha1.Route {
-	route := appmeshv1alpha1.Route{
-		Name: aws.StringValue(r.Data.RouteName),
-		Http: appmeshv1alpha1.HttpRoute{
-			Action: appmeshv1alpha1.HttpRouteAction{},
-			Match:  appmeshv1alpha1.HttpRouteMatch{},
-		},
-	}
-	if r.Data.Spec.HttpRoute != nil {
-		if r.Data.Spec.HttpRoute.Action != nil &&
-			r.Data.Spec.HttpRoute.Action.WeightedTargets != nil {
-			for _, t := range r.Data.Spec.HttpRoute.Action.WeightedTargets {
-				weight := t.Weight
-				route.Http.Action.WeightedTargets = append(route.Http.Action.WeightedTargets, appmeshv1alpha1.WeightedTarget{
-					VirtualNodeName: aws.StringValue(t.VirtualNode),
-					Weight:          aws.Int64Value(weight),
-				})
-			}
-		}
-		if r.Data.Spec.HttpRoute.Match != nil {
-			route.Http.Match.Prefix = aws.StringValue(r.Data.Spec.HttpRoute.Match.Prefix)
-		}
-	}
-	return route
-}
-
 // WeightedTargets converts into our API type
 func (r *Route) WeightedTargets() []appmeshv1alpha1.WeightedTarget {
 	if r.Data.Spec.HttpRoute.Action.WeightedTargets == nil {
@@ -677,14 +651,6 @@ func (r *Route) WeightedTargetSet() set.Set {
 
 type Routes []Route
 
-func (r Routes) Routes() []appmeshv1alpha1.Route {
-	var routes []appmeshv1alpha1.Route
-	for _, route := range r {
-		routes = append(routes, route.Route())
-	}
-	return routes
-}
-
 func (r Routes) RouteNamesSet() set.Set {
 	s := set.NewSet()
 	for _, route := range r {
@@ -727,7 +693,7 @@ func (c *Cloud) GetRoute(ctx context.Context, name string, routerName string, me
 }
 
 // CreateRoute converts the desired virtual service spec into CreateVirtualServiceInput and calls create route.
-func (c *Cloud) CreateRoute(ctx context.Context, route *appmeshv1alpha1.Route, routerName string, meshName string) (*Route, error) {
+func (c *Cloud) CreateRoute(ctx context.Context, route *appmeshv1alpha1.Route, routerName string, meshName string, namespace string) (*Route, error) {
 	ctx, cancel := context.WithTimeout(ctx, time.Second*CreateRouteTimeout)
 	defer cancel()
 
@@ -749,7 +715,7 @@ func (c *Cloud) CreateRoute(ctx context.Context, route *appmeshv1alpha1.Route, r
 	for _, target := range route.Http.Action.WeightedTargets {
 		weight := target.Weight
 		targets = append(targets, &appmesh.WeightedTarget{
-			VirtualNode: aws.String(target.VirtualNodeName),
+			VirtualNode: aws.String(ConstructAppMeshVNodeNameFromCRD(target.VirtualNodeName, namespace)),
 			Weight:      aws.Int64(weight),
 		})
 	}
@@ -800,7 +766,7 @@ func (c *Cloud) GetRoutesForVirtualRouter(ctx context.Context, routerName string
 }
 
 // UpdateRoute converts the desired virtual service spec into UpdateRouteInput and calls update route.
-func (c *Cloud) UpdateRoute(ctx context.Context, route *appmeshv1alpha1.Route, routerName string, meshName string) (*Route, error) {
+func (c *Cloud) UpdateRoute(ctx context.Context, route *appmeshv1alpha1.Route, routerName string, meshName string, namespace string) (*Route, error) {
 	ctx, cancel := context.WithTimeout(ctx, time.Second*UpdateRouteTimeout)
 	defer cancel()
 
@@ -822,7 +788,7 @@ func (c *Cloud) UpdateRoute(ctx context.Context, route *appmeshv1alpha1.Route, r
 	for _, target := range route.Http.Action.WeightedTargets {
 		weight := target.Weight
 		targets = append(targets, &appmesh.WeightedTarget{
-			VirtualNode: aws.String(target.VirtualNodeName),
+			VirtualNode: aws.String(ConstructAppMeshVNodeNameFromCRD(target.VirtualNodeName, namespace)),
 			Weight:      aws.Int64(weight),
 		})
 	}

pkg/aws/appmesh_util.go

@@ -0,0 +1,16 @@
+package aws
+
+import "strings"
+
+// This method is created to address the lack of native support of namespace within AppMesh API.
+// If virtualNodeName in the CRD spec doesn't contain ".", we will construct the new virtualNodeName by appending "-defaultNamespace".
+// If it does, the new virtualNodeName is constructed by converting the "." to "-" since "." isn't a valid character as AppMesh virtual node name.
+// Example 1: virtualNodeName: "foo", defaultNamespace: "bar". The new virtualNodeName will be "foo-bar"
+// Example 2: virtualNodeName: "foo.dummy", defaultNamespace: "bar". The new virtualNodeName will be "foo-dummy"
+func ConstructAppMeshVNodeNameFromCRD(virtualNodeName string, defaultNamespace string) string {
+	if strings.Contains(virtualNodeName, ".") {
+		return strings.ReplaceAll(virtualNodeName, ".", "-")
+	}
+	// no "."
+	return virtualNodeName + "-" + defaultNamespace
+}

pkg/aws/appmesh_util_test.go

@@ -0,0 +1,34 @@
+package aws
+
+import (
+	"testing"
+)
+
+func TestConstructAppMeshVirtualNodeNameFromCrdSpec(t *testing.T) {
+	t.Run("group", func(t *testing.T) {
+		t.Run("noDot", func(t *testing.T) {
+			originalName := "foo"
+			actual := ConstructAppMeshVNodeNameFromCRD(originalName, "bar")
+			expect := "foo-bar"
+			if actual != expect {
+				t.Errorf("got %v, expect %v", actual, expect)
+			}
+		})
+		t.Run("oneDot", func(t *testing.T) {
+			originalName := "foo.dummy"
+			actual := ConstructAppMeshVNodeNameFromCRD(originalName, "bar")
+			expect := "foo-dummy"
+			if actual != expect {
+				t.Errorf("got %v, expect %v", actual, expect)
+			}
+		})
+		t.Run("twoDots", func(t *testing.T) {
+			originalName := "foo.dummy.bummer"
+			actual := ConstructAppMeshVNodeNameFromCRD(originalName, "bar")
+			expect := "foo-dummy-bummer"
+			if actual != expect {
+				t.Errorf("got %v, expect %v", actual, expect)
+			}
+		})
+	})
+}

pkg/controller/controller_util.go

@@ -57,4 +57,4 @@ func parseMeshName(meshName string, namespace string) (string, string) {
 		meshName = meshParts[0]
 	}
 	return meshName, meshNamespace
-}
+}