feat: New module azure-stack-hci/logical-network module #4464
Conversation
avm-team-linter
bot
added
the
Needs: Core Team 🧞
microsoft-github-policy-service
bot
added
Needs: Triage 🔍
This reverts commit 731e6b6.
|
If there is no this line ('avm/res/azure-stack-hci/logical-network' # Failing on resource deletion when trying to delete RBAC at subscription level) is |
xhy8759
changed the title
| @@ -75,6 +75,7 @@ runs: | |||
| $exceptionModulePaths = @( | |||
| 'avm/ptn/lz/sub-vending' # Failing on resource deletion when trying to delete RBAC at subscription level (hub-spoke test) | |||
| 'avm/res/azure-stack-hci/cluster' # Failing on resource deletion when trying to delete RBAC at subscription level | |||
| 'avm/res/azure-stack-hci/logical-network' # Failing on resource deletion when trying to delete RBAC at subscription level | |||
There was a problem hiding this comment.
Hey @xhy8759,
thank you for your contribution 💪
For this change, I take it you tested it first without adding the module to this list of exeptions? As the module is deploying role assignments, I'd not be surprised, but it's also not failing for all modules 😄
| param ipConfigurationReferences array = [ | ||
| /*array of type {ID: string}*/ | ||
| ] |
There was a problem hiding this comment.
Instead of the comment, it would by highly recommnded to instead use a user-defined type like
param ipConfigurationReferences ipConfigurationReferenceType[]?
@export()
@description('The type for an IP configuration reference.')
type ipConfigurationReferenceType = {
@description('Required. The resource ID of something')
resourceId: string
}
resource logicalNetwork 'Microsoft.AzureStackHCI/logicalNetworks@2024-05-01-preview' = {
(...)
ipConfigurationReferences: map((ipConfigurationReferences ?? []), (ipConfigurationReference) => { id: ipConfigurationReference.resourceId })
(...)
}| 'Microsoft.Authorization/roleDefinitions', | ||
| '18d7d88d-d35e-4fb5-a5c3-7773c20a72d9' | ||
| ) | ||
| 'Role Based Access Control Administrator (Preview)': subscriptionResourceId( |
There was a problem hiding this comment.
| 'Role Based Access Control Administrator (Preview)': subscriptionResourceId( | |
| 'Role Based Access Control Administrator': subscriptionResourceId( |
| location: enforcedLocation | ||
| } | ||
|
|
||
| module nestedDependencies '../../../../cluster/tests/e2e/defaults/dependencies.bicep' = { |
There was a problem hiding this comment.
hey @eriqua,
should this be allowed? It's referencing the test setup of the cluster module (also part of azure-stack-hci). I guess duplicating the entire code base is also not ideal. So maybe the scripts should be moved to the shared test assets?
| @description('Required. The service principal ID of the service principal used for the Azure Stack HCI Resource Bridge deployment.') | ||
| @secure() | ||
| #disable-next-line secure-parameter-default | ||
| param arbDeploymentSPObjectId string = '' |
There was a problem hiding this comment.
Hey @xhy8759, any additional inputs to take not of, that were not already part of the hci-cluster module?
| scope: resourceGroup | ||
| params: { | ||
| name: '${namePrefix}${serviceShort}logicalnetwork' | ||
| location: enforcedLocation |
There was a problem hiding this comment.
| location: enforcedLocation |
| customLocationId: customLocation.id | ||
| vmSwitchName: 'ConvergedSwitch(management)' | ||
| ipAllocationMethod: 'Static' | ||
| addressPrefix: '172.20.0.1/24' | ||
| startingAddress: '172.20.0.171' | ||
| endingAddress: '172.20.0.190' | ||
| defaultGateway: '172.20.0.1' | ||
| dnsServers: ['172.20.0.1'] | ||
| routeName: 'default' | ||
| vlanId: null |
There was a problem hiding this comment.
Please note that this test must provide only the required parameters, but contains a lot of optional ones right now. Please remove those so that we can validate the defaults work. Hence the name 😉
| name: '${namePrefix}${serviceShort}logicalnetwork' | ||
| location: enforcedLocation | ||
| customLocationId: customLocation.id | ||
| vmSwitchName: 'ConvergedSwitch(management)' | ||
| ipAllocationMethod: 'Static' | ||
| addressPrefix: '172.20.0.1/24' | ||
| startingAddress: '172.20.0.171' | ||
| endingAddress: '172.20.0.190' | ||
| defaultGateway: '172.20.0.1' | ||
| dnsServers: ['172.20.0.1'] | ||
| routeName: 'default' | ||
| vlanId: null |
There was a problem hiding this comment.
Similar comment to the one in the defaults test. Are all these parameters needed to show how to deploy the module in a WAF-aligned way?
I think what you want is a 3rd test cases that sets these values (usually something we refer to as the max test)
There was a problem hiding this comment.
Note, that max test should then also contain test for e.g. the role assignments (which is something you can copy from other modules in this repository)
Description
Pipeline Reference
Type of Change
version.json:version.json.version.json.Checklist
Set-AVMModulelocally to generate the supporting module files.