Skip to content

Commit 8e146dd

Browse files
committed
fix(valkey): reject foreign nodes during slot restore
1 parent fc9971c commit 8e146dd

2 files changed

Lines changed: 100 additions & 0 deletions

File tree

addons/valkey/scripts-ut-spec/cluster_restore_slot_contract_spec.sh

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,12 @@ peerid peer:6379@16379 master - 0 0 2 connected 2-6'
193193
The stdout should be blank
194194
End
195195

196+
It "maps an engine node address through its announced hostname"
197+
line='id-abc 10.0.0.1:6379@16379,vk-shard-abc-0.h master - 0 0 1 connected'
198+
When call cluster_node_address_matches_fqdn "${line}" "vk-shard-abc-0.h"
199+
The status should be success
200+
End
201+
196202
It "does zero MEET writes when any fresh restored peer cannot resolve"
197203
meet_log=$(mktemp)
198204
resolve_cluster_meet_address() {
@@ -259,6 +265,28 @@ peerid peer:6379@16379 master - 0 0 2 connected 2-6'
259265
The file "${write_log}" should be empty file
260266
End
261267

268+
It "does zero slot writes when every primary sees the same foreign slotless node"
269+
write_log=$(mktemp)
270+
build_cli() { _cli=(mock_foreign_cli "$1" "${write_log}"); }
271+
mock_foreign_cli() {
272+
host="$1" log="$2"; shift 2
273+
[ "$1" = PING ] && echo PONG
274+
[ "$1" = CLUSTER ] && [ "$2" = ADDSLOTSRANGE ] && printf 'WRITE %s\n' "$*" >> "${log}"
275+
}
276+
cluster_nodes_of() {
277+
printf 'id-abc vk-shard-abc-0.h:6379@16379 master - 0 0 1 connected\n'
278+
printf 'id-def vk-shard-def-0.h:6379@16379 master - 0 0 2 connected\n'
279+
printf 'id-ghi vk-shard-ghi-0.h:6379@16379 master - 0 0 3 connected\n'
280+
printf 'id-foreign foreign-0.h:6379@16379 master - 0 0 4 connected\n'
281+
}
282+
When call restore_cluster_from_meta "${restore_meta}"
283+
The status should be failure
284+
The stderr should include "phase=restore-membership"
285+
The stderr should include "retry_safe=no"
286+
The stderr should include "foreign-0.h"
287+
The file "${write_log}" should be empty file
288+
End
289+
262290
It "closes restored primary duties without waiting for later replica actions"
263291
attach_log=$(mktemp)
264292
build_cli() { _cli=(mock_full_cli); }

addons/valkey/scripts/valkey-cluster-manage.sh

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,69 @@ node_id_sets_overlap() {
156156
return 1
157157
}
158158

159+
cluster_node_address_matches_fqdn() {
160+
local line="${1}" fqdn="${2}" address primary token
161+
local -a _cluster_address_parts
162+
address=$(printf '%s\n' "${line}" | awk '{print $2}')
163+
[ -n "${address}" ] || return 1
164+
primary="${address%%:*}"
165+
[ "${primary}" = "${fqdn}" ] && return 0
166+
IFS=',' read -ra _cluster_address_parts <<< "${address}"
167+
for token in "${_cluster_address_parts[@]:1}"; do
168+
token="${token#hostname=}"
169+
[ "${token}" = "${fqdn}" ] && return 0
170+
done
171+
return 1
172+
}
173+
174+
# A restore view may contain any already-attached configured replica, but it
175+
# must not contain a node outside the KubeBlocks roster. Identical views alone
176+
# are insufficient: every primary could agree on the same stale foreign node.
177+
cluster_view_contains_only_configured_members() {
178+
local nodes="${1}" roster="${2}" shard_line fqdns fqdn configured=""
179+
local line matched address seen=""
180+
181+
while IFS= read -r shard_line; do
182+
fqdns="${shard_line#* }"
183+
for fqdn in $(printf '%s\n' "${fqdns}" | tr ',' '\n' | grep -v '^$'); do
184+
if printf '%s\n' "${configured}" | grep -Fqx "${fqdn}"; then
185+
echo "duplicate configured restore member ${fqdn}" >&2
186+
return 1
187+
fi
188+
configured="${configured}${configured:+$'\n'}${fqdn}"
189+
done
190+
done <<< "${roster}"
191+
[ -n "${configured}" ] || {
192+
echo "configured restore roster is empty" >&2
193+
return 1
194+
}
195+
196+
printf '%s\n' "${nodes}" | cluster_node_id_set >/dev/null || return 1
197+
while IFS= read -r line; do
198+
[ -n "${line}" ] || continue
199+
matched=""
200+
while IFS= read -r fqdn; do
201+
[ -n "${fqdn}" ] || continue
202+
cluster_node_address_matches_fqdn "${line}" "${fqdn}" || continue
203+
[ -z "${matched}" ] || {
204+
echo "cluster node address matches multiple configured members: ${line}" >&2
205+
return 1
206+
}
207+
matched="${fqdn}"
208+
done <<< "${configured}"
209+
if [ -z "${matched}" ]; then
210+
address=$(printf '%s\n' "${line}" | awk '{print $2}')
211+
echo "cluster view contains foreign node ${address:-unknown}" >&2
212+
return 1
213+
fi
214+
if printf '%s\n' "${seen}" | grep -Fqx "${matched}"; then
215+
echo "cluster view contains duplicate configured member ${matched}" >&2
216+
return 1
217+
fi
218+
seen="${seen}${seen:+$'\n'}${matched}"
219+
done <<< "${nodes}"
220+
}
221+
159222
# Slot count currently owned by the master whose id is $2, read from $1's
160223
# view of CLUSTER NODES. Prints the count of slot ranges' total slots.
161224
slots_owned_by() {
@@ -599,6 +662,7 @@ restored_primary_cluster_ready_for_replica_attach() {
599662
for host in "${primary_hosts[@]}"; do
600663
[ "$(cluster_state_of "${host}")" = "ok" ] && [ "$(assigned_slots_of "${host}")" = "16384" ] || return 1
601664
nodes=$(cluster_nodes_of "${host}") || return 1
665+
cluster_view_contains_only_configured_members "${nodes}" "${roster}" || return 1
602666
observed=$(printf '%s\n' "${nodes}" | cluster_node_id_set) || return 1
603667
if [ -z "${expected}" ]; then
604668
expected="${observed}"
@@ -694,6 +758,10 @@ restore_cluster_from_meta() {
694758
classify restore-meet yes "malformed coordinator CLUSTER NODES from ${coord_host}"
695759
return 1
696760
}
761+
if ! cluster_view_contains_only_configured_members "${current_nodes}" "${roster}"; then
762+
classify restore-membership no "coordinator ${coord_host} sees a node outside the configured restore roster"
763+
return 1
764+
fi
697765

698766
# Only the deterministic coordinator writes MEET. Materialize and validate
699767
# every pending peer address before the first mutation, so a DNS gap cannot
@@ -759,6 +827,10 @@ restore_cluster_from_meta() {
759827
classify restore-membership yes "malformed CLUSTER NODES from ${host}"
760828
return 1
761829
}
830+
if ! cluster_view_contains_only_configured_members "${other_nodes}" "${roster}"; then
831+
classify restore-membership no "${host} sees a node outside the configured restore roster"
832+
return 1
833+
fi
762834
for other_id in "${primary_ids[@]}"; do
763835
if ! printf '%s\n' "${other_ids}" | grep -Fqx "${other_id}"; then
764836
classify restore-membership yes "${host} does not yet see restored primary ${other_id}"

0 commit comments

Comments
 (0)