diff --git a/internal/db/schema/migrations/oss/postgres/70/11_session_recording.up.sql b/internal/db/schema/migrations/oss/postgres/70/11_session_recording.up.sql index d37d74759..d094b3c41 100644 --- a/internal/db/schema/migrations/oss/postgres/70/11_session_recording.up.sql +++ b/internal/db/schema/migrations/oss/postgres/70/11_session_recording.up.sql @@ -7,6 +7,24 @@ begin; comment on domain rec_timestamp is 'a nullable timestamp with a time zone used for start and end times of recordings'; + + create table recording_session_state_enm ( + name text primary key + constraint only_predefined_states_allowed + check(name in ('unknown', 'started', 'available')) + ); + comment on table recording_session_state_enm is + 'recording_session_state_enm holds valid values for the state of a recording_session row.'; + + insert into recording_session_state_enm (name) + values + ('unknown'), + ('started'), + ('available'); + + create trigger immutable_columns before update on recording_session_state_enm + for each row execute procedure immutable_columns('name'); + create table recording_session ( public_id wt_public_id primary key, storage_bucket_id wt_public_id not null @@ -58,8 +76,22 @@ begin; end_time rec_timestamp null constraint end_time_null_or_after_start_time check (end_time > start_time), + state text not null default 'started' + constraint recording_session_state_enm_fkey + references recording_session_state_enm (name) + on delete restrict + on update cascade, + error_details wt_sentinel not null default wt_to_sentinel('no error details'), constraint recording_session_session_id_public_id_uq - unique (session_id, public_id) + unique (session_id, public_id), + -- Error details are allowed two different types of values: + -- - e'\ufffeno error details\uffff', the sentinel value for "no error details", only when the recording is in the started or available state. + -- - some error message when the recording is in the unknown state. + constraint error_details_set_iff_state_not_started + check ( + (error_details = wt_to_sentinel('no error details') and state in ('started', 'available')) or + (error_details != wt_to_sentinel('no error details') and state = 'unknown') + ) ); comment on table recording_session is 'recording_session holds metadata for the recording of a session. It outlives the session itself.'; @@ -86,6 +118,9 @@ begin; if new.session_id is null then raise exception 'a new recorded session must have a session_id'; end if; + if new.state is not null and new.state != 'started' then + raise exception 'state can only be set to ''started'' on insert'; + end if; select * into strict _session from session @@ -160,6 +195,21 @@ begin; create trigger insert_session_recording before insert on recording_session for each row execute procedure insert_session_recording(); + create function update_session_recording() returns trigger + as $$ + begin + if new.state != old.state and old.state != 'started' then + raise exception 'state can only be updated once'; + end if; + return new; + end; + $$ language plpgsql; + comment on function update_session_recording is + 'update_session_recording is a before update trigger for the recording_session table.'; + + create trigger update_session_recording before update on recording_session + for each row execute procedure update_session_recording(); + create table recording_connection ( public_id wt_public_id primary key, session_id wt_public_id null, -- Can be null if associated session has been deleted diff --git a/internal/db/schema/migrations/oss/postgres/70/12_session_recording_views.up.sql b/internal/db/schema/migrations/oss/postgres/70/12_session_recording_views.up.sql index 3f6fc1c08..a4642f5e6 100644 --- a/internal/db/schema/migrations/oss/postgres/70/12_session_recording_views.up.sql +++ b/internal/db/schema/migrations/oss/postgres/70/12_session_recording_views.up.sql @@ -12,6 +12,8 @@ select rs.update_time, rs.start_time, rs.end_time, + rs.state, + rs.error_details, sb.scope_id as storage_bucket_scope_id, -- fields that cover the user fields at creation time uh.public_id as user_history_public_id, diff --git a/internal/db/sentinel/sentinel.go b/internal/db/sentinel/sentinel.go index 9221a8e6b..5cfa2672a 100644 --- a/internal/db/sentinel/sentinel.go +++ b/internal/db/sentinel/sentinel.go @@ -17,6 +17,8 @@ const ( // ExternalIdNone is a Boundary sentinel indicating that no id was provided by an // external system. ExternalIdNone = "\ufffenone\uffff" + // ErrorDetailsNone is a Boundary sentinel indicating that there were no error details. + NoErrorDetails = "\ufffeno error details\uffff" ) // Is returns true if s is a valid sentinel. diff --git a/internal/db/sqltest/tests/recording/recording_channel_ssh.sql b/internal/db/sqltest/tests/recording/recording_channel_ssh.sql index c2229ebb1..1ac297d99 100644 --- a/internal/db/sqltest/tests/recording/recording_channel_ssh.sql +++ b/internal/db/sqltest/tests/recording/recording_channel_ssh.sql @@ -15,9 +15,9 @@ begin; select wtt_load('widgets', 'iam', 'kms', 'auth', 'hosts', 'targets', 'sessions'); insert into recording_session - (public_id, storage_bucket_id, session_id) + (public_id, storage_bucket_id, session_id, state) values - ('sr_123456789', 'sb____global', 's2_____clare'); + ('sr_123456789', 'sb____global', 's2_____clare', 'started'); insert into session_connection (public_id, session_id) values diff --git a/internal/db/sqltest/tests/recording/recording_session.sql b/internal/db/sqltest/tests/recording/recording_session.sql index 4b7745ba7..a31cf5570 100644 --- a/internal/db/sqltest/tests/recording/recording_session.sql +++ b/internal/db/sqltest/tests/recording/recording_session.sql @@ -4,11 +4,13 @@ -- recording_session tests the following triggers: -- insert_session_recording -- set_once_columns +-- insert_session_recording +-- update_session_recording -- and the following constraints: -- end_time_null_or_after_start_time begin; - select plan(44); + select plan(53); select wtt_load('widgets', 'iam', 'kms', 'auth', 'hosts', 'targets', 'sessions'); select has_view('session_recording_aggregate', 'view for aggregating session recording info does not exist'); @@ -58,16 +60,24 @@ begin; -- Try to insert row with null session id prepare insert_invalid_recording_session as insert into recording_session - (public_id, storage_bucket_id, session_id) + (public_id, storage_bucket_id, session_id, state) values - ('sr_________1', 'sb____global', null); + ('sr_________1', 'sb____global', null, 'started'); select throws_ok('insert_invalid_recording_session', null, null, 'insert invalid recording_session succeeded'); + -- Try to insert row with non-started state + prepare insert_with_invalid_state as + insert into recording_session + (public_id, storage_bucket_id, session_id, state) + values + ('sr_________1', 'sb_________g', 's1_____clare', 'available'); + select throws_ok('insert_with_invalid_state', null, null, 'insert invalid recording_session state succeeded'); + prepare insert_recording_session as insert into recording_session - (public_id, storage_bucket_id, session_id) + (public_id, storage_bucket_id, session_id, state) values - ('sr_________1', 'sb____global', 's2_____clare'); + ('sr_________1', 'sb____global', 's2_____clare', 'started'); select lives_ok('insert_recording_session'); prepare insert_recording_session_target_address as @@ -85,6 +95,65 @@ begin; where public_id = 'sr_________1'; select throws_ok('invalid_close_recording_session', '23514', null, 'setting end_time before start_time succeeded'); + -- Try to set state to unknown without setting error_details + prepare update_state_to_unknown_without_error as + update recording_session set + state = 'unknown' + where public_id = 'sr_________1'; + select throws_ok('update_state_to_unknown_without_error', 23514, null, 'updating the state to ''unknown'' without setting error_details succeeded'); + + -- Try to set state to available while setting error details to a non-sentinel value + prepare update_state_to_available_without_sentinel_error as + update recording_session set + state = 'available', + error_details = 'an actual error' + where public_id = 'sr_________1'; + select throws_ok('update_state_to_available_without_sentinel_error', 23514, null, 'updating the state to ''available'' without setting error_details to the sentinel value succeeded'); + + -- Try to set state to unknown while setting error_details to the no error sentinel value + prepare update_state_to_unknown_with_sentinel_error as + update recording_session set + state = 'unknown', + error_details = wt_to_sentinel('no error details') + where public_id = 'sr_________1'; + select throws_ok('update_state_to_unknown_with_sentinel_error', 23514, null, 'updating the state to ''unknown'' while setting error_details to the no error sentinel value succeeded'); + + -- Try to set error_details without updating state + prepare update_error_details_without_updating_state as + update recording_session set + error_details = 'some error' + where public_id = 'sr_________1'; + select throws_ok('update_error_details_without_updating_state', 23514, null, 'setting error_details without updating the state succeeded'); + + prepare update_state_to_available as + update recording_session set + state = 'available' + where public_id = 'sr_________1'; + select lives_ok('update_state_to_available'); + + -- Updating state again should error + prepare update_state_to_unknown as + update recording_session set + state = 'unknown', + error_details = 'some error' + where public_id = 'sr_________1'; + select throws_ok('update_state_to_unknown', null, null, 'updating the state away from ''available'' succeeded'); + + prepare update_state_to_unknown_with_error as + update recording_session set + state = 'unknown', + error_details = 'some error' + where public_id = 'sr_________2'; + select lives_ok('update_state_to_unknown_with_error'); + + -- Updating state again should error + prepare update_state_to_available_again as + update recording_session set + state = 'available', + error_details = wt_to_sentinel('no error details') + where public_id = 'sr_________2'; + select throws_ok('update_state_to_available_again', null, null, 'updating the state away from ''unknown'' succeeded'); + prepare close_recording_session as update recording_session set start_time = clock_timestamp()::timestamptz,