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

fix: Fixed SG with prefix lists #271

Merged
merged 3 commits into from Jan 13, 2023
Merged

fix: Fixed SG with prefix lists #271

merged 3 commits into from Jan 13, 2023

Conversation

ghost
Copy link

@ghost ghost commented Dec 5, 2022

Custom rules with prefix lists were failing in cases where no CIDR block was specified due to an edge case in the variable interpolation.

If both the rule and the general CIDR block variables are unset, the interpolation returns a list with an empty string as the only element (instead of an empty list).

main.tf has been modified to add a check for this edge case and return an empty list.

Description

The eight (8) resources in main.tf which set the cidr_blocks or ipv6_cidr_blocks argument have been modified to include a check for the edge case where both the custom rule does not include a CIDR block and the corresponding global CIDR block variable is unset (defaulting to an empty list). These resources now correctly evaluate their respective CIDR block arguments to an empty list, allowing the rules to be created with the assigned prefix list id(s).

All eight (8) changes follow the pattern below (in pseudo-code):

IF (rule cidr block is set) OR (global cidr block is set)
THEN (do previous behavior) 
ELSE (return empty list)
END

This preserves the original behavior in all cases except for the edge case, which now returns an empty list.

Motivation and Context

As of v4.16.2, the module does not allow for custom rules to be specified without a CIDR block, even if provided one or more prefix list ids. This appears to be due to a variable interpolation edge case, which is incorrectly returning a list with an empty string (i.e. [""]) instead of an empty list (i.e. []). A check has been added to override the interpolation and return an empty list for this edge case, which allows the security group and SG rule(s) to be created as expected.

Link to issue: #270

Fixes #270

Breaking Changes

All changes are backwards-compatible; I believe this can be safely released as a bug fix.

How Has This Been Tested?

This change has been tested against my code base.

  • I have updated at least one of the examples/* to demonstrate and validate my change(s)
  • I have tested and validated these changes using one or more of the provided examples/* projects
  • I have executed pre-commit run -a on my pull request

Custom rules with prefix lists were failing in cases where no CIDR block was specified due to an edge case in the variable interpolation.

If both the rule and the general CIDR block variables are unset, the interpolation returns a list with an empty string as the only element (instead of an empty list).

`main.tf` has been modified to add a check for this edge case and return an empty list.
@github-actions
Copy link

github-actions bot commented Jan 5, 2023

This PR has been automatically marked as stale because it has been open 30 days
with no activity. Remove stale label or comment or this PR will be closed in 10 days

@github-actions github-actions bot added the stale label Jan 5, 2023
@apamildner
Copy link

@antonbabenko Can we have this one merged/reviewed please?

@github-actions github-actions bot removed the stale label Jan 11, 2023
Copy link

@apamildner apamildner left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link

@loicvolle loicvolle left a comment

Choose a reason for hiding this comment

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

LGTM

@antonbabenko antonbabenko changed the title Fix for issue-270 RE: prefix lists fix: Fixed SG with prefix lists Jan 13, 2023
@antonbabenko antonbabenko merged commit fdd67cd into terraform-aws-modules:master Jan 13, 2023
antonbabenko pushed a commit that referenced this pull request Jan 13, 2023
### [4.17.1](v4.17.0...v4.17.1) (2023-01-13)

### Bug Fixes

* Fixed SG with prefix lists ([#271](#271)) ([fdd67cd](fdd67cd))
@antonbabenko
Copy link
Member

This PR is included in version 4.17.1 🎉

@github-actions
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 13, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot use custom rule maps with prefix lists
3 participants