Skip to content

Conversation

@simonkurtz-MSFT
Copy link
Member

@simonkurtz-MSFT simonkurtz-MSFT commented Jun 2, 2025

Resolves #19

@simonkurtz-MSFT simonkurtz-MSFT requested a review from Copilot June 2, 2025 19:30
@simonkurtz-MSFT simonkurtz-MSFT self-assigned this Jun 2, 2025
@github-actions
Copy link

github-actions bot commented Jun 2, 2025

Python 3.13 Test Results

101 tests  +12   101 ✅ +12   1s ⏱️ ±0s
  1 suites ± 0     0 💤 ± 0 
  1 files   ± 0     0 ❌ ± 0 

Results for commit 9927f28. ± Comparison against base commit 001feeb.

@github-actions
Copy link

github-actions bot commented Jun 2, 2025

Python 3.12 Test Results

101 tests  +12   101 ✅ +12   1s ⏱️ ±0s
  1 suites ± 0     0 💤 ± 0 
  1 files   ± 0     0 ❌ ± 0 

Results for commit 9927f28. ± Comparison against base commit 001feeb.

@simonkurtz-MSFT simonkurtz-MSFT merged commit 17e21f9 into main Jun 2, 2025
7 checks passed
@simonkurtz-MSFT simonkurtz-MSFT deleted the feature/add-policy-fragment-and-product branch June 2, 2025 19:32
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Adds support for reusable policy fragments and product associations in APIM modules and samples.

  • Introduces a new policy-fragment.bicep module to create/manage APIM policy fragments.
  • Extends api.bicep with a productNames parameter, conditional resource association to products, and related outputs.
  • Updates sample Bicep and XML policy files to wire in policy fragments, product links, and use !empty() checks.

Reviewed Changes

Copilot reviewed 23 out of 23 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
shared/bicep/modules/apim/v1/policy-fragment.bicep New module for defining and outputting APIM policy fragments
shared/bicep/modules/apim/v1/api.bicep Added productNames param, refined conditions for policies/operations, and product association resource and outputs
shared/apim-policies/fragments/pf-http-response-200.xml Added HTTP-200 response policy fragment
shared/apim-policies/fragments/pf-authz-match-any.xml Added authorization match-any policy fragment
samples/*/main.bicep & _TEMPLATE/main.bicep Switched length(...) > 0 to !empty(...) for arrays in sample modules
samples/authX-pro/main.bicep & related XML/IPython notebooks Wired in policy fragments, products, and updated README link
README.md Added AuthX Pro sample entry
Comments suppressed due to low confidence (2)

shared/bicep/modules/apim/v1/policy-fragment.bicep:60

  • [nitpick] The output policyFragmentName shadows the input parameter name; consider renaming the output (e.g., createdPolicyFragmentName) to avoid confusion.
output policyFragmentName string = policyFragment.name

shared/bicep/modules/apim/v1/api.bicep:149

  • There are two closing braces on this line which appears to over-close the backend and properties blocks; remove the extra } to balance the resource declaration.
    }  }

| [General](./samples/general/create.ipynb) | Basic demo of APIM sample setup and policy usage. | All infrastructures |
| [Load Balancing](./samples/load-balancing/create.ipynb) | Priority and weighted load balancing across backends. | apim-aca, afd-apim (with ACA) |
| [AuthX](./samples/authx/create.ipynb) | Authentication and role-based authorization in a mock HR API. | All infrastructures |
| [AuthX Pro](./samples/authx-pro/create.ipynb) | Authentication and role-based authorization in a mock product with multiple APIs and policy fragments. | All infrastructures |
Copy link

Copilot AI Jun 2, 2025

Choose a reason for hiding this comment

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

The link path uses lowercase authx-pro, but the folder is named authX-pro; adjust casing so the link resolves correctly on case-sensitive file systems.

Copilot uses AI. Check for mistakes.
Comment on lines +177 to +181
// @description('Array of product names this API is associated with.')
// output associatedProducts string[] = productNames

// @description('Number of products this API is associated with.')
// output productAssociationCount int = length(productNames)
Copy link

Copilot AI Jun 2, 2025

Choose a reason for hiding this comment

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

[nitpick] These commented-out outputs for associated products and count should be either removed if obsolete or uncommented and implemented to avoid dead code clutter.

Suggested change
// @description('Array of product names this API is associated with.')
// output associatedProducts string[] = productNames
// @description('Number of products this API is associated with.')
// output productAssociationCount int = length(productNames)
@description('Array of product names this API is associated with.')
output associatedProducts string[] = productNames
@description('Number of products this API is associated with.')
output productAssociationCount int = length(productNames)

Copilot uses AI. Check for mistakes.
@description('The resource ID for Application Insights')
param appInsightsId string = ''

@description('Array of product names to associate this API with. If empty, no product associations will be created.')
Copy link

Copilot AI Jun 2, 2025

Choose a reason for hiding this comment

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

[nitpick] Consider adding a module-level header comment to document the new productNames parameter alongside the existing parameters section for consistency.

Copilot uses AI. Check for mistakes.
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.

[Scenario]: Add a more sophisticated AuthX Scenario

2 participants