diff --git a/CHANGELOG.md b/CHANGELOG.md index fb88997e6b..3dbeb6bf43 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,8 @@ ### Fixes - Fixed a bug where the `dart-symbol-map` command did not accept the `--url` argument ([#3108](https://github.com/getsentry/sentry-cli/pull/3108)). +- The `dart-symbol-map upload` command now correctly resolves the organization from the auth token payload ([#3065](https://github.com/getsentry/sentry-cli/pull/3065)). +- The `dart-symbol-map upload` command now correctly resolves the organization from the auth token payload ([#3113](https://github.com/getsentry/sentry-cli/pull/3113)). ## 3.1.0 diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 0ee65a94d4..9e9ef12b9b 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,12 +1,3 @@ -# Adding new commands -For new commands, it is recommended to use clap's [Derive API](https://docs.rs/clap/latest/clap/_derive/index.html). -In contrast to the [Builder API](https://docs.rs/clap/latest/clap/_tutorial/index.html), the Derive API makes it: -- Easier to read, write, and modify commands and arguments. -- Easier to keep argument declaration and reading in sync. -- Easier to reuse shared arguments. - -An existing example of how to use the Derive API is the `send-metric` command. - # Integration Tests Integration tests are written using `trycmd` crate. Consult the docs in case you need to understand how it works https://docs.rs/trycmd/latest/trycmd/. diff --git a/src/commands/dart_symbol_map/mod.rs b/src/commands/dart_symbol_map/mod.rs index 315f2122b8..accfaff7b1 100644 --- a/src/commands/dart_symbol_map/mod.rs +++ b/src/commands/dart_symbol_map/mod.rs @@ -1,47 +1,27 @@ use anyhow::Result; -use clap::{ArgMatches, Args, Command, Parser as _, Subcommand}; +use clap::{ArgMatches, Command}; + +use crate::utils::args::ArgExt as _; pub mod upload; const GROUP_ABOUT: &str = "Manage Dart/Flutter symbol maps for Sentry."; -const UPLOAD_ABOUT: &str = - "Upload a Dart/Flutter symbol map (dartsymbolmap) for deobfuscating Dart exception types."; -const UPLOAD_LONG_ABOUT: &str = - "Upload a Dart/Flutter symbol map (dartsymbolmap) for deobfuscating Dart exception types.{n}{n}Examples:{n} sentry-cli dart-symbol-map upload --org my-org --project my-proj path/to/dartsymbolmap.json path/to/debug/file{n}{n}The mapping must be a JSON array of strings with an even number of entries (pairs).{n}The debug file must contain exactly one Debug ID. {n}{n}\ - This command is supported on Sentry SaaS and self-hosted versions ≥25.8.0."; -#[derive(Args)] -pub(super) struct DartSymbolMapArgs { - #[command(subcommand)] - pub(super) subcommand: DartSymbolMapSubcommand, -} +pub(super) fn make_command(mut command: Command) -> Command { + command = command + .about(GROUP_ABOUT) + .subcommand_required(true) + .arg_required_else_help(true) + .org_arg() + .project_arg(false); -#[derive(Subcommand)] -#[command(about = GROUP_ABOUT)] -pub(super) enum DartSymbolMapSubcommand { - #[command(about = UPLOAD_ABOUT)] - #[command(long_about = UPLOAD_LONG_ABOUT)] - Upload(upload::DartSymbolMapUploadArgs), + command = command.subcommand(upload::make_command(Command::new("upload"))); + command } -pub(super) fn make_command(command: Command) -> Command { - DartSymbolMapSubcommand::augment_subcommands( - command - .about(GROUP_ABOUT) - .subcommand_required(true) - .arg_required_else_help(true), - ) -} - -pub(super) fn execute(_: &ArgMatches) -> Result<()> { - let subcommand = match crate::commands::derive_parser::SentryCLI::parse().command { - crate::commands::derive_parser::SentryCLICommand::DartSymbolMap(DartSymbolMapArgs { - subcommand, - }) => subcommand, - _ => unreachable!("expected dart-symbol-map subcommand"), - }; - - match subcommand { - DartSymbolMapSubcommand::Upload(args) => upload::execute(args), +pub(super) fn execute(matches: &ArgMatches) -> Result<()> { + if let Some(sub_matches) = matches.subcommand_matches("upload") { + return upload::execute(sub_matches); } + unreachable!(); } diff --git a/src/commands/dart_symbol_map/upload.rs b/src/commands/dart_symbol_map/upload.rs index 7f4d600ed5..1462b7cb72 100644 --- a/src/commands/dart_symbol_map/upload.rs +++ b/src/commands/dart_symbol_map/upload.rs @@ -4,7 +4,7 @@ use std::fmt::{Display, Formatter, Result as FmtResult}; use std::path::Path; use anyhow::{bail, Context as _, Result}; -use clap::Args; +use clap::{Arg, ArgMatches, Command}; use crate::api::Api; use crate::config::Config; @@ -42,32 +42,37 @@ impl Assemblable for DartSymbolMapObject<'_> { } } -#[derive(Args, Clone)] -pub(crate) struct DartSymbolMapUploadArgs { - #[arg(short = 'o', long = "org")] - #[arg(help = "The organization ID or slug.")] - pub(super) org: Option, - - #[arg(short = 'p', long = "project")] - #[arg(help = "The project ID or slug.")] - pub(super) project: Option, - - #[arg(value_name = "MAPPING")] - #[arg( - help = "Path to the dartsymbolmap JSON file (e.g. dartsymbolmap.json). Must be a JSON array of strings with an even number of entries (pairs)." - )] - pub(super) mapping: String, - - #[arg(value_name = "DEBUG_FILE")] - #[arg( - help = "Path to the corresponding debug file to extract the Debug ID from. The file must contain exactly one Debug ID." - )] - pub(super) debug_file: String, +const MAPPING_ARG: &str = "mapping"; +const DEBUG_FILE_ARG: &str = "debug_file"; + +pub(super) fn make_command(command: Command) -> Command { + command + .about("Upload a Dart/Flutter symbol map (dartsymbolmap) for deobfuscating Dart exception types.") + .long_about( + "Upload a Dart/Flutter symbol map (dartsymbolmap) for deobfuscating Dart exception types.{n}{n}Examples:{n} sentry-cli dart-symbol-map upload --org my-org --project my-proj path/to/dartsymbolmap.json path/to/debug/file{n}{n}The mapping must be a JSON array of strings with an even number of entries (pairs).{n}The debug file must contain exactly one Debug ID. {n}{n}\ + This command is supported on Sentry SaaS and self-hosted versions ≥25.8.0.", + ) + .arg( + Arg::new(MAPPING_ARG) + .value_name("MAPPING") + .required(true) + .help("Path to the dartsymbolmap JSON file (e.g. dartsymbolmap.json). Must be a JSON array of strings with an even number of entries (pairs)."), + ) + .arg( + Arg::new(DEBUG_FILE_ARG) + .value_name("DEBUG_FILE") + .required(true) + .help("Path to the corresponding debug file to extract the Debug ID from. The file must contain exactly one Debug ID."), + ) } -pub(super) fn execute(args: DartSymbolMapUploadArgs) -> Result<()> { - let mapping_path = &args.mapping; - let debug_file_path = &args.debug_file; +pub(super) fn execute(matches: &ArgMatches) -> Result<()> { + let mapping_path = matches + .get_one::(MAPPING_ARG) + .expect("required by clap"); + let debug_file_path = matches + .get_one::(DEBUG_FILE_ARG) + .expect("required by clap"); // Extract Debug ID(s) from the provided debug file let dif = DifFile::open_path(debug_file_path, None)?; @@ -101,8 +106,7 @@ pub(super) fn execute(args: DartSymbolMapUploadArgs) -> Result<()> { let file_name = Path::new(mapping_path) .file_name() .and_then(OsStr::to_str) - .unwrap_or(mapping_path) - ; + .unwrap_or(mapping_path); let mapping_len = mapping_file_bytes.len(); let object = DartSymbolMapObject { @@ -113,27 +117,12 @@ pub(super) fn execute(args: DartSymbolMapUploadArgs) -> Result<()> { // Prepare chunked upload let api = Api::current(); - // Resolve org and project like logs: prefer args, fallback to defaults let config = Config::current(); - let (default_org, default_project) = config.get_org_and_project_defaults(); - let org = args - .org - .as_ref() - .or(default_org.as_ref()) - .ok_or_else(|| anyhow::anyhow!( - "No organization specified. Please specify an organization using the --org argument." - ))?; - let project = args - .project - .as_ref() - .or(default_project.as_ref()) - .ok_or_else(|| anyhow::anyhow!( - "No project specified. Use --project or set a default in config." - ))?; + let org = config.get_org(matches)?; + let project = config.get_project(matches)?; let chunk_upload_options = api .authenticated()? - .get_chunk_upload_options(org)?; - + .get_chunk_upload_options(&org)?; // Early file size check against server or default limits (same as debug files) let effective_max_file_size = if chunk_upload_options.max_file_size > 0 { @@ -148,7 +137,7 @@ pub(super) fn execute(args: DartSymbolMapUploadArgs) -> Result<()> { ); } - let options = ChunkOptions::new(chunk_upload_options, org, project) + let options = ChunkOptions::new(chunk_upload_options, &org, &project) .with_max_wait(DEFAULT_MAX_WAIT); let chunked = Chunked::from(object, options.server_options().chunk_size); diff --git a/src/commands/derive_parser.rs b/src/commands/derive_parser.rs index 31d86215be..3be346c13e 100644 --- a/src/commands/derive_parser.rs +++ b/src/commands/derive_parser.rs @@ -2,7 +2,6 @@ use crate::utils::auth_token::AuthToken; use crate::utils::value_parsers::{auth_token_parser, kv_parser}; use clap::{ArgAction::SetTrue, Parser, Subcommand}; -use super::dart_symbol_map::DartSymbolMapArgs; use super::logs::LogsArgs; #[derive(Parser)] @@ -38,5 +37,4 @@ pub(super) struct SentryCLI { #[derive(Subcommand)] pub(super) enum SentryCLICommand { Logs(LogsArgs), - DartSymbolMap(DartSymbolMapArgs), } diff --git a/src/commands/logs/mod.rs b/src/commands/logs/mod.rs index fb44207218..b5e940bf28 100644 --- a/src/commands/logs/mod.rs +++ b/src/commands/logs/mod.rs @@ -39,9 +39,7 @@ pub(super) fn make_command(command: Command) -> Command { } pub(super) fn execute(_: &ArgMatches) -> Result<()> { - let SentryCLICommand::Logs(LogsArgs { subcommand }) = SentryCLI::parse().command else { - unreachable!("expected logs subcommand"); - }; + let SentryCLICommand::Logs(LogsArgs { subcommand }) = SentryCLI::parse().command; eprintln!("{BETA_WARNING}"); match subcommand { diff --git a/tests/integration/test_utils/test_manager.rs b/tests/integration/test_utils/test_manager.rs index 9b99d09c8c..025caab7cc 100644 --- a/tests/integration/test_utils/test_manager.rs +++ b/tests/integration/test_utils/test_manager.rs @@ -207,6 +207,16 @@ impl AssertCmdTestManager { self } + /// Set a custom environment variable for the test. + pub fn env( + mut self, + key: impl AsRef, + value: impl AsRef, + ) -> Self { + self.command.env(key, value); + self + } + /// Run the command and perform assertions. /// /// This function asserts both the mocks and the command result. diff --git a/tests/integration/upload_dart_symbol_map.rs b/tests/integration/upload_dart_symbol_map.rs index 757bbb7a18..d2d376dd42 100644 --- a/tests/integration/upload_dart_symbol_map.rs +++ b/tests/integration/upload_dart_symbol_map.rs @@ -1,4 +1,7 @@ use std::sync::atomic::{AtomicU8, Ordering}; +use std::sync::LazyLock; + +use serde_json::Value; use crate::integration::test_utils::AssertCommand; use crate::integration::{MockEndpointBuilder, TestManager}; @@ -175,3 +178,103 @@ fn command_upload_dart_symbol_map_with_custom_url() { .with_default_token() .run_and_assert(AssertCommand::Success); } + +/// A test to ensure that the command can resolve an organization from an +/// org auth token. +#[test] +fn command_upload_dart_symbol_map_org_from_token() { + /// Path to the mapping file + const MAPPING_PATH: &str = "tests/integration/_fixtures/dart_symbol_map/dartsymbolmap.json"; + + /// A test org auth token with org="wat-org" and empty URL. + /// Format: sntrys_{base64_payload}_{base64_secret} + /// Payload: {"iat":1704374159.069583,"url":"","region_url":"","org":"wat-org"} + const ORG_AUTH_TOKEN_WAT_ORG: &str = "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"; + + /// Checksum of the mapping file + const EXPECTED_CHECKSUM: &str = "6aa44eb08e4a72d1cf32fe7c2504216fb1a3e862"; + + /// Expected request body for uploading the Dart symbol map + static EXPECTED_REQUEST: LazyLock = LazyLock::new(|| { + serde_json::json!({ + EXPECTED_CHECKSUM: { + "chunks": [EXPECTED_CHECKSUM], + "debug_id": "54fdf14a-41a1-426a-a073-8185e11a89d6-83920e6f", + "name": "dartsymbolmap.json", + } + }) + }); + + // When no --org is provided and SENTRY_ORG is not set, the org should be resolved + // from the org auth token. + let call_count = AtomicU8::new(0); + + TestManager::new() + // This endpoint uses "wat-org" in the path - if org resolution fails, + // the request would go to a different path and not match. + .mock_endpoint( + MockEndpointBuilder::new("GET", "/api/0/organizations/wat-org/chunk-upload/") + .with_response_file("dart_symbol_map/get-chunk-upload.json"), + ) + .mock_endpoint(MockEndpointBuilder::new( + "POST", + "/api/0/organizations/wat-org/chunk-upload/", + )) + .mock_endpoint( + MockEndpointBuilder::new( + "POST", + "/api/0/projects/wat-org/wat-project/files/difs/assemble/", + ) + .with_header_matcher("content-type", "application/json") + .with_response_fn(move |request| { + let body = request.body().expect("body should be readable"); + let body_json: serde_json::Value = + serde_json::from_slice(body).expect("request body should be valid JSON"); + + assert_eq!( + body_json, *EXPECTED_REQUEST, + "assemble request should match expected checksum payload" + ); + + let response = match call_count.fetch_add(1, Ordering::Relaxed) { + 0 => serde_json::json!({ + EXPECTED_CHECKSUM: { + "state": "not_found", + "missingChunks": [EXPECTED_CHECKSUM], + } + }), + 1 => serde_json::json!({ + EXPECTED_CHECKSUM: { + "state": "created", + "missingChunks": [], + } + }), + 2 => serde_json::json!({ + EXPECTED_CHECKSUM: { + "state": "ok", + "missingChunks": [], + } + }), + n => panic!( + "Only 3 calls to the assemble endpoint expected, but there were {}.", + n + 1 + ), + }; + + serde_json::to_vec(&response).expect("assemble response should be valid JSON") + }) + .expect(3), + ) + .assert_cmd([ + "dart-symbol-map", + "upload", + // No --org flag provided! + MAPPING_PATH, + "tests/integration/_fixtures/Sentry.Samples.Console.Basic.pdb", + ]) + // Use org auth token with embedded org="wat-org" instead of default token + .env("SENTRY_AUTH_TOKEN", ORG_AUTH_TOKEN_WAT_ORG) + // Explicitly unset SENTRY_ORG to ensure org comes from token + .env("SENTRY_ORG", "") + .run_and_assert(AssertCommand::Success); +}