diff --git a/ee/lib/remote_development/README.md b/ee/lib/remote_development/README.md index a1cd1819852f6d69ebd8d7f0e335d6ae610905a9..24af018ff69aca8a6398e1e6e05657fb3b407f26 100644 --- a/ee/lib/remote_development/README.md +++ b/ee/lib/remote_development/README.md @@ -11,7 +11,7 @@ - [Railway Oriented Programming and the Result Class](#railway-oriented-programming-and-the-result-class) - [Benefits](#benefits) - [Differences from standard GitLab patterns](#differences-from-standard-gitlab-patterns) -- [Remote Development Settings](#settings) +- [Remote Development Settings](#remote-development-settings) - [FAQ](#faq) ## TL;DR and Quickstart @@ -421,9 +421,9 @@ order of the steps in the 1. First, the default value is used 1. Next, one of the following values are used if defined and not `nil`: - 1.`::Gitlab::CurrentSettings` - 1.`::Gitlab::Settings` (not yet implemented) - 1. Cascading settings via `CascadingNamespaceSettingAttribute` (not yet implemented) + 1. `::Gitlab::CurrentSettings` + 1. `::Gitlab::Settings` (not yet implemented) + 1. Cascading settings via `CascadingNamespaceSettingAttribute` (not yet implemented) 1. Next, an ENV values is used if defined (i.e. not `nil`). The ENV var is intentionally placed as the last step and highest precedence, so it can always be used to easily override any settings for local or temporary testing. diff --git a/ee/lib/remote_development/settings/defaults_initializer.rb b/ee/lib/remote_development/settings/defaults_initializer.rb index 795d28d3fd2104fe3218123d639868315aa88f36..3c5b8f5c5cbfc357f43b29bb421eb0e4583242ec 100644 --- a/ee/lib/remote_development/settings/defaults_initializer.rb +++ b/ee/lib/remote_development/settings/defaults_initializer.rb @@ -18,7 +18,8 @@ def self.default_settings default_branch_name: [UNDEFINED, String], default_max_hours_before_termination: [24, Integer], max_hours_before_termination_limit: [120, Integer], - project_cloner_image: ['alpine/git:2.36.3', String] + project_cloner_image: ['alpine/git:2.36.3', String], + tools_injector_image: ["registry.gitlab.com/gitlab-org/gitlab-web-ide-vscode-fork/web-ide-injector:9", String] } end diff --git a/ee/lib/remote_development/workspaces/create/post_flatten_devfile_validator.rb b/ee/lib/remote_development/workspaces/create/post_flatten_devfile_validator.rb index 6594b33ce171f968ed096356e77a282fbf769657..fb897b8a9983483d7248f780ecf6247948848971 100644 --- a/ee/lib/remote_development/workspaces/create/post_flatten_devfile_validator.rb +++ b/ee/lib/remote_development/workspaces/create/post_flatten_devfile_validator.rb @@ -57,17 +57,17 @@ def self.validate_components(value) return err(_('No components present in devfile')) if components.blank? - inject_tools_components = components.select do |component| + injected_tools_components = components.select do |component| component.dig('attributes', 'gl/inject-editor') end - return err(_("No component has 'gl/inject-editor' attribute")) if inject_tools_components.empty? + return err(_("No component has 'gl/inject-editor' attribute")) if injected_tools_components.empty? - if inject_tools_components.length > 1 + if injected_tools_components.length > 1 return err( format( _("Multiple components '%{name}' have 'gl/inject-editor' attribute"), - name: inject_tools_components.pluck('name') # rubocop:disable CodeReuse/ActiveRecord -- this pluck isn't from ActiveRecord, it's from ActiveSupport + name: injected_tools_components.pluck('name') # rubocop:disable CodeReuse/ActiveRecord -- this pluck isn't from ActiveRecord, it's from ActiveSupport ) ) end diff --git a/ee/lib/remote_development/workspaces/create/tools_component_injector.rb b/ee/lib/remote_development/workspaces/create/tools_component_injector.rb index d9f3a2b9679e47bbf1f886f737c63ee07579010d..5a2df9d16ad765d256e746d70f30a02ef0865c5e 100644 --- a/ee/lib/remote_development/workspaces/create/tools_component_injector.rb +++ b/ee/lib/remote_development/workspaces/create/tools_component_injector.rb @@ -12,46 +12,61 @@ def self.inject(value) value => { processed_devfile: Hash => processed_devfile, volume_mounts: Hash => volume_mounts, - params: Hash => params + settings: Hash => settings } volume_mounts => { data_volume: Hash => data_volume } data_volume => { path: String => volume_path } - params => { agent: Clusters::Agent => agent } + settings => { tools_injector_image: String => image_from_settings } editor_port = WorkspaceCreator::WORKSPACE_PORT ssh_port = 60022 tools_dir = "#{volume_path}/.gl-tools" - enable_marketplace = Feature.enabled?( - :allow_extensions_marketplace_in_workspace, - agent.project.root_namespace, - type: :beta - ) + enable_marketplace = allow_extensions_marketplace_in_workspace_feature_enabled?(value: value) + # NOTE: We will always have exactly one tools_component found, because we have already + # validated this in post_flatten_devfile_validator.rb tools_component = processed_devfile['components'].find { |c| c.dig('attributes', 'gl/inject-editor') } - use_vscode_1_81_attribute = tools_component.fetch('attributes', {}).fetch('gl/use-vscode-1-81', false) - use_vscode_1_81 = [true, "true"].include? use_vscode_1_81_attribute - inject_tools_component(processed_devfile, tools_dir, use_vscode_1_81) - - if tools_component - override_main_container( - tools_component, - tools_dir, - editor_port, - ssh_port, - enable_marketplace - ) - end + + # TODO: https://gitlab.com/gitlab-org/gitlab/-/issues/409775 - choose image based on which editor is passed. + image = vscode_1_81_image_override(tools_component: tools_component) || image_from_settings + + inject_tools_components(processed_devfile: processed_devfile, tools_dir: tools_dir, image: image) + + override_main_container( + tools_component: tools_component, + tools_dir: tools_dir, + editor_port: editor_port, + ssh_port: ssh_port, + enable_marketplace: enable_marketplace + ) value end - # @param [Hash] component + # @param [Hash] value + # @return [Boolean] + def self.allow_extensions_marketplace_in_workspace_feature_enabled?(value:) + Feature.enabled?( + :allow_extensions_marketplace_in_workspace, + value.fetch(:params).fetch(:agent).project.root_namespace, + type: :beta + ) + end + + # @param [Hash] tools_component + # @return [String | false] + def self.vscode_1_81_image_override(tools_component:) + override_image = tools_component.fetch("attributes", {})["gl/use-vscode-1-81"].to_s == "true" + "registry.gitlab.com/gitlab-org/gitlab-web-ide-vscode-fork/web-ide-injector:7" if override_image + end + + # @param [Hash] tools_component # @param [String] tools_dir # @param [Integer] editor_port # @param [Integer] ssh_port # @param [Boolean] enable_marketplace - # @return [Hash] - def self.override_main_container(component, tools_dir, editor_port, ssh_port, enable_marketplace) + # @return [void] + def self.override_main_container(tools_component:, tools_dir:, editor_port:, ssh_port:, enable_marketplace:) # This overrides the main container's command # Open issue to support both starting the editor and running the default command: # https://gitlab.com/gitlab-org/gitlab/-/issues/392853 @@ -65,10 +80,10 @@ def self.override_main_container(component, tools_dir, editor_port, ssh_port, en fi ${GL_TOOLS_DIR}/init_tools.sh SH - component['container']['command'] = %w[/bin/sh -c] - component['container']['args'] = [container_args] - component['container']['env'] = [] if component['container']['env'].nil? - component['container']['env'] += [ + tools_component['container']['command'] = %w[/bin/sh -c] + tools_component['container']['args'] = [container_args] + tools_component['container']['env'] = [] if tools_component['container']['env'].nil? + tools_component['container']['env'] += [ { 'name' => 'GL_TOOLS_DIR', 'value' => tools_dir @@ -91,8 +106,8 @@ def self.override_main_container(component, tools_dir, editor_port, ssh_port, en } ] - component['container']['endpoints'] = [] if component['container']['endpoints'].nil? - component['container']['endpoints'].append( + tools_component['container']['endpoints'] = [] if tools_component['container']['endpoints'].nil? + tools_component['container']['endpoints'].append( { 'name' => 'editor-server', 'targetPort' => editor_port, @@ -107,15 +122,14 @@ def self.override_main_container(component, tools_dir, editor_port, ssh_port, en 'secure' => true } ) - component end # @param [Hash] processed_devfile # @param [String] tools_dir - # @param [Boolean] use_vscode_1_81 + # @param [String] image # @return [Array] - def self.inject_tools_component(processed_devfile, tools_dir, use_vscode_1_81) - processed_devfile['components'] += tools_components(tools_dir, use_vscode_1_81) + def self.inject_tools_components(processed_devfile:, tools_dir:, image:) + processed_devfile['components'] += tools_components(tools_dir: tools_dir, image: image) processed_devfile['commands'] = [] if processed_devfile['commands'].nil? processed_devfile['commands'] += [{ @@ -131,18 +145,14 @@ def self.inject_tools_component(processed_devfile, tools_dir, use_vscode_1_81) end # @param [String] tools_dir - # @param [Boolean] use_vscode_1_81 + # @param [String] image # @return [Array] - def self.tools_components(tools_dir, use_vscode_1_81) - # TODO: https://gitlab.com/gitlab-org/gitlab/-/issues/409775 - choose image based on which editor is passed. - image_name = 'registry.gitlab.com/gitlab-org/gitlab-web-ide-vscode-fork/web-ide-injector' - image_tag = use_vscode_1_81 ? '7' : '9' - + def self.tools_components(tools_dir:, image:) [ { 'name' => 'gl-tools-injector', 'container' => { - 'image' => "#{image_name}:#{image_tag}", + 'image' => image, 'env' => [ { 'name' => 'GL_TOOLS_DIR', diff --git a/ee/spec/lib/remote_development/message_spec.rb b/ee/spec/lib/remote_development/message_spec.rb index 20df13aaff0c2fc2b21bb899e99f5d2a51745a92..c6c14624331a6ea350dd2b37ad3898314c905f0e 100644 --- a/ee/spec/lib/remote_development/message_spec.rb +++ b/ee/spec/lib/remote_development/message_spec.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -require 'fast_spec_helper' +require_relative 'fast_spec_helper' RSpec.describe RemoteDevelopment::Message, feature_category: :remote_development do describe '#==' do diff --git a/ee/spec/lib/remote_development/message_support_spec.rb b/ee/spec/lib/remote_development/message_support_spec.rb index 0c2fd8003e00507e561ae26b737ea9c6b4398de8..97878375aa2f262dbda14d2a570c64039b5512bf 100644 --- a/ee/spec/lib/remote_development/message_support_spec.rb +++ b/ee/spec/lib/remote_development/message_support_spec.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -require 'fast_spec_helper' +require_relative 'fast_spec_helper' RSpec.describe RemoteDevelopment::MessageSupport, feature_category: :remote_development do let(:object) { Object.new.extend(described_class) } diff --git a/ee/spec/lib/remote_development/unmatched_result_error_spec.rb b/ee/spec/lib/remote_development/unmatched_result_error_spec.rb index 7f0b40b47737df0990f0e7d2ba630ddc834b7c26..a608c4f3e37a26c7b4557f49fb66242727c35ff7 100644 --- a/ee/spec/lib/remote_development/unmatched_result_error_spec.rb +++ b/ee/spec/lib/remote_development/unmatched_result_error_spec.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -require 'fast_spec_helper' +require_relative 'fast_spec_helper' RSpec.describe RemoteDevelopment::UnmatchedResultError, feature_category: :remote_development do let(:unmatched_message_class) { stub_const('UnmatchedMessage', Class.new(RemoteDevelopment::Message)) } diff --git a/ee/spec/lib/remote_development/workspaces/create/main_integration_spec.rb b/ee/spec/lib/remote_development/workspaces/create/main_integration_spec.rb index 41ee4ad9e499a657c9ed84e0a912fcf096dacbb4..7fd325d8c5ded77deda6c285793da2bb5470cde1 100644 --- a/ee/spec/lib/remote_development/workspaces/create/main_integration_spec.rb +++ b/ee/spec/lib/remote_development/workspaces/create/main_integration_spec.rb @@ -44,9 +44,14 @@ } end + let(:tools_injector_image_from_settings) do + "registry.gitlab.com/gitlab-org/gitlab-web-ide-vscode-fork/web-ide-injector:9" + end + let(:settings) do { - project_cloner_image: 'alpine/git:2.36.3' + project_cloner_image: 'alpine/git:2.36.3', + tools_injector_image: tools_injector_image_from_settings } end @@ -152,4 +157,21 @@ end end end + + context "when allow_extensions_marketplace_in_workspace feature flag is disabled" do + let(:tools_injector_image_from_settings) { 'my/awesome/image:42' } + + before do + stub_feature_flags(allow_extensions_marketplace_in_workspace: false) + end + + it 'uses image override' do + workspace = response.fetch(:payload).fetch(:workspace) + processed_devfile = YAML.safe_load(workspace.processed_devfile).to_h + image_from_processed_devfile = processed_devfile["components"] + &.find { |component| component['name'] == 'gl-tools-injector' } + &.dig('container', 'image') + expect(image_from_processed_devfile).to eq(tools_injector_image_from_settings) + end + end end diff --git a/ee/spec/lib/remote_development/workspaces/create/tools_component_injector_spec.rb b/ee/spec/lib/remote_development/workspaces/create/tools_component_injector_spec.rb index 4db4347218636af54f41a0d35001154c6056390c..dd8cc01cedbd5d3487ffbd95ca2509409ae85958 100644 --- a/ee/spec/lib/remote_development/workspaces/create/tools_component_injector_spec.rb +++ b/ee/spec/lib/remote_development/workspaces/create/tools_component_injector_spec.rb @@ -1,26 +1,33 @@ # frozen_string_literal: true -require "spec_helper" +require_relative '../../fast_spec_helper' RSpec.describe RemoteDevelopment::Workspaces::Create::ToolsComponentInjector, feature_category: :remote_development do include_context 'with remote development shared fixtures' - let(:agent) { create(:ee_cluster_agent, :with_remote_development_agent_config) } let(:input_processed_devfile_name) { 'example.flattened-devfile.yaml' } let(:input_processed_devfile) { YAML.safe_load(read_devfile(input_processed_devfile_name)).to_h } let(:expected_processed_devfile_name) { 'example.tools-injected-devfile.yaml' } let(:expected_processed_devfile) { YAML.safe_load(read_devfile(expected_processed_devfile_name)).to_h } + let(:tools_injector_image_from_settings) do + "registry.gitlab.com/gitlab-org/gitlab-web-ide-vscode-fork/web-ide-injector:9" + end + + let(:settings) do + { + tools_injector_image: tools_injector_image_from_settings + } + end + let(:value) do { - params: { - agent: agent - }, processed_devfile: input_processed_devfile, volume_mounts: { data_volume: { path: "/projects" } - } + }, + settings: settings } end @@ -28,15 +35,30 @@ described_class.inject(value) end + before do + allow(described_class) + .to receive(:allow_extensions_marketplace_in_workspace_feature_enabled?).and_return(true) + end + it 'injects the tools injector component' do expect(returned_value[:processed_devfile]).to eq(expected_processed_devfile) end + context 'when image is overridden in settings' do + let(:tools_injector_image_from_settings) { 'my/awesome/image:42' } + + it 'uses image override' do + image_from_processed_devfile = returned_value[:processed_devfile]["components"][2]["container"]["image"] + expect(image_from_processed_devfile).to eq(tools_injector_image_from_settings) + end + end + context "when allow_extensions_marketplace_in_workspace is disabled" do let(:expected_processed_devfile_name) { 'example.tools-injected-marketplace-disabled-devfile.yaml' } before do - stub_feature_flags(allow_extensions_marketplace_in_workspace: false) + allow(described_class) + .to receive(:allow_extensions_marketplace_in_workspace_feature_enabled?).and_return(false) end it 'injects the tools injector component' do diff --git a/ee/spec/lib/remote_development/workspaces/create/volume_definer_spec.rb b/ee/spec/lib/remote_development/workspaces/create/volume_definer_spec.rb index 89e5871e3a284c49a4598ad0c28fdc49da460dd2..53d8c9c165d232308b0464646e95c97e63140fde 100644 --- a/ee/spec/lib/remote_development/workspaces/create/volume_definer_spec.rb +++ b/ee/spec/lib/remote_development/workspaces/create/volume_definer_spec.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -require "fast_spec_helper" +require_relative '../../fast_spec_helper' RSpec.describe RemoteDevelopment::Workspaces::Create::VolumeDefiner, feature_category: :remote_development do let(:value) { { params: 1 } } diff --git a/ee/spec/lib/remote_development/workspaces/states_spec.rb b/ee/spec/lib/remote_development/workspaces/states_spec.rb index ab9307bf07dec59f2a5725be6862e42c96d95f39..9328a2483af6bb0880fdb6143defbafb5ce7d2a6 100644 --- a/ee/spec/lib/remote_development/workspaces/states_spec.rb +++ b/ee/spec/lib/remote_development/workspaces/states_spec.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -require 'fast_spec_helper' +require_relative '../fast_spec_helper' RSpec.describe RemoteDevelopment::Workspaces::States, feature_category: :remote_development do let(:object) { Object.new.extend(described_class) }