Make VPC database resources to be conditionally added only if there is a database needed in the network

rocketnova commented 2 months ago

Ticket

N/A

Changes

What was added, updated, or removed in this PR.

Set database subnets and subnet group to be conditionally added only if there is a database specified in the network

Context for reviewers

Testing instructions, background context, more in-depth details of the implementation, and anything else you'd like to call out or ask reviewers.

Right now, the first time you configure the network and run make infra-update-network NETWORK_NAME=<NETWORK_NAME>, the database subnets and subnet group are created even if there are no databases specified in the network.

This change makes creating those resources conditional based on whether or not there databases are needed.

I think it's better to make this conditional to reduce the number of unnecessary resources created. In general, unnecessary resources add additional cost and management overhead (i.e. it was confusing to me to see database subnets being created for an application that had no database).

Testing

Provide evidence that the code works as expected. Explain what was done for testing and the results of the test plan. Include screenshots, GIF demos, shell commands or output to help show the changes working as expected. ProTip: you can drag and drop or paste images into this textbox.

Testing performed by deploying this branch to my local version of https://github.com/navapbc/platform-test and then deploying necessary resources (account, network) to my AWS account. This screenshot shows:

on the top left, has_database is set to false in /infra/app/app-config/main.tf
on the bottom left, the changes in this PR
on the right, the deployed dev network which does not have any database VPC endpoints

CleanShot 2024-05-01 at 16 16 40@2x

rocketnova commented 2 months ago

left some nits. also, i didn't see any evidence of testing in the PR, which i'd ideally like to see before approving

Adding now and will re-ping you when I'm ready for re-review. Thanks!

rocketnova commented 2 months ago

Rollout note: One thing to remember to do after merging, is that on platform-test-nextjs you'll want to run terraform apply in the network layer, since the network layer doesn't get auto-applied as part of CD. I imagine apply would end up removing the subnets and subnet group.

Thank you for the tip! I would have missed that.

rocketnova commented 2 months ago

Rollout note: One thing to remember to do after merging, is that on platform-test-nextjs you'll want to run terraform apply in the network layer, since the network layer doesn't get auto-applied as part of CD. I imagine apply would end up removing the subnets and subnet group.

Thank you for the tip! I would have missed that.

Following up to say that this is complete!

navapbc / template-infra