Skip to content

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

Closed
wants to merge 5 commits into from

Conversation

NirajC-Microsoft
Copy link
Contributor

@NirajC-Microsoft NirajC-Microsoft commented May 30, 2025

##Purpose

  • Made changes in bicep files to reuse Resource group and Log Analytics workspace while deployment, removed existing code resource group creation and added condition in Monitor.bicep to check if existing Log Analytics workspace is provided or not.

Does this introduce a breaking change?

  • Yes
  • No

How to Test

  • Run command - azd auth login
  • To use existing log analytics run this command otherwise you can skip if you wanted to create new resource - azd env set AZURE_ENV_LOG_ANALYTICS_WORKSPACE_ID "/subscriptions//resourcegroups//providers/microsoft.operationalinsights/workspaces/"
  • Run command - azd up
  • I will not create log analytics resource it will use log analytics which you want to reuse

  • To verify Resource Group changes run command - azd up
  • enter/select all details.
  • After selecting region it will ask user to select existing resource group or create new resource group
git clone [repo-address]
cd [repo-name]
git checkout [branch-name]
npm install
  • Test the code

What to Check

Verify that the following are valid

  • ...

Other Information

Copy link

@Copilot Copilot AI left a 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 in main.bicepparam and main.bicep
  • Switched targetScope to resourceGroup, 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)
Copy link
Preview

Copilot AI May 30, 2025

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.

Suggested change
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
Copy link
Contributor

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

Copy link
Contributor Author

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

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.

2 participants