Skip to content

Commit 23e222e

Browse files
eerhardteerhardt
committed
Respond to PR feedback
1 parent 29f911f commit 23e222e

File tree

4 files changed

+168
-2
lines changed

4 files changed

+168
-2
lines changed

‎src/Aspire.Hosting.Azure.Network/AzureSubnetResource.cs‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -134,9 +134,14 @@ internal SubnetResource ToProvisioningEntity(AzureResourceInfrastructure infra,
134134
subnet.NatGatewayId = NatGateway.Id.AsProvisioningParameter(infra);
135135
}
136136

137-
if (NetworkSecurityGroup is not null &&
138-
nsgMap.TryGetValue(NetworkSecurityGroup, out var provisioningNsg))
137+
if (NetworkSecurityGroup is not null)
139138
{
139+
if (!nsgMap.TryGetValue(NetworkSecurityGroup, out var provisioningNsg))
140+
{
141+
throw new InvalidOperationException(
142+
$"The Network Security Group '{NetworkSecurityGroup.Name}' referenced by subnet '{Name}' was not found in the parent Virtual Network's NetworkSecurityGroups collection.");
143+
}
144+
140145
// Set the NSG reference on the subnet by setting the model's Id property.
141146
// This produces the correct bicep: networkSecurityGroup: { id: nsg.id }
142147
subnet.NetworkSecurityGroup.Id = provisioningNsg.Id;

‎src/Aspire.Hosting.Azure.Network/AzureVirtualNetworkExtensions.cs‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -439,6 +439,15 @@ public static IResourceBuilder<AzureSubnetResource> WithNetworkSecurityGroup(
439439
ArgumentNullException.ThrowIfNull(builder);
440440
ArgumentNullException.ThrowIfNull(nsg);
441441

442+
if (nsg.Resource.Parent != builder.Resource.Parent)
443+
{
444+
throw new ArgumentException(
445+
$"The Network Security Group '{nsg.Resource.Name}' belongs to Virtual Network '{nsg.Resource.Parent.Name}', " +
446+
$"but the subnet '{builder.Resource.Name}' belongs to Virtual Network '{builder.Resource.Parent.Name}'. " +
447+
$"The NSG and subnet must belong to the same Virtual Network.",
448+
nameof(nsg));
449+
}
450+
442451
builder.Resource.NetworkSecurityGroup = nsg.Resource;
443452
return builder;
444453
}

‎tests/Aspire.Hosting.Azure.Tests/AzureVirtualNetworkExtensionsTests.cs‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -402,4 +402,66 @@ public void WithSecurityRule_DuplicateName_Throws()
402402

403403
Assert.Contains("allow-https", exception.Message, StringComparison.OrdinalIgnoreCase);
404404
}
405+
406+
[Fact]
407+
public async Task MultipleNSGs_WithSameRuleName_GeneratesDistinctBicepIdentifiers()
408+
{
409+
using var builder = TestDistributedApplicationBuilder.Create(DistributedApplicationOperation.Publish);
410+
411+
var vnet = builder.AddAzureVirtualNetwork("myvnet");
412+
413+
var nsg1 = vnet.AddNetworkSecurityGroup("nsg-one")
414+
.WithSecurityRule(new AzureSecurityRule
415+
{
416+
Name = "allow-https",
417+
Priority = 100,
418+
Direction = SecurityRuleDirection.Inbound,
419+
Access = SecurityRuleAccess.Allow,
420+
Protocol = SecurityRuleProtocol.Tcp,
421+
SourceAddressPrefix = "*",
422+
SourcePortRange = "*",
423+
DestinationAddressPrefix = "*",
424+
DestinationPortRange = "443"
425+
});
426+
427+
var nsg2 = vnet.AddNetworkSecurityGroup("nsg-two")
428+
.WithSecurityRule(new AzureSecurityRule
429+
{
430+
Name = "allow-https",
431+
Priority = 100,
432+
Direction = SecurityRuleDirection.Inbound,
433+
Access = SecurityRuleAccess.Allow,
434+
Protocol = SecurityRuleProtocol.Tcp,
435+
SourceAddressPrefix = "VirtualNetwork",
436+
SourcePortRange = "*",
437+
DestinationAddressPrefix = "*",
438+
DestinationPortRange = "443"
439+
});
440+
441+
vnet.AddSubnet("subnet1", "10.0.1.0/24")
442+
.WithNetworkSecurityGroup(nsg1);
443+
vnet.AddSubnet("subnet2", "10.0.2.0/24")
444+
.WithNetworkSecurityGroup(nsg2);
445+
446+
var manifest = await AzureManifestUtils.GetManifestWithBicep(vnet.Resource);
447+
448+
await Verify(manifest.BicepText, extension: "bicep");
449+
}
450+
451+
[Fact]
452+
public void WithNetworkSecurityGroup_DifferentVNet_Throws()
453+
{
454+
using var builder = TestDistributedApplicationBuilder.Create(DistributedApplicationOperation.Publish);
455+
456+
var vnet1 = builder.AddAzureVirtualNetwork("vnet1");
457+
var vnet2 = builder.AddAzureVirtualNetwork("vnet2");
458+
459+
var nsg = vnet1.AddNetworkSecurityGroup("web-nsg");
460+
var subnet = vnet2.AddSubnet("web-subnet", "10.0.1.0/24");
461+
462+
var exception = Assert.Throws<ArgumentException>(() => subnet.WithNetworkSecurityGroup(nsg));
463+
464+
Assert.Contains("vnet1", exception.Message);
465+
Assert.Contains("vnet2", exception.Message);
466+
}
405467
}

‎tests/Aspire.Hosting.Azure.Tests/Snapshots/AzureVirtualNetworkExtensionsTests.MultipleNSGs_WithSameRuleName_GeneratesDistinctBicepIdentifiers.verified.bicep‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
@description('The location for the resource(s) to be deployed.')
2+
param location string = resourceGroup().location
3+
4+
resource myvnet 'Microsoft.Network/virtualNetworks@2025-05-01' = {
5+
name: take('myvnet-${uniqueString(resourceGroup().id)}', 64)
6+
properties: {
7+
addressSpace: {
8+
addressPrefixes: [
9+
'10.0.0.0/16'
10+
]
11+
}
12+
}
13+
location: location
14+
tags: {
15+
'aspire-resource-name': 'myvnet'
16+
}
17+
}
18+
19+
resource nsg_one 'Microsoft.Network/networkSecurityGroups@2025-05-01' = {
20+
name: take('nsg_one-${uniqueString(resourceGroup().id)}', 80)
21+
location: location
22+
}
23+
24+
resource nsg_one_allow_https 'Microsoft.Network/networkSecurityGroups/securityRules@2025-05-01' = {
25+
name: 'allow-https'
26+
properties: {
27+
access: 'Allow'
28+
destinationAddressPrefix: '*'
29+
destinationPortRange: '443'
30+
direction: 'Inbound'
31+
priority: 100
32+
protocol: 'Tcp'
33+
sourceAddressPrefix: '*'
34+
sourcePortRange: '*'
35+
}
36+
parent: nsg_one
37+
}
38+
39+
resource nsg_two 'Microsoft.Network/networkSecurityGroups@2025-05-01' = {
40+
name: take('nsg_two-${uniqueString(resourceGroup().id)}', 80)
41+
location: location
42+
}
43+
44+
resource nsg_two_allow_https 'Microsoft.Network/networkSecurityGroups/securityRules@2025-05-01' = {
45+
name: 'allow-https'
46+
properties: {
47+
access: 'Allow'
48+
destinationAddressPrefix: '*'
49+
destinationPortRange: '443'
50+
direction: 'Inbound'
51+
priority: 100
52+
protocol: 'Tcp'
53+
sourceAddressPrefix: 'VirtualNetwork'
54+
sourcePortRange: '*'
55+
}
56+
parent: nsg_two
57+
}
58+
59+
resource subnet1 'Microsoft.Network/virtualNetworks/subnets@2025-05-01' = {
60+
name: 'subnet1'
61+
properties: {
62+
addressPrefix: '10.0.1.0/24'
63+
networkSecurityGroup: {
64+
id: nsg_one.id
65+
}
66+
}
67+
parent: myvnet
68+
}
69+
70+
resource subnet2 'Microsoft.Network/virtualNetworks/subnets@2025-05-01' = {
71+
name: 'subnet2'
72+
properties: {
73+
addressPrefix: '10.0.2.0/24'
74+
networkSecurityGroup: {
75+
id: nsg_two.id
76+
}
77+
}
78+
parent: myvnet
79+
dependsOn: [
80+
subnet1
81+
]
82+
}
83+
84+
output subnet1_Id string = subnet1.id
85+
86+
output subnet2_Id string = subnet2.id
87+
88+
output id string = myvnet.id
89+
90+
output name string = myvnet.name

0 commit comments

Comments
 (0)