Skip to content

Commit 5407df1

Browse files
committed
Refacter code to use new ObjectType
Problem: We currently use the variable objType to refer to a client.Object's type (skeleton of an object). We also use the variable obj to refer to a full client.Object (object with fields filled out). However, in many parts of the codebase these two variables are used closely together and are both of type client.Object which can be a little confusing. Solution: I created a new type called ObjectType and refactored the codebase to utilize it.
1 parent b3e7d39 commit 5407df1

File tree

7 files changed

+24
-19
lines changed

7 files changed

+24
-19
lines changed

internal/framework/controller/reconciler.go

+3-2
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"sigs.k8s.io/controller-runtime/pkg/reconcile"
1414

1515
"github.com/nginxinc/nginx-gateway-fabric/internal/framework/events"
16+
"github.com/nginxinc/nginx-gateway-fabric/internal/framework/kinds"
1617
)
1718

1819
// NamespacedNameFilterFunc is a function that returns true if the resource should be processed by the reconciler.
@@ -24,7 +25,7 @@ type ReconcilerConfig struct {
2425
// Getter gets a resource from the k8s API.
2526
Getter Getter
2627
// ObjectType is the type of the resource that the reconciler will reconcile.
27-
ObjectType client.Object
28+
ObjectType kinds.ObjectType
2829
// EventCh is the channel where the reconciler will send events.
2930
EventCh chan<- interface{}
3031
// NamespacedNameFilter filters resources the controller will process. Can be nil.
@@ -52,7 +53,7 @@ func NewReconciler(cfg ReconcilerConfig) *Reconciler {
5253
}
5354
}
5455

55-
func (r *Reconciler) newObject(objectType client.Object) client.Object {
56+
func (r *Reconciler) newObject(objectType kinds.ObjectType) kinds.ObjectType {
5657
if r.cfg.OnlyMetadata {
5758
partialObj := &metav1.PartialObjectMetadata{}
5859
partialObj.SetGroupVersionKind(objectType.GetObjectKind().GroupVersionKind())

internal/framework/controller/register.go

+3-2
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"sigs.k8s.io/controller-runtime/pkg/predicate"
1313

1414
"github.com/nginxinc/nginx-gateway-fabric/internal/framework/controller/index"
15+
"github.com/nginxinc/nginx-gateway-fabric/internal/framework/kinds"
1516
)
1617

1718
const (
@@ -82,7 +83,7 @@ func defaultConfig() config {
8283
// The registered controller will send events to the provided channel.
8384
func Register(
8485
ctx context.Context,
85-
objectType client.Object,
86+
objectType kinds.ObjectType,
8687
mgr manager.Manager,
8788
eventCh chan<- interface{},
8889
options ...Option,
@@ -137,7 +138,7 @@ func Register(
137138
func addIndex(
138139
ctx context.Context,
139140
indexer client.FieldIndexer,
140-
objectType client.Object,
141+
objectType kinds.ObjectType,
141142
field string,
142143
indexerFunc client.IndexerFunc,
143144
) error {

internal/framework/controller/register_test.go

+1-2
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import (
1313
"k8s.io/apimachinery/pkg/runtime/schema"
1414
"k8s.io/apimachinery/pkg/types"
1515
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
16-
"sigs.k8s.io/controller-runtime/pkg/client"
1716
"sigs.k8s.io/controller-runtime/pkg/client/fake"
1817
"sigs.k8s.io/controller-runtime/pkg/log/zap"
1918
v1 "sigs.k8s.io/gateway-api/apis/v1"
@@ -62,7 +61,7 @@ func TestRegister(t *testing.T) {
6261

6362
tests := []struct {
6463
fakes fakes
65-
objectType client.Object
64+
objectType kinds.ObjectType
6665
expectedErr error
6766
msg string
6867
expectedMgrAddCallCount int

internal/framework/kinds/kinds.go

+3
Original file line numberDiff line numberDiff line change
@@ -46,3 +46,6 @@ func NewMustExtractGKV(scheme *runtime.Scheme) MustExtractGVK {
4646
return gvk
4747
}
4848
}
49+
50+
// ObjectType is used when we only care about the type of client.Object
51+
type ObjectType client.Object

internal/mode/provisioner/manager.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"github.com/nginxinc/nginx-gateway-fabric/internal/framework/controller/predicate"
2121
"github.com/nginxinc/nginx-gateway-fabric/internal/framework/events"
2222
"github.com/nginxinc/nginx-gateway-fabric/internal/framework/gatewayclass"
23+
"github.com/nginxinc/nginx-gateway-fabric/internal/framework/kinds"
2324
"github.com/nginxinc/nginx-gateway-fabric/internal/framework/status"
2425
)
2526

@@ -63,7 +64,7 @@ func StartManager(cfg Config) error {
6364
// Note: for any new object type or a change to the existing one,
6465
// make sure to also update firstBatchPreparer creation below
6566
controllerRegCfgs := []struct {
66-
objectType client.Object
67+
objectType kinds.ObjectType
6768
options []controller.Option
6869
}{
6970
{

internal/mode/static/manager.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -356,7 +356,7 @@ func registerControllers(
356356
controlConfigNSName types.NamespacedName,
357357
) error {
358358
type ctlrCfg struct {
359-
objectType client.Object
359+
objectType kinds.ObjectType
360360
options []controller.Option
361361
}
362362

internal/mode/static/state/store.go

+11-11
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,14 @@ import (
1616
// Updater updates the cluster state.
1717
type Updater interface {
1818
Upsert(obj client.Object)
19-
Delete(objType client.Object, nsname types.NamespacedName)
19+
Delete(objType kinds.ObjectType, nsname types.NamespacedName)
2020
}
2121

2222
// objectStore is a store of client.Object
2323
type objectStore interface {
24-
get(objType client.Object, nsname types.NamespacedName) client.Object
24+
get(objType kinds.ObjectType, nsname types.NamespacedName) client.Object
2525
upsert(obj client.Object)
26-
delete(objType client.Object, nsname types.NamespacedName)
26+
delete(objType kinds.ObjectType, nsname types.NamespacedName)
2727
}
2828

2929
// ngfPolicyObjectStore is a store of policies.Policy.
@@ -44,7 +44,7 @@ func newNGFPolicyObjectStore(
4444
}
4545
}
4646

47-
func (p *ngfPolicyObjectStore) get(objType client.Object, nsname types.NamespacedName) client.Object {
47+
func (p *ngfPolicyObjectStore) get(objType kinds.ObjectType, nsname types.NamespacedName) client.Object {
4848
key := graph.PolicyKey{
4949
NsName: nsname,
5050
GVK: p.extractGVKFunc(objType),
@@ -67,7 +67,7 @@ func (p *ngfPolicyObjectStore) upsert(obj client.Object) {
6767
p.policies[key] = pol
6868
}
6969

70-
func (p *ngfPolicyObjectStore) delete(objType client.Object, nsname types.NamespacedName) {
70+
func (p *ngfPolicyObjectStore) delete(objType kinds.ObjectType, nsname types.NamespacedName) {
7171
key := graph.PolicyKey{
7272
NsName: nsname,
7373
GVK: p.extractGVKFunc(objType),
@@ -88,7 +88,7 @@ func newObjectStoreMapAdapter[T client.Object](objects map[types.NamespacedName]
8888
}
8989
}
9090

91-
func (m *objectStoreMapAdapter[T]) get(_ client.Object, nsname types.NamespacedName) client.Object {
91+
func (m *objectStoreMapAdapter[T]) get(_ kinds.ObjectType, nsname types.NamespacedName) client.Object {
9292
obj, exist := m.objects[nsname]
9393
if !exist {
9494
return nil
@@ -105,7 +105,7 @@ func (m *objectStoreMapAdapter[T]) upsert(obj client.Object) {
105105
m.objects[client.ObjectKeyFromObject(obj)] = t
106106
}
107107

108-
func (m *objectStoreMapAdapter[T]) delete(_ client.Object, nsname types.NamespacedName) {
108+
func (m *objectStoreMapAdapter[T]) delete(_ kinds.ObjectType, nsname types.NamespacedName) {
109109
delete(m.objects, nsname)
110110
}
111111

@@ -150,15 +150,15 @@ func (m *multiObjectStore) mustFindStoreForObj(obj client.Object) objectStore {
150150
return store
151151
}
152152

153-
func (m *multiObjectStore) get(objType client.Object, nsname types.NamespacedName) client.Object {
153+
func (m *multiObjectStore) get(objType kinds.ObjectType, nsname types.NamespacedName) client.Object {
154154
return m.mustFindStoreForObj(objType).get(objType, nsname)
155155
}
156156

157157
func (m *multiObjectStore) upsert(obj client.Object) {
158158
m.mustFindStoreForObj(obj).upsert(obj)
159159
}
160160

161-
func (m *multiObjectStore) delete(objType client.Object, nsname types.NamespacedName) {
161+
func (m *multiObjectStore) delete(objType kinds.ObjectType, nsname types.NamespacedName) {
162162
m.mustFindStoreForObj(objType).delete(objType, nsname)
163163
}
164164

@@ -257,7 +257,7 @@ func (s *changeTrackingUpdater) Upsert(obj client.Object) {
257257
s.setChangeType(obj, changingUpsert)
258258
}
259259

260-
func (s *changeTrackingUpdater) delete(objType client.Object, nsname types.NamespacedName) (changed bool) {
260+
func (s *changeTrackingUpdater) delete(objType kinds.ObjectType, nsname types.NamespacedName) (changed bool) {
261261
objTypeGVK := s.extractGVK(objType)
262262

263263
if s.store.persists(objTypeGVK) {
@@ -276,7 +276,7 @@ func (s *changeTrackingUpdater) delete(objType client.Object, nsname types.Names
276276
return stateChanged.delete(objType, nsname)
277277
}
278278

279-
func (s *changeTrackingUpdater) Delete(objType client.Object, nsname types.NamespacedName) {
279+
func (s *changeTrackingUpdater) Delete(objType kinds.ObjectType, nsname types.NamespacedName) {
280280
s.assertSupportedGVK(s.extractGVK(objType))
281281

282282
changingDelete := s.delete(objType, nsname)

0 commit comments

Comments
 (0)