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-218-migrate-ioplollipopassertionsst new module code #1266

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

cfcuffari120400
Copy link
Collaborator

Motivation and Context

Major Changes

Dependencies

Testing

Documentation

Other Considerations

Copy link

sonarcloud bot commented Oct 29, 2024

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

environment = var.environment
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
environment = var.environment
environment = local.itn_environment

Comment on lines +290 to +291
subnet_pep_id = var.subnet_pendpoints_id
private_dns_zone_resource_group_name = var.resource_group_common
Copy link
Contributor

Choose a reason for hiding this comment

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

These values can be gotten from the common_values module

Suggested change
subnet_pep_id = var.subnet_pendpoints_id
private_dns_zone_resource_group_name = var.resource_group_common
subnet_pep_id = module.common_values.pep_subnets.weu.id
private_dns_zone_resource_group_name = module.common_values.resource_groups.weu.common

queue = true
}

action_group_id = data.azurerm_monitor_action_group.example.id
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this actually necessary? Does the existing storage account have alerts?

itn_environment = {
prefix = var.prefix
env_short = var.env_short
location = var.location_short
Copy link
Contributor

Choose a reason for hiding this comment

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

location contains the full name of the azure region. In this case, with this itn_environment local, we want to address resources created in italynorth. We can use the existing itn_location local

Suggested change
location = var.location_short
location = local.itn_location

Comment on lines +114 to +123

variable "subnet_pendpoints_id" {
type = string
description = "Id of the subnet which holds private endpoints"
}

variable "resource_group_common" {
type = string
description = "Name of the common resource group"
}
Copy link
Contributor

Choose a reason for hiding this comment

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

These variables can be removed since we are getting this information from the common_values module

Suggested change
variable "subnet_pendpoints_id" {
type = string
description = "Id of the subnet which holds private endpoints"
}
variable "resource_group_common" {
type = string
description = "Name of the common resource group"
}

Comment on lines +37 to +38
location = "westeurope"
secondary_location = "italynorth"
Copy link
Contributor

Choose a reason for hiding this comment

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

We already have a local containing the italynorth location. It's called itn_location on row 30

Suggested change
location = "westeurope"
secondary_location = "italynorth"

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