diff --git a/ee/app/models/remote_development/workspace.rb b/ee/app/models/remote_development/workspace.rb index b4cf42e21aa02ba284adefce34b498f9dcf9788f..692fc04ec0710e9182a59dbac5d013f87632f2e6 100644 --- a/ee/app/models/remote_development/workspace.rb +++ b/ee/app/models/remote_development/workspace.rb @@ -101,6 +101,14 @@ class Workspace < ApplicationRecord new_record? || actual_state_changed? end + before_save :delete_workspace_token, if: -> do + saved_change_to_desired_state? && !desired_state_running? + end + + after_save :do_create_workspace_token, if: -> do + saved_change_to_desired_state? && desired_state_running? + end + after_save :track_started_workspace, if: -> do saved_change_to_desired_state? && desired_state_running? end @@ -313,6 +321,27 @@ def touch_actual_state_updated_at self.actual_state_updated_at = Time.current.utc end + # @return [void] + def do_create_workspace_token + delete_workspace_token # rotate (delete) the token if it already exists (fail-safe, normally should never happen) + + # NOTE: This is a convenience method provided by Rails for has_one associations. This is also why we have to + # name the method do_..., in order to not conflict with the convenience method. + create_workspace_token! + + nil + end + + # @return [void] + def delete_workspace_token + return unless workspace_token.present? + + workspace_token.destroy + self.workspace_token = nil # ActiveRecord doesn't automatically set the instance state for the attribute to nil. + + nil + end + # @return [void] def track_started_workspace track_internal_event( @@ -323,6 +352,7 @@ def track_started_workspace label: previously_new_record? ? "new" : "existing" } ) + nil end end diff --git a/ee/spec/lib/remote_development/workspace_operations/create/workspace_observer_spec.rb b/ee/spec/lib/remote_development/workspace_operations/create/workspace_observer_spec.rb index 8471198576e0cf565d821c38b4306b29749128ee..4f98fba5360f2fdf10edc42c05435fe7f66bafae 100644 --- a/ee/spec/lib/remote_development/workspace_operations/create/workspace_observer_spec.rb +++ b/ee/spec/lib/remote_development/workspace_operations/create/workspace_observer_spec.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -require "spec_helper" +require "fast_spec_helper" # rubocop:disable RSpec/VerifiedDoubleReference -- We're using the quoted version so we can use fast_spec_helper RSpec.describe RemoteDevelopment::WorkspaceOperations::Create::WorkspaceObserver, feature_category: :workspaces do diff --git a/ee/spec/models/remote_development/workspace_spec.rb b/ee/spec/models/remote_development/workspace_spec.rb index 2959d32f23b5af9e2bb6cf8f6eba0db81f3dc7e8..303baae104c7c76b59a085c9d617324cfd9aa762 100644 --- a/ee/spec/models/remote_development/workspace_spec.rb +++ b/ee/spec/models/remote_development/workspace_spec.rb @@ -11,7 +11,7 @@ let(:workspaces_per_user_quota) { 10 } let(:workspaces_quota) { 10 } let(:dns_zone) { "workspace.me" } - let(:desired_state) { states_module::STOPPED } + let(:desired_state) { states_module::RUNNING } let(:actual_state) { states_module::STOPPED } let_it_be(:agent, reload: true) { create(:ee_cluster_agent) } @@ -142,6 +142,47 @@ } end end + + describe "WorkspaceToken deletion" do + before do + workspace.save! + end + + context "when changing from Running" do + using RSpec::Parameterized::TableSyntax + + let(:desired_state) { states_module::RUNNING } + + shared_examples "deletes the associated WorkspaceToken record if it exists" do + it "deletes the associated WorkspaceToken record if it exists when Stopped" do + expect { workspace.update!(desired_state: new_desired_state) }.to change { + RemoteDevelopment::WorkspaceToken.count + }.by(-1) + + expect(workspace.workspace_token).to be_nil + end + + it "does not raise an error when WorkspaceToken record does not exist" do + workspace.workspace_token.destroy! + workspace.reload + + expect { workspace.update!(desired_state: new_desired_state) }.not_to raise_error + end + end + + where(:new_desired_state) do + [ + states_module::RESTART_REQUESTED, + states_module::STOPPED, + states_module::TERMINATED + ] + end + + with_them do + it_behaves_like "deletes the associated WorkspaceToken record if it exists" + end + end + end end describe "before_validation" do @@ -167,6 +208,8 @@ describe "after_save" do describe "track_started_workspace callback" do context "when desired_state changes to Running" do + let(:desired_state) { states_module::STOPPED } + it "triggers the event" do expect(workspace).to receive(:track_started_workspace) workspace.update!(desired_state: states_module::RUNNING) @@ -175,36 +218,100 @@ it "triggers internal event with new label on new record" do expect { workspace.update!(desired_state: states_module::RUNNING) } .to trigger_internal_events("track_started_workspaces") - .with(user: user, project: project, additional_properties: { - label: "new" - }) - .and increment_usage_metrics("counts.count_total_workspaces_started") + .with(user: user, project: project, additional_properties: { + label: "new" + }) + .and increment_usage_metrics("counts.count_total_workspaces_started") end it "triggers internal event with existing label on existing record" do workspace.save!(desired_state: "Stopped") expect { workspace.update!(desired_state: states_module::RUNNING) } .to trigger_internal_events("track_started_workspaces") - .with(user: user, project: project, additional_properties: { - label: "existing" - }) - .and increment_usage_metrics("counts.count_total_workspaces_started") + .with(user: user, project: project, additional_properties: { + label: "existing" + }) + .and increment_usage_metrics("counts.count_total_workspaces_started") end end context "when desired_state changes to a value other than Running" do + let(:desired_state) { states_module::RUNNING } + it "does not trigger the event and metric" do expect { workspace.update!(desired_state: states_module::STOPPED) } .to not_trigger_internal_events("track_started_workspaces") - .and not_increment_usage_metrics('counts.count_total_workspaces_started') + .and not_increment_usage_metrics('counts.count_total_workspaces_started') end end context "when desired_state doesn't change" do + before do + workspace.save! + end + it "does not trigger the event" do expect { workspace.update!(name: "workspace_new_name") } .to not_trigger_internal_events("track_started_workspaces") - .and not_increment_usage_metrics('counts.count_total_workspaces_started') + .and not_increment_usage_metrics('counts.count_total_workspaces_started') + end + end + end + + describe "WorkspaceToken creation" do + context "for a new record" do + it "creates an associated WorkspaceToken record" do + expect(workspace).to be_new_record # fixture sanity check + + expect { workspace.save! }.to change { + RemoteDevelopment::WorkspaceToken.count + }.by(1) + + expect(workspace.workspace_token).not_to be_nil + end + end + + context "for an existing record" do + context "when changing to Running" do + using RSpec::Parameterized::TableSyntax + + let(:desired_state) { states_module::STOPPED } + + before do + workspace.save! + end + + it "creates an associated WorkspaceToken record" do + expect(workspace.workspace_token).to be_nil # fixture sanity check + + expect { workspace.update!(desired_state: states_module::RUNNING) }.to change { + RemoteDevelopment::WorkspaceToken.count + }.by(1) + + expect(workspace.workspace_token).not_to be_nil + end + + context "when WorkspaceToken record already exists" do + before do + workspace.create_workspace_token! + end + + it "does not raise an error" do + expect { workspace.update!(desired_state: states_module::RUNNING) }.not_to raise_error + + expect(workspace.workspace_token).not_to be_nil + end + + it "rotates (recreates) the existing token" do + previous_token_id = workspace.workspace_token.id + + expect { workspace.update!(desired_state: states_module::RUNNING) }.not_to change { + RemoteDevelopment::WorkspaceToken.count + } + + expect(workspace.workspace_token.id).not_to eq(previous_token_id) + end + end end end end