Merge pull request #268 from wonderflow/refdef

add version into definitionReference for workload/trait/scope definitions

apis/core/v1alpha2/core_types.go

@@ -28,6 +28,10 @@ import (
 type DefinitionReference struct {
 	// Name of the referenced CustomResourceDefinition.
 	Name string `json:"name"`
+
+	// Version indicate which version should be used if CRD has multiple versions
+	// by default it will use the first one if not specified
+	Version string `json:"version,omitempty"`
 }
 
 // A ChildResourceKind defines a child Kubernetes resource kind with a selector

charts/oam-kubernetes-runtime/crds/core.oam.dev_scopedefinitions.yaml

@@ -57,6 +57,11 @@ spec:
                   name:
                     description: Name of the referenced CustomResourceDefinition.
                     type: string
+                  version:
+                    description: Version indicate which version should be used if
+                      CRD has multiple versions by default it will use the first one
+                      if not specified
+                    type: string
                 required:
                 - name
                 type: object

charts/oam-kubernetes-runtime/crds/core.oam.dev_traitdefinitions.yaml

@@ -61,6 +61,11 @@ spec:
                   name:
                     description: Name of the referenced CustomResourceDefinition.
                     type: string
+                  version:
+                    description: Version indicate which version should be used if
+                      CRD has multiple versions by default it will use the first one
+                      if not specified
+                    type: string
                 required:
                 - name
                 type: object

charts/oam-kubernetes-runtime/crds/core.oam.dev_workloaddefinitions.yaml

@@ -77,6 +77,11 @@ spec:
                   name:
                     description: Name of the referenced CustomResourceDefinition.
                     type: string
+                  version:
+                    description: Version indicate which version should be used if
+                      CRD has multiple versions by default it will use the first one
+                      if not specified
+                    type: string
                 required:
                 - name
                 type: object

pkg/oam/discoverymapper/mapper.go

@@ -13,6 +13,7 @@ type DiscoveryMapper interface {
 	GetMapper() (meta.RESTMapper, error)
 	Refresh() (meta.RESTMapper, error)
 	RESTMapping(gk schema.GroupKind, version ...string) (*meta.RESTMapping, error)
+	KindsFor(input schema.GroupVersionResource) ([]schema.GroupVersionKind, error)
 }
 
 var _ DiscoveryMapper = &DefaultDiscoveryMapper{}
@@ -70,3 +71,21 @@ func (d *DefaultDiscoveryMapper) RESTMapping(gk schema.GroupKind, version ...str
 	}
 	return mapping, err
 }
+
+// KindsFor will get kinds from GroupVersionResource, if version not set, all resources matched will be returned.
+func (d *DefaultDiscoveryMapper) KindsFor(input schema.GroupVersionResource) ([]schema.GroupVersionKind, error) {
+	mapper, err := d.GetMapper()
+	if err != nil {
+		return nil, err
+	}
+	mapping, err := mapper.KindsFor(input)
+	if meta.IsNoMatchError(err) {
+		// if no kind match err, refresh and try once more.
+		mapper, err = d.Refresh()
+		if err != nil {
+			return nil, err
+		}
+		mapping, err = mapper.KindsFor(input)
+	}
+	return mapping, err
+}

pkg/oam/discoverymapper/suit_test.go

@@ -112,6 +112,13 @@ var _ = Describe("Mapper discovery resources", func() {
 						OpenAPIV3Schema: &crdv1.JSONSchemaProps{
 							Type: "object",
 						}},
+				}, {
+					Name:   "v1beta1",
+					Served: true,
+					Schema: &crdv1.CustomResourceValidation{
+						OpenAPIV3Schema: &crdv1.JSONSchemaProps{
+							Type: "object",
+						}},
 				}},
 				Scope: crdv1.NamespaceScoped,
 			},
@@ -121,11 +128,24 @@ var _ = Describe("Mapper discovery resources", func() {
 		Eventually(func() error {
 			mapping, err = dism.RESTMapping(schema.GroupKind{Group: "example.com", Kind: "Foo"}, "v1")
 			return err
-		}, time.Second*5, time.Millisecond*500).Should(BeNil())
+		}, time.Second*2, time.Millisecond*300).Should(BeNil())
 		Expect(mapping.Resource).Should(Equal(schema.GroupVersionResource{
 			Group:    "example.com",
 			Version:  "v1",
 			Resource: "foos",
 		}))
+
+		var kinds []schema.GroupVersionKind
+		Eventually(func() error {
+			kinds, err = dism.KindsFor(schema.GroupVersionResource{Group: "example.com", Version: "", Resource: "foos"})
+			return err
+		}, time.Second*2, time.Millisecond*300).Should(BeNil())
+		Expect(kinds).Should(Equal([]schema.GroupVersionKind{
+			{Group: "example.com", Version: "v1", Kind: "Foo"},
+			{Group: "example.com", Version: "v1beta1", Kind: "Foo"},
+		}))
+		kinds, err = dism.KindsFor(schema.GroupVersionResource{Group: "example.com", Version: "v1", Resource: "foos"})
+		Expect(err).Should(BeNil())
+		Expect(kinds).Should(Equal([]schema.GroupVersionKind{{Group: "example.com", Version: "v1", Kind: "Foo"}}))
 	})
 })

pkg/oam/mock/mapper.go

@@ -18,10 +18,14 @@ type Refresh func() (meta.RESTMapper, error)
 // nolint
 type RESTMapping func(gk schema.GroupKind, versions ...string) (*meta.RESTMapping, error)
 
-// NewMockDiscoveryMapper for mock
+// nolint
+type KindsFor func(input schema.GroupVersionResource) ([]schema.GroupVersionKind, error)
+
+// NewMockDiscoveryMapper for unit test only
 func NewMockDiscoveryMapper() *DiscoveryMapper {
 	return &DiscoveryMapper{
 		MockRESTMapping: NewMockRESTMapping(""),
+		MockKindsFor:    NewMockKindsFor(""),
 	}
 }
 
@@ -32,11 +36,30 @@ func NewMockRESTMapping(resource string) RESTMapping {
 	}
 }
 
+// NewMockKindsFor for unit test only
+func NewMockKindsFor(kind string, version ...string) KindsFor {
+	return func(input schema.GroupVersionResource) ([]schema.GroupVersionKind, error) {
+		if len(kind) <= 1 {
+			return []schema.GroupVersionKind{{Version: input.Version, Group: input.Group, Kind: kind}}, nil
+		}
+		var ss []schema.GroupVersionKind
+		for _, v := range version {
+			gvk := schema.GroupVersionKind{Version: v, Group: input.Group, Kind: kind}
+			if input.Version != "" && input.Version == v {
+				return []schema.GroupVersionKind{gvk}, nil
+			}
+			ss = append(ss, gvk)
+		}
+		return ss, nil
+	}
+}
+
 // DiscoveryMapper for unit test only, use GetMapper and refresh will panic
 type DiscoveryMapper struct {
 	MockGetMapper   GetMapper
 	MockRefresh     Refresh
 	MockRESTMapping RESTMapping
+	MockKindsFor    KindsFor
 }
 
 // GetMapper for mock
@@ -53,3 +76,8 @@ func (m *DiscoveryMapper) Refresh() (meta.RESTMapper, error) {
 func (m *DiscoveryMapper) RESTMapping(gk schema.GroupKind, versions ...string) (*meta.RESTMapping, error) {
 	return m.MockRESTMapping(gk, versions...)
 }
+
+// KindsFor for mock
+func (m *DiscoveryMapper) KindsFor(input schema.GroupVersionResource) ([]schema.GroupVersionKind, error) {
+	return m.MockKindsFor(input)
+}

pkg/oam/util/helper.go

@@ -9,6 +9,8 @@ import (
 	"reflect"
 	"time"
 
+	"k8s.io/apimachinery/pkg/api/meta"
+
 	cpv1alpha1 "github.com/crossplane/crossplane-runtime/apis/core/v1alpha1"
 	"github.com/davecgh/go-spew/spew"
 	"github.com/go-logr/logr"
@@ -297,6 +299,23 @@ func GetDefinitionName(dm discoverymapper.DiscoveryMapper, u *unstructured.Unstr
 	return mapping.Resource.Resource + "." + groupVersion.Group, nil
 }
 
+// GetGVKFromDefinition help get Group Version Kind from DefinitionReference
+func GetGVKFromDefinition(dm discoverymapper.DiscoveryMapper, definitionRef v1alpha2.DefinitionReference) (schema.GroupVersionKind, error) {
+	var gvk schema.GroupVersionKind
+	groupResource := schema.ParseGroupResource(definitionRef.Name)
+	gvr := schema.GroupVersionResource{Group: groupResource.Group, Resource: groupResource.Resource, Version: definitionRef.Version}
+	kinds, err := dm.KindsFor(gvr)
+	if err != nil {
+		return gvk, err
+	}
+	if len(kinds) < 1 {
+		return gvk, &meta.NoResourceMatchError{
+			PartialResource: gvr,
+		}
+	}
+	return kinds[0], nil
+}
+
 // Object2Unstructured convert an object to an unstructured struct
 func Object2Unstructured(obj interface{}) (*unstructured.Unstructured, error) {
 	objMap, err := Object2Map(obj)

pkg/oam/util/helper_test.go

@@ -8,6 +8,8 @@ import (
 	"reflect"
 	"testing"
 
+	"k8s.io/apimachinery/pkg/runtime/schema"
+
 	"github.com/crossplane/crossplane-runtime/apis/core/v1alpha1"
 	"github.com/crossplane/crossplane-runtime/pkg/resource/fake"
 	"github.com/crossplane/crossplane-runtime/pkg/test"
@@ -772,6 +774,26 @@ func TestUnstructured(t *testing.T) {
 	}
 }
 
+func TestGetGVKFromDef(t *testing.T) {
+	mapper := mock.NewMockDiscoveryMapper()
+	mapper.MockKindsFor = mock.NewMockKindsFor("Abc", "v1", "v2")
+	gvk, err := util.GetGVKFromDefinition(mapper, v1alpha2.DefinitionReference{Name: "abcs.example.com"})
+	assert.NoError(t, err)
+	assert.Equal(t, schema.GroupVersionKind{
+		Group:   "example.com",
+		Version: "v1",
+		Kind:    "Abc",
+	}, gvk)
+
+	gvk, err = util.GetGVKFromDefinition(mapper, v1alpha2.DefinitionReference{Name: "abcs.example.com", Version: "v2"})
+	assert.NoError(t, err)
+	assert.Equal(t, schema.GroupVersionKind{
+		Group:   "example.com",
+		Version: "v2",
+		Kind:    "Abc",
+	}, gvk)
+}
+
 func TestGenTraitName(t *testing.T) {
 	mts := v1alpha2.ManualScalerTrait{
 		ObjectMeta: metav1.ObjectMeta{

pkg/webhook/v1alpha2/admit.go

@@ -13,7 +13,9 @@ func Add(mgr manager.Manager) error {
 		return err
 	}
 	applicationconfiguration.RegisterMutatingHandler(mgr)
-	component.RegisterMutatingHandler(mgr)
+	if err := component.RegisterMutatingHandler(mgr); err != nil {
+		return err
+	}
 	component.RegisterValidatingHandler(mgr)
 	return nil
 }

pkg/webhook/v1alpha2/component/component_test.go

@@ -4,6 +4,8 @@ import (
 	"context"
 	"fmt"
 
+	"github.com/crossplane/oam-kubernetes-runtime/pkg/oam/mock"
+
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 
@@ -177,6 +179,9 @@ var _ = Describe("Component Admission controller Test", func() {
 				injc := handler.(inject.Client)
 				injc.InjectClient(test.client)
 				mutatingHandler := handler.(*MutatingHandler)
+				dm := mock.NewMockDiscoveryMapper()
+				dm.MockKindsFor = mock.NewMockKindsFor("Foo", "v1")
+				mutatingHandler.Mapper = dm
 				err := mutatingHandler.Mutate(&component)
 				if len(test.errMsg) == 0 {
 					Expect(err).Should(BeNil())

pkg/webhook/v1alpha2/component/mutating_handler.go

@@ -22,7 +22,8 @@ import (
 	"fmt"
 	"net/http"
 
-	crdv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
+	"github.com/crossplane/oam-kubernetes-runtime/pkg/oam/discoverymapper"
+
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/types"
@@ -46,6 +47,7 @@ const (
 // MutatingHandler handles Component
 type MutatingHandler struct {
 	Client client.Client
+	Mapper discoverymapper.DiscoveryMapper
 
 	// Decoder decodes objects
 	Decoder *admission.Decoder
@@ -102,9 +104,8 @@ func (h *MutatingHandler) Mutate(obj *v1alpha2.Component) error {
 		if err := h.Client.Get(context.TODO(), types.NamespacedName{Name: workloadType}, workloadDefinition); err != nil {
 			return err
 		}
-		// fetch the CRDs definition
-		customResourceDefinition := &crdv1.CustomResourceDefinition{}
-		if err := h.Client.Get(context.TODO(), types.NamespacedName{Name: workloadDefinition.Spec.Reference.Name}, customResourceDefinition); err != nil {
+		gvk, err := util.GetGVKFromDefinition(h.Mapper, workloadDefinition.Spec.Reference)
+		if err != nil {
 			return err
 		}
 		// reconstruct the workload CR
@@ -114,11 +115,11 @@ func (h *MutatingHandler) Mutate(obj *v1alpha2.Component) error {
 		}
 		// find out the GVK from the CRD definition and set
 		apiVersion := metav1.GroupVersion{
-			Group:   customResourceDefinition.Spec.Group,
-			Version: customResourceDefinition.Spec.Versions[0].Name,
+			Group:   gvk.Group,
+			Version: gvk.Version,
 		}.String()
 		workload.SetAPIVersion(apiVersion)
-		workload.SetKind(customResourceDefinition.Spec.Names.Kind)
+		workload.SetKind(gvk.Kind)
 		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())
@@ -152,7 +153,12 @@ func (h *MutatingHandler) InjectDecoder(d *admission.Decoder) error {
 }
 
 // RegisterMutatingHandler will register component mutation handler to the webhook
-func RegisterMutatingHandler(mgr manager.Manager) {
+func RegisterMutatingHandler(mgr manager.Manager) error {
+	mapper, err := discoverymapper.New(mgr.GetConfig())
+	if err != nil {
+		return err
+	}
 	server := mgr.GetWebhookServer()
-	server.Register("/mutating-core-oam-dev-v1alpha2-components", &webhook.Admission{Handler: &MutatingHandler{}})
+	server.Register("/mutating-core-oam-dev-v1alpha2-components", &webhook.Admission{Handler: &MutatingHandler{Mapper: mapper}})
+	return nil
 }