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

New Resource: aws_workspaces_ip_group #10904

Merged

Conversation

Tensho
Copy link
Contributor

@Tensho Tensho commented Nov 16, 2019

Relates to #434

Community Note

  • Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
  • Please do not leave "+1" comments, they generate extra noise for pull request followers and do not help prioritize the request

Release note for CHANGELOG:

**New Resource:** `aws_workspaces_ip_group`

Example

resource "aws_workspaces_ip_group" "example" {
  name = "main"
  description = "Main IP access control group"

  rules {
    source = "10.10.10.10/16"
  }

  rules {
    source = "11.11.11.11/16"
    description = "Contractors"
  }
}

Test

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestAccAwsWorkspacesIpGroup_basic'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./... -v -count 1 -parallel 20 -run=TestAccAwsWorkspacesIpGroup_basic -timeout 120m
?   	github.com/terraform-providers/terraform-provider-aws	[no test files]
=== RUN   TestAccAwsWorkspacesIpGroup_basic
=== PAUSE TestAccAwsWorkspacesIpGroup_basic
=== CONT  TestAccAwsWorkspacesIpGroup_basic
--- PASS: TestAccAwsWorkspacesIpGroup_basic (50.25s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	52.266s
testing: warning: no tests to run
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws/internal/flatmap	0.026s [no tests to run]
testing: warning: no tests to run
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags	0.053s [no tests to run]

Notes

This is my first contribution here, please don't hesitate to criticize the PR. I'm eager to make it perfect 🙂
❗️ I need the help to understand the hashing approach for TypeSet values, like rules Documentation doesn't cover how to digest the hash of the set. During HashiCorp Kyiv Meetup #8: Adding new resources to a Terraform Provider @tombuildsstuff suggested to leave all attributes verification to the intermediate import test step. I've done this way, however, I don't like implicit test and do believe explicit one should be more sustainable.

References

@Tensho Tensho requested a review from a team November 16, 2019 22:34
@ghost ghost added size/XL Managed by automation to categorize the size of a PR. needs-triage Waiting for first response or review from a maintainer. provider Pertains to the provider itself, rather than any interaction with AWS. documentation Introduces or discusses updates to documentation. service/workspaces Issues and PRs that pertain to the workspaces service. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. labels Nov 16, 2019
@Tensho Tensho force-pushed the workspaces-ip-access-control-group branch 2 times, most recently from 6f04875 to 49cdf10 Compare November 21, 2019 10:15
@Tensho
Copy link
Contributor Author

Tensho commented Nov 21, 2019

@tombuildsstuff Please, could you provide feedback regarding the latest changes after our yesterday talk?

@Tensho Tensho force-pushed the workspaces-ip-access-control-group branch from 49cdf10 to 9d151a5 Compare November 23, 2019 21:59
@gdavison gdavison self-assigned this Nov 29, 2019
@gdavison gdavison added new-resource Introduces a new resource. and removed needs-triage Waiting for first response or review from a maintainer. labels Nov 29, 2019
Copy link
Contributor

@gdavison gdavison left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, @Tensho. This looks like a good start, and I have just a few changes I'd like to see.

This resource is taggable, so it should have a tags attribute.

Please add a reference to the resource documentation in the file https://github.com/terraform-providers/terraform-provider-aws/blob/master/website/aws.erb

CHANGELOG.md Outdated Show resolved Hide resolved
aws/resource_aws_workspaces_ip_group.go Show resolved Hide resolved
aws/resource_aws_workspaces_ip_group.go Outdated Show resolved Hide resolved
@gdavison gdavison added the waiting-response Maintainers are waiting on response from community or contributor. label Nov 29, 2019
@Tensho Tensho force-pushed the workspaces-ip-access-control-group branch from 9d151a5 to 3e6149b Compare November 29, 2019 13:53
@Tensho
Copy link
Contributor Author

Tensho commented Nov 29, 2019

@gdavison Thank you for review 🙇 I've changed everything you noted. Please check it out again.

@ghost ghost removed the waiting-response Maintainers are waiting on response from community or contributor. label Nov 29, 2019
@Tensho
Copy link
Contributor Author

Tensho commented Dec 4, 2019

@gdavison Please could you help me figure out what's wrong with website rendering? CI reports an issue with the links...

@gdavison
Copy link
Contributor

gdavison commented Dec 4, 2019

I've been wondering the same, and I've looked at it a bunch of times. But I finally got it: the documentation file has the extension markdowm instead of markdown

@Tensho Tensho force-pushed the workspaces-ip-access-control-group branch from 3e6149b to a7cb29b Compare December 5, 2019 08:29
@Tensho
Copy link
Contributor Author

Tensho commented Dec 5, 2019

Nice spot 👍 Fixed!

@gdavison
Copy link
Contributor

gdavison commented Dec 6, 2019

Acceptance test results:

$ make testacc TESTARGS='-run=TestAccAwsWorkspacesIpGroup_basic'

--- PASS: TestAccAwsWorkspacesIpGroup_basic (13.47s)

@gdavison gdavison merged commit e0e9e34 into hashicorp:master Dec 6, 2019
gdavison added a commit that referenced this pull request Dec 6, 2019
@gdavison gdavison added this to the v2.42.0 milestone Dec 6, 2019
@Tensho Tensho deleted the workspaces-ip-access-control-group branch December 6, 2019 08:27
@ghost
Copy link

ghost commented Dec 13, 2019

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

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template for triage. Thanks!

@ghost
Copy link

ghost commented Mar 28, 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 Mar 28, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
documentation Introduces or discusses updates to documentation. new-resource Introduces a new resource. provider Pertains to the provider itself, rather than any interaction with AWS. service/workspaces Issues and PRs that pertain to the workspaces 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
Development

Successfully merging this pull request may close these issues.

2 participants