aws_backup_plan lifecycle cold_storage_after should be optional

[sbutler]

Community Note

• Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
• Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
• If you are interested in working on this issue or have submitted a pull request, please leave a comment

Terraform Version

Terraform v0.11.13

• provider.aws v2.4.0

Affected Resource(s)

• aws_backup_plan

Terraform Configuration Files

resource "aws_backup_vault" "example" {
    name = "example-vault"
}

resource "aws_backup_plan" "example" {
    name = "example-plan"
    rule {
        rule_name = "MainRule"
        target_vault_name = "${aws_backup_vault.example.name}"
        schedule = "cron(5 8 * * ? *)"
        lifecycle {
            delete_after = 30
        }
    }
}

Debug Output

https://gist.github.com/sbutler/f9480ef8e86428b054181b94d6221a75

Expected Behavior

I should be able to create a plan with a rule that only has delete_after and no cold_storage_after.

Actual Behavior

Plan fails, because the request terraform sends is invalid (delete_after not 90 days after cold_storage_after).

Steps to Reproduce

1. terraform apply

References

• r/aws_backup_plan: Adding resource for managing AWS Backup plans #7350 (comment)

[nywilken]

Hi @sbutler thanks for opening up this issue. Quickly looking at the resource code base I can see some inconsistencies around the documentation for the lifecycle attributes which are actually optional, but documented as Required. There also may be a case where we are sending a value for cold storage when we shouldn't be so I am adding the bug label to this issue for further investigation.

[ghost]

I ran into this while working with the new service as well. It appears that both delete after and move to cold storage should be optional.

If cold storage is defined though, delete after must be > move to cold storage + 90.

AWS Backup API » Data Types » Lifecycle

[grom3k]

Hey @kmcdowell85 @nywilken @slapula
After quick investigation I noticed that if one of the lifecycle attribute is left unspecified it gets value 0.

Example.
Resource definition:

resource "aws_backup_plan" "example" {
  provider = "awstest.debug"
  name     = "tf_example_backup_plan"

  rule {
    rule_name         = "tf_example_backup_rule"
    target_vault_name = "Default"
    schedule          = "cron(0 12 * * ? *)"

    lifecycle {
      delete_after = 7
    }
  }
}

Get some additional output:

diff --git a/aws/resource_aws_backup_plan.go b/aws/resource_aws_backup_plan.go
index 8c7f052b4..56e7e1170 100644
--- a/aws/resource_aws_backup_plan.go
+++ b/aws/resource_aws_backup_plan.go
@@ -111,6 +111,7 @@ func resourceAwsBackupPlanCreate(d *schema.ResourceData, meta interface{}) error
 
        resp, err := conn.CreateBackupPlan(input)
        if err != nil {
+               log.Printf("Current rules: %s", rules)
                return fmt.Errorf("error creating Backup Plan: %s", err)
        }
 

Rules list value:

2019-04-06T12:11:59.641+0200 [DEBUG] plugin.terraform-provider-awstest: 2019/04/06 12:11:59 [DEBUG] [aws-sdk-go] {"Code":"ERROR_3015","Context":"[\"tf_example_backup_rule\",\"Inva
lid lifecycle. DeleteAfterDays cannot be less than 90 days apart from MoveToColdStorageAfterDays\"]","Message":"Error in rule tf_example_backup_rule : Invalid lifecycle. DeleteAft
erDays cannot be less than 90 days apart from MoveToColdStorageAfterDays","Type":null}
2019-04-06T12:11:59.641+0200 [DEBUG] plugin.terraform-provider-awstest: 2019/04/06 12:11:59 [DEBUG] [aws-sdk-go] DEBUG: Validate Response Backup/CreateBackupPlan failed, not retry
ing, error InvalidParameterValueException: Error in rule tf_example_backup_rule : Invalid lifecycle. DeleteAfterDays cannot be less than 90 days apart from MoveToColdStorageAfterD
ays
2019-04-06T12:11:59.641+0200 [DEBUG] plugin.terraform-provider-awstest:         status code: 400, request id: 379ef015-56df-42cc-90fc-60ec6f80701e
2019-04-06T12:11:59.641+0200 [DEBUG] plugin.terraform-provider-awstest: 2019/04/06 12:11:59 Current rules: [{
2019-04-06T12:11:59.641+0200 [DEBUG] plugin.terraform-provider-awstest:   CompletionWindowMinutes: 180,
2019-04-06T12:11:59.641+0200 [DEBUG] plugin.terraform-provider-awstest:   Lifecycle: {
2019-04-06T12:11:59.641+0200 [DEBUG] plugin.terraform-provider-awstest:     DeleteAfterDays: 7,
2019-04-06T12:11:59.641+0200 [DEBUG] plugin.terraform-provider-awstest:     MoveToColdStorageAfterDays: 0
2019-04-06T12:11:59.641+0200 [DEBUG] plugin.terraform-provider-awstest:   },
2019-04-06T12:11:59.641+0200 [DEBUG] plugin.terraform-provider-awstest:   RecoveryPointTags: {
2019-04-06T12:11:59.641+0200 [DEBUG] plugin.terraform-provider-awstest: 
2019-04-06T12:11:59.641+0200 [DEBUG] plugin.terraform-provider-awstest:   },
2019-04-06T12:11:59.641+0200 [DEBUG] plugin.terraform-provider-awstest:   RuleName: "tf_example_backup_rule",
2019-04-06T12:11:59.641+0200 [DEBUG] plugin.terraform-provider-awstest:   ScheduleExpression: "cron(0 12 * * ? *)",
2019-04-06T12:11:59.641+0200 [DEBUG] plugin.terraform-provider-awstest:   StartWindowMinutes: 60,
2019-04-06T12:11:59.641+0200 [DEBUG] plugin.terraform-provider-awstest:   TargetBackupVaultName: "Default"
2019-04-06T12:11:59.641+0200 [DEBUG] plugin.terraform-provider-awstest: }]

It applies to both delete_after and cold_storage_after

[slapula]

If I remember correctly, the Lifecycle struct doesn't let me leave this attribute empty thus the reason for the default value. I'm not sure if this is an issue with the API or the SDK at the moment. The alternative I thought of when I wrote this resource was to just tack on 90 days to the delete_after attribute if the other attribute was left undefined but decided against it since that was an assumption I was making for the operator. I can write that option up as a PR and see what the maintainers think when I have some free time. Is this like a huge blocker for people?

[grom3k]

@slapula I've just did some tests and was able to successfully create backup plan while entirely omitting MoveToColdStorageAfterDays or DeleteAfterDays attribute.
To answer your question, I have a use case when I need to set DeleteAfterDays to 7 and leave MoveToColdStorageAfterDays "unspecified" resulting in following settings:

Never transition to cold storage
Expire after 1 week

[slapula]

@grom3k I saw your PR, thanks for catching that! Not sure why I didn't think about doing that in the first place 😁

[nywilken]

@slapula @grom3k thanks for following up on this issue. The zero value check for this resource is the fix needed. There is also a documentation change that needs to happen as the attributes are currently marked as Required. I'm sorry for the delay on this, but I do have a fix with tests written up that I planned on PRing later today. I will open up the PR to help save @grom3k time. But I am happy to retract if you would like to continue with pushing this fix forward.

[dud225]

I've been in touch with the support about failures of my backup plans created by TF, here is some useful information I got :

The reason that it is failing is because your lifecycle policy has MoveToColdStorageAfterDays set to 0, whereas it needs to be set to null. The difference may seem trivial, however a value of 0 means that it will try to move the backup to cold storage instantly (which is not allowed), however a value of null means that it will not attempt to move to cold storage at all. The console is not properly setup to reflect this value as it should not be possible to set it to 0, which is why it's hard to catch when only seeing the console output. To have it set to null, 'MoveToColdStorageAfterDays' must be left out completely.

So your command would look like this:
$ aws backup update-backup-plan --backup-plan-id < id > --backup-plan '{"BackupPlanName":"my-backup", "Rules": [{"RuleName": "dailybackups", "TargetBackupVaultName": "my-vault", "Lifecycle": {"DeleteAfterDays": 90}}]}'

In a previous message I was also told the following :

The minimum amount of days that the object can be moved to cold storage is 1 day. The current console enforces this, however the API and an older version of the console does not, so it has been possible to bypass these checks and submit an invalid lifecycle policy.

The parameters for a valid lifecycle policy are:
1. The transition to cold storage must be at least 1 day after backup creation.
2. The backup must remain in cold storage for at least 90 days before expiration.

[nywilken]

The fix for this issue has bee merged and will release with version 2.6.0 of the Terraform AWS Provider later today.

[bflad]

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

[ghost]

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!