refactor(validation): make DGD validation structural

[sttts]

Dependency

Depends on #11216, which owns the admission-chain test prefactor. Until that PR merges, GitHub includes its test-only commits in this PR's aggregate diff. The structural-only stacked diff is available at sttts/dynamo@sttts-dgd-admission-test-prefactor...sttts-structural-validation.

Overview

Refactor DynamoGraphDeployment validation into a structural recursion that systematically follows the Go API type tree.

Details

• split v1beta1 and v1alpha1 structural validation by their owning API types
• standardize typed field paths, field error lists, validator signatures, and sparse context propagation
• keep helpers receiver-free and colocate validators with their owning types
• leave source-schema and CEL-owned constraints declarative
• preserve fatal conversion failures and served-version compatibility boundaries
• add only structural error expectations and newly discovered regression cases on top of test(validation): unify DGD admission coverage #11216

Where should the reviewer start?

Start with deploy/operator/internal/webhook/validation/AGENTS.md, then follow the root validation recursion in dynamographdeployment.go.

Related Issues

• Confirmed — no related issue

Summary

• make DGD validation structural and type-directed
• preserve v1alpha1 admission compatibility while validating the v1beta1 hub
• keep the unified schema/CEL/conversion/webhook test chain from test(validation): unify DGD admission coverage #11216

Validation

• make check
• make test
• make lint
• validation package coverage: 79.7%

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[github-actions]

🌿 Fern Docs Preview: https://nvidia-preview-019f369a-f228-752b-8ec6-b0e922ad6f9c.docs.buildwithfern.com/dynamo/dev

[devin-ai-integration]

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

[coderabbitai]

Walkthrough

The PR rewrites DynamoGraphDeployment validation to structured Kubernetes field-path-based errors, removes the standalone v1beta1 shared-spec validator, adds helper utilities and handler/test updates, and changes VLLM annotation validation to typed field-path errors.

Changes

DGD Webhook Validator Rewrite

Layer / File(s)	Summary
Validation conventions deploy/operator/internal/webhook/validation/AGENTS.md	Documents structural validator composition, context and warning rules, compatibility boundaries, shared-spec ownership, and test expectations.
Validation helpers deploy/operator/internal/webhook/validation/dynamographdeployment_helpers.go, deploy/operator/internal/webhook/validation/shared_v1alpha1.go	Adds warning helpers, invalid-error conversion, v1alpha1 compatibility detection, topology and Grove helpers, component/KV/GMS/failover utilities, and VLLM backend annotation validation.
Validator entrypoints and stateless validation deploy/operator/internal/webhook/validation/dynamographdeployment.go	Introduces request-scoped validator state, rewires Validate/ValidateUpdate, validates metadata annotations and semver rules, and validates v1alpha1 compatibility-preserved fields.
v1beta1 spec validation deploy/operator/internal/webhook/validation/dynamographdeployment.go, deploy/operator/internal/webhook/validation/shared_v1beta1.go	Implements v1beta1 component, shared-spec, restart, topology, experimental, GMS, failover, and checkpoint validation, and deletes SharedSpecValidatorV1Beta1.
Handler wiring and webhook tests deploy/operator/internal/webhook/validation/dynamographdeployment_handler.go, deploy/operator/internal/webhook/validation/dynamographdeployment_handler_test.go	Updates handler wiring/comments and adds create, update, delete, casting, and admission-context tests.
Validator test updates deploy/operator/internal/webhook/validation/dynamographdeployment_test.go	Rewrites validator expectations to typed field-path errors and adds internal field-path, conversion-failure, and invalid-status assertions.

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 7.14% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly summarizes the main change: making DGD validation structural.
Description check	✅ Passed	The description follows the template with Overview, Details, reviewer start, and Related Issues sections, and the no-issue path is filled out.

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

deploy/operator/internal/webhook/validation/dynamographdeployment_handler.go (1)

125-133: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Preserve stateless warnings on the update error path.

validator.Validate() can already return admission warnings. If ValidateUpdate() then rejects the object, returning only updateWarnings drops those earlier warnings from the response.

Suggested fix

 	updateWarnings, err := validator.ValidateUpdate(ctx, oldDeployment, newDeployment, userInfo, h.operatorPrincipal)
 	if err != nil {
 		username := "<unknown>"
 		if userInfo != nil {
 			username = userInfo.Username
 		}
 		logger.Info("validation failed", "error", err.Error(), "user", username)
-		return updateWarnings, err
+		return append(warnings, updateWarnings...), err
 	}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@deploy/operator/internal/webhook/validation/dynamographdeployment_handler.go`
around lines 125 - 133, Preserve warnings returned by validator.Validate() when
the update is rejected in dynamographdeployment_handler.go: ValidateUpdate
currently returns only updateWarnings on the error path, which can drop
stateless admission warnings from the earlier Validate call. Update the
ValidateUpdate flow in the relevant handler so it carries forward any existing
warnings from the initial validation and combines them with updateWarnings
before returning an error, using the ValidateUpdate/Validate symbols to locate
the code.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@deploy/operator/internal/webhook/validation/dynamographdeployment_handler_test.go`:
- Around line 107-125: The ValidateUpdate test cases in
dynamographdeployment_handler_test should assert the handler’s typed
invalid-error contract instead of only matching err.Error(). Update the
“stateless validation failure” and “stateful validation failure” subtests to use
the package’s typed-error helper or inspect the returned StatusError directly
from ValidateUpdate, verifying the expected StatusError.ErrStatus.Details.Causes
and exact field paths. Ensure the assertions cover the invalid component path
and backendFramework path so the test fails if the handler returns the wrong
field path or omits causes.

In `@deploy/operator/internal/webhook/validation/dynamographdeployment_test.go`:
- Around line 1955-1969: The helper in dynamographdeployment_test.go is only
checking that StatusError has some non-empty Causes and that the rendered error
string contains wantErr; tighten it to validate the exact typed field path in
Details.Causes for the expected error, using the existing statusErr and
cause.Field checks in the test helper. Keep the string match separate or
secondary, but do not rely on strings.Contains(err.Error(), wantErr) as the
primary assertion, since the goal is to assert the specific Cause.Field produced
by the validation path.

In `@deploy/operator/internal/webhook/validation/dynamographdeployment.go`:
- Around line 419-424: The live topology lookup in validation is being skipped
whenever topologyErrs is non-empty, which can hide independent errors from
spec.TopologyConstraint.ClusterTopologyName or bad pack-domain references.
Update the logic around readGroveClusterTopology in dynamographdeployment
validation so the lookup still runs when ClusterTopologyName is set and
opts.generation/opts.grovePathway allow it, and only gate it on conditions that
truly depend on earlier topology validation. Keep topologyErrs aggregation
intact so all independent errors are reported.
- Around line 1022-1027: The replica validation in ValidateUpdate can be
bypassed when ScalingAdapter is removed in the same request because the guard
only checks newComponent.ScalingAdapter. Update the replica protection logic in
dynamographdeployment.go so it also blocks replica changes whenever the old
component had a scaling adapter identity and the replica value changes, even if
the new component clears ScalingAdapter. Use the existing ValidateUpdate flow
and the newComponent/oldComponent checks to ensure direct replica edits are
forbidden during scaling-adapter transitions.

In `@deploy/operator/internal/webhook/validation/shared_v1alpha1.go`:
- Line 223: The validation path in the shared v1alpha1 webhook is bypassing the
helper’s empty-path handling by always concatenating v.fieldPath with
".annotations", which turns an empty field path into a malformed ".annotations"
string. Update the call site in the validator around
vllmDistributedExecutorBackendAnnotationError so it preserves the empty-string
case and only appends ".annotations" when v.fieldPath is non-empty, allowing the
helper’s fieldPath == "" branch to work as intended.

---

Outside diff comments:
In
`@deploy/operator/internal/webhook/validation/dynamographdeployment_handler.go`:
- Around line 125-133: Preserve warnings returned by validator.Validate() when
the update is rejected in dynamographdeployment_handler.go: ValidateUpdate
currently returns only updateWarnings on the error path, which can drop
stateless admission warnings from the earlier Validate call. Update the
ValidateUpdate flow in the relevant handler so it carries forward any existing
warnings from the initial validation and combines them with updateWarnings
before returning an error, using the ValidateUpdate/Validate symbols to locate
the code.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: c6a5daa8-b0d4-4236-bea1-ad8b04dc4f97

📥 Commits

Reviewing files that changed from the base of the PR and between 3cabd83 and a97e250.

📒 Files selected for processing (8)

• deploy/operator/internal/webhook/validation/AGENTS.md
• deploy/operator/internal/webhook/validation/dynamographdeployment.go
• deploy/operator/internal/webhook/validation/dynamographdeployment_handler.go
• deploy/operator/internal/webhook/validation/dynamographdeployment_handler_test.go
• deploy/operator/internal/webhook/validation/dynamographdeployment_helpers.go
• deploy/operator/internal/webhook/validation/dynamographdeployment_test.go
• deploy/operator/internal/webhook/validation/shared_v1alpha1.go
• deploy/operator/internal/webhook/validation/shared_v1beta1.go

💤 Files with no reviewable changes (1)

• deploy/operator/internal/webhook/validation/shared_v1beta1.go

[sttts]

/ok to test 539931d

[datadog-official]

⚠️ Warnings

🚦 1 Pipeline job failed

Docs link check | lychee     

_{This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 78ad643 | Docs | Give us feedback!}

[julienmancuso]

I found four remaining validation-boundary concerns on the current head. The structural direction looks sound, and the previously open review findings appear addressed.

[sttts]

/ok to test 78ad643