From d62750f5e606724106c00a212649b6aeb36d4938 Mon Sep 17 00:00:00 2001 From: Jeffrey Huynh Date: Wed, 27 Sep 2023 16:32:55 -0600 Subject: [PATCH 1/6] preflight checks skeleton --- src/api/remote.rs | 2 +- src/cmd/run.rs | 73 +++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 71 insertions(+), 4 deletions(-) diff --git a/src/api/remote.rs b/src/api/remote.rs index a3af1fa0..398cbf24 100644 --- a/src/api/remote.rs +++ b/src/api/remote.rs @@ -196,7 +196,7 @@ impl PollInterval { /// This is set because Broker is intended to bring eventual observability; /// if users want faster polling than this it's probably because they want to make sure they don't miss revisions, /// in such a case we recommend CI integration. -pub const MIN_POLL_INTERVAL: Duration = Duration::from_secs(60 * 60); +pub const MIN_POLL_INTERVAL: Duration = Duration::from_secs(15); impl TryFrom for PollInterval { type Error = Report; diff --git a/src/cmd/run.rs b/src/cmd/run.rs index 706e2390..b99ffadf 100644 --- a/src/cmd/run.rs +++ b/src/cmd/run.rs @@ -2,7 +2,7 @@ use std::time::Duration; -use error_stack::{Result, ResultExt}; +use error_stack::{report, Result, ResultExt}; use futures::TryStreamExt; use futures::{future::try_join_all, try_join, StreamExt}; use governor::{Quota, RateLimiter}; @@ -18,7 +18,9 @@ use tracing::{debug, info}; use uuid::Uuid; use crate::api::fossa::{self, CliMetadata, ProjectMetadata}; -use crate::api::remote::Reference; +use crate::api::remote::git::repository; +use crate::api::remote::{Integrations, Protocol, Reference}; +use crate::ext::result::{WrapErr, WrapOk}; use crate::ext::tracing::span_record; use crate::fossa_cli::{self, DesiredVersion, Location, SourceUnits}; use crate::queue::Queue; @@ -80,6 +82,14 @@ pub enum Error { /// If we fail to run FOSSA CLI, this error is raised. #[error("run FOSSA CLI")] RunFossaCli, + + /// Failed to connect to at least one integration + #[error("validate integration connections")] + IntegrationConnection, + + /// Failed to connect to FOSSA + #[error("validate FOSSA connection")] + FossaConnection, } /// Similar to [`AppContext`], but scoped for this subcommand. @@ -104,9 +114,66 @@ pub async fn main(ctx: &AppContext, config: Config, db: D) -> Resul db, }; + let preflight_checks = preflight_checks(&ctx); let healthcheck_worker = healthcheck(&ctx.db); let integration_worker = integrations(&ctx); - try_join!(healthcheck_worker, integration_worker).discard_ok() + try_join!(preflight_checks, healthcheck_worker, integration_worker).discard_ok() +} +async fn preflight_checks(ctx: &CmdContext) -> Result<(), Error> { + println!("In preflight checks-----"); + let validate_integration_connections = integration_connections(ctx.config.integrations()); + let validate_fossa_connection = fossa_connection(&ctx.config); + try_join!(validate_integration_connections, validate_fossa_connection).discard_ok() +} + +#[tracing::instrument(skip_all)] +/// Validate that Broker can connect to at least one integration +async fn integration_connections(integrations: &Integrations) -> Result<(), Error> { + if integrations.as_ref().is_empty() { + return Ok(()); + } + + for integration in integrations.iter() { + let Protocol::Git(transport) = integration.protocol(); + if let Ok(_) = repository::ls_remote(transport).await { + return Ok(()); + } + } + + report!(Error::IntegrationConnection).wrap_err() +} + +#[tracing::instrument(skip_all)] +/// Validate that Broker can connect to FOSSA +async fn fossa_connection(config: &Config) -> Result<(), Error> { + let endpoint = config.fossa_api().endpoint().as_ref(); + let path = "/api/cli/organization"; + let timeout_duration: u64 = 30; + let client = reqwest::Client::builder() + .redirect(reqwest::redirect::Policy::none()) + .connect_timeout(Duration::from_secs(timeout_duration)) + .build() + .map_err(|_| Error::FossaConnection)?; + + let url = endpoint.join(path).map_err(|_| Error::FossaConnection)?; + + let fossa_response = client + .get(url.as_str()) + .header(reqwest::header::ACCEPT, "application/json") + .bearer_auth(config.fossa_api().key().expose_secret()) + .send() + .await; + + match fossa_response { + Ok(res) => { + if let Err(status_err) = res.error_for_status() { + return report!(Error::FossaConnection).wrap_err(); + } else { + return Ok(()); + } + } + Err(err) => return report!(Error::FossaConnection).wrap_err(), + } } /// Conduct internal diagnostics to ensure Broker is still in a good state. From 6546bcbdcc86edbb2f7f16e77fe3051c6692009d Mon Sep 17 00:00:00 2001 From: Jeffrey Huynh Date: Thu, 28 Sep 2023 13:59:29 -0600 Subject: [PATCH 2/6] Implementation for preflight checks --- src/cmd/run.rs | 51 ++++++++++++++++++-------------------------------- 1 file changed, 18 insertions(+), 33 deletions(-) diff --git a/src/cmd/run.rs b/src/cmd/run.rs index b99ffadf..2179bd4c 100644 --- a/src/cmd/run.rs +++ b/src/cmd/run.rs @@ -20,7 +20,7 @@ use uuid::Uuid; use crate::api::fossa::{self, CliMetadata, ProjectMetadata}; use crate::api::remote::git::repository; use crate::api::remote::{Integrations, Protocol, Reference}; -use crate::ext::result::{WrapErr, WrapOk}; +use crate::ext::result::WrapErr; use crate::ext::tracing::span_record; use crate::fossa_cli::{self, DesiredVersion, Location, SourceUnits}; use crate::queue::Queue; @@ -84,11 +84,11 @@ pub enum Error { RunFossaCli, /// Failed to connect to at least one integration - #[error("validate integration connections")] + #[error("integration connections")] IntegrationConnection, /// Failed to connect to FOSSA - #[error("validate FOSSA connection")] + #[error("FOSSA connection")] FossaConnection, } @@ -119,8 +119,9 @@ pub async fn main(ctx: &AppContext, config: Config, db: D) -> Resul let integration_worker = integrations(&ctx); try_join!(preflight_checks, healthcheck_worker, integration_worker).discard_ok() } + +/// Checks and catches network misconfigurations before Broker attempts its operations async fn preflight_checks(ctx: &CmdContext) -> Result<(), Error> { - println!("In preflight checks-----"); let validate_integration_connections = integration_connections(ctx.config.integrations()); let validate_fossa_connection = fossa_connection(&ctx.config); try_join!(validate_integration_connections, validate_fossa_connection).discard_ok() @@ -140,40 +141,24 @@ async fn integration_connections(integrations: &Integrations) -> Result<(), Erro } } - report!(Error::IntegrationConnection).wrap_err() + report!(Error::IntegrationConnection) + .wrap_err() + .help("run broker fix for detailed explanation on failing integration connections") + .describe("integration connections") } #[tracing::instrument(skip_all)] /// Validate that Broker can connect to FOSSA async fn fossa_connection(config: &Config) -> Result<(), Error> { - let endpoint = config.fossa_api().endpoint().as_ref(); - let path = "/api/cli/organization"; - let timeout_duration: u64 = 30; - let client = reqwest::Client::builder() - .redirect(reqwest::redirect::Policy::none()) - .connect_timeout(Duration::from_secs(timeout_duration)) - .build() - .map_err(|_| Error::FossaConnection)?; - - let url = endpoint.join(path).map_err(|_| Error::FossaConnection)?; - - let fossa_response = client - .get(url.as_str()) - .header(reqwest::header::ACCEPT, "application/json") - .bearer_auth(config.fossa_api().key().expose_secret()) - .send() - .await; - - match fossa_response { - Ok(res) => { - if let Err(status_err) = res.error_for_status() { - return report!(Error::FossaConnection).wrap_err(); - } else { - return Ok(()); - } - } - Err(err) => return report!(Error::FossaConnection).wrap_err(), - } + let org_lookup = match fossa::OrgConfig::lookup(config.fossa_api()).await { + Ok(_) => Ok(()), + Err(err) => err + .change_context(Error::FossaConnection) + .wrap_err() + .help("ensure that your fossa key is configured correctly") + .describe("fossa key"), + }; + org_lookup } /// Conduct internal diagnostics to ensure Broker is still in a good state. From 5520909ea7c958589f8b40e1820f46e7a7a8c1ad Mon Sep 17 00:00:00 2001 From: Jeffrey Huynh Date: Mon, 2 Oct 2023 10:54:41 -0600 Subject: [PATCH 3/6] Adding env variable for integration tests --- .github/workflows/dynamic-analysis.yml | 2 ++ src/api/remote.rs | 2 +- tests/it/helper.rs | 6 ++++-- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/.github/workflows/dynamic-analysis.yml b/.github/workflows/dynamic-analysis.yml index 423ea925..15f6b8ef 100644 --- a/.github/workflows/dynamic-analysis.yml +++ b/.github/workflows/dynamic-analysis.yml @@ -32,6 +32,8 @@ jobs: - run: cargo nextest run env: RUN_INTEGRATION_TESTS: "1" + # This key is used for integration tests + FOSSA_API_KEY: ${{ secrets.FOSSA_API_KEY }} # nextest doesn't run doctests, but does test everything else: https://github.com/nextest-rs/nextest/issues/16 # run doctests after; this won't result in any extra rebuilds and is very quick. # doctest overview: https://doc.rust-lang.org/rustdoc/write-documentation/documentation-tests.html diff --git a/src/api/remote.rs b/src/api/remote.rs index 398cbf24..a3af1fa0 100644 --- a/src/api/remote.rs +++ b/src/api/remote.rs @@ -196,7 +196,7 @@ impl PollInterval { /// This is set because Broker is intended to bring eventual observability; /// if users want faster polling than this it's probably because they want to make sure they don't miss revisions, /// in such a case we recommend CI integration. -pub const MIN_POLL_INTERVAL: Duration = Duration::from_secs(15); +pub const MIN_POLL_INTERVAL: Duration = Duration::from_secs(60 * 60); impl TryFrom for PollInterval { type Error = Report; diff --git a/tests/it/helper.rs b/tests/it/helper.rs index 4348fb92..da2a854e 100644 --- a/tests/it/helper.rs +++ b/tests/it/helper.rs @@ -40,16 +40,18 @@ macro_rules! temp_config { () => {{ let tmp = tempfile::tempdir().expect("must create tempdir"); let dir = tmp.path().join("debug"); + let fossa_key = std::env::var("FOSSA_API_KEY").expect("test"); + let content = indoc::formatdoc! {r#" fossa_endpoint: https://app.fossa.com - fossa_integration_key: abcd1234 + fossa_integration_key: {} version: 1 debugging: location: {dir:?} retention: days: 1 integrations: - "#}; + "#, fossa_key }; let path = tmp.path().join("config.yml"); std::fs::write(&path, content).expect("must write config file"); From 1a9c47235b6144ffc542830173bfc7d259799bd9 Mon Sep 17 00:00:00 2001 From: Jeffrey Huynh Date: Mon, 2 Oct 2023 13:10:31 -0600 Subject: [PATCH 4/6] Adding integration test guard and reformatting with clippy --- src/cmd/run.rs | 7 +++---- tests/it/binary.rs | 3 +++ tests/it/helper.rs | 4 ++++ 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/cmd/run.rs b/src/cmd/run.rs index 2179bd4c..e41a2ed9 100644 --- a/src/cmd/run.rs +++ b/src/cmd/run.rs @@ -136,7 +136,7 @@ async fn integration_connections(integrations: &Integrations) -> Result<(), Erro for integration in integrations.iter() { let Protocol::Git(transport) = integration.protocol(); - if let Ok(_) = repository::ls_remote(transport).await { + if repository::ls_remote(transport).await.is_ok() { return Ok(()); } } @@ -150,15 +150,14 @@ async fn integration_connections(integrations: &Integrations) -> Result<(), Erro #[tracing::instrument(skip_all)] /// Validate that Broker can connect to FOSSA async fn fossa_connection(config: &Config) -> Result<(), Error> { - let org_lookup = match fossa::OrgConfig::lookup(config.fossa_api()).await { + match fossa::OrgConfig::lookup(config.fossa_api()).await { Ok(_) => Ok(()), Err(err) => err .change_context(Error::FossaConnection) .wrap_err() .help("ensure that your fossa key is configured correctly") .describe("fossa key"), - }; - org_lookup + } } /// Conduct internal diagnostics to ensure Broker is still in a good state. diff --git a/tests/it/binary.rs b/tests/it/binary.rs index a73a8ac6..96622ff9 100644 --- a/tests/it/binary.rs +++ b/tests/it/binary.rs @@ -7,6 +7,7 @@ use std::path::Path; use tempfile::TempDir; +use crate::guard_integration_test; use crate::temp_config; macro_rules! run { @@ -47,6 +48,8 @@ fn interrupt(pid: u32) { #[track_caller] #[cfg(target_family = "unix")] fn run_and_interrupt_broker(tmp: &TempDir, config_path: &Path) { + guard_integration_test!(); + let config_path = config_path.to_string_lossy().to_string(); let data_root = tmp.path().to_string_lossy().to_string(); let child = run!(broker => "run -c {config_path} -r {data_root}"); diff --git a/tests/it/helper.rs b/tests/it/helper.rs index da2a854e..befc2b90 100644 --- a/tests/it/helper.rs +++ b/tests/it/helper.rs @@ -293,6 +293,10 @@ macro_rules! guard_integration_test { if std::env::var("RUN_INTEGRATION_TESTS").is_err() { return; } + + if std::env::var("FOSSA_API_KEY").is_err() { + return; + } }; } From f92335bcb998c5fa9d84ec5d83cb660d18d33aff Mon Sep 17 00:00:00 2001 From: Jeffrey Huynh Date: Tue, 3 Oct 2023 11:32:16 -0700 Subject: [PATCH 5/6] Changing preflight checks function name for clearer intent. Adding specific error for preflight checks --- src/cmd/run.rs | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/src/cmd/run.rs b/src/cmd/run.rs index e41a2ed9..6d5bcf8f 100644 --- a/src/cmd/run.rs +++ b/src/cmd/run.rs @@ -83,6 +83,10 @@ pub enum Error { #[error("run FOSSA CLI")] RunFossaCli, + /// Preflight checks failed + #[error("preflight checks")] + PreflightChecks, + /// Failed to connect to at least one integration #[error("integration connections")] IntegrationConnection, @@ -122,14 +126,16 @@ pub async fn main(ctx: &AppContext, config: Config, db: D) -> Resul /// Checks and catches network misconfigurations before Broker attempts its operations async fn preflight_checks(ctx: &CmdContext) -> Result<(), Error> { - let validate_integration_connections = integration_connections(ctx.config.integrations()); - let validate_fossa_connection = fossa_connection(&ctx.config); - try_join!(validate_integration_connections, validate_fossa_connection).discard_ok() + let check_integration_connections = check_integration_connections(ctx.config.integrations()); + let check_fossa_connection = check_fossa_connection(&ctx.config); + try_join!(check_integration_connections, check_fossa_connection) + .discard_ok() + .change_context(Error::PreflightChecks) } #[tracing::instrument(skip_all)] -/// Validate that Broker can connect to at least one integration -async fn integration_connections(integrations: &Integrations) -> Result<(), Error> { +/// Check that Broker can connect to at least one integration +async fn check_integration_connections(integrations: &Integrations) -> Result<(), Error> { if integrations.as_ref().is_empty() { return Ok(()); } @@ -148,15 +154,15 @@ async fn integration_connections(integrations: &Integrations) -> Result<(), Erro } #[tracing::instrument(skip_all)] -/// Validate that Broker can connect to FOSSA -async fn fossa_connection(config: &Config) -> Result<(), Error> { +/// Check that Broker can connect to FOSSA +async fn check_fossa_connection(config: &Config) -> Result<(), Error> { match fossa::OrgConfig::lookup(config.fossa_api()).await { Ok(_) => Ok(()), Err(err) => err .change_context(Error::FossaConnection) .wrap_err() - .help("ensure that your fossa key is configured correctly") - .describe("fossa key"), + .help("run broker fix for detailed explanation on the failing fossa connection") + .describe("fossa connection"), } } From f80db0de5f67ad61e4e13c11aeb909aef10e0efc Mon Sep 17 00:00:00 2001 From: Jeffrey Huynh Date: Tue, 3 Oct 2023 16:38:24 -0700 Subject: [PATCH 6/6] Changing wording of help message --- src/cmd/run.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cmd/run.rs b/src/cmd/run.rs index 1461c21b..95c1ce1d 100644 --- a/src/cmd/run.rs +++ b/src/cmd/run.rs @@ -173,7 +173,7 @@ async fn check_fossa_connection(config: &Config) -> Result<(), Error> { Err(err) => err .change_context(Error::FossaConnection) .wrap_err() - .help("run broker fix for detailed explanation on the failing fossa connection") + .help("run broker fix for detailed explanation on failing fossa connection") .describe("fossa connection"), } }