Skip to content

AzureBackup: Apply command-boundary workload/datasource validation to DPP protect and policy-create surfaces (NEW-4 follow-up) #2727

Description

@shrja-ms

Apply command-boundary workload/datasource validation to DPP protect and policy create surfaces

Context

PR #2726 (NEW-4) added a command-boundary validator on azmcp_azurebackup_protectableitem_list --workload-type so that unknown values are rejected as a 400 ValidationError instead of leaking the service-layer ArgumentException as a 500. That fix only covers the RSV-only protectableitem_list command, which was the surface called out in the telemetry-triaged report.

A code audit during PR #2726 review found the same anti-pattern on two additional surfaces — both DPP-backed — that were not in the original telemetry report. This issue tracks fixing them in a separate, scoped PR.

Affected surfaces

Command Option Service-layer throw on unknown
azmcp_azurebackup_protecteditem_protect (DPP path) --datasource-type DppBackupOperations.ProtectItemAsyncResolveProfileDppDatasourceRegistry.Resolve throws ArgumentException("Unknown DPP workload type '...'")
azmcp_azurebackup_policy_create (DPP path) --workload-type DppBackupOperations.CreatePolicyAsyncDppDatasourceRegistry.Resolve(workloadType) throws ArgumentException
azmcp_azurebackup_policy_create (RSV path) --workload-type RsvBackupOperations.CreatePolicyRsvDatasourceRegistry.ResolveOrDefault throws ArgumentException

All three currently surface as HTTP 500 / InternalServerError to MCP clients, hiding what is really a user-input mistake.

PolicyCreateValidator already does business-rule validation for the policy create command but does not validate that WorkloadType is in any known set.

Proposed fix

Mirror the NEW-4 pattern, but with DPP-aware allowlists:

  1. ProtectedItemProtectCommand --datasource-type

    • Add a command.Validators.Add(...) that reads the value via commandResult.GetValueOrDefault<string>(AzureBackupOptionDefinitions.DatasourceType.Name) and rejects when the value is non-null and not in DppDatasourceRegistry.KnownTypeNames and not a recognized ARM resource type (via DppDatasourceRegistry.TryAutoDetect).
    • Use the same "Unknown DPP workload type '...'. Supported tokens: …" message format as the service-layer throw so the error is consistent across layers.
  2. PolicyCreateCommand --workload-type

    • Add a validator that accepts any token in the union of RsvDatasourceRegistry.KnownTypeNames and DppDatasourceRegistry.KnownTypeNames. If --vault-type is also supplied on the command line, narrow the allowed set to just that backend's tokens.
  3. Keep the service-layer throws in place as defense-in-depth (same approach as NEW-4 on RsvBackupOperations.NormalizeWorkloadTypeForFilter).

  4. Tests — extend the unit-test patterns introduced in PR azurebackup: fix telemetry-triaged bugs NEW-1, NEW-3, NEW-4 #2726:

    • ExecuteAsync_RejectsUnknownDatasourceType_AsValidationError on ProtectedItemProtectCommandTests (including whitespace cases: " ", "\t ").
    • ExecuteAsync_RejectsUnknownWorkloadType_AsValidationError on PolicyCreateCommandTests (including the vault-type narrowing case where an RSV-only token is rejected when --vault-type DPP is specified, and vice versa).
    • Assert BadRequest status, "Unknown" substring in the message, and Service.DidNotReceive().

Out of scope

  • Anything not on the --datasource-type / --workload-type parameter surfaces.
  • Behavior change of RsvDatasourceRegistry.ResolveOrDefault (which still needs to throw as defense-in-depth).
  • Telemetry tag changes.

Acceptance criteria

  • Unknown values passed to --datasource-type on protecteditem protect (DPP path) return BadRequest (HTTP 400) with an actionable error message and never reach DppBackupOperations.ProtectItemAsync.
  • Unknown values passed to --workload-type on policy create return BadRequest (HTTP 400) and never reach the registry Resolve call.
  • All existing AzureBackup unit tests continue to pass.
  • New unit tests cover the rejection paths including whitespace inputs.
  • No live test re-recording required (the changes are command-boundary only, no Azure traffic).

Related

Metadata

Metadata

Assignees

Type

No type

Fields

No fields configured for issues without a type.

Projects

Status
Untriaged

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions