From 8fa550c2c95b5fd3443af7e2e5837c2178fdb8c8 Mon Sep 17 00:00:00 2001 From: James MacMahon Date: Thu, 7 Dec 2023 18:10:46 -0500 Subject: [PATCH] Remove URL based image creation and import (#2898) Remove both params::ImageSource::Url and params::ImportBlocksFromUrl (along with associated HTTP endpoint): do not allow customers to create an image from a URL, that was for development purposes only. Now that Nexus supports importing blocks via the Pantry this is no longer required. Closes #2893 Co-authored-by: iliana etaoin --- .github/buildomat/jobs/deploy.sh | 51 ++- Cargo.lock | 4 + end-to-end-tests/Cargo.toml | 4 + end-to-end-tests/src/bin/bootstrap.rs | 68 ++- end-to-end-tests/src/helpers/ctx.rs | 155 ++++--- end-to-end-tests/src/instance_launch.rs | 34 +- nexus/db-model/src/image.rs | 1 - nexus/src/app/disk.rs | 26 -- nexus/src/app/image.rs | 116 ----- nexus/src/app/sagas/import_blocks_from_url.rs | 413 ------------------ nexus/src/app/sagas/mod.rs | 2 - nexus/src/external_api/http_entrypoints.rs | 34 -- nexus/tests/integration_tests/endpoints.rs | 21 +- nexus/tests/integration_tests/images.rs | 315 ++----------- nexus/tests/integration_tests/instances.rs | 19 +- nexus/tests/integration_tests/pantry.rs | 46 -- nexus/tests/integration_tests/snapshots.rs | 53 +-- .../integration_tests/volume_management.rs | 18 +- nexus/tests/output/nexus_tags.txt | 1 - nexus/types/src/external_api/params.rs | 15 - nexus/types/src/external_api/views.rs | 3 - openapi/nexus.json | 119 ----- 22 files changed, 242 insertions(+), 1276 deletions(-) delete mode 100644 nexus/src/app/sagas/import_blocks_from_url.rs diff --git a/.github/buildomat/jobs/deploy.sh b/.github/buildomat/jobs/deploy.sh index 3c4b3d88c8..f4f1e0a999 100755 --- a/.github/buildomat/jobs/deploy.sh +++ b/.github/buildomat/jobs/deploy.sh @@ -281,19 +281,15 @@ rmdir pkg E2E_TLS_CERT="/opt/oxide/sled-agent/pkg/initial-tls-cert.pem" # -# Image-related tests use images served by catacomb. The lab network is -# IPv4-only; the propolis zones are IPv6-only. These steps set up tcpproxy -# configured to proxy to catacomb via port 54321 in the global zone. +# Download the Oxide CLI and images from catacomb. # pfexec mkdir -p /usr/oxide -pfexec rm -f /usr/oxide/tcpproxy -pfexec curl -sSfL -o /usr/oxide/tcpproxy \ - http://catacomb.eng.oxide.computer:12346/tcpproxy -pfexec chmod +x /usr/oxide/tcpproxy -pfexec rm -f /var/svc/manifest/site/tcpproxy.xml -pfexec curl -sSfL -o /var/svc/manifest/site/tcpproxy.xml \ - http://catacomb.eng.oxide.computer:12346/tcpproxy.xml -pfexec svccfg import /var/svc/manifest/site/tcpproxy.xml +pfexec curl -sSfL -o /usr/oxide/oxide \ + http://catacomb.eng.oxide.computer:12346/oxide-v0.1.0 +pfexec chmod +x /usr/oxide/oxide + +curl -sSfL -o debian-11-genericcloud-amd64.raw \ + http://catacomb.eng.oxide.computer:12346/debian-11-genericcloud-amd64.raw # # The lab-netdev target is a ramdisk system that is always cleared @@ -336,7 +332,38 @@ echo "Waited for chrony: ${retry}s" export RUST_BACKTRACE=1 export E2E_TLS_CERT IPPOOL_START IPPOOL_END -./tests/bootstrap +eval "$(./tests/bootstrap)" +export OXIDE_HOST OXIDE_TOKEN + +# +# The Nexus resolved in `$OXIDE_RESOLVE` is not necessarily the same one that we +# successfully talked to in bootstrap, so wait a bit for it to fully come online. +# +retry=0 +while ! curl -sSf "$OXIDE_HOST/v1/ping" --resolve "$OXIDE_RESOLVE" --cacert "$E2E_TLS_CERT"; do + if [[ $retry -gt 60 ]]; then + echo "$OXIDE_RESOLVE failed to come up after 60 seconds" + exit 1 + fi + sleep 1 + retry=$((retry + 1)) +done + +/usr/oxide/oxide --resolve "$OXIDE_RESOLVE" --cacert "$E2E_TLS_CERT" \ + project create --name images --description "some images" +/usr/oxide/oxide --resolve "$OXIDE_RESOLVE" --cacert "$E2E_TLS_CERT" \ + disk import \ + --path debian-11-genericcloud-amd64.raw \ + --disk debian11-boot \ + --project images \ + --description "debian 11 cloud image from distros" \ + --snapshot debian11-snapshot \ + --image debian11 \ + --image-description "debian 11 original base image" \ + --image-os debian \ + --image-version "11" +/usr/oxide/oxide --resolve "$OXIDE_RESOLVE" --cacert "$E2E_TLS_CERT" \ + image promote --project images --image debian11 rm ./tests/bootstrap for test_bin in tests/*; do diff --git a/Cargo.lock b/Cargo.lock index 52a16b414e..ed988f4b14 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2089,6 +2089,7 @@ dependencies = [ "base64", "chrono", "http", + "hyper", "omicron-sled-agent", "omicron-test-utils", "omicron-workspace-hack", @@ -2097,9 +2098,12 @@ dependencies = [ "reqwest", "russh", "russh-keys", + "serde", + "serde_json", "tokio", "toml 0.8.8", "trust-dns-resolver", + "uuid", ] [[package]] diff --git a/end-to-end-tests/Cargo.toml b/end-to-end-tests/Cargo.toml index 66e1a58a2c..8a1f91eee8 100644 --- a/end-to-end-tests/Cargo.toml +++ b/end-to-end-tests/Cargo.toml @@ -10,6 +10,7 @@ async-trait.workspace = true base64.workspace = true chrono.workspace = true http.workspace = true +hyper.workspace = true omicron-sled-agent.workspace = true omicron-test-utils.workspace = true oxide-client.workspace = true @@ -17,7 +18,10 @@ rand.workspace = true reqwest.workspace = true russh = "0.40.0" russh-keys = "0.40.0" +serde.workspace = true +serde_json.workspace = true tokio = { workspace = true, features = ["macros", "rt-multi-thread"] } toml.workspace = true trust-dns-resolver.workspace = true +uuid.workspace = true omicron-workspace-hack.workspace = true diff --git a/end-to-end-tests/src/bin/bootstrap.rs b/end-to-end-tests/src/bin/bootstrap.rs index c9001937db..83a37b8c21 100644 --- a/end-to-end-tests/src/bin/bootstrap.rs +++ b/end-to-end-tests/src/bin/bootstrap.rs @@ -1,18 +1,23 @@ use anyhow::Result; -use end_to_end_tests::helpers::ctx::{build_client, Context}; +use end_to_end_tests::helpers::ctx::{ClientParams, Context}; use end_to_end_tests::helpers::{generate_name, get_system_ip_pool}; use omicron_test_utils::dev::poll::{wait_for_condition, CondCheckError}; use oxide_client::types::{ - ByteCount, DiskCreate, DiskSource, IpRange, Ipv4Range, + ByteCount, DeviceAccessTokenRequest, DeviceAuthRequest, DeviceAuthVerify, + DiskCreate, DiskSource, IpRange, Ipv4Range, }; use oxide_client::{ - ClientDisksExt, ClientProjectsExt, ClientSystemNetworkingExt, + ClientDisksExt, ClientHiddenExt, ClientProjectsExt, + ClientSystemNetworkingExt, }; +use serde::{de::DeserializeOwned, Deserialize}; use std::time::Duration; +use uuid::Uuid; #[tokio::main] async fn main() -> Result<()> { - let client = build_client().await?; + let params = ClientParams::new()?; + let client = params.build_client().await?; // ===== ENSURE NEXUS IS UP ===== // eprintln!("waiting for nexus to come up..."); @@ -71,8 +76,61 @@ async fn main() -> Result<()> { .disk(disk_name) .send() .await?; - ctx.cleanup().await?; + // ===== PRINT CLI ENVIRONMENT ===== // + let client_id = Uuid::new_v4(); + let DeviceAuthResponse { device_code, user_code } = + deserialize_byte_stream( + ctx.client + .device_auth_request() + .body(DeviceAuthRequest { client_id }) + .send() + .await?, + ) + .await?; + ctx.client + .device_auth_confirm() + .body(DeviceAuthVerify { user_code }) + .send() + .await?; + let DeviceAccessTokenGrant { access_token } = deserialize_byte_stream( + ctx.client + .device_access_token() + .body(DeviceAccessTokenRequest { + client_id, + device_code, + grant_type: "urn:ietf:params:oauth:grant-type:device_code" + .to_string(), + }) + .send() + .await?, + ) + .await?; + + println!("OXIDE_HOST={}", params.base_url()); + println!("OXIDE_RESOLVE={}", params.resolve_nexus().await?); + println!("OXIDE_TOKEN={}", access_token); + + ctx.cleanup().await?; eprintln!("let's roll."); Ok(()) } + +async fn deserialize_byte_stream( + response: oxide_client::ResponseValue, +) -> Result { + let body = hyper::Body::wrap_stream(response.into_inner_stream()); + let bytes = hyper::body::to_bytes(body).await?; + Ok(serde_json::from_slice(&bytes)?) +} + +#[derive(Deserialize)] +struct DeviceAuthResponse { + device_code: String, + user_code: String, +} + +#[derive(Deserialize)] +struct DeviceAccessTokenGrant { + access_token: String, +} diff --git a/end-to-end-tests/src/helpers/ctx.rs b/end-to-end-tests/src/helpers/ctx.rs index 1c95703807..2c66bd4724 100644 --- a/end-to-end-tests/src/helpers/ctx.rs +++ b/end-to-end-tests/src/helpers/ctx.rs @@ -5,7 +5,8 @@ use omicron_sled_agent::rack_setup::config::SetupServiceConfig; use omicron_test_utils::dev::poll::{wait_for_condition, CondCheckError}; use oxide_client::types::{Name, ProjectCreate}; use oxide_client::CustomDnsResolver; -use oxide_client::{Client, ClientProjectsExt, ClientVpcsExt}; +use oxide_client::{Client, ClientImagesExt, ClientProjectsExt, ClientVpcsExt}; +use reqwest::dns::Resolve; use reqwest::header::{HeaderMap, HeaderValue}; use reqwest::Url; use std::net::IpAddr; @@ -13,6 +14,7 @@ use std::net::SocketAddr; use std::sync::Arc; use std::time::Duration; use trust_dns_resolver::error::ResolveErrorKind; +use uuid::Uuid; const RSS_CONFIG_STR: &str = include_str!(concat!( env!("CARGO_MANIFEST_DIR"), @@ -30,7 +32,7 @@ pub struct Context { impl Context { pub async fn new() -> Result { - Context::from_client(build_client().await?).await + Context::from_client(ClientParams::new()?.build_client().await?).await } pub async fn from_client(client: Client) -> Result { @@ -48,6 +50,10 @@ impl Context { Ok(Context { client, project_name }) } + pub async fn get_silo_image_id(&self, name: &str) -> Result { + Ok(self.client.image_view().image(name).send().await?.id) + } + pub async fn cleanup(self) -> Result<()> { self.client .vpc_subnet_delete() @@ -179,6 +185,27 @@ impl ClientParams { format!("{}://{}", self.proto, self.nexus_dns_name) } + pub async fn resolve_nexus(&self) -> Result { + let address = self + .resolver + .resolve(self.nexus_dns_name.parse()?) + .await + .map_err(anyhow::Error::msg)? + .next() + .with_context(|| { + format!( + "{} did not resolve to any addresses", + self.nexus_dns_name + ) + })?; + let port = match self.proto { + "http" => 80, + "https" => 443, + _ => unreachable!(), + }; + Ok(format!("{}:{}:{}", self.nexus_dns_name, port, address.ip())) + } + pub fn reqwest_builder(&self) -> reqwest::ClientBuilder { let mut builder = reqwest::ClientBuilder::new().dns_resolver(self.resolver.clone()); @@ -189,77 +216,77 @@ impl ClientParams { builder } -} -pub async fn build_client() -> Result { - // Prepare to make a login request. - let client_params = ClientParams::new()?; - let config = &client_params.rss_config; - let base_url = client_params.base_url(); - let silo_name = config.recovery_silo.silo_name.as_str(); - let login_url = format!("{}/v1/login/{}/local", base_url, silo_name); - let username: oxide_client::types::UserId = - config.recovery_silo.user_name.as_str().parse().map_err(|s| { - anyhow!("parsing configured recovery user name: {:?}", s) - })?; - // See the comment in the config file about this password. - let password: oxide_client::types::Password = "oxide".parse().unwrap(); + pub async fn build_client(&self) -> Result { + // Prepare to make a login request. + let config = &self.rss_config; + let base_url = self.base_url(); + let silo_name = config.recovery_silo.silo_name.as_str(); + let login_url = format!("{}/v1/login/{}/local", base_url, silo_name); + let username: oxide_client::types::UserId = + config.recovery_silo.user_name.as_str().parse().map_err(|s| { + anyhow!("parsing configured recovery user name: {:?}", s) + })?; + // See the comment in the config file about this password. + let password: oxide_client::types::Password = "oxide".parse().unwrap(); - // By the time we get here, Nexus might not be up yet. It may not have - // published its names to external DNS, and even if it has, it may not have - // opened its external listening socket. So we have to retry a bit until we - // succeed. - let session_token = wait_for_condition( - || async { - // Use a raw reqwest client because it's not clear that Progenitor - // is intended to support endpoints that return 300-level response - // codes. See progenitor#451. - eprintln!("{}: attempting to log into API", Utc::now()); + // By the time we get here, Nexus might not be up yet. It may not have + // published its names to external DNS, and even if it has, it may not have + // opened its external listening socket. So we have to retry a bit until we + // succeed. + let session_token = wait_for_condition( + || async { + // Use a raw reqwest client because it's not clear that Progenitor + // is intended to support endpoints that return 300-level response + // codes. See progenitor#451. + eprintln!("{}: attempting to log into API", Utc::now()); - let builder = client_params - .reqwest_builder() - .connect_timeout(Duration::from_secs(15)) - .timeout(Duration::from_secs(60)); + let builder = self + .reqwest_builder() + .connect_timeout(Duration::from_secs(15)) + .timeout(Duration::from_secs(60)); - oxide_client::login( - builder, - &login_url, - username.clone(), - password.clone(), - ) - .await - .map_err(|e| { - eprintln!("{}: login failed: {:#}", Utc::now(), e); - if let oxide_client::LoginError::RequestError(e) = &e { - if e.is_connect() { - return CondCheckError::NotYet; + oxide_client::login( + builder, + &login_url, + username.clone(), + password.clone(), + ) + .await + .map_err(|e| { + eprintln!("{}: login failed: {:#}", Utc::now(), e); + if let oxide_client::LoginError::RequestError(e) = &e { + if e.is_connect() { + return CondCheckError::NotYet; + } } - } - CondCheckError::Failed(e) - }) - }, - &Duration::from_secs(1), - &Duration::from_secs(600), - ) - .await - .context("logging in")?; + CondCheckError::Failed(e) + }) + }, + &Duration::from_secs(1), + &Duration::from_secs(600), + ) + .await + .context("logging in")?; - eprintln!("{}: login succeeded", Utc::now()); + eprintln!("{}: login succeeded", Utc::now()); - let mut headers = HeaderMap::new(); - headers.insert( - http::header::COOKIE, - HeaderValue::from_str(&format!("session={}", session_token)).unwrap(), - ); + let mut headers = HeaderMap::new(); + headers.insert( + http::header::COOKIE, + HeaderValue::from_str(&format!("session={}", session_token)) + .unwrap(), + ); - let reqwest_client = client_params - .reqwest_builder() - .default_headers(headers) - .connect_timeout(Duration::from_secs(15)) - .timeout(Duration::from_secs(60)) - .build()?; - Ok(Client::new_with_client(&base_url, reqwest_client)) + let reqwest_client = self + .reqwest_builder() + .default_headers(headers) + .connect_timeout(Duration::from_secs(15)) + .timeout(Duration::from_secs(60)) + .build()?; + Ok(Client::new_with_client(&base_url, reqwest_client)) + } } async fn wait_for_records( diff --git a/end-to-end-tests/src/instance_launch.rs b/end-to-end-tests/src/instance_launch.rs index 30ccd0d4a3..b3d1406070 100644 --- a/end-to-end-tests/src/instance_launch.rs +++ b/end-to-end-tests/src/instance_launch.rs @@ -5,13 +5,11 @@ use anyhow::{ensure, Context as _, Result}; use async_trait::async_trait; use omicron_test_utils::dev::poll::{wait_for_condition, CondCheckError}; use oxide_client::types::{ - ByteCount, DiskCreate, DiskSource, ExternalIpCreate, ImageCreate, - ImageSource, InstanceCpuCount, InstanceCreate, InstanceDiskAttachment, - InstanceNetworkInterfaceAttachment, SshKeyCreate, -}; -use oxide_client::{ - ClientDisksExt, ClientImagesExt, ClientInstancesExt, ClientSessionExt, + ByteCount, DiskCreate, DiskSource, ExternalIpCreate, InstanceCpuCount, + InstanceCreate, InstanceDiskAttachment, InstanceNetworkInterfaceAttachment, + SshKeyCreate, }; +use oxide_client::{ClientDisksExt, ClientInstancesExt, ClientSessionExt}; use russh::{ChannelMsg, Disconnect}; use russh_keys::key::{KeyPair, PublicKey}; use russh_keys::PublicKeyBase64; @@ -38,26 +36,6 @@ async fn instance_launch() -> Result<()> { .send() .await?; - eprintln!("create system image"); - let image_id = ctx - .client - .image_create() - .body(ImageCreate { - name: generate_name("debian")?, - description: String::new(), - os: "debian".try_into().map_err(anyhow::Error::msg)?, - version: "propolis-blob".into(), - source: ImageSource::Url { - url: - "http://[fd00:1122:3344:101::1]:54321/debian-11-genericcloud-amd64.raw" - .into(), - block_size: 512.try_into().map_err(anyhow::Error::msg)?, - }, - }) - .send() - .await? - .id; - eprintln!("create disk"); let disk_name = generate_name("disk")?; let disk_name = ctx @@ -67,7 +45,9 @@ async fn instance_launch() -> Result<()> { .body(DiskCreate { name: disk_name.clone(), description: String::new(), - disk_source: DiskSource::Image { image_id }, + disk_source: DiskSource::Image { + image_id: ctx.get_silo_image_id("debian11").await?, + }, size: ByteCount(2048 * 1024 * 1024), }) .send() diff --git a/nexus/db-model/src/image.rs b/nexus/db-model/src/image.rs index 91a9469d30..6cdf3201be 100644 --- a/nexus/db-model/src/image.rs +++ b/nexus/db-model/src/image.rs @@ -202,7 +202,6 @@ impl From for views::Image { Self { identity: image.identity(), project_id: image.project_id, - url: image.url, os: image.os, version: image.version, digest: image.digest.map(|x| x.into()), diff --git a/nexus/src/app/disk.rs b/nexus/src/app/disk.rs index 28d6c4506c..5cfecc9f08 100644 --- a/nexus/src/app/disk.rs +++ b/nexus/src/app/disk.rs @@ -369,32 +369,6 @@ impl super::Nexus { Ok(()) } - /// Import blocks from a URL into a disk - pub(crate) async fn import_blocks_from_url_for_disk( - self: &Arc, - opctx: &OpContext, - disk_lookup: &lookup::Disk<'_>, - params: params::ImportBlocksFromUrl, - ) -> UpdateResult<()> { - let authz_disk: authz::Disk; - - (.., authz_disk) = - disk_lookup.lookup_for(authz::Action::Modify).await?; - - let saga_params = sagas::import_blocks_from_url::Params { - serialized_authn: authn::saga::Serialized::for_opctx(opctx), - disk_id: authz_disk.id(), - - import_params: params.clone(), - }; - - self - .execute_saga::(saga_params) - .await?; - - Ok(()) - } - /// Move a disk from the "ImportReady" state to the "Importing" state, /// blocking any import from URL jobs. pub(crate) async fn disk_manual_import_start( diff --git a/nexus/src/app/image.rs b/nexus/src/app/image.rs index 8fa9308c1d..5e78b2a096 100644 --- a/nexus/src/app/image.rs +++ b/nexus/src/app/image.rs @@ -23,7 +23,6 @@ use omicron_common::api::external::ListResultVec; use omicron_common::api::external::LookupResult; use omicron_common::api::external::NameOrId; use omicron_common::api::external::UpdateResult; -use std::str::FromStr; use std::sync::Arc; use uuid::Uuid; @@ -96,121 +95,6 @@ impl super::Nexus { } }; let new_image = match ¶ms.source { - params::ImageSource::Url { url, block_size } => { - let db_block_size = db::model::BlockSize::try_from(*block_size) - .map_err(|e| Error::InvalidValue { - label: String::from("block_size"), - message: format!("block_size is invalid: {}", e), - })?; - - let image_id = Uuid::new_v4(); - - let volume_construction_request = - sled_agent_client::types::VolumeConstructionRequest::Url { - id: image_id, - block_size: db_block_size.to_bytes().into(), - url: url.clone(), - }; - - let volume_data = - serde_json::to_string(&volume_construction_request)?; - - // use reqwest to query url for size - let dur = std::time::Duration::from_secs(5); - let client = reqwest::ClientBuilder::new() - .connect_timeout(dur) - .timeout(dur) - .build() - .map_err(|e| { - Error::internal_error(&format!( - "failed to build reqwest client: {}", - e - )) - })?; - - let response = client.head(url).send().await.map_err(|e| { - Error::InvalidValue { - label: String::from("url"), - message: format!("error querying url: {}", e), - } - })?; - - if !response.status().is_success() { - return Err(Error::InvalidValue { - label: String::from("url"), - message: format!( - "querying url returned: {}", - response.status() - ), - }); - } - - // grab total size from content length - let content_length = response - .headers() - .get(reqwest::header::CONTENT_LENGTH) - .ok_or("no content length!") - .map_err(|e| Error::InvalidValue { - label: String::from("url"), - message: format!("error querying url: {}", e), - })?; - - let total_size = - u64::from_str(content_length.to_str().map_err(|e| { - Error::InvalidValue { - label: String::from("url"), - message: format!("content length invalid: {}", e), - } - })?) - .map_err(|e| { - Error::InvalidValue { - label: String::from("url"), - message: format!("content length invalid: {}", e), - } - })?; - - let size: external::ByteCount = total_size.try_into().map_err( - |e: external::ByteCountRangeError| Error::InvalidValue { - label: String::from("size"), - message: format!("total size is invalid: {}", e), - }, - )?; - - // validate total size is divisible by block size - let block_size: u64 = (*block_size).into(); - if (size.to_bytes() % block_size) != 0 { - return Err(Error::InvalidValue { - label: String::from("size"), - message: format!( - "total size {} must be divisible by block size {}", - size.to_bytes(), - block_size - ), - }); - } - - let new_image_volume = - db::model::Volume::new(Uuid::new_v4(), volume_data); - let volume = - self.db_datastore.volume_create(new_image_volume).await?; - - db::model::Image { - identity: db::model::ImageIdentity::new( - image_id, - params.identity.clone(), - ), - silo_id: authz_silo.id(), - project_id: maybe_authz_project.clone().map(|p| p.id()), - volume_id: volume.id(), - url: Some(url.clone()), - os: params.os.clone(), - version: params.version.clone(), - digest: None, // not computed for URL type - block_size: db_block_size, - size: size.into(), - } - } - params::ImageSource::Snapshot { id } => { let image_id = Uuid::new_v4(); diff --git a/nexus/src/app/sagas/import_blocks_from_url.rs b/nexus/src/app/sagas/import_blocks_from_url.rs deleted file mode 100644 index ffee40ba72..0000000000 --- a/nexus/src/app/sagas/import_blocks_from_url.rs +++ /dev/null @@ -1,413 +0,0 @@ -// This Source Code Form is subject to the terms of the Mozilla Public -// License, v. 2.0. If a copy of the MPL was not distributed with this -// file, You can obtain one at https://mozilla.org/MPL/2.0/. - -//! For disks in state ImportReady, send a request to import blocks from a URL. -//! Note the Pantry they're attached to must have addressability to the URL! - -use super::declare_saga_actions; -use super::ActionRegistry; -use super::NexusActionContext; -use super::NexusSaga; -use super::SagaInitError; -use crate::app::sagas::retry_until_known_result; -use nexus_db_model::Generation; -use nexus_db_queries::db::lookup::LookupPath; -use nexus_db_queries::{authn, authz}; -use nexus_types::external_api::params; -use omicron_common::api::external; -use omicron_common::api::external::Error; -use serde::Deserialize; -use serde::Serialize; -use std::net::SocketAddrV6; -use steno::ActionError; -use uuid::Uuid; - -#[derive(Debug, Deserialize, Serialize)] -pub(crate) struct Params { - pub serialized_authn: authn::saga::Serialized, - pub disk_id: Uuid, - - pub import_params: params::ImportBlocksFromUrl, -} - -declare_saga_actions! { - import_blocks_from_url; - SET_IMPORTING_STATE -> "disk_generation_number" { - + sibfu_get_importing_state - - sibfu_get_importing_state_undo - } - GET_PANTRY_ADDRESS -> "pantry_address" { - + sibfu_get_pantry_address - } - CALL_PANTRY_IMPORT_FROM_URL_FOR_DISK -> "call_pantry_import_from_url_for_disk" { - + sibfu_call_pantry_import_from_url_for_disk - } - WAIT_FOR_IMPORT_FROM_URL -> "wait_for_import_from_url" { - + sibfu_wait_for_import_from_url - } - SET_IMPORT_READY_STATE -> "set_import_ready_state" { - + sibfu_get_import_ready_state - } -} - -#[derive(Debug)] -pub(crate) struct SagaImportBlocksFromUrl; -impl NexusSaga for SagaImportBlocksFromUrl { - const NAME: &'static str = "import-blocks-from-url"; - type Params = Params; - - fn register_actions(registry: &mut ActionRegistry) { - import_blocks_from_url_register_actions(registry); - } - - fn make_saga_dag( - _params: &Self::Params, - mut builder: steno::DagBuilder, - ) -> Result { - builder.append(set_importing_state_action()); - - builder.append(get_pantry_address_action()); - - // Call the Pantry's /import_from_url - builder.append(call_pantry_import_from_url_for_disk_action()); - - // Wait for import_from_url job to complete - builder.append(wait_for_import_from_url_action()); - - // Set ImportReady state - builder.append(set_import_ready_state_action()); - - Ok(builder.build()?) - } -} - -async fn sibfu_get_importing_state( - sagactx: NexusActionContext, -) -> Result { - let log = sagactx.user_data().log(); - let osagactx = sagactx.user_data(); - let params = sagactx.saga_params::()?; - let opctx = crate::context::op_context_for_saga_action( - &sagactx, - ¶ms.serialized_authn, - ); - - let (.., authz_disk, db_disk) = - LookupPath::new(&opctx, &osagactx.datastore()) - .disk_id(params.disk_id) - .fetch_for(authz::Action::Modify) - .await - .map_err(ActionError::action_failed)?; - - match db_disk.state().into() { - external::DiskState::ImportReady => { - info!( - log, - "setting disk {} to state importing_from_url", - db_disk.id(), - ); - - osagactx - .datastore() - .disk_update_runtime( - &opctx, - &authz_disk, - &db_disk.runtime().importing_from_url(), - ) - .await - .map_err(ActionError::action_failed)?; - - // Record the disk's new generation number as this saga node's output. It - // will be important later to *only* transition this disk out of maintenance - // if the generation number matches what *this* saga is doing. - let (.., db_disk) = LookupPath::new(&opctx, &osagactx.datastore()) - .disk_id(params.disk_id) - .fetch_for(authz::Action::Read) - .await - .map_err(ActionError::action_failed)?; - - Ok(db_disk.runtime().gen) - } - - _ => Err(ActionError::action_failed(Error::invalid_request(&format!( - "cannot import blocks from a url into disk in state {:?}", - db_disk.state() - )))), - } -} - -async fn sibfu_get_importing_state_undo( - sagactx: NexusActionContext, -) -> Result<(), anyhow::Error> { - let log = sagactx.user_data().log(); - let osagactx = sagactx.user_data(); - let params = sagactx.saga_params::()?; - let opctx = crate::context::op_context_for_saga_action( - &sagactx, - ¶ms.serialized_authn, - ); - - let (.., authz_disk, db_disk) = - LookupPath::new(&opctx, &osagactx.datastore()) - .disk_id(params.disk_id) - .fetch_for(authz::Action::Modify) - .await - .map_err(ActionError::action_failed)?; - - let expected_disk_generation_number = - sagactx.lookup::("disk_generation_number")?; - - match db_disk.state().into() { - external::DiskState::ImportingFromUrl => { - // A previous execution of *this* saga may hav already transitioned this disk to - // import_ready. Another saga racing with this one may have transitioned the disk to - // importing - only set this disk to import_ready if the generation number matches this - // saga. - if expected_disk_generation_number == db_disk.runtime().gen { - info!( - log, - "undo: setting disk {} state from importing_from_url to import_ready", - params.disk_id - ); - - osagactx - .datastore() - .disk_update_runtime( - &opctx, - &authz_disk, - &db_disk.runtime().import_ready(), - ) - .await - .map_err(ActionError::action_failed)?; - } else { - info!( - log, - "disk {} has generation number {:?}, which doesn't match the expected {:?}: skip setting to import_ready", - params.disk_id, - db_disk.runtime().gen, - expected_disk_generation_number, - ); - } - } - - external::DiskState::ImportReady => { - info!(log, "disk {} already import_ready", params.disk_id); - } - - _ => { - warn!(log, "disk is in state {:?}", db_disk.state()); - } - } - - Ok(()) -} - -async fn sibfu_get_pantry_address( - sagactx: NexusActionContext, -) -> Result { - let log = sagactx.user_data().log(); - let osagactx = sagactx.user_data(); - let params = sagactx.saga_params::()?; - let opctx = crate::context::op_context_for_saga_action( - &sagactx, - ¶ms.serialized_authn, - ); - - let (.., db_disk) = LookupPath::new(&opctx, &osagactx.datastore()) - .disk_id(params.disk_id) - .fetch_for(authz::Action::Modify) - .await - .map_err(ActionError::action_failed)?; - - // At any stage of executing this saga, if the disk moves from state - // importing to detached, it will be detached from the corresponding Pantry. - // Any subsequent saga nodes will fail because the pantry address is stored - // as part of the saga state, and requests sent to that Pantry with the - // disk's id will fail. - let pantry_address = db_disk.pantry_address().ok_or_else(|| { - ActionError::action_failed(String::from("disk not attached to pantry!")) - })?; - - info!(log, "disk {} is using pantry at {}", db_disk.id(), pantry_address); - - Ok(pantry_address) -} - -async fn sibfu_call_pantry_import_from_url_for_disk( - sagactx: NexusActionContext, -) -> Result { - let log = sagactx.user_data().log(); - let params = sagactx.saga_params::()?; - - let pantry_address = sagactx.lookup::("pantry_address")?; - let endpoint = format!("http://{}", pantry_address); - - info!( - log, - "sending import from url {} request for disk {} to pantry endpoint {}", - params.import_params.url, - params.disk_id, - endpoint, - ); - - let disk_id = params.disk_id.to_string(); - - let client = crucible_pantry_client::Client::new(&endpoint); - - let request = crucible_pantry_client::types::ImportFromUrlRequest { - url: params.import_params.url, - expected_digest: if let Some(expected_digest) = - params.import_params.expected_digest - { - match expected_digest { - nexus_types::external_api::params::ExpectedDigest::Sha256( - v, - ) => Some( - crucible_pantry_client::types::ExpectedDigest::Sha256(v), - ), - } - } else { - None - }, - }; - - let response = retry_until_known_result(log, || async { - client.import_from_url(&disk_id, &request).await - }) - .await - .map_err(|e| { - ActionError::action_failed(format!( - "import from url failed with {:?}", - e - )) - })?; - - Ok(response.job_id.clone()) -} - -async fn sibfu_wait_for_import_from_url( - sagactx: NexusActionContext, -) -> Result<(), ActionError> { - let log = sagactx.user_data().log(); - let params = sagactx.saga_params::()?; - - let pantry_address = sagactx.lookup::("pantry_address")?; - let job_id = - sagactx.lookup::("call_pantry_import_from_url_for_disk")?; - - let endpoint = format!("http://{}", pantry_address); - - let client = crucible_pantry_client::Client::new(&endpoint); - - info!( - log, - "waiting for import from url job {} for disk {} to complete on pantry {}", - job_id, - params.disk_id, - endpoint, - ); - - loop { - let result = retry_until_known_result(log, || async { - client.is_job_finished(&job_id).await - }) - .await - .map_err(|e| { - ActionError::action_failed(format!( - "is_job_finished failed with {:?}", - e - )) - })?; - - if result.job_is_finished { - break; - } - - tokio::time::sleep(tokio::time::Duration::from_secs(1)).await; - } - - info!( - log, - "import from url job {} for disk {} on pantry {} completed", - job_id, - params.disk_id, - endpoint, - ); - - let response = retry_until_known_result(log, || async { - client.job_result_ok(&job_id).await - }) - .await - .map_err(|e| { - ActionError::action_failed(format!("job_result_ok failed with {:?}", e)) - })?; - - if !response.job_result_ok { - return Err(ActionError::action_failed(format!("Job {job_id} failed"))); - } - - Ok(()) -} - -async fn sibfu_get_import_ready_state( - sagactx: NexusActionContext, -) -> Result<(), ActionError> { - let log = sagactx.user_data().log(); - let osagactx = sagactx.user_data(); - let params = sagactx.saga_params::()?; - let opctx = crate::context::op_context_for_saga_action( - &sagactx, - ¶ms.serialized_authn, - ); - - let (.., authz_disk, db_disk) = - LookupPath::new(&opctx, &osagactx.datastore()) - .disk_id(params.disk_id) - .fetch_for(authz::Action::Modify) - .await - .map_err(ActionError::action_failed)?; - - let expected_disk_generation_number = - sagactx.lookup::("disk_generation_number")?; - - match db_disk.state().into() { - external::DiskState::ImportingFromUrl => { - if expected_disk_generation_number == db_disk.runtime().gen { - info!( - log, - "setting disk {} state from importing_from_url to import_ready", - params.disk_id - ); - - osagactx - .datastore() - .disk_update_runtime( - &opctx, - &authz_disk, - &db_disk.runtime().import_ready(), - ) - .await - .map_err(ActionError::action_failed)?; - } else { - info!( - log, - "disk {} has generation number {:?}, which doesn't match the expected {:?}: skip setting to import_ready", - params.disk_id, - db_disk.runtime().gen, - expected_disk_generation_number, - ); - } - } - - external::DiskState::ImportReady => { - info!(log, "disk {} already import_ready", params.disk_id); - } - - _ => { - warn!(log, "disk is in state {:?}", db_disk.state()); - } - } - - Ok(()) -} diff --git a/nexus/src/app/sagas/mod.rs b/nexus/src/app/sagas/mod.rs index 89e1a10052..c5918d32ef 100644 --- a/nexus/src/app/sagas/mod.rs +++ b/nexus/src/app/sagas/mod.rs @@ -23,7 +23,6 @@ pub mod disk_create; pub mod disk_delete; pub mod finalize_disk; pub mod image_delete; -pub mod import_blocks_from_url; mod instance_common; pub mod instance_create; pub mod instance_delete; @@ -125,7 +124,6 @@ fn make_action_registry() -> ActionRegistry { ::register_actions( &mut registry, ); - ::register_actions(&mut registry); ::register_actions( &mut registry, ); diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index a113451fc7..a2e5f633df 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -154,7 +154,6 @@ pub(crate) fn external_api() -> NexusApiDescription { api.register(disk_bulk_write_import_start)?; api.register(disk_bulk_write_import)?; api.register(disk_bulk_write_import_stop)?; - api.register(disk_import_blocks_from_url)?; api.register(disk_finalize_import)?; api.register(instance_list)?; @@ -1916,39 +1915,6 @@ async fn disk_bulk_write_import_stop( apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await } -/// Request to import blocks from URL -#[endpoint { - method = POST, - path = "/v1/disks/{disk}/import", - tags = ["disks"], -}] -async fn disk_import_blocks_from_url( - rqctx: RequestContext>, - path_params: Path, - query_params: Query, - import_params: TypedBody, -) -> Result { - let apictx = rqctx.context(); - let handler = async { - let opctx = crate::context::op_context_for_external_api(&rqctx).await?; - let nexus = &apictx.nexus; - let path = path_params.into_inner(); - let query = query_params.into_inner(); - let params = import_params.into_inner(); - - let disk_selector = - params::DiskSelector { disk: path.disk, project: query.project }; - let disk_lookup = nexus.disk_lookup(&opctx, disk_selector)?; - - nexus - .import_blocks_from_url_for_disk(&opctx, &disk_lookup, params) - .await?; - - Ok(HttpResponseUpdatedNoContent()) - }; - apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await -} - /// Confirm disk block import completion #[endpoint { method = POST, diff --git a/nexus/tests/integration_tests/endpoints.rs b/nexus/tests/integration_tests/endpoints.rs index db803bfde0..e11902d0fe 100644 --- a/nexus/tests/integration_tests/endpoints.rs +++ b/nexus/tests/integration_tests/endpoints.rs @@ -262,8 +262,6 @@ lazy_static! { ), }; - pub static ref DEMO_IMPORT_DISK_IMPORT_FROM_URL_URL: String = - format!("/v1/disks/{}/import?{}", *DEMO_IMPORT_DISK_NAME, *DEMO_PROJECT_SELECTOR); pub static ref DEMO_IMPORT_DISK_BULK_WRITE_START_URL: String = format!("/v1/disks/{}/bulk-write-start?{}", *DEMO_IMPORT_DISK_NAME, *DEMO_PROJECT_SELECTOR); pub static ref DEMO_IMPORT_DISK_BULK_WRITE_URL: String = @@ -493,10 +491,7 @@ lazy_static! { name: DEMO_IMAGE_NAME.clone(), description: String::from(""), }, - source: params::ImageSource::Url { - url: HTTP_SERVER.url("/image.raw").to_string(), - block_size: params::BlockSize::try_from(4096).unwrap(), - }, + source: params::ImageSource::YouCanBootAnythingAsLongAsItsAlpine, os: "fake-os".to_string(), version: "1.0".to_string() }; @@ -1328,20 +1323,6 @@ lazy_static! { ], }, - VerifyEndpoint { - url: &DEMO_IMPORT_DISK_IMPORT_FROM_URL_URL, - visibility: Visibility::Protected, - unprivileged_access: UnprivilegedAccess::None, - allowed_methods: vec![ - AllowedMethod::Post( - serde_json::to_value(params::ImportBlocksFromUrl { - url: "obviously-fake-url".into(), - expected_digest: None, - }).unwrap() - ) - ], - }, - VerifyEndpoint { url: &DEMO_IMPORT_DISK_BULK_WRITE_START_URL, visibility: Visibility::Protected, diff --git a/nexus/tests/integration_tests/images.rs b/nexus/tests/integration_tests/images.rs index c3db9e8f13..9d608937ce 100644 --- a/nexus/tests/integration_tests/images.rs +++ b/nexus/tests/integration_tests/images.rs @@ -24,15 +24,11 @@ use nexus_types::identity::Resource; use omicron_common::api::external::Disk; use omicron_common::api::external::{ByteCount, IdentityMetadataCreateParams}; -use httptest::{matchers::*, responders::*, Expectation, ServerBuilder}; - type ControlPlaneTestContext = nexus_test_utils::ControlPlaneTestContext; const PROJECT_NAME: &str = "myproj"; -const BLOCK_SIZE: params::BlockSize = params::BlockSize(512); - fn get_project_images_url(project_name: &str) -> String { format!("/v1/images?project={}", project_name) } @@ -56,18 +52,6 @@ async fn test_image_create(cptestctx: &ControlPlaneTestContext) { let client = &cptestctx.external_client; DiskTest::new(&cptestctx).await; - let server = ServerBuilder::new().run().unwrap(); - server.expect( - Expectation::matching(request::method_path("HEAD", "/image.raw")) - .times(1..) - .respond_with( - status_code(200).append_header( - "Content-Length", - format!("{}", 4096 * 1000), - ), - ), - ); - let images_url = get_project_images_url(PROJECT_NAME); // Before project exists, image list 404s @@ -94,10 +78,9 @@ async fn test_image_create(cptestctx: &ControlPlaneTestContext) { assert_eq!(images.len(), 0); // Create an image in the project - let image_create_params = get_image_create(params::ImageSource::Url { - url: server.url("/image.raw").to_string(), - block_size: BLOCK_SIZE, - }); + let image_create_params = get_image_create( + params::ImageSource::YouCanBootAnythingAsLongAsItsAlpine, + ); NexusRequest::objects_post(client, &images_url, &image_create_params) .authn_as(AuthnMode::PrivilegedUser) @@ -120,18 +103,6 @@ async fn test_silo_image_create(cptestctx: &ControlPlaneTestContext) { let client = &cptestctx.external_client; DiskTest::new(&cptestctx).await; - let server = ServerBuilder::new().run().unwrap(); - server.expect( - Expectation::matching(request::method_path("HEAD", "/image.raw")) - .times(1..) - .respond_with( - status_code(200).append_header( - "Content-Length", - format!("{}", 4096 * 1000), - ), - ), - ); - let silo_images_url = "/v1/images"; // Expect no images in the silo @@ -144,10 +115,9 @@ async fn test_silo_image_create(cptestctx: &ControlPlaneTestContext) { assert_eq!(images.len(), 0); // Create an image in the project - let image_create_params = get_image_create(params::ImageSource::Url { - url: server.url("/image.raw").to_string(), - block_size: BLOCK_SIZE, - }); + let image_create_params = get_image_create( + params::ImageSource::YouCanBootAnythingAsLongAsItsAlpine, + ); // Create image NexusRequest::objects_post(client, &silo_images_url, &image_create_params) @@ -165,162 +135,6 @@ async fn test_silo_image_create(cptestctx: &ControlPlaneTestContext) { assert_eq!(images[0].identity.name, "alpine-edge"); } -#[nexus_test] -async fn test_image_create_url_404(cptestctx: &ControlPlaneTestContext) { - let client = &cptestctx.external_client; - DiskTest::new(&cptestctx).await; - - // need a project to post to - create_project(client, PROJECT_NAME).await; - - let server = ServerBuilder::new().run().unwrap(); - server.expect( - Expectation::matching(request::method_path("HEAD", "/image.raw")) - .times(1..) - .respond_with(status_code(404)), - ); - - let image_create_params = get_image_create(params::ImageSource::Url { - url: server.url("/image.raw").to_string(), - block_size: BLOCK_SIZE, - }); - - let images_url = get_project_images_url(PROJECT_NAME); - - let error = NexusRequest::new( - RequestBuilder::new(client, Method::POST, &images_url) - .body(Some(&image_create_params)) - .expect_status(Some(StatusCode::BAD_REQUEST)), - ) - .authn_as(AuthnMode::PrivilegedUser) - .execute() - .await - .expect("unexpected success") - .parsed_body::() - .unwrap(); - assert_eq!( - error.message, - format!("unsupported value for \"url\": querying url returned: 404 Not Found") - ); -} - -#[nexus_test] -async fn test_image_create_bad_url(cptestctx: &ControlPlaneTestContext) { - let client = &cptestctx.external_client; - DiskTest::new(&cptestctx).await; - - // need a project to post to - create_project(client, PROJECT_NAME).await; - - let image_create_params = get_image_create(params::ImageSource::Url { - url: "not_a_url".to_string(), - block_size: BLOCK_SIZE, - }); - - let images_url = get_project_images_url(PROJECT_NAME); - - let error = NexusRequest::new( - RequestBuilder::new(client, Method::POST, &images_url) - .body(Some(&image_create_params)) - .expect_status(Some(StatusCode::BAD_REQUEST)), - ) - .authn_as(AuthnMode::PrivilegedUser) - .execute() - .await - .expect("unexpected success") - .parsed_body::() - .unwrap(); - assert_eq!( - error.message, - format!("unsupported value for \"url\": error querying url: builder error: relative URL without a base") - ); -} - -#[nexus_test] -async fn test_image_create_bad_content_length( - cptestctx: &ControlPlaneTestContext, -) { - let client = &cptestctx.external_client; - DiskTest::new(&cptestctx).await; - - // need a project to post to - create_project(client, PROJECT_NAME).await; - - let server = ServerBuilder::new().run().unwrap(); - server.expect( - Expectation::matching(request::method_path("HEAD", "/image.raw")) - .times(1..) - .respond_with( - status_code(200).append_header("Content-Length", "bad"), - ), - ); - - let image_create_params = get_image_create(params::ImageSource::Url { - url: server.url("/image.raw").to_string(), - block_size: BLOCK_SIZE, - }); - - let images_url = get_project_images_url(PROJECT_NAME); - - let error = NexusRequest::new( - RequestBuilder::new(client, Method::POST, &images_url) - .body(Some(&image_create_params)) - .expect_status(Some(StatusCode::BAD_REQUEST)), - ) - .authn_as(AuthnMode::PrivilegedUser) - .execute() - .await - .expect("unexpected success") - .parsed_body::() - .unwrap(); - assert_eq!( - error.message, - format!("unsupported value for \"url\": content length invalid: invalid digit found in string") - ); -} - -#[nexus_test] -async fn test_image_create_bad_image_size(cptestctx: &ControlPlaneTestContext) { - let client = &cptestctx.external_client; - DiskTest::new(&cptestctx).await; - - // need a project to post to - create_project(client, PROJECT_NAME).await; - - let server = ServerBuilder::new().run().unwrap(); - server.expect( - Expectation::matching(request::method_path("HEAD", "/image.raw")) - .times(1..) - .respond_with(status_code(200).append_header( - "Content-Length", - format!("{}", 4096 * 1000 + 100), - )), - ); - - let image_create_params = get_image_create(params::ImageSource::Url { - url: server.url("/image.raw").to_string(), - block_size: BLOCK_SIZE, - }); - - let images_url = get_project_images_url(PROJECT_NAME); - - let error = NexusRequest::new( - RequestBuilder::new(client, Method::POST, &images_url) - .body(Some(&image_create_params)) - .expect_status(Some(StatusCode::BAD_REQUEST)), - ) - .authn_as(AuthnMode::PrivilegedUser) - .execute() - .await - .expect("unexpected success") - .parsed_body::() - .unwrap(); - assert_eq!( - error.message, - format!("unsupported value for \"size\": total size {} must be divisible by block size {}", 4096*1000 + 100, 512) - ); -} - #[nexus_test] async fn test_make_disk_from_image(cptestctx: &ControlPlaneTestContext) { let client = &cptestctx.external_client; @@ -329,23 +143,10 @@ async fn test_make_disk_from_image(cptestctx: &ControlPlaneTestContext) { // need a project to post both disk and image to create_project(client, PROJECT_NAME).await; - let server = ServerBuilder::new().run().unwrap(); - server.expect( - Expectation::matching(request::method_path("HEAD", "/alpine/edge.raw")) - .times(1..) - .respond_with( - status_code(200).append_header( - "Content-Length", - format!("{}", 4096 * 1000), - ), - ), - ); - // Create an image in the project - let image_create_params = get_image_create(params::ImageSource::Url { - url: server.url("/alpine/edge.raw").to_string(), - block_size: BLOCK_SIZE, - }); + let image_create_params = get_image_create( + params::ImageSource::YouCanBootAnythingAsLongAsItsAlpine, + ); let images_url = get_project_images_url(PROJECT_NAME); @@ -384,23 +185,10 @@ async fn test_make_disk_from_other_project_image_fails( create_project(client, PROJECT_NAME).await; let another_project = create_project(client, "another-proj").await; - let server = ServerBuilder::new().run().unwrap(); - server.expect( - Expectation::matching(request::method_path("HEAD", "/image.raw")) - .times(1..) - .respond_with( - status_code(200).append_header( - "Content-Length", - format!("{}", 4096 * 1000), - ), - ), - ); - let images_url = get_project_images_url(PROJECT_NAME); - let image_create_params = get_image_create(params::ImageSource::Url { - url: server.url("/image.raw").to_string(), - block_size: BLOCK_SIZE, - }); + let image_create_params = get_image_create( + params::ImageSource::YouCanBootAnythingAsLongAsItsAlpine, + ); let image = NexusRequest::objects_post(client, &images_url, &image_create_params) .authn_as(AuthnMode::PrivilegedUser) @@ -443,20 +231,10 @@ async fn test_make_disk_from_image_too_small( // need a project to post both disk and image to create_project(client, PROJECT_NAME).await; - let server = ServerBuilder::new().run().unwrap(); - server.expect( - Expectation::matching(request::method_path("HEAD", "/alpine/edge.raw")) - .times(1..) - .respond_with( - status_code(200).append_header("Content-Length", "2147483648"), - ), - ); - // Create an image in the project - let image_create_params = get_image_create(params::ImageSource::Url { - url: server.url("/alpine/edge.raw").to_string(), - block_size: BLOCK_SIZE, - }); + let image_create_params = get_image_create( + params::ImageSource::YouCanBootAnythingAsLongAsItsAlpine, + ); let images_url = get_project_images_url(PROJECT_NAME); @@ -474,7 +252,9 @@ async fn test_make_disk_from_image_too_small( disk_source: params::DiskSource::Image { image_id: alpine_image.identity.id, }, - size: ByteCount::from(1073741824), + + // Nexus defines YouCanBootAnythingAsLongAsItsAlpine size as 100M + size: ByteCount::from(90 * 1024 * 1024), }; let disks_url = format!("/v1/disks?project={}", PROJECT_NAME); @@ -493,7 +273,7 @@ async fn test_make_disk_from_image_too_small( error.message, format!( "disk size {} must be greater than or equal to image size {}", - 1073741824_u32, 2147483648_u32, + 94371840_u32, 104857600_u32, ) ); } @@ -503,18 +283,6 @@ async fn test_image_promotion(cptestctx: &ControlPlaneTestContext) { let client = &cptestctx.external_client; DiskTest::new(&cptestctx).await; - let server = ServerBuilder::new().run().unwrap(); - server.expect( - Expectation::matching(request::method_path("HEAD", "/image.raw")) - .times(1..) - .respond_with( - status_code(200).append_header( - "Content-Length", - format!("{}", 4096 * 1000), - ), - ), - ); - let silo_images_url = "/v1/images"; let images_url = get_project_images_url(PROJECT_NAME); @@ -528,10 +296,9 @@ async fn test_image_promotion(cptestctx: &ControlPlaneTestContext) { assert_eq!(images.len(), 0); - let image_create_params = get_image_create(params::ImageSource::Url { - url: server.url("/image.raw").to_string(), - block_size: BLOCK_SIZE, - }); + let image_create_params = get_image_create( + params::ImageSource::YouCanBootAnythingAsLongAsItsAlpine, + ); NexusRequest::objects_post(client, &images_url, &image_create_params) .authn_as(AuthnMode::PrivilegedUser) @@ -631,28 +398,15 @@ async fn test_image_from_other_project_snapshot_fails( let client = &cptestctx.external_client; DiskTest::new(&cptestctx).await; - let server = ServerBuilder::new().run().unwrap(); - server.expect( - Expectation::matching(request::method_path("HEAD", "/image.raw")) - .times(1..) - .respond_with( - status_code(200).append_header( - "Content-Length", - format!("{}", 4096 * 1000), - ), - ), - ); - create_project(client, PROJECT_NAME).await; let images_url = get_project_images_url(PROJECT_NAME); let disks_url = format!("/v1/disks?project={}", PROJECT_NAME); let snapshots_url = format!("/v1/snapshots?project={}", PROJECT_NAME); // Create an image - let image_create_params = get_image_create(params::ImageSource::Url { - url: server.url("/image.raw").to_string(), - block_size: BLOCK_SIZE, - }); + let image_create_params = get_image_create( + params::ImageSource::YouCanBootAnythingAsLongAsItsAlpine, + ); let image: views::Image = NexusRequest::objects_post(client, &images_url, &image_create_params) .authn_as(AuthnMode::PrivilegedUser) @@ -749,25 +503,12 @@ async fn test_image_deletion_permissions(cptestctx: &ControlPlaneTestContext) { // Create an image in the default silo using the privileged user - let server = ServerBuilder::new().run().unwrap(); - server.expect( - Expectation::matching(request::method_path("HEAD", "/image.raw")) - .times(1..) - .respond_with( - status_code(200).append_header( - "Content-Length", - format!("{}", 4096 * 1000), - ), - ), - ); - let silo_images_url = "/v1/images"; let images_url = get_project_images_url(PROJECT_NAME); - let image_create_params = get_image_create(params::ImageSource::Url { - url: server.url("/image.raw").to_string(), - block_size: BLOCK_SIZE, - }); + let image_create_params = get_image_create( + params::ImageSource::YouCanBootAnythingAsLongAsItsAlpine, + ); let image = NexusRequest::objects_post(client, &images_url, &image_create_params) diff --git a/nexus/tests/integration_tests/instances.rs b/nexus/tests/integration_tests/instances.rs index f54370c32f..33d4d15d23 100644 --- a/nexus/tests/integration_tests/instances.rs +++ b/nexus/tests/integration_tests/instances.rs @@ -73,8 +73,6 @@ use nexus_test_utils_macros::nexus_test; use nexus_types::external_api::shared::SiloRole; use omicron_sled_agent::sim; -use httptest::{matchers::*, responders::*, Expectation, ServerBuilder}; - type ControlPlaneTestContext = nexus_test_utils::ControlPlaneTestContext; @@ -1280,18 +1278,6 @@ async fn test_instance_using_image_from_other_project_fails( let client = &cptestctx.external_client; create_org_and_project(&client).await; - let server = ServerBuilder::new().run().unwrap(); - server.expect( - Expectation::matching(request::method_path("HEAD", "/image.raw")) - .times(1..) - .respond_with( - status_code(200).append_header( - "Content-Length", - format!("{}", 4096 * 1000), - ), - ), - ); - // Create an image in springfield-squidport. let images_url = format!("/v1/images?project={}", PROJECT_NAME); let image_create_params = params::ImageCreate { @@ -1303,10 +1289,7 @@ async fn test_instance_using_image_from_other_project_fails( }, os: "alpine".to_string(), version: "edge".to_string(), - source: params::ImageSource::Url { - url: server.url("/image.raw").to_string(), - block_size: params::BlockSize::try_from(512).unwrap(), - }, + source: params::ImageSource::YouCanBootAnythingAsLongAsItsAlpine, }; let image = NexusRequest::objects_post(client, &images_url, &image_create_params) diff --git a/nexus/tests/integration_tests/pantry.rs b/nexus/tests/integration_tests/pantry.rs index 26e27e92ee..dc4e8e6c95 100644 --- a/nexus/tests/integration_tests/pantry.rs +++ b/nexus/tests/integration_tests/pantry.rs @@ -302,25 +302,6 @@ async fn bulk_write_stop( .unwrap(); } -async fn import_blocks_from_url(client: &ClientTestContext) { - // Import blocks from a URL - let import_blocks_from_url_url = - format!("/v1/disks/{}/import?project={}", DISK_NAME, PROJECT_NAME,); - - NexusRequest::new( - RequestBuilder::new(client, Method::POST, &import_blocks_from_url_url) - .body(Some(¶ms::ImportBlocksFromUrl { - url: "http://fake.endpoint/image.iso".to_string(), - expected_digest: None, - })) - .expect_status(Some(StatusCode::NO_CONTENT)), - ) - .authn_as(AuthnMode::PrivilegedUser) - .execute() - .await - .unwrap(); -} - async fn finalize_import( client: &ClientTestContext, expected_status: StatusCode, @@ -461,33 +442,6 @@ async fn test_cannot_mount_import_from_bulk_writes_disk( .await; } -// Test the normal flow of importing from a URL -#[nexus_test] -async fn test_import_blocks_from_url(cptestctx: &ControlPlaneTestContext) { - let client = &cptestctx.external_client; - let nexus = &cptestctx.server.apictx().nexus; - - DiskTest::new(&cptestctx).await; - create_org_and_project(client).await; - - create_disk_with_state_importing_blocks(client).await; - - // Import blocks from a URL - import_blocks_from_url(client).await; - - // Validate disk is in state ImportReady - validate_disk_state(client, DiskState::ImportReady).await; - - // Finalize import - finalize_import(client, StatusCode::NO_CONTENT).await; - - // Validate disk is in state Detached - validate_disk_state(client, DiskState::Detached).await; - - // Create an instance to attach the disk. - create_instance_and_attach_disk(client, nexus, StatusCode::ACCEPTED).await; -} - // Test the normal flow of importing from bulk writes #[nexus_test] async fn test_import_blocks_with_bulk_write( diff --git a/nexus/tests/integration_tests/snapshots.rs b/nexus/tests/integration_tests/snapshots.rs index 1dd32e6769..a9ed1b7cb7 100644 --- a/nexus/tests/integration_tests/snapshots.rs +++ b/nexus/tests/integration_tests/snapshots.rs @@ -35,8 +35,6 @@ use omicron_common::api::external::Name; use omicron_nexus::app::MIN_DISK_SIZE_BYTES; use uuid::Uuid; -use httptest::{matchers::*, responders::*, Expectation, ServerBuilder}; - type ControlPlaneTestContext = nexus_test_utils::ControlPlaneTestContext; @@ -64,18 +62,6 @@ async fn test_snapshot_basic(cptestctx: &ControlPlaneTestContext) { let disks_url = get_disks_url(); // Define a global image - let server = ServerBuilder::new().run().unwrap(); - server.expect( - Expectation::matching(request::method_path("HEAD", "/image.raw")) - .times(1..) - .respond_with( - status_code(200).append_header( - "Content-Length", - format!("{}", 4096 * 1000), - ), - ), - ); - let image_create_params = params::ImageCreate { identity: IdentityMetadataCreateParams { name: "alpine-edge".parse().unwrap(), @@ -83,10 +69,7 @@ async fn test_snapshot_basic(cptestctx: &ControlPlaneTestContext) { "you can boot any image, as long as it's alpine", ), }, - source: params::ImageSource::Url { - url: server.url("/image.raw").to_string(), - block_size: params::BlockSize::try_from(512).unwrap(), - }, + source: params::ImageSource::YouCanBootAnythingAsLongAsItsAlpine, os: "alpine".to_string(), version: "edge".to_string(), }; @@ -184,18 +167,6 @@ async fn test_snapshot_without_instance(cptestctx: &ControlPlaneTestContext) { let disks_url = get_disks_url(); // Define a global image - let server = ServerBuilder::new().run().unwrap(); - server.expect( - Expectation::matching(request::method_path("HEAD", "/image.raw")) - .times(1..) - .respond_with( - status_code(200).append_header( - "Content-Length", - format!("{}", 4096 * 1000), - ), - ), - ); - let image_create_params = params::ImageCreate { identity: IdentityMetadataCreateParams { name: "alpine-edge".parse().unwrap(), @@ -203,10 +174,7 @@ async fn test_snapshot_without_instance(cptestctx: &ControlPlaneTestContext) { "you can boot any image, as long as it's alpine", ), }, - source: params::ImageSource::Url { - url: server.url("/image.raw").to_string(), - block_size: params::BlockSize::try_from(512).unwrap(), - }, + source: params::ImageSource::YouCanBootAnythingAsLongAsItsAlpine, os: "alpine".to_string(), version: "edge".to_string(), }; @@ -842,18 +810,6 @@ async fn test_snapshot_unwind(cptestctx: &ControlPlaneTestContext) { let disks_url = get_disks_url(); // Define a global image - let server = ServerBuilder::new().run().unwrap(); - server.expect( - Expectation::matching(request::method_path("HEAD", "/image.raw")) - .times(1..) - .respond_with( - status_code(200).append_header( - "Content-Length", - format!("{}", 4096 * 1000), - ), - ), - ); - let image_create_params = params::ImageCreate { identity: IdentityMetadataCreateParams { name: "alpine-edge".parse().unwrap(), @@ -861,10 +817,7 @@ async fn test_snapshot_unwind(cptestctx: &ControlPlaneTestContext) { "you can boot any image, as long as it's alpine", ), }, - source: params::ImageSource::Url { - url: server.url("/image.raw").to_string(), - block_size: params::BlockSize::try_from(512).unwrap(), - }, + source: params::ImageSource::YouCanBootAnythingAsLongAsItsAlpine, os: "alpine".to_string(), version: "edge".to_string(), }; diff --git a/nexus/tests/integration_tests/volume_management.rs b/nexus/tests/integration_tests/volume_management.rs index 24a0e5591b..5454e1f68f 100644 --- a/nexus/tests/integration_tests/volume_management.rs +++ b/nexus/tests/integration_tests/volume_management.rs @@ -30,8 +30,6 @@ use sled_agent_client::types::{CrucibleOpts, VolumeConstructionRequest}; use std::sync::Arc; use uuid::Uuid; -use httptest::{matchers::*, responders::*, Expectation, ServerBuilder}; - type ControlPlaneTestContext = nexus_test_utils::ControlPlaneTestContext; @@ -63,17 +61,6 @@ async fn create_image(client: &ClientTestContext) -> views::Image { create_org_and_project(client).await; // Define a global image - let server = ServerBuilder::new().run().unwrap(); - server.expect( - Expectation::matching(request::method_path("HEAD", "/image.raw")) - .times(1..) - .respond_with( - status_code(200).append_header( - "Content-Length", - format!("{}", 4096 * 1000), - ), - ), - ); let image_create_params = params::ImageCreate { identity: IdentityMetadataCreateParams { @@ -82,10 +69,7 @@ async fn create_image(client: &ClientTestContext) -> views::Image { "you can boot any image, as long as it's alpine", ), }, - source: params::ImageSource::Url { - url: server.url("/image.raw").to_string(), - block_size: params::BlockSize::try_from(512).unwrap(), - }, + source: params::ImageSource::YouCanBootAnythingAsLongAsItsAlpine, os: "alpine".to_string(), version: "edge".to_string(), }; diff --git a/nexus/tests/output/nexus_tags.txt b/nexus/tests/output/nexus_tags.txt index b236d73551..5a4a61132e 100644 --- a/nexus/tests/output/nexus_tags.txt +++ b/nexus/tests/output/nexus_tags.txt @@ -6,7 +6,6 @@ disk_bulk_write_import_stop POST /v1/disks/{disk}/bulk-write-st disk_create POST /v1/disks disk_delete DELETE /v1/disks/{disk} disk_finalize_import POST /v1/disks/{disk}/finalize -disk_import_blocks_from_url POST /v1/disks/{disk}/import disk_list GET /v1/disks disk_metrics_list GET /v1/disks/{disk}/metrics/{metric} disk_view GET /v1/disks/{disk} diff --git a/nexus/types/src/external_api/params.rs b/nexus/types/src/external_api/params.rs index e582590aa0..cde448c5b7 100644 --- a/nexus/types/src/external_api/params.rs +++ b/nexus/types/src/external_api/params.rs @@ -1219,15 +1219,6 @@ pub enum ExpectedDigest { Sha256(String), } -/// Parameters for importing blocks from a URL to a disk -#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] -pub struct ImportBlocksFromUrl { - /// the source to pull blocks from - pub url: String, - /// Expected digest of all blocks when importing from a URL - pub expected_digest: Option, -} - /// Parameters for importing blocks with a bulk write // equivalent to crucible_pantry_client::types::BulkWriteRequest #[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] @@ -1736,12 +1727,6 @@ pub struct SwitchPortApplySettings { #[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] #[serde(tag = "type", rename_all = "snake_case")] pub enum ImageSource { - Url { - url: String, - - /// The block size in bytes - block_size: BlockSize, - }, Snapshot { id: Uuid, }, diff --git a/nexus/types/src/external_api/views.rs b/nexus/types/src/external_api/views.rs index 047bd71814..af17e7e840 100644 --- a/nexus/types/src/external_api/views.rs +++ b/nexus/types/src/external_api/views.rs @@ -133,9 +133,6 @@ pub struct Image { /// ID of the parent project if the image is a project image pub project_id: Option, - /// URL source of this image, if any - pub url: Option, - /// The family of the operating system like Debian, Ubuntu, etc. pub os: String, diff --git a/openapi/nexus.json b/openapi/nexus.json index 6076663a2d..7afb6cdc2f 100644 --- a/openapi/nexus.json +++ b/openapi/nexus.json @@ -699,55 +699,6 @@ } } }, - "/v1/disks/{disk}/import": { - "post": { - "tags": [ - "disks" - ], - "summary": "Request to import blocks from URL", - "operationId": "disk_import_blocks_from_url", - "parameters": [ - { - "in": "path", - "name": "disk", - "description": "Name or ID of the disk", - "required": true, - "schema": { - "$ref": "#/components/schemas/NameOrId" - } - }, - { - "in": "query", - "name": "project", - "description": "Name or ID of the project", - "schema": { - "$ref": "#/components/schemas/NameOrId" - } - } - ], - "requestBody": { - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/ImportBlocksFromUrl" - } - } - }, - "required": true - }, - "responses": { - "204": { - "description": "resource updated" - }, - "4XX": { - "$ref": "#/components/responses/Error" - }, - "5XX": { - "$ref": "#/components/responses/Error" - } - } - } - }, "/v1/disks/{disk}/metrics/{metric}": { "get": { "tags": [ @@ -10527,22 +10478,6 @@ "request_id" ] }, - "ExpectedDigest": { - "oneOf": [ - { - "type": "object", - "properties": { - "sha256": { - "type": "string" - } - }, - "required": [ - "sha256" - ], - "additionalProperties": false - } - ] - }, "ExternalIp": { "type": "object", "properties": { @@ -11297,11 +11232,6 @@ "type": "string", "format": "date-time" }, - "url": { - "nullable": true, - "description": "URL source of this image, if any", - "type": "string" - }, "version": { "description": "Version of the operating system", "type": "string" @@ -11378,33 +11308,6 @@ "ImageSource": { "description": "The source of the underlying image.", "oneOf": [ - { - "type": "object", - "properties": { - "block_size": { - "description": "The block size in bytes", - "allOf": [ - { - "$ref": "#/components/schemas/BlockSize" - } - ] - }, - "type": { - "type": "string", - "enum": [ - "url" - ] - }, - "url": { - "type": "string" - } - }, - "required": [ - "block_size", - "type", - "url" - ] - }, { "type": "object", "properties": { @@ -11459,28 +11362,6 @@ "offset" ] }, - "ImportBlocksFromUrl": { - "description": "Parameters for importing blocks from a URL to a disk", - "type": "object", - "properties": { - "expected_digest": { - "nullable": true, - "description": "Expected digest of all blocks when importing from a URL", - "allOf": [ - { - "$ref": "#/components/schemas/ExpectedDigest" - } - ] - }, - "url": { - "description": "the source to pull blocks from", - "type": "string" - } - }, - "required": [ - "url" - ] - }, "Instance": { "description": "View of an Instance", "type": "object",