Issue #143 - Add aws_iot_policy resource based on work from @jhedev #986
Conversation
grubernaut
added
enhancement
AlexMabry
changed the title
AlexMabry
changed the title
aws/resource_aws_iot_policy_test.go
Outdated
| Config: testAccAWSIoTPolicy_basic, | ||
| Check: resource.ComposeTestCheckFunc( | ||
| testAccCheckAWSIoTPolicyExists_basic("aws_iot_policy.pubsub"), | ||
| ), |
There was a problem hiding this comment.
Because we run all acceptance tests nightly in parallel (and because tests sometimes do fail and leave named resources behind, blocking the same test from running again) we need to avoid static names.
Have a look at what we do in other resources, e.g.
https:/terraform-providers/terraform-provider-aws/blob/master/aws/resource_aws_dynamodb_table_test.go#L19
Also do you mind adding a few more checks here? e.g.
resource.TestCheckResourceAttr("aws_iot_policy.pubsub", "name", rName),
resource.TestCheckResourceAttrSet("aws_iot_policy.pubsub", "arn"),
resource.TestCheckResourceAttrSet("aws_iot_policy.pubsub", "default_version_id"),
aws/resource_aws_iot_policy_test.go
Outdated
| continue | ||
| } | ||
|
|
||
| out, err := conn.ListPolicies(&iot.ListPoliciesInput{}) |
There was a problem hiding this comment.
Wouldn't it be easier to call GetPolicy here as we only care about the single policy we created as part of the test in this context?
aws/resource_aws_iot_policy.go
Outdated
| } | ||
|
|
||
| d.Set("arn", out.PolicyArn) | ||
| d.Set("defaultVersionId", out.DefaultVersionId) |
There was a problem hiding this comment.
👀 I think the field name should be default_version_id.
| sidebar_current: "docs-aws-resource-iot-policy" | ||
| description: |- | ||
| Provides an IoT policy. | ||
| --- |
There was a problem hiding this comment.
We also need to add a link to the sidebar to make this page discoverable, see https:/terraform-providers/terraform-provider-aws/blob/master/website/aws.erb
aws/resource_aws_iot_policy_test.go
Outdated
| return nil | ||
| } | ||
|
|
||
| func testAccCheckAWSIoTPolicyExists_basic(name string) resource.TestCheckFunc { |
There was a problem hiding this comment.
We usually leverage helpers like resource.TestCheckResourceAttr and resource.TestCheckResourceAttrSet to check whether the resource was recorded in the state.
The point of having a function like this is to allow more thorough check of the API/SDK struct, take a look how we do it in a VPC test: https:/terraform-providers/terraform-provider-aws/blob/master/aws/resource_aws_vpc_test.go#L25-L26
It's not mandatory to perform such thorough checking, but if you decide not to do it there's no point of having this function when the standard helpers can do the job.
radeksimko
added
the
waiting-response
radeksimko
removed
the
waiting-response
|
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! |
ghost
locked and limited conversation to collaborators
My goal is to add support for the AWS IoT service. This is my first time committing, so I just wanted to add a simple resource to get my feet wet. I believe I am followed the guidelines, but please point out anything that needs to be added or change.