chore: record ACA API custom-domain DNS after cutover - #306
Conversation
Fill Route53 CNAME/TXT for api.plattform-kit.poc, pass --environment on ACA managed-cert bind, and note App Insights as a production ACA prerequisite. Co-authored-by: Cursor <cursoragent@cursor.com>
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis PR records the live ACA custom-domain identity in configuration, fixes managed-certificate binding by supplying the ACA environment to Azure CLI, and documents the required Application Insights prerequisite for ACA API deployment. Sequence diagram for ACA managed certificate bindingsequenceDiagram
participant Script as bind-custom-domains.ps1
participant Config as custom-domains.pocpk.json
participant AzureCLI as Azure CLI
participant ACA as Azure Container Apps
Script->>Config: Read environmentName
Config-->>Script: ACA environment name or default
Script->>AzureCLI: az containerapp hostname bind --environment
AzureCLI->>ACA: Bind hostname with managed certificate
ACA-->>AzureCLI: Binding result
AzureCLI-->>Script: Success or failure
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Human Review Brief — chore: record ACA API custom-domain DNS after cutover
Summary
Test evidence
Preview URLs
Open review threads
Infra blockers
Visual review
Next actionagent fixing comments |
Co-authored-by: Cursor <cursoragent@cursor.com>
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="scripts/bind-custom-domains.ps1" line_range="110-114" />
<code_context>
if ($wantCert) {
- Write-Host "==> Managed certificate bind for $hostName (ACA)"
- az containerapp hostname bind -n $name -g $rg --hostname $hostName --validation-method CNAME
+ $envName = if ($b.PSObject.Properties['environmentName'] -and $b.environmentName) {
+ [string]$b.environmentName
+ } else {
+ 'ssd-pocpk-cae-dev-ae'
+ }
+ Write-Host "==> Managed certificate bind for $hostName (ACA env $envName)"
+ # Managed certs require --environment; without it az fails with "specify --certificate and --environment".
</code_context>
<issue_to_address>
**issue (bug_risk):** When the script is run with a custom-domains configuration for a Container App in an environment other than `ssd-pocpk-cae-dev-ae`, the fallback selects this POC-specific environment and `az containerapp hostname bind` targets the wrong environment, causing the bind to fail or applying it to the wrong ACA environment.
**Triggers:** When a containerapp binding omits `environmentName` and uses a different ACA environment.
**Suggested fix:** Require `environmentName` for containerapp bindings or resolve the environment from the target Container App instead of using a POC-specific fallback.
```suggestion
$envName = if ($b.PSObject.Properties['environmentName'] -and $b.environmentName) {
[string]$b.environmentName
} else {
throw "Container App binding '$name' requires environmentName when managedCert is enabled."
}
```
</issue_to_address>Sourcery assessment
Needs a human reviewer. 1 finding to address first, and if the ACA hostname or verification record is wrong, the public API domain could route requests to the wrong container app or leave the custom-domain certificate binding in an incorrect state. Reverting the files restores the intended configuration, but requests and TLS exposure during the bad DNS window cannot be undone.
Blocking findings: scripts/bind-custom-domains.ps1:114
|
Warning Review limit reachedNext included review available in 53 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe change fixes ACA managed certificate binding, replaces custom-domain placeholders with production values, and adds an Application Insights prerequisite to the ACA cutover runbook. ChangesACA custom-domain cutover
Estimated code review effort: 2 (Simple) | ~10 minutes Severity of issue fixed: Low Merge Risk: ⚪ Minimal · up to This change completes ACA custom-domain cutover configuration and documents its deployment prerequisite. The supplied checks and configuration evidence indicate no remaining merge-blocking risk. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🟢 Approval recommended
The changes are low-risk ops/config updates consistent with the PR description, with only a minor non-blocking robustness suggestion noted in review comments.
Pull request overview
This PR captures the post-cutover state for the production API custom domain on Azure Container Apps (ACA) and aligns operational scripts/docs with current Azure CLI and deployment prerequisites.
Changes:
- Updates
scripts/bind-custom-domains.ps1to pass--environmentwhen binding ACA managed certificates. - Records the live ACA FQDN and
customDomainVerificationIdininfra/custom-domains.pocpk.json. - Documents Application Insights (
ssd-pocpk-appi-dev-ae) as a prerequisite for ACA API deployment in the cutover runbook.
File summaries
| File | Description |
|---|---|
| scripts/bind-custom-domains.ps1 | Adds ACA environment support for managed certificate hostname binding via Azure CLI. |
| infra/custom-domains.pocpk.json | Replaces placeholders with the live ACA ingress FQDN and verification TXT value after cutover. |
| docs/aca-api-cutover-303.md | Updates the cutover checklist to include the required App Insights resource prerequisite. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
Picking up has-feedback: derive ACA environment from the Container App (schema optional override) and fix cutover doc step reference. |
Derive managedEnvironmentId from the Container App instead of a hard-coded CAE fallback, document optional environmentName in the schema, and point DNS cutover at step 4 for the FQDN. Co-authored-by: Cursor <cursoragent@cursor.com>
Summary
customDomainVerificationIdintoinfra/custom-domains.pocpk.json(already applied in Route53).bind-custom-domains.ps1to pass--environmentfor managed certs (required by currentaz).ssd-pocpk-appi-dev-aeas a prerequisite fordeploy-aca-api.sh.Ops already done (this chat)
ssd-pocpk-appi-dev-aessd-pocpk-aca-api-dev-aewith imagepocpk-api:1b94b07…api.plattform-kit.pocCNAME → ACASniEnabled)--api-identity containerapphttps://api.plattform-kit.poc.singletonsd.com/health+/health/db→ 200Test plan
infra/custom-domains.pocpk.jsonmatches liveaz containerapp showFQDN + verification id.curl -sS https://api.plattform-kit.poc.singletonsd.com/health→ 200.Preview scenarios: not-applicable - DNS/config docs only, no API/data model change.
Made with Cursor
Summary by Sourcery
Record the live ACA custom-domain configuration and improve the deployment and cutover guidance for reliable certificate binding.
Bug Fixes:
Enhancements:
Documentation:
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
The PR appears safe to merge.
Summary
Reviews (2) · Last reviewed commit: "fix(infra): resolve ACA env for cert bin..."