Skip to content

Cpapke/update azure poc#471

Merged
jsonar-cpapke merged 4 commits into
devfrom
cpapke/update-azure-poc
Jun 23, 2025
Merged

Cpapke/update azure poc#471
jsonar-cpapke merged 4 commits into
devfrom
cpapke/update-azure-poc

Conversation

@jsonar-cpapke

Copy link
Copy Markdown
Collaborator

No description provided.

dra_admin_subnet_id = coalesce(try(var.subnet_ids.dra_admin_subnet_id, null), var.subnet_id, module.network[0].vnet_subnets[0])
dra_analytics_subnet_id = coalesce(try(var.subnet_ids.dra_analytics_subnet_id, null), var.subnet_id, module.network[0].vnet_subnets[1])

subnet_prefixes = cidrsubnets(var.vnet_ip_range, 8, 8)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I understand that this generates two subnet ranges: "10.0.0.0/24" and "10.0.1.0/24". This is different from the AWS deployment. Here, the first one is public and the second is private, see comment at the bottom of the networking.tf file. The deployment we want - VMs with a public interface should be in the public subnet, all the rest in a private subnet. Meaning, Hub main, Hub DR, MX, DRA Admin and DBs - in the public subnet. Agentless GWs main, Agentless GWs DR, Agent GW, DRA Analytics - in the private subnet.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

What I did in this PR keeps everything the same if the subnet_ids aren't passed in. If you want to change how the network module works, that shouldn't be done in this PR, as these are meant to get our tests working, and don't change the default case.

create_network = var.subnet_ids == null && var.subnet_id == null

hub_subnet_id = coalesce(try(var.subnet_ids.hub_subnet_id, null), var.subnet_id, module.network[0].vnet_subnets[0])
hub_dr_subnet_id = coalesce(try(var.subnet_ids.hub_dr_subnet_id, null), var.subnet_id, module.network[0].vnet_subnets[1])

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

vnet_subnets[0]
See comment on subnet_prefixes.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This is what it was already, if you are looking for changes to the existing behaviour, that should be done separately.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

If I understood correctly, there is no existing behavior, the commented out bulk of local variables is a copy & paste from the AWS example, which is incorrect in the Azure case due to different private and public subnet modeling

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I don't understand what you mean. Previously, the hub_dr module used module.network[0].vnet_subnets[1], and now it uses hub_dr_subnet_id which has the same default value. Therefore the existing behaviour hasn't changed.

hub_subnet_id = coalesce(try(var.subnet_ids.hub_subnet_id, null), var.subnet_id, module.network[0].vnet_subnets[0])
hub_dr_subnet_id = coalesce(try(var.subnet_ids.hub_dr_subnet_id, null), var.subnet_id, module.network[0].vnet_subnets[1])

agentless_gw_subnet_id = coalesce(try(var.subnet_ids.agentless_gw_subnet_id, null), var.subnet_id, module.network[0].vnet_subnets[0])

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

vnet_subnets[1]
See comment on subnet_prefixes.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Same as above

agentless_gw_subnet_id = coalesce(try(var.subnet_ids.agentless_gw_subnet_id, null), var.subnet_id, module.network[0].vnet_subnets[0])
agentless_gw_dr_subnet_id = coalesce(try(var.subnet_ids.agentless_gw_dr_subnet_id, null), var.subnet_id, module.network[0].vnet_subnets[1])

db_subnet_ids = coalescelist(try(var.subnet_ids.db_subnet_ids, []), compact([var.subnet_id]), module.network[0].vnet_subnets)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Only vnet_subnets[1]
See comment on subnet_prefixes.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Same as above

db_subnet_ids = coalescelist(try(var.subnet_ids.db_subnet_ids, []), compact([var.subnet_id]), module.network[0].vnet_subnets)

mx_subnet_id = coalesce(try(var.subnet_ids.mx_subnet_id, null), var.subnet_id, module.network[0].vnet_subnets[0])
agent_gw_subnet_id = coalesce(try(var.subnet_ids.agent_gw_subnet_id, null), var.subnet_id, module.network[0].vnet_subnets[0])

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

vnet_subnets[1]
See comment on subnet_prefixes.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Same as above

# }

locals {
create_network = var.subnet_ids == null && var.subnet_id == null

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I like this solution of adding subnet_id variable, the subnet_ids is cumbersome when you only want to provide one subnet. We should port it to the AWS dsf_deployment example sometime.

Comment thread examples/azure/poc/dsf_deployment/variables.tf
Comment thread examples/azure/poc/dsf_deployment/variables.tf Outdated

@lindanasredin lindanasredin left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Talked to Chris and agreed that this is a separate change which is not high in priority at the moment.

@jsonar-cpapke jsonar-cpapke merged commit 66f6a9c into dev Jun 23, 2025
1 check passed
@jsonar-cpapke jsonar-cpapke deleted the cpapke/update-azure-poc branch June 23, 2025 17:29
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