Skip to content
This repository has been archived by the owner on Aug 28, 2024. It is now read-only.

Require ListOptions when SkipOwnerReference is true #493

Merged
merged 1 commit into from
Mar 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion reconcilers/child.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,13 +121,19 @@ type ChildReconciler[Type, ChildType client.Object, ChildListType client.ObjectL
MergeBeforeUpdate func(current, desired ChildType)

// ListOptions allows custom options to be use when listing potential child resources. Each
// resource retrieved as part of the listing is confirmed via OurChild.
// resource retrieved as part of the listing is confirmed via OurChild. There is a performance
// benefit to limiting the number of resource return for each List operation, however,
// excluding an actual child will orphan that resource.
//
// Defaults to filtering by the reconciled resource's namespace:
// []client.ListOption{
// client.InNamespace(resource.GetNamespace()),
// }
//
// ListOptions is required when a Finalizer is defined or SkipOwnerReference is true. An empty
// list is often sufficient although it may incur a performance penalty, especially when
// querying the API sever instead of an informer cache.
//
// +optional
ListOptions func(ctx context.Context, resource Type) []client.ListOption

Expand Down Expand Up @@ -229,6 +235,11 @@ func (r *ChildReconciler[T, CT, CLT]) validate(ctx context.Context) error {
return fmt.Errorf("ChildReconciler %q must implement OurChild since owner references are not used", r.Name)
}

if r.ListOptions == nil && r.SkipOwnerReference {
// ListOptions is required when SkipOwnerReference is true
return fmt.Errorf("ChildReconciler %q must implement ListOptions since owner references are not used", r.Name)
}

// require MergeBeforeUpdate
if r.MergeBeforeUpdate == nil {
return fmt.Errorf("ChildReconciler %q must implement MergeBeforeUpdate", r.Name)
Expand Down
13 changes: 12 additions & 1 deletion reconcilers/childset.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,13 +116,19 @@ type ChildSetReconciler[Type, ChildType client.Object, ChildListType client.Obje
MergeBeforeUpdate func(current, desired ChildType)

// ListOptions allows custom options to be use when listing potential child resources. Each
// resource retrieved as part of the listing is confirmed via OurChild.
// resource retrieved as part of the listing is confirmed via OurChild. There is a performance
// benefit to limiting the number of resource return for each List operation, however,
// excluding an actual child will orphan that resource.
//
// Defaults to filtering by the reconciled resource's namespace:
// []client.ListOption{
// client.InNamespace(resource.GetNamespace()),
// }
//
// ListOptions is required when a Finalizer is defined or SkipOwnerReference is true. An empty
// list is often sufficient although it may incur a performance penalty, especially when
// querying the API sever instead of an informer cache.
//
// +optional
ListOptions func(ctx context.Context, resource Type) []client.ListOption

Expand Down Expand Up @@ -257,6 +263,11 @@ func (r *ChildSetReconciler[T, CT, CLT]) validate(ctx context.Context) error {
return fmt.Errorf("ChildSetReconciler %q must implement OurChild since owner references are not used", r.Name)
}

if r.ListOptions == nil && r.SkipOwnerReference {
// ListOptions is required when SkipOwnerReference is true
return fmt.Errorf("ChildSetReconciler %q must implement ListOptions since owner references are not used", r.Name)
}

// require IdentifyChild
if r.IdentifyChild == nil {
return fmt.Errorf("ChildSetReconciler %q must implement IdentifyChild", r.Name)
Expand Down
33 changes: 33 additions & 0 deletions reconcilers/validate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -463,6 +463,22 @@ func TestChildReconciler_validate(t *testing.T) {
ListOptions: func(ctx context.Context, parent *corev1.ConfigMap) []client.ListOption { return []client.ListOption{} },
},
},
{
name: "ListOptions missing",
parent: &corev1.ConfigMap{},
reconciler: &ChildReconciler[*corev1.ConfigMap, *corev1.Pod, *corev1.PodList]{
Name: "ListOptions missing",
ChildType: &corev1.Pod{},
ChildListType: &corev1.PodList{},
DesiredChild: func(ctx context.Context, parent *corev1.ConfigMap) (*corev1.Pod, error) { return nil, nil },
ReflectChildStatusOnParent: func(ctx context.Context, parent *corev1.ConfigMap, child *corev1.Pod, err error) {},
MergeBeforeUpdate: func(current, desired *corev1.Pod) {},
SkipOwnerReference: true,
// ListOptions: func(ctx context.Context, parent *corev1.ConfigMap) []client.ListOption { return []client.ListOption{} },
OurChild: func(resource *corev1.ConfigMap, child *corev1.Pod) bool { return true },
},
shouldErr: `ChildReconciler "ListOptions missing" must implement ListOptions since owner references are not used`,
},
{
name: "Finalizer without OurChild",
parent: &corev1.ConfigMap{},
Expand Down Expand Up @@ -622,6 +638,23 @@ func TestChildSetReconciler_validate(t *testing.T) {
IdentifyChild: func(child *corev1.Pod) string { return "" },
},
},
{
name: "ListOptions missing",
parent: &corev1.ConfigMap{},
reconciler: &ChildSetReconciler[*corev1.ConfigMap, *corev1.Pod, *corev1.PodList]{
Name: "ListOptions missing",
ChildType: &corev1.Pod{},
ChildListType: &corev1.PodList{},
DesiredChildren: func(ctx context.Context, parent *corev1.ConfigMap) ([]*corev1.Pod, error) { return nil, nil },
ReflectChildrenStatusOnParent: func(ctx context.Context, parent *corev1.ConfigMap, result ChildSetResult[*corev1.Pod]) {},
MergeBeforeUpdate: func(current, desired *corev1.Pod) {},
SkipOwnerReference: true,
// ListOptions: func(ctx context.Context, parent *corev1.ConfigMap) []client.ListOption { return []client.ListOption{} },
OurChild: func(resource *corev1.ConfigMap, child *corev1.Pod) bool { return true },
IdentifyChild: func(child *corev1.Pod) string { return "" },
},
shouldErr: `ChildSetReconciler "ListOptions missing" must implement ListOptions since owner references are not used`,
},
{
name: "Finalizer without OurChild",
parent: &corev1.ConfigMap{},
Expand Down
Loading