Skip to content

Commit

Permalink
feat(crons): Fully drop auth-token based check in support (#1969)
Browse files Browse the repository at this point in the history
Co-authored-by: Daniel Szoke <szokeasaurusrex@users.noreply.github.com>
  • Loading branch information
evanpurkhiser and szokeasaurusrex authored Mar 6, 2024
1 parent 01303af commit 6a449c0
Show file tree
Hide file tree
Showing 6 changed files with 34 additions and 168 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,12 @@

"You know what they say. Fool me once, strike one, but fool me twice... strike three." — Michael Scott

## Unreleased

### Cron Monitor Changes

The `monitors run` subcommand now no longer accepts `--auth-token` or other means of authentication using token-based auth. It is now required to use DSN based auth to monitor cron jobs using the sentry-cli.

## 2.29.1

Updated version 2.29.0 changelog. No code changes.
Expand Down
67 changes: 0 additions & 67 deletions src/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -667,40 +667,6 @@ impl Api {
}
}
}

/// Create a new checkin for a monitor
pub fn create_monitor_checkin(
&self,
monitor_slug: &String,
checkin: &ApiCreateMonitorCheckIn,
) -> ApiResult<ApiMonitorCheckIn> {
let path = &format!("/monitors/{}/checkins/", PathArg(monitor_slug),);
let resp = self.post(path, checkin)?;
if resp.status() == 404 {
return Err(ApiErrorKind::ResourceNotFound.into());
}
resp.convert()
}

/// Update a checkin for a monitor
pub fn update_monitor_checkin(
&self,
monitor_slug: &String,
checkin_id: &Uuid,
checkin: &ApiUpdateMonitorCheckIn,
) -> ApiResult<ApiMonitorCheckIn> {
let path = &format!(
"/monitors/{}/checkins/{}/",
PathArg(monitor_slug),
PathArg(checkin_id),
);
let resp = self.put(path, checkin)?;

if resp.status() == 404 {
return Err(ApiErrorKind::ResourceNotFound.into());
}
resp.convert()
}
}

impl<'a> AuthenticatedApi<'a> {
Expand Down Expand Up @@ -2564,39 +2530,6 @@ pub struct Monitor {
pub status: String,
}

#[derive(Debug, Serialize, Deserialize)]
#[serde(rename_all = "snake_case")]
pub enum ApiMonitorCheckInStatus {
Unknown,
Ok,
InProgress,
Error,
}

#[derive(Debug, Deserialize)]
pub struct ApiMonitorCheckIn {
pub id: Uuid,
pub status: Option<ApiMonitorCheckInStatus>,
pub duration: Option<u64>,
pub environment: Option<String>,
}

#[derive(Debug, Serialize)]
pub struct ApiCreateMonitorCheckIn {
pub status: ApiMonitorCheckInStatus,
pub environment: String,
}

#[derive(Debug, Serialize, Default)]
pub struct ApiUpdateMonitorCheckIn {
#[serde(skip_serializing_if = "Option::is_none")]
pub status: Option<ApiMonitorCheckInStatus>,
#[serde(skip_serializing_if = "Option::is_none")]
pub duration: Option<u64>,
#[serde(skip_serializing_if = "Option::is_none")]
pub environment: Option<String>,
}

#[derive(Deserialize, Debug)]
pub struct RepoProvider {
pub id: String,
Expand Down
97 changes: 11 additions & 86 deletions src/commands/monitors/run.rs
Original file line number Diff line number Diff line change
@@ -1,20 +1,18 @@
use chrono_tz::Tz;
use log::warn;
use std::process;
use std::time::{Duration, Instant};
use uuid::Uuid;

use anyhow::Result;
use clap::{Arg, ArgAction, ArgMatches, Command};
use anyhow::{Context, Result};
use clap::{Arg, ArgMatches, Command};
use console::style;

use sentry::protocol::{MonitorCheckIn, MonitorCheckInStatus, MonitorConfig, MonitorSchedule};
use sentry::types::Dsn;

use crate::api::{Api, ApiCreateMonitorCheckIn, ApiMonitorCheckInStatus, ApiUpdateMonitorCheckIn};
use crate::config::Config;
use crate::utils::event::with_sentry_client;
use crate::utils::system::{print_error, QuietExit};
use crate::utils::system::QuietExit;

pub fn make_command(command: Command) -> Command {
command
Expand All @@ -32,14 +30,6 @@ pub fn make_command(command: Command) -> Command {
.default_value("production")
.help("Specify the environment of the monitor."),
)
.arg(
Arg::new("allow_failure")
.short('f')
.long("allow-failure")
.action(ArgAction::SetTrue)
.help("Run provided command even when Sentry reports an error.")
.hide(true),
)
.arg(
Arg::new("args")
.value_name("ARGS")
Expand Down Expand Up @@ -107,6 +97,8 @@ pub fn make_command(command: Command) -> Command {
issue. Requires --schedule.",
),
)
// Hide auth token from --help output
.arg(Arg::new("auth_token").long("auth-token").hide(true))
}

fn run_program(args: Vec<&String>, monitor_slug: &str) -> (bool, Option<i32>, Duration) {
Expand Down Expand Up @@ -176,51 +168,6 @@ fn dsn_execute(
(success, code)
}

fn token_execute(
args: Vec<&String>,
monitor_slug: &str,
environment: &str,
) -> (bool, Option<i32>, Option<anyhow::Error>) {
let api = Api::current();
let monitor_checkin = api.create_monitor_checkin(
&monitor_slug.to_owned(),
&ApiCreateMonitorCheckIn {
status: ApiMonitorCheckInStatus::InProgress,
environment: environment.to_string(),
},
);

let (success, code, elapsed) = run_program(args, monitor_slug);

match monitor_checkin {
Ok(checkin) => {
let status = if success {
ApiMonitorCheckInStatus::Ok
} else {
ApiMonitorCheckInStatus::Error
};

let duration = Some(elapsed.as_secs() * 1000 + u64::from(elapsed.subsec_millis()));

api.update_monitor_checkin(
&monitor_slug.to_owned(),
&checkin.id,
&ApiUpdateMonitorCheckIn {
status: Some(status),
duration,
environment: Some(environment.to_string()),
},
)
.ok();
}
Err(e) => {
return (success, code, Some(e.into()));
}
}

(success, code, None)
}

fn parse_monitor_config_args(matches: &ArgMatches) -> Result<Option<MonitorConfig>> {
let Some(schedule) = matches.get_one::<String>("schedule") else {
return Ok(None);
Expand All @@ -238,41 +185,19 @@ fn parse_monitor_config_args(matches: &ArgMatches) -> Result<Option<MonitorConfi

pub fn execute(matches: &ArgMatches) -> Result<()> {
let config = Config::current();
let dsn = config.get_dsn().ok();

// Token based auth is deprecated, prefer DSN style auth for monitor checkins.
// Using token based auth *DOES NOT WORK* when using slugs.
if dsn.is_none() {
warn!("Token auth is deprecated for cron monitor checkins and will be removed in the next major version.");
warn!("Please use DSN auth.");
}
// Token based auth has been removed, prefer DSN style auth for monitor checkins
let dsn = config.get_dsn().ok().context(
"Token auth is no longer supported for cron monitor checkins. Please use DSN auth.\n\
See: https://docs.sentry.io/product/crons/getting-started/cli/#configuration",
)?;

let args: Vec<_> = matches.get_many::<String>("args").unwrap().collect();
let monitor_slug = matches.get_one::<String>("monitor_slug").unwrap();
let environment = matches.get_one::<String>("environment").unwrap();
let monitor_config = parse_monitor_config_args(matches)?;

let (success, code) = match dsn {
// Use envelope API when dsn is provided. This is the prefered way to create check-ins,
// and the legacy API will be removed in the next major CLI version.
Some(dsn) => dsn_execute(dsn, args, monitor_slug, environment, monitor_config),
// Use legacy API when DSN is not provided
None => {
if monitor_config.is_some() {
anyhow::bail!("Crons monitor upserts are only supported with DSN auth. Please try again with \
DSN auth or repeat the command without the `schedule` argument.");
}
let (success, code, err) = token_execute(args, monitor_slug, environment);
if let Some(e) = err {
if matches.get_flag("allow_failure") {
print_error(&e);
} else {
return Err(e);
}
}
(success, code)
}
};
let (success, code) = dsn_execute(dsn, args, monitor_slug, environment, monitor_config);

if !success {
return Err(QuietExit(code.unwrap_or(1)).into());
Expand Down
12 changes: 5 additions & 7 deletions tests/integration/_cases/monitors/monitors-run-help.trycmd
Original file line number Diff line number Diff line change
Expand Up @@ -15,28 +15,26 @@ Options:
--header <KEY:VALUE>
Custom headers that should be attached to all requests
in key:value format.
--auth-token <AUTH_TOKEN>
Use the given Sentry auth token.
-s, --schedule <schedule>
Configure the cron monitor with the given schedule (crontab format). Enclose the schedule
in quotes to ensure your command line environment parses the argument correctly.
--check-in-margin <checkin_margin>
The allowed margin of minutes after the expected check-in time that the monitor will not
be considered missed for. Requires --schedule.
--log-level <LOG_LEVEL>
Set the log output verbosity. [possible values: trace, debug, info, warn, error]
--max-runtime <max_runtime>
The allowed duration in minutes that the monitor may be in progress for before being
considered failed due to timeout. Requires --schedule.
--quiet
Do not print any output while preserving correct exit code. This flag is currently
implemented only for selected subcommands. [aliases: silent]
--log-level <LOG_LEVEL>
Set the log output verbosity. [possible values: trace, debug, info, warn, error]
--timezone <timezone>
A tz database string (e.g. "Europe/Vienna") representing the monitor's execution
schedule's timezone. Requires --schedule.
--failure-issue-threshold <failure_issue_threshold>
The number of consecutive missed or error check-ins that trigger an issue. Requires
--schedule.
--quiet
Do not print any output while preserving correct exit code. This flag is currently
implemented only for selected subcommands. [aliases: silent]
--recovery-threshold <recovery_threshold>
The number of consecutive successful check-ins that resolve an issue. Requires --schedule.
-h, --help
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
```
$ sentry-cli monitors run foo-monitor -- cmd.exe /C echo 123
? success
WARN [..] Token auth is deprecated for cron monitor checkins and will be removed in the next major version.
WARN [..] Please use DSN auth.
123
? failed
error: Token auth is no longer supported for cron monitor checkins. Please use DSN auth.
See: https://docs.sentry.io/product/crons/getting-started/cli/#configuration

Add --log-level=[info|debug] or export SENTRY_LOG_LEVEL=[info|debug] to see more output.
Please attach the full debug log to all bug reports.

```
10 changes: 6 additions & 4 deletions tests/integration/_cases/monitors/monitors-run-token-auth.trycmd
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
```
$ sentry-cli monitors run foo-monitor -- echo 123
? success
WARN [..] Token auth is deprecated for cron monitor checkins and will be removed in the next major version.
WARN [..] Please use DSN auth.
123
? failed
error: Token auth is no longer supported for cron monitor checkins. Please use DSN auth.
See: https://docs.sentry.io/product/crons/getting-started/cli/#configuration

Add --log-level=[info|debug] or export SENTRY_LOG_LEVEL=[info|debug] to see more output.
Please attach the full debug log to all bug reports.

```

0 comments on commit 6a449c0

Please sign in to comment.