Fix could not find WorkloadDefinition issue (#208)

* Fix `could not find WorkloadDefinition` issue When there is spec.type in Workload, an annotation will set for the workload which can help find the WorkloadDefinition. Fix issue #207, related to feature #195. Co-authored-by: Sun Jianbo <[email protected]> Signed-off-by: zzxwill <[email protected]> * Address comments and add unit-test Signed-off-by: zzxwill <[email protected]> * Fix goimports issue Signed-off-by: zzxwill <[email protected]> * Change function name and add unit-test Signed-off-by: zzxwill <[email protected]> * Removed maker annotation `definition.oam.dev/name` Signed-off-by: zzxwill [email protected] Signed-off-by: zzxwill <[email protected]> Co-authored-by: Sun Jianbo <[email protected]>
crossplane · Sep 21, 2020 · 1537fb2 · 1537fb2
1 parent ad89876
commit 1537fb2
Show file tree

Hide file tree

Showing 10 changed files with 53 additions and 17 deletions.
diff --git a/.gitignore b/.gitignore
@@ -31,4 +31,10 @@ bin/
 tmp/
 
 # Vscode files
-.vscode
+.vscode
+
+# Webhook certificates
+csr.conf
+oam-kubernetes-runtime-webhook.csr
+oam-kubernetes-runtime-webhook.key
+oam-kubernetes-runtime-webhook.pem
diff --git a/README.md b/README.md
@@ -83,7 +83,7 @@ Or you can install with webhook enabled by following steps:
     kubectl -n oam-system create secret generic webhook-server-cert --from-file=tls.key=./oam-kubernetes-runtime-webhook.key --from-file=tls.crt=./oam-kubernetes-runtime-webhook.pem
     ```
 
-  - Step 3: Get CA Bundle info and install with it's value
+  - Step 3: Get CA Bundle info and install with its value
 
     ```shell script
     caValue=`kubectl config view --raw --minify --flatten -o jsonpath='{.clusters[].cluster.certificate-authority-data}'`

diff --git a/pkg/controller/v1alpha2/applicationconfiguration/render_test.go b/pkg/controller/v1alpha2/applicationconfiguration/render_test.go
@@ -724,7 +724,7 @@ func TestRenderTraitWithoutMetadataName(t *testing.T) {
 	}
 }
 
-func TestGetCRDName(t *testing.T) {
+func TestGetDefinitionName(t *testing.T) {
 	tests := map[string]struct {
 		u      *unstructured.Unstructured
 		exp    string
@@ -749,7 +749,7 @@ func TestGetCRDName(t *testing.T) {
 	}
 	for name, ti := range tests {
 		t.Run(name, func(t *testing.T) {
-			got := util.GetCRDName(ti.u)
+			got := util.GetDefinitionName(ti.u)
 			if got != ti.exp {
 				t.Errorf("%s getCRDName want %s got %s ", ti.reason, ti.exp, got)
 			}

diff --git a/pkg/controller/v1alpha2/core/workloads/containerizedworkload/translate_test.go b/pkg/controller/v1alpha2/core/workloads/containerizedworkload/translate_test.go
@@ -388,6 +388,11 @@ func TestServiceInjector(t *testing.T) {
 		err    error
 	}
 
+	invalidDeployment := &appsv1.StatefulSet{
+		TypeMeta: metav1.TypeMeta{
+			APIVersion: "v1alpha1",
+		}}
+
 	cases := map[string]struct {
 		reason string
 		args   args
@@ -400,6 +405,21 @@ func TestServiceInjector(t *testing.T) {
 			},
 			want: want{},
 		},
+		"InvalidObject": {
+			reason: "invalid object should immediately return nil.",
+			args: args{
+				w: &mock.Workload{},
+				o: []oam.Object{
+					invalidDeployment,
+				},
+			},
+			want: want{
+				result: []oam.Object{
+					invalidDeployment,
+				},
+				err: nil,
+			},
+		},
 		"SuccessfulInjectService_1D_1C_1P": {
 			reason: "A Deployment with a port(s) should have a Service injected for first defined port.",
 			args: args{

diff --git a/pkg/oam/types.go b/pkg/oam/types.go
@@ -27,6 +27,9 @@ import (
 	runtimev1alpha1 "github.com/crossplane/crossplane-runtime/apis/core/v1alpha1"
 )
 
+// WorkloadTypeLabel indicates the type of the workloadDefinition
+const WorkloadTypeLabel = "workload.oam.dev/type"
+
 // ScopeKind contains the type metadata for a kind of an OAM scope resource.
 type ScopeKind schema.GroupVersionKind
 

diff --git a/pkg/oam/util/helper.go b/pkg/oam/util/helper.go
@@ -115,7 +115,7 @@ func FetchWorkload(ctx context.Context, c client.Client, mLog logr.Logger, oamTr
 func FetchScopeDefinition(ctx context.Context, r client.Reader,
 	scope *unstructured.Unstructured) (*v1alpha2.ScopeDefinition, error) {
 	// The name of the scopeDefinition CR is the CRD name of the scope
-	spName := GetCRDName(scope)
+	spName := GetDefinitionName(scope)
 	// the scopeDefinition crd is cluster scoped
 	nn := types.NamespacedName{Name: spName}
 	// Fetch the corresponding scopeDefinition CR
@@ -130,7 +130,7 @@ func FetchScopeDefinition(ctx context.Context, r client.Reader,
 func FetchTraitDefinition(ctx context.Context, r client.Reader,
 	trait *unstructured.Unstructured) (*v1alpha2.TraitDefinition, error) {
 	// The name of the traitDefinition CR is the CRD name of the trait
-	trName := GetCRDName(trait)
+	trName := GetDefinitionName(trait)
 	// the traitDefinition crd is cluster scoped
 	nn := types.NamespacedName{Name: trName}
 	// Fetch the corresponding traitDefinition CR
@@ -145,7 +145,7 @@ func FetchTraitDefinition(ctx context.Context, r client.Reader,
 func FetchWorkloadDefinition(ctx context.Context, r client.Reader,
 	workload *unstructured.Unstructured) (*v1alpha2.WorkloadDefinition, error) {
 	// The name of the workloadDefinition CR is the CRD name of the component
-	wldName := GetCRDName(workload)
+	wldName := GetDefinitionName(workload)
 	// the workloadDefinition crd is cluster scoped
 	nn := types.NamespacedName{Name: wldName}
 	// Fetch the corresponding workloadDefinition CR
@@ -224,9 +224,17 @@ func PassLabelAndAnnotation(parentObj oam.Object, childObj labelAnnotationObject
 	childObj.SetAnnotations(MergeMap(parentObj.GetAnnotations(), childObj.GetAnnotations()))
 }
 
-// GetCRDName return the CRD name of any resources
-// the format of the CRD of a resource is <kind purals>.<group>
-func GetCRDName(u *unstructured.Unstructured) string {
+// GetDefinitionName return the Definition name of any resources
+// the format of the definition of a resource is <kind plurals>.<group>
+// Now the definition name of a resource could also be defined as `definition.oam.dev/name` in `metadata.annotations`
+func GetDefinitionName(u *unstructured.Unstructured) string {
+	if labels := u.GetLabels(); labels != nil {
+		if resourceType, ok := labels[oam.LabelOAMResourceType]; ok && resourceType == "WORKLOAD" {
+			if definitionName, ok := labels[oam.WorkloadTypeLabel]; ok {
+				return definitionName
+			}
+		}
+	}
 	group, _ := APIVersion2GroupVersion(u.GetAPIVersion())
 	resources := []string{Kind2Resource(u.GetKind())}
 	if group != "" {

diff --git a/pkg/oam/util/helper_test.go b/pkg/oam/util/helper_test.go
@@ -717,7 +717,7 @@ var _ = Describe("Test unstructured related helper utils", func() {
 			},
 		}
 		for name, ti := range tests {
-			got := util.GetCRDName(ti.u)
+			got := util.GetDefinitionName(ti.u)
 			By(fmt.Sprint("Running test: ", name))
 			Expect(ti.exp).Should(Equal(got))
 		}

diff --git a/pkg/webhook/v1alpha2/applicationconfiguration/applicationconfiguration.go b/pkg/webhook/v1alpha2/applicationconfiguration/applicationconfiguration.go
@@ -136,7 +136,7 @@ func (h *ValidatingHandler) InjectDecoder(d *admission.Decoder) error {
 	return nil
 }
 
-// Register will regsiter application configuration validation to webhook
+// Register will register application configuration validation to webhook
 func Register(mgr manager.Manager) {
 	server := mgr.GetWebhookServer()
 	server.Register("/validating-core-oam-dev-v1alpha2-applicationconfigurations", &webhook.Admission{Handler: &ValidatingHandler{}})

diff --git a/pkg/webhook/v1alpha2/component/component_test.go b/pkg/webhook/v1alpha2/component/component_test.go
@@ -20,6 +20,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 
 	"github.com/crossplane/oam-kubernetes-runtime/apis/core/v1alpha2"
+	"github.com/crossplane/oam-kubernetes-runtime/pkg/oam"
 	"github.com/crossplane/oam-kubernetes-runtime/pkg/oam/util"
 	. "github.com/crossplane/oam-kubernetes-runtime/pkg/webhook/v1alpha2/component"
 )
@@ -120,7 +121,7 @@ var _ = Describe("Component Admission controller Test", func() {
 			// set up the result
 			mutatedWorkload := baseWorkload.DeepCopy()
 			mutatedWorkload.SetNamespace(component.GetNamespace())
-			mutatedWorkload.SetLabels(util.MergeMap(label, map[string]string{WorkloadTypeLabel: workloadTypeName}))
+			mutatedWorkload.SetLabels(util.MergeMap(label, map[string]string{oam.WorkloadTypeLabel: workloadTypeName}))
 			tests := map[string]struct {
 				client   client.Client
 				workload interface{}

diff --git a/pkg/webhook/v1alpha2/component/mutating_handler.go b/pkg/webhook/v1alpha2/component/mutating_handler.go
@@ -34,15 +34,13 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 
 	"github.com/crossplane/oam-kubernetes-runtime/apis/core/v1alpha2"
+	"github.com/crossplane/oam-kubernetes-runtime/pkg/oam"
 	"github.com/crossplane/oam-kubernetes-runtime/pkg/oam/util"
 )
 
 const (
 	// TypeField is the special field indicate the type of the workloadDefinition
 	TypeField = "type"
-
-	// WorkloadTypeLabel indicates the type of the workloadDefinition
-	WorkloadTypeLabel = "workload.oam.dev/type"
 )
 
 // MutatingHandler handles Component
@@ -124,7 +122,7 @@ func (h *MutatingHandler) Mutate(obj *v1alpha2.Component) error {
 		mutatelog.Info("Set the component workload GVK", "workload api version", workload.GetAPIVersion(), "workload Kind", workload.GetKind())
 		// copy namespace/label/annotation to the workload and add workloadType label
 		workload.SetNamespace(obj.GetNamespace())
-		workload.SetLabels(util.MergeMap(obj.GetLabels(), map[string]string{WorkloadTypeLabel: workloadType}))
+		workload.SetLabels(util.MergeMap(obj.GetLabels(), map[string]string{oam.WorkloadTypeLabel: workloadType}))
 		workload.SetAnnotations(obj.GetAnnotations())
 		// copy back the object
 		rawBye, err := json.Marshal(workload.Object)