fix(source): handle pointer-slice List items in extractItems#303
Merged
Conversation
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>
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>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
After the scheme fix (#301), the manager panicked once the Istio source actually returned items:
extractItemsdidv.Index(i).Addr().Interface().(client.Object). That works for value slices ([]T→Addr()→*T), but generated clients like istio client-go use pointer slices (GatewayList.Items []*Gateway);Addr()-ing a*Telement yields**T, which is not aclient.Object→ panic, crashing the SourceReconciler runnable.Fix
[]*Tuse the element directly; for[]Ttake its address.client.Objectelement so a stray type can't crash the runnable, and return the skip count so the caller logs it (a silent skip would shrink discovery and skew the atomic-wipe guard —logger.Erroris the existing convention in this file).Adds white-box tests for value-slice (
corev1.ServiceList), pointer-slice (istio GatewayList, which reproduced the panic), and the no-Items case.Test plan
TestExtractItems_PointerSlice(istio) — was panicking, now passesTestExtractItems_ValueSlice(corev1),TestExtractItems_NoItemsFieldsourcecycle tests pass;go test -race,golangci-lintclean🤖 Generated with Claude Code