Skip to content

Commit a8e31be

Browse files
committed
fix: address review feedback on runner script
- Add ca-certificates to runner dependencies for TLS verification - Add --fail flag to curl downloads so HTTP errors (404/403) fail instead of silently writing error pages as model files - Guard --model and --flag value parsing against missing arguments to avoid unbound variable errors with set -euo pipefail - Use grep -qF (fixed-string) instead of grep -q (regex) for model cache validation to avoid false matches on dots in model IDs
1 parent eaad407 commit a8e31be

File tree

2 files changed

+20
-11
lines changed

2 files changed

+20
-11
lines changed

pkg/aikit2llb/inference/runner.go

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,13 @@ func isRunnerMode(c *config.InferenceConfig) bool {
2222
func installRunnerDependencies(_ *config.InferenceConfig, s llb.State, merge llb.State, platform specs.Platform) (llb.State, llb.State) {
2323
savedState := s
2424

25-
// Install curl for HTTP/HTTPS downloads and python3 + pip for huggingface-cli.
25+
// Install curl for HTTP/HTTPS downloads, ca-certificates for TLS verification,
26+
// and python3 + pip for huggingface-cli.
2627
// Some backends (diffusers/vllm) already install python, but llama-cpp does not,
2728
// so we always install the minimal set here.
2829
// Note: Runner mode is not supported for Apple Silicon (validated in build).
2930
s = s.Run(
30-
utils.Sh("apt-get update && apt-get install --no-install-recommends -y curl python3 python3-pip && (pip install --break-system-packages huggingface-hub[cli] 2>/dev/null || pip install huggingface-hub[cli]) && apt-get clean"),
31+
utils.Sh("apt-get update && apt-get install --no-install-recommends -y curl ca-certificates python3 python3-pip && (pip install --break-system-packages huggingface-hub[cli] 2>/dev/null || pip install huggingface-hub[cli]) && apt-get clean"),
3132
llb.WithCustomNamef("Installing runner dependencies for platform %s/%s", platform.OS, platform.Architecture),
3233
llb.IgnoreCache,
3334
).Root()
@@ -79,6 +80,7 @@ EXTRA_ARGS=()
7980
while [[ $# -gt 0 ]]; do
8081
case "$1" in
8182
--model)
83+
[[ $# -ge 2 ]] || { echo "Error: --model requires a value"; exit 1; }
8284
MODEL="$2"
8385
shift 2
8486
;;
@@ -91,8 +93,13 @@ while [[ $# -gt 0 ]]; do
9193
shift
9294
;;
9395
--*)
94-
EXTRA_ARGS+=("$1" "$2")
95-
shift 2
96+
if [[ $# -ge 2 ]]; then
97+
EXTRA_ARGS+=("$1" "$2")
98+
shift 2
99+
else
100+
EXTRA_ARGS+=("$1")
101+
shift
102+
fi
96103
;;
97104
*)
98105
if [[ -z "$MODEL" ]]; then
@@ -179,7 +186,7 @@ else
179186
# Direct HTTP/HTTPS download
180187
echo "Downloading model from URL: $MODEL"
181188
FILENAME=$(basename "$MODEL")
182-
curl -L --progress-bar -o "/models/$FILENAME" "$MODEL"
189+
curl -fL --progress-bar -o "/models/$FILENAME" "$MODEL"
183190
else
184191
# HuggingFace repo - download GGUF files
185192
echo "Downloading GGUF files from HuggingFace: $MODEL"
@@ -217,7 +224,7 @@ fi
217224
func generateHFModelConfig(backend string) string {
218225
return fmt.Sprintf(`# Check if model config matches the requested model (volume mount caching)
219226
MODEL_NAME=$(echo "$MODEL" | tr '/' '-')
220-
if [[ -f "/models/aikit-model.yaml" ]] && grep -q "model: ${MODEL}$" /models/aikit-model.yaml 2>/dev/null; then
227+
if [[ -f "/models/aikit-model.yaml" ]] && grep -qF "model: ${MODEL}" /models/aikit-model.yaml 2>/dev/null; then
221228
echo "Found existing model config matching $MODEL in /models, skipping setup"
222229
else
223230
if [[ -f "/models/aikit-model.yaml" ]]; then

pkg/aikit2llb/inference/runner_test.go

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ func TestGenerateRunnerScript(t *testing.T) {
8787
`BACKEND="llama-cpp"`,
8888
".aikit-model-ref",
8989
"huggingface-cli",
90-
"curl -L",
90+
"curl -fL",
9191
"exec /usr/bin/local-ai",
9292
},
9393
expectMissing: []string{
@@ -175,8 +175,10 @@ func TestGenerateRunnerScriptArgParser(t *testing.T) {
175175
if !strings.Contains(script, `--*=*)`) {
176176
t.Error("arg parser should handle --flag=value style arguments with single shift")
177177
}
178-
if !strings.Contains(script, "EXTRA_ARGS+=(\"$1\" \"$2\")") {
179-
t.Error("arg parser should consume both --flag and its value argument")
178+
179+
// Should guard against trailing flags without values
180+
if !strings.Contains(script, `[[ $# -ge 2 ]]`) {
181+
t.Error("arg parser should guard against trailing flags without values")
180182
}
181183

182184
// Should strip huggingface:// URI prefix for kubeairunway compatibility
@@ -269,8 +271,8 @@ func TestGenerateHFModelConfig(t *testing.T) {
269271
script := generateHFModelConfig(tt.backend)
270272

271273
// Should verify cached config matches the requested model
272-
if !strings.Contains(script, "grep -q") {
273-
t.Error("should verify cached config matches requested model")
274+
if !strings.Contains(script, "grep -qF") {
275+
t.Error("should use fixed-string grep to verify cached config matches requested model")
274276
}
275277

276278
// Should contain correct backend reference

0 commit comments

Comments
 (0)