From 0cf9157adc141d487b5f7cb28afb9b6e3c1e8dee Mon Sep 17 00:00:00 2001 From: Tristan Partin Date: Thu, 13 Feb 2025 12:04:36 -0600 Subject: [PATCH] Handle new compute_ctl_config parameter in compute spec requests (#10746) There is now a compute_ctl_config field in the response that currently only contains a JSON Web Key set. compute_ctl currently doesn't do anything with the keys, but will in the future. The reasoning for the new field is due to the nature of empty computes. When an empty compute is created, it does not have a tenant. A compute spec is the primary means of communicating the details of an attached tenant. In the empty compute state, there is no spec. Instead we wait for the control plane to pass us one via /configure. If we were to include the jwks field in the compute spec, we would have a partial compute spec, which doesn't logically make sense. Instead, we can have two means of passing settings to the compute: - spec: tenant specific config details - compute_ctl_config: compute specific settings For instance, the JSON Web Key set passed to the compute is independent of any tenant. It is a setting of the compute whether it is attached or not. Signed-off-by: Tristan Partin --- Cargo.lock | 2 ++ compute_tools/Cargo.toml | 1 + compute_tools/src/bin/compute_ctl.rs | 12 +++++++++--- compute_tools/src/spec.rs | 21 ++++++++++++--------- control_plane/src/endpoint.rs | 13 +++++++++---- libs/compute_api/Cargo.toml | 1 + libs/compute_api/src/requests.rs | 6 ++++-- libs/compute_api/src/responses.rs | 19 +++++++++++++++++-- 8 files changed, 55 insertions(+), 20 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b3a88d46ac..86d9603d36 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1293,6 +1293,7 @@ version = "0.1.0" dependencies = [ "anyhow", "chrono", + "jsonwebtoken", "regex", "remote_storage", "serde", @@ -1320,6 +1321,7 @@ dependencies = [ "flate2", "futures", "http 1.1.0", + "jsonwebtoken", "metrics", "nix 0.27.1", "notify", diff --git a/compute_tools/Cargo.toml b/compute_tools/Cargo.toml index b04f364cbb..b8828fa49f 100644 --- a/compute_tools/Cargo.toml +++ b/compute_tools/Cargo.toml @@ -24,6 +24,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 diff --git a/compute_tools/src/bin/compute_ctl.rs b/compute_tools/src/bin/compute_ctl.rs index df47adda6c..a8803ec793 100644 --- a/compute_tools/src/bin/compute_ctl.rs +++ b/compute_tools/src/bin/compute_ctl.rs @@ -55,7 +55,7 @@ use signal_hook::{consts::SIGINT, iterator::Signals}; use tracing::{error, info, warn}; use url::Url; -use compute_api::responses::ComputeStatus; +use compute_api::responses::{ComputeCtlConfig, ComputeStatus}; use compute_api::spec::ComputeSpec; use compute_tools::compute::{ @@ -281,6 +281,7 @@ fn try_spec_from_cli(cli: &Cli) -> Result { info!("got spec from cli argument {}", spec_json); return Ok(CliSpecParams { spec: Some(serde_json::from_str(spec_json)?), + compute_ctl_config: ComputeCtlConfig::default(), live_config_allowed: false, }); } @@ -290,6 +291,7 @@ fn try_spec_from_cli(cli: &Cli) -> Result { let file = File::open(Path::new(spec_path))?; return Ok(CliSpecParams { spec: Some(serde_json::from_reader(file)?), + compute_ctl_config: ComputeCtlConfig::default(), live_config_allowed: true, }); } @@ -299,8 +301,9 @@ fn try_spec_from_cli(cli: &Cli) -> Result { }; match get_spec_from_control_plane(cli.control_plane_uri.as_ref().unwrap(), &cli.compute_id) { - Ok(spec) => Ok(CliSpecParams { - spec, + Ok(resp) => Ok(CliSpecParams { + spec: resp.0, + compute_ctl_config: resp.1, live_config_allowed: true, }), Err(e) => { @@ -317,6 +320,8 @@ fn try_spec_from_cli(cli: &Cli) -> Result { struct CliSpecParams { /// If a spec was provided via CLI or file, the [`ComputeSpec`] spec: Option, + #[allow(dead_code)] + compute_ctl_config: ComputeCtlConfig, live_config_allowed: bool, } @@ -326,6 +331,7 @@ fn wait_spec( CliSpecParams { spec, live_config_allowed, + compute_ctl_config: _, }: CliSpecParams, ) -> Result> { let mut new_state = ComputeState::new(); diff --git a/compute_tools/src/spec.rs b/compute_tools/src/spec.rs index 73950cd95a..6f28bd9733 100644 --- a/compute_tools/src/spec.rs +++ b/compute_tools/src/spec.rs @@ -11,7 +11,9 @@ use crate::migration::MigrationRunner; use crate::params::PG_HBA_ALL_MD5; use crate::pg_helpers::*; -use compute_api::responses::{ControlPlaneComputeStatus, ControlPlaneSpecResponse}; +use compute_api::responses::{ + ComputeCtlConfig, ControlPlaneComputeStatus, ControlPlaneSpecResponse, +}; use compute_api::spec::ComputeSpec; // Do control plane request and return response if any. In case of error it @@ -73,14 +75,13 @@ fn do_control_plane_request( pub fn get_spec_from_control_plane( base_uri: &str, compute_id: &str, -) -> Result> { +) -> Result<(Option, ComputeCtlConfig)> { let cp_uri = format!("{base_uri}/compute/api/v2/computes/{compute_id}/spec"); let jwt: String = match std::env::var("NEON_CONTROL_PLANE_TOKEN") { Ok(v) => v, Err(_) => "".to_string(), }; let mut attempt = 1; - let mut spec: Result> = Ok(None); info!("getting spec from control plane: {}", cp_uri); @@ -90,7 +91,7 @@ pub fn get_spec_from_control_plane( // - no spec for compute yet (Empty state) -> return Ok(None) // - got spec -> return Ok(Some(spec)) while attempt < 4 { - spec = match do_control_plane_request(&cp_uri, &jwt) { + let result = match do_control_plane_request(&cp_uri, &jwt) { Ok(spec_resp) => { CPLANE_REQUESTS_TOTAL .with_label_values(&[ @@ -99,10 +100,10 @@ pub fn get_spec_from_control_plane( ]) .inc(); match spec_resp.status { - ControlPlaneComputeStatus::Empty => Ok(None), + ControlPlaneComputeStatus::Empty => Ok((None, spec_resp.compute_ctl_config)), ControlPlaneComputeStatus::Attached => { if let Some(spec) = spec_resp.spec { - Ok(Some(spec)) + Ok((Some(spec), spec_resp.compute_ctl_config)) } else { bail!("compute is attached, but spec is empty") } @@ -121,10 +122,10 @@ pub fn get_spec_from_control_plane( } }; - if let Err(e) = &spec { + if let Err(e) = &result { error!("attempt {} to get spec failed with: {}", attempt, e); } else { - return spec; + return result; } attempt += 1; @@ -132,7 +133,9 @@ pub fn get_spec_from_control_plane( } // All attempts failed, return error. - spec + Err(anyhow::anyhow!( + "Exhausted all attempts to retrieve the spec from the control plane" + )) } /// Check `pg_hba.conf` and update if needed to allow external connections. diff --git a/control_plane/src/endpoint.rs b/control_plane/src/endpoint.rs index 3b2634204c..c3c8229c38 100644 --- a/control_plane/src/endpoint.rs +++ b/control_plane/src/endpoint.rs @@ -48,6 +48,8 @@ use std::sync::Arc; use std::time::Duration; use anyhow::{anyhow, bail, Context, Result}; +use compute_api::requests::ConfigurationRequest; +use compute_api::responses::ComputeCtlConfig; use compute_api::spec::Database; use compute_api::spec::PgIdent; use compute_api::spec::RemoteExtSpec; @@ -880,10 +882,13 @@ impl Endpoint { self.external_http_address.port() )) .header(CONTENT_TYPE.as_str(), "application/json") - .body(format!( - "{{\"spec\":{}}}", - serde_json::to_string_pretty(&spec)? - )) + .body( + serde_json::to_string(&ConfigurationRequest { + spec, + compute_ctl_config: ComputeCtlConfig::default(), + }) + .unwrap(), + ) .send() .await?; diff --git a/libs/compute_api/Cargo.toml b/libs/compute_api/Cargo.toml index c0ec40a6c2..c11a1b6688 100644 --- a/libs/compute_api/Cargo.toml +++ b/libs/compute_api/Cargo.toml @@ -7,6 +7,7 @@ license.workspace = true [dependencies] anyhow.workspace = true chrono.workspace = true +jsonwebtoken.workspace = true serde.workspace = true serde_json.workspace = true regex.workspace = true diff --git a/libs/compute_api/src/requests.rs b/libs/compute_api/src/requests.rs index fc3757d981..0c256cae2e 100644 --- a/libs/compute_api/src/requests.rs +++ b/libs/compute_api/src/requests.rs @@ -1,18 +1,20 @@ //! Structs representing the JSON formats used in the compute_ctl's HTTP API. use crate::{ privilege::Privilege, + responses::ComputeCtlConfig, spec::{ComputeSpec, ExtVersion, PgIdent}, }; -use serde::Deserialize; +use serde::{Deserialize, Serialize}; /// Request of the /configure API /// /// We now pass only `spec` in the configuration request, but later we can /// extend it and something like `restart: bool` or something else. So put /// `spec` into a struct initially to be more flexible in the future. -#[derive(Deserialize, Debug)] +#[derive(Debug, Deserialize, Serialize)] pub struct ConfigurationRequest { pub spec: ComputeSpec, + pub compute_ctl_config: ComputeCtlConfig, } #[derive(Deserialize, Debug)] diff --git a/libs/compute_api/src/responses.rs b/libs/compute_api/src/responses.rs index 5286e0e61d..a6248019d9 100644 --- a/libs/compute_api/src/responses.rs +++ b/libs/compute_api/src/responses.rs @@ -3,6 +3,7 @@ use std::fmt::Display; use chrono::{DateTime, Utc}; +use jsonwebtoken::jwk::JwkSet; use serde::{Deserialize, Serialize, Serializer}; use crate::{ @@ -135,13 +136,27 @@ pub struct CatalogObjects { pub databases: Vec, } +#[derive(Debug, Deserialize, Serialize)] +pub struct ComputeCtlConfig { + pub jwks: JwkSet, +} + +impl Default for ComputeCtlConfig { + fn default() -> Self { + Self { + jwks: JwkSet { + keys: Vec::default(), + }, + } + } +} + /// Response of the `/computes/{compute_id}/spec` control-plane API. -/// This is not actually a compute API response, so consider moving -/// to a different place. #[derive(Deserialize, Debug)] pub struct ControlPlaneSpecResponse { pub spec: Option, pub status: ControlPlaneComputeStatus, + pub compute_ctl_config: ComputeCtlConfig, } #[derive(Deserialize, Clone, Copy, Debug, PartialEq, Eq)]