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

[CES-199] - Avoid modules dependencies from existing resources #144

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions infra/modules/azure_app_service_plan_autoscaler/data.tf
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
data "azurerm_linux_web_app" "this" {
count = local.is_app_service ? 1 : 0
count = var.target_service.app_service_name != null && local.is_app_service ? 1 : 0

resource_group_name = var.resource_group_name
name = var.target_service.app_service_name
}

data "azurerm_linux_function_app" "this" {
count = local.is_function_app ? 1 : 0
count = var.target_service.function_app_name != null && local.is_function_app ? 1 : 0

resource_group_name = var.resource_group_name
name = var.target_service.function_app_name
Expand Down
10 changes: 5 additions & 5 deletions infra/modules/azure_app_service_plan_autoscaler/locals.tf
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,13 @@ locals {
is_app_service = var.target_service.app_service_name != null
is_function_app = var.target_service.function_app_name != null

base_name = local.is_app_service ? data.azurerm_linux_web_app.this[0].name : data.azurerm_linux_function_app.this[0].name
base_name = local.is_app_service ? var.target_service.app_service_name : var.target_service.function_app_name

autoscale_name = var.autoscale_name == null ? replace(replace(replace(local.base_name, "fn", "as"), "func", "as"), "app", "as") : var.autoscale_name
resource_group_name = local.is_app_service ? data.azurerm_linux_web_app.this[0].resource_group_name : data.azurerm_linux_function_app.this[0].resource_group_name
location = local.is_app_service ? data.azurerm_linux_web_app.this[0].location : data.azurerm_linux_function_app.this[0].location
app_service_id = local.is_app_service ? data.azurerm_linux_web_app.this[0].id : data.azurerm_linux_function_app.this[0].id
app_service_plan_id = local.is_app_service ? data.azurerm_linux_web_app.this[0].service_plan_id : data.azurerm_linux_function_app.this[0].service_plan_id
resource_group_name = var.resource_group_name
location = local.is_app_service ? coalesce(var.target_service.location, data.azurerm_linux_web_app.this[0].location) : coalesce(var.target_service.location, data.azurerm_linux_function_app.this[0].location)
app_service_id = local.is_app_service ? coalesce(var.target_service.app_service_id, data.azurerm_linux_web_app.this[0].id) : coalesce(var.target_service.function_app_id, data.azurerm_linux_function_app.this[0].id)
app_service_plan_id = local.is_app_service ? coalesce(var.target_service.app_service_plan_id, data.azurerm_linux_web_app.this[0].service_plan_id) : coalesce(var.target_service.app_service_plan_id, data.azurerm_linux_function_app.this[0].service_plan_id)

requests_rule_increase = {
metric_trigger = {
Expand Down
47 changes: 44 additions & 3 deletions infra/modules/azure_app_service_plan_autoscaler/variables.tf
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,54 @@ variable "autoscale_name" {

variable "target_service" {
type = object({
app_service_name = optional(string)
function_app_name = optional(string)
app_service_id = optional(string)
app_service_name = optional(string)
function_app_id = optional(string)
function_app_name = optional(string)
app_service_plan_id = optional(string)
location = optional(string)
})

description = <<EOT
Configuration for the target service (App Service or Function App) to which the autoscaler will be applied.

For existing services:
- Provide either 'app_service_name' or 'function_app_name'.
Copy link
Collaborator

Choose a reason for hiding this comment

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

What about having an app service autoscaler module AND a function autoscaler module instead? you may consider to share most of the code but this configuration.

Copy link
Collaborator

@gunzip gunzip Oct 22, 2024

Choose a reason for hiding this comment

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

another option is to something like this instead:

variable "target_service" {
  type = object({
    app_service = optional(object({
      app_service_id      = string
      app_service_name    = string
      app_service_plan_id = string
      location            = string
    }))
    function_app = optional(object({
      function_app_id   = string
      function_app_name = string
      location          = string
    }))
  })

  validation {
    condition = length([for v in [var.target_service.app_service, var.target_service.function_app] : v if v != null]) == 1
    error_message = "You must define either app_service or function_app, but not both."
  }
}

which will be a breaking change i guess


For new services being created concurrently:
- Provide 'app_service_id' or 'function_app_id' (depending on the service type).
- 'app_service_plan_id' and 'location' must also be provided.

Note: Only one service type (App Service or Function App) should be specified.
EOT

validation {
Copy link
Collaborator

Choose a reason for hiding this comment

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

see comment above to simplify this logic. I think this would be a breaking change anyway.

condition = (var.target_service.app_service_name != null) != (var.target_service.function_app_name != null)
error_message = "Only one between \"app_service_name\" and \"function_app_name\" can have a value. It is not possible to set both of them \"null\"."
error_message = "Exactly one of 'app_service_name' or 'function_app_name' must be provided for existing services."
}

validation {
condition = (var.target_service.app_service_id != null) != (var.target_service.function_app_id != null)
error_message = "Exactly one of 'app_service_id' or 'function_app_id' must be provided for new services."
}

validation {
condition = (
(var.target_service.app_service_id != null || var.target_service.function_app_id != null) &&
var.target_service.app_service_plan_id != null &&
var.target_service.location != null
) || (
var.target_service.app_service_name != null || var.target_service.function_app_name != null
)
error_message = "For new services, 'app_service_plan_id' and 'location' must be provided along with either 'app_service_id' or 'function_app_id'."
}

validation {
condition = (
(var.target_service.app_service_name != null || var.target_service.function_app_name != null) !=
(var.target_service.app_service_id != null || var.target_service.function_app_id != null)
)
error_message = "Provide either name for existing services or ID for new services, but not both."
}
}

Expand Down
2 changes: 1 addition & 1 deletion infra/modules/azure_cosmos_account/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,4 @@
"version": "0.0.1",
"private": true,
"provider": "azurerm"
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
data "azurerm_cosmosdb_account" "cosmos" {
for_each = { for account in local.accounts : "${account.resource_group_name}|${account.account_name}" => account }
for_each = { for account in local.accounts : "${account.resource_group_name}|${account.account_name}" => account if account.account_name != null }

name = each.value.account_name
resource_group_name = each.value.resource_group_name
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ locals {
for assignment in flatten([
for entry in var.cosmos : [
for collection in entry.collections : {
account_name = entry.account_name
account_name = coalesce(entry.account_name, reverse(split("/", entry.account_id))[0])
account_id = coalesce(entry.account_id, data.azurerm_cosmosdb_account.cosmos["${entry.resource_group_name}|${coalesce(entry.account_name, reverse(split("/", entry.account_id))[0])}"].id)
resource_group_name = entry.resource_group_name
role = entry.role
database = entry.database
Expand Down
4 changes: 2 additions & 2 deletions infra/modules/azure_role_assignments/modules/cosmos/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ resource "azurerm_cosmosdb_sql_role_assignment" "this" {

resource_group_name = each.value.resource_group_name
account_name = each.value.account_name
role_definition_id = "${data.azurerm_cosmosdb_account.cosmos["${each.value.resource_group_name}|${each.value.account_name}"].id}/sqlRoleDefinitions/${local.role_definition_id[lower(each.value.role)]}"
role_definition_id = "${each.value.account_id}/sqlRoleDefinitions/${local.role_definition_id[lower(each.value.role)]}"
principal_id = var.principal_id
scope = "${data.azurerm_cosmosdb_account.cosmos["${each.value.resource_group_name}|${each.value.account_name}"].id}${each.value.scope}"
scope = "${each.value.account_id}${each.value.scope}"
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ variable "principal_id" {
variable "cosmos" {
description = "A list of CosmosDB role assignments"
type = list(object({
account_name = string
account_name = optional(string)
account_id = optional(string)
Copy link
Collaborator

Choose a reason for hiding this comment

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

should be documented into var description

resource_group_name = string
role = string
database = optional(string, "*")
Expand All @@ -26,6 +27,7 @@ variable "cosmos" {
for entry in var.cosmos : [
for collection in entry.collections : {
account_name = entry.account_name
account_id = entry.account_id
resource_group_name = entry.resource_group_name
role = entry.role
database = entry.database
Expand All @@ -38,6 +40,7 @@ variable "cosmos" {
for entry in var.cosmos : [
for collection in entry.collections : {
account_name = entry.account_name
account_id = entry.account_id
resource_group_name = entry.resource_group_name
role = entry.role
database = entry.database
Expand All @@ -49,5 +52,12 @@ variable "cosmos" {
error_message = "Each assignment must be unique."
}

validation {
condition = alltrue([
for assignment in var.cosmos : (assignment.account_name != null || assignment.account_id != null)
])
error_message = "Either account_name or account_id must be populated."
}

default = []
}
Original file line number Diff line number Diff line change
@@ -1,12 +1,5 @@
data "azurerm_eventhub_namespace" "this" {
for_each = { for namespace in local.namespaces : "${namespace.resource_group_name}|${namespace.namespace_name}" => namespace }
for_each = { for namespace in local.namespaces : "${namespace.resource_group_name}|${namespace.namespace_name}" => namespace if namespace.namespace_name == null }
name = each.value.namespace_name
resource_group_name = each.value.resource_group_name
}

data "azurerm_eventhub" "this" {
for_each = { for event_hub in local.event_hubs : "${event_hub.namespace_name}|${event_hub.event_hub_name}" => event_hub if event_hub.event_hub_name != "*" }
name = each.value.event_hub_name
namespace_name = each.value.namespace_name
resource_group_name = each.value.resource_group_name
}
Original file line number Diff line number Diff line change
@@ -1,15 +1,16 @@
locals {
namespaces = distinct([for assignment in var.event_hub : { namespace_name = assignment.namespace_name, resource_group_name = assignment.resource_group_name }])
event_hubs = distinct([for assignment in local.assignments : { event_hub_name = assignment.event_hub_name, namespace_name = assignment.namespace_name, resource_group_name = assignment.resource_group_name }])

assignments = {
for assignment in flatten([
for entry in var.event_hub : [
for event_hub_name in entry.event_hub_names : {
namespace_name = entry.namespace_name
namespace_id = coalesce(entry.namespace_id, data.azurerm_eventhub_namespace.this["${each.value.resource_group_name}|${each.value.namespace_name}"].id)
resource_group_name = entry.resource_group_name
role = entry.role
event_hub_name = event_hub_name
event_hub_id = entry.namespace_id != null ? "${entry.namespace_id}/eventhubs/${event_hub_name}" : null
}
]
]) : "${assignment.namespace_name}|${assignment.event_hub_name}|${assignment.role}" => assignment
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
resource "azurerm_role_assignment" "this" {
for_each = local.assignments
role_definition_name = local.role_definition_name[lower(each.value.role)]
scope = each.value.event_hub_name == "*" ? data.azurerm_eventhub_namespace.this["${each.value.resource_group_name}|${each.value.namespace_name}"].id : data.azurerm_eventhub.this["${each.value.namespace_name}|${each.value.event_hub_name}"].id
scope = each.value.event_hub_name == "*" ? each.value.namespace_id : each.value.event_hub_id
principal_id = var.principal_id
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,47 +4,48 @@ variable "principal_id" {
}

variable "event_hub" {
description = "A list of event hub role assignments"
type = list(object({
namespace_name = string
namespace_name = optional(string)
namespace_id = optional(string)
resource_group_name = string
event_hub_names = optional(list(string), ["*"])
event_hub_names = optional(list(string))
event_hub_ids = optional(list(string))
role = string
}))

description = <<EOT
List of Event Hub configurations for role assignments.
For existing Event Hubs:
- Provide namespace_name and event_hub_names
For new Event Hubs being created concurrently:
Copy link
Collaborator

Choose a reason for hiding this comment

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

what do you mean here with "new Event Hubs being created concurrently"?

- Provide namespace_id and event_hub_ids
Each object should contain:
- namespace_name: (Optional) The name of an existing Event Hub Namespace
- namespace_id: (Optional) The full resource ID of a new Event Hub Namespace
- resource_group_name: The name of the Resource Group
- event_hub_names: (Optional) A list of existing Event Hub names within the Namespace
- event_hub_ids: (Optional) A list of full resource IDs for new Event Hubs
- role: The role to assign (must be one of "reader", "writer", or "owner")
EOT

validation {
condition = alltrue([
for assignment in var.event_hub : contains(["reader", "writer", "owner"], assignment.role)
for eh in var.event_hub : contains(["reader", "writer", "owner"], eh.role)
])
error_message = "The role must be set either to \"reader\", \"writer\" or \"owner\""
error_message = "The 'role' must be one of 'reader', 'writer', or 'owner'."
}

validation {
condition = length([
for assignment in flatten([
for entry in var.event_hub : [
for event_hub_name in entry.event_hub_names : {
namespace_name = entry.namespace_name
resource_group_name = entry.resource_group_name
role = entry.role
event_hub_name = event_hub_name
}
]
]) : assignment
]) == length(distinct([
for assignment in flatten([
for entry in var.event_hub : [
for event_hub_name in entry.event_hub_names : {
namespace_name = entry.namespace_name
resource_group_name = entry.resource_group_name
role = entry.role
event_hub_name = event_hub_name
}
]
]) : assignment
]))
error_message = "Each assignment must be unique."
condition = alltrue([
for eh in var.event_hub : (eh.namespace_name != null) != (eh.namespace_id != null)
])
error_message = "Provide either 'namespace_name' for existing namespaces or 'namespace_id' for new namespaces, but not both."
}

default = []
}
validation {
condition = alltrue([
for eh in var.event_hub : (eh.event_hub_names != null) != (eh.event_hub_ids != null)
])
error_message = "Provide either 'event_hub_names' for existing Event Hubs or 'event_hub_ids' for new Event Hubs, but not both."
}
}
9 changes: 6 additions & 3 deletions infra/modules/azure_role_assignments/variables.tf
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ variable "principal_id" {
variable "cosmos" {
description = "A list of CosmosDB role assignments"
type = list(object({
account_name = string
account_name = optional(string)
account_id = optional(string)
Copy link
Collaborator

Choose a reason for hiding this comment

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

should be documented

resource_group_name = string
role = string
database = optional(string, "*")
Expand Down Expand Up @@ -93,9 +94,11 @@ variable "storage_queue" {
variable "event_hub" {
description = "A list of event hub role assignments"
type = list(object({
namespace_name = string
resource_group_name = string
namespace_name = optional(string)
namespace_id = optional(string)
event_hub_ids = optional(list(string), ["*"])
event_hub_names = optional(list(string), ["*"])
resource_group_name = string
role = string
}))

Expand Down
Loading