Skip to content

fix(reconciler): avoid panic converting unstructured objects#29

Merged
moshloop merged 1 commit intomainfrom
fix/unstructured-conversion-panic
Mar 13, 2026
Merged

fix(reconciler): avoid panic converting unstructured objects#29
moshloop merged 1 commit intomainfrom
fix/unstructured-conversion-panic

Conversation

@adityathebe
Copy link
Member

@adityathebe adityathebe commented Mar 13, 2026

runtime.DefaultUnstructuredConverter.FromUnstructured(...) uses reflection to populate every struct field of the destination type as it walks the object graph.

If it encounters an unexported field (lowercase field name), reflection cannot assign to it, and it panics with:

reflect.Value.Set using value obtained using unexported field


Our CRDs have unexported fields like

  • view.query.http.awsConfig
  • playbook.timeout

So the unstructured converter was not “wrong” generally; it’s just incompatible with typed graphs containing private fields in this decode path.

resolves: flanksource/mission-control#2848
resolves: flanksource/mission-control#2864

@adityathebe
Copy link
Member Author

@coderabbitai review

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the reconciler’s unstructured→typed decode path to avoid controller panics caused by reflection attempting to assign into unexported (lowercase) struct fields in CRD Go types.

Changes:

  • Replaces runtime.DefaultUnstructuredConverter.FromUnstructured(...) with a JSON marshal→unmarshal conversion to avoid reflection panics on unexported fields.
  • Adds error handling for marshal failures and continues emitting “MalformedResource” warning events on conversion errors.
Comments suppressed due to low confidence (1)

reconciler.go:110

  • This change is intended to prevent panics caused by unexported fields during unstructured→typed conversion, but the current test suite only covers a schema-shape mismatch (map vs array headers). Add a test case that includes an unexported (lowercase) field in a typed CRD struct (with a corresponding field present in the unstructured object) to ensure reconciliation no longer panics and the controller keeps running.
	obj := PT(new(T))
	payload, err := json.Marshal(raw.Object)
	if err != nil {
		logger.Errorf("[kopper] malformed resource %s: %v", resourceName, err)
		r.Events.Event(raw, "Warning", "MalformedResource",
			fmt.Sprintf("Resource spec does not match expected schema: %v", err))
		return ctrl.Result{}, fmt.Errorf("failed to marshal unstructured resource: %w", err)
	}

	if err := json.Unmarshal(payload, obj); err != nil {
		logger.Errorf("[kopper] malformed resource %s: %v", resourceName, err)
		r.Events.Event(raw, "Warning", "MalformedResource",
			fmt.Sprintf("Resource spec does not match expected schema: %v", err))
		return ctrl.Result{}, fmt.Errorf("failed to convert unstructured to typed object: %w", err)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +101 to +102
r.Events.Event(raw, "Warning", "MalformedResource",
fmt.Sprintf("Resource spec does not match expected schema: %v", err))
@moshloop moshloop reopened this Mar 13, 2026
@moshloop moshloop merged commit 51acea2 into main Mar 13, 2026
12 checks passed
@moshloop moshloop deleted the fix/unstructured-conversion-panic branch March 13, 2026 12:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Playbook Reconcile Error Some views are not reconciled

3 participants