feat: add snapshot and backup protection to Azure NetApp volumes

[timtalbot]

Summary

• Creates a snapshot policy (7 daily at 02:00 UTC), backup vault, and backup policy (30 daily) on every Azure NetApp account — matching the AWS FSx 30-day automatic backup posture
• Wires data_protection into auto-provisioned volumes so they get both snapshots and backups automatically
• Adds Go-side config fields with SetDefaults() so existing ptd.yaml files work without changes
• Snapshot/backup/vault resources are created unconditionally (even without automated_volume_provisioning) so they're available for manually-created volumes

Issue

#250

New config fields (all optional, with defaults)

Field	Default	Notes
netapp_backup_retention_days	30	Daily backups to retain
netapp_daily_backup_start_time	"02:00"	HH:MM UTC for daily snapshots
netapp_snapshots_to_keep	7	Daily snapshots to retain (local, fast restore)

Rollout notes

• Existing workloads: running ptd ensure --only-steps persistent will create the new resources. Volumes not managed by automated_volume_provisioning need snapshot/backup assignment in the portal.
• First backup per volume is a full copy — schedule during low-usage windows.

Test plan

• ptd ensure <azure-staging-target> --only-steps persistent --dry-run — verify snapshot policy, backup vault, and backup policy appear in preview
• Apply to staging and confirm resources created in Azure portal
• Verify auto-provisioned volumes show data_protection with both snapshot and backup references
• Manually assign snapshot policy and enable backup on a non-auto-provisioned volume

[claude]

Claude finished @timtalbot's task —— View job

---

Review Complete

• Read review guidelines
• Analyze PR diff and changed files
• Create and submit review with feedback

Submitted a review with 5 items covering: dual Go/Python defaults risk, asymmetric SetDefaults() call, incomplete time validation, implicit method ordering dependency, and missing self-review comments.

[claude]

Overall this is clean and well-structured — the resources follow existing patterns, naming conventions are consistent, and the protect flag is correctly applied to all new resources. A few items worth discussing:

---

1. Dual-defaults in Go and Python (lib/types/workload.go:295-304 + python-pulumi/src/ptd/azure_workload.py:66-68)

Important: This is the first SetDefaults() on any config type in the codebase. The AWS and other config types get their zero-value-vs-default handling purely from Python dataclass defaults. Having defaults in both Go and Python creates a maintenance risk — if someone changes one without the other, values silently diverge.

Also, SetDefaults() uses zero-value checks, so a user can't explicitly set netapp_backup_retention_days: 0 or netapp_snapshots_to_keep: 0. This is fine since 0 isn't meaningful here, but worth documenting the limitation.

Consider whether Go-side SetDefaults() should become a standard pattern or whether defaults should stay exclusively in Python where Pulumi consumes them.

2. Asymmetric SetDefaults() call (lib/helpers/helpers.go:69)

Important: SetDefaults() is called only for AzureWorkloadConfig, not for AWSWorkloadConfig or AWSControlRoomConfig. If future contributors add default fields to AWS configs, they might not realize they need to wire in a SetDefaults() call here. Consider a Defaultable interface to make this systematic.

3. Time validation is incomplete (azure_workload_persistent.py:556-562)

Suggestion: The HH:MM parse validation is good, but doesn't check ranges. "25:99" would parse fine and get passed to Azure, which rejects it with a less helpful error. Consider adding 0 ≤ hour ≤ 23, 0 ≤ minute ≤ 59 checks.

4. Implicit ordering dependency (azure_workload_persistent.py:658-666)

Important: _define_netapp_volumes() references self.backup_policy, self.backup_vault, and self.snapshot_policy set by _define_file_storage(). The call ordering in __init__ (line 99→100) makes this work, but it's an implicit coupling through instance attributes. If someone reorders the _define_*() calls, they'll get an AttributeError at runtime.

The existing docstring on _define_netapp_volumes doesn't mention this dependency. Consider adding a note — consistent with the CLAUDE.md warning about builder method ordering dependencies.

5. No self-review comments on the PR

Per the review guidelines, PR authors are expected to review their own diff and add inline comments on areas of concern or key decisions. I don't see any — would be helpful especially around the dual-defaults decision and the data_protection wiring.

[timtalbot]

@claude re-review

[claude]

Claude finished @timtalbot's task —— View job

---

Re-Review

• Read review guidelines
• Analyze latest commit addressing review feedback
• Read current state of all changed files
• Submit updated review

Previous feedback was addressed well:

1. Dual defaults — SetDefaults() in Go was removed. Defaults now live exclusively in Python dataclass fields. Clean.
2. Asymmetric SetDefaults() call — no longer applicable since SetDefaults() is gone.
3. Time validation — range checks added (0 ≤ hour ≤ 23, 0 ≤ minute ≤ 59) with a clear error message. Good.
4. Ordering dependency — class docstring updated to document the _define_file_storage() → _define_netapp_volumes() dependency. Sufficient.

No remaining issues. The Go side is struct-fields-only (no defaults needed since Python is authoritative), the naming properties follow existing {prefix}-ptd-{compound_name} conventions, protect is applied consistently, and the resource structure is clean.

LGTM.