CSI volume support: follow-up improvements from PR #552

[hajiler]

PR #552 ("CSI volume support") introduced the initial CSI integration—a CSIDriverConfig CRD, an internal/volume/csi plugin, and control-plane/worker-plane wiring. It landed with seven TODO comments in non-vendored code. This issue tracks them so they aren't lost.

1. Correctness / driver compatibility

• Propagate PublishContext from ControllerPublishVolume to the node mount path
  —

  substrate/internal/volume/csi/plugin.go

  Line 125 in d7629ed

  // TODO: Extend Substrate's VolumePlugin interface to return and propagate

  
  Current behavior: Plugin.AttachVolume discards the response (_ = resp.GetPublishContext()) because the Substrate VolumePlugin interface has nowhere to put it.
  Proposed fix: extend VolumePlugin so AttachVolume returns the publish context, persist it alongside the volume's assignment state, and pass it into NodeStageVolume/NodePublishVolume on the worker.
  Why it matters: drivers that require it (e.g. AWS EBS) cannot successfully mount today, so this gates any real cloud driver beyond the hostpath test driver.

• Support configurable volume access modes instead of hardcoding SINGLE_NODE_WRITER
  —

  substrate/internal/volume/csi/plugin.go

  Line 236 in d7629ed

  // TODO: Support and expose different volume access modes (e.g. ReadWriteMany, ReadOnlyMany)

  
  Current behavior: getStandardCapabilities() returns one hardcoded SINGLE_NODE_WRITER + Mount capability, used for every create, attach, and mount call.
  Proposed fix: add an access-mode field to the volume API type, map it to the corresponding csi.VolumeCapability_AccessMode enum, and validate the requested mode against the driver's reported capabilities. Unblocks ReadWriteMany / ReadOnlyMany volumes shared across actors.

2. Efficiency and log noise

• Query CSI driver capabilities at plugin initialization
  —

  substrate/internal/volume/csi/plugin.go

  Line 113 in d7629ed

  // TODO: Query CSI driver capabilities ahead of time (e.g. during plugin initialization)

  
  Current behavior: AttachVolume optimistically calls ControllerPublishVolume, catches codes.Unimplemented, and logs a warning on every attach for drivers without PUBLISH_UNPUBLISH_VOLUME (including the hostpath driver used in e2e).
  Proposed fix: call ControllerGetCapabilities once during plugin init, cache the capability set on the Plugin, and skip attach/detach entirely when unsupported.

• Reconsider the shared informer used solely for the CSIDriverConfig lister in atelet
  —

  substrate/cmd/atelet/main.go

  Line 221 in d7629ed

  // TODO: Revisit scalability implications of using a shared informer. This lister

  
  Current behavior: atelet spins up a full SharedInformerFactory (resync 0) on every worker node just to obtain Api().V1alpha1().CSIDriverConfigs().Lister(), caching all CSIDriverConfigs cluster-wide per node.
  Proposed fix: given the lister is read infrequently, evaluate a direct client Get with a small TTL cache, or a field/label-filtered (or metadata-only) informer, to reduce per-node memory and API-server watch load.

3. Consistency and API hardening

• Extract a shared volume-plugin lookup helper for control plane and worker plane
  — control plane:

  substrate/cmd/ateapi/internal/controlapi/volumes.go

  Line 145 in d7629ed

  // TODO: Standardize volume plugin lookup and error handling across control plane

  
  — worker plane:

  substrate/cmd/atelet/volumes.go

  Line 69 in d7629ed

  // TODO: Standardize volume plugin lookup and error handling across control plane

  
  Current behavior: deleteActorVolumes (registry.GetPlugin) and unmountExternalVolumes (s.getPlugin) each duplicate plugin resolution and error wrapping with slightly different message formats.
  Proposed fix: add one helper in internal/volume that resolves a plugin by volume type and returns a consistently wrapped error. Constraint: it must preserve the underlying gRPC status code—the worker path relies on status.Code(err) == codes.NotFound to treat an unmount as already completed.

• Harden CSIDriverConfig.ControllerEndpoint validation
  —

  substrate/pkg/api/v1alpha1/csidriverconfig_types.go

  Line 34 in d7629ed

  // TODO: Harden endpoint validation to prevent invalid or unsafe URI inputs.

  
  Current behavior: validated only by +kubebuilder:validation:Pattern= + "^(tcp|dns)://.+$", which accepts effectively any string after the scheme and has no length bound.
  Proposed fix: tighten the CEL/regex validation to a real host:port (or dns:/// target) form, add a MaxLength, and consider a validating webhook for anything the schema can't express. This field determines where the control plane dials, so a permissive value is a request-forgery vector.

[mini-cookie]

Similar to the egress policy discussion — should externalVolume definitions be bound to the Actor (1:1) rather than the ActorTemplate (1:N)?