chore(azure): add diagnostics-clean script and enhance diagnostics.sh… #457
Conversation
… with role checks
There was a problem hiding this comment.
Pull Request Overview
This PR enhances Azure diagnostics capabilities by adding a new cleanup script and improving role permission checking in the existing diagnostics script.
- Added a new
diagnostics-clean.shscript for cleaning up diagnostic data from both local and Azure environments - Enhanced
diagnostics.shwith more comprehensive Azure Service Bus role permission checking across different scopes - Fixed a syntax error in
dependencies.shand added an example environment variable comment
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
examples/azure/local-deploy.sh |
Added commented example for TOPICS environment variable configuration |
examples/azure/diagnostics.sh |
Enhanced Azure Service Bus permission checking with multi-scope role validation |
examples/azure/diagnostics-clean.sh |
New script for cleaning up diagnostic data from local and Azure environments |
examples/azure/dependencies.sh |
Fixed missing line continuation backslash in Redis configuration |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
| ;; | ||
| *) | ||
| shift | ||
| ;; |
There was a problem hiding this comment.
The wildcard case in the argument parser silently ignores unknown arguments without providing user feedback. Consider adding an echo statement to warn users about unrecognized arguments or exit with an error for better user experience.
| ;; | |
| echo "❌ Unknown argument: $1" >&2 | |
| exit 1 |
Copilot uses AI. Check for mistakes.
| local base_url=$1 | ||
| local env_name=$2 | ||
| echo "🧹 Cleaning up $env_name environment at $base_url..." | ||
| TENANT_ID="diagnostics-tenant-x" |
There was a problem hiding this comment.
The tenant ID is hardcoded as a magic string. Consider making this configurable via an environment variable or function parameter to improve flexibility and maintainability.
| TENANT_ID="diagnostics-tenant-x" |
Copilot uses AI. Check for mistakes.
|
|
||
| echo " (Fetching destinations for tenant: $TENANT_ID...)" | ||
| DESTINATION_IDS=$(curl -sf -X GET "$base_url/api/v1/$TENANT_ID/destinations" \ | ||
| -H "Authorization: Bearer $API_KEY" | jq -r '.[].id') |
There was a problem hiding this comment.
The curl command uses the -s flag which suppresses error output, making debugging difficult if the API call fails. Consider providing more descriptive error handling or logging when the API request fails.
| -H "Authorization: Bearer $API_KEY" | jq -r '.[].id') | |
| CURL_OUTPUT=$(mktemp) | |
| if ! curl -f -X GET "$base_url/api/v1/$TENANT_ID/destinations" \ | |
| -H "Authorization: Bearer $API_KEY" >"$CURL_OUTPUT" 2>"$CURL_OUTPUT.err"; then | |
| echo " -> ❌ Failed to fetch destinations for tenant $TENANT_ID." | |
| echo " curl error output:" | |
| cat "$CURL_OUTPUT.err" | |
| rm -f "$CURL_OUTPUT" "$CURL_OUTPUT.err" | |
| return 1 | |
| fi | |
| DESTINATION_IDS=$(jq -r '.[].id' <"$CURL_OUTPUT") | |
| rm -f "$CURL_OUTPUT" "$CURL_OUTPUT.err" |
Copilot uses AI. Check for mistakes.
… with role checks