feat(validation): enrich domain and core validation of user-supplied values

[ezekiel-alexrod]

Summary

User-supplied entry values reaching the write path were not fully validated: the
cache-policy enums accepted arbitrary strings, out-of-range RAID levels slipped
through, a nil CacheOptions panicked, and empty drive lists were passed
straight to the adapters. This tightens validation in the shared domain and in
core, and aligns the megaraid v1 adapter with the resulting optional-cache
contract, so invalid input is rejected up front with a clean error instead of
reaching a controller CLI (or panicking).

Changes

Domain (pkg/domain/entities/logicalvolume)

• Add IsValid() whitelists for ReadPolicy (ra/nora), WritePolicy
  (wt/wb/awb), IOPolicy (direct/cached) and RAIDLevel (0/1/10).
  Arbitrary strings and out-of-range levels are now rejected — previously only the
  Unknown sentinel was.
• CacheOptions.Validate() is now nil-safe (returns an error instead of
  panicking). Read and write policies must be valid; the IO policy is optional
  (unset/Unknown accepted, since not every controller supports it — storcli2
  dropped it entirely) and only rejected when set to an unrecognized value.
• Request.Validate() rejects out-of-range RAID levels and treats CacheOptions
  as optional (nil = controller defaults), matching the adapters which emit no
  cache flags for a nil value.
• checkRAIDRequirement gains a default branch; ValidateRAIDCreation uses
  RAIDLevel.IsValid().

Core (pkg/core)

• AddPDsToLV / DeletePDsFromLV reject an empty physical-drive list
  (ErrNoPhysicalDrives) before delegating — uniform across all adapters, rather
  than relying on each adapter to catch it.

Adapter — megaraid v1 (pkg/implementation/raidcontroller/megaraid)

• Build the add vd cache flags through a nil-safe, fail-closed helper
  (megaraidCreateCacheFlags): a nil CacheOptions emits no flags (was a
  nil-pointer dereference now that cache options are optional) and any policy
  that does not map is rejected instead of emitted verbatim. Unlike storcli2,
  the megaraid v1 add vd command requires all three policies, so an unset or
  Unknown IO policy is rejected here as well.
• In the cache setter, set the IO policy only when the caller provided a valid,
  differing value; an unset or Unknown value is left unchanged.

Testing

• New table-driven tests for the enum IsValid() methods and for
  CacheOptions.Validate / Request.Validate (the domain had no tests).
• First core test: verifies the empty-drive-list guards through an embedded
  ports.RAIDController stub (the guard returns before any adapter call), pinned
  to ErrNoPhysicalDrives with require.ErrorIs.
• New internal test for megaraidCreateCacheFlags (nil, all-valid, unset/Unknown
  IO omitted, invalid read/write fail closed).
• go test ./pkg/..., go vet, golangci-lint and gofmt are all clean; no
  regressions.

Issue: ARTESCA-17807

[claude]

• pkg/domain/entities/logicalvolume/methods.go:60 — unnecessary //nolint:funlen directive; the function is ~36 lines, under the 50-line threshold
• pkg/core/raidcontroller_test.go:37 — tests use require.Error instead of require.ErrorIs(t, err, core.ErrNoPhysicalDrives); the weaker assertion would pass if a different guard returned an error

Review by Claude Code

[claude]

• pkg/core/raidcontroller_test.go:37,47 — Test assertions use require.Error instead of require.ErrorIs(t, err, core.ErrNoPhysicalDrives), so they would pass even if the wrong validation check fires. Pin to the sentinel.
  - pkg/domain/entities/logicalvolume/methods.go:59 — Unnecessary //nolint:funlen directive; function is ~37 lines, well under the 50-line limit configured in .golangci.yaml.
  
  Overall the PR is well-structured: the IsValid() whitelists, nil-safe CacheOptions.Validate, optional IO policy, early empty-drive guards in core, and thorough table-driven tests all look correct. The domain/port/adapter boundaries are respected — validation stays in the domain and core layers without leaking adapter concerns.
  
  Review by Claude Code

[claude]

• pkg/domain/entities/logicalvolume/methods.go:85-87 — Making CacheOptions optional in Request.Validate() opens a nil-pointer dereference path through the legacy MegaRAID adapter (pkg/implementation/raidcontroller/megaraid/logicalvolume.go:307-309), which dereferences request.CacheOptions unconditionally in createLV. The storcli2 adapter handles nil correctly, but the MegaRAID adapter needs a nil guard or early return to match.
  
  Review by Claude Code

[claude]

LGTM — the validation tightening is clean and well-tested.

- Enum IsValid() whitelists, nil-safe CacheOptions.Validate, and the optional-IOPolicy logic are all correct. The IO-policy condition (methods.go:33) threads the needle between rejecting garbage and accepting unset/Unknown precisely.
- megaraidCreateCacheFlags fails closed on bad read/write policies and omits IO policy when not IsValid(), consistent with the storcli2 adapter's storcli2CreateCacheFlags and storcli2CacheOptions.
- setLVCacheOptions IO-policy guard (logicalvolume.go:485) correctly prevents emitting Unknown/empty as a CLI flag.
- The early return for nil CacheOptions in Request.Validate (methods.go:85-87) fixes the panic path and matches how the megaraid and storcli2 adapters already treat nil (no cache flags emitted).
- Core empty-drive guards (ErrNoPhysicalDrives) are uniform for both AddPDsToLV and DeletePDsFromLV, tested with the embedded-interface stub pattern.
- checkRAIDRequirement default branch closes the silent fall-through for out-of-range levels. The RAIDLevelUnknown case inside it is now technically unreachable from Request.Validate (IsValid rejects it first), but it is harmless defense-in-depth.
- Tests are comprehensive: table-driven, parallel, cover the nil/unknown/garbage/valid matrix for all enums and CacheOptions, and the core stub-based tests pin the guard ordering.

Review by Claude Code

[claude]

• cache.go:68 — megaraidSetCacheFlags dereferences desired and current without nil guards, inconsistent with the nil-safe megaraidCreateCacheFlags in the same PR. Latent panic risk if callers change.
  - cache_internal_test.go — No test coverage for megaraidSetCacheFlags (diff-based logic, IO-policy silent-skip behavior). The create path and token mappers are well tested; the set path should match.
  - methods_test.go:30 — pdMetadata helper uses string(rune('0'+i)) which breaks for n >= 10; fmt.Sprintf is safer.
  
  Review by Claude Code

[claude]

LGTM — solid input-validation tightening across domain, core, and megaraid v1 adapter. Enum IsValid() whitelists, nil-safe CacheOptions.Validate(), optional IO policy semantics, empty-drive guards in core, and fail-closed cache-flag mappers in megaraid are all correct and well tested. The adapter-owned CLI token mapping (decoupled from enum string values) mirrors the storcli2 pattern and prevents silent wire-format drift. No correctness, security, or compatibility issues found.

Review by Claude Code

[claude]

LGTM — validation is correct and well-layered.

- Domain-level IsValid() whitelists and nil-safe CacheOptions.Validate() are clean; the IO-policy optionality condition (empty/Unknown accepted, garbage rejected) is correctly structured.
- Request.Validate() properly short-circuits on nil CacheOptions before delegating, matching the adapter contract (nil = controller defaults).
- core guards for empty PD lists are symmetric and well-tested through the nil-embedded-interface stub pattern.
- The megaraid adapter correctly owns its CLI token mapping (fail-closed for add-vd where all three policies are mandatory, lenient for set where IO policy is optional). The rdpolicy= vs rdcache= asymmetry between add-vd and set paths matches the original storcli v1 CLI contract.
- Test coverage is thorough: table-driven for all new IsValid() methods, CacheOptions.Validate, Request.Validate, and both megaraidCreateCacheFlags and megaraidSetCacheFlags with nil/valid/invalid/unknown/garbage cases.
- Hexagonal boundaries respected — domain validation is intentionally loose (different controllers have different IO-policy requirements), adapter validation adds controller-specific strictness.

Review by Claude Code