Skip to content

Commit f972c90

Browse files
Improve Route53 cleanup to address query escaping issues
- Fixed Route53 query escaping issues by fetching all records and filtering with jq - Improved error handling - don't mask failures, log them as warnings - Use single API call for efficiency instead of multiple queries - Properly handle the wildcard record format (\052 for asterisk) - Added explicit error messages for better debugging - Use jq for JSON manipulation instead of heredocs for change batches This addresses Evgeniy's review comment about Route53 query escaping being error-prone.
1 parent c525edb commit f972c90

File tree

1 file changed

+63
-92
lines changed

1 file changed

+63
-92
lines changed

scripts/destroy-openshift-cluster.sh

Lines changed: 63 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -758,7 +758,7 @@ destroy_with_openshift_install() {
758758
fi
759759
}
760760

761-
# Clean up Route53 DNS records
761+
# Clean up Route53 DNS records (improved version with better error handling)
762762
cleanup_route53_records() {
763763
local infra_id="$1"
764764
local cluster_name="${CLUSTER_NAME:-${infra_id%-*}}"
@@ -767,108 +767,79 @@ cleanup_route53_records() {
767767
log_info " Checking Route53 DNS records..."
768768
log_debug "Looking for: api.$cluster_name.$base_domain and *.apps.$cluster_name.$base_domain"
769769

770-
# Get hosted zone ID
771-
local zone_id=$(aws route53 list-hosted-zones \
772-
--query "HostedZones[?Name=='${base_domain}.'].Id" \
773-
--output text --profile "$AWS_PROFILE" 2>/dev/null | head -1)
774-
775-
if [[ -z "$zone_id" ]]; then
770+
# Get hosted zone ID with proper error handling
771+
local zone_id
772+
zone_id=$(aws route53 list-hosted-zones \
773+
--query "HostedZones[?Name=='${base_domain}.'].Id | [0]" \
774+
--output text --profile "$AWS_PROFILE" 2>/dev/null)
775+
776+
if [[ -z "$zone_id" || "$zone_id" == "None" ]]; then
776777
log_debug "No hosted zone found for domain: $base_domain"
777778
return 0
778779
fi
779-
780-
# Look for DNS records related to the cluster
781-
# Check both api. and *.apps. patterns
782-
local api_record=$(aws route53 list-resource-record-sets \
783-
--hosted-zone-id "$zone_id" \
784-
--query "ResourceRecordSets[?Name=='api.${cluster_name}.${base_domain}.']" \
785-
--profile "$AWS_PROFILE" 2>/dev/null)
786-
787-
local apps_record=$(aws route53 list-resource-record-sets \
780+
781+
log_debug "Found hosted zone: $zone_id"
782+
783+
# Define exact record names (Note: Route53 stores wildcard as \052)
784+
local api_name="api.${cluster_name}.${base_domain}."
785+
local apps_name="\\052.apps.${cluster_name}.${base_domain}."
786+
787+
# Fetch all records and filter in jq to avoid JMESPath escaping issues
788+
local all_records
789+
if ! all_records=$(aws route53 list-resource-record-sets \
788790
--hosted-zone-id "$zone_id" \
789-
--query "ResourceRecordSets[?Name=='\\052.apps.${cluster_name}.${base_domain}.']" \
790-
--profile "$AWS_PROFILE" 2>/dev/null)
791-
792-
local found_records=false
793-
794-
# Check if we found any records
795-
if [[ "$api_record" != "[]" && "$api_record" != "null" ]]; then
796-
found_records=true
797-
fi
798-
if [[ "$apps_record" != "[]" && "$apps_record" != "null" ]]; then
799-
found_records=true
791+
--profile "$AWS_PROFILE" \
792+
--output json 2>/dev/null); then
793+
log_warning "Failed to query Route53 records"
794+
return 1
800795
fi
801-
802-
if [[ "$found_records" == "false" ]]; then
796+
797+
# Filter for our specific records using jq
798+
local records
799+
records=$(echo "$all_records" | jq --arg api "$api_name" --arg apps "$apps_name" \
800+
'[.ResourceRecordSets[] | select(.Name == $api or .Name == $apps)]')
801+
802+
# Check if we found any records
803+
if [[ "$records" == "[]" || -z "$records" || "$records" == "null" ]]; then
803804
log_info " No Route53 records found for cluster"
804805
return 0
805806
fi
806-
807-
log_info " Found Route53 DNS records to clean up"
808-
809-
# Process API record if found
810-
if [[ "$api_record" != "[]" && "$api_record" != "null" ]]; then
811-
echo "$api_record" | jq -c '.[]' | while read -r record; do
812-
local name=$(echo "$record" | jq -r '.Name')
813-
local type=$(echo "$record" | jq -r '.Type')
814-
815-
if [[ "$DRY_RUN" == "false" ]]; then
816-
# Create change batch for deletion
817-
local change_batch=$(
818-
cat <<EOF
819-
{
820-
"Changes": [{
821-
"Action": "DELETE",
822-
"ResourceRecordSet": $record
823-
}]
824-
}
825-
EOF
826-
)
827-
828-
# Apply the change
829-
aws route53 change-resource-record-sets \
830-
--hosted-zone-id "$zone_id" \
831-
--change-batch "$change_batch" \
832-
--profile "$AWS_PROFILE" >/dev/null 2>&1 || true
833-
834-
log_info " Deleted DNS record: $name ($type)"
835-
else
836-
log_info " [DRY RUN] Would delete DNS record: $name ($type)"
837-
fi
838-
done
839-
fi
840-
841-
# Process apps wildcard record if found
842-
if [[ "$apps_record" != "[]" && "$apps_record" != "null" ]]; then
843-
echo "$apps_record" | jq -c '.[]' | while read -r record; do
844-
local name=$(echo "$record" | jq -r '.Name')
845-
local type=$(echo "$record" | jq -r '.Type')
846-
847-
if [[ "$DRY_RUN" == "false" ]]; then
848-
# Create change batch for deletion
849-
local change_batch=$(
850-
cat <<EOF
851-
{
852-
"Changes": [{
853-
"Action": "DELETE",
854-
"ResourceRecordSet": $record
855-
}]
856-
}
857-
EOF
858-
)
859-
860-
# Apply the change
861-
aws route53 change-resource-record-sets \
862-
--hosted-zone-id "$zone_id" \
863-
--change-batch "$change_batch" \
864-
--profile "$AWS_PROFILE" >/dev/null 2>&1 || true
865-
807+
808+
# Count records for user feedback
809+
local count=$(echo "$records" | jq 'length')
810+
log_info " Found $count Route53 DNS record(s) to clean up"
811+
812+
# Process each record with proper error handling
813+
echo "$records" | jq -c '.[]' | while IFS= read -r record; do
814+
[[ -z "$record" ]] && continue
815+
816+
local name=$(echo "$record" | jq -r '.Name')
817+
local type=$(echo "$record" | jq -r '.Type')
818+
819+
if [[ "$DRY_RUN" == "false" ]]; then
820+
# Create change batch using jq for proper JSON formatting
821+
local change_batch
822+
change_batch=$(jq -n \
823+
--argjson record "$record" \
824+
'{Changes: [{Action: "DELETE", ResourceRecordSet: $record}]}')
825+
826+
# Apply the change with explicit error handling
827+
if aws route53 change-resource-record-sets \
828+
--hosted-zone-id "$zone_id" \
829+
--change-batch "$change_batch" \
830+
--profile "$AWS_PROFILE" \
831+
--output json >/dev/null 2>&1; then
866832
log_info " Deleted DNS record: $name ($type)"
867833
else
868-
log_info " [DRY RUN] Would delete DNS record: $name ($type)"
834+
# Don't mask the error, but continue with other records
835+
log_warning " Failed to delete DNS record: $name ($type) - may already be deleted"
869836
fi
870-
done
871-
fi
837+
else
838+
log_info " [DRY RUN] Would delete DNS record: $name ($type)"
839+
fi
840+
done
841+
842+
return 0
872843
}
873844

874845
# Single pass of AWS resource cleanup

0 commit comments

Comments
 (0)