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

Add support for security group rule description #1587

Merged
merged 1 commit into from
Oct 10, 2017
Merged

Add support for security group rule description #1587

merged 1 commit into from
Oct 10, 2017

Conversation

ewbankkit
Copy link
Contributor

Fixes #1554.
For backwards compatibility and simplicity I am going with one description per rule resource.
I can justify this to myself as saying it's a description of the rule as a whole rather than one per CIDR range/PL/peer SG.
The same description is set on each CIDR/PL/peer SG and on Read the last non-empty description will be the value for the rule.
Still TODO - Inline rules.

@ewbankkit ewbankkit changed the title [WIP] Add support for security group rule description. [WIP] Add support for security group rule description Sep 4, 2017
@ewbankkit
Copy link
Contributor Author

Acceptance tests so far:

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

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

@plukevdh
Copy link

plukevdh commented Sep 5, 2017

Yes please!

@ewbankkit
Copy link
Contributor Author

Additional acceptance test modified:

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

Still TODO:

  • Acceptance tests for inline security group rules
  • Changes to matchRules?
  • Changes to resourceAwsSecurityGroupUpdateRules given that a change to a rule's description doesn't require the rule to be destroyed and then added back

@ewbankkit ewbankkit mentioned this pull request Sep 6, 2017
Closed
@Ninir Ninir added the enhancement Requests to existing resources that expand the functionality or scope. label Sep 6, 2017
@barnesm999 barnesm999 mentioned this pull request Sep 11, 2017
Closed
@ewbankkit ewbankkit force-pushed the security-group-rules-description branch from b900f48 to 7c21e81 Compare September 12, 2017 01:22
@ewbankkit
Copy link
Contributor Author

Rebased and squashed.
I have new acceptance tests passing.
Still TODO:

  • Ensure no regressions

@Ninir Ninir mentioned this pull request Sep 13, 2017
Closed
@ewbankkit ewbankkit force-pushed the security-group-rules-description branch from 7c21e81 to e46b79b Compare September 13, 2017 23:38
@ewbankkit
Copy link
Contributor Author

Rebased and squashed.
All acceptance tests now passing:

make testacc TEST=./aws/ TESTARGS='-run=TestAccAWSSecurityGroupRule_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -run=TestAccAWSSecurityGroupRule_ -timeout 120m
=== RUN   TestAccAWSSecurityGroupRule_Ingress_VPC
--- PASS: TestAccAWSSecurityGroupRule_Ingress_VPC (19.77s)
=== RUN   TestAccAWSSecurityGroupRule_Ingress_Protocol
--- PASS: TestAccAWSSecurityGroupRule_Ingress_Protocol (33.58s)
=== RUN   TestAccAWSSecurityGroupRule_Ingress_Ipv6
--- PASS: TestAccAWSSecurityGroupRule_Ingress_Ipv6 (34.28s)
=== RUN   TestAccAWSSecurityGroupRule_Ingress_Classic
--- PASS: TestAccAWSSecurityGroupRule_Ingress_Classic (10.58s)
=== RUN   TestAccAWSSecurityGroupRule_MultiIngress
--- PASS: TestAccAWSSecurityGroupRule_MultiIngress (22.29s)
=== RUN   TestAccAWSSecurityGroupRule_Egress
--- PASS: TestAccAWSSecurityGroupRule_Egress (19.66s)
=== RUN   TestAccAWSSecurityGroupRule_SelfReference
--- PASS: TestAccAWSSecurityGroupRule_SelfReference (32.65s)
=== RUN   TestAccAWSSecurityGroupRule_ExpectInvalidTypeError
--- PASS: TestAccAWSSecurityGroupRule_ExpectInvalidTypeError (1.31s)
=== RUN   TestAccAWSSecurityGroupRule_ExpectInvalidCIDR
--- PASS: TestAccAWSSecurityGroupRule_ExpectInvalidCIDR (1.29s)
=== RUN   TestAccAWSSecurityGroupRule_PartialMatching_basic
--- PASS: TestAccAWSSecurityGroupRule_PartialMatching_basic (39.74s)
=== RUN   TestAccAWSSecurityGroupRule_PartialMatching_Source
--- PASS: TestAccAWSSecurityGroupRule_PartialMatching_Source (36.97s)
=== RUN   TestAccAWSSecurityGroupRule_Issue5310
--- PASS: TestAccAWSSecurityGroupRule_Issue5310 (10.33s)
=== RUN   TestAccAWSSecurityGroupRule_Race
--- PASS: TestAccAWSSecurityGroupRule_Race (280.53s)
=== RUN   TestAccAWSSecurityGroupRule_SelfSource
--- PASS: TestAccAWSSecurityGroupRule_SelfSource (33.11s)
=== RUN   TestAccAWSSecurityGroupRule_PrefixListEgress
--- PASS: TestAccAWSSecurityGroupRule_PrefixListEgress (36.60s)
=== RUN   TestAccAWSSecurityGroupRule_IngressDescription
--- PASS: TestAccAWSSecurityGroupRule_IngressDescription (20.22s)
=== RUN   TestAccAWSSecurityGroupRule_EgressDescription
--- PASS: TestAccAWSSecurityGroupRule_EgressDescription (19.43s)
=== RUN   TestAccAWSSecurityGroupRule_IngressDescription_updates
--- PASS: TestAccAWSSecurityGroupRule_IngressDescription_updates (31.33s)
=== RUN   TestAccAWSSecurityGroupRule_EgressDescription_updates
--- PASS: TestAccAWSSecurityGroupRule_EgressDescription_updates (30.96s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	714.652s

make testacc TEST=./aws/ TESTARGS='-run=TestAccAWSSecurityGroup_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -run=TestAccAWSSecurityGroup_ -timeout 120m
=== RUN   TestAccAWSSecurityGroup_importBasic
--- PASS: TestAccAWSSecurityGroup_importBasic (32.62s)
=== RUN   TestAccAWSSecurityGroup_importIpv6
--- PASS: TestAccAWSSecurityGroup_importIpv6 (31.69s)
=== RUN   TestAccAWSSecurityGroup_importSelf
--- PASS: TestAccAWSSecurityGroup_importSelf (36.46s)
=== RUN   TestAccAWSSecurityGroup_importSourceSecurityGroup
--- PASS: TestAccAWSSecurityGroup_importSourceSecurityGroup (36.32s)
=== RUN   TestAccAWSSecurityGroup_importIPRangeAndSecurityGroupWithSameRules
--- PASS: TestAccAWSSecurityGroup_importIPRangeAndSecurityGroupWithSameRules (39.99s)
=== RUN   TestAccAWSSecurityGroup_importIPRangesWithSameRules
--- PASS: TestAccAWSSecurityGroup_importIPRangesWithSameRules (38.38s)
=== RUN   TestAccAWSSecurityGroup_importPrefixList
--- PASS: TestAccAWSSecurityGroup_importPrefixList (39.52s)
=== RUN   TestAccAWSSecurityGroup_basic
--- PASS: TestAccAWSSecurityGroup_basic (30.20s)
=== RUN   TestAccAWSSecurityGroup_basicRuleDescription
--- PASS: TestAccAWSSecurityGroup_basicRuleDescription (30.58s)
=== RUN   TestAccAWSSecurityGroup_ipv6
--- PASS: TestAccAWSSecurityGroup_ipv6 (30.96s)
=== RUN   TestAccAWSSecurityGroup_tagsCreatedFirst
--- PASS: TestAccAWSSecurityGroup_tagsCreatedFirst (21.46s)
=== RUN   TestAccAWSSecurityGroup_namePrefix
--- PASS: TestAccAWSSecurityGroup_namePrefix (10.06s)
=== RUN   TestAccAWSSecurityGroup_self
--- PASS: TestAccAWSSecurityGroup_self (30.06s)
=== RUN   TestAccAWSSecurityGroup_vpc
--- PASS: TestAccAWSSecurityGroup_vpc (30.61s)
=== RUN   TestAccAWSSecurityGroup_vpcNegOneIngress
--- PASS: TestAccAWSSecurityGroup_vpcNegOneIngress (29.91s)
=== RUN   TestAccAWSSecurityGroup_vpcProtoNumIngress
--- PASS: TestAccAWSSecurityGroup_vpcProtoNumIngress (29.34s)
=== RUN   TestAccAWSSecurityGroup_MultiIngress
--- PASS: TestAccAWSSecurityGroup_MultiIngress (35.70s)
=== RUN   TestAccAWSSecurityGroup_Change
--- PASS: TestAccAWSSecurityGroup_Change (49.60s)
=== RUN   TestAccAWSSecurityGroup_ChangeRuleDescription
--- PASS: TestAccAWSSecurityGroup_ChangeRuleDescription (68.28s)
=== RUN   TestAccAWSSecurityGroup_generatedName
--- PASS: TestAccAWSSecurityGroup_generatedName (30.75s)
=== RUN   TestAccAWSSecurityGroup_DefaultEgress_VPC
--- PASS: TestAccAWSSecurityGroup_DefaultEgress_VPC (29.92s)
=== RUN   TestAccAWSSecurityGroup_DefaultEgress_Classic
--- PASS: TestAccAWSSecurityGroup_DefaultEgress_Classic (9.39s)
=== RUN   TestAccAWSSecurityGroup_drift
--- PASS: TestAccAWSSecurityGroup_drift (17.32s)
=== RUN   TestAccAWSSecurityGroup_drift_complex
--- PASS: TestAccAWSSecurityGroup_drift_complex (36.07s)
=== RUN   TestAccAWSSecurityGroup_invalidCIDRBlock
--- PASS: TestAccAWSSecurityGroup_invalidCIDRBlock (1.49s)
=== RUN   TestAccAWSSecurityGroup_tags
--- PASS: TestAccAWSSecurityGroup_tags (48.70s)
=== RUN   TestAccAWSSecurityGroup_CIDRandGroups
--- PASS: TestAccAWSSecurityGroup_CIDRandGroups (36.21s)
=== RUN   TestAccAWSSecurityGroup_ingressWithCidrAndSGs
--- PASS: TestAccAWSSecurityGroup_ingressWithCidrAndSGs (35.52s)
=== RUN   TestAccAWSSecurityGroup_ingressWithCidrAndSGs_classic
--- PASS: TestAccAWSSecurityGroup_ingressWithCidrAndSGs_classic (11.40s)
=== RUN   TestAccAWSSecurityGroup_egressWithPrefixList
--- PASS: TestAccAWSSecurityGroup_egressWithPrefixList (37.31s)
=== RUN   TestAccAWSSecurityGroup_ipv4andipv6Egress
--- PASS: TestAccAWSSecurityGroup_ipv4andipv6Egress (30.86s)
=== RUN   TestAccAWSSecurityGroup_failWithDiffMismatch
--- PASS: TestAccAWSSecurityGroup_failWithDiffMismatch (35.92s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	1012.596s

@ewbankkit
Copy link
Contributor Author

OK, removing WIP and declaring ready-for-review.
As I say at the top, the approach I have taken is to have a single optional description per rule. This leads to a single description per Ingress/Egress with the same description value being set for every CIDR/IPV6 CIDR/PL/SG in the the underlying AWS API.
When reading back the resource, the last non-empty description on the Ingress/Egress is used as the description for the rule.

@ewbankkit ewbankkit changed the title [WIP] Add support for security group rule description Add support for security group rule description Sep 13, 2017
Ninir
Ninir reviewed Sep 18, 2017
View reviewed changes
Copy link
Contributor

@Ninir Ninir left a comment

Choose a reason for hiding this comment

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

Hi @ewbankkit

Just tried your work and it works pretty nicely!

$ make testacc TEST=./aws TESTARGS='-run=TestAccAWSSecurityGroup'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSSecurityGroup -timeout 120m
=== RUN   TestAccAWSSecurityGroup_importBasic
--- PASS: TestAccAWSSecurityGroup_importBasic (53.94s)
=== RUN   TestAccAWSSecurityGroup_importIpv6
--- PASS: TestAccAWSSecurityGroup_importIpv6 (55.96s)
=== RUN   TestAccAWSSecurityGroup_importSelf
--- PASS: TestAccAWSSecurityGroup_importSelf (61.62s)
=== RUN   TestAccAWSSecurityGroup_importSourceSecurityGroup
--- PASS: TestAccAWSSecurityGroup_importSourceSecurityGroup (59.31s)
=== RUN   TestAccAWSSecurityGroup_importIPRangeAndSecurityGroupWithSameRules
--- PASS: TestAccAWSSecurityGroup_importIPRangeAndSecurityGroupWithSameRules (65.59s)
=== RUN   TestAccAWSSecurityGroup_importIPRangesWithSameRules
--- PASS: TestAccAWSSecurityGroup_importIPRangesWithSameRules (61.66s)
=== RUN   TestAccAWSSecurityGroup_importPrefixList
--- PASS: TestAccAWSSecurityGroup_importPrefixList (65.62s)
=== RUN   TestAccAWSSecurityGroupRule_Ingress_VPC
--- PASS: TestAccAWSSecurityGroupRule_Ingress_VPC (30.29s)
=== RUN   TestAccAWSSecurityGroupRule_Ingress_Protocol
--- PASS: TestAccAWSSecurityGroupRule_Ingress_Protocol (55.84s)
=== RUN   TestAccAWSSecurityGroupRule_Ingress_Ipv6
--- PASS: TestAccAWSSecurityGroupRule_Ingress_Ipv6 (57.76s)
=== RUN   TestAccAWSSecurityGroupRule_Ingress_Classic
--- PASS: TestAccAWSSecurityGroupRule_Ingress_Classic (22.28s)
=== RUN   TestAccAWSSecurityGroupRule_MultiIngress
--- PASS: TestAccAWSSecurityGroupRule_MultiIngress (34.81s)
=== RUN   TestAccAWSSecurityGroupRule_Egress
--- PASS: TestAccAWSSecurityGroupRule_Egress (33.22s)
=== RUN   TestAccAWSSecurityGroupRule_SelfReference
--- PASS: TestAccAWSSecurityGroupRule_SelfReference (55.29s)
=== RUN   TestAccAWSSecurityGroupRule_ExpectInvalidTypeError
--- PASS: TestAccAWSSecurityGroupRule_ExpectInvalidTypeError (1.80s)
=== RUN   TestAccAWSSecurityGroupRule_ExpectInvalidCIDR
--- PASS: TestAccAWSSecurityGroupRule_ExpectInvalidCIDR (1.92s)
=== RUN   TestAccAWSSecurityGroupRule_PartialMatching_basic
--- PASS: TestAccAWSSecurityGroupRule_PartialMatching_basic (60.30s)
=== RUN   TestAccAWSSecurityGroupRule_PartialMatching_Source
--- PASS: TestAccAWSSecurityGroupRule_PartialMatching_Source (60.13s)
=== RUN   TestAccAWSSecurityGroupRule_Issue5310
--- PASS: TestAccAWSSecurityGroupRule_Issue5310 (21.43s)
=== RUN   TestAccAWSSecurityGroupRule_Race
--- PASS: TestAccAWSSecurityGroupRule_Race (446.37s)
=== RUN   TestAccAWSSecurityGroupRule_SelfSource
--- PASS: TestAccAWSSecurityGroupRule_SelfSource (53.41s)
=== RUN   TestAccAWSSecurityGroupRule_PrefixListEgress
--- PASS: TestAccAWSSecurityGroupRule_PrefixListEgress (61.44s)
=== RUN   TestAccAWSSecurityGroupRule_IngressDescription
--- PASS: TestAccAWSSecurityGroupRule_IngressDescription (30.39s)
=== RUN   TestAccAWSSecurityGroupRule_EgressDescription
--- PASS: TestAccAWSSecurityGroupRule_EgressDescription (30.24s)
=== RUN   TestAccAWSSecurityGroupRule_IngressDescription_updates
--- PASS: TestAccAWSSecurityGroupRule_IngressDescription_updates (48.28s)
=== RUN   TestAccAWSSecurityGroupRule_EgressDescription_updates
--- PASS: TestAccAWSSecurityGroupRule_EgressDescription_updates (47.24s)
=== RUN   TestAccAWSSecurityGroup_basic
--- PASS: TestAccAWSSecurityGroup_basic (50.98s)
=== RUN   TestAccAWSSecurityGroup_basicRuleDescription
--- PASS: TestAccAWSSecurityGroup_basicRuleDescription (51.15s)
=== RUN   TestAccAWSSecurityGroup_ipv6
--- PASS: TestAccAWSSecurityGroup_ipv6 (50.88s)
=== RUN   TestAccAWSSecurityGroup_tagsCreatedFirst
--- PASS: TestAccAWSSecurityGroup_tagsCreatedFirst (34.93s)
=== RUN   TestAccAWSSecurityGroup_namePrefix
--- PASS: TestAccAWSSecurityGroup_namePrefix (18.59s)
=== RUN   TestAccAWSSecurityGroup_self
--- PASS: TestAccAWSSecurityGroup_self (49.11s)
=== RUN   TestAccAWSSecurityGroup_vpc
--- PASS: TestAccAWSSecurityGroup_vpc (48.89s)
=== RUN   TestAccAWSSecurityGroup_vpcNegOneIngress
--- PASS: TestAccAWSSecurityGroup_vpcNegOneIngress (49.06s)
=== RUN   TestAccAWSSecurityGroup_vpcProtoNumIngress
--- PASS: TestAccAWSSecurityGroup_vpcProtoNumIngress (48.85s)
=== RUN   TestAccAWSSecurityGroup_MultiIngress
--- PASS: TestAccAWSSecurityGroup_MultiIngress (63.72s)
=== RUN   TestAccAWSSecurityGroup_Change
--- PASS: TestAccAWSSecurityGroup_Change (81.37s)
=== RUN   TestAccAWSSecurityGroup_ChangeRuleDescription
--- PASS: TestAccAWSSecurityGroup_ChangeRuleDescription (117.43s)
=== RUN   TestAccAWSSecurityGroup_generatedName
--- PASS: TestAccAWSSecurityGroup_generatedName (50.99s)
=== RUN   TestAccAWSSecurityGroup_DefaultEgress_VPC
--- PASS: TestAccAWSSecurityGroup_DefaultEgress_VPC (50.16s)
=== RUN   TestAccAWSSecurityGroup_DefaultEgress_Classic
--- PASS: TestAccAWSSecurityGroup_DefaultEgress_Classic (18.71s)
=== RUN   TestAccAWSSecurityGroup_drift
--- PASS: TestAccAWSSecurityGroup_drift (26.14s)
=== RUN   TestAccAWSSecurityGroup_drift_complex
--- PASS: TestAccAWSSecurityGroup_drift_complex (59.58s)
=== RUN   TestAccAWSSecurityGroup_invalidCIDRBlock
--- PASS: TestAccAWSSecurityGroup_invalidCIDRBlock (1.98s)
=== RUN   TestAccAWSSecurityGroup_tags
--- PASS: TestAccAWSSecurityGroup_tags (80.29s)
=== RUN   TestAccAWSSecurityGroup_CIDRandGroups
--- PASS: TestAccAWSSecurityGroup_CIDRandGroups (62.04s)
=== RUN   TestAccAWSSecurityGroup_ingressWithCidrAndSGs
--- PASS: TestAccAWSSecurityGroup_ingressWithCidrAndSGs (61.73s)
=== RUN   TestAccAWSSecurityGroup_ingressWithCidrAndSGs_classic
--- FAIL: TestAccAWSSecurityGroup_ingressWithCidrAndSGs_classic (19.72s)
	testing.go:434: Step 0 error: Error applying: 1 error(s) occurred:
		
		* aws_security_group.web: 1 error(s) occurred:
		
		* aws_security_group.web: Error authorizing security group ingress rules: InvalidGroupId.Malformed: Invalid id: "tf_other_acc_tests" (expecting "sg-...")
			status code: 400, request id: 7c29f619-8d32-41e0-8afb-bbf7cc009925
=== RUN   TestAccAWSSecurityGroup_egressWithPrefixList
--- PASS: TestAccAWSSecurityGroup_egressWithPrefixList (63.72s)
=== RUN   TestAccAWSSecurityGroup_ipv4andipv6Egress
--- PASS: TestAccAWSSecurityGroup_ipv4andipv6Egress (51.08s)
=== RUN   TestAccAWSSecurityGroup_failWithDiffMismatch
--- PASS: TestAccAWSSecurityGroup_failWithDiffMismatch (60.40s)
FAIL
exit status 1
FAIL	github.com/terraform-providers/terraform-provider-aws/aws	2847.685s
make: *** [testacc] Error 1

(This error is linked to my personal account, nothing to worry about here)

There is just one use-case that is not working on my side:

  1. Create the security group with description using this work
  2. Update a description in the AWS Console
  3. Terraform plan shows no changes

Can you also check please?

Thanks!

@Ninir Ninir added the waiting-response Maintainers are waiting on response from community or contributor. label Sep 18, 2017
@Ninir Ninir mentioned this pull request Sep 21, 2017
Closed
@ewbankkit
Copy link
Contributor Author

OK, I'll take a look; Thanks.

@kwilczynski kwilczynski mentioned this pull request Sep 27, 2017
Closed
@antonbabenko antonbabenko mentioned this pull request Sep 29, 2017
Closed
@ewbankkit ewbankkit mentioned this pull request Sep 29, 2017
Closed
@ewbankkit ewbankkit force-pushed the security-group-rules-description branch from a5caf1c to bee5cb1 Compare October 5, 2017 23:26
@ewbankkit
Copy link
Contributor Author

Finally got round to doing this.
Fixed the case of SG rule descriptions being added outside of Terraform.
Rebased and squashed.
Acceptance tests:

make testacc TEST=./aws/ TESTARGS='-run=TestAccAWSSecurityGroupRule_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -run=TestAccAWSSecurityGroupRule_ -timeout 120m
=== RUN   TestAccAWSSecurityGroupRule_Ingress_VPC
--- PASS: TestAccAWSSecurityGroupRule_Ingress_VPC (19.96s)
=== RUN   TestAccAWSSecurityGroupRule_Ingress_Protocol
--- PASS: TestAccAWSSecurityGroupRule_Ingress_Protocol (33.54s)
=== RUN   TestAccAWSSecurityGroupRule_Ingress_Ipv6
--- PASS: TestAccAWSSecurityGroupRule_Ingress_Ipv6 (33.52s)
=== RUN   TestAccAWSSecurityGroupRule_Ingress_Classic
--- PASS: TestAccAWSSecurityGroupRule_Ingress_Classic (10.65s)
=== RUN   TestAccAWSSecurityGroupRule_MultiIngress
--- PASS: TestAccAWSSecurityGroupRule_MultiIngress (21.76s)
=== RUN   TestAccAWSSecurityGroupRule_Egress
--- PASS: TestAccAWSSecurityGroupRule_Egress (20.20s)
=== RUN   TestAccAWSSecurityGroupRule_SelfReference
--- PASS: TestAccAWSSecurityGroupRule_SelfReference (33.24s)
=== RUN   TestAccAWSSecurityGroupRule_ExpectInvalidTypeError
--- PASS: TestAccAWSSecurityGroupRule_ExpectInvalidTypeError (1.42s)
=== RUN   TestAccAWSSecurityGroupRule_ExpectInvalidCIDR
--- PASS: TestAccAWSSecurityGroupRule_ExpectInvalidCIDR (1.39s)
=== RUN   TestAccAWSSecurityGroupRule_PartialMatching_basic
--- PASS: TestAccAWSSecurityGroupRule_PartialMatching_basic (35.95s)
=== RUN   TestAccAWSSecurityGroupRule_PartialMatching_Source
--- PASS: TestAccAWSSecurityGroupRule_PartialMatching_Source (36.14s)
=== RUN   TestAccAWSSecurityGroupRule_Issue5310
--- PASS: TestAccAWSSecurityGroupRule_Issue5310 (11.48s)
=== RUN   TestAccAWSSecurityGroupRule_Race
--- PASS: TestAccAWSSecurityGroupRule_Race (281.85s)
=== RUN   TestAccAWSSecurityGroupRule_SelfSource
--- PASS: TestAccAWSSecurityGroupRule_SelfSource (33.83s)
=== RUN   TestAccAWSSecurityGroupRule_PrefixListEgress
--- PASS: TestAccAWSSecurityGroupRule_PrefixListEgress (36.15s)
=== RUN   TestAccAWSSecurityGroupRule_IngressDescription
--- PASS: TestAccAWSSecurityGroupRule_IngressDescription (19.27s)
=== RUN   TestAccAWSSecurityGroupRule_EgressDescription
--- PASS: TestAccAWSSecurityGroupRule_EgressDescription (19.72s)
=== RUN   TestAccAWSSecurityGroupRule_IngressDescription_updates
--- PASS: TestAccAWSSecurityGroupRule_IngressDescription_updates (30.60s)
=== RUN   TestAccAWSSecurityGroupRule_EgressDescription_updates
--- PASS: TestAccAWSSecurityGroupRule_EgressDescription_updates (30.90s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	711.586s

make testacc TEST=./aws/ TESTARGS='-run=TestAccAWSSecurityGroup_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -run=TestAccAWSSecurityGroup_ -timeout 120m
=== RUN   TestAccAWSSecurityGroup_importBasic
--- PASS: TestAccAWSSecurityGroup_importBasic (32.88s)
=== RUN   TestAccAWSSecurityGroup_importIpv6
--- PASS: TestAccAWSSecurityGroup_importIpv6 (32.46s)
=== RUN   TestAccAWSSecurityGroup_importSelf
--- PASS: TestAccAWSSecurityGroup_importSelf (36.77s)
=== RUN   TestAccAWSSecurityGroup_importSourceSecurityGroup
--- PASS: TestAccAWSSecurityGroup_importSourceSecurityGroup (35.46s)
=== RUN   TestAccAWSSecurityGroup_importIPRangeAndSecurityGroupWithSameRules
--- PASS: TestAccAWSSecurityGroup_importIPRangeAndSecurityGroupWithSameRules (39.86s)
=== RUN   TestAccAWSSecurityGroup_importIPRangesWithSameRules
--- PASS: TestAccAWSSecurityGroup_importIPRangesWithSameRules (36.82s)
=== RUN   TestAccAWSSecurityGroup_importPrefixList
--- PASS: TestAccAWSSecurityGroup_importPrefixList (39.54s)
=== RUN   TestAccAWSSecurityGroup_basic
--- PASS: TestAccAWSSecurityGroup_basic (31.29s)
=== RUN   TestAccAWSSecurityGroup_basicRuleDescription
--- PASS: TestAccAWSSecurityGroup_basicRuleDescription (30.88s)
=== RUN   TestAccAWSSecurityGroup_ipv6
--- PASS: TestAccAWSSecurityGroup_ipv6 (31.13s)
=== RUN   TestAccAWSSecurityGroup_tagsCreatedFirst
--- PASS: TestAccAWSSecurityGroup_tagsCreatedFirst (22.10s)
=== RUN   TestAccAWSSecurityGroup_namePrefix
--- PASS: TestAccAWSSecurityGroup_namePrefix (10.13s)
=== RUN   TestAccAWSSecurityGroup_self
--- PASS: TestAccAWSSecurityGroup_self (29.75s)
=== RUN   TestAccAWSSecurityGroup_vpc
--- PASS: TestAccAWSSecurityGroup_vpc (30.02s)
=== RUN   TestAccAWSSecurityGroup_vpcNegOneIngress
--- PASS: TestAccAWSSecurityGroup_vpcNegOneIngress (30.82s)
=== RUN   TestAccAWSSecurityGroup_vpcProtoNumIngress
--- PASS: TestAccAWSSecurityGroup_vpcProtoNumIngress (29.31s)
=== RUN   TestAccAWSSecurityGroup_MultiIngress
--- PASS: TestAccAWSSecurityGroup_MultiIngress (37.00s)
=== RUN   TestAccAWSSecurityGroup_Change
--- PASS: TestAccAWSSecurityGroup_Change (49.25s)
=== RUN   TestAccAWSSecurityGroup_ChangeRuleDescription
--- PASS: TestAccAWSSecurityGroup_ChangeRuleDescription (68.99s)
=== RUN   TestAccAWSSecurityGroup_generatedName
--- PASS: TestAccAWSSecurityGroup_generatedName (31.47s)
=== RUN   TestAccAWSSecurityGroup_DefaultEgress_VPC
--- PASS: TestAccAWSSecurityGroup_DefaultEgress_VPC (29.78s)
=== RUN   TestAccAWSSecurityGroup_DefaultEgress_Classic
--- PASS: TestAccAWSSecurityGroup_DefaultEgress_Classic (10.09s)
=== RUN   TestAccAWSSecurityGroup_drift
--- PASS: TestAccAWSSecurityGroup_drift (16.72s)
=== RUN   TestAccAWSSecurityGroup_drift_complex
--- PASS: TestAccAWSSecurityGroup_drift_complex (36.95s)
=== RUN   TestAccAWSSecurityGroup_invalidCIDRBlock
--- PASS: TestAccAWSSecurityGroup_invalidCIDRBlock (1.37s)
=== RUN   TestAccAWSSecurityGroup_tags
--- PASS: TestAccAWSSecurityGroup_tags (49.15s)
=== RUN   TestAccAWSSecurityGroup_CIDRandGroups
--- PASS: TestAccAWSSecurityGroup_CIDRandGroups (36.48s)
=== RUN   TestAccAWSSecurityGroup_ingressWithCidrAndSGs
--- PASS: TestAccAWSSecurityGroup_ingressWithCidrAndSGs (37.50s)
=== RUN   TestAccAWSSecurityGroup_ingressWithCidrAndSGs_classic
--- PASS: TestAccAWSSecurityGroup_ingressWithCidrAndSGs_classic (11.35s)
=== RUN   TestAccAWSSecurityGroup_egressWithPrefixList
--- PASS: TestAccAWSSecurityGroup_egressWithPrefixList (37.57s)
=== RUN   TestAccAWSSecurityGroup_ipv4andipv6Egress
--- PASS: TestAccAWSSecurityGroup_ipv4andipv6Egress (31.33s)
=== RUN   TestAccAWSSecurityGroup_failWithDiffMismatch
--- PASS: TestAccAWSSecurityGroup_failWithDiffMismatch (36.29s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	1020.542s

@Ninir Ninir removed the waiting-response Maintainers are waiting on response from community or contributor. label Oct 9, 2017
Ninir
Ninir approved these changes Oct 10, 2017
View reviewed changes
Copy link
Contributor

@Ninir Ninir left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Hey @ewbankkit

This looks awesome: played with it and it works like a charm!

$ make testacc TEST=./aws TESTARGS='-run=TestAccAWSSecurityGroup'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSSecurityGroup -timeout 120m
=== RUN   TestAccAWSSecurityGroup_importBasic
--- PASS: TestAccAWSSecurityGroup_importBasic (53.66s)
=== RUN   TestAccAWSSecurityGroup_importIpv6
--- PASS: TestAccAWSSecurityGroup_importIpv6 (52.86s)
=== RUN   TestAccAWSSecurityGroup_importSelf
--- PASS: TestAccAWSSecurityGroup_importSelf (59.38s)
=== RUN   TestAccAWSSecurityGroup_importSourceSecurityGroup
--- PASS: TestAccAWSSecurityGroup_importSourceSecurityGroup (57.80s)
=== RUN   TestAccAWSSecurityGroup_importIPRangeAndSecurityGroupWithSameRules
--- PASS: TestAccAWSSecurityGroup_importIPRangeAndSecurityGroupWithSameRules (64.40s)
=== RUN   TestAccAWSSecurityGroup_importIPRangesWithSameRules
--- PASS: TestAccAWSSecurityGroup_importIPRangesWithSameRules (61.06s)
=== RUN   TestAccAWSSecurityGroup_importPrefixList
--- PASS: TestAccAWSSecurityGroup_importPrefixList (64.12s)
=== RUN   TestAccAWSSecurityGroupRule_Ingress_VPC
--- PASS: TestAccAWSSecurityGroupRule_Ingress_VPC (30.46s)
=== RUN   TestAccAWSSecurityGroupRule_Ingress_Protocol
--- PASS: TestAccAWSSecurityGroupRule_Ingress_Protocol (55.20s)
=== RUN   TestAccAWSSecurityGroupRule_Ingress_Ipv6
--- PASS: TestAccAWSSecurityGroupRule_Ingress_Ipv6 (54.39s)
=== RUN   TestAccAWSSecurityGroupRule_Ingress_Classic
--- PASS: TestAccAWSSecurityGroupRule_Ingress_Classic (24.01s)
=== RUN   TestAccAWSSecurityGroupRule_MultiIngress
--- PASS: TestAccAWSSecurityGroupRule_MultiIngress (35.19s)
=== RUN   TestAccAWSSecurityGroupRule_Egress
--- PASS: TestAccAWSSecurityGroupRule_Egress (30.74s)
=== RUN   TestAccAWSSecurityGroupRule_SelfReference
--- PASS: TestAccAWSSecurityGroupRule_SelfReference (53.99s)
=== RUN   TestAccAWSSecurityGroupRule_ExpectInvalidTypeError
--- PASS: TestAccAWSSecurityGroupRule_ExpectInvalidTypeError (1.86s)
=== RUN   TestAccAWSSecurityGroupRule_ExpectInvalidCIDR
--- PASS: TestAccAWSSecurityGroupRule_ExpectInvalidCIDR (2.26s)
=== RUN   TestAccAWSSecurityGroupRule_PartialMatching_basic
--- PASS: TestAccAWSSecurityGroupRule_PartialMatching_basic (58.83s)
=== RUN   TestAccAWSSecurityGroupRule_PartialMatching_Source
--- PASS: TestAccAWSSecurityGroupRule_PartialMatching_Source (60.55s)
=== RUN   TestAccAWSSecurityGroupRule_Issue5310
--- PASS: TestAccAWSSecurityGroupRule_Issue5310 (21.69s)
=== RUN   TestAccAWSSecurityGroupRule_Race
--- PASS: TestAccAWSSecurityGroupRule_Race (442.90s)
=== RUN   TestAccAWSSecurityGroupRule_SelfSource
--- PASS: TestAccAWSSecurityGroupRule_SelfSource (53.59s)
=== RUN   TestAccAWSSecurityGroupRule_PrefixListEgress
--- PASS: TestAccAWSSecurityGroupRule_PrefixListEgress (60.93s)
=== RUN   TestAccAWSSecurityGroupRule_IngressDescription
--- PASS: TestAccAWSSecurityGroupRule_IngressDescription (30.68s)
=== RUN   TestAccAWSSecurityGroupRule_EgressDescription
--- PASS: TestAccAWSSecurityGroupRule_EgressDescription (32.01s)
=== RUN   TestAccAWSSecurityGroupRule_IngressDescription_updates
--- PASS: TestAccAWSSecurityGroupRule_IngressDescription_updates (48.02s)
=== RUN   TestAccAWSSecurityGroupRule_EgressDescription_updates
--- PASS: TestAccAWSSecurityGroupRule_EgressDescription_updates (49.06s)
=== RUN   TestAccAWSSecurityGroup_basic
--- PASS: TestAccAWSSecurityGroup_basic (50.38s)
=== RUN   TestAccAWSSecurityGroup_basicRuleDescription
--- PASS: TestAccAWSSecurityGroup_basicRuleDescription (52.84s)
=== RUN   TestAccAWSSecurityGroup_ipv6
--- PASS: TestAccAWSSecurityGroup_ipv6 (51.57s)
=== RUN   TestAccAWSSecurityGroup_tagsCreatedFirst
--- PASS: TestAccAWSSecurityGroup_tagsCreatedFirst (35.59s)
=== RUN   TestAccAWSSecurityGroup_namePrefix
--- PASS: TestAccAWSSecurityGroup_namePrefix (18.17s)
=== RUN   TestAccAWSSecurityGroup_self
--- PASS: TestAccAWSSecurityGroup_self (51.32s)
=== RUN   TestAccAWSSecurityGroup_vpc
--- PASS: TestAccAWSSecurityGroup_vpc (49.27s)
=== RUN   TestAccAWSSecurityGroup_vpcNegOneIngress
--- PASS: TestAccAWSSecurityGroup_vpcNegOneIngress (49.65s)
=== RUN   TestAccAWSSecurityGroup_vpcProtoNumIngress
--- PASS: TestAccAWSSecurityGroup_vpcProtoNumIngress (49.79s)
=== RUN   TestAccAWSSecurityGroup_MultiIngress
--- PASS: TestAccAWSSecurityGroup_MultiIngress (61.28s)
=== RUN   TestAccAWSSecurityGroup_Change
--- PASS: TestAccAWSSecurityGroup_Change (82.49s)
=== RUN   TestAccAWSSecurityGroup_ChangeRuleDescription
--- PASS: TestAccAWSSecurityGroup_ChangeRuleDescription (113.66s)
=== RUN   TestAccAWSSecurityGroup_generatedName
--- PASS: TestAccAWSSecurityGroup_generatedName (51.00s)
=== RUN   TestAccAWSSecurityGroup_DefaultEgress_VPC
--- PASS: TestAccAWSSecurityGroup_DefaultEgress_VPC (49.68s)
=== RUN   TestAccAWSSecurityGroup_DefaultEgress_Classic
--- PASS: TestAccAWSSecurityGroup_DefaultEgress_Classic (19.00s)
=== RUN   TestAccAWSSecurityGroup_drift
--- PASS: TestAccAWSSecurityGroup_drift (27.17s)
=== RUN   TestAccAWSSecurityGroup_drift_complex
--- PASS: TestAccAWSSecurityGroup_drift_complex (59.41s)
=== RUN   TestAccAWSSecurityGroup_invalidCIDRBlock
--- PASS: TestAccAWSSecurityGroup_invalidCIDRBlock (2.35s)
=== RUN   TestAccAWSSecurityGroup_tags
--- PASS: TestAccAWSSecurityGroup_tags (80.93s)
=== RUN   TestAccAWSSecurityGroup_CIDRandGroups
--- PASS: TestAccAWSSecurityGroup_CIDRandGroups (63.41s)
=== RUN   TestAccAWSSecurityGroup_ingressWithCidrAndSGs
--- PASS: TestAccAWSSecurityGroup_ingressWithCidrAndSGs (61.55s)
=== RUN   TestAccAWSSecurityGroup_ingressWithCidrAndSGs_classic
--- FAIL: TestAccAWSSecurityGroup_ingressWithCidrAndSGs_classic (20.53s)
	testing.go:434: Step 0 error: Error applying: 1 error(s) occurred:
		
		* aws_security_group.web: 1 error(s) occurred:
		
		* aws_security_group.web: Error authorizing security group ingress rules: InvalidGroupId.Malformed: Invalid id: "tf_other_acc_tests" (expecting "sg-...")
			status code: 400, request id: 158b5f55-bf04-46e8-9e60-70538ef1c644
=== RUN   TestAccAWSSecurityGroup_egressWithPrefixList
--- PASS: TestAccAWSSecurityGroup_egressWithPrefixList (62.47s)
=== RUN   TestAccAWSSecurityGroup_ipv4andipv6Egress
--- PASS: TestAccAWSSecurityGroup_ipv4andipv6Egress (51.35s)
=== RUN   TestAccAWSSecurityGroup_failWithDiffMismatch
--- PASS: TestAccAWSSecurityGroup_failWithDiffMismatch (62.95s)
FAIL
exit status 1
FAIL	github.com/terraform-providers/terraform-provider-aws/aws	2837.520s
make: *** [testacc] Error 1

(^ this is related to my personal account)

Thank you again for the work here! 👍 🎆

@Ninir Ninir merged commit b1af69b into hashicorp:master Oct 10, 2017
@ewbankkit ewbankkit deleted the security-group-rules-description branch October 11, 2017 00:40
@drmaciej
Copy link

when can we expect a release with that included?

@radeksimko
Copy link
Member

@maciejdrozdzowski This just went out in 1.1.0

@svanharmelen svanharmelen mentioned this pull request Mar 5, 2018
Closed
@shivamdixit shivamdixit mentioned this pull request Oct 26, 2018
Open
@ghost
Copy link

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

@Ninir Ninir Ninir approved these changes

Assignees
No one assigned
Labels
enhancement Requests to existing resources that expand the functionality or scope.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support Descriptions for Security Group Rules
5 participants
@ewbankkit @plukevdh @drmaciej @radeksimko @Ninir