Skip to content

feat(aws): add --tool-call-parser and --chat-template flags for RHEL AI #848

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
are-ces wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat(aws): add --tool-call-parser and --chat-template flags for RHEL AI #848

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 6 additions & 0 deletions cmd/mapt/cmd/aws/hosts/rhelai.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,9 @@ func getRHELAICreate() *cobra.Command {
HFToken: viper.GetString(params.RhelAIHFToken),
APIKey: viper.GetString(params.RhelAIAPIKey),
AutoStart: viper.IsSet(params.RhelAIAutoStart),
ToolCallParser: viper.GetString(params.RhelAIToolCallParser),
ChatTemplate: viper.GetString(params.RhelAIChatTemplate),
MaxModelLen: viper.GetInt(params.RhelAIMaxModelLen),
ExposePorts: viper.GetIntSlice(params.RhelAIExposePorts),
})
},
Expand All @@ -87,6 +90,9 @@ func getRHELAICreate() *cobra.Command {
flagSet.StringP(params.RhelAIAPIKey, "", "", params.RhelAIAPIKeyDesc)
flagSet.Bool(params.RhelAIAutoStart, false, params.RhelAIAutoStartDesc)
flagSet.IntSlice(params.RhelAIExposePorts, nil, params.RhelAIExposePortsDesc)
flagSet.StringP(params.RhelAIToolCallParser, "", "", params.RhelAIToolCallParserDesc)
flagSet.StringP(params.RhelAIChatTemplate, "", "", params.RhelAIChatTemplateDesc)
flagSet.Int(params.RhelAIMaxModelLen, 0, params.RhelAIMaxModelLenDesc)
flagSet.StringP(params.Timeout, "", "", params.TimeoutDesc)
params.AddComputeRequestFlags(flagSet)
params.AddSpotFlags(flagSet)
Expand Down
6 changes: 6 additions & 0 deletions cmd/mapt/cmd/params/params.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,12 @@ const (
RhelAIAutoStartDesc string = "automatically configure and start RHAIIS after provisioning"
RhelAIExposePorts string = "expose-ports"
RhelAIExposePortsDesc string = "comma-separated list of ports to expose through the load balancer and security group (e.g. 8000,8080)"
RhelAIToolCallParser string = "tool-call-parser"
RhelAIToolCallParserDesc string = "enable tool calling with the specified parser (e.g. llama3_json, hermes, mistral)"
RhelAIChatTemplate string = "chat-template"
RhelAIChatTemplateDesc string = "chat template jinja filename (e.g. tool_chat_template_llama3.2_json.jinja)"
Comment thread
coderabbitai[bot] marked this conversation as resolved.
RhelAIMaxModelLen string = "max-model-len"
RhelAIMaxModelLenDesc string = "maximum model context length in tokens (default 4096)"

// Serverless
Timeout string = "timeout"
Expand Down
26 changes: 26 additions & 0 deletions pkg/provider/aws/action/rhel-ai/rhelai.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ type rhelAIRequest struct {
hfToken *string
apiKey *string
autoStart bool
toolCallParser *string
chatTemplate *string
maxModelLen int
exposePorts []int
}

Expand Down Expand Up @@ -85,6 +88,9 @@ func Create(mCtxArgs *mc.ContextArgs, args *apiRHELAI.RHELAIArgs) (err error) {
hfToken: &args.HFToken,
apiKey: &args.APIKey,
autoStart: args.AutoStart,
maxModelLen: args.MaxModelLen,
toolCallParser: &args.ToolCallParser,
chatTemplate: &args.ChatTemplate,
exposePorts: args.ExposePorts}
if args.Spot != nil {
r.spot = args.Spot.Spot
Expand Down Expand Up @@ -373,6 +379,26 @@ func (r *rhelAIRequest) rhaiisSetupScript() string {
` && sudo sed -i 's|--model .*|--model %s \\|' %s/install.conf`,
*r.model, confDir)
}
script += fmt.Sprintf(
` && GPU_COUNT=$(nvidia-smi -L 2>/dev/null | wc -l) && [ "$GPU_COUNT" -gt 0 ] && sudo sed -i "s|--tensor-parallel-size 1|--tensor-parallel-size $GPU_COUNT|" %s/install.conf`,
confDir)
maxModelLen := 4096
if r.maxModelLen > 0 {
maxModelLen = r.maxModelLen
}
if len(*r.toolCallParser) > 0 {
toolArgs := fmt.Sprintf(`--enable-auto-tool-choice \\\n --tool-call-parser %s`, *r.toolCallParser)
if len(*r.chatTemplate) > 0 {
toolArgs += fmt.Sprintf(` \\\n --chat-template /opt/app-root/template/%s`, *r.chatTemplate)
}
script += fmt.Sprintf(
` && sudo sed -i 's|--max-model-len.*|--max-model-len %d \\\n %s|' %s/install.conf`,
maxModelLen, toolArgs, confDir)
} else if r.maxModelLen > 0 {
script += fmt.Sprintf(
` && sudo sed -i 's|--max-model-len.*|--max-model-len %d|' %s/install.conf`,
maxModelLen, confDir)
}
Comment on lines +389 to +401

@coderabbitai coderabbitai Bot Jun 27, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

--chat-template is silently ignored unless --tool-call-parser is also set.

Line 380 gates the entire config update on toolCallParser, so a standalone --chat-template value is accepted by the CLI/Tekton layers but never reaches install.conf. Either validate that dependency earlier or handle chatTemplate independently here.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/provider/aws/action/rhel-ai/rhelai.go` around lines 380 - 388, The
install.conf update in rhelai.go only runs when toolCallParser is set, so a
standalone chatTemplate value is dropped even though it was accepted earlier.
Update the config-building logic around the rhelai.go script assembly to either
validate the chatTemplate/toolCallParser dependency up front or apply
chatTemplate independently in this block, using the existing toolCallParser and
chatTemplate symbols so both settings are handled correctly.

if len(*r.apiKey) > 0 {
script += fmt.Sprintf(
" && sudo sed -i '/\\[Install\\]/i Environment=VLLM_API_KEY=%s' %s/install.conf",
Expand Down
9 changes: 6 additions & 3 deletions pkg/target/host/rhelai/api.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,10 @@ type RHELAIArgs struct {
Timeout string
Model string
HFToken string
APIKey string
AutoStart bool
ExposePorts []int
APIKey string
AutoStart bool
ToolCallParser string
ChatTemplate string
MaxModelLen int
ExposePorts []int
}
18 changes: 18 additions & 0 deletions tkn/infra-aws-rhel-ai.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,15 @@ spec:
- name: expose-ports
description: Comma-separated list of ports to expose through the load balancer and security group (e.g. 8000,8080).
default: ""
- name: tool-call-parser
description: Enable tool calling with the specified parser (e.g. llama3_json, hermes, mistral). Automatically adds --enable-auto-tool-choice.
default: ""
- name: chat-template
description: Chat template jinja filename (e.g. tool_chat_template_llama3.2_json.jinja).
default: ""
- name: max-model-len
description: Maximum model context length in tokens (default 4096). Increase for tool calling or larger models.
default: "0"

# Network params
- name: service-endpoints
Expand Down Expand Up @@ -317,6 +326,15 @@ spec:
if [[ "$(params.expose-ports)" != "" ]]; then
cmd+="--expose-ports '$(params.expose-ports)' "
fi
if [[ "$(params.tool-call-parser)" != "" ]]; then
cmd+="--tool-call-parser '$(params.tool-call-parser)' "
fi
if [[ "$(params.chat-template)" != "" ]]; then
cmd+="--chat-template '$(params.chat-template)' "
fi
Comment on lines +329 to +334

@coderabbitai coderabbitai Bot Jun 27, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win

Do not append raw Tekton params into the eval command string.

These values flow into cmd and are executed via eval later in the script. A tool-call-parser or chat-template containing a single quote can break quoting and run arbitrary shell in the task pod, exposing the mounted credentials. Build argv incrementally and exec it directly instead of extending the eval string.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tkn/infra-aws-rhel-ai.yaml` around lines 326 - 331, The cmd-building logic in
the Tekton script appends raw params for tool-call-parser and chat-template into
an eval string, which can break quoting and enable shell injection. Update the
command assembly to stop concatenating these params into cmd; instead, build
argv incrementally in the same script section that handles params and execute
the command directly without eval, using the existing parameter checks around
tool-call-parser and chat-template as the place to refactor.

if [[ "$(params.max-model-len)" != "0" ]]; then
cmd+="--max-model-len '$(params.max-model-len)' "
fi
cmd+="--tags '$(params.tags)' "
fi

Expand Down
12 changes: 12 additions & 0 deletions tkn/infra-azure-rhel-ai.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,12 @@ spec:
- name: disk-size
description: Disk size in GB for the cloud instance
default: "200"
- name: gpus
description: Number of GPUs for the cloud instance (valid marketplace values are 1, 2, 4, 8)
default: "8"
Comment on lines +88 to +90

@coderabbitai coderabbitai Bot Jun 27, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win

Preserve the Azure RHEL AI GPU default unless callers opt in.

With default: "8", the condition on Line 238 is always true for omitted Tekton params, so this task now always passes --gpus 8. That bypasses the CLI/provider default path where unset/0 GPUs are normalized to 1, which can unexpectedly select larger marketplace SKUs and hit quota/cost limits.

Proposed fix 
     - name: gpus
       description: Number of GPUs for the cloud instance (valid marketplace values are 1, 2, 4, 8)
-      default: "8"
+      default: ""

As per path instructions, **: “Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.”

Also applies to: 238-240

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tkn/infra-azure-rhel-ai.yaml` around lines 88 - 90, The Tekton param default
for gpus is forcing the Azure RHEL AI task to always pass --gpus 8 when callers
omit the value, bypassing the provider’s unset/0 normalization path. Update the
gpus parameter in infra-azure-rhel-ai.yaml so it does not default to 8, and let
the existing CLI/provider handling in the task logic around the gpus condition
at the referenced block decide the effective GPU count unless a caller
explicitly opts in.

Source: Path instructions

- name: gpu-manufacturer
description: GPU manufacturer name for instance filtering (e.g. NVIDIA, AMD)
default: ""
- name: compute-sizes
description: Comma seperated list of sizes for the machines to be requested. If set this takes precedence over compute by args
default: "Standard_ND96is_MI300X_v5,Standard_ND96isr_MI300X_v5"
Expand Down Expand Up @@ -229,6 +235,12 @@ spec:
if [[ "$(params.compute-sizes)" != "" ]]; then
cmd+="--compute-sizes '$(params.compute-sizes)' "
fi
if [[ "$(params.gpus)" != "" ]]; then
cmd+="--gpus '$(params.gpus)' "
fi
if [[ "$(params.gpu-manufacturer)" != "" ]]; then
cmd+="--gpu-manufacturer '$(params.gpu-manufacturer)' "
fi
Comment on lines +238 to +243

@coderabbitai coderabbitai Bot Jun 27, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Validate new params before adding them to the eval’d command.

These Tekton params are interpolated into cmd and executed via eval on Line 272. A value containing a single quote can break out of the intended argument and execute shell syntax.

Proposed hardening 
           if [[ "$(params.gpus)" != "" ]]; then
-            cmd+="--gpus '$(params.gpus)' "
+            case "$(params.gpus)" in
+              *[!0-9]*) echo "Parameter gpus must be numeric"; exit 1 ;;
+            esac
+            cmd+="--gpus $(params.gpus) "
           fi
           if [[ "$(params.gpu-manufacturer)" != "" ]]; then
+            case "$(params.gpu-manufacturer)" in
+              *[!A-Za-z0-9._-]*) echo "Parameter gpu-manufacturer contains unsupported characters"; exit 1 ;;
+            esac
             cmd+="--gpu-manufacturer '$(params.gpu-manufacturer)' "
           fi

As per path instructions, **: “Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.”

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if [[ "$(params.gpus)" != "" ]]; then
cmd+="--gpus '$(params.gpus)' "
fi
if [[ "$(params.gpu-manufacturer)" != "" ]]; then
cmd+="--gpu-manufacturer '$(params.gpu-manufacturer)' "
fi
if [[ "$(params.gpus)" != "" ]]; then
case "$(params.gpus)" in
*[!0-9]*) echo "Parameter gpus must be numeric"; exit 1 ;;
esac
cmd+="--gpus $(params.gpus) "
fi
if [[ "$(params.gpu-manufacturer)" != "" ]]; then
case "$(params.gpu-manufacturer)" in
*[!A-Za-z0-9._-]*) echo "Parameter gpu-manufacturer contains unsupported characters"; exit 1 ;;
esac
cmd+="--gpu-manufacturer '$(params.gpu-manufacturer)' "
fi
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tkn/infra-azure-rhel-ai.yaml` around lines 238 - 243, The command-building
logic in the Tekton task is vulnerable because params like gpus and
gpu-manufacturer are interpolated into cmd and later executed with eval. Harden
the command assembly by validating or sanitizing these params before appending
them, or avoid eval entirely by passing arguments safely; update the logic in
the script section where cmd is built so single quotes or shell metacharacters
cannot break out of the intended argument.

Source: Path instructions

if [[ "$(params.marketplace)" == "true" ]]; then
cmd+="--marketplace "
cmd+="--accelerator '$(params.accelerator)' "
Expand Down
18 changes: 18 additions & 0 deletions tkn/template/infra-aws-rhel-ai.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,15 @@ spec:
- name: expose-ports
description: Comma-separated list of ports to expose through the load balancer and security group (e.g. 8000,8080).
default: ""
- name: tool-call-parser
description: Enable tool calling with the specified parser (e.g. llama3_json, hermes, mistral). Automatically adds --enable-auto-tool-choice.
default: ""
- name: chat-template
description: Chat template jinja filename (e.g. tool_chat_template_llama3.2_json.jinja).
default: ""
- name: max-model-len
description: Maximum model context length in tokens (default 4096). Increase for tool calling or larger models.
default: "0"

# Network params
- name: service-endpoints
Expand Down Expand Up @@ -317,6 +326,15 @@ spec:
if [[ "$(params.expose-ports)" != "" ]]; then
cmd+="--expose-ports '$(params.expose-ports)' "
fi
if [[ "$(params.tool-call-parser)" != "" ]]; then
cmd+="--tool-call-parser '$(params.tool-call-parser)' "
fi
if [[ "$(params.chat-template)" != "" ]]; then
cmd+="--chat-template '$(params.chat-template)' "
fi
Comment on lines +329 to +334

@coderabbitai coderabbitai Bot Jun 27, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win

Do not append raw Tekton params into the eval command string.

These values flow into cmd and are executed via eval later in the script. A tool-call-parser or chat-template containing a single quote can break quoting and run arbitrary shell in the task pod, exposing the mounted credentials. Build argv incrementally and exec it directly instead of extending the eval string.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tkn/template/infra-aws-rhel-ai.yaml` around lines 326 - 331, The
command-building logic in the Tekton template is unsafe because `cmd` is later
executed with `eval`, and `params.tool-call-parser` / `params.chat-template` are
being appended as raw quoted text. Update the script to stop concatenating
Tekton params into the eval string; instead, build the argument list
incrementally in the same section that sets `cmd`, and execute the command
directly without `eval`. Use the existing parameter-handling block around
`tool-call-parser` and `chat-template` as the place to switch from string
concatenation to argv-based execution.

if [[ "$(params.max-model-len)" != "0" ]]; then
cmd+="--max-model-len '$(params.max-model-len)' "
fi
cmd+="--tags '$(params.tags)' "
fi

Expand Down