Warn on dynamic config default values with shared structure (#8176)

## What changed?
Log softassert warnings if dynamic config settings are registered with
default values with shared structure.

## Why?
This is very likely unintended and may lead to unexpected behavior of
settings (values will be parsed on top of a copy of the default).

## How did you test it?
- [x] run locally and tested manually
- [x] added new unit test(s)

Author: dnr

cmd/tools/gendynamicconfig/dynamic_config.tmpl

@@ -28,6 +28,11 @@ type {{$P.Name}}TypedConstrainedDefaultSetting[T any] constrainedDefaultSetting[
 // values. The value from dynamic config will be _merged_ over a deep copy of 'def'. Be very careful
 // when using non-empty maps or slices as defaults, the result may not be what you want.
 func New{{$P.Name}}TypedSetting[T any](key Key, def T, description string) {{$P.Name}}TypedSetting[T] {
+	// Warn on any shared structure used with ConvertStructure, even though we handle it by deep copying.
+	warnDefaultSharedStructure(key, def)
+	// If even deep copy won't even work, we should panic early. Do that by calling deep copy once here.
+	_ = deepCopyForMapstructure(def)
+
 	s := {{$P.Name}}TypedSetting[T]{
 		key:         key,
 		def:         def,
@@ -40,6 +45,7 @@ func New{{$P.Name}}TypedSetting[T any](key Key, def T, description string) {{$P.
 
 // New{{$P.Name}}TypedSettingWithConverter creates a setting with a custom converter function.
 func New{{$P.Name}}TypedSettingWithConverter[T any](key Key, convert func(any) (T, error), def T, description string) {{$P.Name}}TypedSetting[T] {
+	{{/* Do not warn on shared structure here, it's the converter's concern. */ -}}
 	s := {{$P.Name}}TypedSetting[T]{
 		key:         key,
 		def:         def,
@@ -52,6 +58,7 @@ func New{{$P.Name}}TypedSettingWithConverter[T any](key Key, convert func(any) (
 
 // New{{$P.Name}}TypedSettingWithConstrainedDefault creates a setting with a compound default value.
 func New{{$P.Name}}TypedSettingWithConstrainedDefault[T any](key Key, convert func(any) (T, error), cdef []TypedConstrainedValue[T], description string) {{$P.Name}}TypedConstrainedDefaultSetting[T] {
+	{{/* Do not warn on shared structure here, it's the converter's concern. */ -}}
 	s := {{$P.Name}}TypedConstrainedDefaultSetting[T]{
 		key:         key,
 		cdef:        cdef,

common/dynamicconfig/collection.go

@@ -104,6 +104,9 @@ var (
 // NewCollection creates a new collection. For subscriptions to work, you must call Start/Stop.
 // Get will work without Start/Stop.
 func NewCollection(client Client, logger log.Logger) *Collection {
+	// Do this at the first convenient place we have a logger:
+	logSharedStructureWarnings(logger)
+
 	return &Collection{
 		client:        client,
 		logger:        logger,
@@ -576,9 +579,6 @@ func convertMap(val any) (map[string]any, error) {
 // treat the fields independently), or the zero value of its type (if you want to treat the fields
 // as a group and default unset fields to zero).
 func ConvertStructure[T any](def T) func(v any) (T, error) {
-	// call deepCopyForMapstructure once to surface any potential panic at static init time.
-	_ = deepCopyForMapstructure(def)
-
 	return func(v any) (T, error) {
 		// if we already have the right type, no conversion is necessary
 		if typedV, ok := v.(T); ok {

common/dynamicconfig/setting_gen.go

@@ -0,0 +1,58 @@
+package dynamicconfig
+
+import (
+	"fmt"
+	"reflect"
+	"sync"
+
+	"go.temporal.io/server/common/log"
+	"go.temporal.io/server/common/softassert"
+)
+
+var (
+	// These should all run at static init time, but use synchronization just in case.
+	sharedStructureWarnings        sync.Map
+	logSharedStructureWarningsOnce sync.Once
+)
+
+func warnDefaultSharedStructure(key Key, def any) {
+	if path := hasSharedStructure(reflect.ValueOf(def), "root"); path != "" {
+		sharedStructureWarnings.Store(key, path)
+	}
+}
+
+func logSharedStructureWarnings(logger log.Logger) {
+	logSharedStructureWarningsOnce.Do(func() {
+		sharedStructureWarnings.Range(func(key, path any) bool {
+			softassert.Fail(logger, fmt.Sprintf("default value for %v contains shared structure at %v", key, path))
+			return true
+		})
+	})
+}
+
+func hasSharedStructure(v reflect.Value, path string) string {
+	// nolint:exhaustive // deliberately not exhaustive
+	switch v.Kind() {
+	case reflect.Map, reflect.Slice, reflect.Pointer:
+		if !v.IsNil() {
+			return path
+		}
+	case reflect.Interface:
+		if !v.IsNil() {
+			return hasSharedStructure(v.Elem(), path)
+		}
+	case reflect.Struct:
+		for i := range v.NumField() {
+			if p := hasSharedStructure(v.Field(i), path+"."+v.Type().Field(i).Name); p != "" {
+				return p
+			}
+		}
+	case reflect.Array:
+		for i := range v.Len() {
+			if p := hasSharedStructure(v.Index(i), path+"."+fmt.Sprintf("[%d]", i)); p != "" {
+				return p
+			}
+		}
+	}
+	return ""
+}

common/dynamicconfig/shared_structure.go

@@ -0,0 +1,41 @@
+package dynamicconfig
+
+import (
+	"reflect"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestHasSharedStructure(t *testing.T) {
+	var v any = 3
+	assert.Empty(t, hasSharedStructure(reflect.ValueOf(v), "root"))
+
+	v = struct {
+		i int
+		s string
+		c struct{ p *string }
+	}{i: 5, s: "hi"}
+	assert.Empty(t, hasSharedStructure(reflect.ValueOf(v), "root"))
+
+	v = [5]*float64{nil, nil, nil, nil, nil}
+	assert.Empty(t, hasSharedStructure(reflect.ValueOf(v), "root"))
+
+	f := float64(35.124)
+	v = [5]*float64{nil, nil, &f, nil, nil}
+	assert.Equal(t, "root.[2]", hasSharedStructure(reflect.ValueOf(v), "root"))
+
+	v = []byte(nil)
+	assert.Empty(t, hasSharedStructure(reflect.ValueOf(v), "root"))
+
+	v = []byte{}
+	assert.Equal(t, "root", hasSharedStructure(reflect.ValueOf(v), "root"))
+
+	str := "test"
+	v = struct {
+		i int
+		s string
+		c struct{ p *string }
+	}{c: struct{ p *string }{p: &str}}
+	assert.Equal(t, "root.c.p", hasSharedStructure(reflect.ValueOf(v), "root"))
+}