Skip to content

Fix: make show-core-output is failing due to lack of access to management storage account#4407

Merged
jonnyry merged 5 commits into
microsoft:mainfrom
ashis-kar91:ashiskar/show-output-mgmt-storage-access
Feb 28, 2025
Merged

Fix: make show-core-output is failing due to lack of access to management storage account#4407
jonnyry merged 5 commits into
microsoft:mainfrom
ashis-kar91:ashiskar/show-output-mgmt-storage-access

Conversation

@ashis-kar91

Copy link
Copy Markdown
Collaborator

Resolves #4404

What is being addressed

  • The make show-core-output command in the make file was executing terraform show was failing due to lack of access to the management storage account. Because of this TRE deployments were failing while using the make all command.
  • There were too many redundant error messages being logged from devops/scripts/mgmtstorage_enable_public_access.sh during every attempt to verify the storage account access.

How is this addressed

  • Added a new script file core/terraform/show_output.sh, which will be using terraform_wrapper.sh to execute the terraform show. terraform_wrapper.sh already has the script for management storage exception.
  • For the redundant error messages in devops/scripts/mgmtstorage_enable_public_access.sh, captured the underlying error in every attempt and printing it at the end if all the attempts fail.
  • Also removed the redundant access checks in devops/scripts/mgmtstorage_enable_public_access.sh before adding and removing network rules, since both the commands are idempotent.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Overview

This PR resolves the issue with the failing management storage access when running the make show-core-output command and cleans up redundant error messages. It also streamlines access verification in the associated shell script.

  • Resolves management storage access issue for terraform show command.
  • Consolidates error message handling in mgmtstorage_enable_public_access.sh.
  • Removes unnecessary access checks to simplify idempotent network rule operations.

Reviewed Changes

File Description
CHANGELOG.md Updated the change log with details on the bug fix and error cleanup.

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Comment thread CHANGELOG.md Outdated
@github-actions

github-actions Bot commented Feb 28, 2025

Copy link
Copy Markdown

Unit Test Results

0 tests   0 ✅  0s ⏱️
0 suites  0 💤
0 files    0 ❌

Results for commit 8956946.

♻️ This comment has been updated with latest results.

Ashis Kar added 2 commits February 28, 2025 04:32
…lean up error messages in mgmtstorage_enable_public_access.sh script
@ashis-kar91

Copy link
Copy Markdown
Collaborator Author

/test 8956946

@github-actions

Copy link
Copy Markdown

🤖 pr-bot 🤖

🏃 Running tests: https://github.com/microsoft/AzureTRE/actions/runs/13581524730 (with refid 89e36f33)

(in response to this comment from @ashis-kar91)

@marrobi marrobi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jonnyry jonnyry left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good, thanks very much :-)

@jonnyry jonnyry merged commit 3fcbdf9 into microsoft:main Feb 28, 2025
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.

make show-core-output is failing due to lack of access to management storage account

4 participants