cell: Decode config fields implementing encoding.TextUnmarshaler - #79
Merged
Merged
Conversation
Config struct fields whose type implements `encoding.TextUnmarshaler`, such as `netip.Addr`, `netip.Prefix` or `time.Time`, currently fail to decode with `"expected a map, got 'string'"`. Each such type needs a bespoke decode hook passed in via `DecodeHooks`. There's currently one in cilium/cilium that handles `netip.Prefix`, see https://github.com/cilium/cilium/blob/4c38f075cad5e7e7b54744c3c613f4e10fcdac0b/pkg/hive/hive.go#L145-L146 This PR adds the more generic `mapstructure.TextUnmarshallerHookFunc()` to the default decode hooks so all of them work out of the box. It is placed before the slice hooks: `StringToSliceHookFunc` matches on the target's `reflect.Kind`, so it would otherwise comma-split types that are themselves slices, such as `net.IP`. Signed-off-by: Hadrien Patte <hadrien.patte@datadoghq.com>
joamaki
approved these changes
Aug 21, 2026
HadrienPatte
added a commit
that referenced
this pull request
Sep 29, 2026
The [mitchellh/mapstructure](https://github.com/mitchellh/mapstructure) package is archived and no longer receives updates (see mitchellh/mapstructure#349). The blessed fork (see https://gist.github.com/mitchellh/90029601268e59a29e64e55bab1c5bdc) is [go-viper/mapstructure](https://github.com/go-viper/mapstructure), which viper itself has used since v1.20. This migration was attempted before in #49 and reverted in #53 because it broke Cilium's `TestNodeAddressWhitelist`. The API is almost the same, but go-viper/mapstructure#6 changed `StringToSliceHookFunc` so it only fires when the target type is exactly `[]string`. The mitchellh version fired for any slice kind. As a result, a comma-separated string coming from the environment or a configmap is no longer split before being decoded into slices of other element types, such as `[]netip.Prefix`. Each element then gets the whole unsplit string. Swapping only the import path, as #49 did, silently changes this behavior. Use `StringToWeakSliceHookFunc` instead. go-viper/mapstructure provides it to bring back the pre-v2 behavior, and its implementation matches the mitchellh `StringToSliceHookFunc`. Hook ordering is unchanged: `TextUnmarshallerHookFunc` still runs before the slice split, so types like `net.IP` (`[]byte`) are not split on commas. `TestHiveTextUnmarshaler` (added in #79, after the revert) covers the case that broke Cilium: it decodes "10.0.0.0/8,192.168.0.0/16" from the configmap into a `[]netip.Prefix`. It fails with a plain import swap and passes with this change. Signed-off-by: Hadrien Patte <hadrien.patte@datadoghq.com>
meefs
pushed a commit
to meefs/cilium
that referenced
this pull request
Sep 30, 2026
`cilium/hive` v1.0.6 includes cilium/hive#79, which adds `mapstructure.TextUnmarshallerHookFunc()` to the default config decode hooks. Config fields of any type implementing `encoding.TextUnmarshaler`, including `netip.Prefix`, now decode out of the box. The default hooks run before the extra hooks passed via `Options.DecodeHooks`, so the `netip.Prefix` hook in `pkg/hive` never sees a string input anymore and is dead code. Remove it, along with the associated TODO, which waited on go-viper/mapstructure#85 and is now obsolete. Signed-off-by: Hadrien Patte <hadrien.patte@datadoghq.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
2 participants
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.
Config struct fields whose type implements
encoding.TextUnmarshaler, such asnetip.Addr,netip.Prefixortime.Time, currently fail to decode with"expected a map, got 'string'". Each such type needs a bespoke decode hook passed in viaDecodeHooks. There's currently one in cilium/cilium that handlesnetip.Prefix.This PR adds the more generic
mapstructure.TextUnmarshallerHookFunc()to the default decode hooks so all of them work out of the box. It is placed before the slice hooks:StringToSliceHookFuncmatches on the target'sreflect.Kind, so it would otherwise comma-split types that are themselves slices, such asnet.IP.Note: This PR addresses this TODO in a more generic way than expressed in the TODO comment and will allow us to delete that bespoke decode hook once hive is bumped in cilium/cilium to a version that includes this PR.