Skip to content

refactoring to allow for slack error posting #24

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.

Sign up for GitHub

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

Merged
MartyEwings merged 1 commit into from
Jan 15, 2026
Merged

refactoring to allow for slack error posting #24

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
130 changes: 87 additions & 43 deletions files/metrics_import.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,55 +1,99 @@
#!/bin/bash
set -o pipefail
shopt -s nullglob

for f in /opt/mft-automations/puppet_enterprise_support*gz /opt/mft-automations/puppet_enterprise_support*gz.gpg /opt/mft-automations/puppet_enterprise_support*tar /opt/mft-automations/puppet_enterprise_support*tar.gz; do
# Does the file have a 5 digit ticket number after puppet_enterprise_support_
has_ticket=$(echo "$f" | grep -Eo -- 'puppet_enterprise_support_[[:digit:]]+_')

if [[ $(find "$f" -mmin -2 2>/dev/null) ]]; then
echo "INFO: $f is still downloading (mtime < 2 minutes). Skipping..."
continue
fi

if ! [[ $has_ticket ]]; then
echo "ERROR: no ticket ID found in $f"
mv "$f" /opt/mft-automations/err
continue
fi

if [[ "${f##*.}" == 'gpg' ]]; then
# Decrypt the file to the same location as the source, stripping the .gpg suffix
# Delete source file if it decrypts ok, otherwise move it to err/
if cat /root/.support_gpg | gpg --pinentry-mode loopback --passphrase-fd 0 --batch --yes --output "${f%.*}" --decrypt "$f"; then
rm -- "$f"
f="${f%.*}"
else
echo "ERROR: failed to decrypt $f"
mv "$f" /opt/mft-automations/err
continue
fi
fi

if ! tar tf "$f" | grep -q -m 1 'metrics\/.*json'; then
echo "No metrics found in $f. Skipping"
rm -- "$f"
continue
fi
# ---------------------------
# Config
# ---------------------------
SEND_SLACK=true # set to false to disable Slack posting
SLACK_WEBHOOK_URL="${SLACK_WEBHOOK_URL:-}" # must come from environment when SEND_SLACK=true

Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The SLACK_WEBHOOK_URL is read from an environment variable but there's no validation that it's set when SEND_SLACK is true. If SEND_SLACK is true but SLACK_WEBHOOK_URL is not set, slack_post will silently do nothing. Consider adding validation at startup to fail early with a clear error message if SEND_SLACK is true but SLACK_WEBHOOK_URL is empty.

Suggested change
# Validate Slack configuration early
if [[ "$SEND_SLACK" == "true" && -z "$SLACK_WEBHOOK_URL" ]]; then
echo "ERROR: SEND_SLACK is true but SLACK_WEBHOOK_URL is not set. Please export SLACK_WEBHOOK_URL or set SEND_SLACK=false." >&2
exit 1
fi

Copilot uses AI. Check for mistakes.
# ---------------------------
# Helpers
# ---------------------------
slack_post() {
# Usage: slack_post "message"
Comment on lines +14 to +15
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The slack_post function lacks documentation explaining its parameters, return behavior, and the fact that it silently returns success (0) when SEND_SLACK is false or SLACK_WEBHOOK_URL is empty. Adding a brief comment would improve maintainability.

Suggested change
slack_post() {
# Usage: slack_post "message"
slack_post() {
# Post a message to Slack.
# Usage: slack_post "message"
# Params:
# $1 - message text to send.
# Behavior / return:
# - If SEND_SLACK is not "true" or SLACK_WEBHOOK_URL is empty, the function
# returns success (exit code 0) without sending anything.
# - Network or curl errors are ignored so callers should not rely on this
# function's exit status to detect delivery failures.

Copilot uses AI. Check for mistakes.
local msg="$1"

# Only post if enabled and webhook present
[[ "$SEND_SLACK" == "true" ]] || return 0
[[ -n "$SLACK_WEBHOOK_URL" ]] || return 0

# Minimal JSON escaping for double-quotes, plus newlines -> \n
local esc="${msg//\\/\\\\}"
esc="${esc//\"/\\\"}"
esc="${esc//$'\n'/\\n}"
Comment on lines +22 to +25
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The JSON escaping implementation is incomplete and could lead to malformed JSON. The current implementation only escapes backslashes, double-quotes, and newlines, but does not handle other JSON control characters such as tabs, carriage returns, backspaces, and form feeds. These characters could break the JSON payload if present in error messages.

Suggested change
# Minimal JSON escaping for double-quotes, plus newlines -> \n
local esc="${msg//\\/\\\\}"
esc="${esc//\"/\\\"}"
esc="${esc//$'\n'/\\n}"
# Minimal JSON escaping for backslashes, double-quotes, and control chars
local esc="${msg//\\/\\\\}"
esc="${esc//\"/\\\"}"
esc="${esc//$'\n'/\\n}"
esc="${esc//$'\r'/\\r}"
esc="${esc//$'\t'/\\t}"
esc="${esc//$'\b'/\\b}"
esc="${esc//$'\f'/\\f}"

Copilot uses AI. Check for mistakes.

curl -sS -X POST -H 'Content-type: application/json' \
--data "{\"text\":\"$esc\"}" \
"$SLACK_WEBHOOK_URL" >/dev/null 2>&1 || true
Comment on lines +27 to +29
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The curl command's error output is suppressed with '2>&1 || true', which means failures to post to Slack will be silently ignored. While this prevents the script from failing when Slack is unavailable, it provides no feedback about posting failures, making it difficult to diagnose connectivity or webhook configuration issues.

Suggested change
curl -sS -X POST -H 'Content-type: application/json' \
--data "{\"text\":\"$esc\"}" \
"$SLACK_WEBHOOK_URL" >/dev/null 2>&1 || true
if ! curl -sS -X POST -H 'Content-type: application/json' \
--data "{\"text\":\"$esc\"}" \
"$SLACK_WEBHOOK_URL" >/dev/null; then
echo "WARN: failed to post message to Slack webhook" >&2
fi

Copilot uses AI. Check for mistakes.
}

# Strip the trailing _, then everything up to the last _ to get just the number
ticket="${has_ticket%_}"
ticket="${ticket##*_}"
log() {
# Usage: log "message"
local msg="$1"
echo "$msg"
slack_post "$msg"
}

_tmp="$(mktemp)"
/opt/puppetlabs/bolt/bin/bolt plan run puppet_operational_dashboards::load_metrics --targets localhost --run-as root influxdb_bucket="$ticket" support_script_file="$f" |& tee "$_tmp"
# ---------------------------
# Main
# ---------------------------
for f in /opt/mft-automations/puppet_enterprise_support*gz \
/opt/mft-automations/puppet_enterprise_support*gz.gpg \
/opt/mft-automations/puppet_enterprise_support*tar \
/opt/mft-automations/puppet_enterprise_support*tar.gz; do

if ! grep -q 'Wrote batch of [[:digit:]]* metrics' "$_tmp"; then
echo "ERROR: failed to import metrics from $f"
# Does the file have a 5 digit ticket number after puppet_enterprise_support_
has_ticket=$(echo "$f" | grep -Eo -- 'puppet_enterprise_support_[[:digit:]]+_')

if [[ $(find "$f" -mmin -2 2>/dev/null) ]]; then
log "INFO: $f is still downloading (mtime < 2 minutes). Skipping..."
continue
fi

if ! [[ $has_ticket ]]; then
log "ERROR: no ticket ID found in $f"
mv "$f" /opt/mft-automations/err
continue
fi

if [[ "${f##*.}" == 'gpg' ]]; then
# Decrypt the file to the same location as the source, stripping the .gpg suffix
# Delete source file if it decrypts ok, otherwise move it to err/
if cat /root/.support_gpg | gpg --pinentry-mode loopback --passphrase-fd 0 --batch --yes --output "${f%.*}" --decrypt "$f"; then
rm -- "$f"
f="${f%.*}"
else
log "ERROR: failed to decrypt $f"
mv "$f" /opt/mft-automations/err
continue
fi
fi
fi

if ! tar tf "$f" | grep -q -m 1 'metrics\/.*json'; then
log "No metrics found in $f. Skipping"
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not all error paths are using the new log() function. The "No metrics found" message at line 75 is informational rather than an error, but other error conditions use log() with "ERROR:" prefix. For consistency, consider whether this message should also be treated as an error or if the current behavior is intentional.

Suggested change
log "No metrics found in $f. Skipping"
log "INFO: No metrics found in $f. Skipping"

Copilot uses AI. Check for mistakes.
rm -- "$f"
continue
fi

# Strip the trailing _, then everything up to the last _ to get just the number
ticket="${has_ticket%_}"
ticket="${ticket##*_}"

_tmp="$(mktemp)"
/opt/puppetlabs/bolt/bin/bolt plan run puppet_operational_dashboards::load_metrics \
--targets localhost --run-as root influxdb_bucket="$ticket" support_script_file="$f" \
|& tee "$_tmp"

if ! grep -q 'Wrote batch of [[:digit:]]* metrics' "$_tmp"; then
log "ERROR: failed to import metrics from $f"
mv "$f" /opt/mft-automations/err
continue
fi

rm -- "$_tmp"
rm -- "$f"
rm -- "$_tmp"
rm -- "$f"
done

# Delete sup scripts older than 30 days
Expand Down