Move alb_health_check module out of the plugin module

The current module location is misleading, since you never want to run
the layer as a plugin: plugins always run after routing, and the health
check should be enacted before routing. Examples have been corrected,
and a test has been added.

CHANGELOG.next.toml

@@ -658,3 +658,9 @@ message = "The AppName property can now be set with `sdk_ua_app_id` in profile f
 references = ["smithy-rs#2724"]
 meta = { "breaking" = false, "tada" = false, "bug" = false }
 author = "rcoh"
+
+[[smithy-rs]]
+message = "The `alb_health_check` module has been moved out of the `plugin` module into a new `layer` module. ALB health checks should be enacted before routing, and plugins run after routing, so the module location was misleading. Examples have been corrected to reflect the intended application of the layer."
+references = ["smithy-rs#2865"]
+meta = { "breaking" = true, "tada" = false, "bug" = false, "target" = "server" }
+author = "david-perez"

examples/pokemon-service/src/main.rs

@@ -10,7 +10,8 @@ use std::{net::SocketAddr, sync::Arc};
 use aws_smithy_http_server::{
     extension::OperationExtensionExt,
     instrumentation::InstrumentExt,
-    plugin::{alb_health_check::AlbHealthCheckLayer, HttpPlugins, IdentityPlugin, Scoped},
+    layer::alb_health_check::AlbHealthCheckLayer,
+    plugin::{HttpPlugins, IdentityPlugin, Scoped},
     request::request_id::ServerRequestIdProviderLayer,
     AddExtensionLayer,
 };
@@ -61,11 +62,7 @@ pub async fn main() {
         // `Response::extensions`, or infer routing failure when it's missing.
         .insert_operation_extension()
         // Adds `tracing` spans and events to the request lifecycle.
-        .instrument()
-        // Handle `/ping` health check requests.
-        .layer(AlbHealthCheckLayer::from_handler("/ping", |_req| async {
-            StatusCode::OK
-        }));
+        .instrument();
 
     let app = PokemonService::builder_with_plugins(plugins, IdentityPlugin)
         // Build a registry containing implementations to all the operations in the service. These
@@ -84,7 +81,11 @@ pub async fn main() {
     let app = app
         // Setup shared state and middlewares.
         .layer(&AddExtensionLayer::new(Arc::new(State::default())))
-        // Add request IDs
+        // Handle `/ping` health check requests.
+        .layer(&AlbHealthCheckLayer::from_handler("/ping", |_req| async {
+            StatusCode::OK
+        }))
+        // Add server request IDs.
         .layer(&ServerRequestIdProviderLayer::new());
 
     // Using `into_make_service_with_connect_info`, rather than `into_make_service`, to adjoin the `SocketAddr`

examples/pokemon-service/tests/simple.rs

@@ -89,3 +89,31 @@ async fn simple_integration_test() {
 
     assert_eq!(result.status(), 200);
 }
+
+#[tokio::test]
+#[serial]
+async fn health_check() {
+    let _child = common::run_server().await;
+
+    use pokemon_service::{DEFAULT_ADDRESS, DEFAULT_PORT};
+    let url = format!("http://{DEFAULT_ADDRESS}:{DEFAULT_PORT}/ping");
+    let uri = url.parse::<hyper::Uri>().expect("invalid URL");
+
+    // Since the `/ping` route is not modeled in Smithy, we use a regular
+    // Hyper HTTP client to make a request to it.
+    let request = hyper::Request::builder()
+        .uri(uri)
+        .body(hyper::Body::empty())
+        .expect("failed to build request");
+
+    let response = hyper::Client::new()
+        .request(request)
+        .await
+        .expect("failed to get response");
+
+    assert_eq!(response.status(), hyper::StatusCode::OK);
+    let body = hyper::body::to_bytes(response.into_body())
+        .await
+        .expect("failed to read response body");
+    assert!(body.is_empty());
+}

rust-runtime/aws-smithy-http-server/src/plugin/alb_health_check.rs → rust-runtime/aws-smithy-http-server/src/layer/alb_health_check.rs

@@ -9,14 +9,17 @@
 //! # Example
 //!
 //! ```no_run
-//! # use aws_smithy_http_server::{body, plugin::{HttpPlugins, alb_health_check::AlbHealthCheckLayer}};
-//! # use hyper::{Body, Response, StatusCode};
-//! let plugins = HttpPlugins::new()
-//!     // Handle all `/ping` health check requests by returning a `200 OK`.
-//!     .layer(AlbHealthCheckLayer::from_handler("/ping", |_req| async {
-//!         StatusCode::OK
-//!     }));
+//! use aws_smithy_http_server::layer::alb_health_check::AlbHealthCheckLayer;
+//! use hyper::StatusCode;
+//! use tower::Layer;
 //!
+//! // Handle all `/ping` health check requests by returning a `200 OK`.
+//! let ping_layer = AlbHealthCheckLayer::from_handler("/ping", |_req| async {
+//!     StatusCode::OK
+//! });
+//! # async fn handle() { }
+//! let app = tower::service_fn(handle);
+//! let app = ping_layer.layer(app);
 //! ```
 
 use std::borrow::Cow;
@@ -31,8 +34,8 @@ use tower::{service_fn, util::Oneshot, Layer, Service, ServiceExt};
 
 use crate::body::BoxBody;
 
-use super::either::EitherProj;
-use super::Either;
+use crate::plugin::either::Either;
+use crate::plugin::either::EitherProj;
 
 /// A [`tower::Layer`] used to apply [`AlbHealthCheckService`].
 #[derive(Clone, Debug)]
@@ -96,9 +99,7 @@ where
     H: Service<Request<Body>, Response = StatusCode, Error = Infallible> + Clone,
 {
     type Response = S::Response;
-
     type Error = S::Error;
-
     type Future = AlbHealthCheckFuture<H, S>;
 
     fn poll_ready(&mut self, _cx: &mut Context<'_>) -> Poll<Result<(), Self::Error>> {
@@ -133,7 +134,11 @@ pin_project! {
     }
 }
 
-impl<H: Service<Request<Body>, Response = StatusCode>, S: Service<Request<Body>>> AlbHealthCheckFuture<H, S> {
+impl<H, S> AlbHealthCheckFuture<H, S>
+where
+    H: Service<Request<Body>, Response = StatusCode>,
+    S: Service<Request<Body>>,
+{
     fn handler_future(handler_future: Oneshot<H, Request<Body>>) -> Self {
         Self {
             inner: Either::Left { value: handler_future },
@@ -147,10 +152,10 @@ impl<H: Service<Request<Body>, Response = StatusCode>, S: Service<Request<Body>>
     }
 }
 
-impl<
-        H: Service<Request<Body>, Response = StatusCode, Error = Infallible>,
-        S: Service<Request<Body>, Response = Response<BoxBody>>,
-    > Future for AlbHealthCheckFuture<H, S>
+impl<H, S> Future for AlbHealthCheckFuture<H, S>
+where
+    H: Service<Request<Body>, Response = StatusCode, Error = Infallible>,
+    S: Service<Request<Body>, Response = Response<BoxBody>>,
 {
     type Output = Result<S::Response, S::Error>;
 

rust-runtime/aws-smithy-http-server/src/layer/mod.rs

@@ -0,0 +1,9 @@
+/*
+ * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+//! This module hosts [`Layer`](tower::Layer)s that are generally meant to be applied _around_ the
+//! [`Router`](crate::routing::Router), so they are enacted before a request is routed.
+
+pub mod alb_health_check;

rust-runtime/aws-smithy-http-server/src/lib.rs

@@ -16,6 +16,7 @@ pub mod body;
 pub(crate) mod error;
 pub mod extension;
 pub mod instrumentation;
+pub mod layer;
 pub mod operation;
 pub mod plugin;
 #[doc(hidden)]

rust-runtime/aws-smithy-http-server/src/plugin/either.rs

@@ -15,6 +15,8 @@ use tower::{Layer, Service};
 
 use super::Plugin;
 
+// TODO(https://github.com/awslabs/smithy-rs/pull/2441#pullrequestreview-1331345692): Seems like
+// this type should land in `tower-0.5`.
 pin_project! {
     /// Combine two different [`Futures`](std::future::Future)/[`Services`](tower::Service)/
     /// [`Layers`](tower::Layer)/[`Plugins`](super::Plugin) into a single type.

rust-runtime/aws-smithy-http-server/src/plugin/mod.rs

@@ -194,9 +194,8 @@
 //! impl ModelMarker for PrintPlugin { }
 //! ```
 
-pub mod alb_health_check;
 mod closure;
-mod either;
+pub(crate) mod either;
 mod filter;
 mod http_plugins;
 mod identity;