From f32c2d25a1811e070ebc151e0c39f5d5cf95d6dd Mon Sep 17 00:00:00 2001 From: Ashis Kar Date: Thu, 27 Feb 2025 21:29:24 +0000 Subject: [PATCH 1/5] Refactor Makefile to use show_output.sh for displaying Terraform output and improve error handling in mgmtstorage scripts. --- Makefile | 2 +- core/terraform/show_output.sh | 12 ++++++++ .../mgmtstorage_enable_public_access.sh | 28 ++++++++----------- 3 files changed, 25 insertions(+), 17 deletions(-) create mode 100755 core/terraform/show_output.sh diff --git a/Makefile b/Makefile index b879490f3..c378a6227 100644 --- a/Makefile +++ b/Makefile @@ -388,7 +388,7 @@ auth: ## 🔐 Create the necessary Azure Active Directory assets show-core-output: $(call target_title,"Display TRE core output") \ && . ${MAKEFILE_DIR}/devops/scripts/check_dependencies.sh env \ - && pushd ${MAKEFILE_DIR}/core/terraform/ > /dev/null && terraform show && popd > /dev/null + && pushd ${MAKEFILE_DIR}/core/terraform/ > /dev/null && . ./show_output.sh && popd > /dev/null api-healthcheck: $(call target_title,"Checking API Health") \ diff --git a/core/terraform/show_output.sh b/core/terraform/show_output.sh new file mode 100755 index 000000000..baab90bb7 --- /dev/null +++ b/core/terraform/show_output.sh @@ -0,0 +1,12 @@ +#!/bin/bash + +set -o errexit +set -o pipefail +set -o nounset +# set -o xtrace + +# shellcheck disable=SC2154 +../../devops/scripts/terraform_wrapper.sh -g "${TF_VAR_mgmt_resource_group_name}" \ + -s "${TF_VAR_mgmt_storage_account_name}" \ + -n "${TF_VAR_terraform_state_container_name}" \ + -k "${TRE_ID}" -c "terraform show" diff --git a/devops/scripts/mgmtstorage_enable_public_access.sh b/devops/scripts/mgmtstorage_enable_public_access.sh index 605d55aaa..156e6529f 100644 --- a/devops/scripts/mgmtstorage_enable_public_access.sh +++ b/devops/scripts/mgmtstorage_enable_public_access.sh @@ -6,6 +6,9 @@ # Note: Ensure you "source" this script, or else the EXIT trap won't fire at the right time. # +# Global variable to capture underlying error +LAST_PUBLIC_ACCESS_ERROR="" + function mgmtstorage_enable_public_access() { local RESOURCE_GROUP RESOURCE_GROUP=$(get_resource_group_name) @@ -19,12 +22,6 @@ function mgmtstorage_enable_public_access() { exit 1 fi - # Pre-check: if public access is already enabled, no need to update. - if is_public_access_enabled "$RESOURCE_GROUP" "$SA_NAME"; then - echo -e " Storage account $SA_NAME is already publicly accessible\n" - return - fi - echo -e "\nEnabling public access on storage account $SA_NAME" # Enable public network access with explicit default action allow @@ -41,6 +38,7 @@ function mgmtstorage_enable_public_access() { done echo -e "Error: Could not enable public access for $SA_NAME after 10 attempts.\n" + echo -e "$LAST_PUBLIC_ACCESS_ERROR\n" exit 1 } @@ -57,12 +55,6 @@ function mgmtstorage_disable_public_access() { exit 1 fi - # Pre-check: if public access is already disabled, no need to update. - if ! is_public_access_enabled "$RESOURCE_GROUP" "$SA_NAME"; then - echo -e " Storage account $SA_NAME is already not publicly accessible\n" - return - fi - echo -e "\nDisabling public access on storage account $SA_NAME" # Disable public network access with explicit default action deny @@ -105,16 +97,20 @@ function does_storage_account_exist() { function is_public_access_enabled() { local RESOURCE_GROUP="$1" local SA_NAME="$2" + LAST_PUBLIC_ACCESS_ERROR="" - # Try listing containers + # Try listing containers and capture error output local containers - if ! containers=$(az storage container list --account-name "$SA_NAME" --auth-mode login --query "[].name" --output tsv); then + if ! containers=$(az storage container list --account-name "$SA_NAME" --auth-mode login --query "[].name" --output tsv 2>&1); then + LAST_PUBLIC_ACCESS_ERROR="$containers" return 1 fi - # For each container found, check blob listing + # For each container found, check blob listing and capture error if any for container in $containers; do - if ! az storage blob list --container-name "$container" --account-name "$SA_NAME" --auth-mode login --output none; then + local blob_output + if ! blob_output=$(az storage blob list --container-name "$container" --account-name "$SA_NAME" --auth-mode login --output none 2>&1); then + LAST_PUBLIC_ACCESS_ERROR="$blob_output" return 1 fi done From 2b9c700224c1ad9809ec9aca6552ac162893ace3 Mon Sep 17 00:00:00 2001 From: Ashis Kar Date: Fri, 28 Feb 2025 04:25:49 +0000 Subject: [PATCH 2/5] CHANGELOG file update with bug fix details. --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3b5da97ea..d24c10b2c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,7 @@ ENHANCEMENTS: * Deny public access to TRE management storage account, and add private endpoint for TRE core [#4353](https://github.com/microsoft/AzureTRE/issues/4353) BUG FIXES: - +* Fix the management storage access error from `make show-core-output` command, and remove redundand error messages from `mgmtstorage_enable_public_access.sh` script ([#4404](https://github.com/microsoft/AzureTRE/issues/4404)) ## 0.21.0 From 0a9475181f99c14016648c3393e0298bde95713e Mon Sep 17 00:00:00 2001 From: Ashis Kar Date: Fri, 28 Feb 2025 04:29:44 +0000 Subject: [PATCH 3/5] Update CHANGELOG to clarify management storage access error fix in `make show-core-output` --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d24c10b2c..0844895eb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,7 @@ ENHANCEMENTS: * Deny public access to TRE management storage account, and add private endpoint for TRE core [#4353](https://github.com/microsoft/AzureTRE/issues/4353) BUG FIXES: -* Fix the management storage access error from `make show-core-output` command, and remove redundand error messages from `mgmtstorage_enable_public_access.sh` script ([#4404](https://github.com/microsoft/AzureTRE/issues/4404)) +* Fix the management storage access error while executing `make show-core-output` command, and remove redundand error messages from `mgmtstorage_enable_public_access.sh` script ([#4404](https://github.com/microsoft/AzureTRE/issues/4404)) ## 0.21.0 From a493eb1dda3ba85904e1a8d0c211005357a48eb4 Mon Sep 17 00:00:00 2001 From: Ashis Kar Date: Fri, 28 Feb 2025 04:32:12 +0000 Subject: [PATCH 4/5] Fix typo in CHANGELOG regarding management storage access error and clean up error messages in mgmtstorage_enable_public_access.sh script --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0844895eb..f5628cd90 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,7 @@ ENHANCEMENTS: * Deny public access to TRE management storage account, and add private endpoint for TRE core [#4353](https://github.com/microsoft/AzureTRE/issues/4353) BUG FIXES: -* Fix the management storage access error while executing `make show-core-output` command, and remove redundand error messages from `mgmtstorage_enable_public_access.sh` script ([#4404](https://github.com/microsoft/AzureTRE/issues/4404)) +* Fix the management storage access error while executing `make show-core-output` command, and remove redundant error messages from `mgmtstorage_enable_public_access.sh` script ([#4404](https://github.com/microsoft/AzureTRE/issues/4404)) ## 0.21.0 From 8956946bd9f998d67f70ce92395b43a6947a45c2 Mon Sep 17 00:00:00 2001 From: Ashis Kar Date: Fri, 28 Feb 2025 04:38:51 +0000 Subject: [PATCH 5/5] Bump core version to 0.12.6 --- core/version.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/version.txt b/core/version.txt index 8e377d6b3..8e2394f4e 100644 --- a/core/version.txt +++ b/core/version.txt @@ -1 +1 @@ -__version__ = "0.12.5" +__version__ = "0.12.6"