thotheod
commented
1 year ago Hi @torokzs , thanks for your feedback. I have made some changes look details below:
○ Application name misleading - Resource name suffix would be better
Agree, it is misleading. I changed to workloadName to match the ASE scenario as well
○ I feel Numeric Suffix unnecessary and also Environment can be merged
we are following this naming convention, and we use the naming module (in ASE Scenario as well), so the suffix is an optional parameter, which for Bicep is ok (is hidden somehow), but unfortunately it cannot be excluded from the ARM template, and it is surfaced as optional param. The same is for environment. I think we need not to deviate from ASE scenario and basic naming convention principles.
○ Both Region and Location is there I don't have a way to avoid that :( Region is automatically inserted (it's not in the template but plays the role of the region/location param in the az deployment sub create ... ). the param location is required in the bicep and of course ARM template. I added default value for the location, to take its value from the deployment().location, aka Region
○ If there is an existing Hub, then vnetHubAddressSpace need to be mandatory?
no it's not, I fixed param's @description. Done that also for subnetHubFirewallAddressSpace and subnetHubBastionAddressSpace
○ Let's clarify what Enable telemetry is doing - Microsoft.Resources/deployments@2021-04-01
We use that across every LZA, we actually keep track of who uses the LZA
○ No info on adminUsername and adminPassword that they needed for jumphost
fixed @Description
○ Sql Admin Password is string not a password. No validation for the complexity. After unsuccessful SQL there is no App service and no Front Door, I would change the order.
it is defined as securestring, not as simple string. Complexity is difficult (if not impossible) to check at that level, if you have any idea on how that could be done I would appreciate it. I added a link to password policy complexity in the @description (template tooltip). Ordering is difficult to change because, AFD has a dependency on Azure App Service (to set origin) and App Service has implicit dependency to SQL Server I assume because it sets connection string in keyvault, and in app service configuration as a keyvault reference.
○ For better user experience I would separate Redis, SQL, Jumphost, etc. deployment decisions
let's discuss, because I believe, for bicep grouping all these features together is more readable. The ARM template is automatically created out of the bicep deployment file, and the Portal has simple text field to present multiline json text, which I agree it doesn't look nice....
○ Multiple slots cannot be deployed when Basic SKU selected: "Cannot complete the operation because the site will exceed the number of slots allowed for the 'Basic' SKU."
good catch!, fixed that ;)
Regarding to the further steps: ○ AFD endpoint retrieval command doesn't find AFD. Do we really need AFD endpoint retrieval in this way since all the 3 parameters can be retrieved from portal more easily anyway. Do we even need the endpoint url?
you are right, I think we only need the 3-4 lines of script under the section 'Approve the App Service private endpoint connection from Front Door in the Azure Portal'
○ What about the staging slot endpoint approval? Can we automatize endpoint approval? Do we need it since both staging and prod endpoints approved already?
Currently I reckon using a web app deployment slot as origin in AFD is not supported. https://learn.microsoft.com/en-us/answers/questions/1179488/azure-frontdoor-premium-support-webapp-slots
○ Documentation mentions to connect to devops VM, but there is no devops VM and there are no details why to connect. The command doesn't work for me: The command failed with an unexpected error. Here is the traceback: cannot import name 'WinDLL' from 'ctypes' (/opt/az/lib/python3.10/ctypes/init.py)
you need to connect to the jumpbox (devops) VM if you wish to access internal/private resources, deploy code to resources etc. I have changed the documentation, I believe AAD Join is not possible for xxx@micorosft.com accounts. I assume you got those errors because you ran az network bastion rdp command from a bash shell, possibly? I think it needs to run from a windows terminal (cmd or PowerShell)
kunalbabre
Description
Pipeline references
Type of Change
Please delete options that are not relevant.
Checklist