Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions .github/workflows/result-server-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ on:
paths:
- "result_server/**"
- "scripts/bk_functions.sh"
- "scripts/collect_timing.sh"
- "scripts/result.sh"
- "scripts/result_server/**"
- "scripts/estimation/**"
Expand Down Expand Up @@ -32,6 +33,7 @@ on:
paths:
- "result_server/**"
- "scripts/bk_functions.sh"
- "scripts/collect_timing.sh"
- "scripts/result.sh"
- "scripts/result_server/**"
- "scripts/estimation/**"
Expand Down
7 changes: 7 additions & 0 deletions docs/deploy/hardening-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ Example options:
```text
WorkingDirectory=<deploy-root>
Environment=PYTHONPATH=<deploy-root>/benchkit/result_server
Environment=BASE_PATH=<result-data-root>
ExecStart=<venv>/bin/gunicorn \
-w 2 \
-b 127.0.0.1:8800 \
Expand All @@ -79,6 +80,7 @@ ExecStart=<venv>/bin/gunicorn \
An equivalent direct import form is:

```text
BASE_PATH=<result-data-root> \
gunicorn --chdir <deploy-root>/benchkit/result_server \
-w 2 \
-b 127.0.0.1:8800 \
Expand All @@ -93,6 +95,11 @@ while the existing `from routes.*` and `from utils.*` imports are resolved by
putting `benchkit/result_server` on `PYTHONPATH` or making it the working
directory.

`BASE_PATH` is required because the application validates the result-data root
when `app.py` is imported. Point it at the directory that contains the portal's
received results, estimated results, profiler archives, and related runtime
data.

For deployments that want a separate audit file in addition to stderr capture,
set:

Expand Down
8 changes: 8 additions & 0 deletions result_server/tests/test_script_source_info_security.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,14 @@ def test_result_script_does_not_source_source_info_env():
assert "jq -n" in result_script


def test_result_script_does_not_source_timing_env():
result_script = (REPO_ROOT / "scripts" / "result.sh").read_text(encoding="utf-8")

assert ". results/timing.env" not in result_script
assert "source results/timing.env" not in result_script
assert "results/pipeline_timing.json" in result_script


def test_bk_fetch_source_writes_encoded_source_info_values():
bk_functions = (REPO_ROOT / "scripts" / "bk_functions.sh").read_text(encoding="utf-8")

Expand Down
46 changes: 36 additions & 10 deletions scripts/collect_timing.sh
Original file line number Diff line number Diff line change
@@ -1,32 +1,58 @@
#!/bin/bash
# collect_timing.sh - Collect timing information from timestamp files
# Reads build/run/queue timestamp files and generates results/timing.env
# Reads build/run/queue timestamp files and generates results/pipeline_timing.json

BUILD_TIME=0
QUEUE_TIME=0
RUN_TIME=0

timestamp_value() {
local path="$1"
local value=""

if [ -f "$path" ]; then
value=$(cat "$path")
fi

case "$value" in
''|*[!0-9]*)
printf ''
;;
*)
printf '%s' "$value"
;;
esac
}

# Build time = build_end - build_start
if [ -f results/build_start ] && [ -f results/build_end ]; then
bs=$(cat results/build_start)
be=$(cat results/build_end)
BUILD_TIME=$((be - bs))
bs=$(timestamp_value results/build_start)
be=$(timestamp_value results/build_end)
if [ -n "$bs" ] && [ -n "$be" ] && [ "$be" -ge "$bs" ]; then
BUILD_TIME=$((be - bs))
fi
fi

# Run time = run_end - run_start
if [ -f results/run_start ] && [ -f results/run_end ]; then
rs=$(cat results/run_start)
re=$(cat results/run_end)
RUN_TIME=$((re - rs))
rs=$(timestamp_value results/run_start)
re=$(timestamp_value results/run_end)
if [ -n "$rs" ] && [ -n "$re" ] && [ "$re" -ge "$rs" ]; then
RUN_TIME=$((re - rs))
fi
fi

# Queue time: not measurable with current Jacamar/pjsub architecture
# (before_script/script all run inside the batch job, so queue_submit
# is recorded after the job has already started)
QUEUE_TIME=0

echo "BUILD_TIME=$BUILD_TIME" > results/timing.env
echo "QUEUE_TIME=$QUEUE_TIME" >> results/timing.env
echo "RUN_TIME=$RUN_TIME" >> results/timing.env
cat > results/pipeline_timing.json <<EOF
{
"build_time": $BUILD_TIME,
"queue_time": $QUEUE_TIME,
"run_time": $RUN_TIME
}
EOF

echo "Timing collected: build=${BUILD_TIME}s queue=${QUEUE_TIME}s run=${RUN_TIME}s"
24 changes: 16 additions & 8 deletions scripts/result.sh
Original file line number Diff line number Diff line change
Expand Up @@ -158,16 +158,24 @@ write_result_json() {
local idx="$1"
local fom_breakdown_block=""

# Build pipeline_timing block if timing.env exists (only for first result to avoid duplication)
# Build pipeline_timing block if pipeline_timing.json exists (only for first result to avoid duplication).
# Treat the file as data; never source generated timing files as shell.
local timing_block=""
if [ "$idx" = "0" ] && [ -f results/timing.env ]; then
source results/timing.env
if [ "$idx" = "0" ] && [ -f results/pipeline_timing.json ]; then
local pipeline_timing_json
pipeline_timing_json=$(jq -c '
def num: if type == "number" then . else (tonumber? // 0) end;
{
build_time: ((.build_time // 0) | num),
queue_time: ((.queue_time // 0) | num),
run_time: ((.run_time // 0) | num)
}
' results/pipeline_timing.json 2>/dev/null || true)
if [ -z "$pipeline_timing_json" ] || [ "$pipeline_timing_json" = "null" ]; then
pipeline_timing_json='{"build_time":0,"queue_time":0,"run_time":0}'
fi
timing_block=",
\"pipeline_timing\": {
\"build_time\": ${BUILD_TIME:-0},
\"queue_time\": ${QUEUE_TIME:-0},
\"run_time\": ${RUN_TIME:-0}
}"
\"pipeline_timing\": $pipeline_timing_json"
fi

# Build execution_mode block
Expand Down
35 changes: 35 additions & 0 deletions scripts/tests/test_result_profile_data.sh
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,20 @@ cat > "${TMP_DIR}/results/result" <<'EOF'
FOM:9.9999999999999995e-07 FOM_version:test Exp:CASE0 node_count:1 numproc_node:1 nthreads:2
EOF

cat > "${TMP_DIR}/results/pipeline_timing.json" <<'EOF'
{
"build_time": "12",
"queue_time": 0,
"run_time": 34
}
EOF

cat > "${TMP_DIR}/results/timing.env" <<'EOF'
BUILD_TIME=$(touch timing_env_was_sourced)
QUEUE_TIME=999
RUN_TIME=999
EOF

cat > "${TMP_DIR}/bk_profiler_artifact/meta.json" <<'EOF'
{
"tool": "fapp",
Expand Down Expand Up @@ -84,9 +98,13 @@ jq -e '
.profile_data.level == "single" and
.profile_data.report_format == "text" and
.profile_data.run_count == 1 and
.pipeline_timing.build_time == 12 and
.pipeline_timing.queue_time == 0 and
.pipeline_timing.run_time == 34 and
(.profile_data.events | index("pa1") != null) and
(.profile_data.report_kinds | index("summary_text") != null)
' "${RESULT_JSON}" >/dev/null
test ! -f "${TMP_DIR}/timing_env_was_sourced"

NCU_RESULT_JSON="${TMP_DIR}/ncu/results/result0.json"
test -f "${NCU_RESULT_JSON}"
Expand All @@ -100,4 +118,21 @@ jq -e '
(.profile_data.report_kinds | index("ncu_report") != null)
' "${NCU_RESULT_JSON}" >/dev/null

TIMING_TMP="${TMP_DIR}/timing"
mkdir -p "${TIMING_TMP}/results"
printf '100\n' > "${TIMING_TMP}/results/build_start"
printf '115\n' > "${TIMING_TMP}/results/build_end"
printf 'bad\n' > "${TIMING_TMP}/results/run_start"
printf '200\n' > "${TIMING_TMP}/results/run_end"
pushd "${TIMING_TMP}" >/dev/null
bash "${REPO_DIR}/scripts/collect_timing.sh" >/dev/null
popd >/dev/null
test -f "${TIMING_TMP}/results/pipeline_timing.json"
test ! -f "${TIMING_TMP}/results/timing.env"
jq -e '
.build_time == 15 and
.queue_time == 0 and
.run_time == 0
' "${TIMING_TMP}/results/pipeline_timing.json" >/dev/null

echo "result profile_data test passed"
Loading