ci: make Icinga deploy snapshots mandatory

[Svaag]

Summary

• make deploy-bracket Icinga snapshots create their controller output directories before writing
• allow the trusted runner's ICINGA_API_USER / ICINGA_API_PASSWORD to satisfy snapshot credentials when mon's helper env file is missing
• make production apply fail if pre/post snapshot directories or uploaded artifact files are missing

Why

Fixes #311. The #312 deploy still produced no snapshot artifact because the snapshot copy task did not create ansible/generated/snapshots/<phase>/, and mon currently lacks /etc/icinga2/scripts/.icinga-snapshot.env.

Validation

• scripts/ci/test-snapshot-bracket.sh
• scripts/ci/render-all.sh

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis ✅ 311 - PR Code Verified Compliant requirements: Wired the snapshot step to ICINGA_API_USER/ICINGA_API_PASSWORD workflow secrets as fallback (inline curl in snapshot.yml + updated icinga-snapshot script). Added task to ensure snapshot directory exists on the controller before writing. Changed snapshot diff step to error and exit 1 on missing directories. Added if-no-files-found: error to the artifact upload step. Requires further human verification: Re-verification on the next loop apply (cannot be assessed from code).

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🏅 Score: 85

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Dry-run regression The pre-deploy Icinga snapshot step now runs with snapshot_required=true for all invocations, including dry-run workflow runs. The old code degraded gracefully (warning + empty diff). If the Icinga snapshot infrastructure is missing or unreachable, a dry-run workflow will now fail before reaching the render-only check, blocking the user from validating configuration changes in a low-risk setting. This is a behavioural change that may surprise operators. The dry-run condition should either skip the snapshot step or set snapshot_required=false when the workflow is a dry-run. env: PLAYBOOK: ${{ inputs.playbook }} run: | ansible-playbook "playbooks/${PLAYBOOK}.yml" \ --tags snapshot \ -e ansible_user=ci \ -e snapshot_required=true \ -e snapshot_phase=pre

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 695239ea92

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[github-actions]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Validate environment variables are non-empty The parameter expansion : "${var:?message}" only triggers an error if the variable is unset, not if it is set to an empty string. If Ansible's env lookup returns an empty string (common when the environment variable is not defined), the script will proceed with empty credentials, leading to a cryptic API failure. Replace the checks with explicit empty string tests to provide a clearer error and fail fast. ansible/roles/firewall/tasks/snapshot.yml [36-37] -: "${ICINGA_API_USER:?ICINGA_API_USER unset and /etc/icinga2/scripts/.icinga-snapshot.env missing}" -: "${ICINGA_API_PASS:?ICINGA_API_PASS unset and /etc/icinga2/scripts/.icinga-snapshot.env missing}" +if [ -z "${ICINGA_API_USER:-}" ]; then + echo "ICINGA_API_USER is empty or unset" >&2 + exit 2 +fi +if [ -z "${ICINGA_API_PASS:-}" ]; then + echo "ICINGA_API_PASS is empty or unset" >&2 + exit 2 +fi Suggestion importance[1-10]: 6 __ Why: The suggestion correctly identifies that : "${var:?message}" only errors on unset variables, not empty ones, which could lead to silent credential failures. Replacing with explicit empty-string checks improves error clarity and fails fast.	Low

[Svaag]

Addressed automated review feedback in 2c2287d:

• fallback snapshot path now uses bash pipefail and explicit non-empty credential checks
• helper script now fails jq parse errors before sorting
• dry-runs skip mandatory snapshots
• unbracketed playbooks skip snapshot diff/artifact enforcement

Local validation: git diff --check, scripts/ci/test-snapshot-bracket.sh, scripts/ci/render-all.sh, sh -n configs/mon/icinga2/scripts/icinga-snapshot.

@codex review

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. What shall we delve into next?

Reviewed commit: 2c2287dd4c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".