diff --git a/ee/app/models/remote_development/remote_development_agent_config.rb b/ee/app/models/remote_development/remote_development_agent_config.rb index ada852523b76a2ed653c1464ec5b2cbef9e2cd96..bf9b6f510fff45827a6547e5872341b580dd9819 100644 --- a/ee/app/models/remote_development/remote_development_agent_config.rb +++ b/ee/app/models/remote_development/remote_development_agent_config.rb @@ -14,7 +14,9 @@ class RemoteDevelopmentAgentConfig < ApplicationRecord validates :agent, presence: true validates :dns_zone, hostname: true - # TODO: https://gitlab.com/gitlab-org/gitlab/-/issues/409772 - Make this a type:enum + # NOTE: We do NOT want to use `enum` in the ActiveRecord models, because they break the `ActiveRecord#save` contract + # by throwing an `ArgumentError` on `#save`, instead of `#save!`. + # See https://gitlab.com/gitlab-org/gitlab/-/merge_requests/129708#note_1538946504 for more context. validates :enabled, inclusion: { in: [true], message: 'is currently immutable, and must be set to true' } # noinspection RubyResolve - likely due to https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/tracked-jetbrains-issues/#ruby-31540 before_validation :prevent_dns_zone_update, if: ->(record) { record.persisted? && record.dns_zone_changed? } diff --git a/ee/app/models/remote_development/workspace.rb b/ee/app/models/remote_development/workspace.rb index 7bc35b786ffb811f2018ab60dceac48bb690fa53..35b1254cda964421b07b6fb24d339c2293144815 100644 --- a/ee/app/models/remote_development/workspace.rb +++ b/ee/app/models/remote_development/workspace.rb @@ -31,14 +31,13 @@ class Workspace < ApplicationRecord # See https://gitlab.com/gitlab-org/remote-development/gitlab-remote-development-docs/blob/main/doc/architecture.md?plain=0#workspace-states # for state validation rules - # TODO: https://gitlab.com/gitlab-org/gitlab/-/issues/409773 - # Add validation preventing desired_state change if it is TERMINATED; you can't restart a terminated workspace - # Also reflect this in GraphQL API and Vue component UI validates :desired_state, inclusion: { in: VALID_DESIRED_STATES } validates :actual_state, inclusion: { in: VALID_ACTUAL_STATES } validates :editor, inclusion: { in: ['webide'], message: "'webide' is currently the only supported editor" } validates :max_hours_before_termination, numericality: { less_than_or_equal_to: MAX_HOURS_BEFORE_TERMINATION_LIMIT } + validate :enforce_permanent_termination + ignore_column :force_full_reconciliation, remove_with: '16.7', remove_after: '2023-11-22' scope :with_desired_state_updated_more_recently_than_last_response_to_agent, -> do @@ -83,6 +82,12 @@ def validate_agent_config_presence errors.add(:agent, _('for Workspace must have an associated RemoteDevelopmentAgentConfig')) end + def enforce_permanent_termination + return unless persisted? && desired_state_changed? && desired_state_was == Workspaces::States::TERMINATED + + errors.add(:desired_state, "is 'Terminated', and cannot be updated. Create a new workspace instead.") + end + def touch_desired_state_updated_at self.desired_state_updated_at = Time.current.utc end diff --git a/ee/spec/models/remote_development/workspace_spec.rb b/ee/spec/models/remote_development/workspace_spec.rb index 06411dcd9fcc69219cea5f3a7ebfd4b5a01a105d..88384a02a7b5637ee28ba283e2ec9ccb08b0dcc2 100644 --- a/ee/spec/models/remote_development/workspace_spec.rb +++ b/ee/spec/models/remote_development/workspace_spec.rb @@ -2,14 +2,19 @@ require 'spec_helper' +# noinspection RubyResolve - https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/tracked-jetbrains-issues/#ruby-31542 RSpec.describe RemoteDevelopment::Workspace, feature_category: :remote_development do let_it_be(:user) { create(:user) } let_it_be(:agent) { create(:ee_cluster_agent, :with_remote_development_agent_config) } let_it_be(:project) { create(:project, :in_group) } let_it_be(:personal_access_token) { create(:personal_access_token, user: user) } + let(:desired_state) { ::RemoteDevelopment::Workspaces::States::STOPPED } + subject do - create(:workspace, user: user, agent: agent, project: project, personal_access_token: personal_access_token) + create(:workspace, + user: user, agent: agent, project: project, + personal_access_token: personal_access_token, desired_state: desired_state) end describe 'associations' do @@ -66,7 +71,6 @@ it 'sets desired_state_updated_at' do subject.save! - # noinspection RubyResolve - https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/tracked-jetbrains-issues/#ruby-31542 expect(subject.desired_state_updated_at).to eq(Time.current) end end @@ -74,7 +78,6 @@ describe 'when updating desired_state' do it 'sets desired_state_updated_at' do expect { subject.update!(desired_state: ::RemoteDevelopment::Workspaces::States::RUNNING) }.to change { - # noinspection RubyResolve - https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/tracked-jetbrains-issues/#ruby-31542 subject.desired_state_updated_at } end @@ -83,7 +86,6 @@ describe 'when updating a field other than desired_state' do it 'does not set desired_state_updated_at' do expect { subject.update!(actual_state: ::RemoteDevelopment::Workspaces::States::RUNNING) }.not_to change { - # noinspection RubyResolve - https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/tracked-jetbrains-issues/#ruby-31542 subject.desired_state_updated_at } end @@ -91,7 +93,6 @@ end describe 'validations' do - # noinspection RubyResolve - https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/tracked-jetbrains-issues/#ruby-31542 it 'validates max_hours_before_termination is no more than 120' do subject.max_hours_before_termination = described_class::MAX_HOURS_BEFORE_TERMINATION_LIMIT expect(subject).to be_valid @@ -120,6 +121,20 @@ expect(subject.errors[:agent]).to include('for Workspace must have an associated RemoteDevelopmentAgentConfig') end end + + context 'when desired_state is Terminated' do + let(:desired_state) { ::RemoteDevelopment::Workspaces::States::TERMINATED } + + before do + subject.desired_state = ::RemoteDevelopment::Workspaces::States::STOPPED + end + + it 'prevents changes to desired_state' do + expect(subject).not_to be_valid + expect(subject.errors[:desired_state]) + .to include("is 'Terminated', and cannot be updated. Create a new workspace instead.") + end + end end describe 'scopes' do