Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Added latest_restorable_time to ignore_changes #356

Merged
merged 5 commits into from Jan 12, 2022

Conversation

FieryCod
Copy link
Contributor

Ignore latest_restorable_time property

Description

Note: Objects have changed outside of Terraform

Terraform detected the following changes made outside of Terraform since the
last "terraform apply":

  # module.xxx.module.db.module.db_instance.aws_db_instance.this[0] has been changed
  ~ resource "aws_db_instance" "this" {
        id                                    = "xxx"
      ~ latest_restorable_time                = "2021-09-20T18:59:05Z" -> "2021-09-20T19:04:05Z"
        name                                  = "xx"
        tags                                  = {
            "Environment" = "staging"
            "Name"        = "xxxx"
            "Owner"       = "xxx"
        }
        # (57 unchanged attributes hidden)

        # (1 unchanged block hidden)
    }

Unless you have made equivalent changes to your configuration, or ignored the
relevant attributes using ignore_changes, the following plan may include
actions to undo or respond to these changes.

Motivation and Context

I would rather not see the outside change of lastest_restorable_time on every apply.

Breaking Changes

None

How Has This Been Tested?

  • I have tested and validated these changes using one or more of the provided examples/* projects

Ignore latest_restorable_time property
@drmaciej
Copy link

Hi @FieryCod is there any chance you could fix this PR so it's in a mergeable state? I would really love to see it, this change on lastest_restorable_time is really annoying to me as well :)

@FieryCod
Copy link
Contributor Author

@drmaciej done.

@drmaciej
Copy link

@FieryCod thanks much!
let's now hope the maintainers can review this soon 🤞

Copy link
Member

@bryantbiggs bryantbiggs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@antonbabenko I believe this one should be acceptable since its an attribute not an argument https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/db_instance#latest_restorable_time

related to upstream AWS provider issue hashicorp/terraform-provider-aws#19727

@antonbabenko antonbabenko changed the title Ignore latest_restorable_time property feat: Added latest_restorable_time to ignore_changes Nov 22, 2021
@antonbabenko
Copy link
Member

I agree. Objects have changed outside of Terraform type of errors are rather painful (20 open issues mentioning it).

Let's fix the CI and it will be ready to merge.

@korovkintaptap
Copy link

can we please merge this ?

@antonbabenko antonbabenko merged commit 77902c2 into terraform-aws-modules:master Jan 12, 2022
antonbabenko pushed a commit that referenced this pull request Jan 12, 2022
## [3.5.0](v3.4.2...v3.5.0) (2022-01-12)

### Features

* Added `latest_restorable_time` to ignore_changes ([#356](#356)) ([77902c2](77902c2))
@antonbabenko
Copy link
Member

This PR is included in version 3.5.0 🎉

@korovkintaptap
Copy link

awesome, unfortunately it doesn't work though
i forked and tried the change, it doesn't change anything

@bryantbiggs
Copy link
Member

i forked and tried the change, it doesn't change anything

could you elaborate? the lifecycle ignore is intended to ignore any future changes so if that is happening then it does work. if latest_restorable_time is still showing up in diffs, then its not working

just a few more words/details makes a world of a difference 😉

@danielcompton
Copy link

I have tested this and found that this PR doesn't have the desired effect.

I updated to 3.5.0 and ran terraform apply -refresh-only. After waiting 10 minutes, running terraform plan shows drift again from latest_restorable_time.

I don't think that ignore_changes works the way that we'd want it to for this case, see hashicorp/terraform#28803 (comment) for more info on this.

@antonbabenko
Copy link
Member

This issue has been resolved in version 4.0.0 🎉

@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 13, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants