Enable App Routing #695
Closed
Enable App Routing #695
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
meecethereese
requested review from
alyssa1303,
anson627,
sumanthreddy29 and
rafael-mendes-pereira
as code owners
There was a problem hiding this comment.
Pull Request Overview
This PR enables the use of web app routing when creating AKS clusters by adding a new optional variable in the Terraform module configuration.
- Introduces the web_app_routing variable in both azure/variables.tf and azure/aks/variables.tf.
- Adds a dynamic block in azure/aks/main.tf to apply the web_app_routing configuration to the azurerm_kubernetes_cluster resource.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| modules/terraform/azure/variables.tf | Added web_app_routing field to the aks_config_list variable. |
| modules/terraform/azure/aks/variables.tf | Added web_app_routing field to the aks_config variable object. |
| modules/terraform/azure/aks/main.tf | Added a dynamic block to conditionally configure web_app_routing. |
Comments suppressed due to low confidence (2)
modules/terraform/azure/variables.tf:188
- Consider adding a descriptive comment for the new 'web_app_routing' variable to clarify its purpose and expected format.
web_app_routing = optional(object({
modules/terraform/azure/aks/main.tf:85
- [nitpick] Consider adding an inline comment to explain the dynamic block for 'web_app_routing', which will help future maintainers understand its conditional usage.
dynamic "web_app_routing" {
| dynamic "web_app_routing" { | ||
| for_each = try(var.aks_config.web_app_routing != null ? [var.aks_config.web_app_routing] : []) | ||
| content { | ||
| dns_zone_ids = var.aks_config.web_app_routing.dns_zone_ids |
There was a problem hiding this comment.
Suggested change
| dns_zone_ids = var.aks_config.web_app_routing.dns_zone_ids | |
| dns_zone_ids = web_app_routing.value.dns_zone_ids |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Added the web_app_routing field to the terraform modules for the aks config. This will allow us to create aks clusters by declaring web_app_routing in the scenario tfvars file.