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

New resource: aws_iot_thing_principal_attachment #5868

Merged
merged 4 commits into from
Oct 29, 2018
Merged

New resource: aws_iot_thing_principal_attachment #5868

merged 4 commits into from
Oct 29, 2018

Conversation

spirius
Copy link
Contributor

@spirius spirius commented Sep 13, 2018

Added aws_iot_thing_principal_attachment resource.

Test results:

$ make testacc TEST=./aws TESTARGS='-run=TestAccAWSIotThingPrincipalAttachment_basic'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSIotThingPrincipalAttachment_basic -timeout 120m
=== RUN   TestAccAWSIotThingPrincipalAttachment_basic
--- PASS: TestAccAWSIotThingPrincipalAttachment_basic (22.51s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	22.549s

@ghost ghost added the size/L Managed by automation to categorize the size of a PR. label Sep 13, 2018
@bflad bflad added new-resource Introduces a new resource. service/iot Issues and PRs that pertain to the iot service. labels Sep 13, 2018
@ghost ghost added the size/L Managed by automation to categorize the size of a PR. label Sep 17, 2018
@spirius
Copy link
Contributor Author

spirius commented Sep 17, 2018

hi @bflad, is there anything missing for this PR to be merged?

@bflad
Copy link
Contributor

bflad commented Sep 28, 2018

Since this PR looks like it would receive some of the same feedback as #5864, I'll let you update this one as well before review. 👍

@bflad bflad mentioned this pull request Oct 15, 2018
Closed
7 tasks
@bflad bflad added the waiting-response Maintainers are waiting on response from community or contributor. label Oct 15, 2018
@ghost ghost added size/XL Managed by automation to categorize the size of a PR. documentation Introduces or discusses updates to documentation. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. and removed size/L Managed by automation to categorize the size of a PR. labels Oct 20, 2018
@spirius spirius mentioned this pull request Oct 20, 2018
Merged
@bflad bflad removed the waiting-response Maintainers are waiting on response from community or contributor. label Oct 29, 2018
bflad
bflad approved these changes Oct 29, 2018
View reviewed changes
Copy link
Contributor

@bflad bflad left a comment

Choose a reason for hiding this comment

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

LGTM, @spirius, with one documentation fix which I can take care of post-merge. Thanks! 🚀

--- PASS: TestAccAWSIotThingPrincipalAttachment_basic (17.32s)

website/aws.erb
@@ -1408,6 +1408,11 @@
<li<%= sidebar_current("docs-aws-resource-iot-thing") %>>
<a href="/docs/providers/aws/r/iot_thing.html">aws_iot_thing</a>
</li>

<li<%= sidebar_current("docs-aws-resource-iot-thing-principal-attachment") %>>
<a href="/docs/providers/aws/r/iot_thing.html">aws_iot_thing_principal_attachment</a>
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing _principal_attachment 😄

aws/resource_aws_iot_thing_principal_attachment.go
})
if isAWSErr(err, iot.ErrCodeResourceNotFoundException, "") {
return false, nil
} else if err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Since the previous conditional returns, the else is extraneous. 👍

aws/resource_aws_iot_thing_principal_attachment.go
found := false
for _, name := range out.Principals {
if principal == aws.StringValue(name) {
found = true
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Since this is only intended to trigger an immediate return, the found logic can be removed and instead this can return true, nil here while below the loop does return false, nil

@bflad bflad added this to the v1.42.0 milestone Oct 29, 2018
bflad added a commit that referenced this pull request Oct 29, 2018
@bflad
Update CHANGELOG for #5868
49ac708
@bflad bflad merged commit afbd540 into hashicorp:master Oct 29, 2018
@bflad
Copy link
Contributor

bflad commented Nov 1, 2018

This has been released in version 1.42.0 of the AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

@spirius spirius deleted the feature/iot-thing-principal-attachment branch November 12, 2018 17:40
@ghost
Copy link

ghost commented Apr 2, 2020

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 ghost locked and limited conversation to collaborators Apr 2, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@bflad bflad bflad approved these changes

Assignees
No one assigned
Labels
documentation Introduces or discusses updates to documentation. new-resource Introduces a new resource. service/iot Issues and PRs that pertain to the iot service. size/XL Managed by automation to categorize the size of a PR. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure.
Projects
None yet
Milestone
v1.42.0
Development

Successfully merging this pull request may close these issues.
2 participants
@spirius @bflad