diff --git a/ee/app/models/remote_development/workspace_variable.rb b/ee/app/models/remote_development/workspace_variable.rb index 242a77dfece6c8a3d6fd887ef9595587ba6c4628..40e1737a4e53be746319b1da254ad536d84469f8 100644 --- a/ee/app/models/remote_development/workspace_variable.rb +++ b/ee/app/models/remote_development/workspace_variable.rb @@ -4,17 +4,24 @@ module RemoteDevelopment class WorkspaceVariable < ApplicationRecord belongs_to :workspace, class_name: 'RemoteDevelopment::Workspace', inverse_of: :workspace_variables - enum variable_type: { - env_var: 0, - file: 1 - }, _prefix: :variable_type - - validates :variable_type, presence: true - + validates :variable_type, presence: true, inclusion: { + in: [ + RemoteDevelopment::Workspaces::Create::WorkspaceVariables::VARIABLE_TYPE_ENV_VAR, + RemoteDevelopment::Workspaces::Create::WorkspaceVariables::VARIABLE_TYPE_FILE + ] + } + validates :encrypted_value, presence: true validates :key, presence: true, length: { maximum: 255 } + scope :with_variable_type_env_var, -> { + where(variable_type: RemoteDevelopment::Workspaces::Create::WorkspaceVariables::VARIABLE_TYPE_ENV_VAR) + } + scope :with_variable_type_file, -> { + where(variable_type: RemoteDevelopment::Workspaces::Create::WorkspaceVariables::VARIABLE_TYPE_FILE) + } + attr_encrypted :value, mode: :per_attribute_iv, key: Settings.attr_encrypted_db_key_base_32, diff --git a/ee/lib/remote_development/messages.rb b/ee/lib/remote_development/messages.rb index 94e5679622a1910576d043decfe9bfeb185e933e..a0fa680302e04ef986004e82687a5c99da0eb40c 100644 --- a/ee/lib/remote_development/messages.rb +++ b/ee/lib/remote_development/messages.rb @@ -45,5 +45,10 @@ module Messages WorkspaceCreateSuccessful = Class.new(Message) WorkspaceUpdateSuccessful = Class.new(Message) WorkspaceReconcileSuccessful = Class.new(Message) + + # Workspace DB events + PersonalAccessTokenModelCreateFailed = Class.new(Message) + WorkspaceModelCreateFailed = Class.new(Message) + WorkspaceVariablesModelCreateFailed = Class.new(Message) end end diff --git a/ee/lib/remote_development/workspaces/create/creator.rb b/ee/lib/remote_development/workspaces/create/creator.rb index 308026822ce212c21a328d9d0578ad48b5f8d679..8fb66614960c4bb1cea997f3daa1710b586c5615 100644 --- a/ee/lib/remote_development/workspaces/create/creator.rb +++ b/ee/lib/remote_development/workspaces/create/creator.rb @@ -5,70 +5,51 @@ module Workspaces module Create # noinspection RubyResolve - Rubymine isn't detecting ActiveRecord db field properties of workspace class Creator - include States include Messages RANDOM_STRING_LENGTH = 6 - WORKSPACE_PORT = 60001 # @param [Hash] value # @return [Result] def self.create(value) value => { - devfile_yaml: String => devfile_yaml, - processed_devfile: Hash => processed_devfile, - volume_mounts: Hash => volume_mounts, current_user: User => user, params: Hash => params, } - volume_mounts => { data_volume: Hash => data_volume } - data_volume => { - path: String => workspace_root, - } params => { - project: Project => project, - agent: Clusters::Agent => agent, + agent: Clusters::Agent => agent } - - processed_devfile_yaml = YAML.dump(processed_devfile.deep_stringify_keys) - - workspace = project.workspaces.build(params) - - workspace.devfile = devfile_yaml - workspace.processed_devfile = processed_devfile_yaml - workspace.actual_state = CREATION_REQUESTED - random_string = SecureRandom.alphanumeric(RANDOM_STRING_LENGTH).downcase - workspace.namespace = "gl-rd-ns-#{agent.id}-#{user.id}-#{random_string}" # TODO: https://gitlab.com/gitlab-org/gitlab/-/issues/409774 # We can come maybe come up with a better/cooler way to get a unique name, for now this works - workspace.name = "workspace-#{agent.id}-#{user.id}-#{random_string}" - - project_dir = "#{workspace_root}/#{project.path}" - workspace.url = URI::HTTPS.build({ - host: workspace_host(workspace: workspace), - query: { - folder: project_dir - }.to_query - }).to_s - - if workspace.save - Result.ok( - WorkspaceCreateSuccessful.new( - value.merge({ - workspace: workspace - }) - ) - ) - else - Result.err(WorkspaceCreateFailed.new({ errors: workspace.errors })) + value[:workspace_name] = "workspace-#{agent.id}-#{user.id}-#{random_string}" + value[:workspace_namespace] = "gl-rd-ns-#{agent.id}-#{user.id}-#{random_string}" + model_errors = nil + + updated_value = ApplicationRecord.transaction do + initial_result = Result.ok(value) + + result = + initial_result + .and_then(PersonalAccessTokenCreator.method(:create)) + .and_then(WorkspaceCreator.method(:create)) + .and_then(WorkspaceVariablesCreator.method(:create)) + + case result + in { err: PersonalAccessTokenModelCreateFailed | + WorkspaceModelCreateFailed | + WorkspaceVariablesModelCreateFailed => message + } + model_errors = message.context[:errors] + raise ActiveRecord::Rollback + else + result.unwrap + end end - end - # @param [Workspace] workspace - # @return [String (frozen)] - def self.workspace_host(workspace:) - "#{WORKSPACE_PORT}-#{workspace.name}.#{workspace.dns_zone}" + return Result.err(WorkspaceCreateFailed.new({ errors: model_errors })) if model_errors.present? + + Result.ok(WorkspaceCreateSuccessful.new(updated_value)) end end end diff --git a/ee/lib/remote_development/workspaces/create/personal_access_token_creator.rb b/ee/lib/remote_development/workspaces/create/personal_access_token_creator.rb new file mode 100644 index 0000000000000000000000000000000000000000..4da2760a027f7e07dfd5db794883919452e8d2a1 --- /dev/null +++ b/ee/lib/remote_development/workspaces/create/personal_access_token_creator.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +module RemoteDevelopment + module Workspaces + module Create + class PersonalAccessTokenCreator + include Messages + + # @param [Hash] value + # @return [Result] + def self.create(value) + value => { + current_user: User => user, + workspace_name: String => workspace_name, + params: Hash => params + } + params => { + max_hours_before_termination: Integer => max_hours_before_termination, + } + + # TODO: Use PAT service injection - https://gitlab.com/gitlab-org/gitlab/-/issues/423415 + personal_access_token = user.personal_access_tokens.build( + name: workspace_name, + impersonation: false, + scopes: [:write_repository], + # Since expires_at is a date, we need to set it to the round it off to the next day. + # e.g. If the max_hours_before_termination of the workspace is 1 hour + # and the workspace is created at 2023-08-20 05:30:00, + # then the expires_at of the PAT would be 2023-08-21. + expires_at: max_hours_before_termination.hours.from_now.to_date.next_day + ) + personal_access_token.save + + if personal_access_token.errors.present? + return Result.err( + PersonalAccessTokenModelCreateFailed.new({ errors: personal_access_token.errors }) + ) + end + + Result.ok( + value.merge({ + personal_access_token: personal_access_token + }) + ) + end + end + end + end +end diff --git a/ee/lib/remote_development/workspaces/create/workspace_creator.rb b/ee/lib/remote_development/workspaces/create/workspace_creator.rb new file mode 100644 index 0000000000000000000000000000000000000000..02f059b97e4cb04e854311c49a17320bcf3854bf --- /dev/null +++ b/ee/lib/remote_development/workspaces/create/workspace_creator.rb @@ -0,0 +1,72 @@ +# frozen_string_literal: true + +module RemoteDevelopment + module Workspaces + module Create + class WorkspaceCreator + include States + include Messages + + WORKSPACE_PORT = 60001 + + # @param [Hash] value + # @return [Result] + def self.create(value) + value => { + devfile_yaml: String => devfile_yaml, + processed_devfile: Hash => processed_devfile, + volume_mounts: Hash => volume_mounts, + personal_access_token: PersonalAccessToken => personal_access_token, + workspace_name: String => workspace_name, + workspace_namespace: String => workspace_namespace, + params: Hash => params, + } + volume_mounts => { data_volume: Hash => data_volume } + data_volume => { + path: String => workspace_root, + } + params => { + project: Project => project, + } + project_dir = "#{workspace_root}/#{project.path}" + + workspace = RemoteDevelopment::Workspace.new(params) + workspace.name = workspace_name + workspace.namespace = workspace_namespace + workspace.personal_access_token = personal_access_token + # noinspection RubyResolve - https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/tracked-jetbrains-issues/#ruby-31542 + workspace.devfile = devfile_yaml + # noinspection RubyResolve - https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/tracked-jetbrains-issues/#ruby-31542 + workspace.processed_devfile = YAML.dump(processed_devfile.deep_stringify_keys) + # noinspection RubyResolve - https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/tracked-jetbrains-issues/#ruby-31542 + workspace.actual_state = CREATION_REQUESTED + workspace.url = URI::HTTPS.build({ + host: workspace_host(workspace: workspace), + query: { + folder: project_dir + }.to_query + }).to_s + workspace.save + + if workspace.errors.present? + return Result.err( + WorkspaceModelCreateFailed.new({ errors: workspace.errors }) + ) + end + + Result.ok( + value.merge({ + workspace: workspace + }) + ) + end + + # @param [Workspace] workspace + # @return [String (frozen)] + def self.workspace_host(workspace:) + "#{WORKSPACE_PORT}-#{workspace.name}.#{workspace.dns_zone}" + end + end + end + end +end diff --git a/ee/lib/remote_development/workspaces/create/workspace_variables.rb b/ee/lib/remote_development/workspaces/create/workspace_variables.rb new file mode 100644 index 0000000000000000000000000000000000000000..c45da94004e423400921196b37b0d49b2970d3b5 --- /dev/null +++ b/ee/lib/remote_development/workspaces/create/workspace_variables.rb @@ -0,0 +1,116 @@ +# frozen_string_literal: true + +module RemoteDevelopment + module Workspaces + module Create + class WorkspaceVariables + VARIABLE_TYPE_ENV_VAR = 0 + VARIABLE_TYPE_FILE = 1 + GIT_CREDENTIAL_STORE_SCRIPT = <<~SH.chomp + #!/bin/sh + # This is a readonly store so we can exit cleanly when git attempts a store or erase action + if [ "$1" != "get" ]; + then + exit 0 + fi + + if [ -z "${GL_TOKEN_FILE_PATH}" ]; + then + echo "We could not find the GL_TOKEN_FILE_PATH variable" + exit 1 + fi + password=$(cat ${GL_TOKEN_FILE_PATH}) + + # The username is derived from the "user.email" configuration item. Ensure it is set. + echo "username=does-not-matter" + echo "password=${password}" + exit 0 + SH + + # @param [String] name + # @param [String] dns_zone + # @param [String] personal_access_token_value + # @param [String] user_name + # @param [String] user_email + # @param [Integer] workspace_id + # @return [Array] + def self.variables(name:, dns_zone:, personal_access_token_value:, user_name:, user_email:, workspace_id:) + [ + { + key: File.basename(RemoteDevelopment::Workspaces::FileMounts::GITLAB_TOKEN_FILE), + value: personal_access_token_value, + variable_type: VARIABLE_TYPE_FILE, + workspace_id: workspace_id + }, + { + key: File.basename(RemoteDevelopment::Workspaces::FileMounts::GITLAB_GIT_CREDENTIAL_STORE_FILE), + value: GIT_CREDENTIAL_STORE_SCRIPT, + variable_type: VARIABLE_TYPE_FILE, + workspace_id: workspace_id + }, + { + key: 'GIT_CONFIG_COUNT', + value: '3', + variable_type: VARIABLE_TYPE_ENV_VAR, + workspace_id: workspace_id + }, + { + key: 'GIT_CONFIG_KEY_0', + value: "credential.helper", + variable_type: VARIABLE_TYPE_ENV_VAR, + workspace_id: workspace_id + }, + { + key: 'GIT_CONFIG_VALUE_0', + value: RemoteDevelopment::Workspaces::FileMounts::GITLAB_GIT_CREDENTIAL_STORE_FILE, + variable_type: VARIABLE_TYPE_ENV_VAR, + workspace_id: workspace_id + }, + { + key: 'GIT_CONFIG_KEY_1', + value: "user.name", + variable_type: VARIABLE_TYPE_ENV_VAR, + workspace_id: workspace_id + }, + { + key: 'GIT_CONFIG_VALUE_1', + value: user_name, + variable_type: VARIABLE_TYPE_ENV_VAR, + workspace_id: workspace_id + }, + { + key: 'GIT_CONFIG_KEY_2', + value: "user.email", + variable_type: VARIABLE_TYPE_ENV_VAR, + workspace_id: workspace_id + }, + { + key: 'GIT_CONFIG_VALUE_2', + value: user_email, + variable_type: VARIABLE_TYPE_ENV_VAR, + workspace_id: workspace_id + }, + { + key: 'GL_GIT_CREDENTIAL_STORE_FILE_PATH', + value: RemoteDevelopment::Workspaces::FileMounts::GITLAB_GIT_CREDENTIAL_STORE_FILE, + variable_type: VARIABLE_TYPE_ENV_VAR, + workspace_id: workspace_id + }, + { + key: 'GL_TOKEN_FILE_PATH', + value: RemoteDevelopment::Workspaces::FileMounts::GITLAB_TOKEN_FILE, + variable_type: VARIABLE_TYPE_ENV_VAR, + workspace_id: workspace_id + }, + { + key: 'GL_WORKSPACE_DOMAIN_TEMPLATE', + value: "${PORT}-#{name}.#{dns_zone}", + variable_type: VARIABLE_TYPE_ENV_VAR, + workspace_id: workspace_id + } + ] + end + end + end + end +end diff --git a/ee/lib/remote_development/workspaces/create/workspace_variables_creator.rb b/ee/lib/remote_development/workspaces/create/workspace_variables_creator.rb new file mode 100644 index 0000000000000000000000000000000000000000..d9098eaad50392dd7703a1d4eefaa8d63311e604 --- /dev/null +++ b/ee/lib/remote_development/workspaces/create/workspace_variables_creator.rb @@ -0,0 +1,46 @@ +# frozen_string_literal: true + +module RemoteDevelopment + module Workspaces + module Create + class WorkspaceVariablesCreator + include Messages + + # @param [Hash] value + # @return [Result] + def self.create(value) + value => { + workspace: RemoteDevelopment::Workspace => workspace, + personal_access_token: PersonalAccessToken => personal_access_token, + current_user: User => user, + } + workspace_variables_params = WorkspaceVariables.variables( + name: workspace.name, + dns_zone: workspace.dns_zone, + personal_access_token_value: personal_access_token.token, + user_name: user.name, + user_email: user.email, + workspace_id: workspace.id + ) + + workspace_variables_params.each do |workspace_variable_params| + workspace_variable = RemoteDevelopment::WorkspaceVariable.new(workspace_variable_params) + workspace_variable.save + + if workspace_variable.errors.present? + return Result.err( + WorkspaceVariablesModelCreateFailed.new({ errors: workspace_variable.errors }) + ) + end + end + + Result.ok( + value.merge({ + workspace_variables_params: workspace_variables_params + }) + ) + end + end + end + end +end diff --git a/ee/lib/remote_development/workspaces/file_mounts.rb b/ee/lib/remote_development/workspaces/file_mounts.rb new file mode 100644 index 0000000000000000000000000000000000000000..918c285e1328cd7c1aa1ec499f0638f9557a7549 --- /dev/null +++ b/ee/lib/remote_development/workspaces/file_mounts.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +module RemoteDevelopment + module Workspaces + module FileMounts + ROOT_DIR = "/.workspace-data" + VARIABLES_DIR = "#{ROOT_DIR}/variables".freeze + VARIABLES_FILE_DIR = "#{VARIABLES_DIR}/file".freeze + GITLAB_GIT_CREDENTIAL_STORE_FILE = "#{VARIABLES_FILE_DIR}/gl_git_credential_store.sh".freeze + GITLAB_TOKEN_FILE = "#{VARIABLES_FILE_DIR}/gl_token".freeze + end + end +end diff --git a/ee/lib/remote_development/workspaces/update/updater.rb b/ee/lib/remote_development/workspaces/update/updater.rb index 7b7f353275d4a7da988b7d69d1c6483506b3ba4f..74c3c08a7c59bbc95ff2a2ec7967a5bc09dacf91 100644 --- a/ee/lib/remote_development/workspaces/update/updater.rb +++ b/ee/lib/remote_development/workspaces/update/updater.rb @@ -5,16 +5,33 @@ module Workspaces module Update class Updater include Messages + include States # @param [Hash] value # @return [Result] def self.update(value) value => { workspace: RemoteDevelopment::Workspace => workspace, params: Hash => params } - if workspace.update(params) - Result.ok(WorkspaceUpdateSuccessful.new({ workspace: workspace })) - else - Result.err(WorkspaceUpdateFailed.new({ errors: workspace.errors })) + model_errors = nil + + ApplicationRecord.transaction do + begin + workspace.personal_access_token.revoke! if params.fetch(:desired_state) == TERMINATED + rescue ActiveRecord::ActiveRecordError + model_errors = workspace.personal_access_token.errors + raise ActiveRecord::Rollback + end + + workspace.update(params) + + if workspace.errors.present? + model_errors = workspace.errors + raise ActiveRecord::Rollback + end end + + return Result.err(WorkspaceUpdateFailed.new({ errors: model_errors })) if model_errors.present? + + Result.ok(WorkspaceUpdateSuccessful.new({ workspace: workspace })) end end end diff --git a/ee/spec/factories/remote_development/workspace_variables.rb b/ee/spec/factories/remote_development/workspace_variables.rb index 4c9bbe81c9201b46f384e0c755b90a1de29ae5af..8f3c64c35c8c7b35c531e9e7a7a231f735c39368 100644 --- a/ee/spec/factories/remote_development/workspace_variables.rb +++ b/ee/spec/factories/remote_development/workspace_variables.rb @@ -6,6 +6,6 @@ key { 'my_key' } value { 'my_value' } - variable_type { RemoteDevelopment::WorkspaceVariable.variable_types[:file] } + variable_type { RemoteDevelopment::Workspaces::Create::WorkspaceVariables::VARIABLE_TYPE_FILE } end end diff --git a/ee/spec/factories/remote_development/workspaces.rb b/ee/spec/factories/remote_development/workspaces.rb index 231521975f83c47554e091ab091670a800593f64..fcd989858a0615d2138da5339b3b8145fb595aaf 100644 --- a/ee/spec/factories/remote_development/workspaces.rb +++ b/ee/spec/factories/remote_development/workspaces.rb @@ -33,6 +33,7 @@ end transient do + without_workspace_variables { false } random_string { SecureRandom.alphanumeric(6).downcase } # noinspection RubyResolve - https://handbook.gitlab.com/handbook/tools-and-tips/editors-and-ides/jetbrains-ides/tracked-jetbrains-issues/#ruby-31542 skip_realistic_after_create_timestamp_updates { false } @@ -80,6 +81,21 @@ responded_to_agent_at: 1.second.ago ) else + unless evaluator.without_workspace_variables + workspace_variables = RemoteDevelopment::Workspaces::Create::WorkspaceVariables.variables( + name: workspace.name, + dns_zone: workspace.dns_zone, + personal_access_token_value: workspace.personal_access_token.token, + user_name: workspace.user.name, + user_email: workspace.user.email, + workspace_id: workspace.id + ) + + workspace_variables.each do |workspace_variable| + workspace.workspace_variables.create!(workspace_variable) + end + end + if workspace.desired_state == workspace.actual_state # The most recent activity was a poll that reconciled the desired and actual state. desired_state_updated_at = 2.seconds.ago @@ -111,5 +127,11 @@ responded_to_agent_at { nil } deployment_resource_version { nil } end + + trait :without_workspace_variables do + transient do + without_workspace_variables { true } + end + end end end diff --git a/ee/spec/lib/remote_development/workspaces/create/creator_spec.rb b/ee/spec/lib/remote_development/workspaces/create/creator_spec.rb index b86511ac9e9c81b13c3480f12735913d57923615..cf6306211f36807147d4bee730baceee815e0563 100644 --- a/ee/spec/lib/remote_development/workspaces/create/creator_spec.rb +++ b/ee/spec/lib/remote_development/workspaces/create/creator_spec.rb @@ -16,6 +16,7 @@ let(:processed_devfile) { YAML.safe_load(example_flattened_devfile) } let(:desired_state) { RemoteDevelopment::Workspaces::States::RUNNING } let(:processed_devfile_yaml) { YAML.safe_load(example_processed_devfile) } + let(:max_hours_before_termination) { 24 } let(:editor) { 'webide' } let(:workspace_root) { '/projects' } @@ -25,7 +26,7 @@ user: user, project: project, editor: editor, - max_hours_before_termination: 24, + max_hours_before_termination: max_hours_before_termination, desired_state: desired_state, devfile_ref: 'main', devfile_path: devfile_path @@ -51,13 +52,19 @@ described_class.create(value) # rubocop:disable Rails/SaveBang end - context 'when workspace create is successful' do + context 'when all db records are created successfully' do before do allow(SecureRandom).to receive(:alphanumeric) { random_string } end - it 'creates the workspace and returns ok result containing successful message with created workspace' do - expect { subject }.to change { project.workspaces.count } + it 'returns ok result containing successful message with created workspace' do + expect { subject }.to change { + [ + project.workspaces.count, + user.personal_access_tokens.reload.count, + project.workspaces.last ? project.workspaces.last.workspace_variables.count : 0 + ] + } expect(result).to be_ok_result do |message| expect(message).to be_a(RemoteDevelopment::Messages::WorkspaceCreateSuccessful) @@ -67,17 +74,55 @@ end end - context 'when workspace create fails' do - let(:desired_state) { 'InvalidDesiredState' } + context 'when workspace fails on creation' do + shared_examples 'err result' do |expected_error_details:| + it 'does not create the db records and returns an error result containing a failed message with model errors' do + expect { subject }.not_to change { + [ + project.workspaces.count, + user.personal_access_tokens.reload.count, + project.workspaces.last ? project.workspaces.last.workspace_variables.count : 0 + ] + } + expect(result).to be_err_result do |message| + expect(message).to be_a(RemoteDevelopment::Messages::WorkspaceCreateFailed) + message.context => { errors: ActiveModel::Errors => errors } + expect(errors.full_messages).to match([/#{expected_error_details}/i]) + end + end + end + + context 'when workspace db record fails on creation' do + let(:desired_state) { 'InvalidDesiredState' } + + it_behaves_like 'err result', expected_error_details: %(desired state) + end + + context 'when personal access token db record fails on creation' do + let(:max_hours_before_termination) { 999999999999 } + + it_behaves_like 'err result', expected_error_details: %(expiration date) + end - it 'does not create the workspace and returns an error result containing a failed message with model errors' do - expect { subject }.not_to change { project.workspaces.count } + context 'when workspace variable db record fails on creation' do + let(:invalid_workspace_variables) do + [ + { + key: "does-not-matter", + value: "does-not-matter", + variable_type: 9999999, + workspace_id: 0 # workspace id does not matter in this case + } + ] + end - expect(result).to be_err_result do |message| - expect(message).to be_a(RemoteDevelopment::Messages::WorkspaceCreateFailed) - message.context => { errors: ActiveModel::Errors => errors } - expect(errors.full_messages).to match([/desired state/i]) + before do + allow(RemoteDevelopment::Workspaces::Create::WorkspaceVariables) + .to receive(:variables) + .and_return(invalid_workspace_variables) end + + it_behaves_like 'err result', expected_error_details: %(variable type) end end end diff --git a/ee/spec/lib/remote_development/workspaces/create/personal_access_token_creator_spec.rb b/ee/spec/lib/remote_development/workspaces/create/personal_access_token_creator_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..edb616b9ce7e06522bf6ceaf88cf1ee92e84412c --- /dev/null +++ b/ee/spec/lib/remote_development/workspaces/create/personal_access_token_creator_spec.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ::RemoteDevelopment::Workspaces::Create::PersonalAccessTokenCreator, feature_category: :remote_development do + include ResultMatchers + + include_context 'with remote development shared fixtures' + + let_it_be(:user) { create(:user) } + let(:workspace_name) { "workspace-example_agent_id-example_user_id-example_random_string" } + let(:max_hours_before_termination) { 24 } + let(:params) do + { + max_hours_before_termination: max_hours_before_termination + } + end + + let(:value) do + { + params: params, + current_user: user, + workspace_name: workspace_name + } + end + + subject(:result) do + described_class.create(value) # rubocop:disable Rails/SaveBang + end + + context 'when personal access token creation is successful' do + it 'returns ok result containing successful message with created token' do + expect { subject }.to change { user.personal_access_tokens.count } + + expect(result).to be_ok_result do |message| + message => { personal_access_token: PersonalAccessToken => personal_access_token } + expect(personal_access_token).to eq(user.personal_access_tokens.reload.last) + end + end + end + + context 'when personal access token creation fails' do + let(:max_hours_before_termination) { 999999999999 } + + it 'returns an error result containing a failed message with model errors' do + expect { subject }.not_to change { user.personal_access_tokens.count } + + expect(result).to be_err_result do |message| + expect(message).to be_a(RemoteDevelopment::Messages::PersonalAccessTokenModelCreateFailed) + message.context => { errors: ActiveModel::Errors => errors } + expect(errors.full_messages).to match([/expiration date/i]) + end + end + end +end diff --git a/ee/spec/lib/remote_development/workspaces/create/workspace_creator_spec.rb b/ee/spec/lib/remote_development/workspaces/create/workspace_creator_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..c21816796acb72e68b3a4516d51d01937a90a4db --- /dev/null +++ b/ee/spec/lib/remote_development/workspaces/create/workspace_creator_spec.rb @@ -0,0 +1,82 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ::RemoteDevelopment::Workspaces::Create::WorkspaceCreator, feature_category: :remote_development do + include ResultMatchers + + include_context 'with remote development shared fixtures' + + let_it_be(:user) { create(:user) } + let_it_be(:project, reload: true) { create(:project, :public, :in_group, :repository) } + let_it_be(:agent) { create(:ee_cluster_agent, :with_remote_development_agent_config) } + let_it_be(:personal_access_token) { create(:personal_access_token, user: user) } + let(:random_string) { 'abcdef' } + let(:devfile_path) { '.devfile.yaml' } + let(:devfile_yaml) { example_devfile } + let(:processed_devfile) { YAML.safe_load(example_flattened_devfile) } + let(:desired_state) { RemoteDevelopment::Workspaces::States::RUNNING } + let(:processed_devfile_yaml) { YAML.safe_load(example_processed_devfile) } + + let(:editor) { 'webide' } + let(:workspace_root) { '/projects' } + let(:params) do + { + agent: agent, + user: user, + project: project, + editor: editor, + max_hours_before_termination: 24, + desired_state: desired_state, + devfile_ref: 'main', + devfile_path: devfile_path + } + end + + let(:value) do + { + params: params, + current_user: user, + devfile_yaml: devfile_yaml, + processed_devfile: processed_devfile, + personal_access_token: personal_access_token, + workspace_name: "workspace-#{agent.id}-#{user.id}-#{random_string}", + workspace_namespace: "gl-rd-ns-#{agent.id}-#{user.id}-#{random_string}", + volume_mounts: { + data_volume: { + name: "gl-workspace-data", + path: workspace_root + } + } + } + end + + subject(:result) do + described_class.create(value) # rubocop:disable Rails/SaveBang + end + + context 'when workspace create is successful' do + it 'creates the workspace and returns ok result containing successful message with created workspace' do + expect { subject }.to change { project.workspaces.count } + + expect(result).to be_ok_result do |message| + message => { workspace: RemoteDevelopment::Workspace => workspace } + expect(workspace).to eq(project.workspaces.last) + end + end + end + + context 'when workspace create fails' do + let(:desired_state) { 'InvalidDesiredState' } + + it 'does not create the workspace and returns an error result containing a failed message with model errors' do + expect { subject }.not_to change { project.workspaces.count } + + expect(result).to be_err_result do |message| + expect(message).to be_a(RemoteDevelopment::Messages::WorkspaceModelCreateFailed) + message.context => { errors: ActiveModel::Errors => errors } + expect(errors.full_messages).to match([/desired state/i]) + end + end + end +end diff --git a/ee/spec/lib/remote_development/workspaces/create/workspace_variables_creator_spec.rb b/ee/spec/lib/remote_development/workspaces/create/workspace_variables_creator_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..112722bdd7798c521f752a238dc558cbb5cdb274 --- /dev/null +++ b/ee/spec/lib/remote_development/workspaces/create/workspace_variables_creator_spec.rb @@ -0,0 +1,78 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ::RemoteDevelopment::Workspaces::Create::WorkspaceVariablesCreator, feature_category: :remote_development do + include ResultMatchers + + include_context 'with remote development shared fixtures' + + let_it_be(:user) { create(:user) } + let_it_be(:personal_access_token) { create(:personal_access_token, user: user) } + let_it_be(:workspace) { create(:workspace, user: user, personal_access_token: personal_access_token) } + let(:invalid_workspace_variables) do + [ + { + key: "does-not-matter", + value: "does-not-matter", + variable_type: 9999999, + workspace_id: workspace.id + } + ] + end + + let(:expected_workspace_variables_params) do + variables = RemoteDevelopment::Workspaces::Create::WorkspaceVariables.variables( + name: workspace.name, + dns_zone: workspace.dns_zone, + personal_access_token_value: personal_access_token.token, + user_name: user.name, + user_email: user.email, + workspace_id: workspace.id + ) + variables.each do |variable| + variable[:workspace_id] = workspace.id + end + end + + let(:value) do + { + workspace: workspace, + current_user: user, + personal_access_token: personal_access_token + } + end + + subject(:result) do + described_class.create(value) # rubocop:disable Rails/SaveBang + end + + context 'when workspace variables create is successful' do + it 'creates the workspace variables and returns ok result containing successful message with created variables' do + expect { subject }.to change { workspace.workspace_variables.count } + + expect(result).to be_ok_result do |message| + message => { workspace_variables_params: Array => workspace_variables_params } + expect(workspace_variables_params).to eq(expected_workspace_variables_params) + end + end + end + + context 'when workspace create fails' do + before do + allow(RemoteDevelopment::Workspaces::Create::WorkspaceVariables) + .to receive(:variables) + .and_return(invalid_workspace_variables) + end + + it 'does not create the workspace and returns an error result containing a failed message with model errors' do + expect { subject }.not_to change { workspace.workspace_variables.count } + + expect(result).to be_err_result do |message| + expect(message).to be_a(RemoteDevelopment::Messages::WorkspaceVariablesModelCreateFailed) + message.context => { errors: ActiveModel::Errors => errors } + expect(errors.full_messages).to match([/variable type/i]) + end + end + end +end diff --git a/ee/spec/lib/remote_development/workspaces/create/workspace_variables_spec.rb b/ee/spec/lib/remote_development/workspaces/create/workspace_variables_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..945508e6928d7233413f681029b2a8bf653ea1f3 --- /dev/null +++ b/ee/spec/lib/remote_development/workspaces/create/workspace_variables_spec.rb @@ -0,0 +1,127 @@ +# frozen_string_literal: true + +require_relative "../../fast_spec_helper" + +RSpec.describe ::RemoteDevelopment::Workspaces::Create::WorkspaceVariables, + feature_category: :remote_development do + let(:name) { "name" } + let(:dns_zone) { "example.dns.zone" } + let(:personal_access_token_value) { "example-pat-value" } + let(:user_name) { "example.user.name" } + let(:user_email) { "example@user.email" } + let(:workspace_id) { 1 } + let(:git_credential_store_script) do + <<~SH.chomp + #!/bin/sh + # This is a readonly store so we can exit cleanly when git attempts a store or erase action + if [ "$1" != "get" ]; + then + exit 0 + fi + + if [ -z "${GL_TOKEN_FILE_PATH}" ]; + then + echo "We could not find the GL_TOKEN_FILE_PATH variable" + exit 1 + fi + password=$(cat ${GL_TOKEN_FILE_PATH}) + + # The username is derived from the "user.email" configuration item. Ensure it is set. + echo "username=does-not-matter" + echo "password=${password}" + exit 0 + SH + end + + let(:expected_variables) do + [ + { + key: "gl_token", + value: "example-pat-value", + variable_type: RemoteDevelopment::Workspaces::Create::WorkspaceVariables::VARIABLE_TYPE_FILE, + workspace_id: workspace_id + }, + { + key: "gl_git_credential_store.sh", + value: git_credential_store_script, + variable_type: RemoteDevelopment::Workspaces::Create::WorkspaceVariables::VARIABLE_TYPE_FILE, + workspace_id: workspace_id + }, + { + key: "GIT_CONFIG_COUNT", + value: "3", + variable_type: RemoteDevelopment::Workspaces::Create::WorkspaceVariables::VARIABLE_TYPE_ENV_VAR, + workspace_id: workspace_id + }, + { + key: "GIT_CONFIG_KEY_0", + value: "credential.helper", + variable_type: RemoteDevelopment::Workspaces::Create::WorkspaceVariables::VARIABLE_TYPE_ENV_VAR, + workspace_id: workspace_id + }, + { + key: "GIT_CONFIG_VALUE_0", + value: "/.workspace-data/variables/file/gl_git_credential_store.sh", + variable_type: RemoteDevelopment::Workspaces::Create::WorkspaceVariables::VARIABLE_TYPE_ENV_VAR, + workspace_id: workspace_id + }, + { + key: "GIT_CONFIG_KEY_1", + value: "user.name", + variable_type: RemoteDevelopment::Workspaces::Create::WorkspaceVariables::VARIABLE_TYPE_ENV_VAR, + workspace_id: workspace_id + }, + { + key: "GIT_CONFIG_VALUE_1", + value: "example.user.name", + variable_type: RemoteDevelopment::Workspaces::Create::WorkspaceVariables::VARIABLE_TYPE_ENV_VAR, + workspace_id: workspace_id + }, + { + key: "GIT_CONFIG_KEY_2", + value: "user.email", + variable_type: RemoteDevelopment::Workspaces::Create::WorkspaceVariables::VARIABLE_TYPE_ENV_VAR, + workspace_id: workspace_id + }, + { + key: "GIT_CONFIG_VALUE_2", + value: "example@user.email", + variable_type: RemoteDevelopment::Workspaces::Create::WorkspaceVariables::VARIABLE_TYPE_ENV_VAR, + workspace_id: workspace_id + }, + { + key: "GL_GIT_CREDENTIAL_STORE_FILE_PATH", + value: "/.workspace-data/variables/file/gl_git_credential_store.sh", + variable_type: RemoteDevelopment::Workspaces::Create::WorkspaceVariables::VARIABLE_TYPE_ENV_VAR, + workspace_id: workspace_id + }, + { + key: "GL_TOKEN_FILE_PATH", + value: "/.workspace-data/variables/file/gl_token", + variable_type: RemoteDevelopment::Workspaces::Create::WorkspaceVariables::VARIABLE_TYPE_ENV_VAR, + workspace_id: workspace_id + }, + { + key: "GL_WORKSPACE_DOMAIN_TEMPLATE", + value: "${PORT}-name.example.dns.zone", + variable_type: RemoteDevelopment::Workspaces::Create::WorkspaceVariables::VARIABLE_TYPE_ENV_VAR, + workspace_id: workspace_id + } + ] + end + + subject(:variables) do + described_class.variables( + name: name, + dns_zone: dns_zone, + personal_access_token_value: personal_access_token_value, + user_name: user_name, + user_email: user_email, + workspace_id: workspace_id + ) + end + + it 'defines correct variables' do + expect(variables).to eq(expected_variables) + end +end diff --git a/ee/spec/lib/remote_development/workspaces/update/updater_spec.rb b/ee/spec/lib/remote_development/workspaces/update/updater_spec.rb index 3cf88d5fa58a32ad79552252b3fc82c07e1c2161..a1715affa42e2385a2002646bffe6f06399c6158 100644 --- a/ee/spec/lib/remote_development/workspaces/update/updater_spec.rb +++ b/ee/spec/lib/remote_development/workspaces/update/updater_spec.rb @@ -11,8 +11,14 @@ let_it_be(:user) { create(:user) } let_it_be(:current_user) { user } - let_it_be(:workspace, refind: true) do - create(:workspace, user: user, desired_state: RemoteDevelopment::Workspaces::States::RUNNING) + let(:personal_access_token) { create(:personal_access_token, user: user) } + let(:workspace) do + create( + :workspace, + user: user, + desired_state: RemoteDevelopment::Workspaces::States::RUNNING, + personal_access_token: personal_access_token + ) end let(:new_desired_state) { RemoteDevelopment::Workspaces::States::STOPPED } @@ -32,15 +38,47 @@ end context 'when workspace update fails' do - let(:new_desired_state) { 'InvalidDesiredState' } + shared_examples 'err result' do |expected_error_details:| + it 'does not update the db record and returns an error result containing a failed message with model errors' do + expect { subject }.not_to change { + [ + workspace.reload, + workspace.personal_access_token.reload + ] + } + expect(result).to be_err_result do |message| + expect(message).to be_a(RemoteDevelopment::Messages::WorkspaceUpdateFailed) + message.context => { errors: ActiveModel::Errors => errors } + expect(errors.full_messages).to match([/#{expected_error_details}/i]) + end + end + end + + context 'when workspace desired state update fails' do + let(:new_desired_state) { 'InvalidDesiredState' } + + it_behaves_like 'err result', expected_error_details: %(desired state) + end + + context 'when personal access token revocation fails' do + let(:new_desired_state) { RemoteDevelopment::Workspaces::States::TERMINATED } + let(:errors) { instance_double(ActiveModel::Errors) } + let(:exception) { ActiveRecord::ActiveRecordError.new } + let(:mock_personal_access_token) { instance_double(PersonalAccessToken) } + + before do + allow(workspace).to receive(:personal_access_token) { mock_personal_access_token } + allow(mock_personal_access_token).to receive(:revoke!).and_raise(exception) + allow(mock_personal_access_token).to receive(:errors).and_return(errors) + end - it 'does not update the workspace and returns an error result containing a failed message with model errors' do - expect { subject }.not_to change { workspace.reload } + it 'does not revoke the token and returns an error result containing a failed message with model errors' do + expect { subject }.not_to change { workspace.reload.updated_at } - expect(result).to be_err_result do |message| - expect(message).to be_a(RemoteDevelopment::Messages::WorkspaceUpdateFailed) - message.context => { errors: ActiveModel::Errors => errors } - expect(errors.full_messages).to match([/desired state/i]) + expect(result).to be_err_result do |message| + expect(message).to be_a(RemoteDevelopment::Messages::WorkspaceUpdateFailed) + expect(message.context[:errors]).to be(errors) + end end end end diff --git a/ee/spec/models/remote_development/workspace_variable_spec.rb b/ee/spec/models/remote_development/workspace_variable_spec.rb index cce2196e4028e48ff480535affc41b0368dc5020..0ca6fb92ab07312bc7a88f86af673e524d9b7f65 100644 --- a/ee/spec/models/remote_development/workspace_variable_spec.rb +++ b/ee/spec/models/remote_development/workspace_variable_spec.rb @@ -3,15 +3,22 @@ require 'spec_helper' RSpec.describe RemoteDevelopment::WorkspaceVariable, feature_category: :remote_development do - let(:variable_type_values) { { env_var: 0, file: 1 } } let(:key) { 'key_1' } let(:value) { 'value_1' } - let_it_be(:workspace) do - create(:workspace) + let(:variable_type_env_var) { RemoteDevelopment::Workspaces::Create::WorkspaceVariables::VARIABLE_TYPE_ENV_VAR } + let(:variable_type_file) { RemoteDevelopment::Workspaces::Create::WorkspaceVariables::VARIABLE_TYPE_FILE } + let(:variable_type) { variable_type_file } + let(:variable_type_values) do + [ + variable_type_env_var, + variable_type_file + ] end + let_it_be(:workspace) { create(:workspace, :without_workspace_variables) } + subject do - create(:workspace_variable, workspace: workspace, key: key, value: value) + create(:workspace_variable, workspace: workspace, key: key, value: value, variable_type: variable_type) end describe 'associations' do @@ -22,16 +29,11 @@ end end - describe 'enums' do - it { is_expected.to define_enum_for(:variable_type).with_values(variable_type_values).with_prefix(:variable_type) } - - it_behaves_like 'having unique enum values' - end - describe 'validations' do it { is_expected.to validate_presence_of(:key) } it { is_expected.to validate_length_of(:key).is_at_most(255) } it { is_expected.to validate_presence_of(:variable_type) } + it { is_expected.to validate_inclusion_of(:variable_type).in_array(variable_type_values) } end describe '#value' do @@ -39,4 +41,22 @@ expect(subject.value).to eq(value) end end + + describe 'scopes' do + describe 'with_variable_type_env_var' do + let(:variable_type) { variable_type_env_var } + + it 'returns the record' do + expect(described_class.with_variable_type_env_var).to eq([subject]) + end + end + + describe 'with_variable_type_file' do + let(:variable_type) { variable_type_file } + + it 'returns the record' do + expect(described_class.with_variable_type_file).to eq([subject]) + end + end + end end