diff --git a/ee/lib/remote_development/messages.rb b/ee/lib/remote_development/messages.rb index e632252dde6fbf07d3df74c3f42b1a17e7d8916f..b04c05ffd03b13912137709d1525334a3fc4b7ff 100644 --- a/ee/lib/remote_development/messages.rb +++ b/ee/lib/remote_development/messages.rb @@ -36,6 +36,7 @@ module Messages # Settings errors SettingsEnvironmentVariableReadFailed = Class.new(Message) SettingsCurrentSettingsReadFailed = Class.new(Message) + SettingsVscodeExtensionsGalleryValidationFailed = Class.new(Message) #--------------------------------------------------------- # Domain Events - message name should describe the outcome diff --git a/ee/lib/remote_development/settings/defaults_initializer.rb b/ee/lib/remote_development/settings/defaults_initializer.rb index 3c5b8f5c5cbfc357f43b29bb421eb0e4583242ec..9d4397bc6cbeb43892389ae0a28df7f9f62e047a 100644 --- a/ee/lib/remote_development/settings/defaults_initializer.rb +++ b/ee/lib/remote_development/settings/defaults_initializer.rb @@ -19,7 +19,17 @@ def self.default_settings default_max_hours_before_termination: [24, Integer], max_hours_before_termination_limit: [120, Integer], 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] + tools_injector_image: [ + "registry.gitlab.com/gitlab-org/gitlab-web-ide-vscode-fork/web-ide-injector:9", String + ], + vscode_extensions_gallery: [ + { + service_url: "https://open-vsx.org/vscode/gallery", + item_url: "https://open-vsx.org/vscode/item", + resource_url_template: "https://open-vsx.org/api/{publisher}/{name}/{version}/file/{path}" + }, + Hash + ] } end diff --git a/ee/lib/remote_development/settings/env_var_reader.rb b/ee/lib/remote_development/settings/env_var_reader.rb index 09cc05dfd9adae908527b0e479a91ea5572864b4..66b9df43529032a73f1a016e4d2f01ad4355623f 100644 --- a/ee/lib/remote_development/settings/env_var_reader.rb +++ b/ee/lib/remote_development/settings/env_var_reader.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require 'oj' + module RemoteDevelopment module Settings class EnvVarReader @@ -53,10 +55,39 @@ def self.cast_value(env_var_name:, env_var_value_string:, setting_type:) end env_var_value_string.to_i + elsif setting_type == Hash + # NOTE: A Hash type is expected to be represented in an ENV var as a valid JSON string + parsed_value = parse_json(env_var_name: env_var_name, value: env_var_value_string) + + unless parsed_value.is_a?(Hash) + raise "ENV var '#{env_var_name}' was a JSON array type, but it should be an object type" + end + + parsed_value + elsif setting_type == Array + # NOTE: An Array type is expected to be represented in an ENV var as a valid JSON string + parsed_value = parse_json(env_var_name: env_var_name, value: env_var_value_string) + + unless parsed_value.is_a?(Array) + raise "ENV var '#{env_var_name}' was a JSON object type, but it should be an array type" + end + + parsed_value else raise "Unsupported Remote Development setting type: #{setting_type}" end end + + # @param [String] env_var_name + # @param [String] value + # @return [Object, Array] + # @raise [EncodingError] + def self.parse_json(env_var_name:, value:) + # noinspection InvalidCallToProtectedPrivateMethod - See https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/code-inspection/why-are-there-noinspection-comments/ + Oj.load(value, mode: :rails, symbol_keys: true) + rescue EncodingError => e + raise "ENV var '#{env_var_name}' value was not valid parseable JSON. Parse error was: '#{e.message}'" + end end end end diff --git a/ee/lib/remote_development/settings/extensions_gallery_validator.rb b/ee/lib/remote_development/settings/extensions_gallery_validator.rb new file mode 100644 index 0000000000000000000000000000000000000000..aa44de52ec61ef239fb8b8e17cb9aae9ff4b0106 --- /dev/null +++ b/ee/lib/remote_development/settings/extensions_gallery_validator.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +module RemoteDevelopment + module Settings + class ExtensionsGalleryValidator + include Messages + + # @param [Hash] value + # @return [Result] + def self.validate(value) + value => { settings: Hash => settings } + settings => { vscode_extensions_gallery: Hash => vscode_extensions_gallery } + + # NOTE: We deep_stringify_keys here, so we can still pass keys as symbols during tests. + # This is the only place where keys need to be strings, because of the JSON schema + # validation, all other places we convert and work with the keys as symbols. + errors = validate_against_schema(vscode_extensions_gallery.deep_stringify_keys) + + if errors.none? + Result.ok(value) + else + Result.err(SettingsVscodeExtensionsGalleryValidationFailed.new(details: errors.join(". "))) + end + end + + # @param [Hash] json_to_validate + # @return [Array] + def self.validate_against_schema(json_to_validate) + schema = { + "required" => + %w[ + service_url + item_url + resource_url_template + ], + "properties" => { + "service_url" => { + "type" => "string" + }, + "item_url" => { + "type" => "string" + }, + "resource_url_template" => { + "type" => "string" + } + } + } + + schemer = JSONSchemer.schema(schema) + errors = schemer.validate(json_to_validate) + errors.map { |error| JSONSchemer::Errors.pretty(error) } + end + end + end +end diff --git a/ee/lib/remote_development/settings/main.rb b/ee/lib/remote_development/settings/main.rb index 3c7b99bdf30077b34e2a75349fc97001bc3a560b..d0aa0734a5c65f5de6ed8ca733119e5c586646e0 100644 --- a/ee/lib/remote_development/settings/main.rb +++ b/ee/lib/remote_development/settings/main.rb @@ -22,6 +22,7 @@ def self.get_settings # NOTE: EnvVarReader is kept as last step, so it can always be used to easily override any settings for # local or temporary testing. .and_then(EnvVarReader.method(:read)) + .and_then(RemoteDevelopment::Settings::ExtensionsGalleryValidator.method(:validate)) .map( # As the final step, return the settings in a SettingsGetSuccessful message ->(value) do @@ -36,6 +37,8 @@ def self.get_settings generate_error_response_from_message(message: message, reason: :internal_server_error) in { err: SettingsCurrentSettingsReadFailed => message } generate_error_response_from_message(message: message, reason: :internal_server_error) + in { err: SettingsVscodeExtensionsGalleryValidationFailed => message } + generate_error_response_from_message(message: message, reason: :internal_server_error) in { ok: SettingsGetSuccessful => message } { settings: message.context.fetch(:settings), status: :success } else diff --git a/ee/spec/lib/remote_development/settings/env_var_reader_spec.rb b/ee/spec/lib/remote_development/settings/env_var_reader_spec.rb index 990f86e660160d68682b2744dc313c4495e07865..392fb5408a319252330510198278a4898411971a 100644 --- a/ee/spec/lib/remote_development/settings/env_var_reader_spec.rb +++ b/ee/spec/lib/remote_development/settings/env_var_reader_spec.rb @@ -6,14 +6,16 @@ include ResultMatchers let(:default_setting_value) { 42 } + let(:setting_name) { 'the_setting' } let(:setting_type) { Integer } + let(:env_var_name) { "GITLAB_REMOTE_DEVELOPMENT_#{setting_name.upcase}" } let(:value) do { settings: { - the_setting: default_setting_value + "#{setting_name}": default_setting_value }, setting_types: { - the_setting: setting_type + "#{setting_name}": setting_type } } end @@ -28,8 +30,6 @@ end context "when an ENV var overrides a default setting" do - let(:env_var_name) { "GITLAB_REMOTE_DEVELOPMENT_THE_SETTING" } - context "when setting_type is String" do let(:env_var_value) { "a string" } let(:setting_type) { String } @@ -57,23 +57,37 @@ )) end end - end - context "when no ENV var overrides a default setting" do - let(:env_var_name) { "GITLAB_REMOTE_DEVELOPMENT_NON_MATCHING_SETTING" } - let(:env_var_value) { "0" } + context "when setting_type is Hash" do + let(:env_var_value) { '{"a": 1}' } + let(:setting_type) { Hash } - it "uses the default setting value which was passed" do - expect(result).to eq(Result.ok( - { - settings: { the_setting: default_setting_value }, - setting_types: { the_setting: setting_type } - } - )) + it "uses the casted type of the overridden ENV var value" do + expect(result).to eq(Result.ok( + { + settings: { the_setting: { a: 1 } }, + setting_types: { the_setting: setting_type } + } + )) + end + end + + context "when setting_type is Array" do + let(:env_var_value) { '["a", 1]' } + let(:setting_type) { Array } + + it "uses the casted type of the overridden ENV var value" do + expect(result).to eq(Result.ok( + { + settings: { the_setting: ["a", 1] }, + setting_types: { the_setting: setting_type } + } + )) + end end end - context "when an ENV matches the pattern but there is no default setting value defined" do + context "when an ENV matches the pattern but there is no matching default setting value defined" do let(:env_var_name) { "GITLAB_REMOTE_DEVELOPMENT_NONEXISTENT_SETTING" } let(:env_var_value) { "maybe some old deprecated setting, doesn't matter, it's ignored" } @@ -88,20 +102,84 @@ end context "when ENV var contains an incorrect type" do - let(:env_var_name) { "GITLAB_REMOTE_DEVELOPMENT_THE_SETTING" } - let(:env_var_value) { "not an Integer type" } - - it "returns an err Result containing a settings environment variable read failed message with details" do - expect(result).to be_err_result( - RemoteDevelopment::Messages::SettingsEnvironmentVariableReadFailed.new( - details: "ENV var '#{env_var_name}' value could not be cast to #{setting_type} type." + context "for Integer type setting" do + let(:env_var_value) { "not an Integer type" } + + it "returns an err Result containing a settings environment variable read failed message with details" do + expect(result).to be_err_result( + RemoteDevelopment::Messages::SettingsEnvironmentVariableReadFailed.new( + details: "ENV var '#{env_var_name}' value could not be cast to #{setting_type} type." + ) ) - ) + end + end + + context "for Hash type setting" do + let(:setting_type) { Hash } + + context "with invalid JSON" do + let(:env_var_value) { "not a JSON string that can be converted to a Hash" } + + it "returns an err Result containing a settings environment variable read failed message with details" do + expect(result).to be_err_result do |message| + expect(message).to be_a(RemoteDevelopment::Messages::SettingsEnvironmentVariableReadFailed) + message.context => { details: String => details } + expect(details).to match( + /ENV var '#{env_var_name}'.*not valid parseable JSON. Parse error was: 'not a number.*line 1, column 1.*'/ + ) + end + end + end + + context "with JSON that is an Array" do + let(:env_var_value) { "[1]" } + + it "returns an err Result containing a settings environment variable read failed message with details" do + expect(result).to be_err_result do |message| + expect(message).to be_a(RemoteDevelopment::Messages::SettingsEnvironmentVariableReadFailed) + message.context => { details: String => details } + expect(details).to match( + /ENV var '#{env_var_name}' was a JSON array type, but it should be an object type/ + ) + end + end + end + end + + context "for Array type setting" do + let(:setting_type) { Array } + + context "with invalid JSON" do + let(:env_var_value) { "not a JSON string that can be converted to an Array" } + + it "returns an err Result containing a settings environment variable read failed message with details" do + expect(result).to be_err_result do |message| + expect(message).to be_a(RemoteDevelopment::Messages::SettingsEnvironmentVariableReadFailed) + message.context => { details: String => details } + expect(details).to match( + /ENV var '#{env_var_name}'.*not valid parseable JSON. Parse error was: 'not a number.*line 1, column 1.*'/ + ) + end + end + + context "with JSON that is a Hash" do + let(:env_var_value) { '{"a": 1}' } + + it "returns an err Result containing a settings environment variable read failed message with details" do + expect(result).to be_err_result do |message| + expect(message).to be_a(RemoteDevelopment::Messages::SettingsEnvironmentVariableReadFailed) + message.context => { details: String => details } + expect(details).to match( + /ENV var '#{env_var_name}' was a JSON object type, but it should be an array type/ + ) + end + end + end + end end end context "when setting_type is an unsupported type" do - let(:env_var_name) { "GITLAB_REMOTE_DEVELOPMENT_THE_SETTING" } let(:env_var_value) { "42" } let(:setting_type) { Float } diff --git a/ee/spec/lib/remote_development/settings/extensions_gallery_validator_spec.rb b/ee/spec/lib/remote_development/settings/extensions_gallery_validator_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..25eea0572e2b48e68b03c5daba3e5dbfa53c53fa --- /dev/null +++ b/ee/spec/lib/remote_development/settings/extensions_gallery_validator_spec.rb @@ -0,0 +1,89 @@ +# frozen_string_literal: true + +require_relative "../rd_fast_spec_helper" + +RSpec.describe RemoteDevelopment::Settings::ExtensionsGalleryValidator, :rd_fast, feature_category: :remote_development do + include ResultMatchers + + let(:service_url) { "https://open-vsx.org/vscode/gallery" } + let(:item_url) { "https://open-vsx.org/vscode/item" } + let(:resource_url_template) { "https://open-vsx.org/api/{publisher}/{name}/{version}/file/{path}" } + let(:vscode_extensions_gallery) do + { + service_url: service_url, + item_url: item_url, + resource_url_template: resource_url_template + } + end + + let(:value) do + { + settings: { + vscode_extensions_gallery: vscode_extensions_gallery + } + } + end + + subject(:result) do + described_class.validate(value) + end + + context "when vscode_extensions_gallery is valid" do + shared_examples "success result" do + it "return an ok Result containing the original value which was passed" do + expect(result).to eq(Result.ok(value)) + end + end + + context "when all settings are present" do + it_behaves_like 'success result' + end + end + + context "when vscode_extensions_gallery is invalid" do + shared_examples "err result" do |expected_error_details:| + it "returns an err Result containing error details" do + expect(result).to be_err_result do |message| + expect(message).to be_a(RemoteDevelopment::Messages::SettingsVscodeExtensionsGalleryValidationFailed) + message.context => { details: String => error_details } + expect(error_details).to eq(expected_error_details) + end + end + end + + context "when missing required entries" do + let(:vscode_extensions_gallery) { {} } + + it_behaves_like "err result", expected_error_details: + "root is missing required keys: service_url, item_url, " \ + "resource_url_template" # rubocop:disable Layout/LineEndStringConcatenationIndentation -- This is being changed in https://gitlab.com/gitlab-org/ruby/gems/gitlab-styles/-/merge_requests/212 + end + + context "for service_url" do + context "when not a string" do + let(:service_url) { { not_a_string: true } } + + it_behaves_like "err result", expected_error_details: + "property '/service_url' is not of type: string" + end + end + + context "for item_url" do + context "when not a string" do + let(:item_url) { { not_a_string: true } } + + it_behaves_like "err result", expected_error_details: + "property '/item_url' is not of type: string" + end + end + + context "for resource_url_template" do + context "when not a string" do + let(:resource_url_template) { { not_a_string: true } } + + it_behaves_like "err result", expected_error_details: + "property '/resource_url_template' is not of type: string" + end + end + end +end diff --git a/ee/spec/lib/remote_development/settings/main_spec.rb b/ee/spec/lib/remote_development/settings/main_spec.rb index cb4f4c2d9975269d19c9e960ebd3ab62d9037fa6..b38c56466724c98156c8acd6d62b0e64fe0c0e43 100644 --- a/ee/spec/lib/remote_development/settings/main_spec.rb +++ b/ee/spec/lib/remote_development/settings/main_spec.rb @@ -5,7 +5,8 @@ RSpec.describe RemoteDevelopment::Settings::Main, :rd_fast, feature_category: :remote_development do include RemoteDevelopment::RailwayOrientedProgrammingHelpers - let(:value) { {} } + let(:settings) { { some_setting: 42 } } + let(:value) { { settings: settings } } let(:error_details) { 'some error details' } let(:err_message_context) { { details: error_details } } @@ -14,26 +15,48 @@ let(:defaults_initializer_class) { RemoteDevelopment::Settings::DefaultsInitializer } let(:current_settings_reader_class) { RemoteDevelopment::Settings::CurrentSettingsReader } let(:env_var_reader_class) { RemoteDevelopment::Settings::EnvVarReader } + let(:extensions_gallery_validator_class) { RemoteDevelopment::Settings::ExtensionsGalleryValidator } # Methods let(:defaults_initializer_method) { defaults_initializer_class.singleton_method(:init) } let(:current_settings_reader_method) { current_settings_reader_class.singleton_method(:read) } let(:env_var_reader_method) { env_var_reader_class.singleton_method(:read) } + let(:extensions_gallery_validator_method) do + extensions_gallery_validator_class.singleton_method(:validate) + end # Subject subject(:response) { described_class.get_settings } before do - allow(defaults_initializer_class).to receive(:method) { defaults_initializer_method } - allow(current_settings_reader_class).to(receive(:method)) { current_settings_reader_method } - allow(env_var_reader_class).to(receive(:method)) { env_var_reader_method } + allow(defaults_initializer_class).to receive(:method).with(:init) { defaults_initializer_method } + allow(current_settings_reader_class).to receive(:method).with(:read) { current_settings_reader_method } + allow(env_var_reader_class).to receive(:method).with(:read) { env_var_reader_method } + allow(extensions_gallery_validator_class).to receive(:method).with(:validate) do + extensions_gallery_validator_method + end + + stub_method_to_modify_and_return_value(defaults_initializer_method, expected_value: {}, returned_value: value) + end + + context 'when all steps are successful' do + before do + stub_methods_to_return_ok_result(current_settings_reader_method, env_var_reader_method, + extensions_gallery_validator_method) + end + + it 'returns a success response with the settings as the payload' do + expect(response).to eq({ + status: :success, + settings: settings + }) + end end context 'when the CurrentSettingsReader returns an err Result' do before do - stub_methods_to_return_value(defaults_initializer_method) stub_methods_to_return_err_result( method: current_settings_reader_method, message_class: RemoteDevelopment::Messages::SettingsCurrentSettingsReadFailed @@ -49,25 +72,8 @@ end end - context 'when the CurrentSettingsReader returns an ok Result' do - before do - stub_methods_to_return_value(defaults_initializer_method) - allow(current_settings_reader_method).to receive(:call).with(value) do - Result.ok({ settings: { int_setting: 1 } }) - end - end - - it 'returns a settings get success response with the settings as the payload' do - expect(response).to eq({ - status: :success, - settings: { int_setting: 1 } - }) - end - end - context 'when the EnvVarReader returns an err Result' do before do - stub_methods_to_return_value(defaults_initializer_method) stub_methods_to_return_ok_result(current_settings_reader_method) stub_methods_to_return_err_result( method: env_var_reader_method, @@ -84,26 +90,26 @@ end end - context 'when the EnvVarReader returns an ok Result' do + context 'when the ExtensionsGalleryValidator returns an err Result' do before do - stub_methods_to_return_value(defaults_initializer_method) - stub_methods_to_return_ok_result(current_settings_reader_method) - allow(env_var_reader_method).to receive(:call).with(value) do - Result.ok({ settings: { int_setting: 1 } }) - end + stub_methods_to_return_ok_result(current_settings_reader_method, env_var_reader_method) + stub_methods_to_return_err_result( + method: extensions_gallery_validator_method, + message_class: RemoteDevelopment::Messages::SettingsVscodeExtensionsGalleryValidationFailed + ) end - it 'returns a settings get success response with the settings as the payload' do + it 'returns an error response' do expect(response).to eq({ - status: :success, - settings: { int_setting: 1 } + status: :error, + message: "Settings VSCode extensions gallery validation failed: #{error_details}", + reason: :internal_server_error }) end end context 'when an invalid Result is returned' do before do - stub_methods_to_return_value(defaults_initializer_method) stub_methods_to_return_ok_result(current_settings_reader_method) stub_methods_to_return_err_result( method: env_var_reader_method, diff --git a/ee/spec/support/helpers/remote_development/railway_oriented_programming_helpers.rb b/ee/spec/support/helpers/remote_development/railway_oriented_programming_helpers.rb index 56a8109b6f3b0549ac87ce03f27f160386bfd2fa..44dc0da1b42564ff42c03f84de44b015782ef14a 100644 --- a/ee/spec/support/helpers/remote_development/railway_oriented_programming_helpers.rb +++ b/ee/spec/support/helpers/remote_development/railway_oriented_programming_helpers.rb @@ -21,5 +21,9 @@ def stub_methods_to_return_value(*methods) allow(method).to receive(:call).with(value) { value } end end + + def stub_method_to_modify_and_return_value(method, expected_value:, returned_value:) + allow(method).to receive(:call).with(expected_value) { returned_value } + end end end