mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-10-18 16:03:24 +00:00
feat: back the variable expiration handler's built-in Slack and Teams destinations
Backfills the Slack bot token grant for g/variable_expiration_handler, mirroring 20231016142659 for g/error_handler: the slack_bot folder's extra_perms is written once at Slack connect and never refreshed, and disconnecting leaves the folder behind, so workspaces that already connected Slack cannot pick the grant up from the creation-time list alone. Adds run_variable_expiration_test_job so an admin can exercise the handler on demand rather than waiting for a real expiry, which may be months away. It goes through push_variable_expiration_handler so the test run proves what matters: the handler resolves, and the group can read what it needs, under the same identity and argument shape the sweep uses. Also covers extra_args on the edit endpoint, which the settings tab now writes for the built-ins' channel. The endpoint rebuilds the setting from the request, so an omitted extra_args erases it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Do7wJTnL6MG6ngFpCGW4DG
This commit is contained in:
co-authored by
Claude Opus 5
parent
5cc4dcb29c
commit
7cc348874a
@@ -1 +1 @@
|
||||
a4934a3213b598f77a35cdb3d774c45ad125e453
|
||||
a9102dfd61e598fa77c157f940ffa69613a2f98f
|
||||
|
||||
@@ -0,0 +1 @@
|
||||
UPDATE folder SET extra_perms = extra_perms - 'g/variable_expiration_handler' WHERE name = 'slack_bot';
|
||||
@@ -0,0 +1,5 @@
|
||||
-- Grant the variable expiration handler group read access to the Slack bot token, so the
|
||||
-- built-in Slack handler can post. Mirrors 20231016142659 for g/error_handler: the folder's
|
||||
-- extra_perms is written once at Slack connect (ON CONFLICT DO NOTHING) and never refreshed,
|
||||
-- so workspaces that connected Slack before this feature need the backfill to get the grant.
|
||||
UPDATE folder SET extra_perms = JSONB_SET(extra_perms, '{g/variable_expiration_handler}', 'false', true) WHERE name = 'slack_bot';
|
||||
@@ -356,6 +356,38 @@ async fn test_edit_handler_endpoint(db: Pool<Postgres>) -> anyhow::Result<()> {
|
||||
Some("script/u/test-user/expiry_handler".to_string())
|
||||
);
|
||||
|
||||
// `extra_args` carries the built-in destinations' channel, so it has to survive a save
|
||||
// that keeps the handler. The endpoint rebuilds the setting from the request rather than
|
||||
// merging into the stored one, so an omitted `extra_args` erases it — the settings tab
|
||||
// compensates by sending back what it loaded.
|
||||
assert_eq!(
|
||||
edit(
|
||||
"SECRET_TOKEN",
|
||||
serde_json::json!({
|
||||
"path": "script/u/test-user/expiry_handler",
|
||||
"extra_args": { "channel": "windmill-alerts" }
|
||||
})
|
||||
)
|
||||
.await?,
|
||||
200
|
||||
);
|
||||
assert_eq!(
|
||||
stored().await?.map(|h| h["extra_args"].clone()),
|
||||
Some(serde_json::json!({ "channel": "windmill-alerts" }))
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
edit(
|
||||
"SECRET_TOKEN",
|
||||
serde_json::json!({
|
||||
"path": "script/u/test-user/expiry_handler",
|
||||
"extra_args": "not-an-object"
|
||||
})
|
||||
)
|
||||
.await?,
|
||||
400
|
||||
);
|
||||
|
||||
assert_eq!(edit("SECRET_TOKEN", serde_json::json!({})).await?, 200);
|
||||
assert!(
|
||||
stored().await?.is_none(),
|
||||
|
||||
@@ -123,6 +123,10 @@ pub fn workspaced_service() -> Router {
|
||||
"/edit_variable_expiration_handler",
|
||||
post(edit_variable_expiration_handler),
|
||||
)
|
||||
.route(
|
||||
"/run_variable_expiration_test_job",
|
||||
post(run_variable_expiration_test_job),
|
||||
)
|
||||
.route(
|
||||
"/edit_large_file_storage_config",
|
||||
post(edit_large_file_storage_config),
|
||||
@@ -405,6 +409,13 @@ struct RunSlackMessageTestJobResponse {
|
||||
job_uuid: String,
|
||||
}
|
||||
|
||||
#[derive(Deserialize)]
|
||||
struct RunVariableExpirationTestJobRequest {
|
||||
/// Prefixed with `script/` or `flow/`, like the stored setting.
|
||||
handler_path: String,
|
||||
extra_args: Option<serde_json::Value>,
|
||||
}
|
||||
|
||||
#[allow(dead_code)]
|
||||
#[derive(Deserialize)]
|
||||
pub struct EditAutoInvite {
|
||||
@@ -1570,6 +1581,49 @@ async fn run_slack_message_test_job(
|
||||
}))
|
||||
}
|
||||
|
||||
/// Run the workspace's variable expiration handler now, against a placeholder variable.
|
||||
///
|
||||
/// Admin-only, like the setting it tests. The placeholder path is deliberately not a real
|
||||
/// variable: the handler is handed a path and never a value, so nothing has to exist for the
|
||||
/// dispatch to be representative, and naming a real one would invite a handler to rotate it.
|
||||
async fn run_variable_expiration_test_job(
|
||||
authed: ApiAuthed,
|
||||
Extension(db): Extension<DB>,
|
||||
Path(w_id): Path<String>,
|
||||
Json(req): Json<RunVariableExpirationTestJobRequest>,
|
||||
) -> JsonResult<RunSlackMessageTestJobResponse> {
|
||||
require_admin(authed.is_admin, &authed.username)?;
|
||||
|
||||
if let Some(extra_args) = req.extra_args.as_ref() {
|
||||
if !extra_args.is_object() {
|
||||
return Err(Error::BadRequest(
|
||||
"Field `extra_args` expected to be JSON object".to_string(),
|
||||
));
|
||||
}
|
||||
}
|
||||
|
||||
let expiring = windmill_queue::ExpiringVariable {
|
||||
workspace_id: w_id.clone(),
|
||||
variable_path: "f/test/variable_expiration_test".to_string(),
|
||||
description: "Test run from the variable expiration settings tab".to_string(),
|
||||
value_expires_at: Utc::now() + chrono::Duration::hours(1),
|
||||
is_secret: true,
|
||||
};
|
||||
|
||||
let uuid = windmill_queue::push_variable_expiration_test_handler(
|
||||
&db,
|
||||
w_id.as_str(),
|
||||
req.handler_path.as_str(),
|
||||
&expiring,
|
||||
req.extra_args.map(|v| sqlx::types::Json(to_raw_value(&v))),
|
||||
)
|
||||
.await?;
|
||||
|
||||
Ok(Json(RunSlackMessageTestJobResponse {
|
||||
job_uuid: uuid.to_string(),
|
||||
}))
|
||||
}
|
||||
|
||||
#[derive(Deserialize)]
|
||||
struct SetSlackOAuthConfigRequest {
|
||||
slack_oauth_client_id: String,
|
||||
|
||||
@@ -4531,6 +4531,40 @@ paths:
|
||||
schema:
|
||||
type: string
|
||||
|
||||
/w/{workspace}/workspaces/run_variable_expiration_test_job:
|
||||
post:
|
||||
summary: run the variable expiration handler against a placeholder variable
|
||||
operationId: runVariableExpirationTestJob
|
||||
tags:
|
||||
- workspace
|
||||
parameters:
|
||||
- $ref: "#/components/parameters/WorkspaceId"
|
||||
requestBody:
|
||||
description: handler to run and the extra args to pass it
|
||||
required: true
|
||||
content:
|
||||
application/json:
|
||||
schema:
|
||||
type: object
|
||||
required:
|
||||
- handler_path
|
||||
properties:
|
||||
handler_path:
|
||||
type: string
|
||||
description: prefixed with `script/` or `flow/`, like the stored setting
|
||||
extra_args:
|
||||
$ref: "#/components/schemas/ScriptArgs"
|
||||
responses:
|
||||
"200":
|
||||
description: status
|
||||
content:
|
||||
text/json:
|
||||
schema:
|
||||
type: object
|
||||
properties:
|
||||
job_uuid:
|
||||
type: string
|
||||
|
||||
/w/{workspace}/workspaces/edit_success_handler:
|
||||
post:
|
||||
summary: edit success handler
|
||||
|
||||
@@ -3023,6 +3023,27 @@ pub(crate) async fn push_variable_expiration_handler<'c>(
|
||||
.await
|
||||
}
|
||||
|
||||
/// Dispatch the variable expiration handler once, on demand, for a variable the caller
|
||||
/// names rather than one the sweep found due.
|
||||
///
|
||||
/// Goes through `push_variable_expiration_handler` rather than reimplementing the push so a
|
||||
/// test run proves the thing that actually matters: that the handler resolves and that
|
||||
/// `g/variable_expiration_handler` can read what it needs, under the same identity and the
|
||||
/// same argument shape a real expiry would use.
|
||||
pub async fn push_variable_expiration_test_handler(
|
||||
db: &Pool<Postgres>,
|
||||
w_id: &str,
|
||||
handler_path: &str,
|
||||
expiring: &ExpiringVariable,
|
||||
extra_args: Option<Json<Box<RawValue>>>,
|
||||
) -> windmill_common::error::Result<Uuid> {
|
||||
let tx = db.begin().await?;
|
||||
let (uuid, tx) =
|
||||
push_variable_expiration_handler(db, tx, w_id, handler_path, expiring, extra_args).await?;
|
||||
tx.commit().await?;
|
||||
Ok(uuid)
|
||||
}
|
||||
|
||||
pub async fn push_success_handler<'a, 'c, T: Serialize + Send + Sync>(
|
||||
db: &Pool<Postgres>,
|
||||
job_id: Uuid,
|
||||
|
||||
Reference in New Issue
Block a user