From 29bb337872b61c0046d490c0f5cad8961be15500 Mon Sep 17 00:00:00 2001 From: Hugo Caillard <911307+hugocaillard@users.noreply.github.com> Date: Wed, 23 Oct 2024 14:54:29 +0200 Subject: [PATCH] refactor: cargo clippy --- .github/workflows/ci.yaml | 21 ++- .vscode/settings.json | 3 + src/api_config.rs | 2 +- src/config.rs | 28 ++-- src/lib.rs | 234 ++++++++++++++-------------------- src/main.rs | 34 ++--- src/resources/service.rs | 2 +- src/resources/stateful_set.rs | 10 +- src/resources/tests.rs | 46 +++---- src/responder.rs | 18 +-- src/routes.rs | 24 ++-- src/tests/mod.rs | 107 +++++++--------- 12 files changed, 228 insertions(+), 301 deletions(-) create mode 100644 .vscode/settings.json diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index ac35284..3fa6207 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -14,20 +14,27 @@ concurrency: group: ${{ github.workflow }} @ ${{ github.event.pull_request.head.label || github.head_ref || github.ref }} cancel-in-progress: true +# Make sure CI fails on all warnings, including Clippy lints +env: + RUSTFLAGS: "-Dwarnings" + jobs: format: - name: Format check runs-on: ubuntu-latest steps: - - name: Checkout repository - uses: actions/checkout@v4 - - - name: Install Rust toolchain - run: rustup toolchain install stable --profile minimal --component rustfmt - + - uses: actions/checkout@v4 - name: Run rustfmt run: cargo fmt --all -- --check + clippy_check: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - name: test + run: cargo --version + - name: Run Clippy + run: cargo clippy --all-targets --all-features + build: runs-on: ubuntu-latest outputs: diff --git a/.vscode/settings.json b/.vscode/settings.json new file mode 100644 index 0000000..1c565eb --- /dev/null +++ b/.vscode/settings.json @@ -0,0 +1,3 @@ +{ + "rust-analyzer.check.command": "clippy" +} diff --git a/src/api_config.rs b/src/api_config.rs index f7f4bbf..865ed0d 100644 --- a/src/api_config.rs +++ b/src/api_config.rs @@ -42,7 +42,7 @@ impl ApiConfig { let config_file: ApiConfig = match toml::from_slice(&file_buffer) { Ok(s) => s, Err(e) => { - panic!("Config file malformatted {}", e.to_string()); + panic!("Config file malformatted {}", e); } }; config_file diff --git a/src/config.rs b/src/config.rs index 91d026f..65842c3 100644 --- a/src/config.rs +++ b/src/config.rs @@ -45,26 +45,23 @@ impl StacksDevnetConfig { ); if user_id != self.namespace { - let msg = + let message = format!("{context}, ERROR: devnet namespace must match authenticated user id"); - ctx.try_log(|logger| slog::warn!(logger, "{}", msg)); - return Err(DevNetError { - message: msg.into(), - code: 400, - }); + ctx.try_log(|logger| slog::warn!(logger, "{}", message)); + return Err(DevNetError { message, code: 400 }); } let project_manifest_yaml_string = self .get_project_manifest_yaml_string() - .map_err(|e| log_and_return_err(e, &context, &ctx))?; + .map_err(|e| log_and_return_err(e, &context, ctx))?; let (network_manifest_yaml_string, devnet_config) = self .get_network_manifest_string_and_devnet_config() - .map_err(|e| log_and_return_err(e, &context, &ctx))?; + .map_err(|e| log_and_return_err(e, &context, ctx))?; let deployment_plan_yaml_string = self .get_deployment_plan_yaml_string() - .map_err(|e| log_and_return_err(e, &context, &ctx))?; + .map_err(|e| log_and_return_err(e, &context, ctx))?; let mut contracts: Vec<(String, String)> = vec![]; for (contract_identifier, (src, _)) in self.deployment_plan.contracts { @@ -170,7 +167,7 @@ impl StacksDevnetConfig { spec.location = contracts_loc.clone(); }, TransactionSpecification::EmulatedContractCall(_) | TransactionSpecification::EmulatedContractPublish(_) => { - return Err(format!("devnet deployment plans do not support emulated-contract-calls or emulated-contract-publish types")) + return Err("devnet deployment plans do not support emulated-contract-calls or emulated-contract-publish types".to_string()) } TransactionSpecification::ContractCall(_) => {}, TransactionSpecification::BtcTransfer(_) => {}, @@ -184,12 +181,9 @@ impl StacksDevnetConfig { } fn log_and_return_err(e: String, context: &str, ctx: &Context) -> DevNetError { - let msg = format!("{context}, ERROR: {e}"); - ctx.try_log(|logger: &hiro_system_kit::Logger| slog::warn!(logger, "{}", msg)); - DevNetError { - message: msg.into(), - code: 400, - } + let message = format!("{context}, ERROR: {e}"); + ctx.try_log(|logger: &hiro_system_kit::Logger| slog::warn!(logger, "{}", message)); + DevNetError { message, code: 400 } } #[cfg(test)] mod tests { @@ -221,7 +215,7 @@ mod tests { let config_file: StacksDevnetConfig = match serde_json::from_slice(&file_buffer) { Ok(s) => s, Err(e) => { - panic!("Config file malformatted {}", e.to_string()); + panic!("Config file malformatted {}", e); } }; config_file diff --git a/src/lib.rs b/src/lib.rs index f384144..b74b89d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -116,7 +116,7 @@ impl StacksDevnetApiK8sManager { Err(_) => { if cfg!(test) { let is_ci = match env::var("GITHUB_ACTIONS") { - Ok(is_ci) => is_ci == format!("true"), + Ok(is_ci) => is_ci == *"true", Err(_) => false, }; if is_ci { @@ -124,7 +124,7 @@ impl StacksDevnetApiK8sManager { } else { // ensures that if no context is supplied and we're running // tests locally, we deploy to the local kind cluster - Some(format!("kind-kind")) + Some("kind-kind".to_string()) } } else { None @@ -138,14 +138,11 @@ impl StacksDevnetApiK8sManager { cluster: Some(context), user: None, }; - let client_config = - Config::from_kubeconfig(&kube_config) - .await - .unwrap_or_else(|e| { - panic!("could not create kube client config: {}", e.to_string()) - }); + let client_config = Config::from_kubeconfig(&kube_config) + .await + .unwrap_or_else(|e| panic!("could not create kube client config: {}", e)); Client::try_from(client_config) - .unwrap_or_else(|e| panic!("could not create kube client: {}", e.to_string())) + .unwrap_or_else(|e| panic!("could not create kube client: {}", e)) } None => Client::try_default() .await @@ -186,36 +183,32 @@ impl StacksDevnetApiK8sManager { let user_id = &config.user_id; let context = format!("NAMESPACE: {}", &namespace); - let namespace_exists = self.check_namespace_exists(&namespace).await?; + let namespace_exists = self.check_namespace_exists(namespace).await?; if !namespace_exists { if cfg!(debug_assertions) { - self.deploy_namespace(&namespace).await?; + self.deploy_namespace(namespace).await?; } else { - let msg = format!( + let message = format!( "cannot create devnet because namespace {} does not exist", namespace ); - self.ctx.try_log(|logger| slog::warn!(logger, "{}", msg)); - return Err(DevNetError { - message: msg.into(), - code: 400, - }); + self.ctx + .try_log(|logger| slog::warn!(logger, "{}", message)); + return Err(DevNetError { message, code: 400 }); } } let any_assets_exist = self - .check_any_devnet_assets_exist(&namespace, &user_id) + .check_any_devnet_assets_exist(namespace, user_id) .await?; if any_assets_exist { - let msg = format!( + let message = format!( "cannot create devnet because assets already exist {}", context ); - self.ctx.try_log(|logger| slog::warn!(logger, "{}", msg)); - return Err(DevNetError { - message: msg.into(), - code: 409, - }); + self.ctx + .try_log(|logger| slog::warn!(logger, "{}", message)); + return Err(DevNetError { message, code: 409 }); }; self.deploy_bitcoin_node(&config).await?; @@ -309,12 +302,12 @@ impl StacksDevnetApiK8sManager { if errors.is_empty() { Ok(()) } else if errors.len() == 1 { - match errors.get(0) { + match errors.first() { Some(e) => Err(e.clone()), None => unreachable!(), } } else { - let mut msg = format!("multiple errors occurred while deleting devnet: "); + let mut msg = "multiple errors occurred while deleting devnet: ".to_string(); for e in errors { msg = format!("{} \n- {}", msg, e.message); } @@ -325,15 +318,13 @@ impl StacksDevnetApiK8sManager { } } false => { - let msg = format!( + let message = format!( "cannot delete devnet because assets do not exist NAMESPACE: {}", &namespace ); - self.ctx.try_log(|logger| slog::warn!(logger, "{}", msg)); - return Err(DevNetError { - message: msg.into(), - code: 409, - }); + self.ctx + .try_log(|logger| slog::warn!(logger, "{}", message)); + Err(DevNetError { message, code: 409 }) } } } @@ -365,16 +356,10 @@ impl StacksDevnetApiK8sManager { } } Err(e) => { - let msg = format!( - "error getting namespace {}: {}", - namespace_str, - e.to_string() - ); - self.ctx.try_log(|logger| slog::error!(logger, "{}", msg)); - Err(DevNetError { - message: msg, - code: 500, - }) + let message = format!("error getting namespace {}: {}", namespace_str, e); + self.ctx + .try_log(|logger| slog::error!(logger, "{}", message)); + Err(DevNetError { message, code: 500 }) } } } @@ -512,7 +497,7 @@ impl StacksDevnetApiK8sManager { self.ctx.try_log(|logger: &hiro_system_kit::Logger| { slog::info!(logger, "getting pod status {}", context) }); - let pod_api: Api = Api::namespaced(self.client.to_owned(), &namespace); + let pod_api: Api = Api::namespaced(self.client.to_owned(), namespace); let pod_label_selector = format!("{COMPONENT_SELECTOR}={pod}"); let user_label_selector = format!("{USER_SELECTOR}={user_id}"); @@ -533,10 +518,7 @@ impl StacksDevnetApiK8sManager { self.ctx.try_log(|logger: &hiro_system_kit::Logger| { slog::info!(logger, "successfully retrieved pod status {}", context) }); - let start_time = match &status.start_time { - Some(st) => Some(st.0.to_string()), - None => None, - }; + let start_time = status.start_time.as_ref().map(|st| st.0.to_string()); Ok(PodStatusResponse { status: status.phase.to_owned(), start_time, @@ -600,7 +582,7 @@ impl StacksDevnetApiK8sManager { Ok(config) } Err(e) => { - let msg = format!("failed to parse JSON response: {}, ERROR: {}, Raw body: {}", context, e.to_string(), body_str); + let msg = format!("failed to parse JSON response: {}, ERROR: {}, Raw body: {}", context, e, body_str); self.ctx.try_log(|logger| slog::error!(logger, "{}", msg)); Err(DevNetError { message: msg, @@ -612,8 +594,7 @@ impl StacksDevnetApiK8sManager { Err(e) => { let msg = format!( "failed to parse response bytes: {}, ERROR: {}", - context, - e.to_string() + context, e ); self.ctx.try_log(|logger| slog::error!(logger, "{}", msg)); Err(DevNetError { @@ -623,18 +604,14 @@ impl StacksDevnetApiK8sManager { } }, Err(e) => { - let msg = format!( - "failed to query stacks node: {}, ERROR: {}", - context, - e.to_string() - ); + let msg = format!("failed to query stacks node: {}, ERROR: {}", context, e); self.ctx.try_log(|logger| slog::warn!(logger, "{}", msg)); Ok(StacksV2InfoResponse::default()) // Return default response on error } } } Err(e) => { - let msg = format!("failed to parse url: {} ERROR: {}", url, e.to_string()); + let msg = format!("failed to parse url: {} ERROR: {}", url, e); self.ctx.try_log(|logger| slog::error!(logger, "{}", msg)); Err(DevNetError { message: msg, @@ -651,7 +628,7 @@ impl StacksDevnetApiK8sManager { ) -> Result { let context = format!("NAMESPACE: {}", namespace); - match self.check_all_devnet_assets_exist(&namespace).await? { + match self.check_all_devnet_assets_exist(namespace).await? { false => { let msg = format!("not all devnet assets exist {}", context); self.ctx @@ -681,14 +658,10 @@ impl StacksDevnetApiK8sManager { start_time: stacks_api_started_at, }, ) = try_join3( - self.get_pod_status_info(&namespace, user_id, StacksDevnetPod::BitcoindNode), + self.get_pod_status_info(namespace, user_id, StacksDevnetPod::BitcoindNode), + self.get_pod_status_info(namespace, user_id, StacksDevnetPod::StacksBlockchain), self.get_pod_status_info( - &namespace, - user_id, - StacksDevnetPod::StacksBlockchain, - ), - self.get_pod_status_info( - &namespace, + namespace, user_id, StacksDevnetPod::StacksBlockchainApi, ), @@ -696,7 +669,7 @@ impl StacksDevnetApiK8sManager { .await?; // Try to fetch chain info, but handle errors by using default values for the chain tips - let chain_info = match self.get_stacks_v2_info(&namespace).await { + let chain_info = match self.get_stacks_v2_info(namespace).await { Ok(info) => info, Err(e) => { self.ctx.try_log(|logger: &hiro_system_kit::Logger| { @@ -759,7 +732,7 @@ impl StacksDevnetApiK8sManager { } } - async fn get_resource_by_label>( + async fn get_resource_by_label( &self, namespace: &str, name: &str, @@ -771,8 +744,9 @@ impl StacksDevnetApiK8sManager { K: DeserializeOwned, K: std::fmt::Debug, K: Serialize, + K: kube::Resource, { - let resource_api: Api = Api::namespaced(self.client.to_owned(), &namespace); + let resource_api: Api = Api::namespaced(self.client.to_owned(), namespace); let pod_label_selector = format!("{COMPONENT_SELECTOR}={name}"); let user_label_selector = format!("{USER_SELECTOR}={user_id}"); @@ -795,7 +769,7 @@ impl StacksDevnetApiK8sManager { match resource_api.list(&lp).await { Ok(pods) => { - if pods.items.len() > 0 { + if !pods.items.is_empty() { let pod = &pods.items[0]; Ok(Some(pod.clone())) } else { @@ -812,29 +786,24 @@ impl StacksDevnetApiK8sManager { } e => (e.to_string(), 500), }; - let msg = format!("failed to fetch {}, ERROR: {}", resource_details, msg); - self.ctx.try_log(|logger| slog::error!(logger, "{}", msg)); - Err(DevNetError { - message: msg, - code: code, - }) + let message = format!("failed to fetch {}, ERROR: {}", resource_details, msg); + self.ctx + .try_log(|logger| slog::error!(logger, "{}", message)); + Err(DevNetError { message, code }) } } } - async fn get_resource>( - &self, - namespace: &str, - name: &str, - ) -> Result, DevNetError> + async fn get_resource(&self, namespace: &str, name: &str) -> Result, DevNetError> where ::DynamicType: Default, K: Clone, K: DeserializeOwned, K: std::fmt::Debug, K: Serialize, + K: kube::Resource, { - let resource_api: Api = Api::namespaced(self.client.to_owned(), &namespace); + let resource_api: Api = Api::namespaced(self.client.to_owned(), namespace); let resource_details = format!( "RESOURCE: {}, NAME: {}, NAMESPACE: {}", @@ -845,7 +814,7 @@ impl StacksDevnetApiK8sManager { self.ctx .try_log(|logger| slog::info!(logger, "fetching {}", resource_details)); - match resource_api.get_opt(&name).await { + match resource_api.get_opt(name).await { Ok(r) => match r { Some(r) => { self.ctx.try_log(|logger| { @@ -865,17 +834,15 @@ impl StacksDevnetApiK8sManager { kube::Error::Api(api_error) => (api_error.message, api_error.code), e => (e.to_string(), 500), }; - let msg = format!("failed to fetch {}, ERROR: {}", resource_details, msg); - self.ctx.try_log(|logger| slog::error!(logger, "{}", msg)); - Err(DevNetError { - message: msg, - code: code, - }) + let message = format!("failed to fetch {}, ERROR: {}", resource_details, msg); + self.ctx + .try_log(|logger| slog::error!(logger, "{}", message)); + Err(DevNetError { message, code }) } } } - async fn check_resource_exists_by_label>( + async fn check_resource_exists_by_label( &self, namespace: &str, name: &str, @@ -887,6 +854,7 @@ impl StacksDevnetApiK8sManager { K: DeserializeOwned, K: std::fmt::Debug, K: Serialize, + K: kube::Resource, { match self .get_resource_by_label::(namespace, name, user_id) @@ -897,7 +865,7 @@ impl StacksDevnetApiK8sManager { } } - async fn check_resource_exists>( + async fn check_resource_exists( &self, namespace: &str, name: &str, @@ -908,6 +876,7 @@ impl StacksDevnetApiK8sManager { K: DeserializeOwned, K: std::fmt::Debug, K: Serialize, + K: kube::Resource, { match self.get_resource::(namespace, name).await? { Some(_) => Ok(true), @@ -915,7 +884,7 @@ impl StacksDevnetApiK8sManager { } } - async fn deploy_resource>( + async fn deploy_resource( &self, namespace: &str, resource: K, @@ -927,8 +896,9 @@ impl StacksDevnetApiK8sManager { K: DeserializeOwned, K: std::fmt::Debug, K: Serialize, + K: kube::Resource, { - let resource_api: Api = Api::namespaced(self.client.to_owned(), &namespace); + let resource_api: Api = Api::namespaced(self.client.to_owned(), namespace); let pp = PostParams::default(); let name = match resource.meta().name.as_ref() { @@ -1132,7 +1102,7 @@ impl StacksDevnetApiK8sManager { if let Some(configmap_data) = configmap_data { let mut map = BTreeMap::new(); for (key, value) in configmap_data { - map.insert(key.into(), value.into()); + map.insert(key, value); } configmap.data = Some(map); } @@ -1187,14 +1157,14 @@ impl StacksDevnetApiK8sManager { self.deploy_configmap( StacksDevnetConfigmap::BitcoindNode, - &namespace, + namespace, Some(vec![("bitcoin.conf".into(), bitcoind_conf)]), ) .await?; self.deploy_configmap( StacksDevnetConfigmap::ProjectManifest, - &namespace, + namespace, Some(vec![( "Clarinet.toml".into(), config.project_manifest_yaml_string.to_owned(), @@ -1204,7 +1174,7 @@ impl StacksDevnetApiK8sManager { self.deploy_configmap( StacksDevnetConfigmap::Devnet, - &namespace, + namespace, Some(vec![( "Devnet.toml".into(), config.network_manifest_yaml_string.to_owned(), @@ -1214,7 +1184,7 @@ impl StacksDevnetApiK8sManager { self.deploy_configmap( StacksDevnetConfigmap::DeploymentPlan, - &namespace, + namespace, Some(vec![( "default.devnet-plan.yaml".into(), config.deployment_plan_yaml_string.to_owned(), @@ -1224,15 +1194,15 @@ impl StacksDevnetApiK8sManager { self.deploy_configmap( StacksDevnetConfigmap::ProjectDir, - &namespace, + namespace, Some(config.contract_configmap_data.to_owned()), ) .await?; - self.deploy_deployment(StacksDevnetDeployment::BitcoindNode, &namespace, &user_id) + self.deploy_deployment(StacksDevnetDeployment::BitcoindNode, namespace, user_id) .await?; - self.deploy_service(StacksDevnetService::BitcoindNode, namespace, &user_id) + self.deploy_service(StacksDevnetService::BitcoindNode, namespace, user_id) .await?; Ok(()) @@ -1327,7 +1297,7 @@ impl StacksDevnetApiK8sManager { )); let bitcoind_chain_coordinator_host = - get_service_url(&namespace, StacksDevnetService::BitcoindNode); + get_service_url(namespace, StacksDevnetService::BitcoindNode); stacks_conf.push_str(&format!( r#" @@ -1350,7 +1320,7 @@ impl StacksDevnetApiK8sManager { include_data_events = false events_keys = ["*"] "#, - get_service_url(&namespace, StacksDevnetService::StacksBlockchainApi), + get_service_url(namespace, StacksDevnetService::StacksBlockchainApi), get_service_port(StacksDevnetService::StacksBlockchainApi, ServicePort::Event) .unwrap(), )); @@ -1358,12 +1328,12 @@ impl StacksDevnetApiK8sManager { for signer_idx in SignerIdx::iter() { let (url, port) = match signer_idx { SignerIdx::Signer0 => ( - get_service_url(&namespace, StacksDevnetService::StacksSigner0), + get_service_url(namespace, StacksDevnetService::StacksSigner0), get_service_port(StacksDevnetService::StacksSigner0, ServicePort::Event) .unwrap(), ), SignerIdx::Signer1 => ( - get_service_url(&namespace, StacksDevnetService::StacksSigner1), + get_service_url(namespace, StacksDevnetService::StacksSigner1), get_service_port(StacksDevnetService::StacksSigner1, ServicePort::Event) .unwrap(), ), @@ -1378,9 +1348,7 @@ impl StacksDevnetApiK8sManager { include_data_events = false events_keys = ["stackerdb", "block_proposal", "burn_blocks"] "#, - signer_idx.to_string(), - url, - port, + signer_idx, url, port, )); } @@ -1464,19 +1432,15 @@ impl StacksDevnetApiK8sManager { self.deploy_configmap( StacksDevnetConfigmap::StacksBlockchain, - &namespace, + namespace, Some(vec![("Stacks.toml".into(), stacks_conf)]), ) .await?; - self.deploy_deployment( - StacksDevnetDeployment::StacksBlockchain, - &namespace, - &user_id, - ) - .await?; + self.deploy_deployment(StacksDevnetDeployment::StacksBlockchain, namespace, user_id) + .await?; - self.deploy_service(StacksDevnetService::StacksBlockchain, namespace, &user_id) + self.deploy_service(StacksDevnetService::StacksBlockchain, namespace, user_id) .await?; Ok(()) @@ -1495,13 +1459,13 @@ impl StacksDevnetApiK8sManager { ]); self.deploy_configmap( StacksDevnetConfigmap::StacksBlockchainApiPg, - &namespace, + namespace, Some(stacks_api_pg_env), ) .await?; // configmap env vars for api conatainer - let stacks_node_host = get_service_url(&namespace, StacksDevnetService::StacksBlockchain); + let stacks_node_host = get_service_url(namespace, StacksDevnetService::StacksBlockchain); let rpc_port = get_service_port(StacksDevnetService::StacksBlockchain, ServicePort::RPC).unwrap(); let api_port = @@ -1535,24 +1499,20 @@ impl StacksDevnetApiK8sManager { ]); self.deploy_configmap( StacksDevnetConfigmap::StacksBlockchainApi, - &namespace, + namespace, Some(stacks_api_env), ) .await?; self.deploy_stateful_set( StacksDevnetStatefulSet::StacksBlockchainApi, - &namespace, + namespace, user_id, ) .await?; - self.deploy_service( - StacksDevnetService::StacksBlockchainApi, - &namespace, - &user_id, - ) - .await?; + self.deploy_service(StacksDevnetService::StacksBlockchainApi, namespace, user_id) + .await?; Ok(()) } @@ -1602,27 +1562,27 @@ impl StacksDevnetApiK8sManager { db_path = "/chainstate/stacks-signer-{}.sqlite" "#, signer_key, - get_service_url(&namespace, StacksDevnetService::StacksBlockchain), + get_service_url(namespace, StacksDevnetService::StacksBlockchain), get_service_port(StacksDevnetService::StacksBlockchain, ServicePort::RPC).unwrap(), signer_port, - signer_idx.to_string() + signer_idx ); self.deploy_configmap( configmap, - &namespace, + namespace, Some(vec![("Signer.toml".into(), signer_conf)]), ) .await?; - self.deploy_stateful_set(sts, &namespace, user_id).await?; + self.deploy_stateful_set(sts, namespace, user_id).await?; - self.deploy_service(service, &namespace, &user_id).await?; + self.deploy_service(service, namespace, user_id).await?; Ok(()) } - async fn delete_resource>( + async fn delete_resource( &self, namespace: &str, resource_name: &str, @@ -1632,8 +1592,9 @@ impl StacksDevnetApiK8sManager { K: Clone, K: DeserializeOwned, K: std::fmt::Debug, + K: kube::Resource, { - let api: Api = Api::namespaced(self.client.to_owned(), &namespace); + let api: Api = Api::namespaced(self.client.to_owned(), namespace); let dp = DeleteParams::default(); let resource_details = format!( @@ -1666,7 +1627,7 @@ impl StacksDevnetApiK8sManager { } } - async fn delete_resource_by_label>( + async fn delete_resource_by_label( &self, namespace: &str, resource_name: &str, @@ -1677,8 +1638,9 @@ impl StacksDevnetApiK8sManager { K: Clone, K: DeserializeOwned, K: std::fmt::Debug, + K: kube::Resource, { - let api: Api = Api::namespaced(self.client.to_owned(), &namespace); + let api: Api = Api::namespaced(self.client.to_owned(), namespace); let dp = DeleteParams::default(); let pod_label_selector = format!("{COMPONENT_SELECTOR}={resource_name}"); @@ -1742,13 +1704,13 @@ impl StacksDevnetApiK8sManager { code: api_error.code, }), Err(e) => Err(DevNetError { - message: format!("unable to delete namespace: {}", e.to_string()), + message: format!("unable to delete namespace: {}", e), code: 500, }), } } else { Err(DevNetError { - message: format!("namespace deletion can only occur in debug mode"), + message: "namespace deletion can only occur in debug mode".to_string(), code: 403, }) } @@ -1763,7 +1725,7 @@ impl StacksDevnetApiK8sManager { match serde_yaml::from_str(template_str) { Ok(resource) => Ok(resource), Err(e) => { - let msg = format!("unable to parse template file: {}", e.to_string()); + let msg = format!("unable to parse template file: {}", e); self.ctx.try_log(|logger| slog::error!(logger, "{}", msg)); Err(DevNetError { message: msg, diff --git a/src/main.rs b/src/main.rs index 584575b..633190d 100644 --- a/src/main.rs +++ b/src/main.rs @@ -91,10 +91,10 @@ async fn handle_request( }); let headers = request.headers().clone(); let responder = Responder::new(http_response_config, headers.clone(), ctx.clone()).unwrap(); - if method == &Method::OPTIONS { + if method == Method::OPTIONS { return responder.ok(); } - if method == &Method::GET && (path == "/" || path == &format!("{API_PATH}status")) { + if method == Method::GET && (path == "/" || path == format!("{API_PATH}status")) { return handle_get_status(responder, ctx).await; } let auth_header = auth_config @@ -124,10 +124,10 @@ async fn handle_request( let request_time = SystemTime::now() .duration_since(UNIX_EPOCH) .expect("Could not get current time in secs") - .as_secs() as u64; + .as_secs(); if path == "/api/v1/networks" { - return match method { - &Method::POST => { + return match *method { + Method::POST => { handle_new_devnet( request, &user_id, @@ -172,17 +172,14 @@ async fn handle_request( // the path only contained the network path and network id, // so it must be a request to DELETE a network or GET network info if path_parts.subroute.is_none() { - return match method { - &Method::DELETE => { - match request_store.lock() { - Ok(mut store) => { - store.remove(&user_id); - } - Err(_) => {} + return match *method { + Method::DELETE => { + if let Ok(mut store) = request_store.lock() { + store.remove(&user_id); } handle_delete_devnet(k8s_manager, &network, &user_id, responder).await } - &Method::GET => { + Method::GET => { handle_get_devnet( k8s_manager, &network, @@ -194,7 +191,7 @@ async fn handle_request( ) .await } - &Method::HEAD => { + Method::HEAD => { handle_check_devnet(k8s_manager, &network, &user_id, responder).await } _ => responder @@ -204,18 +201,15 @@ async fn handle_request( // the above methods with no subroute are initiated from our infra, // but any remaning requests would come from the actual user, so we'll // track this request as the last time a user made a request - match request_store.lock() { - Ok(mut store) => { - store.insert(user_id.to_string(), request_time); - } - Err(_) => {} + if let Ok(mut store) = request_store.lock() { + store.insert(user_id.to_string(), request_time); } let subroute = path_parts.subroute.unwrap(); if subroute == "commands" { return responder.err_not_implemented("commands route in progress".into()); } else { - let remaining_path = path_parts.remainder.unwrap_or(String::new()); + let remaining_path = path_parts.remainder.unwrap_or_default(); return handle_try_proxy_service( &remaining_path, &subroute, diff --git a/src/resources/service.rs b/src/resources/service.rs index 1a1d6a2..9d78d26 100644 --- a/src/resources/service.rs +++ b/src/resources/service.rs @@ -62,7 +62,7 @@ pub fn get_user_facing_port(service: StacksDevnetService) -> Option { } pub fn get_service_url(namespace: &str, service: StacksDevnetService) -> String { - format!("{}.{}.svc.cluster.local", service.to_string(), namespace) + format!("{}.{}.svc.cluster.local", service, namespace) } pub fn get_service_from_path_part(path_part: &str) -> Option { diff --git a/src/resources/stateful_set.rs b/src/resources/stateful_set.rs index 13bdf0a..897de4b 100644 --- a/src/resources/stateful_set.rs +++ b/src/resources/stateful_set.rs @@ -24,11 +24,11 @@ pub enum SignerIdx { Signer1, } -impl SignerIdx { - pub fn to_string(&self) -> String { - match &self { - SignerIdx::Signer0 => String::from("0"), - SignerIdx::Signer1 => String::from("1"), +impl fmt::Display for SignerIdx { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + SignerIdx::Signer0 => write!(f, "0"), + SignerIdx::Signer1 => write!(f, "1"), } } } diff --git a/src/resources/tests.rs b/src/resources/tests.rs index 2ffe755..9d8ef7d 100644 --- a/src/resources/tests.rs +++ b/src/resources/tests.rs @@ -6,45 +6,45 @@ use super::{ }; use test_case::test_case; -#[test_case(StacksDevnetConfigmap::BitcoindNode => is equal_to "bitcoind".to_string(); "for BitcoinNode")] -#[test_case(StacksDevnetConfigmap::StacksBlockchain => is equal_to "stacks-blockchain".to_string(); "for StacksBlockchain")] -#[test_case(StacksDevnetConfigmap::StacksBlockchainApi => is equal_to "stacks-blockchain-api".to_string(); "for StacksBlockchainApi")] -#[test_case(StacksDevnetConfigmap::StacksBlockchainApiPg => is equal_to "stacks-blockchain-api-pg".to_string(); "for StacksBlockchainApiPg")] -#[test_case(StacksDevnetConfigmap::StacksSigner0 => is equal_to "stacks-signer-0".to_string(); "for StacksSigner0")] -#[test_case(StacksDevnetConfigmap::StacksSigner1 => is equal_to "stacks-signer-1".to_string(); "for StacksSigner1")] -#[test_case(StacksDevnetConfigmap::DeploymentPlan => is equal_to "deployment-plan".to_string(); "for DeploymentPlan")] -#[test_case(StacksDevnetConfigmap::Devnet => is equal_to "devnet".to_string(); "for Devnet")] -#[test_case(StacksDevnetConfigmap::ProjectDir => is equal_to "project-dir".to_string(); "for ProjectDir")] -#[test_case(StacksDevnetConfigmap::ProjectManifest => is equal_to "project-manifest".to_string(); "for ProjectManifest")] +#[test_case(StacksDevnetConfigmap::BitcoindNode => is equal_to "bitcoind"; "for BitcoinNode")] +#[test_case(StacksDevnetConfigmap::StacksBlockchain => is equal_to "stacks-blockchain"; "for StacksBlockchain")] +#[test_case(StacksDevnetConfigmap::StacksBlockchainApi => is equal_to "stacks-blockchain-api"; "for StacksBlockchainApi")] +#[test_case(StacksDevnetConfigmap::StacksBlockchainApiPg => is equal_to "stacks-blockchain-api-pg"; "for StacksBlockchainApiPg")] +#[test_case(StacksDevnetConfigmap::StacksSigner0 => is equal_to "stacks-signer-0"; "for StacksSigner0")] +#[test_case(StacksDevnetConfigmap::StacksSigner1 => is equal_to "stacks-signer-1"; "for StacksSigner1")] +#[test_case(StacksDevnetConfigmap::DeploymentPlan => is equal_to "deployment-plan"; "for DeploymentPlan")] +#[test_case(StacksDevnetConfigmap::Devnet => is equal_to "devnet"; "for Devnet")] +#[test_case(StacksDevnetConfigmap::ProjectDir => is equal_to "project-dir"; "for ProjectDir")] +#[test_case(StacksDevnetConfigmap::ProjectManifest => is equal_to "project-manifest"; "for ProjectManifest")] fn it_prints_correct_name_for_configmap(configmap: StacksDevnetConfigmap) -> String { configmap.to_string() } -#[test_case(StacksDevnetPod::BitcoindNode => is equal_to "bitcoind-chain-coordinator".to_string(); "for BitcoindNode")] -#[test_case(StacksDevnetPod::StacksBlockchain => is equal_to "stacks-blockchain".to_string(); "for StacksBlockchain")] -#[test_case(StacksDevnetPod::StacksBlockchainApi => is equal_to "stacks-blockchain-api".to_string(); "for StacksBlockchainApi")] +#[test_case(StacksDevnetPod::BitcoindNode => is equal_to "bitcoind-chain-coordinator"; "for BitcoindNode")] +#[test_case(StacksDevnetPod::StacksBlockchain => is equal_to "stacks-blockchain"; "for StacksBlockchain")] +#[test_case(StacksDevnetPod::StacksBlockchainApi => is equal_to "stacks-blockchain-api"; "for StacksBlockchainApi")] fn it_prints_correct_name_for_pod(pod: StacksDevnetPod) -> String { pod.to_string() } -#[test_case(StacksDevnetDeployment::BitcoindNode => is equal_to "bitcoind-chain-coordinator".to_string(); "for BitcoindNode")] -#[test_case(StacksDevnetDeployment::StacksBlockchain => is equal_to "stacks-blockchain".to_string(); "for StacksBlockchain")] +#[test_case(StacksDevnetDeployment::BitcoindNode => is equal_to "bitcoind-chain-coordinator"; "for BitcoindNode")] +#[test_case(StacksDevnetDeployment::StacksBlockchain => is equal_to "stacks-blockchain"; "for StacksBlockchain")] fn it_prints_correct_name_for_deployment(deployment: StacksDevnetDeployment) -> String { deployment.to_string() } -#[test_case(StacksDevnetStatefulSet::StacksBlockchainApi => is equal_to "stacks-blockchain-api".to_string(); "for StacksBlockchainApi")] -#[test_case(StacksDevnetStatefulSet::StacksSigner0 => is equal_to "stacks-signer-0".to_string(); "for StacksSigner0")] -#[test_case(StacksDevnetStatefulSet::StacksSigner1 => is equal_to "stacks-signer-1".to_string(); "for StacksSigner1")] +#[test_case(StacksDevnetStatefulSet::StacksBlockchainApi => is equal_to "stacks-blockchain-api"; "for StacksBlockchainApi")] +#[test_case(StacksDevnetStatefulSet::StacksSigner0 => is equal_to "stacks-signer-0"; "for StacksSigner0")] +#[test_case(StacksDevnetStatefulSet::StacksSigner1 => is equal_to "stacks-signer-1"; "for StacksSigner1")] fn it_prints_correct_name_for_stateful_set(pod: StacksDevnetStatefulSet) -> String { pod.to_string() } -#[test_case(StacksDevnetService::BitcoindNode => is equal_to "bitcoind-chain-coordinator".to_string(); "for BitcoindNode")] -#[test_case(StacksDevnetService::StacksBlockchain => is equal_to "stacks-blockchain".to_string(); "for StacksBlockchain")] -#[test_case(StacksDevnetService::StacksBlockchainApi => is equal_to "stacks-blockchain-api".to_string(); "for StacksBlockchainApi")] -#[test_case(StacksDevnetService::StacksSigner0 => is equal_to "stacks-signer-0".to_string(); "for StacksSigner0")] -#[test_case(StacksDevnetService::StacksSigner1 => is equal_to "stacks-signer-1".to_string(); "for StacksSigner1")] +#[test_case(StacksDevnetService::BitcoindNode => is equal_to "bitcoind-chain-coordinator"; "for BitcoindNode")] +#[test_case(StacksDevnetService::StacksBlockchain => is equal_to "stacks-blockchain"; "for StacksBlockchain")] +#[test_case(StacksDevnetService::StacksBlockchainApi => is equal_to "stacks-blockchain-api"; "for StacksBlockchainApi")] +#[test_case(StacksDevnetService::StacksSigner0 => is equal_to "stacks-signer-0"; "for StacksSigner0")] +#[test_case(StacksDevnetService::StacksSigner1 => is equal_to "stacks-signer-1"; "for StacksSigner1")] fn it_prints_correct_name_for_service(service: StacksDevnetService) -> String { service.to_string() } diff --git a/src/responder.rs b/src/responder.rs index 5eb1957..4bf50c1 100644 --- a/src/responder.rs +++ b/src/responder.rs @@ -66,7 +66,7 @@ impl Responder { break; } } - return builder; + builder } None => builder, } @@ -74,19 +74,7 @@ impl Responder { fn _respond(&self, code: StatusCode, body: String) -> Result, Infallible> { let builder = self.response_builder(); - let body = match Body::try_from(body) { - Ok(b) => b, - Err(e) => { - self.ctx.try_log(|logger| { - slog::error!( - logger, - "responder failed to create response body: {}", - e.to_string() - ) - }); - Body::empty() - } - }; + let body = Body::from(body); match builder.status(code).body(body) { Ok(r) => Ok(r), Err(e) => { @@ -125,7 +113,7 @@ impl Responder { .body(body) { Ok(r) => Ok(r), - Err(e) => self.err_internal(format!("failed to send response: {}", e.to_string())), + Err(e) => self.err_internal(format!("failed to send response: {}", e)), } } diff --git a/src/routes.rs b/src/routes.rs index 1ad6913..c70480a 100644 --- a/src/routes.rs +++ b/src/routes.rs @@ -28,7 +28,7 @@ pub async fn handle_get_status( let version_info = match serde_json::to_vec(&version_info) { Ok(v) => v, Err(e) => { - let msg = format!("failed to parse version info: {}", e.to_string()); + let msg = format!("failed to parse version info: {}", e); ctx.try_log(|logger| slog::error!(logger, "{}", msg)); return responder.err_internal(msg); } @@ -58,11 +58,8 @@ pub async fn handle_new_devnet( Ok(config) => match config.to_validated_config(user_id, ctx) { Ok(config) => match k8s_manager.deploy_devnet(config).await { Ok(_) => { - match request_store.lock() { - Ok(mut store) => { - store.insert(user_id.to_string(), request_time); - } - Err(_) => {} + if let Ok(mut store) = request_store.lock() { + store.insert(user_id.to_string(), request_time); } responder.ok() } @@ -112,7 +109,7 @@ pub async fn handle_get_devnet( request_time: u64, ctx: Context, ) -> Result, Infallible> { - match k8s_manager.get_devnet_info(&network, user_id).await { + match k8s_manager.get_devnet_info(network, user_id).await { Ok(devnet_info) => { let last_request_time = match request_store.lock() { Ok(mut store) => match store.get(user_id) { @@ -135,8 +132,7 @@ pub async fn handle_get_devnet( Err(e) => { let msg = format!( "failed to form response body: NAMESPACE: {}, ERROR: {}", - &network, - e.to_string() + &network, e ); ctx.try_log(|logger: &hiro_system_kit::Logger| slog::error!(logger, "{}", msg)); responder.err_internal(msg) @@ -174,18 +170,18 @@ pub async fn handle_try_proxy_service( responder: Responder, ctx: &Context, ) -> Result, Infallible> { - match k8s_manager.check_all_devnet_assets_exist(&network).await { + match k8s_manager.check_all_devnet_assets_exist(network).await { Ok(exists) => match exists { true => { let service = get_service_from_path_part(subroute); match service { Some(service) => { - let base_url = get_service_url(&network, service.clone()); + let base_url = get_service_url(network, service.clone()); let port = get_user_facing_port(service).unwrap(); let forward_url = format!("{}:{}", base_url, port); let proxy_request = - mutate_request_for_proxy(request, &forward_url, &remaining_path); - proxy(proxy_request, responder, &ctx).await + mutate_request_for_proxy(request, &forward_url, remaining_path); + proxy(proxy_request, responder, ctx).await } None => responder.err_bad_request("invalid request path".into()), } @@ -229,7 +225,7 @@ async fn proxy( match client.request(request).await { Ok(response) => Ok(response), Err(e) => { - let msg = format!("error proxying request: {}", e.to_string()); + let msg = format!("error proxying request: {}", e); ctx.try_log(|logger| slog::error!(logger, "{}", msg)); responder.err_internal(msg) } diff --git a/src/tests/mod.rs b/src/tests/mod.rs index 87e7eba..7d6c2ea 100644 --- a/src/tests/mod.rs +++ b/src/tests/mod.rs @@ -48,7 +48,7 @@ fn get_template_config() -> StacksDevnetConfig { match serde_json::from_slice::(&file_buffer) { Ok(s) => s, Err(e) => { - panic!("Config file malformatted {}", e.to_string()); + panic!("Config file malformatted {}", e); } }; config_file @@ -138,21 +138,21 @@ async fn it_responds_to_valid_requests_with_deploy( let new_path: String; if request_path.contains("{namespace}") { - new_path = request_path.replace("{namespace}", &namespace); + new_path = request_path.replace("{namespace}", namespace); request_path = &new_path; } let (k8s_manager, ctx) = get_k8s_manager().await; - let request_builder = get_request_builder(request_path, method, &namespace); + let request_builder = get_request_builder(request_path, method, namespace); - let _ = k8s_manager.deploy_namespace(&namespace).await.unwrap(); + k8s_manager.deploy_namespace(namespace).await.unwrap(); let mut config = get_template_config(); config.namespace = namespace.to_owned(); - let validated_config = config.to_validated_config(&namespace, &ctx).unwrap(); + let validated_config = config.to_validated_config(namespace, &ctx).unwrap(); let user_id = &namespace; - let _ = k8s_manager.deploy_devnet(validated_config).await.unwrap(); + k8s_manager.deploy_devnet(validated_config).await.unwrap(); // short delay to allow assets to start sleep(Duration::new(5, 0)); @@ -191,7 +191,7 @@ async fn it_responds_to_valid_requests_with_deploy( } } } - let _ = k8s_manager.delete_namespace(&namespace).await.unwrap(); + k8s_manager.delete_namespace(namespace).await.unwrap(); (status, body_str) } @@ -213,16 +213,16 @@ async fn it_responds_to_valid_requests( let new_path: String; if request_path.contains("{namespace}") { - new_path = request_path.replace("{namespace}", &namespace); + new_path = request_path.replace("{namespace}", namespace); request_path = &new_path; } let (k8s_manager, ctx) = get_k8s_manager().await; - let request_builder = get_request_builder(request_path, method, &namespace); + let request_builder = get_request_builder(request_path, method, namespace); if set_up { - let _ = k8s_manager.deploy_namespace(&namespace).await.unwrap(); + k8s_manager.deploy_namespace(namespace).await.unwrap(); } let request: Request = request_builder.body(Body::empty()).unwrap(); @@ -242,7 +242,7 @@ async fn it_responds_to_valid_requests( let body_str = String::from_utf8(bytes).unwrap(); if set_up { - let _ = k8s_manager.delete_namespace(&namespace).await.unwrap(); + k8s_manager.delete_namespace(namespace).await.unwrap(); } (response.status(), body_str) @@ -258,7 +258,7 @@ async fn deploy_devnet( config.namespace = namespace.to_owned(); let body = Body::from(serde_json::to_string(&config).unwrap()); - let request: Request = get_request_builder("/api/v1/networks", Method::POST, &namespace) + let request: Request = get_request_builder("/api/v1/networks", Method::POST, namespace) .body(body) .unwrap(); let _ = handle_request( @@ -281,7 +281,7 @@ async fn get_devnet_info( let request: Request = get_request_builder( &format!("/api/v1/network/{namespace}"), Method::GET, - &namespace, + namespace, ) .body(Body::empty()) .unwrap(); @@ -309,7 +309,7 @@ async fn delete_devnet( let request: Request = get_request_builder( &format!("/api/v1/network/{namespace}"), Method::DELETE, - &namespace, + namespace, ) .body(Body::empty()) .unwrap(); @@ -331,25 +331,19 @@ async fn it_tracks_requests_time_for_user() { let namespace2 = &get_random_namespace(); let (k8s_manager, ctx) = get_k8s_manager().await; - let _ = k8s_manager.deploy_namespace(&namespace).await.unwrap(); - let _ = k8s_manager.deploy_namespace(&namespace2).await.unwrap(); + k8s_manager.deploy_namespace(namespace).await.unwrap(); + k8s_manager.deploy_namespace(namespace2).await.unwrap(); let request_store = Arc::new(Mutex::new(HashMap::new())); // create one devnet and assert request time is stored let created_time = { - deploy_devnet(&namespace, k8s_manager.clone(), request_store.clone(), &ctx).await; + deploy_devnet(namespace, k8s_manager.clone(), request_store.clone(), &ctx).await; let store = request_store.lock().unwrap(); - store.get(namespace).unwrap().clone() + *store.get(namespace).unwrap() }; // create another devnet and assert request time is stored { - deploy_devnet( - &namespace2, - k8s_manager.clone(), - request_store.clone(), - &ctx, - ) - .await; + deploy_devnet(namespace2, k8s_manager.clone(), request_store.clone(), &ctx).await; // after creating a devnet, there should be an entry assert!(request_store.lock().unwrap().get(namespace).is_some()); } @@ -358,7 +352,7 @@ async fn it_tracks_requests_time_for_user() { let secs_after_first_get1 = { let info = - get_devnet_info(&namespace, k8s_manager.clone(), request_store.clone(), &ctx).await; + get_devnet_info(namespace, k8s_manager.clone(), request_store.clone(), &ctx).await; // time should have elapsed since our last request let secs_after_first_get = info.metadata.secs_since_last_request; assert!(secs_after_first_get > 0); @@ -372,13 +366,8 @@ async fn it_tracks_requests_time_for_user() { // confirm time has elapsed since our last request let secs_after_first_get2 = { - let info = get_devnet_info( - &namespace2, - k8s_manager.clone(), - request_store.clone(), - &ctx, - ) - .await; + let info = + get_devnet_info(namespace2, k8s_manager.clone(), request_store.clone(), &ctx).await; let secs_after_first_get = info.metadata.secs_since_last_request; assert!(secs_after_first_get > 0); secs_after_first_get @@ -389,7 +378,7 @@ async fn it_tracks_requests_time_for_user() { let request: Request = get_request_builder( &format!("/api/v1/network/{namespace}/some-path"), Method::GET, - &namespace, + namespace, ) .body(Body::empty()) .unwrap(); @@ -407,19 +396,14 @@ async fn it_tracks_requests_time_for_user() { // immediately make another get request to confirm that the time since last request was updated { let info = - get_devnet_info(&namespace, k8s_manager.clone(), request_store.clone(), &ctx).await; + get_devnet_info(namespace, k8s_manager.clone(), request_store.clone(), &ctx).await; assert!(secs_after_first_get1 > info.metadata.secs_since_last_request); } // and verify that the time since the last request wasn't updated for our other namespace { - let info = get_devnet_info( - &namespace2, - k8s_manager.clone(), - request_store.clone(), - &ctx, - ) - .await; + let info = + get_devnet_info(namespace2, k8s_manager.clone(), request_store.clone(), &ctx).await; assert!(info.metadata.secs_since_last_request >= secs_after_first_get2); } @@ -428,7 +412,7 @@ async fn it_tracks_requests_time_for_user() { assert_eq!(request_store.lock().unwrap().keys().len(), 0); // confirm that our infrastructure pinging will insert request times if none exist { - let _ = get_devnet_info(&namespace, k8s_manager.clone(), request_store.clone(), &ctx).await; + let _ = get_devnet_info(namespace, k8s_manager.clone(), request_store.clone(), &ctx).await; assert_eq!(request_store.lock().unwrap().keys().len(), 1); } @@ -440,8 +424,8 @@ async fn it_tracks_requests_time_for_user() { // clean up delete_devnet(namespace2, k8s_manager.clone(), request_store.clone(), &ctx).await; - let _ = k8s_manager.delete_namespace(&namespace).await.unwrap(); - let _ = k8s_manager.delete_namespace(&namespace2).await.unwrap(); + k8s_manager.delete_namespace(namespace).await.unwrap(); + k8s_manager.delete_namespace(namespace2).await.unwrap(); } async fn mock_k8s_handler(handle: &mut Handle, Response>) { @@ -495,23 +479,23 @@ async fn get_mock_k8s_manager() -> (StacksDevnetApiK8sManager, Context) { #[test_case("/api", Method::GET, "some-user" => is equal_to (StatusCode::BAD_REQUEST, "invalid request path".to_string()) ; "400 for invalid requet path /api")] #[test_case("/api/v1", Method::GET, "some-user" => is equal_to (StatusCode::BAD_REQUEST, "invalid request path".to_string()) ; "400 for invalid requet path /api/v1")] #[test_case("/api/v1/network2", Method::GET, "some-user" => is equal_to (StatusCode::BAD_REQUEST, "invalid request path".to_string()) ; "400 for invalid requet path /api/v1/network2")] -#[test_case("/api/v1/network/undeployed", Method::GET, "undeployed" => +#[test_case("/api/v1/network/undeployed", Method::GET, "undeployed" => is equal_to (StatusCode::NOT_FOUND, "network undeployed does not exist".to_string()); "404 for undeployed namespace")] -#[test_case("/api/v1/network/500_err", Method::GET, "500_err" => +#[test_case("/api/v1/network/500_err", Method::GET, "500_err" => is equal_to (StatusCode::INTERNAL_SERVER_ERROR, "error getting namespace 500_err: \"\"".to_string()); "forwarded error if fetching namespace returns error")] -#[test_case("/api/v1/network/test", Method::POST, "test" => +#[test_case("/api/v1/network/test", Method::POST, "test" => is equal_to (StatusCode::METHOD_NOT_ALLOWED, "can only GET/DELETE/HEAD at provided route".to_string()); "405 for network route with POST request")] -#[test_case("/api/v1/network/test/commands", Method::GET, "test" => +#[test_case("/api/v1/network/test/commands", Method::GET, "test" => is equal_to (StatusCode::NOT_FOUND, "commands route in progress".to_string()); "404 for network commands route")] -#[test_case("/api/v1/network/", Method::GET, "test" => +#[test_case("/api/v1/network/", Method::GET, "test" => is equal_to (StatusCode::BAD_REQUEST, "no network id provided".to_string()); "400 for missing namespace")] -#[test_case("/api/v1/networks", Method::GET, "test" => +#[test_case("/api/v1/networks", Method::GET, "test" => is equal_to (StatusCode::METHOD_NOT_ALLOWED, "network creation must be a POST request".to_string()); "405 for network creation request with GET method")] -#[test_case("/api/v1/networks", Method::DELETE, "test" => +#[test_case("/api/v1/networks", Method::DELETE, "test" => is equal_to (StatusCode::METHOD_NOT_ALLOWED, "network creation must be a POST request".to_string()); "405 for network creation request with DELETE method")] -#[test_case("/api/v1/networks", Method::POST, "test" => +#[test_case("/api/v1/networks", Method::POST, "test" => is equal_to (StatusCode::BAD_REQUEST, "invalid configuration to create network: EOF while parsing a value at line 1 column 0".to_string()); "400 for network creation request invalid config")] -#[test_case("/api/v1/network/test", Method::GET, "wrong-id" => +#[test_case("/api/v1/network/test", Method::GET, "wrong-id" => is equal_to (StatusCode::BAD_REQUEST, "network id must match authenticated user id".to_string()); "400 for request with non-matching user")] #[tokio::test] async fn it_responds_to_invalid_requests( @@ -521,7 +505,7 @@ async fn it_responds_to_invalid_requests( ) -> (StatusCode, String) { let (k8s_manager, ctx) = get_mock_k8s_manager().await; - let request_builder = get_request_builder(request_path, method, &user_id); + let request_builder = get_request_builder(request_path, method, user_id); let request: Request = request_builder.body(Body::empty()).unwrap(); let request_store = Arc::new(Mutex::new(HashMap::new())); let mut response = handle_request( @@ -564,7 +548,7 @@ async fn it_responds_to_invalid_request_header() { assert_eq!(body_str, "missing required auth header".to_string()); } -#[test_case("/api/v1/network/test", Method::OPTIONS => is equal_to "Ok".to_string())] +#[test_case("/api/v1/network/test", Method::OPTIONS => is equal_to *"Ok")] #[test_case("/api/v1/status", Method::GET => is equal_to get_version_info() )] #[test_case("/", Method::GET => is equal_to get_version_info())] #[tokio::test] @@ -586,8 +570,7 @@ async fn it_ignores_request_header_for_some_requests(request_path: &str, method: assert_eq!(response.status(), 200); let body = response.body_mut(); let bytes = body::to_bytes(body).await.unwrap().to_vec(); - let body_str = String::from_utf8(bytes).unwrap(); - body_str + String::from_utf8(bytes).unwrap() } #[test_case("" => is equal_to PathParts { route: String::new(), ..Default::default() }; "for empty path")] @@ -598,9 +581,9 @@ async fn it_ignores_request_header_for_some_requests(request_path: &str, method: #[test_case("/api/v1/some-route/some-network/" => is equal_to PathParts { route: String::from("some-route"), network: Some(String::from("some-network")), ..Default::default() }; "for /api/v1/some-route/some-network/ path trailing slash")] #[test_case("/api/v1/some-route/some-network/some-subroute" => is equal_to PathParts { route: String::from("some-route"), network: Some(String::from("some-network")), subroute: Some(String::from("some-subroute")), ..Default::default() }; "for /api/v1/some-route/some-network/some-subroute path")] #[test_case("/api/v1/some-route/some-network/some-subroute/" => is equal_to PathParts { route: String::from("some-route"), network: Some(String::from("some-network")), subroute: Some(String::from("some-subroute")), ..Default::default() }; "for /api/v1/some-route/some-network/some-subroute/ path trailing slash")] -#[test_case("/api/v1/some-route/some-network/some-subroute/the/remaining/path" => is equal_to PathParts { route: String::from("some-route"), network: Some(String::from("some-network")), subroute: Some(String::from("some-subroute")), remainder: Some(String::from("the/remaining/path")), ..Default::default() }; "for /api/v1/some-route/some-network/some-subroute/the/remaining/path path ")] -#[test_case("/api/v1/some-route/some-network/some-subroute/the/remaining/path/" => is equal_to PathParts { route: String::from("some-route"), network: Some(String::from("some-network")), subroute: Some(String::from("some-subroute")), remainder: Some(String::from("the/remaining/path")), ..Default::default() }; "for /api/v1/some-route/some-network/some-subroute/the/remaining/path/ path trailing slash")] -#[test_case("/api/v1/some-route/some-network/some-subroute/the//remaining//path/" => is equal_to PathParts { route: String::from("some-route"), network: Some(String::from("some-network")), subroute: Some(String::from("some-subroute")), remainder: Some(String::from("the//remaining//path")), ..Default::default() }; "for /api/v1/some-route/some-network/some-subroute/the//remaining//path/ path extra internal slash")] +#[test_case("/api/v1/some-route/some-network/some-subroute/the/remaining/path" => is equal_to PathParts { route: String::from("some-route"), network: Some(String::from("some-network")), subroute: Some(String::from("some-subroute")), remainder: Some(String::from("the/remaining/path")) }; "for /api/v1/some-route/some-network/some-subroute/the/remaining/path path ")] +#[test_case("/api/v1/some-route/some-network/some-subroute/the/remaining/path/" => is equal_to PathParts { route: String::from("some-route"), network: Some(String::from("some-network")), subroute: Some(String::from("some-subroute")), remainder: Some(String::from("the/remaining/path")) }; "for /api/v1/some-route/some-network/some-subroute/the/remaining/path/ path trailing slash")] +#[test_case("/api/v1/some-route/some-network/some-subroute/the//remaining//path/" => is equal_to PathParts { route: String::from("some-route"), network: Some(String::from("some-network")), subroute: Some(String::from("some-subroute")), remainder: Some(String::from("the//remaining//path")) }; "for /api/v1/some-route/some-network/some-subroute/the//remaining//path/ path extra internal slash")] fn request_paths_are_parsed_correctly(path: &str) -> PathParts { get_standardized_path_parts(path) } @@ -661,7 +644,7 @@ async fn namespace_prefix_config_prepends_header() { // using the ApiConfig's `namespace_prefix` field will add the prefix // before the `user_id` as the authenticated user, which should match the request path let namespace = &get_random_namespace(); - let _ = k8s_manager.deploy_namespace(&namespace).await.unwrap(); + k8s_manager.deploy_namespace(namespace).await.unwrap(); let (namespace_prefix, user_id) = namespace.split_at(4); let api_config = ApiConfig {