restart: getRefConfigMapRefs returns after the first ConfigMap, so only one is ever watched

[whg517]

getRefConfigMapRefs uses return append(...) instead of configMaps = append(...), so it returns as soon as it finds the first ConfigMap reference and never looks at the rest.

internal/controller/restart/statefulset_controller.go:122-147 — all three loops:

func (h *StatefulSetHandler) getRefConfigMapRefs() []string {
	configMaps := make([]string, 0)
	for _, volume := range h.Sts.Spec.Template.Spec.Volumes {
		if volume.ConfigMap != nil {
			return append(configMaps, volume.ConfigMap.Name)   // L126 — returns
		}
	}
	for _, container := range h.Sts.Spec.Template.Spec.InitContainers {
		for _, env := range container.Env {
			if env.ValueFrom != nil && env.ValueFrom.ConfigMapKeyRef != nil {
				return append(configMaps, env.ValueFrom.ConfigMapKeyRef.Name)   // L133 — returns
			}
		}
	}
	for _, container := range h.Sts.Spec.Template.Spec.Containers {
		for _, env := range container.Env {
			if env.ValueFrom != nil && env.ValueFrom.ConfigMapKeyRef != nil {
				return append(configMaps, env.ValueFrom.ConfigMapKeyRef.Name)   // L141 — returns
			}
		}
	}
	return configMaps
}

The Secret collector immediately above it is correct, which makes the asymmetry easy to see — getRefSecretRefs accumulates at L102, L108 and L115 with secrets = append(secrets, ...).

Impact

A StatefulSet that references more than one ConfigMap gets a restart annotation for only the first one. Changes to every other ConfigMap it mounts are silently ignored — the user has labelled the workload restarter.kubedoop.dev/enable=true and reasonably believes config changes now roll their pods, but for those ConfigMaps they never will.

The failure is silent in both directions: nothing logs, and the pods keep running the old configuration.

It also short-circuits between categories, not just within one: a workload with a ConfigMap volume never has its env configMapKeyRef references examined at all, because the volume loop returns first.

Why it survived

Every case in statefulset_controller_test.go uses a single ConfigMap, so the accumulation is never exercised. A test with two ConfigMap volumes on one StatefulSet fails immediately.

Note for operator-go

Raised while confirming, for operator-go, exactly what the restarter contract guarantees. The SDK's own role group mounts exactly one ConfigMap (the config volume), so the framework's default path is unaffected — but a product operator that mounts an additional ConfigMap would hit this, and operator-go's documentation would be promising something the restarter does not deliver.

🤖 Generated with Claude Code