Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions app/actions/task_create.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -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
Expand Down
11 changes: 10 additions & 1 deletion app/messages/task_create_message.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,29 @@

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

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?
Expand Down
1 change: 1 addition & 0 deletions app/models/runtime/app_model.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ module VCAP::CloudController
class AppModel < Sequel::Model(:apps)
include Serializer
APP_NAME_REGEX = /\A[[:alnum:][:punct:][:print:]]+\Z/
DEFAULT_USER = 'vcap'.freeze
Comment thread
tcdowney marked this conversation as resolved.
Outdated

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
Expand Down
16 changes: 16 additions & 0 deletions app/models/runtime/droplet_model.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 || 'root'
end

def staging?
state == STAGING_STATE
end
Expand Down
19 changes: 3 additions & 16 deletions app/models/runtime/process_model.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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_USER
end

def specified_or_detected_command
Expand Down Expand Up @@ -573,23 +572,11 @@ def open_ports
private

def permitted_users
Set.new([DEFAULT_USER]) + Config.config.get(:additional_allowed_process_users)
Set.new([AppModel::DEFAULT_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

container_user.presence || 'root'
desired_droplet.docker_user.presence || AppModel::DEFAULT_USER
end

def non_unique_process_types
Expand Down
17 changes: 17 additions & 0 deletions app/models/runtime/task_model.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,24 @@ def after_destroy
create_stop_event unless terminal_state?
end

def run_action_user
return user if user.present?

docker? ? docker_run_action_user : AppModel::DEFAULT_USER
end

delegate :docker?, to: :droplet

private

def permitted_users
Set.new([AppModel::DEFAULT_USER]) + Config.config.get(:additional_allowed_process_users)
end

def docker_run_action_user
droplet.docker_user.presence || AppModel::DEFAULT_USER
end

def running_state?
state == RUNNING_STATE
end
Expand All @@ -68,6 +84,7 @@ def validate
validate_org_quotas
validate_space_quotas

ProcessUserPolicy.new(self, permitted_users).validate
MinLogRateLimitPolicy.new(self).validate
end

Expand Down
1 change: 1 addition & 0 deletions app/presenters/v3/task_presenter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion config/cloud_controller.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
13 changes: 13 additions & 0 deletions db/migrations/20250630170610_add_user_to_tasks.rb
Original file line number Diff line number Diff line change
@@ -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
4 changes: 4 additions & 0 deletions docs/v3/source/includes/api_resources/_tasks.erb
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
21 changes: 11 additions & 10 deletions docs/v3/source/includes/resources/tasks/_create.md.erb
Original file line number Diff line number Diff line change
Expand Up @@ -46,16 +46,17 @@ Name | Type | Description

#### Optional parameters

Name | Type | Description | Default
---- | ---- | ----------- | -------
**name** | _string_ | Name of the task | auto-generated
**disk_in_mb**<sup>[1]</sup> | _integer_ | Amount of disk to allocate for the task in MB | operator-configured `default_app_disk_in_mb`
**memory_in_mb**<sup>[1]</sup> | _integer_ | Amount of memory to allocate for the task in MB | operator-configured `default_app_memory`
**log_rate_limit_per_second**<sup>[1]</sup> | _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_ | User used to run the task | "vcap" |
Comment thread
tcdowney marked this conversation as resolved.
Outdated
| **disk_in_mb**<sup>[1]</sup> | _integer_ | Amount of disk to allocate for the task in MB | operator-configured `default_app_disk_in_mb` |
| **memory_in_mb**<sup>[1]</sup> | _integer_ | Amount of memory to allocate for the task in MB | operator-configured `default_app_memory` |
| **log_rate_limit_per_second**<sup>[1]</sup> | _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 | |

<sup>1 If not provided, and a `template.process.guid` is provided, this field will use the value from the process with the given guid.</sup>

Expand Down
1 change: 1 addition & 0 deletions docs/v3/source/includes/resources/tasks/_object.md.erb
Original file line number Diff line number Diff line change
Expand Up @@ -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 user used to run the task
**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
Expand Down
2 changes: 1 addition & 1 deletion lib/cloud_controller/config_schemas/base/api_schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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,

Expand Down
2 changes: 1 addition & 1 deletion lib/cloud_controller/config_schemas/base/clock_schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 },

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion lib/cloud_controller/config_schemas/base/worker_schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down
2 changes: 1 addition & 1 deletion lib/cloud_controller/diego/buildpack/lifecycle_protocol.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Comment thread
tcdowney marked this conversation as resolved.
end

def desired_lrp_builder(config, process)
Expand Down
2 changes: 1 addition & 1 deletion lib/cloud_controller/diego/docker/task_action_builder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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}",
Expand Down
55 changes: 55 additions & 0 deletions spec/migrations/20250630170610_add_user_to_tasks_spec.rb
Original file line number Diff line number Diff line change
@@ -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
Loading