Skip to content

Conversation

tommyp1ckles
Copy link
Contributor

@tommyp1ckles tommyp1ckles commented Nov 27, 2024

pkg/maps/*: export Map{Key,Value} fields to fix cilium-dbg panics.

The cilium-dbg map {get,list} open-api handler will generically try to
unpack bpf map data into their respective key/value pairs.
However, if the Map{Key,Value} structs have unexported fields this will
panic causing the following errors:

level=warning msg="Cilium API handler panicked" client=@ method=GET panic_message="reflect: reflect.Value.SetUint using value obtained using unexported field" subsys=api url=/v1/map

This can be reproduced by enabling a feature that uses such a map, such
as socketLB.enabled=true, followed by doing a map list:

root@kind-worker:/home/cilium# cilium map list
Error: Get "http://localhost/v1/map": EOF

This commit exports any unexported fields in types used as key/values in
pkg/maps to avoid such errors.

Fixes: #33766

@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Nov 27, 2024
@tommyp1ckles
Copy link
Contributor Author

/test

@tommyp1ckles tommyp1ckles force-pushed the pr/tp/fix-api-handler-panic branch from 6b574c9 to 22fcb3d Compare November 28, 2024 00:02
@tommyp1ckles tommyp1ckles changed the title [wip] Fix api handler panic Export Map{Key,Value} fields to prevent map {get,list} handler panics. Nov 28, 2024
@tommyp1ckles
Copy link
Contributor Author

/test

The `cilium-dbg map {get,list}` open-api handler will generically try to
unpack bpf map data into their respective key/value pairs.
However, if the Map{Key,Value} structs have unexported fields this will
panic causing the following errors:

level=warning msg="Cilium API handler panicked" client=@ method=GET panic_message="reflect: reflect.Value.SetUint using value obtained using unexported field" subsys=api url=/v1/map

This can be reproduced by enabling a feature that uses such a map, such
as `socketLB.enabled=true`, followed by doing a map list:

```
root@kind-worker:/home/cilium# cilium map list
Error: Get "http://localhost/v1/map": EOF
```

This commit exports any unexported fields in types used as key/values in
pkg/maps to avoid such errors.

Fixes: #33766

Signed-off-by: Tom Hadlaw <tom.hadlaw@isovalent.com>
@tommyp1ckles tommyp1ckles force-pushed the pr/tp/fix-api-handler-panic branch from 22fcb3d to 0412cb6 Compare November 28, 2024 00:17
@tommyp1ckles
Copy link
Contributor Author

/test

@tommyp1ckles tommyp1ckles added kind/bug This is a bug in the Cilium logic. area/CI Continuous Integration testing issue or flake release-note/bug This PR fixes an issue in a previous release of Cilium. labels Nov 28, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Nov 28, 2024
@tommyp1ckles tommyp1ckles marked this pull request as ready for review November 28, 2024 05:24
@tommyp1ckles tommyp1ckles requested a review from a team as a code owner November 28, 2024 05:24
@tommyp1ckles tommyp1ckles requested a review from jibi November 28, 2024 05:24
@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Nov 29, 2024
@julianwiedmann julianwiedmann added this pull request to the merge queue Nov 29, 2024
@julianwiedmann
Copy link
Member

The linked issue says this also affects v1.16, setting backport label (but also author label, so that @tommyp1ckles can confirm)

@julianwiedmann julianwiedmann added backport/author The backport will be carried out by the author of the PR. needs-backport/1.16 This PR / issue needs backporting to the v1.16 branch labels Nov 29, 2024
Merged via the queue into main with commit 575bee4 Nov 29, 2024
277 checks passed
@julianwiedmann julianwiedmann deleted the pr/tp/fix-api-handler-panic branch November 29, 2024 12:30
@giorio94
Copy link
Member

👋 @tommyp1ckles Backporter here. Double checking whether this PR should be backported to v1.16 or the label should be dropped.

@aanm
Copy link
Member

aanm commented Aug 13, 2025

👋 @tommyp1ckles Backporter here. Double checking whether this PR should be backported to v1.16 or the label should be dropped.

@tommyp1ckles ping

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/CI Continuous Integration testing issue or flake backport/author The backport will be carried out by the author of the PR. kind/bug This is a bug in the Cilium logic. needs-backport/1.16 This PR / issue needs backporting to the v1.16 branch ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/bug This PR fixes an issue in a previous release of Cilium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

panic: reflect: reflect.Value.SetUint using value obtained using unexported field
5 participants