Skip to content

Conversation

cfcuffari120400
Copy link
Contributor

@cfcuffari120400 cfcuffari120400 commented Nov 8, 2024

Motivation and Context

Storage accounts must be replicated to italy north in view of the infrastructure migration

Major Changes

Create replication of iopweucitizenauthst in italy north

Dependencies

Testing

Documentation

Other Considerations

@FasanoBip FasanoBip changed the title CES-465-migrate-iopweucitizenauthst new code [CES-465] Added new storage account iopweucitizenauthst for ITN migration Nov 8, 2024
Comment on lines 36 to 39

data "azurerm_resource_group" "data_rg_itn" {
name = "${local.project_itn}-data-rg-01"
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Since the data is only used to get the name of the resource group and its input is actually that name, I think we can get rid of it

Suggested change
data "azurerm_resource_group" "data_rg_itn" {
name = "${local.project_itn}-data-rg-01"
}

Comment on lines 40 to 50

data "azurerm_virtual_network" "vnet_itn" {
name = "${local.prefix}-${local.env_short}-itn-common-vnet-01"
resource_group_name = "${local.prefix}-${local.env_short}-itn-common-rg-01"
}

data "azurerm_subnet" "subnet_pep_itn" {
name = "io-p-itn-pep-snet-01 "
resource_group_name = data.azurerm_virtual_network.vnet_itn.resource_group_name
virtual_network_name = data.azurerm_virtual_network.vnet_itn.name
}
Copy link
Contributor

Choose a reason for hiding this comment

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

We can use the common_values module to get the information of the PEPs subnet.
I'd add the following code to the main.tf file:

module "common_values" {
 source = "github.com/pagopa/io-infra//src/_modules/common_values?ref=main"
}
Suggested change
data "azurerm_virtual_network" "vnet_itn" {
name = "${local.prefix}-${local.env_short}-itn-common-vnet-01"
resource_group_name = "${local.prefix}-${local.env_short}-itn-common-rg-01"
}
data "azurerm_subnet" "subnet_pep_itn" {
name = "io-p-itn-pep-snet-01 "
resource_group_name = data.azurerm_virtual_network.vnet_itn.resource_group_name
virtual_network_name = data.azurerm_virtual_network.vnet_itn.name
}

env_short = local.env_short
location = var.location
# domain = local.domain
app_name = local.app_name
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
app_name = local.app_name
app_name = var.domain

Comment on lines 46 to 47
prefix = local.prefix
env_short = local.env_short
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
prefix = local.prefix
env_short = local.env_short
prefix = var.prefix
env_short = var.env_short

Comment on lines 40 to 44
prefix = "io"
env_short = "p"
# domain = "citizenauth"
app_name = "citizenauth"
instance_number = "01"
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it is not necessary to have these locals since we have the itn_environment dictionary

Suggested change
prefix = "io"
env_short = "p"
# domain = "citizenauth"
app_name = "citizenauth"
instance_number = "01"

environment = local.itn_environment
resource_group_name = data.azurerm_resource_group.data_rg_itn.name
tier = "l"
subnet_pep_id = data.azurerm_subnet.subnet_pep_itn.id
Copy link
Contributor

Choose a reason for hiding this comment

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

After initializing the module common_values, we can get the pep snet id from it

Suggested change
subnet_pep_id = data.azurerm_subnet.subnet_pep_itn.id
subnet_pep_id = module.common_values.pep_subnets.itn.id

resource_group_name = data.azurerm_resource_group.data_rg_itn.name
tier = "l"
subnet_pep_id = data.azurerm_subnet.subnet_pep_itn.id
private_dns_zone_resource_group_name = "${local.prefix}-${local.env_short}-rg-common"
Copy link
Contributor

Choose a reason for hiding this comment

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

Since we have the common_values module, we can get the resource group name straight from it

Suggested change
private_dns_zone_resource_group_name = "${local.prefix}-${local.env_short}-rg-common"
private_dns_zone_resource_group_name = module.common_values.resource_groups.weu.common

source = "github.com/pagopa/dx//infra/modules/azure_storage_account?ref=main"

environment = local.itn_environment
resource_group_name = data.azurerm_resource_group.data_rg_itn.name
Copy link
Contributor

Choose a reason for hiding this comment

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

Since the data is only used to get the name of the resource group and its input is actually that name, I think we can get rid of it

Suggested change
resource_group_name = data.azurerm_resource_group.data_rg_itn.name
resource_group_name = "${local.project_itn}-data-rg-01"

force_public_network_access_enabled = false

tags = var.tags
}
Copy link
Contributor

Choose a reason for hiding this comment

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

As you can see here, the original storage account has a table and a queue defined via terraform.
Can you please add them for the new storage account as well?

resource "azurerm_storage_table" "profile_emails_itn" {
  name                 = "profileEmails"
  storage_account_name = module.io_citizen_auth_storage_itn.name
}

resource "azurerm_storage_queue" "profiles_to_sanitize_itn" {
  name                 = "profiles-to-sanitize"
  storage_account_name = module.io_citizen_auth_storage_itn.name
}

}


module "azure_storage_account" {
Copy link
Contributor

Choose a reason for hiding this comment

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

I would give the module a more talking name. Most importantly when other storage accounts are defined in the same terraform configuration.

Suggested change
module "azure_storage_account" {
module "io_citizen_auth_storage_itn" {

Copy link

@Krusty93 Krusty93 marked this pull request as ready for review December 10, 2024 09:03
@Krusty93 Krusty93 requested review from a team as code owners December 10, 2024 09:03
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.

4 participants