[AV-132963] Add local validation for backup and snapshot restore inputs#627
[AV-132963] Add local validation for backup and snapshot restore inputs#627nimiyajoseph wants to merge 8 commits into
Conversation
|
No PR template selections were detected. Please make sure to fill out the PR template properly by selecting the appropriate checkboxes. |
|
Droid finished @nimiyajoseph's task —— View job |
There was a problem hiding this comment.
Pull request overview
This PR strengthens plan-time/local validation for backup- and snapshot-restore-related Terraform configurations so invalid inputs are rejected by terraform validate/plan rather than failing later during provider execution/API calls.
Changes:
- Added
ModifyPlanvalidation to blockbackup.restoreandbackup.restore_timeswhen creating acouchbase-capella_backup. - Added local enum validation for
backup.restore.replace_ttland forbackup_schedule.weekly_schedulefields (day_of_week,start_at,incremental_every,retention_time). - Added/updated acceptance tests to assert local validation errors for these edge cases (including cloud snapshot restore datasource filter validation scenarios).
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/resources/backup.go | Adds ResourceWithModifyPlan and create-time plan validation to reject restore-only fields during initial backup creation. |
| internal/resources/backup_schema.go | Adds OneOf validation for restore.replace_ttl. |
| internal/resources/backup_schedule_schema.go | Adds local enum/range-style validation for weekly schedule fields. |
| internal/errors/errors.go | Introduces a new error constant for “restore must not be set while create backup”. |
| acceptance_tests/cloud_snapshot_restore_datasource_test.go | Adds negative acceptance tests for invalid/empty filter inputs on the list datasource. |
| acceptance_tests/backup_schedule_acceptance_test.go | Updates negative tests to expect local validation errors; adds a new negative test for incremental_every. |
| acceptance_tests/backup_acceptance_test.go | Adds negative tests for restore on create and invalid replace_ttl. |
![factory-droid[bot]](https://avatars.githubusercontent.com/in/358017?s=60&v=4){kind=small}
| capellaschema.AddAttr(weeklyScheduleAttrs, "start_at", backupScheduleBuilder, int64Attribute(required), "WeeklySchedule") | ||
| capellaschema.AddAttr(weeklyScheduleAttrs, "incremental_every", backupScheduleBuilder, int64Attribute(required), "WeeklySchedule") | ||
| capellaschema.AddAttr(weeklyScheduleAttrs, "retention_time", backupScheduleBuilder, stringAttribute([]string{required}), "WeeklySchedule") | ||
| capellaschema.AddAttr(weeklyScheduleAttrs, "day_of_week", backupScheduleBuilder, stringAttribute([]string{required}, stringvalidator.OneOf("sunday", "monday", "tuesday", "wednesday", "thursday", "friday", "saturday")), "WeeklySchedule") |
There was a problem hiding this comment.
I see you did supply the schema name here but also have manually defined the OneOf validators. Was there an issue with the automated validation? If so worth tracking with a ticket I would be happy to pick it up.
There was a problem hiding this comment.
Yes, there is an issue with the automated validation path for this case.
The generated enum metadata exists, but it is stored under CreateScheduledBackupRequest using nested OpenAPI field paths:
"weeklySchedule.dayOfWeek" "weeklySchedule.startAt" "weeklySchedule.incrementalEvery" "weeklySchedule.retentionTime"
The schema code calls AddAttr on child Terraform fields like:
day_of_week start_at incremental_every retention_time
Current enum lookup converts those to candidates like dayOfWeek, startAt, etc., and only checks exact keys in the selected schema table. It does not match nested generated keys like weeklySchedule.dayOfWeek. So even if we pass "CreateScheduledBackupRequest" into AddAttr, the generated validator is not found for the nested weekly schedule fields. That is why the manual OneOf validators were still needed.
There was a problem hiding this comment.
matty271828
left a comment
matty271828
left a comment
There was a problem hiding this comment.
note acceptance test failure looks related to this change:
=== CONT TestAccBackupResourceRestoreInvalidReplaceTTL
backup_acceptance_test.go:160: Step 1/1, expected an error with pattern, no match on: Error running pre-apply plan: exit status 1
Error: Invalid Backup Restore Configuration
with couchbase-capella_backup.tf_acc_backup_invalid_replace_ttl_lntibbwgwb,
on terraform_plugin_test.tf line 34, in resource "couchbase-capella_backup" "tf_acc_backup_invalid_replace_ttl_lntibbwgwb":
34: restore = {
35: target_cluster_id = "33333333-3333-3333-3333-333333333333"
36: source_cluster_id = "22222222-2222-2222-2222-222222222222"
37: services = ["data"]
38: force_updates = true
39: auto_remove_collections = false
40: replace_ttl = "forever"
41: replace_ttl_with = "0"
42: include_data = "bucket.scope.collection"
43: exclude_data = null
44: filter_keys = null
45: filter_values = null
46: map_data = null
47: }
restore must not be set during backup creation
Jira
Description
Summary
This PR adds local Terraform validation coverage for backup, backup schedule, and cloud snapshot restore configuration edge cases that were previously accepted by terraform validate and only failed later during provider/API execution.
Changes include:
Tests Added
Total new acceptance tests added: 5
New tests:
Also updated existing backup schedule negative tests to expect local validation errors instead of API/create-time failures:
Type of Change
Manual Testing Approach
How was this change tested and do you have evidence? (REQUIRED: Select at least 1)
Testing
Testing
Further comments