-
Notifications
You must be signed in to change notification settings - Fork 678
fix: infra fixes for terraform and bicep #233
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
/test-e2e |
|
🚀 @pauldotyu Starting E2E tests... Workflow Run: 16982292690 |
|
❌ @pauldotyu E2E tests failed! Workflow Run: 16982292690 Please check the workflow logs for details and try again. |
feat: bring back azd-hooks and preprovision script for azure provider registration and cli extension installation chore: remove azure-bicep.yaml file and add azure-build-from-source.yaml with bicep options commented chore: introduce azure-build-from-source.yaml for building images from source and deploying with kustomize (for e2e testing) refactor: revert azure.yaml to deploy with helm (better for non-e2e testing deployments) docs: enhance azd.md documentation for clarity on deployment options and environment variables fix: modify bicep parameters to default to false for observability tools and azure services chore: upgrade terraform provider versions in .terraform.lock.hcl for improved stability fix: update terraform main.tf and outputs.tf to correctly handle source registry and outputs
|
/test-e2e |
|
🚀 @pauldotyu Starting E2E tests... Workflow Run: 16999994699 |
|
✅ @pauldotyu E2E tests completed successfully! Workflow Run: 16999994699 All tests passed and resources have been cleaned up. |
…nt name which azd needs to be able to delete resources
|
/test-e2e |
|
🚀 @pauldotyu Starting E2E tests... Workflow Run: 17046960531 |
|
❌ @pauldotyu E2E tests failed! Workflow Run: 17046960531 Please check the workflow logs for details and try again. |
|
🚀 @pauldotyu Starting E2E tests... Workflow Run: 17046960531 |
|
✅ @pauldotyu E2E tests completed successfully! Workflow Run: 17046960531 All tests passed and resources have been cleaned up. |
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
…RE_SERVICE_BUS_URI
…Y setting behavior
|
/test-e2e |
|
🚀 @pauldotyu Starting E2E tests... Workflow Run: 17052994374 |
|
❌ @pauldotyu E2E tests failed! Workflow Run: 17052994374 Please check the workflow logs for details and try again. |
|
/test-e2e |
|
🚀 @pauldotyu Starting E2E tests... Workflow Run: 17053255276 |
There was a problem hiding this comment.
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 refactors the Azure Developer CLI deployment infrastructure to support multiple deployment scenarios, moving from a Kustomize-only approach to a default Helm-based deployment with pre-built container images. The key changes enable flexible deployment options while maintaining support for building from source.
- Restructures Azure infrastructure configuration to use direct Azure role assignments instead of the AVM role assignment module
- Updates all Azure Verified Modules (AVM) to their latest versions across Bicep and Terraform
- Replaces the complex service-specific Kustomize configuration with a simplified Helm-based deployment using pre-built container images
Reviewed Changes
Copilot reviewed 27 out of 28 changed files in this pull request and generated 16 comments.
Show a summary per file
| File | Description |
|---|---|
| infra/terraform/variables.tf | Updates default VM size to Standard_D2s_v4 |
| infra/terraform/servicebus.tf | Replaces AVM role assignment module with direct azurerm_role_assignment |
| infra/terraform/outputs.tf | Adds new deployment status outputs and renames registry-related outputs |
| infra/terraform/openai.tf | Updates AVM module version and replaces role assignment module |
| infra/terraform/main.tf | Removes Azure AD group creation and adds source registry local variable |
| infra/terraform/kubernetes.tf | Updates AVM module versions and replaces group-based RBAC with direct assignment |
| infra/terraform/cosmosdb.tf | Updates AVM module to latest version |
| infra/bicep/*.bicep | Updates all AVM module versions to latest releases |
| infra/bicep/main.parameters.json | Removes default false values from deployment parameters |
| infra/bicep/main.bicep | Changes default deployment flags to false and adds conditional resource access |
| azure.yaml | Simplifies from service-specific Kustomize to single Helm deployment with azd-hooks |
| azd-hooks/* | Adds deployment scripts for provider registration, Helm value generation, and container management |
| docs/azd.md | Updates documentation for new deployment approach and environment variables |
Files not reviewed (1)
- infra/terraform/.terraform.lock.hcl: Language not supported
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
|
✅ @pauldotyu E2E tests completed successfully! Workflow Run: 17053255276 All tests passed and resources have been cleaned up. |
Purpose
With 2.0.0 release of this app, the azd deployments have been revamped to take a different approach which includes using AZD's Kustomize capability and setting each subproject in this mono-repo as a service. This allows for e2e testing with each container image being built from source. However, there are instances where we want to run
azd upand deploy from pre-built container images in ghcr.io.So the following changes have been added to allow for that while still being flexible for other deployment options:
Does this introduce a breaking change?
Pull Request Type
What kind of change does this Pull Request introduce?
How to Test
azd upWhat to Check
Verify that the following are valid
Other Information