From 3e20b558a57fec9fea53b17abbc0cc71da50212c Mon Sep 17 00:00:00 2001 From: hermes Date: Mon, 9 Nov 2026 12:15:50 +1000 Subject: [PATCH] fix(ci): harden the CA refresh script and add shellcheck to CI Round-2 review findings, all low severity. - `curl` now has `--connect-timeout` and `--max-time`, so an unreachable API server fails the job instead of hanging the runner indefinitely. - A guard for a kubeconfig with no cluster entries, or an empty one, with a clear message rather than an opaque kubectl error. - The fingerprint mismatch message now prints both the expected and the actual value, which cuts the time to diagnose the next CA rotation. - `set -euo pipefail` was already there; the script now passes shellcheck clean, and a shellcheck step was added to the CI test job so the operational scripts get the same treatment as the Python code. --- .gitea/workflows/build_push.yml | 10 ++++++++++ scripts/kube-ca-refresh.sh | 23 +++++++++++++++++++++-- 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/.gitea/workflows/build_push.yml b/.gitea/workflows/build_push.yml index d013b01..8adc926 100644 --- a/.gitea/workflows/build_push.yml +++ b/.gitea/workflows/build_push.yml @@ -28,6 +28,16 @@ jobs: . .venv/bin/activate python -m pytest tests/ -q + - name: Shellcheck the scripts + run: | + # The operational scripts are shell, so they get the same treatment + # as the Python code. + if ! command -v shellcheck >/dev/null 2>&1; then + apt-get update -qq + apt-get install -y -qq shellcheck + fi + shellcheck scripts/*.sh + # NOTE: the build job deliberately does NOT have a kubeconfig. The # `KUBEC_CONFIG_BUILDX` secret is cluster-admin-equivalent, and a build step # that has no need of the cluster should not be able to read it. The deploy diff --git a/scripts/kube-ca-refresh.sh b/scripts/kube-ca-refresh.sh index 73a6b09..f80117b 100755 --- a/scripts/kube-ca-refresh.sh +++ b/scripts/kube-ca-refresh.sh @@ -55,16 +55,33 @@ if [ -z "$EXPECTED" ]; then exit 1 fi +# Guard against a kubeconfig with no clusters, or an empty one. +CLUSTER_COUNT=$(kubectl config view --raw -o jsonpath='{.clusters}' 2>/dev/null \ + | grep -c '"name"' || true) +if [ "${CLUSTER_COUNT:-0}" -eq 0 ]; then + echo "ERROR: the kubeconfig has no cluster entries." >&2 + echo " The KUBEC_CONFIG_BUILDX_NEW_2 secret looks wrong or empty." >&2 + exit 1 +fi + SERVER=$(kubectl config view --raw -o jsonpath='{.clusters[0].cluster.server}') # The cluster entry's NAME - patching the CA into a *new* entry named # "default" would leave the original entry (and its stale CA) in use. CLUSTER_NAME=$(kubectl config view --raw -o jsonpath='{.clusters[0].name}') + +if [ -z "$SERVER" ] || [ -z "$CLUSTER_NAME" ]; then + echo "ERROR: could not read the server or cluster name from the kubeconfig." >&2 + exit 1 +fi + echo "server: $SERVER cluster entry: $CLUSTER_NAME" CA_FILE=$(mktemp) trap 'rm -f "$CA_FILE"' EXIT -curl -sk "$SERVER/cacerts" -o "$CA_FILE" +# Timeouts so an unreachable API server fails the job rather than hanging +# the runner indefinitely. +curl -sk --connect-timeout 10 --max-time 30 "$SERVER/cacerts" -o "$CA_FILE" if [ ! -s "$CA_FILE" ]; then echo "ERROR: could not fetch the cluster CA from $SERVER/cacerts" >&2 exit 1 @@ -73,7 +90,7 @@ fi # Normalise both to lowercase hex without separators, so the comparison does # not depend on the colon-separated format openssl prints. normalise() { - tr -d ':' | tr 'A-Z' 'a-z' + tr -d ':' | tr '[:upper:]' '[:lower:]' } ACTUAL=$(openssl x509 -in "$CA_FILE" -noout -fingerprint -sha256 | cut -d= -f2 | normalise) @@ -84,6 +101,8 @@ echo "actual CA fingerprint: $ACTUAL" if [ "$ACTUAL" != "$WANTED" ]; then echo "ERROR: the cluster CA does not match the pinned fingerprint." >&2 + echo " expected: $WANTED" >&2 + echo " actual: $ACTUAL" >&2 echo " Refusing to use it. Either the CA has been rotated and the" >&2 echo " pinned fingerprint needs updating, or the connection was" >&2 echo " intercepted." >&2