Skip to content

Commit

Permalink
Fix: make show-core-output is failing due to lack of access to manage…
Browse files Browse the repository at this point in the history
…ment storage account (#4407)

* Refactor Makefile to use show_output.sh for displaying Terraform output and improve error handling in mgmtstorage scripts.

* CHANGELOG file update with bug fix details.

* Update CHANGELOG to clarify management storage access error fix in `make show-core-output`

* Fix typo in CHANGELOG regarding management storage access error and clean up error messages in mgmtstorage_enable_public_access.sh script

* Bump core version to 0.12.6

---------

Co-authored-by: Ashis Kar <[email protected]>
  • Loading branch information
ashis-kar91 and Ashis Kar authored Feb 28, 2025
1 parent 0ec539a commit 3fcbdf9
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 19 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 redundant error messages from `mgmtstorage_enable_public_access.sh` script ([#4404](https://github.com/microsoft/AzureTRE/issues/4404))

## 0.21.0

Expand Down
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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") \
Expand Down
12 changes: 12 additions & 0 deletions core/terraform/show_output.sh
Original file line number Diff line number Diff line change
@@ -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"
2 changes: 1 addition & 1 deletion core/version.txt
Original file line number Diff line number Diff line change
@@ -1 +1 @@
__version__ = "0.12.5"
__version__ = "0.12.6"
28 changes: 12 additions & 16 deletions devops/scripts/mgmtstorage_enable_public_access.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Expand All @@ -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
}

Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 3fcbdf9

Please sign in to comment.