resource/aws_security_group_rule: Validate conflicting arguments are not simultaneously specified

[mschuchard]

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" or other comments that do not add relevant new information or questions, they generate extra noise for pull request followers and do not help prioritize the request

Closes #15606

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestAccXXX'

...

[github-actions]

Thank you for your contribution! 🚀

Please note that the CHANGELOG.md file contents are handled by the maintainers during merge. This is to prevent pull request merge conflicts, especially for contributions which may not be merged immediately. Please see the Contributing Guide for additional pull request review items.

Remove any changes to the CHANGELOG.md file and commit them in this pull request to prevent delays with reviewing and potentially merging this pull request.

[mschuchard]

Removed CHANGELOG modifications. Created one because of item 2 in Pull Request Lifecycle in the contribution guidelines. Should that entry be removed?

[github-actions]

Welcome @mschuchard 👋

It looks like this is your first Pull Request submission to the Terraform AWS Provider! If you haven’t already done so please make sure you have checked out our CONTRIBUTING guide and FAQ to make sure your contribution is adhering to best practice and has all the necessary elements in place for a successful approval.

Also take a look at our FAQ which details how we prioritize Pull Requests for inclusion.

Thanks again, and welcome to the community! 😃

[ghost]

This has been released in version 3.38.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!

[onematchfox]

Hi @mschuchard and @gdavison,

Just a heads up at this point - this change breaks the https://github.com/terraform-aws-modules/terraform-aws-security-group module. Caught me completely off guard 😞 - there's also nothing in the release notes for v3.38.0 about this either?

Error being thrown by the module is:

line 145, in resource "aws_security_group_rule" "computed_ingress_with_source_security_group_id": 
 145:   ipv6_cidr_blocks         = var.ingress_ipv6_cidr_blocks 
"ipv6_cidr_blocks": conflicts with source_security_group_idp

For now, I'm just going to restrict my provider version to < 3.38.0 so I can go back to my weekend. Will try log something more detailed on Monday if someone else doesn't beat me to it.

[mschuchard]

@onematchfox The module maintainers would need to update/fix the module to not attempt simultaneously specifying those arguments since they are not allowed by the AWS API. The PR was an improvement/bug fix to invalidate that usage before the API throws an error during create operations during a apply. You can check the linked issue above for more.

[onematchfox]

@onematchfox The module maintainers would need to update/fix the module to not attempt simultaneously specifying those arguments since they are not allowed by the AWS API. The PR was an improvement/bug fix to invalidate that usage before the API throws an error during create operations during a apply. You can check the linked issue above for more.

Fair enough. Except that I could argue that this change is a Breaking Change. Resources that previously planned and applied successfully no longer plan.

I haven't dug too deep but my suspicion is that this is a result of how ConflictsWith works in comparison to the apply of the resource itself. I think the module uses blank lists ([]) as defaults which either weren't actually applied when the resource was applied (or were just ignored by the AWS API). Whereas it would seem like ConflictsWith views the blank list as a value and thus fails validation. Anyway, hopefully it is as simple as adjusting the defaults used on the module.

[mschuchard]

For Terraform versions >= 0.12, the null type is used in situations like this for optional arguments. Unless they are still supporting < 0.12, that sounds like the fix here based on your description.

[github-actions]

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.