From 9f500e49782314261ae9fd33c64b8cc6e4ae94f8 Mon Sep 17 00:00:00 2001 From: Philip Lin Date: Mon, 15 Oct 2018 10:37:39 -0700 Subject: [PATCH] Update iotedged to return image pull failed message (#414) * Output more details when image pull error for invalid image name, invalid image host and unauthentication for image pull. * Add new tests for image pull scenarios. * Manually ran E2E tests to ensure details error message is updated in Edge Agent module twin. --- edgelet/edgelet-docker/src/error.rs | 52 ++- edgelet/edgelet-docker/tests/runtime.rs | 344 ++++++++++++++++-- .../src/server/module/mod.rs | 31 +- 3 files changed, 373 insertions(+), 54 deletions(-) diff --git a/edgelet/edgelet-docker/src/error.rs b/edgelet/edgelet-docker/src/error.rs index 868f95b279b..dd2b31869a3 100644 --- a/edgelet/edgelet-docker/src/error.rs +++ b/edgelet/edgelet-docker/src/error.rs @@ -8,7 +8,7 @@ use hyper::{Error as HyperError, StatusCode}; use serde_json; use url::ParseError; -use docker::apis::Error as DockerError; +use docker::apis::{ApiError as DockerApiError, Error as DockerError}; use edgelet_core::{Error as CoreError, ErrorKind as CoreErrorKind}; use edgelet_http::Error as HttpError; use edgelet_utils::Error as UtilsError; @@ -20,6 +20,26 @@ pub struct Error { inner: Context, } +fn get_message( + error: DockerApiError, +) -> ::std::result::Result> { + let DockerApiError { code, content } = error; + + match content { + Some(serde_json::Value::Object(props)) => { + if let serde_json::Value::String(message) = &props["message"] { + return Ok(message.clone()); + } + + Err(DockerApiError { + code, + content: Some(serde_json::Value::Object(props)), + }) + } + _ => Err(DockerApiError { code, content }), + } +} + #[derive(Debug, Fail)] pub enum ErrorKind { #[fail(display = "Invalid docker URI - {}", _0)] @@ -34,14 +54,16 @@ pub enum ErrorKind { Transport, #[fail(display = "Invalid URL")] UrlParse, - #[fail(display = "Not found")] - NotFound, + #[fail(display = "{}", _0)] + NotFound(String), #[fail(display = "Conflict with current operation")] Conflict, #[fail(display = "Container already in this state")] NotModified, #[fail(display = "Container runtime error")] Docker, + #[fail(display = "{}", _0)] + FormattedDockerRuntime(String), #[fail(display = "Container runtime error - {:?}", _0)] DockerRuntime(DockerError), #[fail(display = "Core error")] @@ -127,16 +149,20 @@ impl From> for Error { DockerError::Serde(error) => Error { inner: Error::from(error).context(ErrorKind::Docker), }, - DockerError::ApiError(ref error) if error.code == StatusCode::NOT_FOUND => { - Error::from(ErrorKind::NotFound) - } - DockerError::ApiError(ref error) if error.code == StatusCode::CONFLICT => { - Error::from(ErrorKind::Conflict) - } - DockerError::ApiError(ref error) if error.code == StatusCode::NOT_MODIFIED => { - Error::from(ErrorKind::NotModified) - } - _ => Error::from(ErrorKind::DockerRuntime(err)), + DockerError::ApiError(error) => match error.code { + StatusCode::NOT_FOUND => get_message(error) + .map(|message| Error::from(ErrorKind::NotFound(message))) + .unwrap_or_else(|e| { + Error::from(ErrorKind::DockerRuntime(DockerError::ApiError(e))) + }), + StatusCode::CONFLICT => Error::from(ErrorKind::Conflict), + StatusCode::NOT_MODIFIED => Error::from(ErrorKind::NotModified), + _ => get_message(error) + .map(|message| Error::from(ErrorKind::FormattedDockerRuntime(message))) + .unwrap_or_else(|e| { + Error::from(ErrorKind::DockerRuntime(DockerError::ApiError(e))) + }), + }, } } } diff --git a/edgelet/edgelet-docker/tests/runtime.rs b/edgelet/edgelet-docker/tests/runtime.rs index 0a4aef6b16a..86dee279b15 100644 --- a/edgelet/edgelet-docker/tests/runtime.rs +++ b/edgelet/edgelet-docker/tests/runtime.rs @@ -40,6 +40,272 @@ use edgelet_test_utils::{get_unused_tcp_port, run_tcp_server}; const IMAGE_NAME: &str = "nginx:latest"; +#[cfg(unix)] +const INVALID_IMAGE_NAME: &str = "invalidname:latest"; +#[cfg(unix)] +const INVALID_IMAGE_HOST: &str = "invalidhost.com/nginx:latest"; + +#[cfg(unix)] +#[cfg_attr(feature = "cargo-clippy", allow(needless_pass_by_value))] +fn invalid_image_name_pull_handler( + req: Request, +) -> Box, Error = HyperError> + Send> { + // verify that path is /images/create and that the "fromImage" query + // parameter has the image name we expect + assert_eq!(req.uri().path(), "/images/create"); + + let query_map: HashMap = parse_query(req.uri().query().unwrap().as_bytes()) + .into_owned() + .collect(); + assert!(query_map.contains_key("fromImage")); + assert_eq!( + query_map.get("fromImage"), + Some(&INVALID_IMAGE_NAME.to_string()) + ); + + let response = format!( + r#"{{ + "message": "manifest for {} not found" + }} + "#, + &INVALID_IMAGE_NAME.to_string() + ); + + let response_len = response.len(); + + let mut response = Response::new(response.into()); + response + .headers_mut() + .typed_insert(&ContentLength(response_len as u64)); + response + .headers_mut() + .typed_insert(&ContentType(mime::APPLICATION_JSON)); + *response.status_mut() = hyper::StatusCode::NOT_FOUND; + + Box::new(future::ok(response)) +} + +// This test is super flaky on Windows for some reason. It keeps occassionally +// failing on Windows with error 10054 which means the server keeps dropping the +// socket for no reason apparently. +#[cfg(unix)] +#[test] +fn image_pull_with_invalid_image_name_fails() { + let port = get_unused_tcp_port(); + let server = run_tcp_server("127.0.0.1", port, invalid_image_name_pull_handler) + .map_err(|err| eprintln!("{}", err)); + + let mri = + DockerModuleRuntime::new(&Url::parse(&format!("http://localhost:{}/", port)).unwrap()) + .unwrap(); + + let auth = AuthConfig::new() + .with_username("u1".to_string()) + .with_password("bleh".to_string()) + .with_email("u1@bleh.com".to_string()) + .with_serveraddress("svr1".to_string()); + let config = + DockerConfig::new(INVALID_IMAGE_NAME, ContainerCreateBody::new(), Some(auth)).unwrap(); + + let task = mri.pull(&config); + + let mut runtime = tokio::runtime::current_thread::Runtime::new().unwrap(); + runtime.spawn(server); + + // Assert + let err = runtime + .block_on(task) + .expect_err("Expected runtime pull method to fail due to invalid image name."); + + if let edgelet_docker::ErrorKind::NotFound(message) = err.kind() { + assert_eq!( + &format!("manifest for {} not found", &INVALID_IMAGE_NAME.to_string()), + message + ); + } else { + panic!("Specific docker runtime message is expected for invalid image name."); + } +} + +#[cfg(unix)] +#[cfg_attr(feature = "cargo-clippy", allow(needless_pass_by_value))] +fn invalid_image_host_pull_handler( + req: Request, +) -> Box, Error = HyperError> + Send> { + // verify that path is /images/create and that the "fromImage" query + // parameter has the image name we expect + assert_eq!(req.uri().path(), "/images/create"); + + let query_map: HashMap = parse_query(req.uri().query().unwrap().as_bytes()) + .into_owned() + .collect(); + assert!(query_map.contains_key("fromImage")); + assert_eq!( + query_map.get("fromImage"), + Some(&INVALID_IMAGE_HOST.to_string()) + ); + + let response = format!( + r#" + {{ + "message":"Get https://invalidhost.com: dial tcp: lookup {} on X.X.X.X: no such host" + }} + "#, + &INVALID_IMAGE_HOST.to_string() + ); + let response_len = response.len(); + + let mut response = Response::new(response.into()); + response + .headers_mut() + .typed_insert(&ContentLength(response_len as u64)); + response + .headers_mut() + .typed_insert(&ContentType(mime::APPLICATION_JSON)); + *response.status_mut() = hyper::StatusCode::INTERNAL_SERVER_ERROR; + Box::new(future::ok(response)) +} + +// This test is super flaky on Windows for some reason. It keeps occassionally +// failing on Windows with error 10054 which means the server keeps dropping the +// socket for no reason apparently. +#[cfg(unix)] +#[test] +fn image_pull_with_invalid_image_host_fails() { + let port = get_unused_tcp_port(); + let server = run_tcp_server("127.0.0.1", port, invalid_image_host_pull_handler) + .map_err(|err| eprintln!("{}", err)); + + let mri = + DockerModuleRuntime::new(&Url::parse(&format!("http://localhost:{}/", port)).unwrap()) + .unwrap(); + + let auth = AuthConfig::new() + .with_username("u1".to_string()) + .with_password("bleh".to_string()) + .with_email("u1@bleh.com".to_string()) + .with_serveraddress("svr1".to_string()); + let config = + DockerConfig::new(INVALID_IMAGE_HOST, ContainerCreateBody::new(), Some(auth)).unwrap(); + + let task = mri.pull(&config); + + let mut runtime = tokio::runtime::current_thread::Runtime::new().unwrap(); + runtime.spawn(server); + + // Assert + let err = runtime + .block_on(task) + .expect_err("Expected runtime pull method to fail due to invalid image host."); + + if let edgelet_docker::ErrorKind::FormattedDockerRuntime(message) = err.kind() { + assert_eq!( + &format!( + "Get https://invalidhost.com: dial tcp: lookup {} on X.X.X.X: no such host", + &INVALID_IMAGE_HOST.to_string() + ), + message + ); + } else { + panic!("Specific docker runtime message is expected for invalid image host."); + } +} + +#[cfg(unix)] +#[cfg_attr(feature = "cargo-clippy", allow(needless_pass_by_value))] +fn image_pull_with_invalid_creds_handler( + req: Request, +) -> Box, Error = HyperError> + Send> { + // verify that path is /images/create and that the "fromImage" query + // parameter has the image name we expect + assert_eq!(req.uri().path(), "/images/create"); + + let query_map: HashMap = parse_query(req.uri().query().unwrap().as_bytes()) + .into_owned() + .collect(); + assert!(query_map.contains_key("fromImage")); + assert_eq!(query_map.get("fromImage"), Some(&IMAGE_NAME.to_string())); + + // verify registry creds + let auth_str = req + .headers() + .get_all("X-Registry-Auth") + .into_iter() + .map(|bytes| base64::decode(bytes).unwrap()) + .map(|raw| str::from_utf8(&raw).unwrap().to_owned()) + .collect::>() + .join(""); + let auth_config: AuthConfig = serde_json::from_str(&auth_str.to_string()).unwrap(); + assert_eq!(auth_config.username(), Some(&"u1".to_string())); + assert_eq!(auth_config.password(), Some(&"wrong_password".to_string())); + assert_eq!(auth_config.email(), Some(&"u1@bleh.com".to_string())); + assert_eq!(auth_config.serveraddress(), Some(&"svr1".to_string())); + + let response = format!( + r#" + {{ + "message":"Get {}: unauthorized: authentication required" + }} + "#, + IMAGE_NAME + ); + let response_len = response.len(); + + let mut response = Response::new(response.into()); + response + .headers_mut() + .typed_insert(&ContentLength(response_len as u64)); + response + .headers_mut() + .typed_insert(&ContentType(mime::APPLICATION_JSON)); + *response.status_mut() = hyper::StatusCode::INTERNAL_SERVER_ERROR; + Box::new(future::ok(response)) +} + +// This test is super flaky on Windows for some reason. It keeps occassionally +// failing on Windows with error 10054 which means the server keeps dropping the +// socket for no reason apparently. +#[cfg(unix)] +#[test] +fn image_pull_with_invalid_creds_fails() { + let port = get_unused_tcp_port(); + let server = run_tcp_server("127.0.0.1", port, image_pull_with_invalid_creds_handler) + .map_err(|err| eprintln!("{}", err)); + + let mri = + DockerModuleRuntime::new(&Url::parse(&format!("http://localhost:{}/", port)).unwrap()) + .unwrap(); + + let auth = AuthConfig::new() + .with_username("u1".to_string()) + .with_password("wrong_password".to_string()) + .with_email("u1@bleh.com".to_string()) + .with_serveraddress("svr1".to_string()); + let config = DockerConfig::new(IMAGE_NAME, ContainerCreateBody::new(), Some(auth)).unwrap(); + + let task = mri.pull(&config); + + let mut runtime = tokio::runtime::current_thread::Runtime::new().unwrap(); + runtime.spawn(server); + + // Assert + let err = runtime + .block_on(task) + .expect_err("Expected runtime pull method to fail due to unauthentication."); + + if let edgelet_docker::ErrorKind::FormattedDockerRuntime(message) = err.kind() { + assert_eq!( + &format!( + "Get {}: unauthorized: authentication required", + &IMAGE_NAME.to_string() + ), + message + ); + } else { + panic!("Specific docker runtime message is expected for unauthentication."); + } +} + #[cfg(unix)] #[cfg_attr(feature = "cargo-clippy", allow(needless_pass_by_value))] fn image_pull_handler( @@ -745,20 +1011,16 @@ fn runtime_init_network_exist_do_not_create() { //let mut got_called = false; - let server = - run_tcp_server( - "127.0.0.1", - port, - move |req: Request| { - let method = req.method(); - match method { - &Method::GET => { - let mut list_got_called_w = list_got_called_lock.write().unwrap(); - *list_got_called_w = true; - - assert_eq!(req.uri().path(), "/networks"); - - let response = json!([ + let server = run_tcp_server("127.0.0.1", port, move |req: Request| { + let method = req.method(); + match method { + &Method::GET => { + let mut list_got_called_w = list_got_called_lock.write().unwrap(); + *list_got_called_w = true; + + assert_eq!(req.uri().path(), "/networks"); + + let response = json!([ { "Name": "azure-iot-edge", "Id": "8e3209d08ed5e73d1c9c8e7580ddad232b6dceb5bf0c6d74cadbed75422eef0e", @@ -777,36 +1039,42 @@ fn runtime_init_network_exist_do_not_create() { "Options": {} } ]).to_string(); - let response_len = response.len(); + let response_len = response.len(); - let mut response = Response::new(response.into()); - response.headers_mut().typed_insert(&ContentLength(response_len as u64)); - response.headers_mut().typed_insert(&ContentType(mime::APPLICATION_JSON)); - return Box::new(future::ok(response)); - } - &Method::POST => { - //Netowk create. - let mut create_got_called_w = create_got_called_lock.write().unwrap(); - *create_got_called_w = true; + let mut response = Response::new(response.into()); + response + .headers_mut() + .typed_insert(&ContentLength(response_len as u64)); + response + .headers_mut() + .typed_insert(&ContentType(mime::APPLICATION_JSON)); + return Box::new(future::ok(response)); + } + &Method::POST => { + //Netowk create. + let mut create_got_called_w = create_got_called_lock.write().unwrap(); + *create_got_called_w = true; - assert_eq!(req.uri().path(), "/networks/create"); + assert_eq!(req.uri().path(), "/networks/create"); - let response = json!({ + let response = json!({ "Id": "12345", "Warnings": "" }).to_string(); - let response_len = response.len(); - - let mut response = Response::new(response.into()); - response.headers_mut().typed_insert(&ContentLength(response_len as u64)); - response.headers_mut().typed_insert(&ContentType(mime::APPLICATION_JSON)); - return Box::new(future::ok(response)); - } - _ => panic!("Method is not a get neither a post."), - } - }, - ) - .map_err(|err| eprintln!("{}", err)); + let response_len = response.len(); + + let mut response = Response::new(response.into()); + response + .headers_mut() + .typed_insert(&ContentLength(response_len as u64)); + response + .headers_mut() + .typed_insert(&ContentType(mime::APPLICATION_JSON)); + return Box::new(future::ok(response)); + } + _ => panic!("Method is not a get neither a post."), + } + }).map_err(|err| eprintln!("{}", err)); let mri = DockerModuleRuntime::new(&Url::parse(&format!("http://localhost:{}/", port)).unwrap()) diff --git a/edgelet/edgelet-http-mgmt/src/server/module/mod.rs b/edgelet/edgelet-http-mgmt/src/server/module/mod.rs index 0cd75c87ade..aa444dd06e0 100644 --- a/edgelet/edgelet-http-mgmt/src/server/module/mod.rs +++ b/edgelet/edgelet-http-mgmt/src/server/module/mod.rs @@ -47,7 +47,7 @@ impl IntoResponse for DockerError { } let status_code = match *self.kind() { - DockerErrorKind::NotFound => StatusCode::NOT_FOUND, + DockerErrorKind::NotFound(_) => StatusCode::NOT_FOUND, DockerErrorKind::Conflict => StatusCode::CONFLICT, DockerErrorKind::NotModified => StatusCode::NOT_MODIFIED, _ => StatusCode::INTERNAL_SERVER_ERROR, @@ -194,7 +194,9 @@ pub mod tests { #[test] fn not_found() { // arrange - let error = DockerError::from(DockerErrorKind::NotFound); + let error = DockerError::from(DockerErrorKind::NotFound( + "manifest for image:latest not found".to_string(), + )); // act let response = error.into_response(); @@ -206,7 +208,7 @@ pub mod tests { .concat2() .and_then(|b| { let error: ErrorResponse = serde_json::from_slice(&b).unwrap(); - assert_eq!("Not found", error.message()); + assert_eq!("manifest for image:latest not found", error.message()); Ok(()) }).wait() .unwrap(); @@ -253,4 +255,27 @@ pub mod tests { }).wait() .unwrap(); } + + #[test] + fn formatted_docker_runtime() { + // arrange + let error = DockerError::from(DockerErrorKind::FormattedDockerRuntime( + "manifest for image:latest not found".to_string(), + )); + + // act + let response = error.into_response(); + + // assert + assert_eq!(StatusCode::INTERNAL_SERVER_ERROR, response.status()); + response + .into_body() + .concat2() + .and_then(|b| { + let error: ErrorResponse = serde_json::from_slice(&b).unwrap(); + assert_eq!("manifest for image:latest not found", error.message()); + Ok(()) + }).wait() + .unwrap(); + } }