Skip to content

fix(develop): 17 corrections + orphan fix for PR #1636#2026

Merged
paulyuk merged 2 commits into
microsoft:pass-equity-gap-2from
simonjj:fix/pr-1636-develop
Apr 24, 2026
Merged

fix(develop): 17 corrections + orphan fix for PR #1636#2026
paulyuk merged 2 commits into
microsoft:pass-equity-gap-2from
simonjj:fix/pr-1636-develop

Conversation

@simonjj

@simonjj simonjj commented Apr 23, 2026

Copy link
Copy Markdown
Contributor

Fixes applied to PR #1636 (Container Apps Develop)

19 corrections across 10 files (17 content fixes + 1 orphan fix + 1 version bump).

Critical Fixes (6)

  1. Cosmos DB: disableLocalAuthentication to disableLocalAuth (correct ARM API property)
  2. Cosmos RBAC: changed to sqlRoleAssignments (data-plane RBAC)
  3. Redis RBAC: Redis Cache Contributor to Redis Cache Data Owner (e12a10f1-dcd0-4ee7-abb0-0b2e24e345e7)
  4. Redis Dapr: added missing useEntraID metadata for MI auth
  5. Event job: added identity block and registries
  6. Functions: added AzureWebJobsStorage__clientId env var

Major Fixes (10)

  1. Worker/ServiceBus/Functions: KEDA auth[] to identity field
  2. Worker: wired uamiClientId param to AZURE_CLIENT_ID
  3. Worker: added missing EventHub/Queue scaler metadata
  4. Redis: rewrote Node.js SDK to token-based auth
  5. Composition: fixed param name mismatch
  6. Composition: Cosmos RBAC resource type
  7. Composition: reworded contradictory rule
  8. Microservice: declared logAnalytics params
  9. API: added CORS production warning
  10. ServiceBus: added identity-based auth tip

CI + Orphan Fix

  1. SKILL.md version bump 1.1.1 to 1.1.2
  2. Linked all 15 template files into skill reference chain via README/analyze/research

Model-Matrix Test Results: 15/15 PASS

GPT-5 mini, GPT-5.3-Codex, Claude Haiku 4.5, Claude Sonnet 4.6, Claude Opus 4.6 all passed.

simonjj and others added 2 commits April 22, 2026 13:50
Critical fixes (6):
- Cosmos DB: disableLocalAuth property name (ARM API)
- Cosmos DB: sqlRoleAssignments for data-plane RBAC
- Redis: Redis Cache Data Owner role + correct GUID
- Redis: add useEntraID to Dapr state store metadata
- Event job: add identity block and registries
- Functions: add AzureWebJobsStorage__clientId

Major fixes (10):
- Worker/ServiceBus/Functions: KEDA identity-based auth
- Worker: wire uamiClientId param to AZURE_CLIENT_ID
- Worker: add missing EventHub/Queue scaler fields
- Redis: fix Node.js SDK to use token-based auth
- Composition: fix param name principalId mismatch
- Composition: Cosmos RBAC resource type correction
- Composition: reword contradictory rule microsoft#2
- Microservice: declare logAnalytics params
- API: add CORS production warning

CI fix:
- SKILL.md version bump 1.1.1 -> 1.1.2

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add Templates & Recipes section to container-apps/README.md linking
to selection.md, composition.md, and recipes/README.md — resolves
orphaned files CI check.

Also add ACA template loading guidance in analyze.md and research.md
for parity with existing Functions template references.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings April 23, 2026 19:05

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR applies a set of corrective updates to the Azure Container Apps “develop” templates/recipes under azure-prepare, aligning examples with managed-identity auth patterns (KEDA identity fields, Entra ID for Redis, Cosmos property name), and fixes reference-chain/orphan issues by linking templates from higher-level docs. It also bumps the azure-prepare skill version.

Changes:

  • Update Container Apps templates/recipes to use managed identity for KEDA scalers and ACR pulls, plus fix several Azure resource configuration details (Cosmos local auth property, Cosmos SQL RBAC type, Redis RBAC role).
  • Update docs to link selection/composition guidance into the reference chain (README/analyze/research).
  • Bump plugin/skills/azure-prepare skill version.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
plugin/skills/azure-prepare/references/services/container-apps/templates/worker.md Adds uamiClientId, switches Service Bus scaler auth to MI identity, expands scaler metadata examples
plugin/skills/azure-prepare/references/services/container-apps/templates/recipes/servicebus/README.md Switches KEDA rule from auth[] to identity + adds MI tip
plugin/skills/azure-prepare/references/services/container-apps/templates/recipes/redis/README.md Updates Redis RBAC role, switches Node example to token-based auth, adds Dapr useEntraID
plugin/skills/azure-prepare/references/services/container-apps/templates/recipes/cosmos/README.md Fixes Cosmos DB account property to disableLocalAuth and updates warning text
plugin/skills/azure-prepare/references/services/container-apps/templates/recipes/composition.md Fixes composition param naming + switches Cosmos RBAC example to sqlRoleAssignments
plugin/skills/azure-prepare/references/services/container-apps/templates/microservice.md Reworks Log Analytics wiring to use explicit parameters
plugin/skills/azure-prepare/references/services/container-apps/templates/job.md Adds identity + registry config for event-triggered job example
plugin/skills/azure-prepare/references/services/container-apps/templates/functions-on-aca.md Adds AzureWebJobsStorage__clientId and MI identity for Service Bus scaler
plugin/skills/azure-prepare/references/services/container-apps/templates/api.md Adds production warning for permissive CORS origins
plugin/skills/azure-prepare/references/services/container-apps/README.md Adds links to selection guide and composition algorithm
plugin/skills/azure-prepare/references/research.md Updates research flow to include Container Apps template selection/composition docs
plugin/skills/azure-prepare/references/analyze.md Adds explicit Container Apps composition loading + “don’t hand-write IaC” guidance
plugin/skills/azure-prepare/SKILL.md Bumps skill metadata version from 1.1.1 → 1.1.2
Comments suppressed due to low confidence (1)

plugin/skills/azure-prepare/references/services/container-apps/templates/job.md:137

  • The event-triggered job now configures a UAMI at the job level and uses it for ACR pulls, but the KEDA event trigger rule shown below still has no managed-identity auth configuration. For consistency with the other templates/recipes, the event trigger scale.rules[] should include the identity-based auth field (or explicitly document the required auth mechanism) so the example actually works without connection strings.
      registries: [
        {
          server: '${containerRegistryName}.azurecr.io'
          identity: userAssignedIdentityId
        }
      ]
      eventTriggerConfig: {
        parallelism: 1
        replicaCompletionCount: 1
        scale: {
          minExecutions: 0
          maxExecutions: 10
          rules: [
            {
              name: 'queue-trigger'
              type: 'azure-servicebus'
              metadata: {
                namespace: '<sb-namespace>'
                queueName: '<queue>'
                messageCount: '1'
              }
            }

Comment on lines +112 to +115
{
name: 'AzureWebJobsStorage__clientId'
value: uamiClientId // Required for UAMI — runtime defaults to system MI without this
}

Copilot AI Apr 23, 2026

Copy link

Choose a reason for hiding this comment

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

uamiClientId is referenced for AzureWebJobsStorage__clientId, but this Bicep snippet doesn't declare a uamiClientId parameter (or otherwise define it). As written, the template won't compile; add a param uamiClientId string (consistent with other templates) or derive the clientId from the UAMI resource ID being passed in.

Copilot uses AI. Check for mistakes.
Comment on lines 84 to 90
namespace: sb.outputs.namespace
queueName: sb.outputs.queueName
messageCount: '5'
}
auth: [
{
secretRef: 'sb-identity'
triggerParameter: 'connection'
}
]
identity: managedIdentityResourceId
}
}

Copilot AI Apr 23, 2026

Copy link

Choose a reason for hiding this comment

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

The KEDA rule sets identity: managedIdentityResourceId, but managedIdentityResourceId isn't defined anywhere in this snippet (and earlier sections use principalId / uami.outputs.clientId). Rename this to the established userAssignedIdentityId (resource ID) or explicitly show how managedIdentityResourceId is obtained/passed so the example is copy/pasteable.

Copilot uses AI. Check for mistakes.
@@ -100,12 +101,7 @@ resource worker 'Microsoft.App/containerApps@2024-03-01' = {
queueName: queueName
messageCount: '5'
}

Copilot AI Apr 23, 2026

Copy link

Choose a reason for hiding this comment

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

The main worker example uses managed identity auth for the Service Bus scaler (identity: userAssignedIdentityId), but the later “Service Bus Queue Scaling (KEDA)” snippet in the same file still omits identity entirely. That inconsistency makes it easy to copy the wrong pattern; update the scaling-pattern snippet to include identity (or call out that it’s required for MI auth).

Suggested change
}
}
// Required when the Service Bus scaler uses managed identity authentication.

Copilot uses AI. Check for mistakes.
Comment on lines +81 to +87
resource cosmosRbac 'Microsoft.DocumentDB/databaseAccounts/sqlRoleAssignments@2024-05-15' = {
parent: cosmos
name: guid(cosmos.id, uami.id, 'data-contributor')
properties: {
roleDefinitionId: subscriptionResourceId(
'Microsoft.Authorization/roleDefinitions',
'00000000-0000-0000-0000-000000000002' // from recipe
)
roleDefinitionId: '${cosmos.id}/sqlRoleDefinitions/00000000-0000-0000-0000-000000000002'
principalId: uami.outputs.principalId
principalType: 'ServicePrincipal'
scope: cosmos.id

Copilot AI Apr 23, 2026

Copy link

Choose a reason for hiding this comment

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

This RBAC example mixes incompatible shapes for uami: it uses uami.outputs.principalId (module output) but also uami.id (resource property). Bicep modules don't have .id, and resources don't have .outputs, so the snippet won't compile as-is. Pick one representation (module vs resource/param) and update all uami.* references consistently (including the guid(...) inputs).

Copilot uses AI. Check for mistakes.
| Redis Cache Contributor | `e0f68234-74aa-48ed-b826-c38b57376e17` | Manage cache + data |
| Redis Cache Data Access | Custom role | Data plane operations |
| Redis Cache Data Owner | `e12a10f1-dcd0-4ee7-abb0-0b2e24e345e7` | Read + write data |
| Redis Cache Data Contributor | `e12a10f1-dcd0-4ee7-abb0-0b2e24e345c2` | Read-only data |

Copilot AI Apr 23, 2026

Copy link

Choose a reason for hiding this comment

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

In the RBAC roles table, “Redis Cache Data Contributor” is described as “Read-only data”, which conflicts with the role name and is inconsistent with the Cosmos recipe using “Data Reader” for read-only access. Please verify the intended built-in role and GUID, and either change the access description or switch to the appropriate read-only role name/GUID to avoid misleading guidance.

Suggested change
| Redis Cache Data Contributor | `e12a10f1-dcd0-4ee7-abb0-0b2e24e345c2` | Read-only data |
| Redis Cache Data Contributor | `e12a10f1-dcd0-4ee7-abb0-0b2e24e345c2` | Read + write data |

Copilot uses AI. Check for mistakes.
@paulyuk

paulyuk commented Apr 24, 2026

Copy link
Copy Markdown
Member

Thanks Simon — great work on this! 🎉

I cross-referenced your Cosmos DB fixes against the known-good production template (functions-quickstart-dotnet-azd-cosmosdb) and everything checks out:

Fix Your PR Reference Template
disableLocalAuth (not disableLocalAuthentication) disableLocalAuth: true in db.bicep
sqlRoleAssignments resource type Microsoft.DocumentDB/databaseAccounts/sqlRoleAssignments Same in rbac.bicep
roleDefinitionId format {cosmos.id}/sqlRoleDefinitions/00000000-...-02 cosmosFQRoleID same pattern
scope: cosmos.id scope: cosmosDbAccount.id

You addressed 13 of 16 JonG review comments — the remaining 3 (ACR AcrPull/AcrPush split, Functions azure.yaml metadata, PostgreSQL user field) I'll fix directly on the branch after merging this.

Merging now — thanks for the thorough testing across 5 models too!

@paulyuk
paulyuk merged commit 1452d81 into microsoft:pass-equity-gap-2 Apr 24, 2026
6 of 7 checks passed
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.

3 participants