-
Notifications
You must be signed in to change notification settings - Fork 158
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: storage account 403 error when creating new tre env #4406
base: main
Are you sure you want to change the base?
Conversation
…ntributor" and "Storage Account Contributor" roles
Unit Test Results0 tests 0 ✅ 0s ⏱️ Results for commit 4c26cde. ♻️ This comment has been updated with latest results. |
…tor in bootstrap.sh
/test |
🤖 pr-bot 🤖 🏃 Running tests: https://github.com/microsoft/AzureTRE/actions/runs/13545863577 (with refid (in response to this comment from @ShakutaiGit) |
/test |
🤖 pr-bot 🤖 🏃 Running tests: https://github.com/microsoft/AzureTRE/actions/runs/13547308284 (with refid (in response to this comment from @ShakutaiGit) |
…mline script execution
…enhance storage container creation with retry logic
/test |
🤖 pr-bot 🤖 🏃 Running tests: https://github.com/microsoft/AzureTRE/actions/runs/13553941503 (with refid (in response to this comment from @ShakutaiGit) |
/test |
🤖 pr-bot 🤖 🏃 Running tests: https://github.com/microsoft/AzureTRE/actions/runs/13559362952 (with refid (in response to this comment from @ShakutaiGit) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple of comments. Thanks for looking into making this more robust.
az role assignment list --assignee "$USER_OBJECT_ID" --role "Storage Blob Data Contributor" --scope "/subscriptions/$ARM_SUBSCRIPTION_ID/resourceGroups/$TF_VAR_mgmt_resource_group_name/providers/Microsoft.Storage/storageAccounts/$TF_VAR_mgmt_storage_account_name" --query "[].id" --output tsv | ||
check_role_assignments() { | ||
local sbdc | ||
sbdc=$(az role assignment list \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion from copilot, not tested, but makes sense to have a timeout of some sort...
# Function to check if the role assignments exist
check_role_assignments() {
local roles
roles=$(az role assignment list \
--assignee "$USER_OBJECT_ID" \
--scope "/subscriptions/$ARM_SUBSCRIPTION_ID/resourceGroups/$TF_VAR_mgmt_resource_group_name/providers/Microsoft.Storage/storageAccounts/$TF_VAR_mgmt_storage_account_name" \
--query "[?roleDefinitionName=='Storage Blob Data Contributor' || roleDefinitionName=='Storage Account Contributor'].roleDefinitionName" --output tsv)
if [[ $roles == *"Storage Blob Data Contributor"* && $roles == *"Storage Account Contributor"* ]]; then
echo "both"
fi
}
# Wait for the role assignment to be applied with a timeout
echo -e "\n\e[34m»»» ⏳ \e[96mWaiting for role assignment to be applied\e[0m..."
timeout=300 # 5 minutes timeout
start_time=$(date +%s)
while [ -z "$(check_role_assignments)" ]; do
echo "Waiting for role assignment..."
sleep 10
current_time=$(date +%s)
elapsed_time=$((current_time - start_time))
if [ $elapsed_time -ge $timeout ]; then
echo "ERROR: Timeout waiting for role assignments."
exit 1
fi
done
echo "Role assignment applied."
echo "Waiting for role assignment..." | ||
sleep 10 | ||
done | ||
echo "Role assignment applied." | ||
|
||
sleep 30 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need to sleep if we are retrying below?
@@ -38,29 +38,67 @@ if [ -n "${ARM_CLIENT_ID:-}" ]; then | |||
else | |||
USER_OBJECT_ID=$(az ad signed-in-user show --query id --output tsv) | |||
fi | |||
|
|||
az role assignment create --assignee "$USER_OBJECT_ID" \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you work out why this is required? I thought the object ID that creates the storage account should have sufficient permissions?
Resolves #4405
What is being addressed
Testing: