From e2d9e28254bb2dbb44327400269336dc66119e62 Mon Sep 17 00:00:00 2001 From: Michael Welles Date: Tue, 21 Jul 2026 11:30:02 -0400 Subject: [PATCH] feat(dgraph): harden backup login and expose backup ports on the headless Service Declare http (8080) and grpc (9080) on the alpha headless Service. The backup CronJobs reach a pinned alpha-0 through this Service; under a STRICT mTLS mesh the client sidecar only builds an mTLS route for ports the Service declares, so without them the /admin login falls through to plaintext and the sidecar resets it. Gate both ports on backups.full.enabled or backups.incremental.enabled, since their only consumer is the backup CronJobs. Guard each backup.sh curl with require_json: capture the curl exit status errexit-safely and validate the body is JSON before parsing. A mesh or network failure returns a plaintext body (e.g. an Envoy connect error) that now surfaces as a clear curl-status / URL / body message instead of a cryptic parse failure or a silently empty token. Convert the login bodies and response parsing to jq: get_token_rest and get_token_graphql build their request bodies with 'jq -n --arg' instead of hand-interpolating user/password into a JSON string, so credentials containing quotes or other JSON-breaking characters can no longer corrupt or inject into the request. All four functions parse responses with 'jq -r ... // empty' instead of grep -oP/grep -q errors, removing the dependency on grep's PCRE mode. backup_graphql's destination stays hand-interpolated since it is operator-controlled, not user credentials. Reset HEADERS and CERTOPTS at the top of get_token and backup so a second call in the same shell does not accumulate headers left over from an earlier call. Quote the get_token arguments in the full and incremental CronJobs so an admin user or password containing whitespace is passed as a single argument. --- .../dgraph/templates/alpha/svc-headless.yaml | 14 ++ charts/dgraph/templates/backups/configs.yaml | 120 ++++++++++-------- .../templates/backups/cronjob-full.yaml | 2 +- .../dgraph/templates/backups/cronjob-inc.yaml | 2 +- 4 files changed, 85 insertions(+), 53 deletions(-) diff --git a/charts/dgraph/templates/alpha/svc-headless.yaml b/charts/dgraph/templates/alpha/svc-headless.yaml index 00422a63c..cbbbc4e86 100644 --- a/charts/dgraph/templates/alpha/svc-headless.yaml +++ b/charts/dgraph/templates/alpha/svc-headless.yaml @@ -12,6 +12,20 @@ spec: - name: grpc-alpha-int port: 7080 targetPort: 7080 + ## http-alpha (8080) and grpc-alpha (9080) are exposed here only when backups are + ## enabled (backups.full.enabled or backups.incremental.enabled): the backup CronJobs + ## are their consumer, addressing a single pinned pod (alpha-0) through this headless + ## Service, and under a STRICT mTLS service mesh (e.g. Istio) the client sidecar only + ## builds an mTLS route for ports the Service declares. Without 8080 here, the backup + ## /admin login falls through to plaintext and Alpha's sidecar resets the connection. + {{- if or .Values.backups.full.enabled .Values.backups.incremental.enabled }} + - name: http-alpha + port: 8080 + targetPort: 8080 + - name: grpc-alpha + port: 9080 + targetPort: 9080 + {{- end }} selector: app: {{ template "dgraph.name" . }} component: {{ .Values.alpha.name }} diff --git a/charts/dgraph/templates/backups/configs.yaml b/charts/dgraph/templates/backups/configs.yaml index 6a1d693db..413b9f867 100644 --- a/charts/dgraph/templates/backups/configs.yaml +++ b/charts/dgraph/templates/backups/configs.yaml @@ -9,51 +9,69 @@ metadata: data: backup.sh: | ###### - # get_token_rest - get accessJWT token with REST command for Dgraph 1.x + # require_json - validate a curl response before parsing it as JSON + # params: + # 1: body - the response body to validate + # 2: curl_status - exit status of the curl that produced the body + # 3: url - request URL, for the error message + # A failed connection through a service mesh (or any network error) returns a + # non-JSON body, e.g. Envoy's "upstream connect error ...", which the field + # extraction below cannot parse. Surface the curl status, the URL, and the raw + # body instead of failing silently with an empty token. Only bodies that fail + # validation are echoed, so a successful login response is never printed. ########################## - get_token_rest() { - JSON="{\"userid\": \"${USER}\", \"password\": \"${PASSWORD}\" }" - RESULT=$( - /usr/bin/curl --silent \ - "${HEADERS[@]}" \ - "${CERTOPTS[@]}" \ - --request POST \ - ${ALPHA_HOST}:8080/login \ - --data "${JSON}" - ) - - if grep -q errors <<< "$RESULT"; then - ERROR=$(grep -oP '(?<=message":")[^"]*' <<< $RESULT) - echo "ERROR: $ERROR" + require_json() { + BODY=${1} + CURL_STATUS=${2} + URL=${3} + if [[ "$CURL_STATUS" -ne 0 ]]; then + echo "ERROR: curl to ${URL} failed with exit ${CURL_STATUS}: ${BODY}" >&2 return 1 fi + if [[ -z "$BODY" ]]; then + echo "ERROR: empty response from ${URL}" >&2 + return 1 + fi + if ! jq -e . >/dev/null 2>&1 <<< "$BODY"; then + echo "ERROR: non-JSON response from ${URL}: ${BODY}" >&2 + return 1 + fi + } - grep -oP '(?<=accessJWT":")[^"]*' <<< "$RESULT" - + ###### + # get_token_rest - get accessJWT token with REST command for Dgraph 1.x + ########################## + get_token_rest() { + JSON=$(jq -n --arg userid "$USER" --arg password "$PASSWORD" '{userid: $userid, password: $password}') + CURL_STATUS=0 + RESULT=$( + /usr/bin/curl --silent "${HEADERS[@]}" "${CERTOPTS[@]}" \ + --request POST ${ALPHA_HOST}:8080/login --data "${JSON}" + ) || CURL_STATUS=$? + require_json "$RESULT" "$CURL_STATUS" "${ALPHA_HOST}:8080/login" || return 1 + ERROR=$(jq -r '.errors[0].message // empty' <<< "$RESULT") + if [[ -n "$ERROR" ]]; then echo "ERROR: $ERROR"; return 1; fi + TOKEN=$(jq -r '.data.accessJWT // empty' <<< "$RESULT") + [[ -n "$TOKEN" ]] || { echo "ERROR: could not parse accessJWT from /login response" >&2; return 1; } + echo "$TOKEN" } ###### # get_token_graphql - get accessJWT token using GraphQL for Dgraph 20.03.1+ ########################## get_token_graphql() { - GQL="{\"query\": \"mutation { login(userId: \\\"${USER}\\\" password: \\\"${PASSWORD}\\\") { response { accessJWT } } }\"}" + GQL=$(jq -n --arg userId "$USER" --arg password "$PASSWORD" '{query: "mutation ($userId: String, $password: String) { login(userId: $userId, password: $password) { response { accessJWT } } }", variables: {userId: $userId, password: $password}}') + CURL_STATUS=0 RESULT=$( - /usr/bin/curl --silent \ - "${HEADERS[@]}" \ - "${CERTOPTS[@]}" \ - --request POST \ - ${ALPHA_HOST}:8080/admin \ - --data "${GQL}" - ) - - if grep -q errors <<< "$RESULT"; then - ERROR=$(grep -oP '(?<=message":")[^"]*' <<< $RESULT) - echo "ERROR: $ERROR" - return 1 - fi - - grep -oP '(?<=accessJWT":")[^"]*' <<< "$RESULT" - + /usr/bin/curl --silent "${HEADERS[@]}" "${CERTOPTS[@]}" \ + --request POST ${ALPHA_HOST}:8080/admin --data "${GQL}" + ) || CURL_STATUS=$? + require_json "$RESULT" "$CURL_STATUS" "${ALPHA_HOST}:8080/admin" || return 1 + ERROR=$(jq -r '.errors[0].message // empty' <<< "$RESULT") + if [[ -n "$ERROR" ]]; then echo "ERROR: $ERROR"; return 1; fi + TOKEN=$(jq -r '.data.login.response.accessJWT // empty' <<< "$RESULT") + [[ -n "$TOKEN" ]] || { echo "ERROR: could not parse accessJWT from /admin login response" >&2; return 1; } + echo "$TOKEN" } ###### @@ -74,6 +92,8 @@ data: CACERT_PATH=${CACERT_PATH:-""} CLIENT_CERT_PATH=${CLIENT_CERT_PATH:-""} CLIENT_KEY_PATH=${CLIENT_KEY_PATH:-""} + HEADERS=() + CERTOPTS=() ## user/password required for login if [[ -z "$USER" || -z "$PASSWORD" ]]; then @@ -125,6 +145,8 @@ data: CACERT_PATH=${CACERT_PATH:-""} CLIENT_CERT_PATH=${CLIENT_CERT_PATH:-""} CLIENT_KEY_PATH=${CLIENT_KEY_PATH:-""} + HEADERS=() + CERTOPTS=() API_TYPE=${API_TYPE:-"graphql"} @@ -188,24 +210,22 @@ data: backup_rest() { URL_PATH="admin/backup?force_full=$FORCE_FULL" + CURL_STATUS=0 RESULT=$(/usr/bin/curl --silent \ "${HEADERS[@]}" \ "${CERTOPTS[@]}" \ --request POST \ ${ALPHA_HOST}:8080/$URL_PATH \ --data "destination=$BACKUP_DESTINATION" - ) - - if grep -q errors <<< "$RESULT"; then - ERROR=$(grep -oP '(?<=message":")[^"]*' <<< $RESULT) - MESSAGE="ERROR: $ERROR" - if grep -q code <<< "$RESULT"; then - CODE=$(grep -oP '(?<=code":")[^"]*' <<< $RESULT) - echo "$MESSAGE REASON='$CODE'" - fi + ) || CURL_STATUS=$? + require_json "$RESULT" "$CURL_STATUS" "${ALPHA_HOST}:8080/${URL_PATH}" || return 1 + + ERROR=$(jq -r '.errors[0].message // empty' <<< "$RESULT") + if [[ -n "$ERROR" ]]; then + CODE=$(jq -r '.errors[0].code // empty' <<< "$RESULT") + if [[ -n "$CODE" ]]; then echo "ERROR: $ERROR REASON='$CODE'"; else echo "ERROR: $ERROR"; fi return 1 fi - echo $RESULT } @@ -216,20 +236,18 @@ data: backup_graphql() { GQL="{\"query\": \"mutation { backup(input: {destination: \\\"${BACKUP_DESTINATION}\\\" forceFull: $FORCE_FULL }) { response { message code } } }\"}" + CURL_STATUS=0 RESULT=$(/usr/bin/curl --silent \ "${HEADERS[@]}" \ "${CERTOPTS[@]}" \ --request POST \ $ALPHA_HOST:8080/admin \ --data "$GQL" - ) - - if grep -q errors <<< "$RESULT"; then - ERROR=$(grep -oP '(?<=message":")[^"]*' <<< $RESULT) - echo "ERROR: $ERROR" - return 1 - fi + ) || CURL_STATUS=$? + require_json "$RESULT" "$CURL_STATUS" "${ALPHA_HOST}:8080/admin" || return 1 + ERROR=$(jq -r '.errors[0].message // empty' <<< "$RESULT") + if [[ -n "$ERROR" ]]; then echo "ERROR: $ERROR"; return 1; fi echo $RESULT } {{- end }} diff --git a/charts/dgraph/templates/backups/cronjob-full.yaml b/charts/dgraph/templates/backups/cronjob-full.yaml index 8f7f89e91..43cb8fda4 100644 --- a/charts/dgraph/templates/backups/cronjob-full.yaml +++ b/charts/dgraph/templates/backups/cronjob-full.yaml @@ -73,7 +73,7 @@ spec: {{- end }} {{- if .Values.alpha.acl.enabled }} - ACCESS_TOKEN=$(get_token {{ .Values.backups.admin.user }} $(cat /backup_secrets/backup_admin_password) $AUTH_TOKEN ) + ACCESS_TOKEN=$(get_token "{{ .Values.backups.admin.user }}" "$(cat /backup_secrets/backup_admin_password)" "${AUTH_TOKEN:-}") {{- end }} ## Full Backup with optional access and auth tokens diff --git a/charts/dgraph/templates/backups/cronjob-inc.yaml b/charts/dgraph/templates/backups/cronjob-inc.yaml index 8a591e794..162bb519d 100644 --- a/charts/dgraph/templates/backups/cronjob-inc.yaml +++ b/charts/dgraph/templates/backups/cronjob-inc.yaml @@ -73,7 +73,7 @@ spec: {{- end }} {{- if .Values.alpha.acl.enabled }} - ACCESS_TOKEN=$(get_token {{ .Values.backups.admin.user }} $(cat /backup_secrets/backup_admin_password) $AUTH_TOKEN ) + ACCESS_TOKEN=$(get_token "{{ .Values.backups.admin.user }}" "$(cat /backup_secrets/backup_admin_password)" "${AUTH_TOKEN:-}") {{- end }} ## Incremental Backup with optional access and auth tokens