Clean up (#25)

simonkurtz-MSFT · web-flow · commit 8eac79bb7be1 · 2025-06-02T22:32:20.000-04:00
diff --git a/.gitignore b/.gitignore
@@ -27,4 +27,5 @@ labs-in-progress/
 .coverage
 tests/python/htmlcov/
 
-shared/bicep/modules/**/*.json
+shared/bicep/modules/**/*.json
+main.json
diff --git a/infrastructure/afd-apim/create.ipynb b/infrastructure/afd-apim/create.ipynb
@@ -57,9 +57,9 @@
     "if use_ACA:\n",
     "    utils.print_info('ACA APIs will be created.')\n",
     "\n",
-    "    aca_backend_1_policy_xml    = utils.read_policy_xml(ACA_BACKEND_1_XML_POLICY_PATH)\n",
-    "    aca_backend_2_policy_xml    = utils.read_policy_xml(ACA_BACKEND_2_XML_POLICY_PATH)\n",
-    "    aca_backend_pool_policy_xml = utils.read_policy_xml(ACA_BACKEND_POOL_XML_POLICY_PATH)\n",
+    "    aca_backend_1_policy_xml    = utils.read_policy_xml(BACKEND_XML_POLICY_PATH).format(backend_id = 'aca-backend-1')\n",
+    "    aca_backend_2_policy_xml    = utils.read_policy_xml(BACKEND_XML_POLICY_PATH).format(backend_id = 'aca-backend-2')\n",
+    "    aca_backend_pool_policy_xml = utils.read_policy_xml(BACKEND_XML_POLICY_PATH).format(backend_id = 'aca-backend-pool')\n",
     "\n",
     "    # Hello World (ACA Backend 1)\n",
     "    api_hwaca_1_get = GET_APIOperation('This is a GET for Hello World on ACA Backend 1')\n",
diff --git a/infrastructure/afd-apim/main.bicep b/infrastructure/afd-apim/main.bicep
@@ -226,7 +226,7 @@ module backendPoolModule '../../shared/bicep/modules/apim/v1/backend-pool.bicep'
 
 // 8. APIM APIs
 module apisModule '../../shared/bicep/modules/apim/v1/api.bicep' = [for api in apis: if(length(apis) > 0) {
-  name: '${api.name}-${resourceSuffix}'
+  name: 'api-${api.name}'
   params: {
     apimName: apimName
     appInsightsInstrumentationKey: appInsightsInstrumentationKey
diff --git a/infrastructure/apim-aca/create.ipynb b/infrastructure/apim-aca/create.ipynb
@@ -46,9 +46,9 @@
     "\n",
     "# Policies\n",
     "hello_world_policy_xml      = utils.read_policy_xml(HELLO_WORLD_XML_POLICY_PATH)\n",
-    "aca_backend_1_policy_xml    = utils.read_policy_xml(ACA_BACKEND_1_XML_POLICY_PATH)\n",
-    "aca_backend_2_policy_xml    = utils.read_policy_xml(ACA_BACKEND_2_XML_POLICY_PATH)\n",
-    "aca_backend_pool_policy_xml = utils.read_policy_xml(ACA_BACKEND_POOL_XML_POLICY_PATH)\n",
+    "aca_backend_1_policy_xml    = utils.read_policy_xml(BACKEND_XML_POLICY_PATH).format(backend_id = 'aca-backend-1')\n",
+    "aca_backend_2_policy_xml    = utils.read_policy_xml(BACKEND_XML_POLICY_PATH).format(backend_id = 'aca-backend-2')\n",
+    "aca_backend_pool_policy_xml = utils.read_policy_xml(BACKEND_XML_POLICY_PATH).format(backend_id = 'aca-backend-pool')\n",
     "\n",
     "# Hello World (Root)\n",
     "api_hwroot_get = GET_APIOperation('This is a GET for Hello World in the root', hello_world_policy_xml)\n",
diff --git a/infrastructure/apim-aca/main.bicep b/infrastructure/apim-aca/main.bicep
@@ -134,7 +134,7 @@ module backendPoolModule '../../shared/bicep/modules/apim/v1/backend-pool.bicep'
 // 7. APIM APIs
 module apisModule '../../shared/bicep/modules/apim/v1/api.bicep' = [
   for api in apis: if (length(apis) > 0) {
-    name: '${api.name}-${resourceSuffix}'
+    name: 'api-${api.name}'
     params: {
       apimName: apimName
       appInsightsInstrumentationKey: appInsightsInstrumentationKey
diff --git a/infrastructure/simple-apim/main.bicep b/infrastructure/simple-apim/main.bicep
@@ -49,7 +49,7 @@ module apimModule '../../shared/bicep/modules/apim/v1/apim.bicep' = {
 
 // 4. APIM APIs
 module apisModule '../../shared/bicep/modules/apim/v1/api.bicep' = [for api in apis: if(length(apis) > 0) {
-  name: '${api.name}-${resourceSuffix}'
+  name: 'api-${api.name}'
   params: {
     apimName: apimName
     appInsightsInstrumentationKey: appInsightsInstrumentationKey
diff --git a/samples/authX-pro/create.ipynb b/samples/authX-pro/create.ipynb
@@ -126,8 +126,9 @@
     "    hr_member_role_id = '{{HRMemberRoleId}}'\n",
     ")\n",
     "\n",
+    "hr_product_name = 'hr'\n",
     "products: List[Product] = [\n",
-    "    Product('hr', 'Human Resources', 'Product for Human Resources APIs providing access to employee data, organizational structure, benefits information, and HR management services. Includes JWT-based authentication for HR members.', 'published', False, False, hr_product_xml)\n",
+    "    Product(hr_product_name, 'Human Resources', 'Product for Human Resources APIs providing access to employee data, organizational structure, benefits information, and HR management services. Includes JWT-based authentication for HR members.', 'published', False, False, hr_product_xml)\n",
     "]\n",
     "\n",
     "# 6) Define the APIs and their operations and policies\n",
@@ -136,13 +137,13 @@
     "hremployees_api_path = f'/{api_prefix}employees'\n",
     "hremployees_get = GET_APIOperation('Gets the employees', utils.read_policy_xml('./hr_get.xml'))\n",
     "hremployees_post = POST_APIOperation('Creates a new employee', utils.read_policy_xml('./hr_post.xml'))\n",
-    "hremployees = API(f'{api_prefix}Employees', 'Employees Pro', hremployees_api_path, 'This is a Human Resources API for employee information', utils.read_policy_xml(DEFAULT_XML_POLICY_PATH), operations = [hremployees_get, hremployees_post], tags = tags, productNames = ['hr'])\n",
+    "hremployees = API(f'{api_prefix}Employees', 'Employees Pro', hremployees_api_path, 'This is a Human Resources API for employee information', utils.read_policy_xml(DEFAULT_XML_POLICY_PATH), operations = [hremployees_get, hremployees_post], tags = tags, productNames = [hr_product_name])\n",
     "\n",
     "# Benefits (HR)\n",
     "hrbenefits_api_path = f'/{api_prefix}benefits'\n",
     "hrbenefits_get = GET_APIOperation('Gets employee benefits', utils.read_policy_xml('./hr_get.xml'))\n",
     "hrbenefits_post = POST_APIOperation('Creates employee benefits', utils.read_policy_xml('./hr_post.xml'))\n",
-    "hrbenefits = API(f'{api_prefix}Benefits', 'Benefits Pro', hrbenefits_api_path, 'This is a Human Resources API for employee benefits', utils.read_policy_xml(DEFAULT_XML_POLICY_PATH), operations = [hrbenefits_get, hrbenefits_post], tags = tags, productNames = ['hr'])\n",
+    "hrbenefits = API(f'{api_prefix}Benefits', 'Benefits Pro', hrbenefits_api_path, 'This is a Human Resources API for employee benefits', utils.read_policy_xml(DEFAULT_XML_POLICY_PATH), operations = [hrbenefits_get, hrbenefits_post], tags = tags, productNames = [hr_product_name])\n",
     "\n",
     "# APIs Array\n",
     "apis: List[API] = [hremployees, hrbenefits]\n",
@@ -171,7 +172,8 @@
     "bicep_parameters = {\n",
     "    'apis': {'value': [api.to_dict() for api in apis]},\n",
     "    'namedValues': {'value': [nv.to_dict() for nv in nvs]},\n",
-    "    'policyFragments': {'value': [pf.to_dict() for pf in pfs]}\n",
+    "    'policyFragments': {'value': [pf.to_dict() for pf in pfs]},\n",
+    "    'products': {'value': [product.to_dict() for product in products]}\n",
     "}\n",
     "\n",
     "# 2) Infrastructure must be in place before samples can be layered on top\n",
diff --git a/samples/authX-pro/main.bicep b/samples/authX-pro/main.bicep
@@ -78,7 +78,8 @@ module productHr '../../shared/bicep/modules/apim/v1/product.bicep' = [for produ
   ]
 }]
 
-// APIM APIs
+// APIM APIs (deployed after products are ready to avoid race conditions)
+@batchSize(1)  // Deploy APIs sequentially to avoid race conditions
 module apisModule '../../shared/bicep/modules/apim/v1/api.bicep' = [for api in apis: {
   name: 'api-${api.name}'
   params:{
@@ -90,7 +91,8 @@ module apisModule '../../shared/bicep/modules/apim/v1/api.bicep' = [for api in a
   }
   dependsOn: [
     namedValue              // ensure all named values are created before APIs
-    productHr               // ensure products are created before APIs that reference them
+    policyFragment          // ensure all policy fragments are created before APIs
+    productHr               // ensure all products are fully created before APIs
   ]
 }]
 
diff --git a/samples/authX/hr_product.xml b/samples/authX/hr_product.xml
diff --git a/samples/authX/main.bicep b/samples/authX/main.bicep
@@ -34,7 +34,7 @@ resource apimService 'Microsoft.ApiManagement/service@2024-06-01-preview' existi
 
 // APIM Named Values
 module namedValue '../../shared/bicep/modules/apim/v1/named-value.bicep' = [for nv in namedValues: if (!empty(namedValues)) {
-  name: nv.name
+  name: 'nv-${nv.name}'
   params: {
     apimName: apimName
     namedValueName: nv.name
@@ -45,7 +45,7 @@ module namedValue '../../shared/bicep/modules/apim/v1/named-value.bicep' = [for
 
 // APIM APIs
 module apisModule '../../shared/bicep/modules/apim/v1/api.bicep' = [for api in apis: if(!empty(apis)) {
-  name: '${api.name}-${resourceSuffix}'
+  name: 'api-${api.name}'
   params: {
     apimName: apimName
     appInsightsInstrumentationKey: appInsightsInstrumentationKey
diff --git a/samples/general/create.ipynb b/samples/general/create.ipynb
@@ -46,7 +46,7 @@
     "# 1) User-defined parameters (change these as needed)\n",
     "rg_location = 'eastus2'\n",
     "index       = 1\n",
-    "deployment  = INFRASTRUCTURE.AFD_APIM_PE\n",
+    "deployment  = INFRASTRUCTURE.SIMPLE_APIM\n",
     "tags        = ['general']\n",
     "\n",
     "# 2) Service-defined parameters (please do not change these)\n",
diff --git a/samples/general/main.bicep b/samples/general/main.bicep
@@ -32,7 +32,7 @@ resource apimService 'Microsoft.ApiManagement/service@2024-06-01-preview' existi
 
 // APIM APIs
 module apisModule '../../shared/bicep/modules/apim/v1/api.bicep' = [for api in apis: if(!empty(apis)) {
-  name: '${api.name}-${resourceSuffix}'
+  name: 'api-${api.name}'
   params: {
     apimName: apimName
     appInsightsInstrumentationKey: appInsightsInstrumentationKey
diff --git a/samples/load-balancing/main.bicep b/samples/load-balancing/main.bicep
@@ -197,7 +197,7 @@ module backendPoolModule4 '../../shared/bicep/modules/apim/v1/backend-pool.bicep
 // APIM APIs
 module apisModule '../../shared/bicep/modules/apim/v1/api.bicep' = [
   for api in apis: if (!empty(apis)) {
-    name: '${api.name}-${resourceSuffix}'
+    name: 'api-${api.name}'
     params: {
       apimName: apimName
       appInsightsInstrumentationKey: appInsightsInstrumentationKey
diff --git a/shared/apim-policies/aca-backend-2.xml b/shared/apim-policies/aca-backend-2.xml
diff --git a/shared/apim-policies/aca-backend-pool.xml b/shared/apim-policies/aca-backend-pool.xml
diff --git a/shared/apim-policies/backend.xml b/shared/apim-policies/backend.xml
@@ -4,7 +4,7 @@
 <policies>
     <inbound>
         <base />
-        <set-backend-service backend-id="aca-backend-1" />
+        <set-backend-service backend-id="{backend_id}" />
     </inbound>
     <backend>
         <base />
diff --git a/shared/apim-policies/fragments/pf-authz-match-all.xml b/shared/apim-policies/fragments/pf-authz-match-all.xml
@@ -0,0 +1,26 @@
+<!--
+    - Expected context variables:
+      - "jwt": The parsed JWT token which should already be set by the pf-authx fragment.
+      - "authz_roles": A csv of required role GUIDs to check against. All roles must be present.
+      
+    - This fragment blocks access (returns 403) when not ALL required roles are present. If all roles are present, processing continues normally.
+-->
+<fragment>
+    <choose>
+        <!-- Check if ALL required role GUIDs are present in the JWT -->
+        <when condition="@{
+            var jwt = (Jwt)context.Variables["jwt"];
+            var requiredRoleGuids = context.Variables.GetValueOrDefault<string>("authz_roles", "").ToString().Split(',');
+            var jwtRoles = jwt.Claims.GetValueOrDefault("roles", new string[0]);
+            
+            // Check if ALL required role GUIDs are present in the JWT
+            return !requiredRoleGuids.All(requiredRole => jwtRoles.Contains(requiredRole.Trim()));
+        }">
+            <return-response>
+                <set-status code="403" reason="Forbidden" />
+                <set-body>Access denied - not all required roles found</set-body>
+            </return-response>
+        </when>
+        <!-- If all roles are present, continue processing (no action needed) -->
+    </choose>
+</fragment>
diff --git a/shared/apim-policies/fragments/pf-authz-match-any.xml b/shared/apim-policies/fragments/pf-authz-match-any.xml
@@ -3,8 +3,7 @@
       - "jwt": The parsed JWT token which should already be set by the pf-authx fragment.
       - "authz_roles": A csv of allowed role GUIDs to check against. Any match will grant access.
       
-    - This fragment only blocks access (returns 403) when no roles match. If roles match, processing
-continues normally.
+    - This fragment only blocks access (returns 403) when no roles match. If roles match, processing continues normally.
 -->
 <fragment>
     <choose>
diff --git a/shared/bicep/modules/apim/v1/api.bicep b/shared/bicep/modules/apim/v1/api.bicep
@@ -149,11 +149,24 @@ resource apiDiagnostics 'Microsoft.ApiManagement/service/apis/diagnostics@2024-0
     }  }
 }
 
+// Product associations are handled directly in this module with proper dependency management // to prevent race conditions while keeping the architecture simple
+// Reference existing products for association (with explicit dependency timing)
+resource apimProducts 'Microsoft.ApiManagement/service/products@2024-06-01-preview' existing = [for productName in productNames: {
+  name: productName
+  parent: apimService
+}]
+
+// Create product-API associations with proper dependency management
 // https://learn.microsoft.com/azure/templates/microsoft.apimanagement/service/products/apis
-resource apiProductAssociation 'Microsoft.ApiManagement/service/products/apis@2024-05-01' = [for productName in productNames: {
-  name: '${apimName}/${productName}/${api.name}'
+resource apiProductAssociation 'Microsoft.ApiManagement/service/products/apis@2024-06-01-preview' = [for (productName, index) in productNames: {
+  name: apimApi.name
+  parent: apimProducts[index]
   dependsOn: [
-    apimApi
+    apimProducts[index]        // Ensure the specific product exists and is ready
+    apiPolicy                  // Ensure API policy is applied if present
+    apiOperation               // Ensure all operations are created
+    apiOperationPolicy         // Ensure all operation policies are applied
+    apiDiagnostics             // Ensure diagnostics are configured if present
   ]
 }]
 
@@ -174,10 +187,10 @@ output apiDisplayName string = apimApi.properties.displayName
 @description('The path of the created API.')
 output apiPath string = apimApi.properties.path
 
-// @description('Array of product names this API is associated with.')
-// output associatedProducts string[] = productNames
+@description('Array of product names this API is associated with.')
+output associatedProducts array = productNames
 
-// @description('Number of products this API is associated with.')
-// output productAssociationCount int = length(productNames)
+@description('Number of products this API is associated with.')
+output productAssociationCount int = length(productNames)
 
 //output subscriptionPrimaryKey string = apimSubscription.listSecrets().primaryKey
diff --git a/shared/python/apimtypes.py b/shared/python/apimtypes.py
@@ -16,9 +16,7 @@
 DEFAULT_XML_POLICY_PATH             = f'{SHARED_XML_POLICY_BASE_PATH}/default.xml'
 HELLO_WORLD_XML_POLICY_PATH         = f'{SHARED_XML_POLICY_BASE_PATH}/hello-world.xml'
 REQUEST_HEADERS_XML_POLICY_PATH     = f'{SHARED_XML_POLICY_BASE_PATH}/request-headers.xml'
-ACA_BACKEND_1_XML_POLICY_PATH       = f'{SHARED_XML_POLICY_BASE_PATH}/aca-backend-1.xml'
-ACA_BACKEND_2_XML_POLICY_PATH       = f'{SHARED_XML_POLICY_BASE_PATH}/aca-backend-2.xml'
-ACA_BACKEND_POOL_XML_POLICY_PATH    = f'{SHARED_XML_POLICY_BASE_PATH}/aca-backend-pool.xml'
+BACKEND_XML_POLICY_PATH             = f'{SHARED_XML_POLICY_BASE_PATH}/backend.xml'
 
 SUBSCRIPTION_KEY_PARAMETER_NAME = 'api_key'
 SLEEP_TIME_BETWEEN_REQUESTS_MS  = 50
diff --git a/tests/python/test_apimrequests.py b/tests/python/test_apimrequests.py
diff --git a/tests/python/test_apimtypes.py b/tests/python/test_apimtypes.py
diff --git a/tests/python/test_utils.py b/tests/python/test_utils.py

Original file line number	Diff line number	Diff line change
`@@ -78,7 +78,8 @@ module productHr '../../shared/bicep/modules/apim/v1/product.bicep' = [for produ`
`78`	`78`	`]`
`79`	`79`	`}]`
`80`	`80`
`81`		`-// APIM APIs`
	`81`	`+// APIM APIs (deployed after products are ready to avoid race conditions)`
	`82`	`+@batchSize(1) // Deploy APIs sequentially to avoid race conditions`
`82`	`83`	`module apisModule '../../shared/bicep/modules/apim/v1/api.bicep' = [for api in apis: {`
`83`	`84`	`name: 'api-${api.name}'`
`84`	`85`	`params:{`
`@@ -90,7 +91,8 @@ module apisModule '../../shared/bicep/modules/apim/v1/api.bicep' = [for api in a`
`90`	`91`	`}`
`91`	`92`	`dependsOn: [`
`92`	`93`	`namedValue // ensure all named values are created before APIs`
`93`		`- productHr // ensure products are created before APIs that reference them`
	`94`	`+ policyFragment // ensure all policy fragments are created before APIs`
	`95`	`+ productHr // ensure all products are fully created before APIs`
`94`	`96`	`]`
`95`	`97`	`}]`
`96`	`98`