Fix defaultConfigOverwrite mounting to /etc/barbican/

Deydra71 · claude · Deydra71 · commit 1fa98353aaba · 2026-05-14T07:55:24.000+02:00
The defaultConfigOverwrite field is documented to place files in
/etc/barbican/, but barbican-operator was mounting them into
/etc/barbican/barbican.conf.d/ This prevented overriding files
that upstream barbican hardcodes to /etc/barbican/
(e.g. barbican-api-paste.ini) and was inconsistent with how other
operators (nova, keystone, etc.) handle this field.

Fixed the bug by adding per-file SubPath volume mounts from the existing
config-data-custom Secret into /etc/barbican/&lt;key&gt;. SubPath mounts
are used instead of a directory mount to avoid shadowing existing
files in /etc/barbican/.

For backwards compatibility, each defaultConfigOverwrite key is
mounted at BOTH locations:
  1. /etc/barbican/&lt;key&gt; (correct path per CRD)
  2. /etc/barbican/barbican.conf.d/&lt;key&gt; (old path)

The dual mount preserves existing customer workarounds where e.g.
policy.yaml was referenced via customServiceConfig as:
  [oslo_policy]
  policy_file = /etc/barbican/barbican.conf.d/policy.yaml

Co-authored-by: Claude Opus 4.6 &lt;noreply@anthropic.com&gt;
Signed-off-by: Veronika Fisarova &lt;vfisarov@redhat.com&gt;
diff --git a/api/v1beta1/barbican_types.go b/api/v1beta1/barbican_types.go
@@ -17,11 +17,10 @@ limitations under the License.
 package v1beta1
 
 import (
+	topologyv1 "github.com/openstack-k8s-operators/infra-operator/apis/topology/v1beta1"
 	"github.com/openstack-k8s-operators/lib-common/modules/common/condition"
 	"github.com/openstack-k8s-operators/lib-common/modules/common/util"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	topologyv1 "github.com/openstack-k8s-operators/infra-operator/apis/topology/v1beta1"
-
 )
 
 const (
@@ -103,7 +102,6 @@ type BarbicanSpecBase struct {
 	// +kubebuilder:validation:Optional
 	// ConfigOverwrite - interface to overwrite default config files like e.g. logging.conf or policy.json.
 	// But can also be used to add additional files. Those get added to the service config dir in /etc/<service> .
-	// TODO(dmendiza): -> implement
 	DefaultConfigOverwrite map[string]string `json:"defaultConfigOverwrite,omitempty"`
 
 	// +kubebuilder:validation:Required
diff --git a/api/v1beta1/common_types.go b/api/v1beta1/common_types.go
@@ -100,7 +100,6 @@ type BarbicanComponentTemplate struct {
 	// +kubebuilder:validation:Optional
 	// ConfigOverwrite - interface to overwrite default config files like e.g. policy.json.
 	// But can also be used to add additional files. Those get added to the service config dir in /etc/<service> .
-	// TODO: -> implement
 	DefaultConfigOverwrite map[string]string `json:"defaultConfigOverwrite,omitempty"`
 
 	// +kubebuilder:validation:Optional
diff --git a/internal/barbican/const.go b/internal/barbican/const.go
@@ -58,6 +58,9 @@ const (
 	CustomConfigVolume = "config-data-custom"
 	// CustomConfigMountPoint is the mount point for custom service config
 	CustomConfigMountPoint = "/etc/barbican/barbican.conf.d"
+	// ConfigOverwriteBasePath is the base directory where defaultConfigOverwrite
+	// files are placed via SubPath mounts (e.g. /etc/barbican/policy.yaml and /etc/barbican/api-paste.ini).
+	ConfigOverwriteBasePath = "/etc/barbican"
 	// ScriptVolume is the default volume name used to mount scripts
 	ScriptVolume = "scripts"
 	// ScriptMountPoint is the mount point for scripts
diff --git a/internal/barbican/volumes.go b/internal/barbican/volumes.go
@@ -1,6 +1,9 @@
 package barbican
 
 import (
+	"fmt"
+	"slices"
+
 	barbicanv1beta1 "github.com/openstack-k8s-operators/barbican-operator/api/v1beta1"
 	corev1 "k8s.io/api/core/v1"
 )
@@ -145,6 +148,37 @@ func GetCustomConfigVolumeMount() corev1.VolumeMount {
 	}
 }
 
+// GetConfigOverwriteVolumeMounts returns SubPath volume mounts that place
+// each defaultConfigOverwrite key as an individual file under /etc/barbican/.
+// SubPath mounts are used instead of a directory mount so that existing files
+// in /etc/barbican/ (barbican.conf, api-paste.ini, etc.) are not shadowed.
+// The overwrite data lives in the same config-data-custom Secret (merged via
+// CustomData).
+//
+// Backwards compatibility: the overwrite keys also remain accessible at
+// /etc/barbican/barbican.conf.d/<key> because the config-data-custom Secret
+// is already directory-mounted at /etc/barbican/barbican.conf.d/ by
+// GetCustomConfigVolumeMount. This preserves customer workarounds where e.g.
+// policy.yaml was referenced via customServiceConfig as:
+//
+//	[oslo_policy]
+//	policy_file = /etc/barbican/barbican.conf.d/policy.yaml
+func GetConfigOverwriteVolumeMounts(overwriteKeys []string) []corev1.VolumeMount {
+	mounts := make([]corev1.VolumeMount, 0, len(overwriteKeys))
+	sorted := make([]string, len(overwriteKeys))
+	copy(sorted, overwriteKeys)
+	slices.Sort(sorted)
+	for _, key := range sorted {
+		mounts = append(mounts, corev1.VolumeMount{
+			Name:      CustomConfigVolume,
+			MountPath: fmt.Sprintf("%s/%s", ConfigOverwriteBasePath, key),
+			SubPath:   key,
+			ReadOnly:  true,
+		})
+	}
+	return mounts
+}
+
 // GetDBSyncVolumes - dbsync volumes
 // Unlike the individual Barbican services, the DbSyncJob doesn't need a
 // secret that contains all of the config snippets required by every
diff --git a/internal/barbicanapi/deployment.go b/internal/barbicanapi/deployment.go
@@ -26,6 +26,7 @@ func Deployment(
 	labels map[string]string,
 	annotations map[string]string,
 	topology *topologyv1.Topology,
+	overwriteKeys []string,
 ) (*appsv1.Deployment, error) {
 	envVars := map[string]env.Setter{}
 	envVars["KOLLA_CONFIG_STRATEGY"] = env.SetValue("COPY_ALWAYS")
@@ -58,7 +59,7 @@ func Deployment(
 	}
 	readinessProbe.HTTPGet = livenessProbe.HTTPGet
 
-	apiVolumes, apiVolumeMounts, err := GetAPIVolumesAndMounts(instance)
+	apiVolumes, apiVolumeMounts, err := GetAPIVolumesAndMounts(instance, overwriteKeys)
 	if err != nil {
 		return nil, err
 	}
diff --git a/internal/barbicanapi/volumes.go b/internal/barbicanapi/volumes.go
@@ -11,8 +11,10 @@ import (
 	corev1 "k8s.io/api/core/v1"
 )
 
-// GetAPIVolumesAndMounts returns the volumes and mounts for a BarbicanAPI deployment
-func GetAPIVolumesAndMounts(instance *barbicanv1beta1.BarbicanAPI) ([]corev1.Volume, []corev1.VolumeMount, error) {
+// GetAPIVolumesAndMounts returns the volumes and mounts for a BarbicanAPI deployment.
+// overwriteKeys lists the defaultConfigOverwrite filenames that need SubPath
+// mounts into /etc/barbican/ (e.g. policy.yaml).
+func GetAPIVolumesAndMounts(instance *barbicanv1beta1.BarbicanAPI, overwriteKeys []string) ([]corev1.Volume, []corev1.VolumeMount, error) {
 	apiVolumes := []corev1.Volume{
 		barbican.GetCustomConfigVolume(instance.Name),
 		barbican.GetLogVolume(),
@@ -23,6 +25,7 @@ func GetAPIVolumesAndMounts(instance *barbicanv1beta1.BarbicanAPI) ([]corev1.Vol
 		barbican.GetKollaConfigVolumeMount(instance.Name),
 		barbican.GetLogVolumeMount(),
 	}
+	apiVolumeMounts = append(apiVolumeMounts, barbican.GetConfigOverwriteVolumeMounts(overwriteKeys)...)
 
 	// prepend general config volumes and mounts
 	apiVolumes = append(barbican.GetVolumes("barbican"), apiVolumes...)
diff --git a/internal/barbicankeystonelistener/deployment.go b/internal/barbicankeystonelistener/deployment.go
@@ -24,13 +24,14 @@ func Deployment(
 	labels map[string]string,
 	annotations map[string]string,
 	topology *topologyv1.Topology,
+	overwriteKeys []string,
 ) *appsv1.Deployment {
 	envVars := map[string]env.Setter{}
 	envVars["KOLLA_CONFIG_STRATEGY"] = env.SetValue("COPY_ALWAYS")
 	envVars["CONFIG_HASH"] = env.SetValue(configHash)
 	args := []string{"-c", ServiceCommand}
 
-	keystoneListenerVolumes, keystoneListenerVolumeMounts := GetListenerVolumesAndMounts(instance)
+	keystoneListenerVolumes, keystoneListenerVolumeMounts := GetListenerVolumesAndMounts(instance, overwriteKeys)
 
 	deployment := &appsv1.Deployment{
 		ObjectMeta: metav1.ObjectMeta{
diff --git a/internal/barbicankeystonelistener/volumes.go b/internal/barbicankeystonelistener/volumes.go
@@ -6,8 +6,10 @@ import (
 	corev1 "k8s.io/api/core/v1"
 )
 
-// GetListenerVolumesAndMounts returns the volumes and mounts for a BarbicanKeystoneListener deployment
-func GetListenerVolumesAndMounts(instance *barbicanv1beta1.BarbicanKeystoneListener) ([]corev1.Volume, []corev1.VolumeMount) {
+// GetListenerVolumesAndMounts returns the volumes and mounts for a BarbicanKeystoneListener deployment.
+// overwriteKeys lists the defaultConfigOverwrite filenames that need SubPath
+// mounts into /etc/barbican/ (e.g. policy.yaml).
+func GetListenerVolumesAndMounts(instance *barbicanv1beta1.BarbicanKeystoneListener, overwriteKeys []string) ([]corev1.Volume, []corev1.VolumeMount) {
 	listenerVolumes := []corev1.Volume{
 		barbican.GetCustomConfigVolume(instance.Name),
 		barbican.GetLogVolume(),
@@ -18,6 +20,7 @@ func GetListenerVolumesAndMounts(instance *barbicanv1beta1.BarbicanKeystoneListe
 		barbican.GetKollaConfigVolumeMount(instance.Name),
 		barbican.GetLogVolumeMount(),
 	}
+	listenerVolumeMounts = append(listenerVolumeMounts, barbican.GetConfigOverwriteVolumeMounts(overwriteKeys)...)
 
 	// prepend general config volumes and mounts
 	listenerVolumes = append(barbican.GetVolumes("barbican"), listenerVolumes...)
diff --git a/internal/barbicanworker/deployment.go b/internal/barbicanworker/deployment.go
@@ -26,6 +26,7 @@ func Deployment(
 	labels map[string]string,
 	annotations map[string]string,
 	topology *topologyv1.Topology,
+	overwriteKeys []string,
 ) *appsv1.Deployment {
 	envVars := map[string]env.Setter{}
 	envVars["KOLLA_CONFIG_STRATEGY"] = env.SetValue("COPY_ALWAYS")
@@ -54,7 +55,7 @@ func Deployment(
 	//}
 	//readinessProbe.HTTPGet = livenessProbe.HTTPGet
 
-	workerVolumes, workerVolumeMounts := GetWorkerVolumesAndMounts(instance)
+	workerVolumes, workerVolumeMounts := GetWorkerVolumesAndMounts(instance, overwriteKeys)
 
 	deployment := &appsv1.Deployment{
 		ObjectMeta: metav1.ObjectMeta{
diff --git a/internal/barbicanworker/volumes.go b/internal/barbicanworker/volumes.go
@@ -8,8 +8,10 @@ import (
 	corev1 "k8s.io/api/core/v1"
 )
 
-// GetWorkerVolumesAndMounts returns the volumes and mounts for a BarbicanWorker deployment
-func GetWorkerVolumesAndMounts(instance *barbicanv1beta1.BarbicanWorker) ([]corev1.Volume, []corev1.VolumeMount) {
+// GetWorkerVolumesAndMounts returns the volumes and mounts for a BarbicanWorker deployment.
+// overwriteKeys lists the defaultConfigOverwrite filenames that need SubPath
+// mounts into /etc/barbican/ (e.g. policy.yaml).
+func GetWorkerVolumesAndMounts(instance *barbicanv1beta1.BarbicanWorker, overwriteKeys []string) ([]corev1.Volume, []corev1.VolumeMount) {
 	workerVolumes := []corev1.Volume{
 		barbican.GetCustomConfigVolume(instance.Name),
 		barbican.GetLogVolume(),
@@ -20,6 +22,7 @@ func GetWorkerVolumesAndMounts(instance *barbicanv1beta1.BarbicanWorker) ([]core
 		barbican.GetKollaConfigVolumeMount(instance.Name),
 		barbican.GetLogVolumeMount(),
 	}
+	workerVolumeMounts = append(workerVolumeMounts, barbican.GetConfigOverwriteVolumeMounts(overwriteKeys)...)
 
 	// prepend general config volumes and mounts
 	workerVolumes = append(barbican.GetVolumes("barbican"), workerVolumes...)
diff --git a/internal/controller/barbican_common.go b/internal/controller/barbican_common.go
@@ -102,7 +102,6 @@ func GenerateConfigsGeneric(
 	scripts bool,
 ) error {
 	cms := []util.Template{
-		// Templates where the BarbicanAPI config is stored
 		{
 			Name:          fmt.Sprintf("%s-config-data", instance.GetName()),
 			Namespace:     instance.GetNamespace(),
@@ -126,6 +125,30 @@ func GenerateConfigsGeneric(
 	return secret.EnsureSecrets(ctx, h, instance, cms, envVars)
 }
 
+// collectOverwriteKeys returns the merged set of defaultConfigOverwrite keys
+// from both the parent Barbican CR and the component's own DefaultConfigOverwrite.
+// These keys are used to create per-file SubPath volume mounts into /etc/barbican/.
+// The parentOverwrite map is passed from the already-fetched parent Barbican
+// CR to avoid a redundant API call (generateServiceConfigs already fetches it).
+func collectOverwriteKeys(
+	parentOverwrite map[string]string,
+	componentOverwrite map[string]string,
+) []string {
+	merged := make(map[string]struct{}, len(parentOverwrite)+len(componentOverwrite))
+	for k := range parentOverwrite {
+		merged[k] = struct{}{}
+	}
+	for k := range componentOverwrite {
+		merged[k] = struct{}{}
+	}
+	keys := make([]string, 0, len(merged))
+	for k := range merged {
+		keys = append(keys, k)
+	}
+	slices.Sort(keys)
+	return keys
+}
+
 // GenerateSecretStoreTemplateMap generates a template map for configured secret stores
 func GenerateSecretStoreTemplateMap(
 	enabledSecretStores []barbicanv1beta1.SecretStore,
diff --git a/internal/controller/barbicanapi_controller.go b/internal/controller/barbicanapi_controller.go
@@ -279,22 +279,30 @@ func (r *BarbicanAPIReconciler) createHashOfInputHashes(
 	return hash, changed, nil
 }
 
-// generateServiceConfigs - create Secret which holds the service configuration
+// generateServiceConfigs creates the Secret holding the service configuration.
+// It returns the parent Barbican CR's DefaultConfigOverwrite map so the caller
+// can pass it to collectOverwriteKeys without a redundant API fetch.
 func (r *BarbicanAPIReconciler) generateServiceConfigs(
 	ctx context.Context,
 	h *helper.Helper,
 	instance *barbicanv1beta1.BarbicanAPI,
 	envVars *map[string]env.Setter,
-) error {
+) (map[string]string, error) {
 	Log := r.GetLogger(ctx)
 	Log.Info("generateServiceConfigs - reconciling")
 	labels := labels.GetLabels(instance, labels.GetGroupLabel(barbican.ServiceName), map[string]string{})
 
-	// customData hold any customization for the service.
+	// customData holds any customization for the service.
+	// Keys matching *.conf (CustomServiceConfigFileName, DefaultsConfigFileName, etc.)
+	// are placed into /etc/barbican/barbican.conf.d/ for oslo.config snippet merging.
+	// All other keys (from DefaultConfigOverwrite, e.g. policy.yaml) are placed into
+	// /etc/barbican/ as standalone files via SubPath mounts.
 	customData := map[string]string{
 		barbican.CustomServiceConfigFileName: instance.Spec.CustomServiceConfig,
 	}
 
+	parentOverwrite := map[string]string{}
+
 	// Fetch the two service config snippets (DefaultsConfigFileName and
 	// CustomConfigFileName) from the Secret generated by the top level
 	// barbican controller, and add them to this service specific Secret.
@@ -303,7 +311,7 @@ func (r *BarbicanAPIReconciler) generateServiceConfigs(
 		barbicanSecretName := owner + "-config-data"
 		barbicanSecret, _, err := secret.GetSecret(ctx, h, barbicanSecretName, instance.Namespace)
 		if err != nil {
-			return err
+			return nil, err
 		}
 		customData[barbican.DefaultsConfigFileName] = string(barbicanSecret.Data[barbican.DefaultsConfigFileName])
 		customData[barbican.CustomConfigFileName] = string(barbicanSecret.Data[barbican.CustomConfigFileName])
@@ -320,9 +328,10 @@ func (r *BarbicanAPIReconciler) generateServiceConfigs(
 		ownerInstance := &barbicanv1beta1.Barbican{}
 		err = h.GetClient().Get(ctx, types.NamespacedName{Name: owner, Namespace: instance.Namespace}, ownerInstance)
 		if err != nil {
-			return err
+			return nil, fmt.Errorf("failed to fetch parent Barbican %s/%s: %w", instance.Namespace, owner, err)
 		}
-		maps.Copy(customData, ownerInstance.Spec.DefaultConfigOverwrite)
+		parentOverwrite = ownerInstance.Spec.DefaultConfigOverwrite
+		maps.Copy(customData, parentOverwrite)
 	}
 
 	maps.Copy(customData, instance.Spec.DefaultConfigOverwrite)
@@ -331,7 +340,7 @@ func (r *BarbicanAPIReconciler) generateServiceConfigs(
 	for _, secretName := range instance.Spec.CustomServiceConfigSecrets {
 		secret, _, err := secret.GetSecret(ctx, h, secretName, instance.Namespace)
 		if err != nil {
-			return err
+			return nil, err
 		}
 		for _, key := range slices.Sorted(maps.Keys(secret.Data)) {
 			customSecrets += string(secret.Data[key]) + "\n"
@@ -359,7 +368,7 @@ func (r *BarbicanAPIReconciler) generateServiceConfigs(
 	// This gets overridden in the PKCS11 section below if needed.
 	templateParameters["PKCS11ClientDataPath"] = barbicanv1beta1.DefaultPKCS11ClientDataPath
 
-	return GenerateConfigsGeneric(ctx, h, instance, envVars, templateParameters, customData, labels, false)
+	return parentOverwrite, GenerateConfigsGeneric(ctx, h, instance, envVars, templateParameters, customData, labels, false)
 }
 
 func (r *BarbicanAPIReconciler) reconcileInit(
@@ -729,7 +738,7 @@ func (r *BarbicanAPIReconciler) reconcileNormal(ctx context.Context, instance *b
 	//
 	// create custom config for this barbican service
 	//
-	err = r.generateServiceConfigs(ctx, helper, instance, &configVars)
+	parentOverwrite, err := r.generateServiceConfigs(ctx, helper, instance, &configVars)
 	if err != nil {
 		instance.Status.Conditions.Set(condition.FalseCondition(
 			condition.ServiceConfigReadyCondition,
@@ -853,9 +862,11 @@ func (r *BarbicanAPIReconciler) reconcileNormal(ctx context.Context, instance *b
 		return ctrl.Result{}, fmt.Errorf("waiting for Topology requirements: %w", err)
 	}
 
+	overwriteKeys := collectOverwriteKeys(parentOverwrite, instance.Spec.DefaultConfigOverwrite)
+
 	Log.Info(fmt.Sprintf("[API] Defining deployment '%s'", instance.Name))
 	// Define a new Deployment object
-	deplDef, err := barbicanapi.Deployment(instance, inputHash, serviceLabels, serviceAnnotations, topology)
+	deplDef, err := barbicanapi.Deployment(instance, inputHash, serviceLabels, serviceAnnotations, topology, overwriteKeys)
 	if err != nil {
 		instance.Status.Conditions.Set(condition.FalseCondition(
 			condition.DeploymentReadyCondition,
diff --git a/internal/controller/barbicankeystonelistener_controller.go b/internal/controller/barbicankeystonelistener_controller.go
diff --git a/internal/controller/barbicanworker_controller.go b/internal/controller/barbicanworker_controller.go
diff --git a/test/functional/barbican_controller_test.go b/test/functional/barbican_controller_test.go
