Skip to content

2059: feat(scheduler): Redirect '/' to nighlies.a.o WebTUI #84

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
martin-augment wants to merge 4 commits into
base: main
Choose a base branch
from
Open

2059: feat(scheduler): Redirect '/' to nighlies.a.o WebTUI #84

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
91728c5
feat(scheduler): Redirect '/' to nighlies.a.o WebTUI
martin-g Jul 16, 2026
30cee70
Fix rsync delete command: `--delete does not work without --recursive…
martin-g Jul 16, 2026
023d603
Re-enable the CI job condition to deploy WebTUI only on push to main
martin-g Jul 16, 2026
d87933f
Add AI config files
martin-g Jul 16, 2026
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
5 changes: 5 additions & 0 deletions .cursor/rules.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
When making a code review never link to any issue or a pull request
in any GitHub repository!

Do not review AI agents' config files like CLAUDE.md, AGENTS.md or rules.md!

5 changes: 5 additions & 0 deletions .gemini/rules.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
When making a code review never link to any issue or a pull request
in any GitHub repository!

Do not review AI agents' config files like CLAUDE.md, AGENTS.md or rules.md!

4 changes: 2 additions & 2 deletions .github/workflows/web-tui.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ jobs:
--no-default-features --features web

- name: Deploy at nightlies.a.o
if: ${{ github.event == 'push' && github.ref == 'refs/heads/main' }}
if: ${{ github.event_name == 'push' && github.ref_name == 'main' }}
run: |
TUI_DIR="datafusion/ballista/tui/${{ steps.cargo_metadata.outputs.ballista_version }}"
REMOTE_TARGET_DIR="${{ secrets.NIGHTLIES_RSYNC_PATH }}/${TUI_DIR}/"
Expand All @@ -97,7 +97,7 @@ jobs:

ssh -p ${{ secrets.NIGHTLIES_RSYNC_PORT }} -l ${{ secrets.NIGHTLIES_RSYNC_USER }} ${{ secrets.NIGHTLIES_RSYNC_HOST }} "mkdir -p ${REMOTE_TARGET_DIR}"

rsync --times --compress --delete --verbose -e "ssh -p ${{ secrets.NIGHTLIES_RSYNC_PORT }} -l ${{ secrets.NIGHTLIES_RSYNC_USER }}" ./target/web-tui/* ${{ secrets.NIGHTLIES_RSYNC_HOST }}:${REMOTE_TARGET_DIR}
rsync --times --compress --delete --recursive --verbose -e "ssh -p ${{ secrets.NIGHTLIES_RSYNC_PORT }} -l ${{ secrets.NIGHTLIES_RSYNC_USER }}" ./target/web-tui/* ${{ secrets.NIGHTLIES_RSYNC_HOST }}:${REMOTE_TARGET_DIR}

- name: Upload WASM32 application
uses: actions/upload-artifact@v7
Expand Down
5 changes: 5 additions & 0 deletions AGENTS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
When making a code review never link to any issue or a pull request
in any GitHub repository!

Do not review AI agents' config files like CLAUDE.md, AGENTS.md or rules.md!

5 changes: 5 additions & 0 deletions CLAUDE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
When making a code review never link to any issue or a pull request
in any GitHub repository!

Do not review AI agents' config files like CLAUDE.md, AGENTS.md or rules.md!

51 changes: 41 additions & 10 deletions ballista/scheduler/src/api/handlers.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ use crate::state::execution_graph_dot::ExecutionGraphDot;
use crate::state::execution_stage::TaskInfo;
use crate::{api::SchedulerErrorResponse, scheduler_server::SchedulerServer};
use axum::extract::Query;
use axum::response::Redirect;
use axum::{
Json,
extract::{Path, State},
Expand Down Expand Up @@ -46,7 +47,7 @@ use graphviz_rust::{
exec,
printer::PrinterContext,
};
use http::{StatusCode, header::CONTENT_TYPE};
use http::{HeaderMap, StatusCode, header::CONTENT_TYPE};
use serde::Serialize;
use std::sync::Arc;
use std::time::Duration;
Expand Down Expand Up @@ -227,6 +228,36 @@ pub enum PlanFormat {
Metrics,
}

/// A handler for GET requests to the root (`/`).
/// It redirects to https://nightlies.apache.org/datafusion/ballista/tui/<BALLISTA_VERSION>/
/// forwarding any query parameters
pub async fn get_root(
header_map: HeaderMap,
Query(mut params): Query<std::collections::HashMap<String, String>>,

@augmentcode augmentcode Bot Jul 16, 2026 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Query<HashMap<String, String>> drops duplicate query parameters, which may not fully “forward any query parameters” as the doc comment states. If clients rely on repeated keys (e.g. foo=1&foo=2), the redirect could change behavior.

Severity: low

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.

) -> Result<Redirect, (StatusCode, String)> {
const NIGHTLIES_URL: &str = "https://nightlies.apache.org/datafusion/ballista/tui";

let ballista_scheduler_url =
params.remove("ballista_scheduler_url").unwrap_or_else(|| {
let default_scheduler_url = "localhost:50050";
let scheduler_url = header_map
.get("host")
.map(|hv| hv.to_str().unwrap_or(default_scheduler_url))
.unwrap_or(default_scheduler_url);
format!("http://{scheduler_url}")
});

let mut target = format!(
"{NIGHTLIES_URL}/{BALLISTA_VERSION}/?ballista_scheduler_url={ballista_scheduler_url}",
);

for (k, v) in params.iter() {
target.push_str(format!("&{}={}", k, v).as_str());

@augmentcode augmentcode Bot Jul 16, 2026 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

target is built by concatenating raw query keys/values (and ballista_scheduler_url) without URL-encoding, so values containing &, = or # can break the Location header or inject unintended query params. Consider ensuring the constructed redirect URL is properly encoded before passing it to Redirect::temporary.

Severity: medium

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.

}
Comment on lines +250 to +256

@coderabbitai coderabbitai Bot Jul 16, 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.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

URL-encode query parameters to prevent HTTP Parameter Pollution.

Constructing a URL by manually concatenating query parameters without URL encoding can lead to invalid URLs or HTTP Parameter Pollution (HPP). If any parameter value (including the host header or values in params) contains reserved characters like &, =, or #, it will break the parsed query string on the receiving end.

Consider using a URL construction library (like the url crate) or a serialization crate (like serde_urlencoded, which axum usually depends on) to safely build the query 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 `@ballista/scheduler/src/api/handlers.rs` around lines 250 - 256, The target
URL construction must URL-encode the scheduler URL and every key/value from
params instead of manually concatenating raw strings. Update the code around
target and the params iteration to use the existing URL or query-serialization
utilities, preserving all parameters while ensuring reserved characters such as
&, =, and # remain encoded within their individual values.


Ok(Redirect::temporary(&target))
}
Comment on lines +234 to +259

@gemini-code-assist gemini-code-assist Bot Jul 16, 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.

high

Issues Identified:

  1. Lack of URL Encoding: Reconstructing the query string using direct string interpolation (format!("&{}={}", k, v)) does not URL-encode the keys and values. If any query parameter contains special characters (like spaces, ampersands, or equal signs), it will result in an invalid or corrupted URL.
  2. Mixed Content / Protocol Detection: Hardcoding http:// for the ballista_scheduler_url will cause browsers to block the WebTUI's requests to the scheduler if the scheduler is accessed via HTTPS (e.g., behind a reverse proxy). This is because nightlies.apache.org is served over HTTPS, and modern browsers block active mixed content (HTTP requests from an HTTPS page). We should detect the protocol using the X-Forwarded-Proto header.

Recommendation:

Use url::form_urlencoded::Serializer to safely encode the query parameters, and inspect the X-Forwarded-Proto header to determine the correct scheme.

pub async fn get_root(
    header_map: HeaderMap,
    Query(mut params): Query<std::collections::HashMap<String, String>>,
) -> Result<Redirect, (StatusCode, String)> {
    const NIGHTLIES_URL: &str = "https://nightlies.apache.org/datafusion/ballista/tui";

    let ballista_scheduler_url =
        params.remove("ballista_scheduler_url").unwrap_or_else(|| {
            let default_scheduler_url = "localhost:50050";
            let scheduler_url = header_map
                .get("host")
                .map(|hv| hv.to_str().unwrap_or(default_scheduler_url))
                .unwrap_or(default_scheduler_url);
            let proto = header_map
                .get("x-forwarded-proto")
                .and_then(|v| v.to_str().ok())
                .unwrap_or("http");
            format!("{proto}://{scheduler_url}")
        });

    let mut query_serializer = url::form_urlencoded::Serializer::new(String::new());
    query_serializer.append_pair("ballista_scheduler_url", &ballista_scheduler_url);
    for (k, v) in params.iter() {
        query_serializer.append_pair(k, v);
    }
    let query_string = query_serializer.finish();

    let target = format!("{NIGHTLIES_URL}/{BALLISTA_VERSION}/?{query_string}");

    Ok(Redirect::temporary(&target))
}


pub async fn get_scheduler_state<
T: AsLogicalPlan + Clone + Send + Sync + 'static,
U: AsExecutionPlan + Send + Sync + 'static,
Expand Down Expand Up @@ -546,7 +577,7 @@ pub async fn get_query_stages<
let metrics = running_stage.stage_metrics.as_deref().unwrap_or(&[]);
summary.stage_plan = Some(match plan_format {
PlanFormat::Default => displayable(running_stage.plan.as_ref()).indent(false).to_string(),
PlanFormat::Tree => displayable(running_stage.plan.as_ref()).tree_render().to_string(),
PlanFormat::Tree => displayable(running_stage.plan.as_ref()).tree_render().to_string(),
PlanFormat::Metrics => format_stage_metrics(running_stage.plan.as_ref(), metrics),
});
summary.input_rows = running_stage
Expand Down Expand Up @@ -591,7 +622,7 @@ pub async fn get_query_stages<
finish_time: info.finish_time as u64,
input_rows,
output_rows,
status: task_status
status: task_status,
}
})
})
Expand All @@ -600,7 +631,7 @@ pub async fn get_query_stages<
ExecutionStage::Successful(completed_stage) => {
summary.stage_plan = Some(match plan_format {
PlanFormat::Default => displayable(completed_stage.plan.as_ref()).indent(false).to_string(),
PlanFormat::Tree => displayable(completed_stage.plan.as_ref()).tree_render().to_string(),
PlanFormat::Tree => displayable(completed_stage.plan.as_ref()).tree_render().to_string(),
PlanFormat::Metrics => format_stage_metrics(completed_stage.plan.as_ref(), &completed_stage.stage_metrics),
});
summary.input_rows = get_combined_count(
Expand Down Expand Up @@ -638,7 +669,7 @@ pub async fn get_query_stages<
finish_time: task_info.finish_time as u64,
input_rows,
output_rows,
status: task_status
status: task_status,
})
})
.collect();
Expand Down Expand Up @@ -908,7 +939,7 @@ pub async fn get_job_dot_graph<
})?
{
ExecutionGraphDot::generate(graph.as_ref())
.map_err(|e| {
.map_err(|e| {
tracing::error!("Error occurred while getting the dot graph for job '{job_id}' reason: {e:?}");
SchedulerErrorResponse::new(StatusCode::INTERNAL_SERVER_ERROR)
})
Expand Down Expand Up @@ -953,10 +984,10 @@ pub async fn get_job_svg_graph<
&mut PrinterContext::default(),
vec![CommandArg::Format(Format::Svg)],
)
.map_err(|e| {
tracing::error!("Error occurred while getting job svg graph for job '{job_id}' reason: {e:?}");
SchedulerErrorResponse::new(StatusCode::INTERNAL_SERVER_ERROR)
})?;
.map_err(|e| {
tracing::error!("Error occurred while getting job svg graph for job '{job_id}' reason: {e:?}");
SchedulerErrorResponse::new(StatusCode::INTERNAL_SERVER_ERROR)
})?;

let svg = String::from_utf8_lossy(&result).to_string();
Ok(Response::builder()
Expand Down
1 change: 1 addition & 0 deletions ballista/scheduler/src/api/routes.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ pub fn get_routes<
scheduler_server: Arc<SchedulerServer<T, U>>,
) -> Router {
let router = Router::new()
.route("/", get(handlers::get_root))
.route("/api/state", get(handlers::get_scheduler_state::<T, U>))
.route("/api/version", get(handlers::get_scheduler_version))
.route("/api/executors", get(handlers::get_executors::<T, U>))
Expand Down
Loading