From 408274f33992bad0af250771fb62f8c108080ff5 Mon Sep 17 00:00:00 2001 From: Rain Date: Fri, 12 Jul 2024 15:41:48 -0700 Subject: [PATCH] =?UTF-8?q?[=F0=9D=98=80=F0=9D=97=BD=F0=9D=97=BF]=20initia?= =?UTF-8?q?l=20version?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Created using spr 1.3.6-beta.1 --- Cargo.lock | 35 ++-- Cargo.toml | 12 +- .../Cargo.toml | 2 +- .../src/lib.rs | 4 +- dev-tools/openapi-manager/Cargo.toml | 1 + dev-tools/openapi-manager/src/spec.rs | 12 ++ .../Cargo.toml | 20 +-- installinator-api/src/lib.rs | 167 ++++++++++++++++++ .../src/bin/installinator-artifactd.rs | 38 ---- installinator-artifactd/src/context.rs | 13 -- .../src/http_entrypoints.rs | 115 ------------ installinator-artifactd/src/lib.rs | 29 --- installinator-artifactd/src/server.rs | 74 -------- installinator-artifactd/src/store.rs | 79 --------- .../tests/integration_tests/mod.rs | 5 - .../tests/integration_tests/openapi.rs | 39 ---- installinator-artifactd/tests/mod.rs | 17 -- .../tests/output/cmd-server-openapi-stderr | 0 installinator/Cargo.toml | 2 +- installinator/src/artifact.rs | 7 +- installinator/src/errors.rs | 2 +- installinator/src/mock_peers.rs | 8 +- installinator/src/peers.rs | 2 +- ...ator-artifactd.json => installinator.json} | 4 +- wicketd/Cargo.toml | 4 +- wicketd/src/artifacts.rs | 3 +- wicketd/src/artifacts/server.rs | 99 +++++++---- wicketd/src/bin/wicketd.rs | 5 +- wicketd/src/installinator_progress.rs | 2 +- wicketd/src/lib.rs | 64 ++++--- wicketd/tests/integration_tests/setup.rs | 7 +- 31 files changed, 340 insertions(+), 531 deletions(-) rename clients/{installinator-artifact-client => installinator-client}/Cargo.toml (92%) rename clients/{installinator-artifact-client => installinator-client}/src/lib.rs (91%) rename {installinator-artifactd => installinator-api}/Cargo.toml (55%) create mode 100644 installinator-api/src/lib.rs delete mode 100644 installinator-artifactd/src/bin/installinator-artifactd.rs delete mode 100644 installinator-artifactd/src/context.rs delete mode 100644 installinator-artifactd/src/http_entrypoints.rs delete mode 100644 installinator-artifactd/src/lib.rs delete mode 100644 installinator-artifactd/src/server.rs delete mode 100644 installinator-artifactd/src/store.rs delete mode 100644 installinator-artifactd/tests/integration_tests/mod.rs delete mode 100644 installinator-artifactd/tests/integration_tests/openapi.rs delete mode 100644 installinator-artifactd/tests/mod.rs delete mode 100644 installinator-artifactd/tests/output/cmd-server-openapi-stderr rename openapi/{installinator-artifactd.json => installinator.json} (99%) diff --git a/Cargo.lock b/Cargo.lock index 84669a13e7..9a202fb04a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3606,7 +3606,7 @@ dependencies = [ "hex-literal", "http 0.2.12", "illumos-utils", - "installinator-artifact-client", + "installinator-client", "installinator-common", "ipcc", "itertools 0.12.1", @@ -3638,43 +3638,35 @@ dependencies = [ ] [[package]] -name = "installinator-artifact-client" +name = "installinator-api" version = "0.1.0" dependencies = [ + "anyhow", + "dropshot", + "hyper 0.14.28", "installinator-common", + "omicron-common", "omicron-workspace-hack", - "progenitor", - "regress", - "reqwest", "schemars", "serde", - "serde_json", "slog", - "update-engine", "uuid", ] [[package]] -name = "installinator-artifactd" +name = "installinator-client" version = "0.1.0" dependencies = [ - "anyhow", - "async-trait", - "clap", - "dropshot", - "expectorate", - "hyper 0.14.28", "installinator-common", - "omicron-common", - "omicron-test-utils", "omicron-workspace-hack", - "openapi-lint", - "openapiv3", + "progenitor", + "regress", + "reqwest", "schemars", "serde", "serde_json", "slog", - "subprocess", + "update-engine", "uuid", ] @@ -6109,6 +6101,7 @@ dependencies = [ "dropshot", "fs-err", "indent_write", + "installinator-api", "nexus-internal-api", "omicron-workspace-hack", "openapi-lint", @@ -11176,8 +11169,8 @@ dependencies = [ "hyper 0.14.28", "illumos-utils", "installinator", - "installinator-artifact-client", - "installinator-artifactd", + "installinator-api", + "installinator-client", "installinator-common", "internal-dns", "itertools 0.12.1", diff --git a/Cargo.toml b/Cargo.toml index e5783b39eb..13149ca637 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -9,7 +9,7 @@ members = [ "clients/dns-service-client", "clients/dpd-client", "clients/gateway-client", - "clients/installinator-artifact-client", + "clients/installinator-client", "clients/nexus-client", "clients/oxide-client", "clients/oximeter-client", @@ -31,7 +31,7 @@ members = [ "gateway-test-utils", "gateway", "illumos-utils", - "installinator-artifactd", + "installinator-api", "installinator-common", "installinator", "internal-dns-cli", @@ -100,7 +100,7 @@ default-members = [ "clients/dns-service-client", "clients/dpd-client", "clients/gateway-client", - "clients/installinator-artifact-client", + "clients/installinator-client", "clients/nexus-client", "clients/oxide-client", "clients/oximeter-client", @@ -125,7 +125,7 @@ default-members = [ "gateway-test-utils", "gateway", "illumos-utils", - "installinator-artifactd", + "installinator-api", "installinator-common", "installinator", "internal-dns-cli", @@ -318,8 +318,8 @@ indent_write = "2.2.0" indexmap = "2.2.6" indicatif = { version = "0.17.8", features = ["rayon"] } installinator = { path = "installinator" } -installinator-artifactd = { path = "installinator-artifactd" } -installinator-artifact-client = { path = "clients/installinator-artifact-client" } +installinator-api = { path = "installinator-api" } +installinator-client = { path = "clients/installinator-client" } installinator-common = { path = "installinator-common" } internal-dns = { path = "internal-dns" } ipcc = { path = "ipcc" } diff --git a/clients/installinator-artifact-client/Cargo.toml b/clients/installinator-client/Cargo.toml similarity index 92% rename from clients/installinator-artifact-client/Cargo.toml rename to clients/installinator-client/Cargo.toml index f1e896864f..ca2de0476a 100644 --- a/clients/installinator-artifact-client/Cargo.toml +++ b/clients/installinator-client/Cargo.toml @@ -1,5 +1,5 @@ [package] -name = "installinator-artifact-client" +name = "installinator-client" version = "0.1.0" edition = "2021" license = "MPL-2.0" diff --git a/clients/installinator-artifact-client/src/lib.rs b/clients/installinator-client/src/lib.rs similarity index 91% rename from clients/installinator-artifact-client/src/lib.rs rename to clients/installinator-client/src/lib.rs index 96806c2cab..a39ff3ff80 100644 --- a/clients/installinator-artifact-client/src/lib.rs +++ b/clients/installinator-client/src/lib.rs @@ -2,10 +2,10 @@ // 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/. -//! Interface for making API requests to installinator-artifactd. +//! Interface for installinator to make API requests. progenitor::generate_api!( - spec = "../../openapi/installinator-artifactd.json", + spec = "../../openapi/installinator.json", inner_type = slog::Logger, pre_hook = (|log: &slog::Logger, request: &reqwest::Request| { slog::debug!(log, "client request"; diff --git a/dev-tools/openapi-manager/Cargo.toml b/dev-tools/openapi-manager/Cargo.toml index b50aeec69f..654a36dd4e 100644 --- a/dev-tools/openapi-manager/Cargo.toml +++ b/dev-tools/openapi-manager/Cargo.toml @@ -15,6 +15,7 @@ clap.workspace = true dropshot.workspace = true fs-err.workspace = true indent_write.workspace = true +installinator-api.workspace = true nexus-internal-api.workspace = true omicron-workspace-hack.workspace = true openapiv3.workspace = true diff --git a/dev-tools/openapi-manager/src/spec.rs b/dev-tools/openapi-manager/src/spec.rs index 37330d6922..8883742154 100644 --- a/dev-tools/openapi-manager/src/spec.rs +++ b/dev-tools/openapi-manager/src/spec.rs @@ -14,6 +14,18 @@ use openapiv3::OpenAPI; /// All APIs managed by openapi-manager. pub fn all_apis() -> Vec { vec![ + ApiSpec { + title: "Installinator API".to_string(), + version: "0.0.1".to_string(), + description: "API for installinator to fetch artifacts \ + and report progress" + .to_string(), + boundary: ApiBoundary::Internal, + api_description: + installinator_api::installinator_api::stub_api_description, + filename: "installinator.json".to_string(), + extra_validation: None, + }, ApiSpec { title: "Nexus internal API".to_string(), version: "0.0.1".to_string(), diff --git a/installinator-artifactd/Cargo.toml b/installinator-api/Cargo.toml similarity index 55% rename from installinator-artifactd/Cargo.toml rename to installinator-api/Cargo.toml index 236ea7a51c..52db4362c6 100644 --- a/installinator-artifactd/Cargo.toml +++ b/installinator-api/Cargo.toml @@ -1,5 +1,5 @@ [package] -name = "installinator-artifactd" +name = "installinator-api" version = "0.1.0" edition = "2021" license = "MPL-2.0" @@ -9,24 +9,12 @@ workspace = true [dependencies] anyhow.workspace = true -async-trait.workspace = true -clap.workspace = true dropshot.workspace = true hyper.workspace = true +installinator-common.workspace = true +omicron-common.workspace = true +omicron-workspace-hack.workspace = true schemars.workspace = true serde.workspace = true -serde_json.workspace = true slog.workspace = true uuid.workspace = true - -installinator-common.workspace = true -omicron-common.workspace = true -omicron-workspace-hack.workspace = true - -[dev-dependencies] -expectorate.workspace = true -omicron-test-utils.workspace = true -openapiv3.workspace = true -openapi-lint.workspace = true -serde_json.workspace = true -subprocess.workspace = true diff --git a/installinator-api/src/lib.rs b/installinator-api/src/lib.rs new file mode 100644 index 0000000000..cd87643a66 --- /dev/null +++ b/installinator-api/src/lib.rs @@ -0,0 +1,167 @@ +// 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/. + +//! The REST API that installinator is a client of. +//! +//! Note that most of our APIs are named by their server. This one is instead +//! named by the client, since it is expected that multiple services will +//! implement it. + +use anyhow::{anyhow, Result}; +use dropshot::{ + ConfigDropshot, FreeformBody, HandlerTaskMode, HttpError, + HttpResponseHeaders, HttpResponseOk, HttpResponseUpdatedNoContent, + HttpServerStarter, Path, RequestContext, TypedBody, +}; +use hyper::{header, Body, StatusCode}; +use installinator_common::EventReport; +use omicron_common::update::ArtifactHashId; +use schemars::JsonSchema; +use serde::Deserialize; +use uuid::Uuid; + +#[derive(Debug, Deserialize, JsonSchema)] +pub struct ReportQuery { + /// A unique identifier for the update. + pub update_id: Uuid, +} + +#[dropshot::api_description] +pub trait InstallinatorApi { + type Context; + + /// Fetch an artifact by hash. + #[endpoint { + method = GET, + path = "/artifacts/by-hash/{kind}/{hash}", + }] + async fn get_artifact_by_hash( + rqctx: RequestContext, + path: Path, + ) -> Result>, HttpError>; + + /// Report progress and completion to the server. + /// + /// This method requires an `update_id` path parameter. This update ID is + /// matched against the server currently performing an update. If the + /// server is unaware of the update ID, it will return an HTTP 422 + /// Unprocessable Entity code. + #[endpoint { + method = POST, + path = "/report-progress/{update_id}", + }] + async fn report_progress( + rqctx: RequestContext, + path: Path, + report: TypedBody, + ) -> Result; +} + +/// Add a content length header to a response. +/// +/// Intended to be called by `get_artifact_by_hash` implementations. +pub fn body_to_artifact_response( + size: u64, + body: Body, +) -> HttpResponseHeaders> { + let mut response = + HttpResponseHeaders::new_unnamed(HttpResponseOk(body.into())); + let headers = response.headers_mut(); + headers.append(header::CONTENT_LENGTH, size.into()); + response +} + +/// The result of processing an installinator event report. +#[derive(Copy, Clone, Debug, Eq, PartialEq, PartialOrd, Ord, Hash)] +#[must_use] +pub enum EventReportStatus { + /// This report was processed by the server. + Processed, + + /// The update ID was not recognized by the server. + UnrecognizedUpdateId, + + /// The progress receiver is closed. + ReceiverClosed, +} + +impl EventReportStatus { + /// Convert this status to an HTTP result. + /// + /// Intended to be called by `report_progress` implementations. + pub fn to_http_result( + self, + update_id: Uuid, + ) -> Result { + match self { + EventReportStatus::Processed => Ok(HttpResponseUpdatedNoContent()), + EventReportStatus::UnrecognizedUpdateId => { + Err(HttpError::for_client_error( + None, + StatusCode::UNPROCESSABLE_ENTITY, + format!( + "update ID {update_id} unrecognized by this server" + ), + )) + } + EventReportStatus::ReceiverClosed => { + Err(HttpError::for_client_error( + None, + StatusCode::GONE, + format!("update ID {update_id}: receiver closed"), + )) + } + } + } +} + +/// Creates a default `ConfigDropshot` for the installinator API. +pub fn default_config(bind_address: std::net::SocketAddr) -> ConfigDropshot { + ConfigDropshot { + bind_address, + // Even though the installinator sets an upper bound on the number of + // items in a progress report, they can get pretty large if they + // haven't gone through for a bit. Ensure that hitting the max request + // size won't cause a failure by setting a generous upper bound for the + // request size. + // + // TODO: replace with an endpoint-specific option once + // https://github.com/oxidecomputer/dropshot/pull/618 lands and is + // available in omicron. + request_body_max_bytes: 4 * 1024 * 1024, + default_handler_task_mode: HandlerTaskMode::Detached, + } +} + +/// Make an `HttpServerStarter` for the installinator API with default settings. +pub fn make_server_starter( + context: T::Context, + bind_address: std::net::SocketAddr, + log: &slog::Logger, +) -> Result> { + let dropshot_config = dropshot::ConfigDropshot { + bind_address, + // Even though the installinator sets an upper bound on the number + // of items in a progress report, they can get pretty large if they + // haven't gone through for a bit. Ensure that hitting the max + // request size won't cause a failure by setting a generous upper + // bound for the request size. + // + // TODO: replace with an endpoint-specific option once + // https://github.com/oxidecomputer/dropshot/pull/618 lands and is + // available in omicron. + request_body_max_bytes: 4 * 1024 * 1024, + default_handler_task_mode: HandlerTaskMode::Detached, + }; + + let api = crate::installinator_api::api_description::()?; + let server = + dropshot::HttpServerStarter::new(&dropshot_config, api, context, &log) + .map_err(|error| { + anyhow!(error) + .context("failed to create installinator artifact server") + })?; + + Ok(server) +} diff --git a/installinator-artifactd/src/bin/installinator-artifactd.rs b/installinator-artifactd/src/bin/installinator-artifactd.rs deleted file mode 100644 index abe63bbe31..0000000000 --- a/installinator-artifactd/src/bin/installinator-artifactd.rs +++ /dev/null @@ -1,38 +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/. - -//! Executable that generates OpenAPI definitions for the installinator artifact server. - -use anyhow::Result; -use clap::Parser; -use omicron_common::cmd::CmdError; - -#[derive(Debug, Parser)] -#[clap(name = "installinator-artifactd")] -enum Args { - /// Print the external OpenAPI Spec document and exit - Openapi, - // NOTE: this server is not intended to be run as a standalone service. Instead, it should be - // embedded as part of other servers (e.g. wicketd). -} - -fn main() { - if let Err(cmd_error) = do_run() { - omicron_common::cmd::fatal(cmd_error); - } -} - -fn do_run() -> Result<(), CmdError> { - let args = Args::parse(); - - match args { - Args::Openapi => { - installinator_artifactd::run_openapi().map_err(|error| { - CmdError::Failure( - error.context("failed to generate OpenAPI spec"), - ) - }) - } - } -} diff --git a/installinator-artifactd/src/context.rs b/installinator-artifactd/src/context.rs deleted file mode 100644 index beea2593aa..0000000000 --- a/installinator-artifactd/src/context.rs +++ /dev/null @@ -1,13 +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/. - -// Copyright 2023 Oxide Computer Company - -//! User provided dropshot server context - -use crate::store::ArtifactStore; - -pub struct ServerContext { - pub(crate) artifact_store: ArtifactStore, -} diff --git a/installinator-artifactd/src/http_entrypoints.rs b/installinator-artifactd/src/http_entrypoints.rs deleted file mode 100644 index 13163e007b..0000000000 --- a/installinator-artifactd/src/http_entrypoints.rs +++ /dev/null @@ -1,115 +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/. - -// Copyright 2022 Oxide Computer Company - -use dropshot::{ - endpoint, ApiDescription, ApiDescriptionRegisterError, FreeformBody, - HttpError, HttpResponseHeaders, HttpResponseOk, - HttpResponseUpdatedNoContent, Path, RequestContext, TypedBody, -}; -use hyper::{header, Body, StatusCode}; -use installinator_common::EventReport; -use omicron_common::update::ArtifactHashId; -use schemars::JsonSchema; -use serde::Deserialize; -use uuid::Uuid; - -use crate::{context::ServerContext, EventReportStatus}; - -type ArtifactServerApiDesc = ApiDescription; - -/// Return a description of the artifact server api for use in generating an OpenAPI spec -pub fn api() -> ArtifactServerApiDesc { - fn register_endpoints( - api: &mut ArtifactServerApiDesc, - ) -> Result<(), ApiDescriptionRegisterError> { - api.register(get_artifact_by_hash)?; - api.register(report_progress)?; - Ok(()) - } - - let mut api = ArtifactServerApiDesc::new(); - if let Err(err) = register_endpoints(&mut api) { - panic!("failed to register entrypoints: {}", err); - } - api -} - -/// Fetch an artifact by hash. -#[endpoint { - method = GET, - path = "/artifacts/by-hash/{kind}/{hash}", -}] -async fn get_artifact_by_hash( - rqctx: RequestContext, - path: Path, -) -> Result>, HttpError> { - match rqctx - .context() - .artifact_store - .get_artifact_by_hash(&path.into_inner()) - .await - { - Some((size, body)) => Ok(body_to_artifact_response(size, body)), - None => { - Err(HttpError::for_not_found(None, "Artifact not found".into())) - } - } -} - -#[derive(Debug, Deserialize, JsonSchema)] -pub(crate) struct ReportQuery { - /// A unique identifier for the update. - pub(crate) update_id: Uuid, -} - -/// Report progress and completion to the server. -/// -/// This method requires an `update_id` path parameter. This update ID is -/// matched against the server currently performing an update. If the server -/// is unaware of the update ID, it will return an HTTP 422 Unprocessable Entity -/// code. -#[endpoint { - method = POST, - path = "/report-progress/{update_id}", -}] -async fn report_progress( - rqctx: RequestContext, - path: Path, - report: TypedBody, -) -> Result { - let update_id = path.into_inner().update_id; - match rqctx - .context() - .artifact_store - .report_progress(update_id, report.into_inner()) - .await? - { - EventReportStatus::Processed => Ok(HttpResponseUpdatedNoContent()), - EventReportStatus::UnrecognizedUpdateId => { - Err(HttpError::for_client_error( - None, - StatusCode::UNPROCESSABLE_ENTITY, - format!("update ID {update_id} unrecognized by this server"), - )) - } - EventReportStatus::ReceiverClosed => Err(HttpError::for_client_error( - None, - StatusCode::GONE, - format!("update ID {update_id}: receiver closed"), - )), - } -} - -fn body_to_artifact_response( - size: u64, - body: Body, -) -> HttpResponseHeaders> { - let mut response = - HttpResponseHeaders::new_unnamed(HttpResponseOk(body.into())); - let headers = response.headers_mut(); - headers.append(header::CONTENT_LENGTH, size.into()); - response -} diff --git a/installinator-artifactd/src/lib.rs b/installinator-artifactd/src/lib.rs deleted file mode 100644 index c54ed78a97..0000000000 --- a/installinator-artifactd/src/lib.rs +++ /dev/null @@ -1,29 +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/. - -// Copyright 2023 Oxide Computer Company - -mod context; -mod http_entrypoints; -mod server; -mod store; - -pub use context::ServerContext; -pub use server::ArtifactServer; -pub use store::{ArtifactGetter, EventReportStatus}; - -use anyhow::Result; - -/// Run the OpenAPI generator for the API; which emits the OpenAPI spec -/// to stdout. -pub fn run_openapi() -> Result<()> { - http_entrypoints::api() - .openapi("Oxide Installinator Artifact Server", "0.0.1") - .description("API for use by the installinator to retrieve artifacts") - .contact_url("https://oxide.computer") - .contact_email("api@oxide.computer") - .write(&mut std::io::stdout())?; - - Ok(()) -} diff --git a/installinator-artifactd/src/server.rs b/installinator-artifactd/src/server.rs deleted file mode 100644 index 88b622b756..0000000000 --- a/installinator-artifactd/src/server.rs +++ /dev/null @@ -1,74 +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/. - -// Copyright 2023 Oxide Computer Company - -//! The installinator artifact server. - -use std::net::SocketAddrV6; - -use anyhow::{anyhow, Result}; -use dropshot::{HandlerTaskMode, HttpServer}; - -use crate::{ - context::ServerContext, - store::{ArtifactGetter, ArtifactStore}, -}; - -/// The installinator artifact server. -#[derive(Debug)] -pub struct ArtifactServer { - address: SocketAddrV6, - log: slog::Logger, - store: ArtifactStore, -} - -impl ArtifactServer { - /// Creates a new artifact server with the given address. - pub fn new( - getter: Getter, - address: SocketAddrV6, - log: &slog::Logger, - ) -> Self { - let log = log.new(slog::o!("component" => "installinator artifactd")); - let store = ArtifactStore::new(getter, &log); - Self { address, log, store } - } - - /// Starts the artifact server. - /// - /// This returns an `HttpServer`, which can be awaited to completion. - pub fn start(self) -> Result> { - let context = ServerContext { artifact_store: self.store }; - - let dropshot_config = dropshot::ConfigDropshot { - bind_address: std::net::SocketAddr::V6(self.address), - // Even though the installinator sets an upper bound on the number - // of items in a progress report, they can get pretty large if they - // haven't gone through for a bit. Ensure that hitting the max - // request size won't cause a failure by setting a generous upper - // bound for the request size. - // - // TODO: replace with an endpoint-specific option once - // https://github.com/oxidecomputer/dropshot/pull/618 lands and is - // available in omicron. - request_body_max_bytes: 4 * 1024 * 1024, - default_handler_task_mode: HandlerTaskMode::Detached, - }; - - let api = crate::http_entrypoints::api(); - let server = dropshot::HttpServerStarter::new( - &dropshot_config, - api, - context, - &self.log, - ) - .map_err(|error| { - anyhow!(error) - .context("failed to create installinator artifact server") - })?; - - Ok(server.start()) - } -} diff --git a/installinator-artifactd/src/store.rs b/installinator-artifactd/src/store.rs deleted file mode 100644 index 12e2880893..0000000000 --- a/installinator-artifactd/src/store.rs +++ /dev/null @@ -1,79 +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/. - -// Copyright 2023 Oxide Computer Company - -use std::fmt; - -use async_trait::async_trait; -use dropshot::HttpError; -use hyper::Body; -use installinator_common::EventReport; -use omicron_common::update::ArtifactHashId; -use slog::Logger; -use uuid::Uuid; - -/// Represents a way to fetch artifacts. -#[async_trait] -pub trait ArtifactGetter: fmt::Debug + Send + Sync + 'static { - /// Gets an artifact by hash, returning it as a [`Body`]. - async fn get_by_hash(&self, id: &ArtifactHashId) -> Option<(u64, Body)>; - - /// Reports update progress events from the installinator. - async fn report_progress( - &self, - update_id: Uuid, - report: EventReport, - ) -> Result; -} - -/// The status returned by [`ArtifactGetter::report_progress`]. -#[derive(Copy, Clone, Debug, Eq, PartialEq, PartialOrd, Ord, Hash)] -#[must_use] -pub enum EventReportStatus { - /// This report was processed by the server. - Processed, - - /// The update ID was not recognized by the server. - UnrecognizedUpdateId, - - /// The progress receiver is closed. - ReceiverClosed, -} - -/// The artifact store -- a simple wrapper around a dynamic [`ArtifactGetter`] that does some basic -/// logging. -#[derive(Debug)] -pub(crate) struct ArtifactStore { - log: Logger, - getter: Box, - // TODO: implement this -} - -impl ArtifactStore { - pub(crate) fn new( - getter: Getter, - log: &Logger, - ) -> Self { - let log = log.new(slog::o!("component" => "artifact store")); - Self { log, getter: Box::new(getter) } - } - - pub(crate) async fn get_artifact_by_hash( - &self, - id: &ArtifactHashId, - ) -> Option<(u64, Body)> { - slog::debug!(self.log, "Artifact requested by hash: {:?}", id); - self.getter.get_by_hash(id).await - } - - pub(crate) async fn report_progress( - &self, - update_id: Uuid, - report: EventReport, - ) -> Result { - slog::debug!(self.log, "Report for {update_id}: {report:?}"); - self.getter.report_progress(update_id, report).await - } -} diff --git a/installinator-artifactd/tests/integration_tests/mod.rs b/installinator-artifactd/tests/integration_tests/mod.rs deleted file mode 100644 index ebb67c3880..0000000000 --- a/installinator-artifactd/tests/integration_tests/mod.rs +++ /dev/null @@ -1,5 +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/. - -mod openapi; diff --git a/installinator-artifactd/tests/integration_tests/openapi.rs b/installinator-artifactd/tests/integration_tests/openapi.rs deleted file mode 100644 index 09441731d0..0000000000 --- a/installinator-artifactd/tests/integration_tests/openapi.rs +++ /dev/null @@ -1,39 +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/. - -use std::path::PathBuf; - -use expectorate::assert_contents; -use omicron_test_utils::dev::test_cmds::{ - assert_exit_code, path_to_executable, run_command, EXIT_SUCCESS, -}; -use openapiv3::OpenAPI; -use subprocess::Exec; - -// name of executable -const CMD_SERVER: &str = env!("CARGO_BIN_EXE_installinator-artifactd"); - -fn path_to_server() -> PathBuf { - path_to_executable(CMD_SERVER) -} - -#[test] -fn test_server_openapi() { - let exec = Exec::cmd(path_to_server()).arg("openapi"); - let (exit_status, stdout_text, stderr_text) = run_command(exec); - assert_exit_code(exit_status, EXIT_SUCCESS, &stderr_text); - assert_contents("tests/output/cmd-server-openapi-stderr", &stderr_text); - - let spec: OpenAPI = serde_json::from_str(&stdout_text) - .expect("stdout was not valid OpenAPI"); - - // Check for lint errors. - let errors = openapi_lint::validate(&spec); - assert!(errors.is_empty(), "{}", errors.join("\n\n")); - - // Confirm that the output hasn't changed. It's expected that we'll change - // this file as the API evolves, but pay attention to the diffs to ensure - // that the changes match your expectations. - assert_contents("../openapi/installinator-artifactd.json", &stdout_text); -} diff --git a/installinator-artifactd/tests/mod.rs b/installinator-artifactd/tests/mod.rs deleted file mode 100644 index 66fee5d99c..0000000000 --- a/installinator-artifactd/tests/mod.rs +++ /dev/null @@ -1,17 +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/. - -//! Integration tests for the installinator artifact server. -//! -//! Why use this weird layer of indirection, you might ask? Cargo chooses to -//! compile *each file* within the "tests/" subdirectory as a separate crate. -//! This means that doing "file-granularity" conditional compilation is -//! difficult, since a file like "test_for_illumos_only.rs" would get compiled -//! and tested regardless of the contents of "mod.rs". -//! -//! However, by lumping all tests into a submodule, all integration tests are -//! joined into a single crate, which itself can filter individual files -//! by (for example) choice of target OS. - -mod integration_tests; diff --git a/installinator-artifactd/tests/output/cmd-server-openapi-stderr b/installinator-artifactd/tests/output/cmd-server-openapi-stderr deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/installinator/Cargo.toml b/installinator/Cargo.toml index c21c3f2ee2..00dfb6440b 100644 --- a/installinator/Cargo.toml +++ b/installinator/Cargo.toml @@ -20,7 +20,7 @@ futures.workspace = true hex.workspace = true http.workspace = true illumos-utils.workspace = true -installinator-artifact-client.workspace = true +installinator-client.workspace = true installinator-common.workspace = true ipcc.workspace = true itertools.workspace = true diff --git a/installinator/src/artifact.rs b/installinator/src/artifact.rs index 734759a2c2..12e85e0938 100644 --- a/installinator/src/artifact.rs +++ b/installinator/src/artifact.rs @@ -7,7 +7,7 @@ use std::net::SocketAddr; use anyhow::{Context, Result}; use clap::Args; use futures::StreamExt; -use installinator_artifact_client::ClientError; +use installinator_client::ClientError; use installinator_common::EventReport; use ipcc::{InstallinatorImageId, Ipcc}; use omicron_common::update::{ArtifactHash, ArtifactHashId}; @@ -63,7 +63,7 @@ impl ArtifactIdOpts { #[derive(Debug)] pub(crate) struct ArtifactClient { log: slog::Logger, - client: installinator_artifact_client::Client, + client: installinator_client::Client, } impl ArtifactClient { @@ -81,8 +81,7 @@ impl ArtifactClient { let log = log.new( slog::o!("component" => "ArtifactClient", "peer" => addr.to_string()), ); - let client = - installinator_artifact_client::Client::new(&endpoint, log.clone()); + let client = installinator_client::Client::new(&endpoint, log.clone()); Self { log, client } } diff --git a/installinator/src/errors.rs b/installinator/src/errors.rs index 1349cf7d89..577d0d6f4d 100644 --- a/installinator/src/errors.rs +++ b/installinator/src/errors.rs @@ -4,7 +4,7 @@ use std::{net::SocketAddr, time::Duration}; -use installinator_artifact_client::ClientError; +use installinator_client::ClientError; use thiserror::Error; #[derive(Debug, Error)] diff --git a/installinator/src/mock_peers.rs b/installinator/src/mock_peers.rs index 434276649f..ccb35a2f06 100644 --- a/installinator/src/mock_peers.rs +++ b/installinator/src/mock_peers.rs @@ -16,7 +16,7 @@ use std::{ use anyhow::{bail, Result}; use async_trait::async_trait; use bytes::Bytes; -use installinator_artifact_client::{ClientError, ResponseValue}; +use installinator_client::{ClientError, ResponseValue}; use installinator_common::EventReport; use omicron_common::update::ArtifactHashId; use proptest::prelude::*; @@ -342,7 +342,7 @@ impl MockPeer { tokio::time::sleep(after).await; _ = sender .send(Err(ClientError::ErrorResponse(ResponseValue::new( - installinator_artifact_client::types::Error { + installinator_client::types::Error { error_code: None, message: format!("not-found error after {after:?}"), request_id: "mock-request-id".to_owned(), @@ -356,7 +356,7 @@ impl MockPeer { tokio::time::sleep(after).await; _ = sender .send(Err(ClientError::ErrorResponse(ResponseValue::new( - installinator_artifact_client::types::Error { + installinator_client::types::Error { error_code: None, message: format!("forbidden error after {after:?}"), request_id: "mock-request-id".to_owned(), @@ -526,7 +526,7 @@ impl PeersImpl for MockReportPeers { Ok(()) } else if peer == Self::invalid_peer() { Err(ClientError::ErrorResponse(ResponseValue::new( - installinator_artifact_client::types::Error { + installinator_client::types::Error { error_code: None, message: "invalid peer => HTTP 422".to_owned(), request_id: "mock-request-id".to_owned(), diff --git a/installinator/src/peers.rs b/installinator/src/peers.rs index 644507da4b..3d2e05077d 100644 --- a/installinator/src/peers.rs +++ b/installinator/src/peers.rs @@ -16,7 +16,7 @@ use buf_list::BufList; use bytes::Bytes; use display_error_chain::DisplayErrorChain; use futures::{Stream, StreamExt}; -use installinator_artifact_client::ClientError; +use installinator_client::ClientError; use installinator_common::{ EventReport, InstallinatorProgressMetadata, StepContext, StepProgress, }; diff --git a/openapi/installinator-artifactd.json b/openapi/installinator.json similarity index 99% rename from openapi/installinator-artifactd.json rename to openapi/installinator.json index 61f555e10d..0631344b25 100644 --- a/openapi/installinator-artifactd.json +++ b/openapi/installinator.json @@ -1,8 +1,8 @@ { "openapi": "3.0.3", "info": { - "title": "Oxide Installinator Artifact Server", - "description": "API for use by the installinator to retrieve artifacts", + "title": "Installinator API", + "description": "API for installinator to fetch artifacts and report progress", "contact": { "url": "https://oxide.computer", "email": "api@oxide.computer" diff --git a/wicketd/Cargo.toml b/wicketd/Cargo.toml index bfd8a4cf45..792201c6ff 100644 --- a/wicketd/Cargo.toml +++ b/wicketd/Cargo.toml @@ -52,7 +52,7 @@ uuid.workspace = true bootstrap-agent-client.workspace = true omicron-ddm-admin-client.workspace = true gateway-client.workspace = true -installinator-artifactd.workspace = true +installinator-api.workspace = true installinator-common.workspace = true omicron-certificates.workspace = true omicron-common.workspace = true @@ -76,7 +76,7 @@ fs-err.workspace = true gateway-test-utils.workspace = true http.workspace = true installinator.workspace = true -installinator-artifact-client.workspace = true +installinator-client.workspace = true maplit.workspace = true omicron-test-utils.workspace = true openapi-lint.workspace = true diff --git a/wicketd/src/artifacts.rs b/wicketd/src/artifacts.rs index 3e5854d17e..59981b2ac3 100644 --- a/wicketd/src/artifacts.rs +++ b/wicketd/src/artifacts.rs @@ -5,5 +5,6 @@ mod server; mod store; -pub(crate) use self::server::WicketdArtifactServer; +pub(crate) use self::server::WicketdInstallinatorApiImpl; +pub(crate) use self::server::WicketdInstallinatorContext; pub(crate) use self::store::WicketdArtifactStore; diff --git a/wicketd/src/artifacts/server.rs b/wicketd/src/artifacts/server.rs index 3808f01753..6d677c7b4f 100644 --- a/wicketd/src/artifacts/server.rs +++ b/wicketd/src/artifacts/server.rs @@ -2,62 +2,99 @@ // 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/. -use super::store::WicketdArtifactStore; use crate::installinator_progress::IprArtifactServer; -use async_trait::async_trait; +use dropshot::FreeformBody; use dropshot::HttpError; +use dropshot::HttpResponseHeaders; +use dropshot::HttpResponseOk; +use dropshot::HttpResponseUpdatedNoContent; +use dropshot::Path; +use dropshot::RequestContext; +use dropshot::TypedBody; use hyper::Body; -use installinator_artifactd::ArtifactGetter; -use installinator_artifactd::EventReportStatus; +use installinator_api::body_to_artifact_response; +use installinator_api::InstallinatorApi; +use installinator_api::ReportQuery; +use installinator_common::EventReport; use omicron_common::update::ArtifactHashId; use slog::error; use slog::Logger; -use uuid::Uuid; + +use super::WicketdArtifactStore; + +pub(crate) enum WicketdInstallinatorApiImpl {} /// The artifact server interface for wicketd. #[derive(Debug)] -pub(crate) struct WicketdArtifactServer { - #[allow(dead_code)] +pub struct WicketdInstallinatorContext { log: Logger, store: WicketdArtifactStore, ipr_artifact: IprArtifactServer, } -impl WicketdArtifactServer { +impl WicketdInstallinatorContext { pub(crate) fn new( log: &Logger, store: WicketdArtifactStore, ipr_artifact: IprArtifactServer, ) -> Self { - let log = log.new(slog::o!("component" => "wicketd artifact server")); - Self { log, store, ipr_artifact } + Self { + log: log + .new(slog::o!("component" => "wicketd installinator server")), + store, + ipr_artifact, + } } } -#[async_trait] -impl ArtifactGetter for WicketdArtifactServer { - async fn get_by_hash(&self, id: &ArtifactHashId) -> Option<(u64, Body)> { - let data_handle = self.store.get_by_hash(id)?; - let size = data_handle.file_size() as u64; - let data_stream = match data_handle.reader_stream().await { - Ok(stream) => stream, - Err(err) => { - error!( - self.log, "failed to open extracted archive on demand"; - "error" => #%err, - ); - return None; - } - }; +impl InstallinatorApi for WicketdInstallinatorApiImpl { + type Context = WicketdInstallinatorContext; + + async fn get_artifact_by_hash( + rqctx: RequestContext, + path: Path, + ) -> Result>, HttpError> + { + let context = rqctx.context(); + match context.store.get_by_hash(&path.into_inner()) { + Some(data_handle) => { + let size = data_handle.file_size() as u64; + let data_stream = match data_handle.reader_stream().await { + Ok(stream) => stream, + Err(err) => { + error!( + context.log, "failed to open extracted archive on demand"; + "error" => #%err, + ); + return Err(HttpError::for_internal_error(format!( + // TODO: print error chain + "Artifact not found: {err}" + ))); + } + }; - Some((size, Body::wrap_stream(data_stream))) + Ok(body_to_artifact_response( + size, + Body::wrap_stream(data_stream), + )) + } + None => { + Err(HttpError::for_not_found(None, "Artifact not found".into())) + } + } } async fn report_progress( - &self, - update_id: Uuid, - report: installinator_common::EventReport, - ) -> Result { - Ok(self.ipr_artifact.report_progress(update_id, report)) + rqctx: RequestContext, + path: Path, + report: TypedBody, + ) -> Result { + let context = rqctx.context(); + let update_id = path.into_inner().update_id; + + context + .ipr_artifact + .report_progress(update_id, report.into_inner()) + .to_http_result(update_id) } } diff --git a/wicketd/src/bin/wicketd.rs b/wicketd/src/bin/wicketd.rs index 4037bc4c23..6ef616d708 100644 --- a/wicketd/src/bin/wicketd.rs +++ b/wicketd/src/bin/wicketd.rs @@ -144,9 +144,8 @@ async fn do_run() -> Result<(), CmdError> { .to_logger("wicketd") .context("failed to initialize logger") .map_err(CmdError::Failure)?; - let server = Server::start(log, args) - .await - .map_err(|err| CmdError::Failure(anyhow!(err)))?; + let server = + Server::start(log, args).await.map_err(CmdError::Failure)?; server .wait_for_finish() .await diff --git a/wicketd/src/installinator_progress.rs b/wicketd/src/installinator_progress.rs index 77baec2c94..7d076e7b0e 100644 --- a/wicketd/src/installinator_progress.rs +++ b/wicketd/src/installinator_progress.rs @@ -12,7 +12,7 @@ use std::{ sync::{Arc, Mutex}, }; -use installinator_artifactd::EventReportStatus; +use installinator_api::EventReportStatus; use tokio::sync::{oneshot, watch}; use update_engine::events::StepEventIsTerminal; use uuid::Uuid; diff --git a/wicketd/src/lib.rs b/wicketd/src/lib.rs index 5926fc468d..9fb204b675 100644 --- a/wicketd/src/lib.rs +++ b/wicketd/src/lib.rs @@ -16,8 +16,11 @@ mod preflight_check; mod rss_config; mod update_tracker; -use anyhow::{anyhow, Context, Result}; -use artifacts::{WicketdArtifactServer, WicketdArtifactStore}; +use anyhow::{anyhow, bail, Context, Result}; +use artifacts::{ + WicketdArtifactStore, WicketdInstallinatorApiImpl, + WicketdInstallinatorContext, +}; use bootstrap_addrs::BootstrapPeers; pub use config::Config; pub(crate) use context::ServerContext; @@ -118,7 +121,7 @@ impl SmfConfigValues { pub struct Server { pub wicketd_server: HttpServer, - pub artifact_server: HttpServer, + pub installinator_server: HttpServer, pub artifact_store: WicketdArtifactStore, pub update_tracker: Arc, pub ipr_update_tracker: IprUpdateTracker, @@ -127,14 +130,14 @@ pub struct Server { impl Server { /// Run an instance of the wicketd server - pub async fn start(log: slog::Logger, args: Args) -> Result { + pub async fn start(log: slog::Logger, args: Args) -> anyhow::Result { let (drain, registration) = slog_dtrace::with_drain(log); let log = slog::Logger::root(drain.fuse(), slog::o!(FileKv)); if let slog_dtrace::ProbeRegistration::Failed(e) = registration { let msg = format!("failed to register DTrace probes: {}", e); error!(log, "{}", msg); - return Err(msg); + bail!(msg); } else { debug!(log, "registered DTrace probes"); }; @@ -174,7 +177,8 @@ impl Server { addr, ) .map_err(|err| { - format!("Could not create internal DNS resolver: {err}") + anyhow!(err) + .context("Could not create internal DNS resolver") }) }) .transpose()?; @@ -186,7 +190,9 @@ impl Server { &log, ) .await - .map_err(|err| format!("failed to start Nexus TCP proxy: {err}"))?; + .map_err(|err| { + anyhow!(err).context("failed to start Nexus TCP proxy") + })?; let wicketd_server = { let ds_log = log.new(o!("component" => "dropshot (wicketd)")); @@ -209,25 +215,39 @@ impl Server { }, &ds_log, ) - .map_err(|err| format!("initializing http server: {}", err))? + .map_err(|err| anyhow!(err).context("initializing http server"))? .start() }; - let server = - WicketdArtifactServer::new(&log, store.clone(), ipr_artifact); - let artifact_server = installinator_artifactd::ArtifactServer::new( - server, - args.artifact_address, - &log, - ) - .start() - .map_err(|error| { - format!("failed to start artifact server: {error:?}") - })?; + let installinator_server = { + let installinator_config = installinator_api::default_config( + SocketAddr::V6(args.artifact_address), + ); + let api_description = + installinator_api::installinator_api::api_description::< + WicketdInstallinatorApiImpl, + >()?; + + dropshot::HttpServerStarter::new( + &installinator_config, + api_description, + WicketdInstallinatorContext::new( + &log, + store.clone(), + ipr_artifact, + ), + &log, + ) + .map_err(|err| { + anyhow!(err) + .context("failed to create installinator artifact server") + })? + .start() + }; Ok(Self { wicketd_server, - artifact_server, + installinator_server, artifact_store: store, update_tracker, ipr_update_tracker, @@ -240,7 +260,7 @@ impl Server { self.wicketd_server.close().await.map_err(|error| { anyhow!("error closing wicketd server: {error}") })?; - self.artifact_server.close().await.map_err(|error| { + self.installinator_server.close().await.map_err(|error| { anyhow!("error closing artifact server: {error}") })?; self.nexus_tcp_proxy.shutdown(); @@ -257,7 +277,7 @@ impl Server { Err(err) => Err(format!("running wicketd server: {err}")), } } - res = self.artifact_server => { + res = self.installinator_server => { match res { Ok(()) => Err("artifact server exited unexpectedly".to_owned()), // The artifact server returns an anyhow::Error, which has a diff --git a/wicketd/tests/integration_tests/setup.rs b/wicketd/tests/integration_tests/setup.rs index 62682a73ab..01f01e21e1 100644 --- a/wicketd/tests/integration_tests/setup.rs +++ b/wicketd/tests/integration_tests/setup.rs @@ -16,7 +16,7 @@ pub struct WicketdTestContext { // this way. pub wicketd_raw_client: ClientTestContext, pub artifact_addr: SocketAddrV6, - pub artifact_client: installinator_artifact_client::Client, + pub artifact_client: installinator_client::Client, pub server: wicketd::Server, pub gateway: GatewayTestContext, } @@ -62,14 +62,15 @@ impl WicketdTestContext { ) }; - let artifact_addr = assert_ipv6(server.artifact_server.local_addr()); + let artifact_addr = + assert_ipv6(server.installinator_server.local_addr()); let artifact_client = { let endpoint = format!( "http://[{}]:{}", artifact_addr.ip(), artifact_addr.port() ); - installinator_artifact_client::Client::new( + installinator_client::Client::new( &endpoint, log.new(slog::o!("component" => "artifact test client")), )