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

Included Workload Profile, UDR and Azure Firewall into Terraform implementation #132

Merged
merged 14 commits into from
Feb 7, 2024

Conversation

HoussemDellai
Copy link
Contributor

Changes in this PR:

  • Created Azure Firewall module for SKU Basic
  • Created Firewall Policy with rules to allow egress traffic for needed resources
  • Created Route Table attached to infra subnet to route all traffic to Firewall
  • Created Workload Profile in ACA Environment
  • Changed infra subnet CIDR range to /27 (just to demo this is possible)
  • Added Diagnostic Settings for Firewall
  • Added naming convention for Firewall and Firewall Policy

Tested and working:

  • Tested the egress traffic from ACA App go through Azure Firewall public IP (curl ifconf.me)

Tested and not working:

  • Redeployed the same template multiple times, and it breaks because of dependencies between modules (noticed this same behavior with the main branch).
  • Also noticed that Terraform starts creating Diagnostic Settings (DS), then for some reason stops immediately. DS is then created but Terraform is not aware about the creation, so it is not saved into the state file. On next execution, I notice a conflict.

Copy link
Contributor

@ibersanoMS ibersanoMS left a comment

Choose a reason for hiding this comment

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

Great work here! Thanks for the contribution. Had a few comments around making sure properties are controlled by variables, workload profile is still optional and ensuring the new modules are consistent with the existing Bicep implementation. Once you've made those updates, I'll do a test deployment, verify everything looks good and approve. Thanks!

scenarios/aca-internal/terraform/outputs.tf Outdated Show resolved Hide resolved
scenarios/aca-internal/terraform/providers.tf Show resolved Hide resolved
scenarios/aca-internal/terraform/tfplan Outdated Show resolved Hide resolved
scenarios/shared/terraform/modules/networking/vnet/main.tf Outdated Show resolved Hide resolved
scenarios/aca-internal/terraform/terraform.tfvars Outdated Show resolved Hide resolved
@HoussemDellai
Copy link
Contributor Author

@ibersanoMS done :) You can re-check !

Copy link
Contributor

@ibersanoMS ibersanoMS left a comment

Choose a reason for hiding this comment

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

If you can just fix the IP, I'll approve and we can get this merged! Thanks!

scenarios/aca-internal/terraform/terraform.tfvars Outdated Show resolved Hide resolved
@HoussemDellai
Copy link
Contributor Author

@ibersanoMS done :)

Copy link
Contributor

@thotheod thotheod left a comment

Choose a reason for hiding this comment

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

Look Great! Thanks!

@thotheod thotheod merged commit 3f6f48c into Azure:main Feb 7, 2024
5 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants