diff --git a/app/actions/task_create.rb b/app/actions/task_create.rb index 084fecd04fd..2eddace7bad 100644 --- a/app/actions/task_create.rb +++ b/app/actions/task_create.rb @@ -27,6 +27,7 @@ def create(app, message, user_audit_info, droplet: nil) state: TaskModel::PENDING_STATE, droplet: droplet, command: command(message, template_process), + user: user(message, template_process), disk_in_mb: disk_in_mb(message, template_process), memory_in_mb: memory_in_mb(message, template_process), log_rate_limit: log_rate_limit(message, template_process), @@ -70,6 +71,13 @@ def command(message, template_process) template_process.specified_or_detected_command end + def user(message, template_process) + return message.user if message.requested?(:user) + return template_process.user if message.template_requested? + + nil + end + def memory_in_mb(message, template_process) message.memory_in_mb || template_process.try(:memory) || config.get(:default_app_memory) end diff --git a/app/messages/task_create_message.rb b/app/messages/task_create_message.rb index b52cb73982b..3ad0c5efbb9 100644 --- a/app/messages/task_create_message.rb +++ b/app/messages/task_create_message.rb @@ -2,7 +2,7 @@ module VCAP::CloudController class TaskCreateMessage < MetadataBaseMessage - register_allowed_keys %i[name command disk_in_mb memory_in_mb log_rate_limit_in_bytes_per_second droplet_guid template] + register_allowed_keys %i[name command disk_in_mb memory_in_mb log_rate_limit_in_bytes_per_second droplet_guid template user] validates_with NoAdditionalKeysValidator @@ -10,12 +10,21 @@ def self.validate_template? @validate_template ||= proc { |a| a.template_requested? } end + def self.user_requested? + @user_requested ||= proc { |a| a.requested?(:user) } + end + validates :disk_in_mb, numericality: { only_integer: true, greater_than: 0 }, allow_nil: true validates :memory_in_mb, numericality: { only_integer: true, greater_than: 0 }, allow_nil: true validates :log_rate_limit_in_bytes_per_second, numericality: { only_integer: true, greater_than: -2, less_than_or_equal_to: MAX_DB_BIGINT }, allow_nil: true validates :droplet_guid, guid: true, allow_nil: true validates :template_process_guid, guid: true, if: validate_template? validate :has_command + validates :user, + string: true, + length: { in: 1..255, message: 'must be between 1 and 255 characters' }, + allow_nil: true, + if: user_requested? def template_process_guid return unless template_requested? diff --git a/app/models/runtime/app_model.rb b/app/models/runtime/app_model.rb index 81cc8396f19..bdb1dd7282a 100644 --- a/app/models/runtime/app_model.rb +++ b/app/models/runtime/app_model.rb @@ -7,6 +7,8 @@ module VCAP::CloudController class AppModel < Sequel::Model(:apps) include Serializer APP_NAME_REGEX = /\A[[:alnum:][:punct:][:print:]]+\Z/ + DEFAULT_CONTAINER_USER = 'vcap'.freeze + DEFAULT_DOCKER_CONTAINER_USER = 'root'.freeze many_to_many :routes, join_table: :route_mappings, left_key: :app_guid, left_primary_key: :guid, right_primary_key: :guid, right_key: :route_guid one_to_many :route_mappings, class: 'VCAP::CloudController::RouteMappingModel', key: :app_guid, primary_key: :guid diff --git a/app/models/runtime/droplet_model.rb b/app/models/runtime/droplet_model.rb index f27ed20b100..1c4e572e357 100644 --- a/app/models/runtime/droplet_model.rb +++ b/app/models/runtime/droplet_model.rb @@ -125,6 +125,22 @@ def docker_ports exposed_ports end + def docker_user + return '' unless docker? + + container_user = '' + if execution_metadata.present? + begin + docker_exec_metadata = Oj.load(execution_metadata) + container_user = docker_exec_metadata['user'] + rescue EncodingError + # ignore + end + end + + container_user.presence || AppModel::DEFAULT_DOCKER_CONTAINER_USER + end + def staging? state == STAGING_STATE end diff --git a/app/models/runtime/process_model.rb b/app/models/runtime/process_model.rb index efa35a1bf35..e83b4460bc5 100644 --- a/app/models/runtime/process_model.rb +++ b/app/models/runtime/process_model.rb @@ -34,7 +34,6 @@ def after_initialize NO_APP_PORT_SPECIFIED = -1 DEFAULT_HTTP_PORT = 8080 DEFAULT_PORTS = [DEFAULT_HTTP_PORT].freeze - DEFAULT_USER = 'vcap'.freeze UNLIMITED_LOG_RATE = -1 many_to_one :app, class: 'VCAP::CloudController::AppModel', key: :app_guid, primary_key: :guid, without_guid_generation: true @@ -396,7 +395,7 @@ def started_command def run_action_user return user if user.present? - docker? ? docker_run_action_user : DEFAULT_USER + docker? ? docker_run_action_user : AppModel::DEFAULT_CONTAINER_USER end def specified_or_detected_command @@ -573,23 +572,13 @@ def open_ports private def permitted_users - Set.new([DEFAULT_USER]) + Config.config.get(:additional_allowed_process_users) + Set.new([AppModel::DEFAULT_CONTAINER_USER]) + Config.config.get(:additional_allowed_process_users) end def docker_run_action_user - return DEFAULT_USER unless docker? - - container_user = '' - if execution_metadata.present? - begin - docker_exec_metadata = Oj.load(execution_metadata) - container_user = docker_exec_metadata['user'] - rescue EncodingError - container_user = '' - end - end + return AppModel::DEFAULT_CONTAINER_USER unless docker? - container_user.presence || 'root' + desired_droplet&.docker_user.presence || AppModel::DEFAULT_DOCKER_CONTAINER_USER end def non_unique_process_types diff --git a/app/models/runtime/task_model.rb b/app/models/runtime/task_model.rb index c952bcefe8e..5ddf3021919 100644 --- a/app/models/runtime/task_model.rb +++ b/app/models/runtime/task_model.rb @@ -44,8 +44,31 @@ def after_destroy create_stop_event unless terminal_state? end + def run_action_user + return user if user.present? + + if docker? + docker_run_action_user + elsif cnb? + 'root' # TODO: Why do CNB tasks default to this user instead of vcap? + else + AppModel::DEFAULT_CONTAINER_USER + end + end + + delegate :docker?, to: :droplet + delegate :cnb?, to: :droplet + private + def permitted_users + Set.new([AppModel::DEFAULT_CONTAINER_USER]) + Config.config.get(:additional_allowed_process_users) + end + + def docker_run_action_user + droplet.docker_user.presence || AppModel::DEFAULT_CONTAINER_USER + end + def running_state? state == RUNNING_STATE end @@ -68,6 +91,7 @@ def validate validate_org_quotas validate_space_quotas + ProcessUserPolicy.new(self, permitted_users).validate MinLogRateLimitPolicy.new(self).validate end diff --git a/app/presenters/v3/task_presenter.rb b/app/presenters/v3/task_presenter.rb index 85d0a71186c..7b02588c3a5 100644 --- a/app/presenters/v3/task_presenter.rb +++ b/app/presenters/v3/task_presenter.rb @@ -15,6 +15,7 @@ def to_hash sequence_id: task.sequence_id, name: task.name, command: task.command, + user: task.run_action_user, state: task.state, memory_in_mb: task.memory_in_mb, disk_in_mb: task.disk_in_mb, diff --git a/config/cloud_controller.yml b/config/cloud_controller.yml index 04001270dd4..92f91eaf5a4 100644 --- a/config/cloud_controller.yml +++ b/config/cloud_controller.yml @@ -67,7 +67,7 @@ maximum_app_disk_in_mb: 2048 max_retained_deployments_per_app: 100 max_retained_builds_per_app: 100 max_retained_revisions_per_app: 100 -additional_allowed_process_users: ['ContainerUser'] +additional_allowed_process_users: ['ContainerUser', 'TestUser'] broker_client_default_async_poll_interval_seconds: 60 broker_client_max_async_poll_duration_minutes: 10080 diff --git a/db/migrations/20250630170610_add_user_to_tasks.rb b/db/migrations/20250630170610_add_user_to_tasks.rb new file mode 100644 index 00000000000..6328fc58e34 --- /dev/null +++ b/db/migrations/20250630170610_add_user_to_tasks.rb @@ -0,0 +1,13 @@ +Sequel.migration do + up do + alter_table :tasks do + add_column :user, String, null: true, default: nil, size: 255 unless @db.schema(:tasks).map(&:first).include?(:user) + end + end + + down do + alter_table :tasks do + drop_column :user if @db.schema(:tasks).map(&:first).include?(:user) + end + end +end diff --git a/docs/v3/source/includes/api_resources/_tasks.erb b/docs/v3/source/includes/api_resources/_tasks.erb index 33dda9c7723..90b657fb5ae 100644 --- a/docs/v3/source/includes/api_resources/_tasks.erb +++ b/docs/v3/source/includes/api_resources/_tasks.erb @@ -4,6 +4,7 @@ "sequence_id": 1, "name": "migrate", "command": "rake db:migrate", + "user": "vcap", "state": "RUNNING", "memory_in_mb": 512, "disk_in_mb": 1024, @@ -49,6 +50,7 @@ "sequence_id": 1, "name": "migrate", "command": "rake db:migrate", + "user": "vcap", "state": "CANCELING", "memory_in_mb": 512, "disk_in_mb": 1024, @@ -109,6 +111,7 @@ "guid": "d5cc22ec-99a3-4e6a-af91-a44b4ab7b6fa", "sequence_id": 1, "name": "hello", + "user": "vcap", "state": "SUCCEEDED", "memory_in_mb": 512, "disk_in_mb": 1024, @@ -150,6 +153,7 @@ "guid": "63b4cd89-fd8b-4bf1-a311-7174fcc907d6", "sequence_id": 2, "name": "migrate", + "user": "vcap", "state": "FAILED", "memory_in_mb": 512, "disk_in_mb": 1024, diff --git a/docs/v3/source/includes/resources/tasks/_create.md.erb b/docs/v3/source/includes/resources/tasks/_create.md.erb index 4366a275152..8bc7c9a6a99 100644 --- a/docs/v3/source/includes/resources/tasks/_create.md.erb +++ b/docs/v3/source/includes/resources/tasks/_create.md.erb @@ -46,16 +46,17 @@ Name | Type | Description #### Optional parameters -Name | Type | Description | Default ----- | ---- | ----------- | ------- -**name** | _string_ | Name of the task | auto-generated -**disk_in_mb**[1] | _integer_ | Amount of disk to allocate for the task in MB | operator-configured `default_app_disk_in_mb` -**memory_in_mb**[1] | _integer_ | Amount of memory to allocate for the task in MB | operator-configured `default_app_memory` -**log_rate_limit_per_second**[1] | _integer_ | Amount of log rate to allocate for the task in bytes | operator-configured `default_app_log_rate_limit_in_bytes_per_second` -**droplet_guid** | _uuid_ | The guid of the droplet that will be used to run the command | the app's current droplet -**template.process.guid** | _uuid_ | The guid of the process that will be used as a template | `null` -**metadata.labels** | [_label object_](#labels) | Labels applied to the package -**metadata.annotations** | [_annotation object_](#annotations) | Annotations applied to the package +| Name | Type | Description | Default | +|---------------------------------------------|-------------------------------------|--------------------------------------------------------------|----------------------------------------------------------------------| +| **name** | _string_ | Name of the task | auto-generated | +| **user** | _string_ | OS user used to run the task in the runtime | "vcap" | +| **disk_in_mb**[1] | _integer_ | Amount of disk to allocate for the task in MB | operator-configured `default_app_disk_in_mb` | +| **memory_in_mb**[1] | _integer_ | Amount of memory to allocate for the task in MB | operator-configured `default_app_memory` | +| **log_rate_limit_per_second**[1] | _integer_ | Amount of log rate to allocate for the task in bytes | operator-configured `default_app_log_rate_limit_in_bytes_per_second` | +| **droplet_guid** | _uuid_ | The guid of the droplet that will be used to run the command | the app's current droplet | +| **template.process.guid** | _uuid_ | The guid of the process that will be used as a template | `null` | +| **metadata.labels** | [_label object_](#labels) | Labels applied to the package | | +| **metadata.annotations** | [_annotation object_](#annotations) | Annotations applied to the package | | 1 If not provided, and a `template.process.guid` is provided, this field will use the value from the process with the given guid. diff --git a/docs/v3/source/includes/resources/tasks/_object.md.erb b/docs/v3/source/includes/resources/tasks/_object.md.erb index 6d5a9340c13..017e590410b 100644 --- a/docs/v3/source/includes/resources/tasks/_object.md.erb +++ b/docs/v3/source/includes/resources/tasks/_object.md.erb @@ -15,6 +15,7 @@ Name | Type | Description **sequence_id** | _integer_ | User-facing id of the task; this number is unique for every task associated with a given app **name** | _string_ | Name of the task **command** | _string_ | Command that will be executed; this field may be excluded based on a user's role +**user** | _string_ or _null_ | The OS user used to run the task in the runtime **state** | _string_ | State of the task Possible states are `PENDING`, `RUNNING`, `SUCCEEDED`, `CANCELING`, and `FAILED` **memory_in_mb** | _integer_ | Amount of memory to allocate for the task in MB **disk_in_mb** | _integer_ | Amount of disk to allocate for the task in MB diff --git a/lib/cloud_controller/config_schemas/base/api_schema.rb b/lib/cloud_controller/config_schemas/base/api_schema.rb index 84161f23e2f..4374a2c6054 100644 --- a/lib/cloud_controller/config_schemas/base/api_schema.rb +++ b/lib/cloud_controller/config_schemas/base/api_schema.rb @@ -45,7 +45,7 @@ class ApiSchema < VCAP::Config maximum_app_disk_in_mb: Integer, default_health_check_timeout: Integer, maximum_health_check_timeout: Integer, - optional(:additional_allowed_process_users) => Array, + additional_allowed_process_users: Array, instance_file_descriptor_limit: Integer, diff --git a/lib/cloud_controller/config_schemas/base/clock_schema.rb b/lib/cloud_controller/config_schemas/base/clock_schema.rb index 34aea008bc8..1470de05d68 100644 --- a/lib/cloud_controller/config_schemas/base/clock_schema.rb +++ b/lib/cloud_controller/config_schemas/base/clock_schema.rb @@ -157,7 +157,7 @@ class ClockSchema < VCAP::Config max_retained_deployments_per_app: Integer, max_retained_builds_per_app: Integer, max_retained_revisions_per_app: Integer, - optional(:additional_allowed_process_users) => Array, + additional_allowed_process_users: Array, diego_sync: { frequency_in_seconds: Integer }, diff --git a/lib/cloud_controller/config_schemas/base/deployment_updater_schema.rb b/lib/cloud_controller/config_schemas/base/deployment_updater_schema.rb index 2e498b3126a..d160f712d8e 100644 --- a/lib/cloud_controller/config_schemas/base/deployment_updater_schema.rb +++ b/lib/cloud_controller/config_schemas/base/deployment_updater_schema.rb @@ -58,7 +58,7 @@ class DeploymentUpdaterSchema < VCAP::Config default_app_disk_in_mb: Integer, maximum_app_disk_in_mb: Integer, instance_file_descriptor_limit: Integer, - optional(:additional_allowed_process_users) => Array, + additional_allowed_process_users: Array, deployment_updater: { update_frequency_in_seconds: Integer, diff --git a/lib/cloud_controller/config_schemas/base/worker_schema.rb b/lib/cloud_controller/config_schemas/base/worker_schema.rb index 80f599b846e..c669ab87e62 100644 --- a/lib/cloud_controller/config_schemas/base/worker_schema.rb +++ b/lib/cloud_controller/config_schemas/base/worker_schema.rb @@ -167,7 +167,7 @@ class WorkerSchema < VCAP::Config maximum_app_disk_in_mb: Integer, default_app_log_rate_limit_in_bytes_per_second: Integer, default_app_ssh_access: bool, - optional(:additional_allowed_process_users) => Array, + additional_allowed_process_users: Array, jobs: { global: { diff --git a/lib/cloud_controller/diego/buildpack/lifecycle_protocol.rb b/lib/cloud_controller/diego/buildpack/lifecycle_protocol.rb index 4305a62581d..340e91abfe6 100644 --- a/lib/cloud_controller/diego/buildpack/lifecycle_protocol.rb +++ b/lib/cloud_controller/diego/buildpack/lifecycle_protocol.rb @@ -13,7 +13,7 @@ def staging_action_builder(config, staging_details) end def task_action_builder(config, task) - TaskActionBuilder.new(config, task, task_lifecycle_data(task), 'vcap', ['app', task.command, ''], 'buildpack') + TaskActionBuilder.new(config, task, task_lifecycle_data(task), task.run_action_user, ['app', task.command, ''], 'buildpack') end def desired_lrp_builder(config, process) diff --git a/lib/cloud_controller/diego/cnb/lifecycle_protocol.rb b/lib/cloud_controller/diego/cnb/lifecycle_protocol.rb index ae095673b8b..9bce82bf361 100644 --- a/lib/cloud_controller/diego/cnb/lifecycle_protocol.rb +++ b/lib/cloud_controller/diego/cnb/lifecycle_protocol.rb @@ -15,7 +15,7 @@ def staging_action_builder(config, staging_details) end def task_action_builder(config, task) - VCAP::CloudController::Diego::Buildpack::TaskActionBuilder.new(config, task, task_lifecycle_data(task), 'root', ['--', task.command], 'cnb') + VCAP::CloudController::Diego::Buildpack::TaskActionBuilder.new(config, task, task_lifecycle_data(task), task.run_action_user, ['--', task.command], 'cnb') end def desired_lrp_builder(config, process) diff --git a/lib/cloud_controller/diego/docker/task_action_builder.rb b/lib/cloud_controller/diego/docker/task_action_builder.rb index 2cbdd77b347..eb9dc36886b 100644 --- a/lib/cloud_controller/diego/docker/task_action_builder.rb +++ b/lib/cloud_controller/diego/docker/task_action_builder.rb @@ -18,7 +18,7 @@ def action ::Diego::ActionBuilder.action( ::Diego::Bbs::Models::RunAction.new( - user: 'root', + user: task.run_action_user, path: '/tmp/lifecycle/launcher', args: launcher_args, log_source: "APP/TASK/#{task.name}", diff --git a/spec/migrations/20250630170610_add_user_to_tasks_spec.rb b/spec/migrations/20250630170610_add_user_to_tasks_spec.rb new file mode 100644 index 00000000000..6d52d8bfca2 --- /dev/null +++ b/spec/migrations/20250630170610_add_user_to_tasks_spec.rb @@ -0,0 +1,55 @@ +require 'spec_helper' +require 'migrations/helpers/migration_shared_context' + +RSpec.describe 'migration to add user column to tasks table', isolation: :truncation, type: :migration do + include_context 'migration' do + let(:migration_filename) { '20250630170610_add_user_to_tasks.rb' } + end + + describe 'tasks table' do + it 'adds a column `user`' do + expect(db[:tasks].columns).not_to include(:user) + expect { Sequel::Migrator.run(db, migrations_path, target: current_migration_index, allow_missing_migration_files: true) }.not_to raise_error + expect(db[:tasks].columns).to include(:user) + end + + describe 'idempotency of up' do + context '`user` column already exists' do + before do + db.add_column :tasks, :user, String, size: 255, if_not_exists: true + end + + it 'does not fail' do + expect(db[:tasks].columns).to include(:user) + expect { Sequel::Migrator.run(db, migrations_path, target: current_migration_index, allow_missing_migration_files: true) }.not_to raise_error + end + end + end + + describe 'idempotency of down' do + context 'user column exists' do + before do + Sequel::Migrator.run(db, migrations_path, target: current_migration_index, allow_missing_migration_files: true) + end + + it 'continues to remove the `user_guid` column' do + expect(db[:tasks].columns).to include(:user) + expect { Sequel::Migrator.run(db, migrations_path, target: current_migration_index - 1, allow_missing_migration_files: true) }.not_to raise_error + expect(db[:tasks].columns).not_to include(:user) + end + end + + context 'column does not exist' do + before do + Sequel::Migrator.run(db, migrations_path, target: current_migration_index, allow_missing_migration_files: true) + db.drop_column :tasks, :user + end + + it 'does not fail' do + expect(db[:tasks].columns).not_to include(:user) + expect { Sequel::Migrator.run(db, migrations_path, target: current_migration_index - 1, allow_missing_migration_files: true) }.not_to raise_error + end + end + end + end +end diff --git a/spec/request/tasks_spec.rb b/spec/request/tasks_spec.rb index dff0094d06f..8fa34e57a70 100644 --- a/spec/request/tasks_spec.rb +++ b/spec/request/tasks_spec.rb @@ -103,6 +103,7 @@ task1 = VCAP::CloudController::TaskModel.make( name: 'task one', command: 'echo task', + user: 'TestUser', app_guid: app_model.guid, droplet: app_model.droplet, memory_in_mb: 5, @@ -142,6 +143,7 @@ 'guid' => task1.guid, 'sequence_id' => task1.sequence_id, 'name' => 'task one', + 'user' => 'TestUser', 'state' => 'RUNNING', 'memory_in_mb' => 5, 'disk_in_mb' => 10, @@ -174,6 +176,7 @@ 'guid' => task2.guid, 'sequence_id' => task2.sequence_id, 'name' => 'task two', + 'user' => 'vcap', 'state' => 'RUNNING', 'memory_in_mb' => 100, 'disk_in_mb' => 500, @@ -213,6 +216,7 @@ VCAP::CloudController::TaskModel.make( name: 'task one', command: 'echo task', + user: 'TestUser', app_guid: app_model.guid, droplet: app_model.droplet ) @@ -236,6 +240,7 @@ 'guid' => task1.guid, 'sequence_id' => task1.sequence_id, 'name' => 'task one', + 'user' => 'TestUser', 'state' => 'RUNNING', 'memory_in_mb' => 256, 'disk_in_mb' => nil, @@ -268,6 +273,7 @@ 'guid' => task2.guid, 'sequence_id' => task2.sequence_id, 'name' => 'task two', + 'user' => 'vcap', 'state' => 'RUNNING', 'memory_in_mb' => 5, 'disk_in_mb' => 10, @@ -429,6 +435,7 @@ 'guid' => task.guid, 'sequence_id' => task.sequence_id, 'name' => 'task', + 'user' => 'vcap', 'state' => 'RUNNING', 'memory_in_mb' => 5, 'disk_in_mb' => 50, @@ -543,6 +550,7 @@ 'sequence_id' => task.sequence_id, 'name' => 'task', 'command' => 'echo task', + 'user' => 'vcap', 'state' => 'RUNNING', 'memory_in_mb' => 5, 'disk_in_mb' => 50, @@ -621,6 +629,7 @@ sequence_id: task.sequence_id, name: 'task', command: 'echo task', + user: 'vcap', state: 'CANCELING', memory_in_mb: 256, disk_in_mb: nil, @@ -737,6 +746,7 @@ 'sequence_id' => task1.sequence_id, 'name' => 'task one', 'command' => 'echo task', + 'user' => 'vcap', 'state' => 'RUNNING', 'memory_in_mb' => 5, 'disk_in_mb' => 50, @@ -770,6 +780,7 @@ 'sequence_id' => task2.sequence_id, 'name' => 'task two', 'command' => 'echo task', + 'user' => 'vcap', 'state' => 'RUNNING', 'memory_in_mb' => 100, 'disk_in_mb' => 500, @@ -836,6 +847,7 @@ 'guid' => task1.guid, 'sequence_id' => task1.sequence_id, 'name' => 'task one', + 'user' => 'vcap', 'state' => 'RUNNING', 'memory_in_mb' => 256, 'disk_in_mb' => nil, @@ -868,6 +880,7 @@ 'guid' => task2.guid, 'sequence_id' => task2.sequence_id, 'name' => 'task two', + 'user' => 'vcap', 'state' => 'RUNNING', 'memory_in_mb' => 5, 'disk_in_mb' => 10, @@ -1023,6 +1036,7 @@ { name: 'best task ever', command: 'be rake && true', + user: 'TestUser', memory_in_mb: 1234, disk_in_mb: 1000, log_rate_limit_in_bytes_per_second: task_log_rate_limit_in_bytes_per_second, @@ -1056,6 +1070,7 @@ 'sequence_id' => sequence_id, 'name' => 'best task ever', 'command' => 'be rake && true', + 'user' => 'TestUser', 'state' => 'RUNNING', 'memory_in_mb' => 1234, 'disk_in_mb' => 1000, @@ -1207,7 +1222,7 @@ end context 'when the client specifies a template' do - let(:process) { VCAP::CloudController::ProcessModel.make(app: app_model, command: 'start') } + let(:process) { VCAP::CloudController::ProcessModel.make(app: app_model, command: 'start', user: 'TestUser') } it 'uses the command from the template process' do body = { @@ -1231,6 +1246,7 @@ 'sequence_id' => sequence_id, 'name' => 'best task ever', 'command' => process.command, + 'user' => 'TestUser', 'state' => 'RUNNING', 'memory_in_mb' => 1234, 'disk_in_mb' => 1000, diff --git a/spec/support/fakes/blueprints.rb b/spec/support/fakes/blueprints.rb index 911a91ee61b..86a528857b0 100644 --- a/spec/support/fakes/blueprints.rb +++ b/spec/support/fakes/blueprints.rb @@ -73,6 +73,7 @@ module VCAP::CloudController name { Sham.name } space { Space.make } buildpack_lifecycle_data { nil.tap { |_| object.save } } + cnb_lifecycle_data { nil.tap { |_| object.save } } end BuildModel.blueprint do @@ -217,6 +218,19 @@ module VCAP::CloudController failure_reason {} end + TaskModel.blueprint(:docker) do + guid { Sham.guid } + app { AppModel.make(:docker) } + name { Sham.name } + droplet { DropletModel.make(:docker, app:) } + command { 'bundle exec rake' } + state { VCAP::CloudController::TaskModel::RUNNING_STATE } + memory_in_mb { 256 } + disk_in_mb {} + sequence_id { Sham.sequence_id } + failure_reason {} + end + TaskModel.blueprint(:cnb) do guid { Sham.guid } app { AppModel.make(:cnb) } diff --git a/spec/unit/actions/task_create_spec.rb b/spec/unit/actions/task_create_spec.rb index 3b0eeb10f64..762d00ee6e0 100644 --- a/spec/unit/actions/task_create_spec.rb +++ b/spec/unit/actions/task_create_spec.rb @@ -296,6 +296,38 @@ module VCAP::CloudController describe 'process templates' do let(:process) { VCAP::CloudController::ProcessModel.make(app: app, command: 'start') } + describe 'users' do + context 'when there is a template and no user provided' do + let(:process) { VCAP::CloudController::ProcessModel.make(app: app, user: 'ContainerUser') } + let(:message) { TaskCreateMessage.new name: name, disk_in_mb: 2048, memory_in_mb: 1024, template: { process: { guid: process.guid } } } + + it 'uses the user from the template' do + task = task_create_action.create(app, message, user_audit_info) + expect(task.user).to eq('ContainerUser') + end + end + + context 'when there is a template and a user provided' do + let(:process) { VCAP::CloudController::ProcessModel.make(app: app, user: 'ContainerUser') } + let(:message) { TaskCreateMessage.new name: name, user: 'vcap', template: { process: { guid: process.guid } } } + + it 'uses the user from the message' do + task = task_create_action.create(app, message, user_audit_info) + expect(task.user).to eq('vcap') + end + end + + context 'when there is a template without a user and no user is provided' do + let(:process) { VCAP::CloudController::ProcessModel.make(app:) } + let(:message) { TaskCreateMessage.new name: name, template: { process: { guid: process.guid } } } + + it 'uses the user from the message' do + task = task_create_action.create(app, message, user_audit_info) + expect(task.user).to be_nil + end + end + end + describe 'commands' do context 'when there is a template and no command provided' do let(:message) { TaskCreateMessage.new name: name, disk_in_mb: 2048, memory_in_mb: 1024, template: { process: { guid: process.guid } } } diff --git a/spec/unit/lib/cloud_controller/diego/buildpack/lifecycle_protocol_spec.rb b/spec/unit/lib/cloud_controller/diego/buildpack/lifecycle_protocol_spec.rb index d057e726757..5ba4fd83ba0 100644 --- a/spec/unit/lib/cloud_controller/diego/buildpack/lifecycle_protocol_spec.rb +++ b/spec/unit/lib/cloud_controller/diego/buildpack/lifecycle_protocol_spec.rb @@ -166,7 +166,7 @@ module Buildpack end describe '#task_action_builder' do - let(:task) { TaskModel.make } + let(:task) { TaskModel.make(user: 'ContainerUser') } let(:config) { Config.new({ some: 'config' }) } it 'returns a TaskActionBuilder' do @@ -181,7 +181,7 @@ module Buildpack droplet_uri: 'droplet-download-url', stack: 'potato-stack' }, - 'vcap', + 'ContainerUser', ['app', task.command, ''], 'buildpack') end diff --git a/spec/unit/lib/cloud_controller/diego/cnb/lifecycle_protocol_spec.rb b/spec/unit/lib/cloud_controller/diego/cnb/lifecycle_protocol_spec.rb index bf676b596fd..4b7b5eb8334 100644 --- a/spec/unit/lib/cloud_controller/diego/cnb/lifecycle_protocol_spec.rb +++ b/spec/unit/lib/cloud_controller/diego/cnb/lifecycle_protocol_spec.rb @@ -175,6 +175,31 @@ module CNB 'cnb') end + context 'when the task has a user specified' do + let(:task) { TaskModel.make(:cnb, user: 'TestUser') } + + it 'sets the appropriate user' do + task.app.update(cnb_lifecycle_data: CNBLifecycleDataModel.make(stack: 'potato-stack')) + + task_action_builder = instance_double(Buildpack::TaskActionBuilder) + allow(Buildpack::TaskActionBuilder).to receive(:new).and_return task_action_builder + + expect(lifecycle_protocol.task_action_builder(config, task)).to be task_action_builder + + expect(Buildpack::TaskActionBuilder).to have_received(:new).with( + config, + task, + { + droplet_uri: 'droplet-download-url', + stack: 'potato-stack' + }, + 'TestUser', + ['--', task.command], + 'cnb' + ) + end + end + context 'when the blobstore_url_generator returns nil' do let(:droplet_download_url) { nil } diff --git a/spec/unit/lib/cloud_controller/diego/docker/task_action_builder_spec.rb b/spec/unit/lib/cloud_controller/diego/docker/task_action_builder_spec.rb index 40a795e63e4..f40fdee9fb7 100644 --- a/spec/unit/lib/cloud_controller/diego/docker/task_action_builder_spec.rb +++ b/spec/unit/lib/cloud_controller/diego/docker/task_action_builder_spec.rb @@ -17,7 +17,7 @@ module Docker }) end - let(:task) { TaskModel.make command: command, name: 'my-task' } + let(:task) { TaskModel.make(:docker, command: command, name: 'my-task') } let(:lifecycle_data) do { droplet_path: 'user/image' diff --git a/spec/unit/messages/task_create_message_spec.rb b/spec/unit/messages/task_create_message_spec.rb index 87818515861..fd376c1822f 100644 --- a/spec/unit/messages/task_create_message_spec.rb +++ b/spec/unit/messages/task_create_message_spec.rb @@ -281,6 +281,59 @@ module VCAP::CloudController end end end + + context 'when user is not a string' do + let(:params) { { user: 32.77 } } + + it 'is not valid' do + message = ProcessUpdateMessage.new(params) + + expect(message).not_to be_valid + expect(message.errors[:user]).to include('must be a string') + end + end + + context 'when user is nil' do + let(:params) { { user: nil } } + + it 'is valid' do + message = ProcessUpdateMessage.new(params) + + expect(message).to be_valid + end + end + + context 'when user is too long' do + let(:params) { { user: 'a' * 256 } } + + it 'is not valid' do + message = ProcessUpdateMessage.new(params) + + expect(message).not_to be_valid + expect(message.errors[:user]).to include('must be between 1 and 255 characters') + end + end + + context 'when user is empty' do + let(:params) { { user: '' } } + + it 'is not valid' do + message = ProcessUpdateMessage.new(params) + + expect(message).not_to be_valid + expect(message.errors[:user]).to include('must be between 1 and 255 characters') + end + end + + context 'when user is just right' do + let(:params) { { user: 'vcap' } } + + it 'is not valid' do + message = ProcessUpdateMessage.new(params) + + expect(message).to be_valid + end + end end end end diff --git a/spec/unit/models/runtime/droplet_model_spec.rb b/spec/unit/models/runtime/droplet_model_spec.rb index b72ed263c2a..14264dbaf11 100644 --- a/spec/unit/models/runtime/droplet_model_spec.rb +++ b/spec/unit/models/runtime/droplet_model_spec.rb @@ -345,5 +345,58 @@ module VCAP::CloudController expect(droplet.annotations.first.value).to eq('asparagus') end end + + describe '#docker_user' do + let(:droplet_model) { DropletModel.make } + + context 'when the droplet DOES NOT belong to a Docker lifecycle app' do + it 'returns an empty string' do + expect(droplet_model.docker_user).to eq('') + end + end + + context 'when the droplet belongs to a Docker lifecycle app' do + let(:droplet_execution_metadata) { '{"entrypoint":["/image-entrypoint.sh"],"user":"cnb"}' } + let(:droplet_model) { DropletModel.make(:docker, execution_metadata: droplet_execution_metadata) } + + context 'when the droplet execution metadata specifies a user' do + it 'returns the specified user' do + expect(droplet_model.docker_user).to eq('cnb') + end + end + + context 'when the droplet execution metadata DOES NOT specify a user' do + let(:droplet_execution_metadata) { '{"entrypoint":["/image-entrypoint.sh"]}' } + + it 'defaults the user to root' do + expect(droplet_model.docker_user).to eq('root') + end + end + + context 'when the droplet execution metadata is an empty string' do + let(:droplet_execution_metadata) { '' } + + it 'defaults the user to root' do + expect(droplet_model.docker_user).to eq('root') + end + end + + context 'when the droplet execution metadata is nil' do + let(:droplet_execution_metadata) { nil } + + it 'defaults the user to root' do + expect(droplet_model.docker_user).to eq('root') + end + end + + context 'when the droplet execution metadata has invalid json' do + let(:droplet_execution_metadata) { '{' } + + it 'defaults the user to root' do + expect(droplet_model.docker_user).to eq('root') + end + end + end + end end end diff --git a/spec/unit/models/runtime/process_model_spec.rb b/spec/unit/models/runtime/process_model_spec.rb index 95c8b1270d1..40c4b7951e7 100644 --- a/spec/unit/models/runtime/process_model_spec.rb +++ b/spec/unit/models/runtime/process_model_spec.rb @@ -369,7 +369,7 @@ def expect_no_validator(validator_class) context 'when user is not permitted' do let(:process_user) { 'some-random-user' } - it 'does raises an error' do + it 'raises an error' do expect { process.save }.to raise_error(/user invalid/) end end @@ -679,10 +679,12 @@ def act_as_cf_admin context 'when the process belongs to a Docker lifecycle app' do subject(:process) { ProcessModelFactory.make({ docker_image: 'example.com/image' }) } - let(:droplet_execution_metadata) { '{"entrypoint":["/cnb/lifecycle/launcher"],"user":"cnb"}' } + let(:droplet_execution_metadata) { '{"entrypoint":["/image-entrypoint.sh"],"user":"some-user"}' } before do process.desired_droplet.update(execution_metadata: droplet_execution_metadata) + process.desired_droplet.buildpack_lifecycle_data.delete + process.desired_droplet.reload end context 'when the process has a user specified' do @@ -697,12 +699,12 @@ def act_as_cf_admin context 'when the droplet execution metadata specifies a user' do it 'returns the specified user' do - expect(process.run_action_user).to eq('cnb') + expect(process.run_action_user).to eq('some-user') end end context 'when the droplet execution metadata DOES NOT specify a user' do - let(:droplet_execution_metadata) { '{"entrypoint":["/cnb/lifecycle/launcher"]}' } + let(:droplet_execution_metadata) { '{"entrypoint":["/image-entrypoint.sh"]}' } it 'defaults the user to root' do expect(process.run_action_user).to eq('root') @@ -732,6 +734,17 @@ def act_as_cf_admin expect(process.run_action_user).to eq('root') end end + + context 'when the app does not have a droplet assigned' do + before do + process.app.update(droplet: nil) + process.reload + end + + it 'defaults the user to root' do + expect(process.run_action_user).to eq('root') + end + end end context 'when the process DOES NOT belong to a Docker lifecycle app' do diff --git a/spec/unit/models/runtime/task_model_spec.rb b/spec/unit/models/runtime/task_model_spec.rb index 6794627ea0a..cf0e65d5a9b 100644 --- a/spec/unit/models/runtime/task_model_spec.rb +++ b/spec/unit/models/runtime/task_model_spec.rb @@ -164,6 +164,114 @@ module VCAP::CloudController end end + describe '#run_action_user' do + let(:task) { TaskModel.make(app: parent_app, droplet: droplet, state: TaskModel::SUCCEEDED_STATE) } + let(:droplet) { DropletModel.make } + + context 'when the task belongs to a CNB lifecycle app' do + let(:parent_app) { AppModel.make(:cnb) } + let(:task) { TaskModel.make(app: parent_app, droplet: droplet, state: TaskModel::SUCCEEDED_STATE) } + let(:droplet) { DropletModel.make(:cnb) } + + context 'when the task has a user specified' do + before do + task.update(user: 'TestUser') + end + + it 'returns the user' do + expect(task.run_action_user).to eq('TestUser') + end + end + + context 'when the task does not have a user specified' do + before do + task.update(user: nil) + end + + it 'defaults the user to root' do + expect(task.run_action_user).to eq('root') + end + end + end + + context 'when the task belongs to a Docker lifecycle app' do + let(:parent_app) { AppModel.make(:docker) } + let(:task) { TaskModel.make(app: parent_app, droplet: droplet, state: TaskModel::SUCCEEDED_STATE) } + let(:droplet) { DropletModel.make(:docker) } + let(:droplet_execution_metadata) { '{"entrypoint":["/image-entrypoint.sh"],"user":"cnb"}' } + + before do + task.droplet.update(execution_metadata: droplet_execution_metadata) + end + + context 'when the task has a user specified' do + before do + task.update(user: 'ContainerUser') + end + + it 'returns the user' do + expect(task.run_action_user).to eq('ContainerUser') + end + end + + context 'when the droplet execution metadata specifies a user' do + it 'returns the specified user' do + expect(task.run_action_user).to eq('cnb') + end + end + + context 'when the droplet execution metadata DOES NOT specify a user' do + let(:droplet_execution_metadata) { '{"entrypoint":["/image-entrypoint.sh"]}' } + + it 'defaults the user to root' do + expect(task.run_action_user).to eq('root') + end + end + + context 'when the droplet execution metadata is an empty string' do + let(:droplet_execution_metadata) { '' } + + it 'defaults the user to root' do + expect(task.run_action_user).to eq('root') + end + end + + context 'when the droplet execution metadata is nil' do + let(:droplet_execution_metadata) { nil } + + it 'defaults the user to root' do + expect(task.run_action_user).to eq('root') + end + end + + context 'when the droplet execution metadata has invalid json' do + let(:droplet_execution_metadata) { '{' } + + it 'defaults the user to root' do + expect(task.run_action_user).to eq('root') + end + end + end + + context 'when the task DOES NOT belong to a Docker lifecycle app' do + context 'when the task has a user specified' do + before do + task.update(user: 'ContainerUser') + end + + it 'returns the user' do + expect(task.run_action_user).to eq('ContainerUser') + end + end + + context 'when the task DOES NOT have a user specified' do + it 'returns the default "vcap" user' do + expect(task.run_action_user).to eq('vcap') + end + end + end + end + describe 'validations' do let(:task) { TaskModel.make } let(:org) { Organization.make } @@ -499,6 +607,49 @@ module VCAP::CloudController end end + describe 'user' do + subject(:task) { TaskModel.make(user: task_user) } + let(:task_user) { 'vcap' } + + before do + TestConfig.override(additional_allowed_process_users: %w[some_user some_other_user]) + end + + context 'when user is vcap' do + before do + TestConfig.override(additional_allowed_process_users: []) + end + + it 'is always permitted' do + expect { task.save }.not_to raise_error + end + end + + context 'when user is a permitted user' do + let(:task_user) { 'some_user' } + + it 'does not raise an error' do + expect { task.save }.not_to raise_error + end + end + + context 'when user is nil' do + let(:task_user) { nil } + + it 'does not raise an error' do + expect { task.save }.not_to raise_error + end + end + + context 'when user is not permitted' do + let(:task_user) { 'some-random-user' } + + it 'raises an error' do + expect { task.save }.to raise_error(/user invalid/) + end + end + end + describe 'when the quota has an app_task_limit' do let(:quota) { SpaceQuotaDefinition.make(app_task_limit: 1, organization: org) } diff --git a/spec/unit/presenters/v3/task_presenter_spec.rb b/spec/unit/presenters/v3/task_presenter_spec.rb index 467538cad97..da9f10297ab 100644 --- a/spec/unit/presenters/v3/task_presenter_spec.rb +++ b/spec/unit/presenters/v3/task_presenter_spec.rb @@ -61,6 +61,7 @@ module VCAP::CloudController::Presenters::V3 expect(result[:guid]).to eq(task.guid) expect(result[:name]).to eq(task.name) expect(result[:command]).to eq(task.command) + expect(result[:user]).to eq(task.run_action_user) expect(result[:state]).to eq(task.state) expect(result[:result][:failure_reason]).to eq 'sup dawg' expect(result[:memory_in_mb]).to eq(task.memory_in_mb)