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.

Sign up for GitHub

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

Jump to bottom

resource/aws_sqs_queue_policy: Prevent missing policy error on read #2739

Merged
merged 2 commits into from
Jan 22, 2018
Merged

resource/aws_sqs_queue_policy: Prevent missing policy error on read #2739

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
811c457
r/aws_sqs_queue_policy: Add StateChangeConf for create/update, remove…
bflad Dec 21, 2017
91a723f
resource/aws_sqs_queue_policy: Use resource.Retry instead of resource…
bflad Jan 18, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 41 additions & 4 deletions aws/resource_aws_sqs_queue_policy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,13 @@ package aws
import (
"fmt"
"log"
"time"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/sqs"
"github.com/hashicorp/terraform/helper/resource"
"github.com/hashicorp/terraform/helper/schema"
"github.com/jen20/awspolicyequivalence"
)

func resourceAwsSqsQueuePolicy() *schema.Resource {
Expand Down Expand Up @@ -40,18 +43,51 @@ func resourceAwsSqsQueuePolicy() *schema.Resource {

func resourceAwsSqsQueuePolicyUpsert(d *schema.ResourceData, meta interface{}) error {
conn := meta.(*AWSClient).sqsconn
policy := d.Get("policy").(string)
url := d.Get("queue_url").(string)

_, err := conn.SetQueueAttributes(&sqs.SetQueueAttributesInput{
QueueUrl: aws.String(url),
Attributes: aws.StringMap(map[string]string{
"Policy": d.Get("policy").(string),
"Policy": policy,
}),
})
if err != nil {
return fmt.Errorf("Error updating SQS attributes: %s", err)
}

// https://docs.aws.amazon.com/AWSSimpleQueueService/latest/APIReference/API_SetQueueAttributes.html
// When you change a queue's attributes, the change can take up to 60 seconds
// for most of the attributes to propagate throughout the Amazon SQS system.
wait := resource.StateChangeConf{
Pending: []string{""},
Target: []string{"SQS queue policy updated"},
Timeout: 1 * time.Minute,
MinTimeout: 1 * time.Second,
Refresh: func() (interface{}, string, error) {
out, err := conn.GetQueueAttributes(&sqs.GetQueueAttributesInput{
QueueUrl: aws.String(url),
AttributeNames: []*string{aws.String("Policy")},
})
if err != nil {
return out, "", err
}
queuePolicy, ok := out.Attributes["Policy"]
if ok {
equivalent, err := awspolicy.PoliciesAreEquivalent(*queuePolicy, policy)
if err != nil || !equivalent {
return out, "", nil
}
return out, "SQS queue policy updated", nil
}
return out, "", nil
},
}
_, err = wait.WaitForState()
Copy link
Member

Choose a reason for hiding this comment

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

Have you considered using the resource.Retry helper instead of this wrapper? We'd still need to make up the error message when policies aren't equivalent, but I feel that it'd be a little bit cleaner as the whole thing isn't really "stateful". 🤔

Copy link
Member

Choose a reason for hiding this comment

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

Also we're catching all errors here and treating them as "retryable" which (IMO) isn't ideal. I assume the goal is to only catch AWS.SimpleQueueService.NonExistentQueue?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call on both of these -- I'll get this PR updated shortly.

if err != nil {
return err
}

d.SetId(url)

return resourceAwsSqsQueuePolicyRead(d, meta)
Expand All @@ -77,11 +113,12 @@ func resourceAwsSqsQueuePolicyRead(d *schema.ResourceData, meta interface{}) err
}

policy, ok := out.Attributes["Policy"]
if !ok {
return fmt.Errorf("SQS Queue policy not found for %s", d.Id())
if ok {
d.Set("policy", policy)
} else {
d.Set("policy", "")
}

d.Set("policy", policy)
d.Set("queue_url", d.Id())

return nil
Expand Down