From 69738db6006b1d8d5196fa00af220b8c2750b11e Mon Sep 17 00:00:00 2001 From: slinkydeveloper Date: Tue, 18 May 2021 16:24:51 +0200 Subject: [PATCH 1/9] Add Group field and ResolveGroup function Signed-off-by: Francesco Guardiani --- apis/duck/v1/knative_reference.go | 51 +++++++ .../knative_reference_resolve_group_test.go | 124 ++++++++++++++++++ 2 files changed, 175 insertions(+) create mode 100644 apis/duck/v1/knative_reference_resolve_group_test.go diff --git a/apis/duck/v1/knative_reference.go b/apis/duck/v1/knative_reference.go index 27a0a262b0..4d93ce4a5d 100644 --- a/apis/duck/v1/knative_reference.go +++ b/apis/duck/v1/knative_reference.go @@ -20,6 +20,11 @@ import ( "context" "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" + "knative.dev/pkg/apis" ) @@ -42,6 +47,11 @@ type KReference struct { // API version of the referent. APIVersion string `json:"apiVersion"` + + // 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 { @@ -86,3 +96,44 @@ func (kr *KReference) SetDefaults(ctx context.Context) { kr.Namespace = apis.ParentMeta(ctx).Namespace } } + +// 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 (kr *KReference) ResolveGroup(crdLister apiextensionsv1lister.CustomResourceDefinitionLister) error { + if kr.Group == "" { + // Nothing to do here + return nil + } + if kr.Group == "core" { + // We statically resolve it to core/v1 + kr.APIVersion = "core/v1" + return nil + } + + actualGvk := schema.GroupVersionKind{Group: kr.Group, Kind: kr.Kind} + pluralGvk, _ := meta.UnsafeGuessKindToResource(actualGvk) + crd, err := crdLister.Get(pluralGvk.GroupResource().String()) + if err != nil { + return err + } + + actualGvk.Version, err = findCRDStorageVersion(crd) + if err != nil { + return err + } + + kr.APIVersion, kr.Kind = actualGvk.ToAPIVersionAndKind() + + return 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) +} diff --git a/apis/duck/v1/knative_reference_resolve_group_test.go b/apis/duck/v1/knative_reference_resolve_group_test.go new file mode 100644 index 0000000000..2124156dad --- /dev/null +++ b/apis/duck/v1/knative_reference_resolve_group_test.go @@ -0,0 +1,124 @@ +package v1_test + +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", + }, + }, + "core api group": { + input: &KReference{ + Kind: "Abc", + Name: "123", + Group: "core", + }, + output: &KReference{ + Kind: "Abc", + Name: "123", + Group: "core", + APIVersion: "core/v1", + }, + }, + "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) { + err := tc.input.ResolveGroup(fakeCrdInformer.Lister()) + 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, tc.input) { + t.Errorf("ResolveGroup diff: (-want, +got) =\n%s", cmp.Diff(tc.input, tc.output)) + } + } + }) + } +} From 8433bab5d9baefdc023721b03a10eacfed54d8db Mon Sep 17 00:00:00 2001 From: slinkydeveloper Date: Tue, 18 May 2021 17:40:08 +0200 Subject: [PATCH 2/9] Remove core special case Signed-off-by: Francesco Guardiani --- apis/duck/v1/knative_reference.go | 5 ----- .../duck/v1/knative_reference_resolve_group_test.go | 13 ------------- 2 files changed, 18 deletions(-) diff --git a/apis/duck/v1/knative_reference.go b/apis/duck/v1/knative_reference.go index 4d93ce4a5d..8ddc3b49b1 100644 --- a/apis/duck/v1/knative_reference.go +++ b/apis/duck/v1/knative_reference.go @@ -105,11 +105,6 @@ func (kr *KReference) ResolveGroup(crdLister apiextensionsv1lister.CustomResourc // Nothing to do here return nil } - if kr.Group == "core" { - // We statically resolve it to core/v1 - kr.APIVersion = "core/v1" - return nil - } actualGvk := schema.GroupVersionKind{Group: kr.Group, Kind: kr.Kind} pluralGvk, _ := meta.UnsafeGuessKindToResource(actualGvk) diff --git a/apis/duck/v1/knative_reference_resolve_group_test.go b/apis/duck/v1/knative_reference_resolve_group_test.go index 2124156dad..b2e87b8f8e 100644 --- a/apis/duck/v1/knative_reference_resolve_group_test.go +++ b/apis/duck/v1/knative_reference_resolve_group_test.go @@ -65,19 +65,6 @@ func TestResolveGroup(t *testing.T) { Name: "123", }, }, - "core api group": { - input: &KReference{ - Kind: "Abc", - Name: "123", - Group: "core", - }, - output: &KReference{ - Kind: "Abc", - Name: "123", - Group: "core", - APIVersion: "core/v1", - }, - }, "imc channel": { input: &KReference{ Kind: "InMemoryChannel", From 90c4222abaa44969d02143c4ac1d712b40d54990 Mon Sep 17 00:00:00 2001 From: slinkydeveloper Date: Tue, 18 May 2021 17:54:39 +0200 Subject: [PATCH 3/9] Copyright Signed-off-by: Francesco Guardiani --- .../v1/knative_reference_resolve_group_test.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/apis/duck/v1/knative_reference_resolve_group_test.go b/apis/duck/v1/knative_reference_resolve_group_test.go index b2e87b8f8e..9d9a73b195 100644 --- a/apis/duck/v1/knative_reference_resolve_group_test.go +++ b/apis/duck/v1/knative_reference_resolve_group_test.go @@ -1,3 +1,19 @@ +/* +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 v1_test import ( From 2168ef48914bda988c8f42c68867f9cebee61253 Mon Sep 17 00:00:00 2001 From: slinkydeveloper Date: Wed, 19 May 2021 09:40:10 +0200 Subject: [PATCH 4/9] Added validation code Signed-off-by: Francesco Guardiani --- apis/duck/v1/knative_reference.go | 35 +++++++++++++++- apis/duck/v1/knative_reference_test.go | 58 ++++++++++++++++++++++++++ 2 files changed, 91 insertions(+), 2 deletions(-) diff --git a/apis/duck/v1/knative_reference.go b/apis/duck/v1/knative_reference.go index 8ddc3b49b1..909e985960 100644 --- a/apis/duck/v1/knative_reference.go +++ b/apis/duck/v1/knative_reference.go @@ -64,8 +64,25 @@ func (kr *KReference) Validate(ctx context.Context) *apis.FieldError { if kr.Name == "" { errs = errs.Also(apis.ErrMissingField("name")) } - if kr.APIVersion == "" { - errs = errs.Also(apis.ErrMissingField("apiVersion")) + if isKReferenceGroupAllowed(ctx) { + if kr.APIVersion == "" && kr.Group == "" { + errs = errs.Also(apis.ErrMissingField("apiVersion")). + Also(apis.ErrMissingField("group")) + } + if kr.APIVersion != "" && kr.Group != "" { + errs = errs.Also(&apis.FieldError{ + Message: "both apiVersion and group are specified", + 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")) @@ -132,3 +149,17 @@ func findCRDStorageVersion(crd *apiextensionsv1.CustomResourceDefinition) (strin } return "", fmt.Errorf("this CRD %s doesn't have a storage version! Kubernetes, you're drunk, go home", crd.Name) } + +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 will 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{}{}) +} diff --git a/apis/duck/v1/knative_reference_test.go b/apis/duck/v1/knative_reference_test.go index 058cb91b3f..c3dd361988 100644 --- a/apis/duck/v1/knative_reference_test.go +++ b/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,60 @@ 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 buth both api version and group are configured": { + 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", + Paths: []string{"apiVersion", "group"}, + Details: "Only one of them must be specified", + }, + }, + "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 and 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, From 4a139d9e01cba5cf9014ba18337bbdbcc49426e1 Mon Sep 17 00:00:00 2001 From: slinkydeveloper Date: Wed, 19 May 2021 11:17:06 +0200 Subject: [PATCH 5/9] Fix comment Signed-off-by: Francesco Guardiani --- apis/duck/v1/knative_reference.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apis/duck/v1/knative_reference.go b/apis/duck/v1/knative_reference.go index 909e985960..93afd6ca08 100644 --- a/apis/duck/v1/knative_reference.go +++ b/apis/duck/v1/knative_reference.go @@ -158,7 +158,7 @@ func isKReferenceGroupAllowed(ctx context.Context) bool { // KReferenceGroupAllowed notes on the context that further validation // should allow the KReference.Group, which is disabled by default. -// Note: This API is EXPERIMENTAL and will disappear once the KReference.Group feature will stabilize. +// 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{}{}) From ab610a560f3c9fd2b4ef2c0b7416b8771714eca3 Mon Sep 17 00:00:00 2001 From: slinkydeveloper Date: Mon, 7 Jun 2021 08:57:45 +0200 Subject: [PATCH 6/9] Add omitempty Signed-off-by: Francesco Guardiani --- apis/duck/v1/knative_reference.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apis/duck/v1/knative_reference.go b/apis/duck/v1/knative_reference.go index 93afd6ca08..9258a3bd96 100644 --- a/apis/duck/v1/knative_reference.go +++ b/apis/duck/v1/knative_reference.go @@ -46,7 +46,7 @@ type KReference struct { Name string `json:"name"` // API version of the referent. - APIVersion string `json:"apiVersion"` + 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 From ee55b44c12d5256ca312d570df6da5169752ae53 Mon Sep 17 00:00:00 2001 From: slinkydeveloper Date: Tue, 8 Jun 2021 08:05:05 +0200 Subject: [PATCH 7/9] Moved ResolveGroup code to kref Made ResolveGroup a util method, more than an instance method Signed-off-by: Francesco Guardiani --- apis/duck/v1/knative_reference.go | 46 +------------ apis/duck/v1/knative_reference_test.go | 17 ++++- kref/knative_reference.go | 65 +++++++++++++++++++ .../knative_reference_resolve_group_test.go | 6 +- 4 files changed, 85 insertions(+), 49 deletions(-) create mode 100644 kref/knative_reference.go rename {apis/duck/v1 => kref}/knative_reference_resolve_group_test.go (96%) diff --git a/apis/duck/v1/knative_reference.go b/apis/duck/v1/knative_reference.go index 9258a3bd96..8a61ca2643 100644 --- a/apis/duck/v1/knative_reference.go +++ b/apis/duck/v1/knative_reference.go @@ -19,11 +19,7 @@ package v1 import ( "context" "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" + "strings" "knative.dev/pkg/apis" ) @@ -69,9 +65,9 @@ func (kr *KReference) Validate(ctx context.Context) *apis.FieldError { errs = errs.Also(apis.ErrMissingField("apiVersion")). Also(apis.ErrMissingField("group")) } - if kr.APIVersion != "" && kr.Group != "" { + if kr.APIVersion != "" && kr.Group != "" && !strings.HasPrefix(kr.APIVersion, kr.Group) { errs = errs.Also(&apis.FieldError{ - Message: "both apiVersion and group are specified", + 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", }) @@ -114,42 +110,6 @@ func (kr *KReference) SetDefaults(ctx context.Context) { } } -// 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 (kr *KReference) ResolveGroup(crdLister apiextensionsv1lister.CustomResourceDefinitionLister) error { - if kr.Group == "" { - // Nothing to do here - return nil - } - - actualGvk := schema.GroupVersionKind{Group: kr.Group, Kind: kr.Kind} - pluralGvk, _ := meta.UnsafeGuessKindToResource(actualGvk) - crd, err := crdLister.Get(pluralGvk.GroupResource().String()) - if err != nil { - return err - } - - actualGvk.Version, err = findCRDStorageVersion(crd) - if err != nil { - return err - } - - kr.APIVersion, kr.Kind = actualGvk.ToAPIVersionAndKind() - - return 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) -} - type isGroupAllowed struct{} func isKReferenceGroupAllowed(ctx context.Context) bool { diff --git a/apis/duck/v1/knative_reference_test.go b/apis/duck/v1/knative_reference_test.go index c3dd361988..5af752537f 100644 --- a/apis/duck/v1/knative_reference_test.go +++ b/apis/duck/v1/knative_reference_test.go @@ -101,7 +101,7 @@ func TestValidate(t *testing.T) { ctx: ctx, want: apis.ErrMissingField("apiVersion").Also(apis.ErrDisallowedFields("group")), }, - "invalid ref, group allowed buth both api version and group are configured": { + "invalid ref, group allowed and both api version and group are specified, but they are conflicting": { ref: &KReference{ Namespace: namespace, Name: name, @@ -111,11 +111,22 @@ func TestValidate(t *testing.T) { }, ctx: KReferenceGroupAllowed(ctx), want: &apis.FieldError{ - Message: "both apiVersion and group are specified", + 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, @@ -135,7 +146,7 @@ func TestValidate(t *testing.T) { ctx: KReferenceGroupAllowed(ctx), want: nil, }, - "valid ref, group enabled and but apiVersion configured": { + "valid ref, group enabled but apiVersion configured": { ref: &KReference{ Namespace: namespace, Name: name, diff --git a/kref/knative_reference.go b/kref/knative_reference.go new file mode 100644 index 0000000000..5afd3eb82d --- /dev/null +++ b/kref/knative_reference.go @@ -0,0 +1,65 @@ +/* +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" +) + +// 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 ResolveGroup(kr *duckv1.KReference, crdLister apiextensionsv1lister.CustomResourceDefinitionLister) (*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 := 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) +} diff --git a/apis/duck/v1/knative_reference_resolve_group_test.go b/kref/knative_reference_resolve_group_test.go similarity index 96% rename from apis/duck/v1/knative_reference_resolve_group_test.go rename to kref/knative_reference_resolve_group_test.go index 9d9a73b195..afe1bbcefb 100644 --- a/apis/duck/v1/knative_reference_resolve_group_test.go +++ b/kref/knative_reference_resolve_group_test.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package v1_test +package kref import ( "context" @@ -106,7 +106,7 @@ func TestResolveGroup(t *testing.T) { for name, tc := range tests { t.Run(name, func(t *testing.T) { - err := tc.input.ResolveGroup(fakeCrdInformer.Lister()) + kr, err := ResolveGroup(tc.input, fakeCrdInformer.Lister()) if err != nil { if !tc.wantErr { t.Error("ResolveGroup() =", err) @@ -118,7 +118,7 @@ func TestResolveGroup(t *testing.T) { } if tc.output != nil { - if !cmp.Equal(tc.output, tc.input) { + if !cmp.Equal(tc.output, kr) { t.Errorf("ResolveGroup diff: (-want, +got) =\n%s", cmp.Diff(tc.input, tc.output)) } } From 7303d0bb972f10418fb682e510a8058530cdc45b Mon Sep 17 00:00:00 2001 From: slinkydeveloper Date: Tue, 8 Jun 2021 08:59:06 +0200 Subject: [PATCH 8/9] New type KReferenceResolver Signed-off-by: Francesco Guardiani --- kref/knative_reference.go | 16 ++++++++++++++-- kref/knative_reference_resolve_group_test.go | 2 +- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/kref/knative_reference.go b/kref/knative_reference.go index 5afd3eb82d..14f99c4e67 100644 --- a/kref/knative_reference.go +++ b/kref/knative_reference.go @@ -26,10 +26,22 @@ import ( 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 ResolveGroup(kr *duckv1.KReference, crdLister apiextensionsv1lister.CustomResourceDefinitionLister) (*duckv1.KReference, error) { +func (resolver *KReferenceResolver) ResolveGroup(kr *duckv1.KReference) (*duckv1.KReference, error) { if kr.Group == "" { // Nothing to do here return kr, nil @@ -39,7 +51,7 @@ func ResolveGroup(kr *duckv1.KReference, crdLister apiextensionsv1lister.CustomR actualGvk := schema.GroupVersionKind{Group: kr.Group, Kind: kr.Kind} pluralGvk, _ := meta.UnsafeGuessKindToResource(actualGvk) - crd, err := crdLister.Get(pluralGvk.GroupResource().String()) + crd, err := resolver.crdLister.Get(pluralGvk.GroupResource().String()) if err != nil { return nil, err } diff --git a/kref/knative_reference_resolve_group_test.go b/kref/knative_reference_resolve_group_test.go index afe1bbcefb..ce61e7fa01 100644 --- a/kref/knative_reference_resolve_group_test.go +++ b/kref/knative_reference_resolve_group_test.go @@ -106,7 +106,7 @@ func TestResolveGroup(t *testing.T) { for name, tc := range tests { t.Run(name, func(t *testing.T) { - kr, err := ResolveGroup(tc.input, fakeCrdInformer.Lister()) + kr, err := NewKReferenceResolver(fakeCrdInformer.Lister()).ResolveGroup(tc.input) if err != nil { if !tc.wantErr { t.Error("ResolveGroup() =", err) From 57ab67eab4782bd3066d1917b555c654daa633c7 Mon Sep 17 00:00:00 2001 From: slinkydeveloper Date: Wed, 9 Jun 2021 11:17:38 +0200 Subject: [PATCH 9/9] Added +optional as suggested Signed-off-by: Francesco Guardiani --- apis/duck/v1/knative_reference.go | 1 + 1 file changed, 1 insertion(+) diff --git a/apis/duck/v1/knative_reference.go b/apis/duck/v1/knative_reference.go index 8a61ca2643..a0b169d6f8 100644 --- a/apis/duck/v1/knative_reference.go +++ b/apis/duck/v1/knative_reference.go @@ -42,6 +42,7 @@ type KReference struct { Name string `json:"name"` // API version of the referent. + // +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.