fix(security/azure): switch Logic App scheduled tasks to user-assigned identities (unbreak Azure deploy)

[cristim]

Summary

Every Azure deploy since #74 fails terraform plan with:

Error: Missing required argument
  with module.compute_container_apps[0].azurerm_role_assignment.recommendations_kv_secrets_user[0],
  on ../../modules/compute/azure/container-apps/scheduled-tasks.tf line 379, in resource "azurerm_role_assignment" "recommendations_kv_secrets_user":
 379:   principal_id = azurerm_logic_app_workflow.recommendations[0].identity[0].principal_id
The argument "principal_id" is required, but no definition was found.

…and the same for cleanup_kv_secrets_user. (ri_exchange doesn't fail because enable_ri_exchange_schedule is currently false, so its role assignment has count=0 and isn't planned.)

Root cause

In azurerm 3.117.1, azurerm_logic_app_workflow.<x>.identity[0].principal_id evaluates to null at plan time when the resource is being created from scratch (no prior state). azurerm_role_assignment.principal_id is a required string the validator can't accept null for, so the plan fails before apply ever runs. There's no apply state for Terraform to learn the identity's principal_id from, so it never recovers.

Fix

Switch the three Logic App workflows from system-assigned to user-assigned managed identities. UA identities are first-class azurerm_user_assigned_identity resources whose .principal_id attribute is populated at plan time, so the role assignment resolves cleanly. The same module already uses this pattern for the Container App's own RBAC (azurerm_user_assigned_identity.container_app at main.tf:242), so the codebase is now consistent.

Changes in scheduled-tasks.tf:

• +3 azurerm_user_assigned_identity resources: one per workflow (recommendations, ri_exchange, cleanup), gated on the same enable_scheduled_tasks / enable_ri_exchange_schedule flags as their workflows.
• Each Logic App's identity block: type = "SystemAssigned" → type = "UserAssigned" with identity_ids = [azurerm_user_assigned_identity.<x>[0].id].
• Each role assignment's principal_id: reads azurerm_user_assigned_identity.<x>[0].principal_id directly — no more identity[0] chain.
• Each get-secret action body JSON: the authentication block gains identity = <UA-resource-id>. With UA identities the Logic Apps runtime needs to know which identity to use for the KV call (with SA there was exactly one, picked by default).

Variables, secret-name plumbing, and the precondition/validation block are unchanged. The change is contained to scheduled-tasks.tf.

Test plan

• terraform validate clean against terraform/environments/azure.
• Deploy run on merge — the actual repro that broke. Expect plan to succeed where it previously errored on principal_id.
• Post-deploy: trigger one Logic App workflow manually (via Azure portal "Run trigger") to confirm the get-secret action successfully fetches the KV secret using the UA identity. May 403 in the first 5–10 minutes if RBAC propagation lags — the existing depends_on = [azurerm_role_assignment.<x>_kv_secrets_user] on each get-secret action is preserved.

Out of scope

• Re-doing fix(security/azure): migrate Logic App SCHEDULED_TASK_SECRET to Key Vault connection #74's CodeRabbit nitpick about adding depends_on from the workflows themselves to the role assignments — already implemented at the *_get_secret level, which is the actual gate for runtime KV access.
• Sharing one UA identity across all three workflows. Per-workflow identities preserve the count-gating discipline (recommendations + cleanup share enable_scheduled_tasks; ri_exchange has its own flag) without complicating the role-assignment count expression.

🤖 Generated with claude-flow

Summary by CodeRabbit

• Chores
  • Enhanced security configuration for identity and authentication management across backend workflows to improve access control and credential handling.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 9bc25754-a5b4-4536-8b80-088e07e58cc0

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

📝 Walkthrough

Walkthrough

The PR migrates Logic App workflows' managed identity from system-assigned to user-assigned identities, adds new Azure identity resources for each workflow, updates identity configurations and Key Vault authentication references, and adjusts role assignments accordingly.

Changes

Cohort / File(s)

Summary

Managed Identity Migration terraform/modules/compute/azure/container-apps/scheduled-tasks.tf

Adds conditional azurerm_user_assigned_identity resources for three workflows (recommendations, ri_exchange, cleanup). Updates each workflow's identity block to reference the corresponding user-assigned identity. Modifies Key Vault get-secret actions to explicitly authenticate using the new user-assigned identity. Updates three azurerm_role_assignment resources to grant Key Vault Secrets User role to user-assigned identity principals instead of system-assigned identities.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• PR #74: Continues from the initial Logic App Key Vault migration; this PR switches managed identity type and wiring by transitioning from system-assigned to user-assigned identities with corresponding resource and role assignment updates.

Poem

🐰 Three workflows shed their system ways,
Now user-assigned identities guide their days!
With keys held close and permissions clear,
The rabbit hops—secure credentials here! 🔐✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically describes the main change: converting Logic App scheduled tasks from system-assigned to user-assigned identities, with context about fixing Azure deployments.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/azure-logic-app-uami

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[cristim]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

terraform/modules/compute/azure/container-apps/scheduled-tasks.tf (2)

4-10: ⚠️ Potential issue | 🟡 Minor

Stale comment: still references system-assigned identity.

The header comment at line 6 states "system-assigned managed identity" but the implementation now uses user-assigned identities. Update the comment to reflect the actual architecture.

📝 Suggested fix

 # SECURITY: The shared scheduled-task secret is NEVER interpolated into the
 # workflow definition or Terraform state. Each Logic App workflow has a
-# system-assigned managed identity that holds "Key Vault Secrets User" on the
+# user-assigned managed identity that holds "Key Vault Secrets User" on the
 # vault that stores `scheduled-task-secret`. At workflow runtime the first
 # action (`get-secret`) calls the Key Vault data-plane REST API authenticated
 # by the workflow's managed identity, and the call-endpoint action references

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@terraform/modules/compute/azure/container-apps/scheduled-tasks.tf` around
lines 4 - 10, Update the header comment to replace "system-assigned managed
identity" with the actual "user-assigned managed identity" architecture: state
that each Logic App workflow is assigned a user-assigned managed identity which
has the "Key Vault Secrets User" role on the vault storing
`scheduled-task-secret`, and that at runtime the `get-secret` action calls the
Key Vault data-plane REST API authenticated by that user-assigned identity and
references `@body('get-secret')['value']` in the outgoing Authorization header.

---

140-142: ⚠️ Potential issue | 🟡 Minor

Stale comment: references system-assigned identity.

This comment still says "system-assigned managed identity" but the implementation now uses user-assigned. Update for consistency.

📝 Suggested fix

 # Step 1: Fetch the shared secret from Key Vault using the workflow's
-# system-assigned managed identity. The secret value lives in the workflow
+# user-assigned managed identity. The secret value lives in the workflow
 # run's transient state only — never in the workflow definition or TF state.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@terraform/modules/compute/azure/container-apps/scheduled-tasks.tf` around
lines 140 - 142, Update the stale Step 1 comment that currently says
"system-assigned managed identity" to reflect the implementation using a
user-assigned managed identity: replace that phrase with "user-assigned managed
identity" and adjust the surrounding wording so it clearly states the secret is
fetched using the workflow's user-assigned managed identity and remains
transient (not stored in workflow definition or TF state); locate and edit the
comment block that begins "Step 1: Fetch the shared secret..." in the
scheduled-tasks.tf file.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@terraform/modules/compute/azure/container-apps/scheduled-tasks.tf`:
- Around line 4-10: Update the header comment to replace "system-assigned
managed identity" with the actual "user-assigned managed identity" architecture:
state that each Logic App workflow is assigned a user-assigned managed identity
which has the "Key Vault Secrets User" role on the vault storing
`scheduled-task-secret`, and that at runtime the `get-secret` action calls the
Key Vault data-plane REST API authenticated by that user-assigned identity and
references `@body('get-secret')['value']` in the outgoing Authorization header.
- Around line 140-142: Update the stale Step 1 comment that currently says
"system-assigned managed identity" to reflect the implementation using a
user-assigned managed identity: replace that phrase with "user-assigned managed
identity" and adjust the surrounding wording so it clearly states the secret is
fetched using the workflow's user-assigned managed identity and remains
transient (not stored in workflow definition or TF state); locate and edit the
comment block that begins "Step 1: Fetch the shared secret..." in the
scheduled-tasks.tf file.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 70577e89-1c07-486f-88bb-220a6527c853

📥 Commits

Reviewing files that changed from the base of the PR and between 9351fb2 and 840f594.

📒 Files selected for processing (1)

• terraform/modules/compute/azure/container-apps/scheduled-tasks.tf

[cristim]

Pushed 4389faf87 — addressed both stale-comment nits.

File header comment + the Step 1 comment above recommendations_get_secret both now describe the user-assigned identity wiring. The other system-assigned references in the file (around lines 66, 67, 161) are deliberate — they explain why the migration happened (SA principal_id plan-time null), so they stay.

Comment-only change; no terraform plan diff. Ready to merge once CI catches up.