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

Create public subnets only when public_subnets_enabled is true #163

Conversation

triThirty
Copy link
Contributor

what

To check if create public subnet when set public_subnets_enabled false

why

Currently, when set set public_subnets_enabled false, module still creates public subnet, according to the logic of creating private subnet, there should be a check if public_subnets_enabled is false, not creating public subnet.

references

Slack thread: https://sweetops.slack.com/archives/CCT1E7JJY/p1652862041154429

@triThirty triThirty requested review from a team as code owners May 19, 2022 02:58
@nitrocode nitrocode added the patch A minor, backward compatible change label May 19, 2022
@@ -12,7 +12,7 @@ module "public_label" {
}

resource "aws_subnet" "public" {
count = local.subnet_az_count
count = local.public_enabled ? local.subnet_az_count : 0
Copy link
Member

Choose a reason for hiding this comment

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

Is private also impacted by this ?

cc: @Nuru

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here is the private, I think you mean private subnet creation? Private is controled by private_enabled.

@nitrocode nitrocode requested review from Nuru and a team May 19, 2022 03:01
@nitrocode
Copy link
Member

/test all

@nitrocode nitrocode changed the title to check if create public subnet when set public_subnets_enabled false Create public subnets only when public_subnets_enabled is true May 19, 2022
@nitrocode
Copy link
Member

Ah it looks like a PR was already created for this. Thank you for the contribution anyway @triThirty .

#162

@nitrocode nitrocode closed this May 19, 2022
@nitrocode nitrocode reopened this May 19, 2022
@nitrocode
Copy link
Member

Actually, on second thought, I don't see the same fix applied in the other PR.

@nitrocode nitrocode added no-release Do not create a new release (wait for additional code changes) and removed patch A minor, backward compatible change labels May 19, 2022
@nitrocode nitrocode merged commit dd95ccd into cloudposse:master May 19, 2022
@nitrocode
Copy link
Member

I changed to no-release since #162 is most likely going to be merged soon enough. This way we can combine the two PRs in a single release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-release Do not create a new release (wait for additional code changes)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants