r/aws_backup_plan: Adding Copy Action support #11923

slapula · 2020-02-06T03:58:02Z

Community Note

Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for pull request followers and do not help prioritize the request

Release note for CHANGELOG:

r/aws_backup_plan: adding copy action support

Output from acceptance testing:

 make testacc TEST=./aws TESTARGS='-run=TestAccAwsBackupPlan_withCopyActions'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAwsBackupPlan_withCopyActions -timeout 120m
=== RUN   TestAccAwsBackupPlan_withCopyActions
=== PAUSE TestAccAwsBackupPlan_withCopyActions
=== RUN   TestAccAwsBackupPlan_withCopyActionsCrossRegion
=== PAUSE TestAccAwsBackupPlan_withCopyActionsCrossRegion
=== CONT  TestAccAwsBackupPlan_withCopyActions
=== CONT  TestAccAwsBackupPlan_withCopyActionsCrossRegion
--- PASS: TestAccAwsBackupPlan_withCopyActionsCrossRegion (24.55s)
--- PASS: TestAccAwsBackupPlan_withCopyActions (49.66s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	49.722s

giuliocalzolari · 2020-03-10T08:46:39Z

would be great to have this code merged into master, any ETA?

infa-sgowtham · 2020-03-10T10:22:10Z

Any ETA when it will be live?

bflad

Hey @slapula 👋 This is looking pretty good. A few little things and this should be good to go. Please reach out if you have any questions or do not have time to implement the feedback. 👍

bflad · 2020-03-16T01:12:26Z

aws/resource_aws_backup_plan.go

@@ -69,6 +69,35 @@ func resourceAwsBackupPlan() *schema.Resource {
 								},
 							},
 						},
+						"copy_action": {
+							Type:     schema.TypeList,


If this attribute supports more than 1, we should add a covering acceptance test configuration to determine if the API orders results (or requires TypeSet), otherwise, we should ensure this has MaxItems: 1 as well. 👍

bflad · 2020-03-16T01:12:55Z

aws/resource_aws_backup_plan.go

+											},
+										},
+									},
+									"destination_vault_arn": {


Nit: We can add plan-time validation for this attribute via ValidateFunc: validateArn,

bflad · 2020-03-16T01:15:27Z

aws/resource_aws_backup_plan.go

+		for _, action := range rule.CopyActions {
+			mRule["copy_action"] = []interface{}{


Should this be overwriting the results every loop?

bflad · 2020-03-16T01:17:54Z

aws/resource_aws_backup_plan_test.go

@@ -625,3 +760,109 @@ resource "aws_backup_plan" "test" {
 }
 `, rName)
 }
+
+func testAccAwsBackupPlanConfig_copyAction(rName1 string, rName2 string) string {


Nit: Can remove the second randomized name parameter by appending -2 or similar in the template below, e.g. name = "%[1]s-2"

bflad · 2020-03-16T01:19:20Z

aws/resource_aws_backup_plan_test.go

+`, rName1, rName2)
+}
+
+func testAccAwsBackupPlanConfig_copyActionUpdated(rName1 string, rName2 string) string {


Nit: Can remove this mostly duplicated configuration by parameterizing cold_storage_after and delete_after, e.g.

lifecycle { cold_storage_after = %[3]d delete_after = %[4]d }

bflad · 2020-03-16T01:21:32Z

website/docs/r/backup_plan.html.markdown

+* `completion_window` - (Optional) The amount of time AWS Backup attempts a backup before canceling the job and returning an error.
+* `lifecycle` - (Optional) The lifecycle defines when a protected resource is transitioned to cold storage and when it expires.  Fields documented below.
+* `recovery_point_tags` - (Optional) Metadata that you can assign to help organize the resources that you create.
+* `copy_action` - (Optional) 


Looks like this is missing. 😄

Suggested change

* `copy_action` - (Optional)

* `copy_action` - (Optional) Configuration block(s) with copy operation settings. Detailed below.

infa-sgowtham · 2020-03-16T11:59:47Z

@slapula When can I expect this to be live?

ldm810 · 2020-03-26T21:50:43Z

Hey @slapula thanks for putting this PR together. Do you have an ETA on when you can address the changes? Looking forwarding to using this!
Thanks again!

bflad

Hi again @slapula 👋 Since we didn't hear back from you, we addressed the review feedback in a followup commit. Thanks for starting this.

Output from acceptance testing:

--- PASS: TestAccAwsBackupPlan_disappears (18.44s)
--- PASS: TestAccAwsBackupPlan_basic (22.03s)
--- PASS: TestAccAwsBackupPlan_Rule_CopyAction_Multiple (22.15s)
--- PASS: TestAccAwsBackupPlan_Rule_CopyAction_CrossRegion (22.21s)
--- PASS: TestAccAwsBackupPlan_withRecoveryPointTags (46.63s)
--- PASS: TestAccAwsBackupPlan_withRules (46.98s)
--- PASS: TestAccAwsBackupPlan_withTags (47.99s)
--- PASS: TestAccAwsBackupPlan_Rule_CopyAction_SameRegion (48.55s)
--- PASS: TestAccAwsBackupPlan_withLifecycle (62.46s)

Reference: #11923 (review) Output from acceptance testing: ``` --- PASS: TestAccAwsBackupPlan_disappears (18.44s) --- PASS: TestAccAwsBackupPlan_basic (22.03s) --- PASS: TestAccAwsBackupPlan_Rule_CopyAction_Multiple (22.15s) --- PASS: TestAccAwsBackupPlan_Rule_CopyAction_CrossRegion (22.21s) --- PASS: TestAccAwsBackupPlan_withRecoveryPointTags (46.63s) --- PASS: TestAccAwsBackupPlan_withRules (46.98s) --- PASS: TestAccAwsBackupPlan_withTags (47.99s) --- PASS: TestAccAwsBackupPlan_Rule_CopyAction_SameRegion (48.55s) --- PASS: TestAccAwsBackupPlan_withLifecycle (62.46s) ```

ldm810 · 2020-04-16T15:37:51Z

Thank you for merging!

ghost · 2020-04-17T00:43:48Z

This has been released in version 2.58.0 of the Terraform AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template for triage. Thanks!

ghost · 2020-05-15T17:10:55Z

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!

slapula added 2 commits February 3, 2020 09:12

initial commit

ae1af3c

fixing bugs, adding tests, updating docs

99f311d

slapula requested a review from a team February 6, 2020 03:58

removing unnecessary nil check

c8830d3

ewbankkit mentioned this pull request Mar 7, 2020

AWS Backup Code to copy snapshot to different region #12306

Closed

bflad suggested changes Mar 16, 2020

View reviewed changes

bflad added enhancement Requests to existing resources that expand the functionality or scope. and removed needs-triage Waiting for first response or review from a maintainer. labels Mar 16, 2020

bflad self-assigned this Mar 16, 2020

bflad added the waiting-response Maintainers are waiting on response from community or contributor. label Apr 1, 2020

bflad added this to the v2.58.0 milestone Apr 15, 2020

bflad reviewed Apr 15, 2020

View reviewed changes

bflad merged commit 6262e3e into hashicorp:master Apr 15, 2020

bflad added a commit that referenced this pull request Apr 15, 2020

Update CHANGELOG for #11923

f712cdd

ghost removed the waiting-response Maintainers are waiting on response from community or contributor. label Apr 16, 2020

ghost locked and limited conversation to collaborators May 15, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

r/aws_backup_plan: Adding Copy Action support #11923

r/aws_backup_plan: Adding Copy Action support #11923

slapula commented Feb 6, 2020

giuliocalzolari commented Mar 10, 2020

infa-sgowtham commented Mar 10, 2020

bflad left a comment

bflad Mar 16, 2020

bflad Mar 16, 2020

bflad Mar 16, 2020

bflad Mar 16, 2020

bflad Mar 16, 2020

bflad Mar 16, 2020

infa-sgowtham commented Mar 16, 2020

ldm810 commented Mar 26, 2020

bflad left a comment

ldm810 commented Apr 16, 2020

ghost commented Apr 17, 2020

ghost commented May 15, 2020

		for _, action := range rule.CopyActions {
		mRule["copy_action"] = []interface{}{

	* `copy_action` - (Optional)
	* `copy_action` - (Optional) Configuration block(s) with copy operation settings. Detailed below.

r/aws_backup_plan: Adding Copy Action support #11923

r/aws_backup_plan: Adding Copy Action support #11923

Conversation

slapula commented Feb 6, 2020

Community Note

giuliocalzolari commented Mar 10, 2020

infa-sgowtham commented Mar 10, 2020

bflad left a comment

Choose a reason for hiding this comment

bflad Mar 16, 2020

Choose a reason for hiding this comment

bflad Mar 16, 2020

Choose a reason for hiding this comment

bflad Mar 16, 2020

Choose a reason for hiding this comment

bflad Mar 16, 2020

Choose a reason for hiding this comment

bflad Mar 16, 2020

Choose a reason for hiding this comment

bflad Mar 16, 2020

Choose a reason for hiding this comment

infa-sgowtham commented Mar 16, 2020

ldm810 commented Mar 26, 2020

bflad left a comment

Choose a reason for hiding this comment

ldm810 commented Apr 16, 2020

ghost commented Apr 17, 2020

ghost commented May 15, 2020