resource/security_group: fix rule gathering with description included #4416
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
before: after: |
loivis
force-pushed
the
r-security-group-rule-description
branch
from
1a51d53
to
cbd920c
Compare
|
It really takes some time to run all tests 😓 . Some unrelated errors remain. |
loivis
changed the title
bflad
added
bug
bflad
reviewed
May 9, 2018
| if v := perm.ToPort; v != nil { | ||
| toPort = *v | ||
| } | ||
| k := fmt.Sprintf("%s-%d-%d-%s", *perm.IpProtocol, fromPort, toPort, desc) |
There was a problem hiding this comment.
This was previously: k := fmt.Sprintf("%s-%d-%d", *perm.IpProtocol, fromPort, toPort)
Doesn't the addition of -%s need to be based on desc != "" to prevent differences when upgrading?
There was a problem hiding this comment.
It looks like this map key doesn't appear to be used in the downstream code, so I think its actually okay. 👍
There was a problem hiding this comment.
yes, k is more of an intermediate variable
bflad
approved these changes
May 10, 2018
There was a problem hiding this comment.
I've run the acceptance testing a few times now with passing results. Let's get this in. I believe it will be okay for provider upgrades. Thanks so much @loivis 🚀
56 tests passed (all tests)
=== RUN TestAccAWSSecurityGroupRule_ExpectInvalidCIDR
--- PASS: TestAccAWSSecurityGroupRule_ExpectInvalidCIDR (2.66s)
=== RUN TestAccAWSSecurityGroupRule_ExpectInvalidTypeError
--- PASS: TestAccAWSSecurityGroupRule_ExpectInvalidTypeError (2.70s)
=== RUN TestAccAWSSecurityGroupRule_Issue5310
--- PASS: TestAccAWSSecurityGroupRule_Issue5310 (16.96s)
=== RUN TestAccAWSSecurityGroupRule_Ingress_Classic
--- PASS: TestAccAWSSecurityGroupRule_Ingress_Classic (17.07s)
=== RUN TestAccAWSSecurityGroupRule_Egress
--- PASS: TestAccAWSSecurityGroupRule_Egress (42.76s)
=== RUN TestAccAWSSecurityGroupRule_EgressDescription
--- PASS: TestAccAWSSecurityGroupRule_EgressDescription (49.40s)
=== RUN TestAccAWSSecurityGroupRule_IngressDescription
--- PASS: TestAccAWSSecurityGroupRule_IngressDescription (60.88s)
=== RUN TestAccAWSSecurityGroupRule_Ingress_VPC
--- PASS: TestAccAWSSecurityGroupRule_Ingress_VPC (105.04s)
=== RUN TestAccAWSSecurityGroupRule_Ingress_Ipv6
--- PASS: TestAccAWSSecurityGroupRule_Ingress_Ipv6 (108.12s)
=== RUN TestAccAWSSecurityGroupRule_EgressDescription_updates
--- PASS: TestAccAWSSecurityGroupRule_EgressDescription_updates (55.02s)
=== RUN TestAccAWSSecurityGroupRule_IngressDescription_updates
--- PASS: TestAccAWSSecurityGroupRule_IngressDescription_updates (79.24s)
=== RUN TestAccAWSSecurityGroupRule_MultiIngress
--- PASS: TestAccAWSSecurityGroupRule_MultiIngress (124.69s)
=== RUN TestAccAWSSecurityGroup_importIPRangesWithSameRules
--- PASS: TestAccAWSSecurityGroup_importIPRangesWithSameRules (147.76s)
=== RUN TestAccAWSSecurityGroup_basic
--- PASS: TestAccAWSSecurityGroup_basic (65.18s)
=== RUN TestAccAWSSecurityGroup_importIPRangeAndSecurityGroupWithSameRules
--- PASS: TestAccAWSSecurityGroup_importIPRangeAndSecurityGroupWithSameRules (194.56s)
=== RUN TestAccAWSSecurityGroup_namePrefix
--- PASS: TestAccAWSSecurityGroup_namePrefix (12.45s)
=== RUN TestAccAWSSecurityGroup_importPrefixList
--- PASS: TestAccAWSSecurityGroup_importPrefixList (275.26s)
=== RUN TestAccAWSSecurityGroup_vpc
--- PASS: TestAccAWSSecurityGroup_vpc (65.73s)
=== RUN TestAccAWSSecurityGroup_ipv6
--- PASS: TestAccAWSSecurityGroup_ipv6 (193.64s)
=== RUN TestAccAWSSecurityGroupRule_Ingress_Protocol
--- PASS: TestAccAWSSecurityGroupRule_Ingress_Protocol (347.22s)
=== RUN TestAccAWSSecurityGroupRule_PrefixListEgress
--- PASS: TestAccAWSSecurityGroupRule_PrefixListEgress (352.82s)
=== RUN TestAccAWSSecurityGroup_self
--- PASS: TestAccAWSSecurityGroup_self (163.93s)
=== RUN TestAccAWSSecurityGroup_vpcNegOneIngress
--- PASS: TestAccAWSSecurityGroup_vpcNegOneIngress (34.86s)
=== RUN TestAccAWSSecurityGroupRule_MultiDescription
--- PASS: TestAccAWSSecurityGroupRule_MultiDescription (300.43s)
=== RUN TestAccAWSSecurityGroup_MultiIngress
--- PASS: TestAccAWSSecurityGroup_MultiIngress (33.55s)
=== RUN TestAccAWSSecurityGroupRule_SelfSource
--- PASS: TestAccAWSSecurityGroupRule_SelfSource (389.76s)
=== RUN TestAccAWSSecurityGroup_tagsCreatedFirst
--- PASS: TestAccAWSSecurityGroup_tagsCreatedFirst (224.03s)
=== RUN TestAccAWSSecurityGroup_DefaultEgress_Classic
--- PASS: TestAccAWSSecurityGroup_DefaultEgress_Classic (14.58s)
=== RUN TestAccAWSSecurityGroup_invalidCIDRBlock
--- PASS: TestAccAWSSecurityGroup_invalidCIDRBlock (0.93s)
=== RUN TestAccAWSSecurityGroup_vpcProtoNumIngress
--- PASS: TestAccAWSSecurityGroup_vpcProtoNumIngress (62.97s)
=== RUN TestAccAWSSecurityGroup_drift
--- PASS: TestAccAWSSecurityGroup_drift (19.53s)
=== RUN TestAccAWSSecurityGroup_generatedName
--- PASS: TestAccAWSSecurityGroup_generatedName (57.91s)
=== RUN TestAccAWSSecurityGroup_ingressWithCidrAndSGs_classic
--- PASS: TestAccAWSSecurityGroup_ingressWithCidrAndSGs_classic (18.30s)
=== RUN TestAccAWSSecurityGroup_DefaultEgress_VPC
--- PASS: TestAccAWSSecurityGroup_DefaultEgress_VPC (131.30s)
=== RUN TestAccAWSSecurityGroupRule_PartialMatching_Source
--- PASS: TestAccAWSSecurityGroupRule_PartialMatching_Source (534.96s)
=== RUN TestAccAWSSecurityGroup_Change
--- PASS: TestAccAWSSecurityGroup_Change (203.09s)
=== RUN TestAccAWSSecurityGroup_emptyRuleDescription
--- PASS: TestAccAWSSecurityGroup_emptyRuleDescription (61.21s)
=== RUN TestAccAWSSecurityGroup_importBasic
--- PASS: TestAccAWSSecurityGroup_importBasic (581.05s)
=== RUN TestAccAWSSecurityGroup_CIDRandGroups
--- PASS: TestAccAWSSecurityGroup_CIDRandGroups (180.83s)
=== RUN TestAccAWSSecurityGroup_ipv4andipv6Egress
--- PASS: TestAccAWSSecurityGroup_ipv4andipv6Egress (50.31s)
=== RUN TestAccAWSSecurityGroup_tags
--- PASS: TestAccAWSSecurityGroup_tags (198.53s)
=== RUN TestAccAWSSecurityGroup_failWithDiffMismatch
--- PASS: TestAccAWSSecurityGroup_failWithDiffMismatch (41.48s)
=== RUN TestAccAWSSecurityGroup_ingressWithCidrAndSGs
--- PASS: TestAccAWSSecurityGroup_ingressWithCidrAndSGs (195.00s)
=== RUN TestAccAWSSecurityGroup_drift_complex
--- PASS: TestAccAWSSecurityGroup_drift_complex (216.36s)
=== RUN TestAccAWSSecurityGroupRule_SelfReference
--- PASS: TestAccAWSSecurityGroupRule_SelfReference (617.02s)
=== RUN TestAccAWSSecurityGroupRule_PartialMatching_basic
--- PASS: TestAccAWSSecurityGroupRule_PartialMatching_basic (633.79s)
=== RUN TestAccAWSSecurityGroup_importIpv6
--- PASS: TestAccAWSSecurityGroup_importIpv6 (638.11s)
=== RUN TestAccAWSSecurityGroup_ruleGathering
--- PASS: TestAccAWSSecurityGroup_ruleGathering (591.87s)
=== RUN TestAccAWSSecurityGroup_basicRuleDescription
--- PASS: TestAccAWSSecurityGroup_basicRuleDescription (576.28s)
=== RUN TestAccAWSSecurityGroup_ChangeRuleDescription
--- PASS: TestAccAWSSecurityGroup_ChangeRuleDescription (406.80s)
=== RUN TestAccAWSSecurityGroup_importSourceSecurityGroup
--- PASS: TestAccAWSSecurityGroup_importSourceSecurityGroup (788.29s)
=== RUN TestAccAWSSecurityGroup_importSelf
--- PASS: TestAccAWSSecurityGroup_importSelf (816.38s)
=== RUN TestAccAWSSecurityGroup_egressWithPrefixList
--- PASS: TestAccAWSSecurityGroup_egressWithPrefixList (418.46s)
=== RUN TestAccAWSSecurityGroup_forceRevokeRules_true
--- PASS: TestAccAWSSecurityGroup_forceRevokeRules_true (875.54s)
=== RUN TestAccAWSSecurityGroup_forceRevokeRules_false
--- PASS: TestAccAWSSecurityGroup_forceRevokeRules_false (924.98s)
=== RUN TestAccAWSSecurityGroupRule_Race
--- PASS: TestAccAWSSecurityGroupRule_Race (1052.94s)
bflad
added a commit
that referenced
this pull request
May 10, 2018
This was referenced May 10, 2018
Closed
Closed
|
This has been released in version 1.19.0 of the AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. |
|
@bflad the issue #2879 doesn't seem to be fix by this. But the resource has been deleted and the next plan is clear. |
|
@zioalex does it work if you capitalize the protocol of the rule you're trying to remove? e.g. |
|
@bflad I recreated all the resources with lower case "tcp" but it didn't help. |
|
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
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fix #2018
fix #2069
fix #2379
fix #2879
fix #3139
fix #3202
fix #3204
fix #3261
fix #3346
fix #4346
fix #4410
There have been a few issues regarding rules with the same
protocol,from_portandto_portbut differentdescription. The lastdescriptionwill eventually override previous ones and nextplanshows changes to apply.One simple example could be: