-
Notifications
You must be signed in to change notification settings - Fork 559
feat: Changes for reuse of resource group and log analytics while deployment #1812
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
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 enables reusing an existing Resource Group and Log Analytics workspace during deployments by introducing a parameter for the workspace ID and converting all modules to target the current resource group rather than creating a new one.
- Added
existingLogAnalyticsResourceId
parameter inmain.bicepparam
andmain.bicep
- Switched
targetScope
toresourceGroup
, removed explicit Resource Group creation, and updated module scopes - Enhanced the monitoring module to conditionally create a new Log Analytics workspace or use an existing one
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
infra/main.bicepparam | Added existingLogAnalyticsResourceId parameter |
infra/main.bicep | Changed targetScope , removed RG creation, updated module scopes, added existingLogAnalyticsResourceId |
infra/core/monitor/monitoring.bicep | Conditional loganalytics module, variables for existing workspace |
azure.yaml | Added infra provider configuration for Bicep path |
Comments suppressed due to low confidence (1)
infra/core/monitor/monitoring.bicep:13
- New conditional logic for creating or reusing Log Analytics workspace should be covered by tests (e.g., ensure both branches produce the expected outputs).
module logAnalytics 'loganalytics.bicep' = if (!useExistingLogAnalytics) {
infra/main.bicep
Outdated
@@ -356,7 +351,7 @@ module managedIdentityModule './core/security/managed-identity.bicep' = if (data | |||
solutionName: resourceToken | |||
solutionLocation: location | |||
} | |||
scope: rg | |||
scope: resourceGroup(resourceGroup().name) |
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.
Since targetScope
is set to resourceGroup
, these explicit scope
declarations are redundant and could be removed to simplify the module declarations.
scope: resourceGroup(resourceGroup().name) | |
// scope declaration removed as it is redundant |
Copilot uses AI. Check for mistakes.
@@ -14,20 +19,25 @@ module logAnalytics 'loganalytics.bicep' = { | |||
} | |||
} | |||
|
|||
var logAnalyticsWorkspaceId = useExistingLogAnalytics ? existingLogAnalyticsResourceId : logAnalytics.outputs.id |
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.
Move this to monitoring.bicep to keep the main.bicep clean
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.
This logic is already present in monitoring.bicep
##Purpose
Does this introduce a breaking change?
How to Test
What to Check
Verify that the following are valid
Other Information