Skip to content
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

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading