Skip to content

Commit 1b34ab1

Browse files
author
Kate Osborn
committed
Add panic for GVK
1 parent 974f503 commit 1b34ab1

File tree

2 files changed

+48
-17
lines changed

2 files changed

+48
-17
lines changed

internal/framework/controller/register.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,9 @@ func Register(
107107

108108
var forOpts []ctlrBuilder.ForOption
109109
if cfg.onlyMetadata {
110+
if objectType.GetObjectKind().GroupVersionKind().Empty() {
111+
panic("the object must have its GVK set")
112+
}
110113
forOpts = append(forOpts, ctlrBuilder.OnlyMetadata)
111114
}
112115

internal/framework/controller/register_test.go

Lines changed: 45 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,10 @@ import (
1010
"github.com/onsi/gomega/gcustom"
1111
gtypes "github.com/onsi/gomega/types"
1212
"k8s.io/apimachinery/pkg/runtime"
13+
"k8s.io/apimachinery/pkg/runtime/schema"
1314
"k8s.io/apimachinery/pkg/types"
1415
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
16+
"sigs.k8s.io/controller-runtime/pkg/client"
1517
"sigs.k8s.io/controller-runtime/pkg/client/fake"
1618
"sigs.k8s.io/controller-runtime/pkg/log/zap"
1719
v1 "sigs.k8s.io/gateway-api/apis/v1"
@@ -50,14 +52,24 @@ func TestRegister(t *testing.T) {
5052

5153
testError := errors.New("test error")
5254

55+
objectTypeWithGVK := &v1.HTTPRoute{}
56+
objectTypeWithGVK.SetGroupVersionKind(
57+
schema.GroupVersionKind{Group: v1.GroupName, Version: "v1", Kind: "HTTPRoute"},
58+
)
59+
60+
objectTypeNoGVK := &v1.HTTPRoute{}
61+
5362
tests := []struct {
5463
fakes fakes
64+
objectType client.Object
5565
expectedErr error
5666
msg string
5767
expectedMgrAddCallCount int
68+
expectPanic bool
5869
}{
5970
{
6071
fakes: getDefaultFakes(),
72+
objectType: objectTypeWithGVK,
6173
expectedErr: nil,
6274
expectedMgrAddCallCount: 1,
6375
msg: "normal case",
@@ -67,6 +79,7 @@ func TestRegister(t *testing.T) {
6779
f.indexer.IndexFieldReturns(testError)
6880
return f
6981
}(getDefaultFakes()),
82+
objectType: objectTypeWithGVK,
7083
expectedErr: testError,
7184
expectedMgrAddCallCount: 0,
7285
msg: "preparing index fails",
@@ -76,13 +89,20 @@ func TestRegister(t *testing.T) {
7689
f.mgr.AddReturns(testError)
7790
return f
7891
}(getDefaultFakes()),
92+
objectType: objectTypeWithGVK,
7993
expectedErr: testError,
8094
expectedMgrAddCallCount: 1,
8195
msg: "building controller fails",
8296
},
97+
{
98+
fakes: getDefaultFakes(),
99+
objectType: objectTypeNoGVK,
100+
expectPanic: true,
101+
expectedMgrAddCallCount: 0,
102+
msg: "adding OnlyMetadata option panics",
103+
},
83104
}
84105

85-
objectType := &v1.HTTPRoute{}
86106
nsNameFilter := func(nsname types.NamespacedName) (bool, string) {
87107
return true, ""
88108
}
@@ -104,28 +124,36 @@ func TestRegister(t *testing.T) {
104124

105125
newReconciler := func(c controller.ReconcilerConfig) *controller.Reconciler {
106126
g.Expect(c.Getter).To(BeIdenticalTo(test.fakes.mgr.GetClient()))
107-
g.Expect(c.ObjectType).To(BeIdenticalTo(objectType))
127+
g.Expect(c.ObjectType).To(BeIdenticalTo(test.objectType))
108128
g.Expect(c.EventCh).To(BeIdenticalTo(eventCh))
109129
g.Expect(c.NamespacedNameFilter).Should(beSameFunctionPointer(nsNameFilter))
110130

111131
return controller.NewReconciler(c)
112132
}
113133

114-
err := controller.Register(
115-
context.Background(),
116-
objectType,
117-
test.fakes.mgr,
118-
eventCh,
119-
controller.WithNamespacedNameFilter(nsNameFilter),
120-
controller.WithK8sPredicate(predicate.ServicePortsChangedPredicate{}),
121-
controller.WithFieldIndices(fieldIndexes),
122-
controller.WithNewReconciler(newReconciler),
123-
)
124-
125-
if test.expectedErr == nil {
126-
g.Expect(err).To(BeNil())
134+
register := func() error {
135+
return controller.Register(
136+
context.Background(),
137+
test.objectType,
138+
test.fakes.mgr,
139+
eventCh,
140+
controller.WithNamespacedNameFilter(nsNameFilter),
141+
controller.WithK8sPredicate(predicate.ServicePortsChangedPredicate{}),
142+
controller.WithFieldIndices(fieldIndexes),
143+
controller.WithNewReconciler(newReconciler),
144+
controller.WithOnlyMetadata(),
145+
)
146+
}
147+
148+
if test.expectPanic {
149+
g.Expect(func() { _ = register() }).To(Panic())
127150
} else {
128-
g.Expect(err).To(MatchError(test.expectedErr))
151+
err := register()
152+
if test.expectedErr == nil {
153+
g.Expect(err).To(BeNil())
154+
} else {
155+
g.Expect(err).To(MatchError(test.expectedErr))
156+
}
129157
}
130158

131159
indexCallCount := test.fakes.indexer.IndexFieldCallCount()
@@ -134,7 +162,7 @@ func TestRegister(t *testing.T) {
134162

135163
_, objType, field, indexFunc := test.fakes.indexer.IndexFieldArgsForCall(0)
136164

137-
g.Expect(objType).To(BeIdenticalTo(objectType))
165+
g.Expect(objType).To(BeIdenticalTo(test.objectType))
138166
g.Expect(field).To(BeIdenticalTo(index.KubernetesServiceNameIndexField))
139167

140168
expectedIndexFunc := fieldIndexes[index.KubernetesServiceNameIndexField]

0 commit comments

Comments
 (0)