Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TRUNK-14370] Do not error on forbidden/unauthorized #443

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

[TRUNK-14370] Do not error on forbidden/unauthorized #443

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a163003
[TRUNK-14370] Do not error on forbidden/unauthorized
cmillar-trunk Feb 27, 2025
ba8c765
Merge branch 'main' into christian/trunk-14370-do-not-report-out-sent…
cmillar-trunk Feb 28, 2025
469e88e
Change exit code on unauthorized to not report OK
cmillar-trunk Feb 28, 2025
ff8374a
Fix failing tests
cmillar-trunk Feb 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 81 additions & 0 deletions cli-tests/src/upload.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use api::message::{
GetQuarantineConfigResponse,
};
use assert_matches::assert_matches;
use axum::http::StatusCode;
use axum::{extract::State, Json};
use bundle::{BundleMeta, FileSetType};
use codeowners::CodeOwners;
Expand Down Expand Up @@ -601,3 +602,83 @@ async fn quarantines_tests_regardless_of_upload() {
*CREATE_BUNDLE_RESPONSE.lock().unwrap() = CreateBundleResponse::Success;
command.assert().success();
}
#[tokio::test(flavor = "multi_thread")]
async fn is_ok_on_unauthorized() {
let temp_dir = tempdir().unwrap();
generate_mock_git_repo(&temp_dir);
generate_mock_valid_junit_xmls(&temp_dir);
generate_mock_codeowners(&temp_dir);

let mut mock_server_builder = MockServerBuilder::new();

mock_server_builder.set_get_quarantining_config_handler(
|Json(_): Json<GetQuarantineConfigRequest>| async {
Err::<Json<GetQuarantineConfigResponse>, StatusCode>(StatusCode::UNAUTHORIZED)
},
);

mock_server_builder.set_create_bundle_handler(
|State(_): State<SharedMockServerState>, _: Json<CreateBundleUploadRequest>| async {
Err::<Json<CreateBundleUploadResponse>, StatusCode>(StatusCode::UNAUTHORIZED)
},
);
let state = mock_server_builder.spawn_mock_server().await;

let mut command = CommandBuilder::upload(temp_dir.path(), state.host.clone()).command();

command.assert().success();
}

#[tokio::test(flavor = "multi_thread")]
async fn is_ok_on_forbidden() {
let temp_dir = tempdir().unwrap();
generate_mock_git_repo(&temp_dir);
generate_mock_valid_junit_xmls(&temp_dir);
generate_mock_codeowners(&temp_dir);

let mut mock_server_builder = MockServerBuilder::new();

mock_server_builder.set_get_quarantining_config_handler(
|Json(_): Json<GetQuarantineConfigRequest>| async {
Err::<Json<GetQuarantineConfigResponse>, StatusCode>(StatusCode::FORBIDDEN)
},
);

mock_server_builder.set_create_bundle_handler(
|State(_): State<SharedMockServerState>, _: Json<CreateBundleUploadRequest>| async {
Err::<Json<CreateBundleUploadResponse>, StatusCode>(StatusCode::FORBIDDEN)
},
);
let state = mock_server_builder.spawn_mock_server().await;

let mut command = CommandBuilder::upload(temp_dir.path(), state.host.clone()).command();

command.assert().success();
}

#[tokio::test(flavor = "multi_thread")]
async fn is_not_ok_on_bad_request() {
let temp_dir = tempdir().unwrap();
generate_mock_git_repo(&temp_dir);
generate_mock_valid_junit_xmls(&temp_dir);
generate_mock_codeowners(&temp_dir);

let mut mock_server_builder = MockServerBuilder::new();

mock_server_builder.set_get_quarantining_config_handler(
|Json(_): Json<GetQuarantineConfigRequest>| async {
Err::<Json<GetQuarantineConfigResponse>, StatusCode>(StatusCode::BAD_REQUEST)
},
);

mock_server_builder.set_create_bundle_handler(
|State(_): State<SharedMockServerState>, _: Json<CreateBundleUploadRequest>| async {
Err::<Json<CreateBundleUploadResponse>, StatusCode>(StatusCode::BAD_REQUEST)
},
);
let state = mock_server_builder.spawn_mock_server().await;

let mut command = CommandBuilder::upload(temp_dir.path(), state.host.clone()).command();

command.assert().failure();
}
33 changes: 23 additions & 10 deletions cli/src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use std::env;

use clap::{Parser, Subcommand};
use clap_verbosity_flag::{log::LevelFilter, InfoLevel, Verbosity};
use http::StatusCode;
use third_party::sentry;
use tracing_subscriber::prelude::*;
use trunk_analytics_cli::{
Expand Down Expand Up @@ -74,18 +75,30 @@ fn main() -> anyhow::Result<()> {
);
match run(cli).await {
Ok(exit_code) => std::process::exit(exit_code),
Err(e) => match (*(e.root_cause())).downcast_ref::<std::io::Error>() {
Some(io_error) if io_error.kind() == std::io::ErrorKind::ConnectionRefused => {
tracing::warn!("Could not connect to trunk's server: {:?}", e);
guard.flush(None);
std::process::exit(exitcode::OK);
Err(error) => {
let root_cause = error.root_cause();
if let Some(io_error) = root_cause.downcast_ref::<std::io::Error>() {
if io_error.kind() == std::io::ErrorKind::ConnectionRefused {
tracing::warn!("Could not connect to trunk's server: {:?}", error);
guard.flush(None);
std::process::exit(exitcode::OK);
}
}
_ => {
tracing::error!("Error: {:?}", e);
guard.flush(None);
std::process::exit(exitcode::SOFTWARE);

if let Some(reqwest_error) = root_cause.downcast_ref::<reqwest::Error>() {
if let Some(status) = reqwest_error.status() {
if status == StatusCode::UNAUTHORIZED || status == StatusCode::FORBIDDEN {
tracing::warn!("Unauthorized to access trunk, are you sure your token is correct? {:?}", error);
guard.flush(None);
std::process::exit(exitcode::OK);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might be a footgun when enabling trunk. If we exit success even though it's misconfigured you'll see the CI job succeed and assume all is well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair; I'll switch it to use the software exit code.

}
}
}
},

tracing::error!("Error: {:?}", error);
guard.flush(None);
std::process::exit(exitcode::SOFTWARE);
}
}
})
}
Expand Down