Skip to content

Commit 192b0c9

Browse files
slinkydeveloperslinkydeveloper
authored
Add KReference.Group field and ResolveGroup function (#2127)
* Add Group field and ResolveGroup function Signed-off-by: Francesco Guardiani <[email protected]> * Remove core special case Signed-off-by: Francesco Guardiani <[email protected]> * Copyright Signed-off-by: Francesco Guardiani <[email protected]> * Added validation code Signed-off-by: Francesco Guardiani <[email protected]> * Fix comment Signed-off-by: Francesco Guardiani <[email protected]> * Add omitempty Signed-off-by: Francesco Guardiani <[email protected]> * Moved ResolveGroup code to kref Made ResolveGroup a util method, more than an instance method Signed-off-by: Francesco Guardiani <[email protected]> * New type KReferenceResolver Signed-off-by: Francesco Guardiani <[email protected]> * Added +optional as suggested Signed-off-by: Francesco Guardiani <[email protected]>
1 parent b2bf37c commit 192b0c9

File tree

4 files changed

+314
-3
lines changed

4 files changed

+314
-3
lines changed

‎apis/duck/v1/knative_reference.go‎

Lines changed: 41 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package v1
1919
import (
2020
"context"
2121
"fmt"
22+
"strings"
2223

2324
"knative.dev/pkg/apis"
2425
)
@@ -41,7 +42,13 @@ type KReference struct {
4142
Name string `json:"name"`
4243

4344
// API version of the referent.
44-
APIVersion string `json:"apiVersion"`
45+
// +optional
46+
APIVersion string `json:"apiVersion,omitempty"`
47+
48+
// 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.
49+
// Note: This API is EXPERIMENTAL and might break anytime. For more details: https://github.com/knative/eventing/issues/5086
50+
// +optional
51+
Group string `json:"group,omitempty"`
4552
}
4653

4754
func (kr *KReference) Validate(ctx context.Context) *apis.FieldError {
@@ -54,8 +61,25 @@ func (kr *KReference) Validate(ctx context.Context) *apis.FieldError {
5461
if kr.Name == "" {
5562
errs = errs.Also(apis.ErrMissingField("name"))
5663
}
57-
if kr.APIVersion == "" {
58-
errs = errs.Also(apis.ErrMissingField("apiVersion"))
64+
if isKReferenceGroupAllowed(ctx) {
65+
if kr.APIVersion == "" && kr.Group == "" {
66+
errs = errs.Also(apis.ErrMissingField("apiVersion")).
67+
Also(apis.ErrMissingField("group"))
68+
}
69+
if kr.APIVersion != "" && kr.Group != "" && !strings.HasPrefix(kr.APIVersion, kr.Group) {
70+
errs = errs.Also(&apis.FieldError{
71+
Message: "both apiVersion and group are specified and they refer to different API groups",
72+
Paths: []string{"apiVersion", "group"},
73+
Details: "Only one of them must be specified",
74+
})
75+
}
76+
} else {
77+
if kr.Group != "" {
78+
errs = errs.Also(apis.ErrDisallowedFields("group"))
79+
}
80+
if kr.APIVersion == "" {
81+
errs = errs.Also(apis.ErrMissingField("apiVersion"))
82+
}
5983
}
6084
if kr.Kind == "" {
6185
errs = errs.Also(apis.ErrMissingField("kind"))
@@ -86,3 +110,17 @@ func (kr *KReference) SetDefaults(ctx context.Context) {
86110
kr.Namespace = apis.ParentMeta(ctx).Namespace
87111
}
88112
}
113+
114+
type isGroupAllowed struct{}
115+
116+
func isKReferenceGroupAllowed(ctx context.Context) bool {
117+
return ctx.Value(isGroupAllowed{}) != nil
118+
}
119+
120+
// KReferenceGroupAllowed notes on the context that further validation
121+
// should allow the KReference.Group, which is disabled by default.
122+
// Note: This API is EXPERIMENTAL and may disappear once the KReference.Group feature will stabilize.
123+
// For more details: https://github.com/knative/eventing/issues/5086
124+
func KReferenceGroupAllowed(ctx context.Context) context.Context {
125+
return context.WithValue(ctx, isGroupAllowed{}, struct{}{})
126+
}

‎apis/duck/v1/knative_reference_test.go‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,10 @@ import (
2525
"knative.dev/pkg/apis"
2626
)
2727

28+
const (
29+
group = "group.my"
30+
)
31+
2832
func TestValidate(t *testing.T) {
2933
ctx := context.Background()
3034

@@ -87,6 +91,71 @@ func TestValidate(t *testing.T) {
8791
Details: `parent namespace: "diffns" does not match ref: "b-namespace"`,
8892
},
8993
},
94+
"invalid ref, disallowed group": {
95+
ref: &KReference{
96+
Namespace: namespace,
97+
Name: name,
98+
Kind: kind,
99+
Group: group,
100+
},
101+
ctx: ctx,
102+
want: apis.ErrMissingField("apiVersion").Also(apis.ErrDisallowedFields("group")),
103+
},
104+
"invalid ref, group allowed and both api version and group are specified, but they are conflicting": {
105+
ref: &KReference{
106+
Namespace: namespace,
107+
Name: name,
108+
Kind: kind,
109+
Group: group,
110+
APIVersion: apiVersion,
111+
},
112+
ctx: KReferenceGroupAllowed(ctx),
113+
want: &apis.FieldError{
114+
Message: "both apiVersion and group are specified and they refer to different API groups",
115+
Paths: []string{"apiVersion", "group"},
116+
Details: "Only one of them must be specified",
117+
},
118+
},
119+
"invalid ref, group allowed and both api version and group are specified": {
120+
ref: &KReference{
121+
Namespace: namespace,
122+
Name: name,
123+
Kind: kind,
124+
Group: "eventing.knative.dev",
125+
APIVersion: "eventing.knative.dev/v1",
126+
},
127+
ctx: KReferenceGroupAllowed(ctx),
128+
want: nil,
129+
},
130+
"valid ref, group enabled and both apiVersion and group missing": {
131+
ref: &KReference{
132+
Namespace: namespace,
133+
Name: name,
134+
Kind: kind,
135+
},
136+
ctx: KReferenceGroupAllowed(ctx),
137+
want: apis.ErrMissingField("apiVersion").Also(apis.ErrMissingField("group")),
138+
},
139+
"valid ref, group enabled and configured": {
140+
ref: &KReference{
141+
Namespace: namespace,
142+
Name: name,
143+
Kind: kind,
144+
Group: group,
145+
},
146+
ctx: KReferenceGroupAllowed(ctx),
147+
want: nil,
148+
},
149+
"valid ref, group enabled but apiVersion configured": {
150+
ref: &KReference{
151+
Namespace: namespace,
152+
Name: name,
153+
Kind: kind,
154+
APIVersion: apiVersion,
155+
},
156+
ctx: KReferenceGroupAllowed(ctx),
157+
want: nil,
158+
},
90159
"valid ref, mismatched namespaces, but overridden": {
91160
ref: &KReference{
92161
Namespace: namespace,

‎kref/knative_reference.go‎

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
/*
2+
Copyright 2020 The Knative Authors
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package kref
18+
19+
import (
20+
"fmt"
21+
22+
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
23+
apiextensionsv1lister "k8s.io/apiextensions-apiserver/pkg/client/listers/apiextensions/v1"
24+
"k8s.io/apimachinery/pkg/api/meta"
25+
"k8s.io/apimachinery/pkg/runtime/schema"
26+
duckv1 "knative.dev/pkg/apis/duck/v1"
27+
)
28+
29+
// KReferenceResolver is an object that resolves the KReference.Group field
30+
// Note: This API is EXPERIMENTAL and might break anytime. For more details: https://github.com/knative/eventing/issues/5086
31+
type KReferenceResolver struct {
32+
crdLister apiextensionsv1lister.CustomResourceDefinitionLister
33+
}
34+
35+
// NewKReferenceResolver creates a new KReferenceResolver from a crdLister
36+
// Note: This API is EXPERIMENTAL and might break anytime. For more details: https://github.com/knative/eventing/issues/5086
37+
func NewKReferenceResolver(crdLister apiextensionsv1lister.CustomResourceDefinitionLister) *KReferenceResolver {
38+
return &KReferenceResolver{crdLister: crdLister}
39+
}
40+
41+
// ResolveGroup resolves the APIVersion of a KReference starting from the Group.
42+
// In order to execute this method, you need RBAC to read the CRD of the Resource referred in this KReference.
43+
// Note: This API is EXPERIMENTAL and might break anytime. For more details: https://github.com/knative/eventing/issues/5086
44+
func (resolver *KReferenceResolver) ResolveGroup(kr *duckv1.KReference) (*duckv1.KReference, error) {
45+
if kr.Group == "" {
46+
// Nothing to do here
47+
return kr, nil
48+
}
49+
50+
kr = kr.DeepCopy()
51+
52+
actualGvk := schema.GroupVersionKind{Group: kr.Group, Kind: kr.Kind}
53+
pluralGvk, _ := meta.UnsafeGuessKindToResource(actualGvk)
54+
crd, err := resolver.crdLister.Get(pluralGvk.GroupResource().String())
55+
if err != nil {
56+
return nil, err
57+
}
58+
59+
actualGvk.Version, err = findCRDStorageVersion(crd)
60+
if err != nil {
61+
return nil, err
62+
}
63+
64+
kr.APIVersion, kr.Kind = actualGvk.ToAPIVersionAndKind()
65+
66+
return kr, nil
67+
}
68+
69+
// This function runs under the assumption that there must be exactly one "storage" version
70+
func findCRDStorageVersion(crd *apiextensionsv1.CustomResourceDefinition) (string, error) {
71+
for _, version := range crd.Spec.Versions {
72+
if version.Storage {
73+
return version.Name, nil
74+
}
75+
}
76+
return "", fmt.Errorf("this CRD %s doesn't have a storage version! Kubernetes, you're drunk, go home", crd.Name)
77+
}

‎kref/knative_reference_resolve_group_test.go‎

Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
/*
2+
Copyright 2021 The Knative Authors
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package kref
18+
19+
import (
20+
"context"
21+
"testing"
22+
23+
"github.com/google/go-cmp/cmp"
24+
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
25+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
26+
"k8s.io/client-go/rest"
27+
28+
. "knative.dev/pkg/apis/duck/v1"
29+
customresourcedefinitioninformer "knative.dev/pkg/client/injection/apiextensions/informers/apiextensions/v1/customresourcedefinition/fake"
30+
"knative.dev/pkg/injection"
31+
)
32+
33+
func TestResolveGroup(t *testing.T) {
34+
const crdGroup = "messaging.knative.dev"
35+
const crdName = "inmemorychannels." + crdGroup
36+
37+
ctx, _ := injection.Fake.SetupInformers(context.TODO(), &rest.Config{})
38+
39+
fakeCrdInformer := customresourcedefinitioninformer.Get(ctx)
40+
fakeCrdInformer.Informer().GetIndexer().Add(
41+
&apiextensionsv1.CustomResourceDefinition{
42+
ObjectMeta: metav1.ObjectMeta{
43+
Name: crdName,
44+
},
45+
Spec: apiextensionsv1.CustomResourceDefinitionSpec{
46+
Versions: []apiextensionsv1.CustomResourceDefinitionVersion{{
47+
Name: "v1beta1",
48+
Storage: false,
49+
}, {
50+
Name: "v1",
51+
Storage: true,
52+
}},
53+
},
54+
},
55+
)
56+
57+
tests := map[string]struct {
58+
input *KReference
59+
output *KReference
60+
wantErr bool
61+
}{
62+
"No group": {
63+
input: &KReference{
64+
Kind: "Abc",
65+
Name: "123",
66+
APIVersion: "something/v1",
67+
},
68+
output: &KReference{
69+
Kind: "Abc",
70+
Name: "123",
71+
APIVersion: "something/v1",
72+
},
73+
},
74+
"No group nor api version": {
75+
input: &KReference{
76+
Kind: "Abc",
77+
Name: "123",
78+
},
79+
output: &KReference{
80+
Kind: "Abc",
81+
Name: "123",
82+
},
83+
},
84+
"imc channel": {
85+
input: &KReference{
86+
Kind: "InMemoryChannel",
87+
Name: "my-cool-channel",
88+
Group: crdGroup,
89+
},
90+
output: &KReference{
91+
Kind: "InMemoryChannel",
92+
Name: "my-cool-channel",
93+
Group: crdGroup,
94+
APIVersion: crdGroup + "/v1",
95+
},
96+
},
97+
"unknown CRD": {
98+
input: &KReference{
99+
Kind: "MyChannel",
100+
Name: "my-cool-channel",
101+
Group: crdGroup,
102+
},
103+
wantErr: true,
104+
},
105+
}
106+
107+
for name, tc := range tests {
108+
t.Run(name, func(t *testing.T) {
109+
kr, err := NewKReferenceResolver(fakeCrdInformer.Lister()).ResolveGroup(tc.input)
110+
if err != nil {
111+
if !tc.wantErr {
112+
t.Error("ResolveGroup() =", err)
113+
}
114+
return
115+
} else if tc.wantErr {
116+
t.Errorf("ResolveGroup() = %v, wanted error", err)
117+
return
118+
}
119+
120+
if tc.output != nil {
121+
if !cmp.Equal(tc.output, kr) {
122+
t.Errorf("ResolveGroup diff: (-want, +got) =\n%s", cmp.Diff(tc.input, tc.output))
123+
}
124+
}
125+
})
126+
}
127+
}

0 commit comments

Comments
 (0)