r/aws_sqs_queue_policy: Support import by queue URL

[bflad]

Via migrating the ID to remove the sqs-policy- prefix. Includes some other minor cleanup.

Closes #1501

make testacc TEST=./aws TESTARGS='-run=\(TestAWSSqsQueuePolicyMigrateState\|TestAccAWSSQSQueuePolicy\)'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=\(TestAWSSqsQueuePolicyMigrateState\|TestAccAWSSQSQueuePolicy\) -timeout 120m
=== RUN   TestAWSSqsQueuePolicyMigrateState
2017/12/05 01:50:46 [INFO] Found AWS SQS Query Policy State v0; migrating to v1
2017/12/05 01:50:46 [DEBUG] Attributes before migration: map[string]string{"policy":"{}", "queue_url":"https://queue.amazonaws.com/0123456789012/myqueue"}
2017/12/05 01:50:46 [DEBUG] Attributes after migration: map[string]string{"queue_url":"https://queue.amazonaws.com/0123456789012/myqueue", "id":"https://queue.amazonaws.com/0123456789012/myqueue", "policy":"{}"}, new id: https://queue.amazonaws.com/0123456789012/myqueue
--- PASS: TestAWSSqsQueuePolicyMigrateState (0.00s)
=== RUN   TestAccAWSSQSQueuePolicy_basic
--- PASS: TestAccAWSSQSQueuePolicy_basic (15.93s)
=== RUN   TestAccAWSSQSQueuePolicy_import
--- PASS: TestAccAWSSQSQueuePolicy_import (16.23s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	32.198s

[Ninir]

Hey @bflad

This is looking very good to me :)
Just asking Radek about the will behind the initial ID string, otherwise this is looking very good!

$ make testacc TEST=./aws TESTARGS='-run=\(TestAWSSqsQueuePolicyMigrateState\|TestAccAWSSQSQueuePolicy\)'

==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=\(TestAWSSqsQueuePolicyMigrateState\|TestAccAWSSQSQueuePolicy\) -timeout 120m
=== RUN   TestAWSSqsQueuePolicyMigrateState
2017/12/08 15:48:44 [INFO] Found AWS SQS Query Policy State v0; migrating to v1
2017/12/08 15:48:44 [DEBUG] Attributes before migration: map[string]string{"policy":"{}", "queue_url":"https://queue.amazonaws.com/0123456789012/myqueue"}
2017/12/08 15:48:44 [DEBUG] Attributes after migration: map[string]string{"queue_url":"https://queue.amazonaws.com/0123456789012/myqueue", "id":"https://queue.amazonaws.com/0123456789012/myqueue", "policy":"{}"}, new id: https://queue.amazonaws.com/0123456789012/myqueue
--- PASS: TestAWSSqsQueuePolicyMigrateState (0.00s)
=== RUN   TestAccAWSSQSQueuePolicy_basic
--- PASS: TestAccAWSSQSQueuePolicy_basic (15.37s)
=== RUN   TestAccAWSSQSQueuePolicy_import
--- PASS: TestAccAWSSQSQueuePolicy_import (15.93s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	31.348s

[Ninir]

@radeksimko Do you recall why this was set as is?

[radeksimko]

I don't think there was a strong reason - maybe just to separate it from SQS queue IDs, but really we don't need to do that.

However I'd be cautious about changing the ID here in this context. Can you please verify this actually works for existing SQS policy which is already in state with the old ID? I suspect refresh might fail because it will try to read queue attributes for queue URL sqs-policy-"+url.

In other words I believe we need state migration.

[bflad]

Hi @radeksimko, are you referring to something different than my state migration implementation?

MigrateState:  resourceAwsSqsQueuePolicyMigrateState,
SchemaVersion: 1,

If there's a better way to handle that, please let me know!

[radeksimko]

doh 🤦‍♂️ sorry! I missed that one... In that case it's all good! Thanks.

[radeksimko]

One thing I wanted to add (also to explain the upstream-terraform label) - we have some bigger plans around imports in the core to address the problem of "importing by multiple different fields" in provider/resource-agnostic way.

I cannot share any timelines around that, but I'd suggest we avoid spending more time on resource-specific solutions for the time being.

[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!