Add KReference.Group field and ResolveGroup function (#2127)

* Add Group field and ResolveGroup function

Signed-off-by: Francesco Guardiani <francescoguard@gmail.com>

* Remove core special case

Signed-off-by: Francesco Guardiani <francescoguard@gmail.com>

* Copyright

Signed-off-by: Francesco Guardiani <francescoguard@gmail.com>

* Added validation code

Signed-off-by: Francesco Guardiani <francescoguard@gmail.com>

* Fix comment

Signed-off-by: Francesco Guardiani <francescoguard@gmail.com>

* Add omitempty

Signed-off-by: Francesco Guardiani <francescoguard@gmail.com>

* Moved ResolveGroup code to kref
Made ResolveGroup a util method, more than an instance method

Signed-off-by: Francesco Guardiani <francescoguard@gmail.com>

* New type KReferenceResolver

Signed-off-by: Francesco Guardiani <francescoguard@gmail.com>

* Added +optional as suggested

Signed-off-by: Francesco Guardiani <francescoguard@gmail.com>

apis/duck/v1/knative_reference.go

@@ -19,6 +19,7 @@ package v1
 import (
 	"context"
 	"fmt"
+	"strings"
 
 	"knative.dev/pkg/apis"
 )
@@ -41,7 +42,13 @@ type KReference struct {
 	Name string `json:"name"`
 
 	// API version of the referent.
-	APIVersion string `json:"apiVersion"`
+	// +optional
+	APIVersion string `json:"apiVersion,omitempty"`
+
+	// Group of the API, without the version of the group. This can be used as an alternative to the APIVersion, and then resolved using ResolveGroup.
+	// Note: This API is EXPERIMENTAL and might break anytime. For more details: https://github.com/knative/eventing/issues/5086
+	// +optional
+	Group string `json:"group,omitempty"`
 }
 
 func (kr *KReference) Validate(ctx context.Context) *apis.FieldError {
@@ -54,9 +61,26 @@ func (kr *KReference) Validate(ctx context.Context) *apis.FieldError {
 	if kr.Name == "" {
 		errs = errs.Also(apis.ErrMissingField("name"))
 	}
+	if isKReferenceGroupAllowed(ctx) {
+		if kr.APIVersion == "" && kr.Group == "" {
+			errs = errs.Also(apis.ErrMissingField("apiVersion")).
+				Also(apis.ErrMissingField("group"))
+		}
+		if kr.APIVersion != "" && kr.Group != "" && !strings.HasPrefix(kr.APIVersion, kr.Group) {
+			errs = errs.Also(&apis.FieldError{
+				Message: "both apiVersion and group are specified and they refer to different API groups",
+				Paths:   []string{"apiVersion", "group"},
+				Details: "Only one of them must be specified",
+			})
+		}
+	} else {
+		if kr.Group != "" {
+			errs = errs.Also(apis.ErrDisallowedFields("group"))
+		}
 		if kr.APIVersion == "" {
 			errs = errs.Also(apis.ErrMissingField("apiVersion"))
 		}
+	}
 	if kr.Kind == "" {
 		errs = errs.Also(apis.ErrMissingField("kind"))
 	}
@@ -86,3 +110,17 @@ func (kr *KReference) SetDefaults(ctx context.Context) {
 		kr.Namespace = apis.ParentMeta(ctx).Namespace
 	}
 }
+
+type isGroupAllowed struct{}
+
+func isKReferenceGroupAllowed(ctx context.Context) bool {
+	return ctx.Value(isGroupAllowed{}) != nil
+}
+
+// KReferenceGroupAllowed notes on the context that further validation
+// should allow the KReference.Group, which is disabled by default.
+// Note: This API is EXPERIMENTAL and may disappear once the KReference.Group feature will stabilize.
+// For more details: https://github.com/knative/eventing/issues/5086
+func KReferenceGroupAllowed(ctx context.Context) context.Context {
+	return context.WithValue(ctx, isGroupAllowed{}, struct{}{})
+}

apis/duck/v1/knative_reference_test.go

@@ -25,6 +25,10 @@ import (
 	"knative.dev/pkg/apis"
 )
 
+const (
+	group = "group.my"
+)
+
 func TestValidate(t *testing.T) {
 	ctx := context.Background()
 
@@ -87,6 +91,71 @@ func TestValidate(t *testing.T) {
 				Details: `parent namespace: "diffns" does not match ref: "b-namespace"`,
 			},
 		},
+		"invalid ref, disallowed group": {
+			ref: &KReference{
+				Namespace: namespace,
+				Name:      name,
+				Kind:      kind,
+				Group:     group,
+			},
+			ctx:  ctx,
+			want: apis.ErrMissingField("apiVersion").Also(apis.ErrDisallowedFields("group")),
+		},
+		"invalid ref, group allowed and both api version and group are specified, but they are conflicting": {
+			ref: &KReference{
+				Namespace:  namespace,
+				Name:       name,
+				Kind:       kind,
+				Group:      group,
+				APIVersion: apiVersion,
+			},
+			ctx: KReferenceGroupAllowed(ctx),
+			want: &apis.FieldError{
+				Message: "both apiVersion and group are specified and they refer to different API groups",
+				Paths:   []string{"apiVersion", "group"},
+				Details: "Only one of them must be specified",
+			},
+		},
+		"invalid ref, group allowed and both api version and group are specified": {
+			ref: &KReference{
+				Namespace:  namespace,
+				Name:       name,
+				Kind:       kind,
+				Group:      "eventing.knative.dev",
+				APIVersion: "eventing.knative.dev/v1",
+			},
+			ctx:  KReferenceGroupAllowed(ctx),
+			want: nil,
+		},
+		"valid ref, group enabled and both apiVersion and group missing": {
+			ref: &KReference{
+				Namespace: namespace,
+				Name:      name,
+				Kind:      kind,
+			},
+			ctx:  KReferenceGroupAllowed(ctx),
+			want: apis.ErrMissingField("apiVersion").Also(apis.ErrMissingField("group")),
+		},
+		"valid ref, group enabled and configured": {
+			ref: &KReference{
+				Namespace: namespace,
+				Name:      name,
+				Kind:      kind,
+				Group:     group,
+			},
+			ctx:  KReferenceGroupAllowed(ctx),
+			want: nil,
+		},
+		"valid ref, group enabled but apiVersion configured": {
+			ref: &KReference{
+				Namespace:  namespace,
+				Name:       name,
+				Kind:       kind,
+				APIVersion: apiVersion,
+			},
+			ctx:  KReferenceGroupAllowed(ctx),
+			want: nil,
+		},
 		"valid ref, mismatched namespaces, but overridden": {
 			ref: &KReference{
 				Namespace:  namespace,

kref/knative_reference.go

@@ -0,0 +1,77 @@
+/*
+Copyright 2020 The Knative 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 kref
+
+import (
+	"fmt"
+
+	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
+	apiextensionsv1lister "k8s.io/apiextensions-apiserver/pkg/client/listers/apiextensions/v1"
+	"k8s.io/apimachinery/pkg/api/meta"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	duckv1 "knative.dev/pkg/apis/duck/v1"
+)
+
+// KReferenceResolver is an object that resolves the KReference.Group field
+// Note: This API is EXPERIMENTAL and might break anytime. For more details: https://github.com/knative/eventing/issues/5086
+type KReferenceResolver struct {
+	crdLister apiextensionsv1lister.CustomResourceDefinitionLister
+}
+
+// NewKReferenceResolver creates a new KReferenceResolver from a crdLister
+// Note: This API is EXPERIMENTAL and might break anytime. For more details: https://github.com/knative/eventing/issues/5086
+func NewKReferenceResolver(crdLister apiextensionsv1lister.CustomResourceDefinitionLister) *KReferenceResolver {
+	return &KReferenceResolver{crdLister: crdLister}
+}
+
+// ResolveGroup resolves the APIVersion of a KReference starting from the Group.
+// In order to execute this method, you need RBAC to read the CRD of the Resource referred in this KReference.
+// Note: This API is EXPERIMENTAL and might break anytime. For more details: https://github.com/knative/eventing/issues/5086
+func (resolver *KReferenceResolver) ResolveGroup(kr *duckv1.KReference) (*duckv1.KReference, error) {
+	if kr.Group == "" {
+		// Nothing to do here
+		return kr, nil
+	}
+
+	kr = kr.DeepCopy()
+
+	actualGvk := schema.GroupVersionKind{Group: kr.Group, Kind: kr.Kind}
+	pluralGvk, _ := meta.UnsafeGuessKindToResource(actualGvk)
+	crd, err := resolver.crdLister.Get(pluralGvk.GroupResource().String())
+	if err != nil {
+		return nil, err
+	}
+
+	actualGvk.Version, err = findCRDStorageVersion(crd)
+	if err != nil {
+		return nil, err
+	}
+
+	kr.APIVersion, kr.Kind = actualGvk.ToAPIVersionAndKind()
+
+	return kr, nil
+}
+
+// This function runs under the assumption that there must be exactly one "storage" version
+func findCRDStorageVersion(crd *apiextensionsv1.CustomResourceDefinition) (string, error) {
+	for _, version := range crd.Spec.Versions {
+		if version.Storage {
+			return version.Name, nil
+		}
+	}
+	return "", fmt.Errorf("this CRD %s doesn't have a storage version! Kubernetes, you're drunk, go home", crd.Name)
+}

kref/knative_reference_resolve_group_test.go

@@ -0,0 +1,127 @@
+/*
+Copyright 2021 The Knative 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 kref
+
+import (
+	"context"
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/client-go/rest"
+
+	. "knative.dev/pkg/apis/duck/v1"
+	customresourcedefinitioninformer "knative.dev/pkg/client/injection/apiextensions/informers/apiextensions/v1/customresourcedefinition/fake"
+	"knative.dev/pkg/injection"
+)
+
+func TestResolveGroup(t *testing.T) {
+	const crdGroup = "messaging.knative.dev"
+	const crdName = "inmemorychannels." + crdGroup
+
+	ctx, _ := injection.Fake.SetupInformers(context.TODO(), &rest.Config{})
+
+	fakeCrdInformer := customresourcedefinitioninformer.Get(ctx)
+	fakeCrdInformer.Informer().GetIndexer().Add(
+		&apiextensionsv1.CustomResourceDefinition{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: crdName,
+			},
+			Spec: apiextensionsv1.CustomResourceDefinitionSpec{
+				Versions: []apiextensionsv1.CustomResourceDefinitionVersion{{
+					Name:    "v1beta1",
+					Storage: false,
+				}, {
+					Name:    "v1",
+					Storage: true,
+				}},
+			},
+		},
+	)
+
+	tests := map[string]struct {
+		input   *KReference
+		output  *KReference
+		wantErr bool
+	}{
+		"No group": {
+			input: &KReference{
+				Kind:       "Abc",
+				Name:       "123",
+				APIVersion: "something/v1",
+			},
+			output: &KReference{
+				Kind:       "Abc",
+				Name:       "123",
+				APIVersion: "something/v1",
+			},
+		},
+		"No group nor api version": {
+			input: &KReference{
+				Kind: "Abc",
+				Name: "123",
+			},
+			output: &KReference{
+				Kind: "Abc",
+				Name: "123",
+			},
+		},
+		"imc channel": {
+			input: &KReference{
+				Kind:  "InMemoryChannel",
+				Name:  "my-cool-channel",
+				Group: crdGroup,
+			},
+			output: &KReference{
+				Kind:       "InMemoryChannel",
+				Name:       "my-cool-channel",
+				Group:      crdGroup,
+				APIVersion: crdGroup + "/v1",
+			},
+		},
+		"unknown CRD": {
+			input: &KReference{
+				Kind:  "MyChannel",
+				Name:  "my-cool-channel",
+				Group: crdGroup,
+			},
+			wantErr: true,
+		},
+	}
+
+	for name, tc := range tests {
+		t.Run(name, func(t *testing.T) {
+			kr, err := NewKReferenceResolver(fakeCrdInformer.Lister()).ResolveGroup(tc.input)
+			if err != nil {
+				if !tc.wantErr {
+					t.Error("ResolveGroup() =", err)
+				}
+				return
+			} else if tc.wantErr {
+				t.Errorf("ResolveGroup() = %v, wanted error", err)
+				return
+			}
+
+			if tc.output != nil {
+				if !cmp.Equal(tc.output, kr) {
+					t.Errorf("ResolveGroup diff: (-want, +got) =\n%s", cmp.Diff(tc.input, tc.output))
+				}
+			}
+		})
+	}
+}