refactor: lift effective-props resolver into pkg/effectiveprops (Phase 10.1)

Moves the Controller → RG → RD → Resource hierarchy resolver out
of `internal/controller.ResourceReconciler` into a shared
`pkg/effectiveprops` package so the upcoming satellite-side
controller-runtime reconcilers can use the same code path
without duplicating 80 lines of merge logic.

`pkg/effectiveprops.Resolve(ctx, client.Reader, target, rd)` is
the single entry point — takes any controller-runtime reader
(both controller's mgr.GetClient() and satellite's). Internals
unchanged from the previous private implementation:

  - typed `Spec.DRBDOptions` via drbd.ResolveDRBDOptions across
    ControllerConfig → RG → RD → Resource scopes;
  - legacy Spec.Props via drbd.ResolveOptions for forward-compat;
  - ExtraProps + ControllerConfig.Spec.ExtraProps overlay.

`internal/controller.resolveEffectiveProps` becomes a one-line
wrapper. The KVEntry-shaped legacy `controllerProps` reader is
now exposed as `effectiveprops.LegacyControllerProps` so the
csi-volumes filter test can pin its instance-filter invariant
directly against the package. Old test file moves alongside.

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Andrei Kvapil <kvapss@gmail.com>

Author: kvaps

internal/controller/export_test.go

@@ -84,14 +84,6 @@ func (r *ResourceDefinitionReconciler) SetQuorum(ctx context.Context, rd *blocks
 	return r.setQuorum(ctx, rd, value)
 }
 
-// ControllerProps exposes the cluster-KV ControllerProps reader.
-// Tests pin the instance-filter so KVEntry rows belonging to other
-// instances (e.g. csi-volumes) don't leak into the prop bag the
-// dispatcher hands the satellite.
-func (r *ResourceReconciler) ControllerProps(ctx context.Context) (map[string]string, error) {
-	return r.controllerProps(ctx)
-}
-
 // LookupRD exposes the soft-fail RD fetcher. Tests pin the
 // (nil, nil) return on NotFound so the dispatcher can still set up
 // connection state when the RD is being deleted concurrently.

internal/controller/resource_controller.go

@@ -18,7 +18,6 @@ package controller
 
 import (
 	"context"
-	"maps"
 	"slices"
 	"time"
 
@@ -35,6 +34,7 @@ import (
 	apiv1 "github.com/cozystack/blockstor/pkg/api/v1"
 	"github.com/cozystack/blockstor/pkg/dispatcher"
 	"github.com/cozystack/blockstor/pkg/drbd"
+	"github.com/cozystack/blockstor/pkg/effectiveprops"
 	"github.com/cozystack/blockstor/pkg/store"
 )
 
@@ -795,116 +795,14 @@ func (r *ResourceReconciler) takenOnNode(ctx context.Context, nodeName string, p
 	return out, nil
 }
 
-// resolveEffectiveProps walks the controller → RG → RD → Resource
-// scopes and returns the merged DRBD-options bag the dispatcher
-// hands to the satellite. Lower scopes override upper. Each scope is
-// best-effort: a missing controller-props KV instance, missing RG
-// reference, or missing RD all silently degrade to "empty" rather
-// than block the dispatch — the resource can still come up with
-// satellite-only defaults.
-//
-// As of Phase 10.3 step 5 the typed `Spec.DRBDOptions` is the source
-// of truth — we resolve via `drbd.ResolveDRBDOptions`, then flatten
-// to a string Props map for the satellite proto (typed proto comes
-// in a later step). Legacy `Spec.Props` keys + `ExtraProps` are
-// merged in too so a partially-migrated cluster keeps rendering
-// the same .res file.
+// resolveEffectiveProps delegates to the shared `pkg/effectiveprops`
+// package (Phase 10.1 lift-out) so the satellite-side reconciler
+// can use the same hierarchy resolution without duplicating
+// 80 lines of merge logic. The wrapper survives because the
+// existing call sites are `r.resolveEffectiveProps(ctx, target, rd)`
+// and we don't want to churn them.
 func (r *ResourceReconciler) resolveEffectiveProps(ctx context.Context, target *blockstoriov1alpha1.Resource, rdPtr *blockstoriov1alpha1.ResourceDefinition) (map[string]string, error) {
-	controllerProps, err := r.controllerProps(ctx)
-	if err != nil {
-		return nil, err
-	}
-
-	ctrlCfg, err := r.controllerConfig(ctx)
-	if err != nil {
-		return nil, err
-	}
-
-	var (
-		ctrlTyped  *blockstoriov1alpha1.DRBDOptions
-		ctrlExtras map[string]string
-		rgProps    map[string]string
-		rdProps    map[string]string
-		rgTyped    *blockstoriov1alpha1.DRBDOptions
-		rdTyped    *blockstoriov1alpha1.DRBDOptions
-		rgExtras   map[string]string
-		rdExtras   map[string]string
-	)
-
-	if ctrlCfg != nil {
-		ctrlTyped = ctrlCfg.Spec.DRBDOptions
-		ctrlExtras = ctrlCfg.Spec.ExtraProps
-	}
-
-	if rdPtr != nil {
-		rdProps = rdPtr.Spec.Props
-		rdTyped = rdPtr.Spec.DRBDOptions
-		rdExtras = rdPtr.Spec.ExtraProps
-
-		if rdPtr.Spec.ResourceGroupName != "" {
-			var rg blockstoriov1alpha1.ResourceGroup
-
-			err := r.Get(ctx, client.ObjectKey{Name: rdPtr.Spec.ResourceGroupName}, &rg)
-			switch {
-			case err == nil:
-				rgProps = rg.Spec.Props
-				rgTyped = rg.Spec.DRBDOptions
-				rgExtras = rg.Spec.ExtraProps
-			case errors.IsNotFound(err):
-				// Soft-fail: the RG was deleted out from under
-				// this RD. Skip the level rather than refuse to
-				// dispatch — the rest of the hierarchy still
-				// produces a usable .res.
-			default:
-				return nil, err
-			}
-		}
-	}
-
-	// Resolve typed options through the same scope hierarchy as the
-	// legacy Props resolver. Lower scope wins per non-nil field.
-	// Phase 10.4: ControllerConfig.Spec.DRBDOptions is the typed
-	// controller scope. The legacy KVEntry-shaped controllerProps
-	// remains for forward-compat with any pre-migration cluster.
-	typed := drbd.ResolveDRBDOptions(ctrlTyped, rgTyped, rdTyped, target.Spec.DRBDOptions)
-
-	// Flatten typed → string Props so the existing satellite-side
-	// renderer keeps working. Then layer on the legacy Props
-	// hierarchy (controller → RG → RD → Resource) so any keys still
-	// living on Spec.Props (residual non-DRBD or pre-migration data)
-	// or in ExtraProps still feed into the .res file. Lower scope
-	// wins on conflict, matching the typed resolver's semantics.
-	out := drbd.ResolveOptions(controllerProps, rgProps, rdProps, target.Spec.Props)
-
-	maps.Copy(out, drbd.TypedDRBDOptionsToProps(typed))
-	maps.Copy(out, ctrlExtras)
-	maps.Copy(out, rgExtras)
-	maps.Copy(out, rdExtras)
-	maps.Copy(out, target.Spec.ExtraProps)
-
-	return out, nil
-}
-
-// controllerConfig fetches the singleton ControllerConfig CRD. We
-// only recognise the canonical name `default`; any other instance
-// is silently ignored so a stale or duplicated ControllerConfig
-// can't quietly take effect. Missing CRD or missing object both
-// return (nil, nil) — the reconciler falls through to legacy
-// KVEntry-based controllerProps for forward-compat with
-// pre-migration clusters.
-func (r *ResourceReconciler) controllerConfig(ctx context.Context) (*blockstoriov1alpha1.ControllerConfig, error) {
-	var cfg blockstoriov1alpha1.ControllerConfig
-
-	err := r.Get(ctx, client.ObjectKey{Name: blockstoriov1alpha1.ControllerConfigName}, &cfg)
-	if err != nil {
-		if errors.IsNotFound(err) {
-			return nil, nil //nolint:nilnil // optional singleton — nil means "not configured"
-		}
-
-		return nil, err
-	}
-
-	return &cfg, nil
+	return effectiveprops.Resolve(ctx, r.Client, target, rdPtr)
 }
 
 // resolveLayerStack walks the RD → RG hierarchy and returns the
@@ -933,35 +831,6 @@ func (r *ResourceReconciler) resolveLayerStack(ctx context.Context, rd *blocksto
 	return rg.Spec.SelectFilter.LayerStack
 }
 
-// controllerProps reads the cluster-wide ControllerProps KV instance
-// via the KVEntry CRD. Empty when no entries exist (fresh cluster).
-//
-// We list every KVEntry and filter by Instance in-process; an
-// indexed `client.MatchingFields{"spec.instance": "ControllerProps"}`
-// would be cheaper but the controller-runtime cache rejects field
-// selectors that aren't pre-registered as field indexers, and the
-// extra wiring isn't worth it for a small KV store.
-func (r *ResourceReconciler) controllerProps(ctx context.Context) (map[string]string, error) {
-	var list blockstoriov1alpha1.KVEntryList
-
-	err := r.List(ctx, &list)
-	if err != nil {
-		return nil, err
-	}
-
-	out := map[string]string{}
-
-	for i := range list.Items {
-		if list.Items[i].Spec.Instance != "ControllerProps" {
-			continue
-		}
-
-		out[list.Items[i].Spec.Key] = list.Items[i].Spec.Value
-	}
-
-	return out, nil
-}
-
 // EnsureDRBDIDsForTest is an exported alias for ensureDRBDIDs. The
 // allocator is package-private because it's an internal reconciler
 // step, but the property tests live in package controller_test and

pkg/effectiveprops/effective.go

@@ -0,0 +1,194 @@
+/*
+Copyright 2026 Cozystack contributors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+// Package effectiveprops resolves the DRBD-options bag for one
+// Resource by walking the upstream-LINSTOR override hierarchy:
+// Controller → ResourceGroup → ResourceDefinition → Resource.
+//
+// Lower scopes override upper, per non-nil field. Each scope is
+// best-effort — a missing ControllerConfig / missing RG /
+// missing RD degrades to "empty" rather than blocking the
+// dispatch, so a partially-migrated cluster still produces a
+// usable .res file.
+//
+// Lifted out of `internal/controller.ResourceReconciler` in
+// Phase 10.1 so both the controller-side dispatcher AND the new
+// satellite-side `pkg/satellite/controllers.ResourceReconciler`
+// share one implementation.
+package effectiveprops
+
+import (
+	"context"
+	"maps"
+
+	"github.com/cockroachdb/errors"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+
+	blockstoriov1alpha1 "github.com/cozystack/blockstor/api/v1alpha1"
+	"github.com/cozystack/blockstor/pkg/drbd"
+)
+
+// LegacyControllerPropsInstance is the KVEntry-instance name
+// the pre-Phase-10.4 `ControllerProps` used. Read-only fallback
+// — once every cluster has migrated to `ControllerConfig` this
+// path stops firing and the KVEntry CRD can go.
+const LegacyControllerPropsInstance = "ControllerProps"
+
+// Resolve walks the four scopes and returns the merged Props
+// map. The `c` reader is typically a controller-runtime
+// `manager.GetClient()` — both controller and satellite hold
+// one. `target` is the Resource whose effective props we want;
+// `rd` is the parent ResourceDefinition (may be nil; the
+// reconciler usually fetched it already).
+//
+// Phase 10.1 step.
+func Resolve(ctx context.Context, c client.Reader, target *blockstoriov1alpha1.Resource, rd *blockstoriov1alpha1.ResourceDefinition) (map[string]string, error) {
+	if target == nil {
+		return map[string]string{}, nil
+	}
+
+	controllerProps, err := LegacyControllerProps(ctx, c)
+	if err != nil {
+		return nil, err
+	}
+
+	ctrlCfg, err := controllerConfig(ctx, c)
+	if err != nil {
+		return nil, err
+	}
+
+	var (
+		ctrlTyped  *blockstoriov1alpha1.DRBDOptions
+		ctrlExtras map[string]string
+	)
+
+	if ctrlCfg != nil {
+		ctrlTyped = ctrlCfg.Spec.DRBDOptions
+		ctrlExtras = ctrlCfg.Spec.ExtraProps
+	}
+
+	rgInfo, rdInfo, err := scopeInputs(ctx, c, rd)
+	if err != nil {
+		return nil, err
+	}
+
+	typed := drbd.ResolveDRBDOptions(ctrlTyped, rgInfo.Typed, rdInfo.Typed, target.Spec.DRBDOptions)
+
+	out := drbd.ResolveOptions(controllerProps, rgInfo.Props, rdInfo.Props, target.Spec.Props)
+
+	maps.Copy(out, drbd.TypedDRBDOptionsToProps(typed))
+	maps.Copy(out, ctrlExtras)
+	maps.Copy(out, rgInfo.Extras)
+	maps.Copy(out, rdInfo.Extras)
+	maps.Copy(out, target.Spec.ExtraProps)
+
+	return out, nil
+}
+
+// scopeInputs gathers the RG + RD scope inputs the hierarchy
+// resolver needs. Returns zero-valued info structs for missing
+// scopes — a missing RG / RD softly degrades to "no input at
+// this level" rather than blocking dispatch.
+type scopeInfo struct {
+	Props  map[string]string
+	Typed  *blockstoriov1alpha1.DRBDOptions
+	Extras map[string]string
+}
+
+func scopeInputs(ctx context.Context, c client.Reader, rd *blockstoriov1alpha1.ResourceDefinition) (scopeInfo, scopeInfo, error) {
+	var (
+		rgInfo scopeInfo
+		rdInfo scopeInfo
+	)
+
+	if rd == nil {
+		return rgInfo, rdInfo, nil
+	}
+
+	rdInfo = scopeInfo{
+		Props:  rd.Spec.Props,
+		Typed:  rd.Spec.DRBDOptions,
+		Extras: rd.Spec.ExtraProps,
+	}
+
+	if rd.Spec.ResourceGroupName == "" {
+		return rgInfo, rdInfo, nil
+	}
+
+	var rgObj blockstoriov1alpha1.ResourceGroup
+
+	getErr := c.Get(ctx, client.ObjectKey{Name: rd.Spec.ResourceGroupName}, &rgObj)
+	switch {
+	case getErr == nil:
+		rgInfo = scopeInfo{
+			Props:  rgObj.Spec.Props,
+			Typed:  rgObj.Spec.DRBDOptions,
+			Extras: rgObj.Spec.ExtraProps,
+		}
+	case apierrors.IsNotFound(getErr):
+		// Soft-fail; see package doc.
+	default:
+		return rgInfo, rdInfo, errors.Wrapf(getErr, "get ResourceGroup %q", rd.Spec.ResourceGroupName)
+	}
+
+	return rgInfo, rdInfo, nil
+}
+
+// controllerConfig fetches the singleton ControllerConfig CRD.
+// Returns (nil, nil) when missing — caller falls through to
+// the legacy KVEntry path. Phase 10.4 step 1.
+func controllerConfig(ctx context.Context, c client.Reader) (*blockstoriov1alpha1.ControllerConfig, error) {
+	var cfg blockstoriov1alpha1.ControllerConfig
+
+	err := c.Get(ctx, client.ObjectKey{Name: blockstoriov1alpha1.ControllerConfigName}, &cfg)
+	if err != nil {
+		if apierrors.IsNotFound(err) {
+			return nil, nil //nolint:nilnil // optional singleton
+		}
+
+		return nil, errors.Wrap(err, "get ControllerConfig")
+	}
+
+	return &cfg, nil
+}
+
+// LegacyControllerProps reads the KVEntry-shaped ControllerProps
+// instance and returns its flat `{key: value}` view. Drained as
+// more keys migrate to ControllerConfig's typed fields;
+// eventually disappears with the KVEntry CRD. Exported so the
+// controller_props test can pin the instance-filter invariant
+// directly against the package.
+func LegacyControllerProps(ctx context.Context, c client.Reader) (map[string]string, error) {
+	var list blockstoriov1alpha1.KVEntryList
+
+	err := c.List(ctx, &list)
+	if err != nil {
+		return nil, errors.Wrap(err, "list KVEntries")
+	}
+
+	out := map[string]string{}
+
+	for i := range list.Items {
+		if list.Items[i].Spec.Instance != LegacyControllerPropsInstance {
+			continue
+		}
+
+		out[list.Items[i].Spec.Key] = list.Items[i].Spec.Value
+	}
+
+	return out, nil
+}