Skip to content

Commit

Permalink
Authorize compute_ctl requests from the control plane (#10530)
Browse files Browse the repository at this point in the history
The compute should only act if requests come from the control plane.

Signed-off-by: Tristan Partin <[email protected]>

Signed-off-by: Tristan Partin <[email protected]>
  • Loading branch information
tristan957 authored Mar 4, 2025
1 parent 4bbdb75 commit 7b7e4a9
Show file tree
Hide file tree
Showing 14 changed files with 417 additions and 83 deletions.
68 changes: 59 additions & 9 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ aws-credential-types = "1.2.0"
aws-sigv4 = { version = "1.2", features = ["sign-http"] }
aws-types = "1.3"
axum = { version = "0.8.1", features = ["ws"] }
axum-extra = { version = "0.10.0", features = ["typed-header"] }
base64 = "0.13.0"
bincode = "1.3"
bindgen = "0.71"
Expand Down Expand Up @@ -191,7 +192,7 @@ toml = "0.8"
toml_edit = "0.22"
tonic = {version = "0.12.3", default-features = false, features = ["channel", "tls", "tls-roots"]}
tower = { version = "0.5.2", default-features = false }
tower-http = { version = "0.6.2", features = ["request-id", "trace"] }
tower-http = { version = "0.6.2", features = ["auth", "request-id", "trace"] }

# This revision uses opentelemetry 0.27. There's no tag for it.
tower-otel = { git = "https://github.com/mattiapenati/tower-otel", rev = "56a7321053bcb72443888257b622ba0d43a11fcd" }
Expand Down
2 changes: 2 additions & 0 deletions compute_tools/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ aws-sdk-kms.workspace = true
aws-smithy-types.workspace = true
anyhow.workspace = true
axum = { workspace = true, features = [] }
axum-extra.workspace = true
camino.workspace = true
chrono.workspace = true
cfg-if.workspace = true
Expand All @@ -25,6 +26,7 @@ fail.workspace = true
flate2.workspace = true
futures.workspace = true
http.workspace = true
jsonwebtoken.workspace = true
metrics.workspace = true
nix.workspace = true
notify.workspace = true
Expand Down
1 change: 1 addition & 0 deletions compute_tools/src/bin/compute_ctl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,7 @@ fn main() -> Result<()> {
live_config_allowed: cli_spec.live_config_allowed,
},
cli_spec.spec,
cli_spec.compute_ctl_config,
)?;

let exit_code = compute_node.run()?;
Expand Down
24 changes: 20 additions & 4 deletions compute_tools/src/compute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use std::{env, fs};
use anyhow::{Context, Result};
use chrono::{DateTime, Utc};
use compute_api::privilege::Privilege;
use compute_api::responses::{ComputeMetrics, ComputeStatus};
use compute_api::responses::{ComputeCtlConfig, ComputeMetrics, ComputeStatus};
use compute_api::spec::{ComputeFeature, ComputeMode, ComputeSpec, ExtVersion, PgIdent};
use futures::StreamExt;
use futures::future::join_all;
Expand Down Expand Up @@ -132,6 +132,8 @@ pub struct ComputeState {
/// passed by the control plane with a /configure HTTP request.
pub pspec: Option<ParsedSpec>,

pub compute_ctl_config: ComputeCtlConfig,

/// If the spec is passed by a /configure request, 'startup_span' is the
/// /configure request's tracing span. The main thread enters it when it
/// processes the compute startup, so that the compute startup is considered
Expand All @@ -155,6 +157,7 @@ impl ComputeState {
last_active: None,
error: None,
pspec: None,
compute_ctl_config: ComputeCtlConfig::default(),
startup_span: None,
metrics: ComputeMetrics::default(),
}
Expand Down Expand Up @@ -365,7 +368,11 @@ pub(crate) fn construct_superuser_query(spec: &ComputeSpec) -> String {
}

impl ComputeNode {
pub fn new(params: ComputeNodeParams, cli_spec: Option<ComputeSpec>) -> Result<Self> {
pub fn new(
params: ComputeNodeParams,
cli_spec: Option<ComputeSpec>,
compute_ctl_config: ComputeCtlConfig,
) -> Result<Self> {
let connstr = params.connstr.as_str();
let conn_conf = postgres::config::Config::from_str(connstr)
.context("cannot build postgres config from connstr")?;
Expand All @@ -377,6 +384,7 @@ impl ComputeNode {
let pspec = ParsedSpec::try_from(cli_spec).map_err(|msg| anyhow::anyhow!(msg))?;
new_state.pspec = Some(pspec);
}
new_state.compute_ctl_config = compute_ctl_config;

Ok(ComputeNode {
params,
Expand Down Expand Up @@ -405,11 +413,19 @@ impl ComputeNode {

// Launch the external HTTP server first, so that we can serve control plane
// requests while configuration is still in progress.
crate::http::server::Server::External(this.params.external_http_port).launch(&this);
crate::http::server::Server::External {
port: this.params.external_http_port,
jwks: this.state.lock().unwrap().compute_ctl_config.jwks.clone(),
compute_id: this.params.compute_id.clone(),
}
.launch(&this);

// The internal HTTP server could be launched later, but there isn't much
// sense in waiting.
crate::http::server::Server::Internal(this.params.internal_http_port).launch(&this);
crate::http::server::Server::Internal {
port: this.params.internal_http_port,
}
.launch(&this);

// If we got a spec from the CLI already, use that. Otherwise wait for the
// control plane to pass it to us with a /configure HTTP request
Expand Down
2 changes: 2 additions & 0 deletions compute_tools/src/http/extract/mod.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
pub(crate) mod json;
pub(crate) mod path;
pub(crate) mod query;
pub(crate) mod request_id;

pub(crate) use json::Json;
pub(crate) use path::Path;
pub(crate) use query::Query;
pub(crate) use request_id::RequestId;
86 changes: 86 additions & 0 deletions compute_tools/src/http/extract/request_id.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
use std::{
fmt::Display,
ops::{Deref, DerefMut},
};

use axum::{extract::FromRequestParts, response::IntoResponse};
use http::{StatusCode, request::Parts};

use crate::http::{JsonResponse, headers::X_REQUEST_ID};

/// Extract the request ID from the `X-Request-Id` header.
#[derive(Debug, Clone, Default)]
pub(crate) struct RequestId(pub String);

#[derive(Debug)]
/// Rejection used for [`RequestId`].
///
/// Contains one variant for each way the [`RequestId`] extractor can
/// fail.
pub(crate) enum RequestIdRejection {
/// The request is missing the header.
MissingRequestId,

/// The value of the header is invalid UTF-8.
InvalidUtf8,
}

impl RequestIdRejection {
pub fn status(&self) -> StatusCode {
match self {
RequestIdRejection::MissingRequestId => StatusCode::INTERNAL_SERVER_ERROR,
RequestIdRejection::InvalidUtf8 => StatusCode::BAD_REQUEST,
}
}

pub fn message(&self) -> String {
match self {
RequestIdRejection::MissingRequestId => "request ID is missing",
RequestIdRejection::InvalidUtf8 => "request ID is invalid UTF-8",
}
.to_string()
}
}

impl IntoResponse for RequestIdRejection {
fn into_response(self) -> axum::response::Response {
JsonResponse::error(self.status(), self.message())
}
}

impl<S> FromRequestParts<S> for RequestId
where
S: Send + Sync,
{
type Rejection = RequestIdRejection;

async fn from_request_parts(parts: &mut Parts, _state: &S) -> Result<Self, Self::Rejection> {
match parts.headers.get(X_REQUEST_ID) {
Some(value) => match value.to_str() {
Ok(request_id) => Ok(Self(request_id.to_string())),
Err(_) => Err(RequestIdRejection::InvalidUtf8),
},
None => Err(RequestIdRejection::MissingRequestId),
}
}
}

impl Deref for RequestId {
type Target = String;

fn deref(&self) -> &Self::Target {
&self.0
}
}

impl DerefMut for RequestId {
fn deref_mut(&mut self) -> &mut Self::Target {
&mut self.0
}
}

impl Display for RequestId {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.write_str(&self.0)
}
}
2 changes: 2 additions & 0 deletions compute_tools/src/http/headers.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
/// Constant for `X-Request-Id` header.
pub const X_REQUEST_ID: &str = "x-request-id";
Loading

1 comment on commit 7b7e4a9

@github-actions
Copy link

Choose a reason for hiding this comment

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

7940 tests run: 7552 passed, 0 failed, 388 skipped (full report)


Code coverage* (full report)

  • functions: 32.9% (8680 of 26394 functions)
  • lines: 48.8% (73903 of 151436 lines)

* collected from Rust tests only


The comment gets automatically updated with the latest test results
7b7e4a9 at 2025-03-04T20:44:19.108Z :recycle:

Please sign in to comment.