From 704f538092f7b8227d613506d9239f7863dc4723 Mon Sep 17 00:00:00 2001 From: Vishal Tak Date: Wed, 7 May 2025 16:07:33 +0530 Subject: [PATCH 1/2] Add validations for max_resources_per_workspace and shared_namespace --- .../workspaces_agent_config.rb | 17 +++++++++++++ .../example.agent_config.yaml | 2 +- .../create/main_integration_spec.rb | 14 +++++++---- .../workspaces_agent_config_spec.rb | 24 +++++++++++++++++++ locale/gitlab.pot | 3 +++ 5 files changed, 54 insertions(+), 6 deletions(-) diff --git a/ee/app/models/remote_development/workspaces_agent_config.rb b/ee/app/models/remote_development/workspaces_agent_config.rb index d0e373830cfe23..6ce2978afc65b5 100644 --- a/ee/app/models/remote_development/workspaces_agent_config.rb +++ b/ee/app/models/remote_development/workspaces_agent_config.rb @@ -65,6 +65,8 @@ class WorkspacesAgentConfig < ApplicationRecord validate :validate_image_pull_secrets_namespace + validate :validate_max_resources_per_workspace + scope :by_cluster_agent_ids, ->(ids) { where(cluster_agent_id: ids) } private @@ -118,5 +120,20 @@ def validate_image_pull_secrets_namespace ) nil end + + # max_resources_per_workspace must be an empty hash if shared_namespace is specified + # @return [void] + def validate_max_resources_per_workspace + return if shared_namespace.blank? + return if max_resources_per_workspace.empty? + + errors.add( + :max_resources_per_workspace, + format( + _("max_resources_per_workspace must be an empty hash if shared_namespace is specified") + ) + ) + nil + end end end diff --git a/ee/spec/fixtures/remote_development/example.agent_config.yaml b/ee/spec/fixtures/remote_development/example.agent_config.yaml index 795def76a97286..7dc257701efc66 100644 --- a/ee/spec/fixtures/remote_development/example.agent_config.yaml +++ b/ee/spec/fixtures/remote_development/example.agent_config.yaml @@ -54,4 +54,4 @@ remote_development: max_active_hours_before_stop: 60 max_stopped_hours_before_termination: 4332 - shared_namespace: "default" + shared_namespace: "" diff --git a/ee/spec/lib/remote_development/workspace_operations/create/main_integration_spec.rb b/ee/spec/lib/remote_development/workspace_operations/create/main_integration_spec.rb index 6d709e6f1351d4..a14c9a1fb43d50 100644 --- a/ee/spec/lib/remote_development/workspace_operations/create/main_integration_spec.rb +++ b/ee/spec/lib/remote_development/workspace_operations/create/main_integration_spec.rb @@ -124,7 +124,8 @@ expect(workspace.actual_state).to eq(states_module::CREATION_REQUESTED) expect(workspace.actual_state_updated_at).to eq(Time.current) expect(workspace.name).to eq("workspace-#{agent.id}-#{user.id}-#{random_string}") - expect(workspace.namespace).to eq("default") + expect(workspace.namespace) + .to eq("#{create_constants_module::NAMESPACE_PREFIX}-#{agent.id}-#{user.id}-#{random_string}") expect(workspace.workspaces_agent_config_version).to eq(expected_workspaces_agent_config_version) expect(workspace.url).to eq(URI::HTTPS.build({ host: "#{create_constants_module::WORKSPACE_EDITOR_PORT}-#{workspace.name}." \ @@ -165,15 +166,18 @@ end end - context "without shared namespace" do + context "with shared namespace" do before do - agent.unversioned_latest_workspaces_agent_config.update!(shared_namespace: "") + # max_resources_per_workspace must be an empty hash if shared_namespace is specified + agent.unversioned_latest_workspaces_agent_config.update!( + shared_namespace: "default", + max_resources_per_workspace: {} + ) end it 'uses a unique namespace', :aggregate_failures do workspace = response.fetch(:payload).fetch(:workspace) - expect(workspace.namespace) - .to eq("#{create_constants_module::NAMESPACE_PREFIX}-#{agent.id}-#{user.id}-#{random_string}") + expect(workspace.namespace).to eq("default") end end end diff --git a/ee/spec/models/remote_development/workspaces_agent_config_spec.rb b/ee/spec/models/remote_development/workspaces_agent_config_spec.rb index 683c1d9356d323..9046096725bd08 100644 --- a/ee/spec/models/remote_development/workspaces_agent_config_spec.rb +++ b/ee/spec/models/remote_development/workspaces_agent_config_spec.rb @@ -205,6 +205,30 @@ end end + context 'when max_resources_per_workspace and shared_namespace are specified' do + using RSpec::Parameterized::TableSyntax + + where(:shared_namespace, :max_resources_per_workspace, :validity, :error_field, :errors) do + # rubocop:disable Layout/LineLength -- The RSpec table syntax often requires long lines for errors + '' | {} | true | :max_resources_per_workspace | [] + '' | { requests: { cpu: "1", memory: "1Gi" }, limits: { cpu: "2", memory: "2Gi" } } | true | :max_resources_per_workspace | [] + 'my-namespace' | {} | true | :max_resources_per_workspace | [] + 'my-namespace' | { requests: { cpu: "1", memory: "1Gi" }, limits: { cpu: "2", memory: "2Gi" } } | false | :max_resources_per_workspace | ["max_resources_per_workspace must be an empty hash if shared_namespace is specified"] + # rubocop:enable Layout/LineLength + end + + with_them do + before do + config.shared_namespace = shared_namespace + config.max_resources_per_workspace = max_resources_per_workspace + config.validate + end + + it { expect(config.valid?).to eq(validity) } + it { expect(config.errors[error_field]).to eq(errors) } + end + end + it 'when network_policy_egress is not specified explicitly' do expect(config).to be_valid expect(config.network_policy_egress).to eq(default_network_policy_egress) diff --git a/locale/gitlab.pot b/locale/gitlab.pot index e3c990c82f13f9..2408c042715420 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -72301,6 +72301,9 @@ msgstr "" msgid "math|Too many expansions. Consider using multiple math blocks." msgstr "" +msgid "max_resources_per_workspace must be an empty hash if shared_namespace is specified" +msgstr "" + msgid "member" msgid_plural "members" msgstr[0] "" -- GitLab From a12645ec32a98f8ec2daf67cdc606887d386c8ff Mon Sep 17 00:00:00 2001 From: Vishal Tak Date: Thu, 8 May 2025 09:22:42 +0530 Subject: [PATCH 2/2] Fix validation nil bug --- ee/app/models/remote_development/workspaces_agent_config.rb | 2 +- .../models/remote_development/workspaces_agent_config_spec.rb | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/ee/app/models/remote_development/workspaces_agent_config.rb b/ee/app/models/remote_development/workspaces_agent_config.rb index 6ce2978afc65b5..982cdafd1b1faf 100644 --- a/ee/app/models/remote_development/workspaces_agent_config.rb +++ b/ee/app/models/remote_development/workspaces_agent_config.rb @@ -125,7 +125,7 @@ def validate_image_pull_secrets_namespace # @return [void] def validate_max_resources_per_workspace return if shared_namespace.blank? - return if max_resources_per_workspace.empty? + return if max_resources_per_workspace.nil? || max_resources_per_workspace.empty? errors.add( :max_resources_per_workspace, diff --git a/ee/spec/models/remote_development/workspaces_agent_config_spec.rb b/ee/spec/models/remote_development/workspaces_agent_config_spec.rb index 9046096725bd08..1a5fe93e7a3f10 100644 --- a/ee/spec/models/remote_development/workspaces_agent_config_spec.rb +++ b/ee/spec/models/remote_development/workspaces_agent_config_spec.rb @@ -212,7 +212,9 @@ # rubocop:disable Layout/LineLength -- The RSpec table syntax often requires long lines for errors '' | {} | true | :max_resources_per_workspace | [] '' | { requests: { cpu: "1", memory: "1Gi" }, limits: { cpu: "2", memory: "2Gi" } } | true | :max_resources_per_workspace | [] + '' | nil | false | :max_resources_per_workspace | ["must be a valid json schema", "must be a hash"] 'my-namespace' | {} | true | :max_resources_per_workspace | [] + 'my-namespace' | nil | false | :max_resources_per_workspace | ["must be a valid json schema", "must be a hash"] 'my-namespace' | { requests: { cpu: "1", memory: "1Gi" }, limits: { cpu: "2", memory: "2Gi" } } | false | :max_resources_per_workspace | ["max_resources_per_workspace must be an empty hash if shared_namespace is specified"] # rubocop:enable Layout/LineLength end -- GitLab