diff --git a/app/controllers/execution_environments_controller.rb b/app/controllers/execution_environments_controller.rb index 648f7140..68a84625 100644 --- a/app/controllers/execution_environments_controller.rb +++ b/app/controllers/execution_environments_controller.rb @@ -107,8 +107,15 @@ class ExecutionEnvironmentsController < ApplicationController def execution_environment_params if params[:execution_environment].present? - params[:execution_environment].permit(:docker_image, :exposed_ports, :editor_mode, :file_extension, :file_type_id, :help, :indent_size, :memory_limit, :cpu_limit, :name, :network_enabled, :permitted_execution_time, :pool_size, :run_command, :test_command, :testing_framework).merge( - user_id: current_user.id, user_type: current_user.class.name + exposed_ports = if params[:execution_environment][:exposed_ports_list].present? + # Transform the `exposed_ports_list` to `exposed_ports` array + params[:execution_environment].delete(:exposed_ports_list).scan(/\d+/) + else + [] + end + + params[:execution_environment].permit(:docker_image, :editor_mode, :file_extension, :file_type_id, :help, :indent_size, :memory_limit, :cpu_limit, :name, :network_enabled, :permitted_execution_time, :pool_size, :run_command, :test_command, :testing_framework).merge( + user_id: current_user.id, user_type: current_user.class.name, exposed_ports: exposed_ports ) end end diff --git a/app/models/execution_environment.rb b/app/models/execution_environment.rb index cee38252..8aafa19d 100644 --- a/app/models/execution_environment.rb +++ b/app/models/execution_environment.rb @@ -28,7 +28,8 @@ class ExecutionEnvironment < ApplicationRecord validates :pool_size, numericality: {only_integer: true}, presence: true validates :run_command, presence: true validates :cpu_limit, presence: true, numericality: {greater_than: 0, only_integer: true} - validates :exposed_ports, format: {with: /\A(([[:digit:]]{1,5},)*([[:digit:]]{1,5}))?\z/} + before_validation :clean_exposed_ports + validates :exposed_ports, array: {numericality: {greater_than_or_equal_to: 0, less_than: 65_536, only_integer: true}} def set_default_values set_default_values_if_present(permitted_execution_time: 60, pool_size: 0) @@ -47,14 +48,19 @@ class ExecutionEnvironment < ApplicationRecord cpuLimit: cpu_limit, memoryLimit: memory_limit, networkAccess: network_enabled, - exposedPorts: exposed_ports_list, + exposedPorts: exposed_ports, }.to_json end def exposed_ports_list - (exposed_ports || '').split(',').map(&:to_i) + exposed_ports.join(', ') end + def clean_exposed_ports + self.exposed_ports = exposed_ports.uniq.sort + end + private :clean_exposed_ports + def valid_test_setup? if test_command? ^ testing_framework? errors.add(:test_command, diff --git a/app/views/execution_environments/_form.html.slim b/app/views/execution_environments/_form.html.slim index 9447d1c6..e5853f90 100644 --- a/app/views/execution_environments/_form.html.slim +++ b/app/views/execution_environments/_form.html.slim @@ -14,9 +14,9 @@ = f.text_field(:docker_image, class: 'alternative-input form-control', disabled: true) .help-block.form-text == t('.hints.docker_image') .form-group - = f.label(:exposed_ports) - = f.text_field(:exposed_ports, class: 'form-control', placeholder: '3000,4000', pattern: '^((\d{1,5},)*(\d{1,5}))?$') - .help-block.form-text == t('.hints.exposed_ports') + = f.label(:exposed_ports_list) + = f.text_field(:exposed_ports_list, class: 'form-control', placeholder: '3000, 4000', pattern: '^(\s*(\d{1,5},\s*)*(\d{1,5}\s*))?$') + .help-block.form-text = t('.hints.exposed_ports_list') .form-group = f.label(:memory_limit) = f.number_field(:memory_limit, class: 'form-control', min: DockerClient::MINIMUM_MEMORY_LIMIT, value: f.object.memory_limit || DockerClient::DEFAULT_MEMORY_LIMIT) diff --git a/app/views/execution_environments/show.html.slim b/app/views/execution_environments/show.html.slim index 6d9fb32e..e1b15ec6 100644 --- a/app/views/execution_environments/show.html.slim +++ b/app/views/execution_environments/show.html.slim @@ -5,7 +5,7 @@ h1 = row(label: 'execution_environment.name', value: @execution_environment.name) = row(label: 'execution_environment.user', value: link_to_if(policy(@execution_environment.author).show?, @execution_environment.author, @execution_environment.author)) = row(label: 'execution_environment.file_type', value: @execution_environment.file_type.present? ? link_to(@execution_environment.file_type, @execution_environment.file_type) : nil) -- [:docker_image, :exposed_ports, :memory_limit, :cpu_limit, :network_enabled, :permitted_execution_time, :pool_size].each do |attribute| +- [:docker_image, :exposed_ports_list, :memory_limit, :cpu_limit, :network_enabled, :permitted_execution_time, :pool_size].each do |attribute| = row(label: "execution_environment.#{attribute}", value: @execution_environment.send(attribute)) - [:run_command, :test_command].each do |attribute| = row(label: "execution_environment.#{attribute}") do diff --git a/config/locales/de.yml b/config/locales/de.yml index ea5f34eb..a1ff2414 100644 --- a/config/locales/de.yml +++ b/config/locales/de.yml @@ -10,6 +10,7 @@ de: execution_environment: docker_image: Docker-Image exposed_ports: Zugängliche Ports + exposed_ports_list: Zugängliche Ports file_type: Standard-Dateityp file_type_id: Standard-Dateityp help: Hilfetext @@ -282,7 +283,7 @@ de: hints: command: filename wird automatisch durch den richtigen Dateinamen ersetzt. docker_image: 'Wählen Sie ein Docker-Image aus der Liste oder fügen Sie ein neues hinzu, welches über DockerHub verfügbar ist.' - exposed_ports: Während der Ausführung sind diese Ports für den Nutzer zugänglich. Die Portnummern müssen mit Komma, aber ohne Leerzeichen voneinander getrennt sein. + exposed_ports_list: Während der Ausführung sind diese Ports für den Nutzer zugänglich. Die Portnummern müssen nummerisch und mit Komma voneinander getrennt sein. errors: not_synced_to_runner_management: Die Ausführungsumgebung wurde erstellt, aber aufgrund eines Fehlers nicht zum Runnermanagement synchronisiert. index: diff --git a/config/locales/en.yml b/config/locales/en.yml index b2995a27..41722ab7 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -10,6 +10,7 @@ en: execution_environment: docker_image: Docker Image exposed_ports: Exposed Ports + exposed_ports_list: Exposed Ports file_type: Default File Type file_type_id: Default File Type help: Help Text @@ -282,7 +283,7 @@ en: hints: command: filename is automatically replaced with the correct filename. docker_image: Pick a Docker image listed above or add a new one which is available via DockerHub. - exposed_ports: During code execution these ports are accessible for the user. Port numbers must be separated by a comma but no space. + exposed_ports_list: During code execution these ports are accessible for the user. Port numbers must be numeric and separated by a comma. errors: not_synced_to_runner_management: The execution environment was created but not synced to the runner management due to an error. index: diff --git a/db/migrate/20210602071834_change_type_of_exposed_ports_in_execution_environment.rb b/db/migrate/20210602071834_change_type_of_exposed_ports_in_execution_environment.rb new file mode 100644 index 00000000..bac19565 --- /dev/null +++ b/db/migrate/20210602071834_change_type_of_exposed_ports_in_execution_environment.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +class ChangeTypeOfExposedPortsInExecutionEnvironment < ActiveRecord::Migration[6.1] + # rubocop:disable Rails/SkipsModelValidations: + def up + rename_column :execution_environments, :exposed_ports, :exposed_ports_migration + add_column :execution_environments, :exposed_ports, :integer, array: true, default: [], nil: true + + ExecutionEnvironment.all.each do |execution_environment| + next if execution_environment.exposed_ports_migration.nil? + + cleaned = execution_environment.exposed_ports_migration.scan(/\d+/) + list = cleaned.map(&:to_i).uniq.sort + execution_environment.update_columns(exposed_ports: list) + end + + remove_column :execution_environments, :exposed_ports_migration + end + + def down + rename_column :execution_environments, :exposed_ports, :exposed_ports_migration + add_column :execution_environments, :exposed_ports, :string + + ExecutionEnvironment.all.each do |execution_environment| + next if execution_environment.exposed_ports_migration.empty? + + list = execution_environment.exposed_ports_migration + if list.empty? + execution_environment.update_columns(exposed_ports: nil) + else + execution_environment.update_columns(exposed_ports: list.join(',')) + end + end + remove_column :execution_environments, :exposed_ports_migration + end + # rubocop:enable Rails/SkipsModelValidations: +end diff --git a/db/migrate/20210602071834_clean_exposed_ports_in_execution_environment.rb b/db/migrate/20210602071834_clean_exposed_ports_in_execution_environment.rb deleted file mode 100644 index c50cc53f..00000000 --- a/db/migrate/20210602071834_clean_exposed_ports_in_execution_environment.rb +++ /dev/null @@ -1,17 +0,0 @@ -# frozen_string_literal: true - -class CleanExposedPortsInExecutionEnvironment < ActiveRecord::Migration[6.1] - def change - ExecutionEnvironment.all.each do |execution_environment| - next if execution_environment.exposed_ports.nil? - - cleaned = execution_environment.exposed_ports.gsub(/[[:space:]]/, '') - list = cleaned.split(',').map(&:to_i).uniq - if list.empty? - execution_environment.update(exposed_ports: nil) - else - execution_environment.update(exposed_ports: list.join(',')) - end - end - end -end diff --git a/db/schema.rb b/db/schema.rb index 9a53d05b..59de10d1 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -104,7 +104,6 @@ ActiveRecord::Schema.define(version: 2021_06_02_071834) do t.string "test_command", limit: 255 t.string "testing_framework", limit: 255 t.text "help" - t.string "exposed_ports", limit: 255 t.integer "permitted_execution_time" t.integer "user_id" t.string "user_type", limit: 255 @@ -113,6 +112,7 @@ ActiveRecord::Schema.define(version: 2021_06_02_071834) do t.integer "memory_limit" t.boolean "network_enabled" t.integer "cpu_limit", default: 20, null: false + t.integer "exposed_ports", default: [], array: true end create_table "exercise_collection_items", id: :serial, force: :cascade do |t| diff --git a/lib/docker_client.rb b/lib/docker_client.rb index 313bb226..ff9a9f45 100644 --- a/lib/docker_client.rb +++ b/lib/docker_client.rb @@ -448,7 +448,7 @@ container_execution_time: nil} end def self.mapped_ports(execution_environment) - (execution_environment.exposed_ports || '').gsub(/\s/, '').split(',').map do |port| + execution_environment.exposed_ports.map do |port| ["#{port}/tcp", [{'HostPort' => PortPool.available_port.to_s}]] end.to_h end diff --git a/spec/factories/execution_environment.rb b/spec/factories/execution_environment.rb index 9d145031..94ecfdd4 100644 --- a/spec/factories/execution_environment.rb +++ b/spec/factories/execution_environment.rb @@ -122,7 +122,7 @@ FactoryBot.define do default_cpu_limit docker_image { 'hklement/ubuntu-sinatra:latest' } file_type { association :dot_rb, user: user } - exposed_ports { '4567' } + exposed_ports { [4567] } help name { 'Sinatra' } network_enabled { true } diff --git a/spec/lib/docker_client_spec.rb b/spec/lib/docker_client_spec.rb index 5b112416..54208a77 100644 --- a/spec/lib/docker_client_spec.rb +++ b/spec/lib/docker_client_spec.rb @@ -332,7 +332,7 @@ describe DockerClient, docker: true do describe '.mapped_ports' do context 'with exposed ports' do - before { execution_environment.exposed_ports = '3000' } + before { execution_environment.exposed_ports = [3000] } it 'returns a mapping' do expect(described_class.mapped_ports(execution_environment)).to be_a(Hash) diff --git a/spec/models/execution_environment_spec.rb b/spec/models/execution_environment_spec.rb index d7b057ab..6525869d 100644 --- a/spec/models/execution_environment_spec.rb +++ b/spec/models/execution_environment_spec.rb @@ -178,17 +178,17 @@ describe ExecutionEnvironment do end describe '#exposed_ports_list' do - it 'returns an empty array if no ports are exposed' do - execution_environment.exposed_ports = nil - expect(execution_environment.exposed_ports_list).to eq([]) + it 'returns an empty string if no ports are exposed' do + execution_environment.exposed_ports = [] + expect(execution_environment.exposed_ports_list).to eq('') end - it 'returns an array of integers representing the exposed ports' do - execution_environment.exposed_ports = '1,2,3' - expect(execution_environment.exposed_ports_list).to eq([1, 2, 3]) + it 'returns an string with comma-separated integers representing the exposed ports' do + execution_environment.exposed_ports = [1, 2, 3] + expect(execution_environment.exposed_ports_list).to eq('1, 2, 3') - execution_environment.exposed_ports_list.each do |port| - expect(execution_environment.exposed_ports).to include(port.to_s) + execution_environment.exposed_ports.each do |port| + expect(execution_environment.exposed_ports_list).to include(port.to_s) end end end