Skip to content

Commit f399d8f

Browse files
golgoth31claude
andauthored
fix(source): handle pointer-slice List items in extractItems (#303)
* fix(source): handle pointer-slice List items in extractItems extractItems took the address of each reflected Items element and asserted client.Object. That assumes value slices ([]T → *T). Generated clients like istio client-go use pointer slices ([]*T); Addr()-ing those yields **T, which is not a client.Object, so the SourceReconciler panicked once the istio Gateway scheme was registered and the list actually returned items: panic: interface conversion: **v1.Gateway is not client.Object: missing method DeepCopyObject Detect the element kind: for []*T use the element directly, for []T take its address. Skip (don't panic) on a non-client.Object element so a stray type can't crash the runnable. Adds white-box tests for both value-slice (corev1) and pointer-slice (istio GatewayList) lists. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(source): surface skipped non-client.Object list elements Review (silent-failure): extractItems now returns how many elements it skipped; the cycle caller logs it. A silent skip would shrink discovery and skew the atomic-wipe guard with no trace — should never happen for registered source types, but no longer invisible if it does. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * chore(lint): use reflect.Pointer instead of deprecated reflect.Ptr Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent fe357a8 commit f399d8f

2 files changed

Lines changed: 92 additions & 6 deletions

File tree

internal/controller/source/cycle.go

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,14 @@ func Cycle(
7474
metrics.SourceErrorsTotal.WithLabelValues(string(kind)).Inc()
7575
continue
7676
}
77-
items := extractItems(list)
77+
items, skipped := extractItems(list)
78+
if skipped > 0 {
79+
// Should never happen for registered source types; surface it rather
80+
// than silently shrink discovery (which would also skew the
81+
// atomic-wipe guard below).
82+
logger.Error(nil, "skipped list elements that are not client.Object",
83+
"kind", kind, "skipped", skipped)
84+
}
7885
entries := make([]domainsource.EnrichedEndpoint, 0, len(items))
7986
resolveErrs := 0
8087
for _, obj := range items {
@@ -154,15 +161,31 @@ func computeEnabledKinds(ctx context.Context, c client.Client) (map[registry.Sou
154161
}
155162

156163
// extractItems extracts client.Object slice from any *List via reflection.
157-
func extractItems(list client.ObjectList) []client.Object {
164+
func extractItems(list client.ObjectList) (items []client.Object, skipped int) {
158165
v := reflect.ValueOf(list).Elem().FieldByName("Items")
159166
if !v.IsValid() || v.Kind() != reflect.Slice {
160-
return nil
167+
return nil, 0
161168
}
162169
out := make([]client.Object, 0, v.Len())
163170
for i := 0; i < v.Len(); i++ {
164-
item := v.Index(i).Addr().Interface().(client.Object)
165-
out = append(out, item)
171+
elem := v.Index(i)
172+
// k8s core lists use value slices ([]T): take the element's address to
173+
// get *T. Other generated clients (e.g. istio client-go) use pointer
174+
// slices ([]*T): the element is already *T. Addr()-ing a pointer slice
175+
// yields **T, which is not a client.Object and used to panic here.
176+
if elem.Kind() != reflect.Pointer {
177+
elem = elem.Addr()
178+
}
179+
obj, ok := elem.Interface().(client.Object)
180+
if !ok {
181+
// Defensive: every source resolver's list element is a client.Object
182+
// once registered in the scheme. Skip rather than panic so a stray
183+
// type can't crash the SourceReconciler runnable; the caller logs
184+
// any skips.
185+
skipped++
186+
continue
187+
}
188+
out = append(out, obj)
166189
}
167-
return out
190+
return out, skipped
168191
}
Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
/*
2+
Copyright 2026.
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 source
18+
19+
import (
20+
"testing"
21+
22+
istionetworkingv1 "istio.io/client-go/pkg/apis/networking/v1"
23+
corev1 "k8s.io/api/core/v1"
24+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
25+
)
26+
27+
// TestExtractItems_ValueSlice covers k8s core lists whose Items is []T.
28+
func TestExtractItems_ValueSlice(t *testing.T) {
29+
list := &corev1.ServiceList{Items: []corev1.Service{
30+
{ObjectMeta: metav1.ObjectMeta{Name: "a"}},
31+
{ObjectMeta: metav1.ObjectMeta{Name: "b"}},
32+
}}
33+
got, skipped := extractItems(list)
34+
if len(got) != 2 || skipped != 0 {
35+
t.Fatalf("want 2 items 0 skipped, got %d items %d skipped", len(got), skipped)
36+
}
37+
if got[0].GetName() != "a" || got[1].GetName() != "b" {
38+
t.Fatalf("unexpected names: %q, %q", got[0].GetName(), got[1].GetName())
39+
}
40+
}
41+
42+
// TestExtractItems_PointerSlice covers generated clients whose Items is []*T
43+
// (e.g. istio client-go). Before the fix, Addr()-ing a *T element yielded **T
44+
// and panicked on the client.Object assertion.
45+
func TestExtractItems_PointerSlice(t *testing.T) {
46+
list := &istionetworkingv1.GatewayList{Items: []*istionetworkingv1.Gateway{
47+
{ObjectMeta: metav1.ObjectMeta{Name: "gw"}},
48+
}}
49+
got, skipped := extractItems(list)
50+
if len(got) != 1 || skipped != 0 {
51+
t.Fatalf("want 1 item 0 skipped, got %d items %d skipped", len(got), skipped)
52+
}
53+
if got[0].GetName() != "gw" {
54+
t.Fatalf("unexpected name: %q", got[0].GetName())
55+
}
56+
}
57+
58+
// TestExtractItems_NoItemsField returns nil rather than panicking.
59+
func TestExtractItems_NoItemsField(t *testing.T) {
60+
if got, _ := extractItems(&corev1.ServiceList{}); len(got) != 0 {
61+
t.Fatalf("want empty for no items, got %d", len(got))
62+
}
63+
}

0 commit comments

Comments
 (0)