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

tfautomv fails to find move for aws_sqs_queue #53

Open
FalconerTC opened this issue Jul 18, 2023 · 1 comment
Open

tfautomv fails to find move for aws_sqs_queue #53

FalconerTC opened this issue Jul 18, 2023 · 1 comment

Comments

@FalconerTC
Copy link

FalconerTC commented Jul 18, 2023

Hello! I like this project and would like to give some data to help improve it. There's a scenario where I'd like to be able to use tfautomv.

  # aws_sqs_queue.backup_queues["statstracking-v3-test-notification-backup"] will be created
  + resource "aws_sqs_queue" "backup_queues" {
      + arn                               = (known after apply)
      + content_based_deduplication       = false
      + deduplication_scope               = (known after apply)
      + delay_seconds                     = 0
      + fifo_queue                        = false
      + fifo_throughput_limit             = (known after apply)
      + id                                = (known after apply)
      + kms_data_key_reuse_period_seconds = (known after apply)
      + max_message_size                  = 262144
      + message_retention_seconds         = 1209600
      + name                              = "statstracking-v3-test-notification-backup"
      + name_prefix                       = (known after apply)
      + policy                            = (known after apply)
      + receive_wait_time_seconds         = 0
      + redrive_allow_policy              = (known after apply)
      + redrive_policy                    = (known after apply)
      + sqs_managed_sse_enabled           = (known after apply)
      + url                               = (known after apply)
      + visibility_timeout_seconds        = 30
    }

  # aws_sqs_queue.statstracking_notification_queue_backup will be destroyed
  # (because aws_sqs_queue.statstracking_notification_queue_backup is not in configuration)
  - resource "aws_sqs_queue" "statstracking_notification_queue_backup" {
      - arn                               = "arn:aws:sqs:eu-west-1:REDACTED:statstracking-v3-test-notification-backup" -> null
      - content_based_deduplication       = false -> null
      - delay_seconds                     = 0 -> null
      - fifo_queue                        = false -> null
      - id                                = "https://sqs.eu-west-1.amazonaws.com/REDACTED/statstracking-v3-test-notification-backup" -> null
      - kms_data_key_reuse_period_seconds = 300 -> null
      - max_message_size                  = 262144 -> null
      - message_retention_seconds         = 1209600 -> null
      - name                              = "statstracking-v3-test-notification-backup" -> null
      - receive_wait_time_seconds         = 0 -> null
      - sqs_managed_sse_enabled           = false -> null
      - url                               = "https://sqs.eu-west-1.amazonaws.com/REDACTED/statstracking-v3-test-notification-backup" -> null
      - visibility_timeout_seconds        = 30 -> null
    }

Here I have tried to refactor a resource to use a for_each. The correct move command is this

moved {
  from = aws_sqs_queue.statstracking_notification_queue_backup
  to   = aws_sqs_queue.backup_queues["statstracking-v3-test-notification-backup"]
}

It seems like tfautomv should be able to pick this up because the queue has the same name in each.

edit: Running the analysis shows the mismatch failed due to changes to the tags. I see in the README I can ignore changes to certain attributes, but it would be nice to have a way of running that identified a move based only on the "primary" attribute of a resource. In this case, two queues with the same name is desirable whereas changes to tags is part of the refactor.

│ │ Mismatch: aws_sqs_queue.statstracking_notification_queue_backup
│ │ ╷
│ │ │ + tags_all.Terraformed = "True"
│ │ │ - tags_all.Terraformed = <nil>
│ │ │ + tags_all.Environment = "Dev"
│ │ │ - tags_all.Environment = "test"
│ │ │ + tags_all.Source = "REDACTED"
│ │ │ - tags_all.Source = <nil>

I see the above even when running tfautomv -terraform-bin=terragrunt -ignore="everything:aws_sqs_queue:tags_all" -ignore="everything:aws_sqs_queue:tags"

@busser
Copy link
Owner

busser commented Jul 22, 2023

Thank you for opening this issue! Indeed, the best way to improve tfautomv is through studying cases such as yours.

The current way attribute comparison works is by flattening them into a map with no nesting. Since a single rule can only concern a single attribute — eg. a single tag — there is currently no way to tell tfautomv to ignore all tags. This makes me think that it would be nice to have some form of wildcard to match a subset of attributes. Something like all_tags.*. I am a fan of this, although the exact semantics would need to be thought through.

You mentioned another interesting idea: ignoring all attributes except a single one — the resource's ID, in essence. This could be a new rule and should be fairly straightforward to implement. Something like everything_except:my_resource:my_attribute. Again, the exact semantics would require some thought.

I am currently in the process of rewriting tfautomv's internals in order to find moves spanning different working directories. Once that is done, which should be soon, then I will gladly discuss implementing the two features detailed above. Starting work on them beforehand would yield too many conflicts, so I don't think it's worth it.

In the mean time, I would like to know what you think of these feature ideas. Do you think they would solve your problem? Do you see any ways we could improve these ideas, or any pitfalls we should avoid?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants