From 5d6d2e1bddc7355dc427a55b1c085e604a298b12 Mon Sep 17 00:00:00 2001 From: Tim Downey Date: Wed, 11 Jun 2025 13:42:21 -0600 Subject: [PATCH] Support overriding the user on Processes Issue: https://github.com/cloudfoundry/cloud_controller_ng/issues/4372 --- app/actions/deployment_create.rb | 1 + app/actions/process_update.rb | 1 + app/messages/process_update_message.rb | 12 ++- app/models.rb | 1 + .../constraints/process_user_policy.rb | 26 +++++ app/models/runtime/process_model.rb | 30 +++++- app/presenters/v3/process_presenter.rb | 1 + config/cloud_controller.yml | 1 + .../20250610212414_add_user_to_processes.rb | 13 +++ .../includes/api_resources/_processes.erb | 3 + .../resources/processes/_object.md.erb | 1 + .../config_schemas/base/api_schema.rb | 1 + .../config_schemas/base/clock_schema.rb | 1 + .../base/deployment_updater_schema.rb | 1 + .../config_schemas/base/worker_schema.rb | 1 + .../diego/buildpack/desired_lrp_builder.rb | 7 +- .../diego/cnb/desired_lrp_builder.rb | 7 +- .../diego/docker/desired_lrp_builder.rb | 13 +-- .../diego/docker/lifecycle_protocol.rb | 1 + .../diego/lifecycle_protocol.rb | 1 + ...250610212414_add_user_to_processes_spec.rb | 55 +++++++++++ spec/request/processes_spec.rb | 13 +++ spec/unit/actions/deployment_create_spec.rb | 1 + spec/unit/actions/process_update_spec.rb | 5 + .../buildpack/desired_lrp_builder_spec.rb | 1 + .../buildpack/lifecycle_protocol_spec.rb | 1 + .../diego/cnb/desired_lrp_builder_spec.rb | 1 + .../diego/cnb/lifecycle_protocol_spec.rb | 1 + .../diego/docker/desired_lrp_builder_spec.rb | 13 +-- .../diego/docker/lifecycle_protocol_spec.rb | 3 +- .../messages/process_update_message_spec.rb | 53 ++++++++++ .../constraints/process_user_policy_spec.rb | 57 +++++++++++ .../unit/models/runtime/process_model_spec.rb | 99 +++++++++++++++++++ .../presenters/v3/process_presenter_spec.rb | 22 +++++ 34 files changed, 410 insertions(+), 38 deletions(-) create mode 100644 app/models/runtime/constraints/process_user_policy.rb create mode 100644 db/migrations/20250610212414_add_user_to_processes.rb create mode 100644 spec/migrations/20250610212414_add_user_to_processes_spec.rb create mode 100644 spec/unit/models/runtime/constraints/process_user_policy_spec.rb diff --git a/app/actions/deployment_create.rb b/app/actions/deployment_create.rb index 0d2b3727490..67a92c6c606 100644 --- a/app/actions/deployment_create.rb +++ b/app/actions/deployment_create.rb @@ -113,6 +113,7 @@ def clone_existing_web_process(app, revision, process_instances) state: ProcessModel::STOPPED, instances: process_instances, command: command, + user: web_process.user, memory: web_process.memory, file_descriptors: web_process.file_descriptors, disk_quota: web_process.disk_quota, diff --git a/app/actions/process_update.rb b/app/actions/process_update.rb index f216f02baa8..826bd939d68 100644 --- a/app/actions/process_update.rb +++ b/app/actions/process_update.rb @@ -31,6 +31,7 @@ def update(process, message, strategy_class) end process.command = strategy.updated_command if message.requested?(:command) + process.user = message.user if message.requested?(:user) process.health_check_timeout = message.health_check_timeout if message.requested?(:health_check_timeout) process.health_check_invocation_timeout = message.health_check_invocation_timeout if message.requested?(:health_check_invocation_timeout) diff --git a/app/messages/process_update_message.rb b/app/messages/process_update_message.rb index f252179a610..115d7a0209a 100644 --- a/app/messages/process_update_message.rb +++ b/app/messages/process_update_message.rb @@ -3,7 +3,7 @@ module VCAP::CloudController class ProcessUpdateMessage < MetadataBaseMessage - register_allowed_keys %i[command health_check readiness_health_check] + register_allowed_keys %i[command user health_check readiness_health_check] # rubocop:disable Metrics/CyclomaticComplexity def initialize(params={}) @@ -25,6 +25,10 @@ def self.command_requested? @command_requested ||= proc { |a| a.requested?(:command) } end + def self.user_requested? + @user_requested ||= proc { |a| a.requested?(:user) } + end + validates_with NoAdditionalKeysValidator validates :command, @@ -33,6 +37,12 @@ def self.command_requested? allow_nil: true, if: command_requested? + validates :user, + string: true, + length: { in: 1..255, message: 'must be between 1 and 255 characters' }, + allow_nil: true, + if: user_requested? + validates :health_check_type, inclusion: { in: [HealthCheckTypes::PORT, HealthCheckTypes::PROCESS, HealthCheckTypes::HTTP], diff --git a/app/models.rb b/app/models.rb index 2c6840fc8e4..93e1594b38d 100644 --- a/app/models.rb +++ b/app/models.rb @@ -63,6 +63,7 @@ require 'models/runtime/constraints/readiness_health_check_policy' require 'models/runtime/constraints/docker_policy' require 'models/runtime/constraints/sidecar_memory_less_than_process_memory_policy' +require 'models/runtime/constraints/process_user_policy' require 'models/runtime/revision_model' require 'models/runtime/revision_process_command_model' require 'models/runtime/revision_sidecar_model' diff --git a/app/models/runtime/constraints/process_user_policy.rb b/app/models/runtime/constraints/process_user_policy.rb new file mode 100644 index 00000000000..8a79bc824da --- /dev/null +++ b/app/models/runtime/constraints/process_user_policy.rb @@ -0,0 +1,26 @@ +class ProcessUserPolicy + ERROR_MSG = 'invalid (requested user %s not allowed, permitted users are: %s)'.freeze + + def initialize(process, allowed_users) + @process = process + @allowed_users = allowed_users + @errors = process.errors + end + + def validate + return if @process.user.blank? + return if @allowed_users.map(&:downcase).include?(@process.user.downcase) + + @errors.add(:user, sprintf(ERROR_MSG, requested_user: quote_user(@process.user), allowed_users: formatted_users_for_error)) + end + + private + + def formatted_users_for_error + @allowed_users.map { |u| quote_user(u) }.join(', ') + end + + def quote_user(user) + "'#{user}'" + end +end diff --git a/app/models/runtime/process_model.rb b/app/models/runtime/process_model.rb index 9f4b72fc200..9acf22152cd 100644 --- a/app/models/runtime/process_model.rb +++ b/app/models/runtime/process_model.rb @@ -13,7 +13,7 @@ require_relative 'buildpack' module VCAP::CloudController - class ProcessModel < Sequel::Model(:processes) + class ProcessModel < Sequel::Model(:processes) # rubocop:disable Metrics/ClassLength include Serializer plugin :serialization @@ -34,7 +34,8 @@ def after_initialize NO_APP_PORT_SPECIFIED = -1 DEFAULT_HTTP_PORT = 8080 DEFAULT_PORTS = [DEFAULT_HTTP_PORT].freeze - UNLIMITED_LOG_RATE = -1 + 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 many_to_one :revision, class: 'VCAP::CloudController::RevisionModel', key: :revision_guid, primary_key: :guid, without_guid_generation: true @@ -267,7 +268,8 @@ def validation_policies ReadinessHealthCheckPolicy.new(self, readiness_health_check_invocation_timeout, readiness_health_check_type, readiness_health_check_http_endpoint, readiness_health_check_interval), DockerPolicy.new(self), - PortsPolicy.new(self) + PortsPolicy.new(self), + ProcessUserPolicy.new(self, permitted_users) ] end @@ -391,6 +393,12 @@ def started_command specified_commands[type] || revision.droplet&.process_start_command(type) || '' end + def run_action_user + return user if user.present? + + docker? ? docker_run_action_user : DEFAULT_USER + end + def specified_or_detected_command command.presence || detected_start_command end @@ -446,6 +454,8 @@ def debug delegate :cnb?, to: :app + delegate :windows_gmsa_credential_refs, to: :app + def database_uri service_binding_uris = service_bindings.map do |binding| binding.credentials['uri'] if binding.credentials.present? @@ -453,8 +463,6 @@ def database_uri DatabaseUriGenerator.new(service_binding_uris).database_uri end - delegate :windows_gmsa_credential_refs, to: :app - def max_app_disk_in_mb VCAP::CloudController::Config.config.get(:maximum_app_disk_in_mb) end @@ -564,6 +572,18 @@ def open_ports private + def permitted_users + Set.new([DEFAULT_USER]) + Config.config.get(:additional_allowed_process_users) + end + + def docker_run_action_user + return DEFAULT_USER unless docker? + + docker_exec_metadata = Oj.load(execution_metadata) + container_user = docker_exec_metadata['user'] + container_user.presence || 'root' + end + def non_unique_process_types return [] unless app diff --git a/app/presenters/v3/process_presenter.rb b/app/presenters/v3/process_presenter.rb index 8dce0533953..b83f955f3ba 100644 --- a/app/presenters/v3/process_presenter.rb +++ b/app/presenters/v3/process_presenter.rb @@ -28,6 +28,7 @@ def to_hash version: process.version, type: process.type, command: redact(process.specified_or_detected_command), + user: process.run_action_user, instances: process.instances, memory_in_mb: process.memory, disk_in_mb: process.disk_quota, diff --git a/config/cloud_controller.yml b/config/cloud_controller.yml index f0cbb85e6c2..04001270dd4 100644 --- a/config/cloud_controller.yml +++ b/config/cloud_controller.yml @@ -67,6 +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'] broker_client_default_async_poll_interval_seconds: 60 broker_client_max_async_poll_duration_minutes: 10080 diff --git a/db/migrations/20250610212414_add_user_to_processes.rb b/db/migrations/20250610212414_add_user_to_processes.rb new file mode 100644 index 00000000000..d9cce2da35b --- /dev/null +++ b/db/migrations/20250610212414_add_user_to_processes.rb @@ -0,0 +1,13 @@ +Sequel.migration do + up do + alter_table :processes do + add_column :user, String, null: true, default: nil, size: 255 unless @db.schema(:processes).map(&:first).include?(:user) + end + end + + down do + alter_table :processes do + drop_column :user if @db.schema(:processes).map(&:first).include?(:user) + end + end +end diff --git a/docs/v3/source/includes/api_resources/_processes.erb b/docs/v3/source/includes/api_resources/_processes.erb index f730a5bf967..9e6d102fb7f 100644 --- a/docs/v3/source/includes/api_resources/_processes.erb +++ b/docs/v3/source/includes/api_resources/_processes.erb @@ -3,6 +3,7 @@ "guid": "6a901b7c-9417-4dc1-8189-d3234aa0ab82", "type": "web", "command": "rackup", + "user": "vcap", "instances": 5, "memory_in_mb": 256, "disk_in_mb": 1024, @@ -80,6 +81,7 @@ "guid": "6a901b7c-9417-4dc1-8189-d3234aa0ab82", "type": "web", "command": "[PRIVATE DATA HIDDEN IN LISTS]", + "user": "vcap", "instances": 5, "memory_in_mb": 256, "disk_in_mb": 1024, @@ -141,6 +143,7 @@ "guid": "3fccacd9-4b02-4b96-8d02-8e865865e9eb", "type": "worker", "command": "[PRIVATE DATA HIDDEN IN LISTS]", + "user": "vcap", "instances": 1, "memory_in_mb": 256, "disk_in_mb": 1024, diff --git a/docs/v3/source/includes/resources/processes/_object.md.erb b/docs/v3/source/includes/resources/processes/_object.md.erb index 0a9218bda9b..c16305b6d60 100644 --- a/docs/v3/source/includes/resources/processes/_object.md.erb +++ b/docs/v3/source/includes/resources/processes/_object.md.erb @@ -15,6 +15,7 @@ Name | Type | Description **version** | _uuid_ | Random identifier that changes every time the process will be recreated in the runtime. **type** | _string_ | Process type; a unique identifier for processes belonging to an app **command** | _string_ or _null_ | The command used to start the process; use _null_ to revert to the buildpack-detected or procfile-provided start command +**user** | _string_ or _null_ | The user used to run the process; use _null_ to revert to the docker-detected or default 'vcap' user **instances** | _integer_ | The number of instances to run **memory_in_mb** | _integer_ | The memory in MB allocated per instance **log_rate_limit_in_bytes_per_second** | _integer_ | The log rate in bytes per second allocated per instance diff --git a/lib/cloud_controller/config_schemas/base/api_schema.rb b/lib/cloud_controller/config_schemas/base/api_schema.rb index 5ec25e1e492..84161f23e2f 100644 --- a/lib/cloud_controller/config_schemas/base/api_schema.rb +++ b/lib/cloud_controller/config_schemas/base/api_schema.rb @@ -45,6 +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, 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 85574a71986..34aea008bc8 100644 --- a/lib/cloud_controller/config_schemas/base/clock_schema.rb +++ b/lib/cloud_controller/config_schemas/base/clock_schema.rb @@ -157,6 +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, 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 6cd6fbecf9d..2e498b3126a 100644 --- a/lib/cloud_controller/config_schemas/base/deployment_updater_schema.rb +++ b/lib/cloud_controller/config_schemas/base/deployment_updater_schema.rb @@ -58,6 +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, 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 d57392f6398..80f599b846e 100644 --- a/lib/cloud_controller/config_schemas/base/worker_schema.rb +++ b/lib/cloud_controller/config_schemas/base/worker_schema.rb @@ -167,6 +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, jobs: { global: { diff --git a/lib/cloud_controller/diego/buildpack/desired_lrp_builder.rb b/lib/cloud_controller/diego/buildpack/desired_lrp_builder.rb index 949fda534fd..9cd5e72fa74 100644 --- a/lib/cloud_controller/diego/buildpack/desired_lrp_builder.rb +++ b/lib/cloud_controller/diego/buildpack/desired_lrp_builder.rb @@ -5,7 +5,7 @@ class DesiredLrpBuilder include ::Diego::ActionBuilder class InvalidStack < StandardError; end - attr_reader :start_command + attr_reader :start_command, :action_user def initialize(config, opts) @config = config @@ -17,6 +17,7 @@ def initialize(config, opts) @checksum_algorithm = opts[:checksum_algorithm] @checksum_value = opts[:checksum_value] @start_command = opts[:start_command] + @action_user = opts[:action_user] @additional_container_env_vars = opts[:additional_container_env_vars] end @@ -115,10 +116,6 @@ def port_environment_variables def privileged? @config.get(:diego, :use_privileged_containers_for_running) end - - def action_user - 'vcap' - end end end end diff --git a/lib/cloud_controller/diego/cnb/desired_lrp_builder.rb b/lib/cloud_controller/diego/cnb/desired_lrp_builder.rb index a08c2533495..6646db49193 100644 --- a/lib/cloud_controller/diego/cnb/desired_lrp_builder.rb +++ b/lib/cloud_controller/diego/cnb/desired_lrp_builder.rb @@ -5,7 +5,7 @@ class DesiredLrpBuilder include ::Diego::ActionBuilder class InvalidStack < StandardError; end - attr_reader :start_command + attr_reader :start_command, :action_user def initialize(config, opts) @config = config @@ -17,6 +17,7 @@ def initialize(config, opts) @checksum_algorithm = opts[:checksum_algorithm] @checksum_value = opts[:checksum_value] @start_command = opts[:start_command] + @action_user = opts[:action_user] @additional_container_env_vars = opts[:additional_container_env_vars] end @@ -114,10 +115,6 @@ def privileged? @config.get(:diego, :use_privileged_containers_for_running) end - def action_user - 'vcap' - end - private def default_container_env diff --git a/lib/cloud_controller/diego/docker/desired_lrp_builder.rb b/lib/cloud_controller/diego/docker/desired_lrp_builder.rb index 545bceb1080..d50bbc2b90b 100644 --- a/lib/cloud_controller/diego/docker/desired_lrp_builder.rb +++ b/lib/cloud_controller/diego/docker/desired_lrp_builder.rb @@ -2,7 +2,7 @@ module VCAP::CloudController module Diego module Docker class DesiredLrpBuilder - attr_reader :start_command + attr_reader :start_command, :action_user def initialize(config, opts) @config = config @@ -10,6 +10,7 @@ def initialize(config, opts) @execution_metadata = opts[:execution_metadata] @ports = opts[:ports] @start_command = opts[:start_command] + @action_user = opts[:action_user] @additional_container_env_vars = opts[:additional_container_env_vars] end @@ -68,16 +69,6 @@ def port_environment_variables def privileged? false end - - def action_user - execution_metadata = Oj.load(@execution_metadata) - user = execution_metadata['user'] - if user.nil? || user.empty? - 'root' - else - user - end - end end end end diff --git a/lib/cloud_controller/diego/docker/lifecycle_protocol.rb b/lib/cloud_controller/diego/docker/lifecycle_protocol.rb index 14f584d8f25..a2b4b302871 100644 --- a/lib/cloud_controller/diego/docker/lifecycle_protocol.rb +++ b/lib/cloud_controller/diego/docker/lifecycle_protocol.rb @@ -37,6 +37,7 @@ def builder_opts(process) docker_image: process.actual_droplet.docker_receipt_image, execution_metadata: process.execution_metadata, start_command: process.command, + action_user: process.run_action_user, additional_container_env_vars: container_env_vars_for_process(process) } end diff --git a/lib/cloud_controller/diego/lifecycle_protocol.rb b/lib/cloud_controller/diego/lifecycle_protocol.rb index 5f2c52f4527..967a8290e12 100644 --- a/lib/cloud_controller/diego/lifecycle_protocol.rb +++ b/lib/cloud_controller/diego/lifecycle_protocol.rb @@ -66,6 +66,7 @@ def builder_opts(process) checksum_algorithm: checksum_info['type'], checksum_value: checksum_info['value'], start_command: process.started_command, + action_user: process.run_action_user, additional_container_env_vars: container_env_vars_for_process(process) } end diff --git a/spec/migrations/20250610212414_add_user_to_processes_spec.rb b/spec/migrations/20250610212414_add_user_to_processes_spec.rb new file mode 100644 index 00000000000..37deae8fcc6 --- /dev/null +++ b/spec/migrations/20250610212414_add_user_to_processes_spec.rb @@ -0,0 +1,55 @@ +require 'spec_helper' +require 'migrations/helpers/migration_shared_context' + +RSpec.describe 'migration to add user column to processes table', isolation: :truncation, type: :migration do + include_context 'migration' do + let(:migration_filename) { '20250610212414_add_user_to_processes.rb' } + end + + describe 'processes table' do + it 'adds a column `user`' do + expect(db[:processes].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[:processes].columns).to include(:user) + end + + describe 'idempotency of up' do + context '`user` column already exists' do + before do + db.add_column :processes, :user, String, size: 255, if_not_exists: true + end + + it 'does not fail' do + expect(db[:processes].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[:processes].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[:processes].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 :processes, :user + end + + it 'does not fail' do + expect(db[:processes].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/processes_spec.rb b/spec/request/processes_spec.rb index 3bfb6365dbb..5e647610b9c 100644 --- a/spec/request/processes_spec.rb +++ b/spec/request/processes_spec.rb @@ -116,6 +116,7 @@ }, 'type' => 'web', 'command' => '[PRIVATE DATA HIDDEN IN LISTS]', + 'user' => 'vcap', 'instances' => 2, 'memory_in_mb' => 1024, 'disk_in_mb' => 1024, @@ -155,6 +156,7 @@ }, 'type' => 'worker', 'command' => '[PRIVATE DATA HIDDEN IN LISTS]', + 'user' => 'vcap', 'instances' => 1, 'memory_in_mb' => 100, 'disk_in_mb' => 200, @@ -421,6 +423,7 @@ 'revision' => { 'data' => { 'guid' => revision.guid } } }, 'command' => 'rackup', + 'user' => 'vcap', 'instances' => 2, 'memory_in_mb' => 1024, 'disk_in_mb' => 1024, @@ -704,6 +707,7 @@ }, 'type' => 'web', 'command' => 'new command', + 'user' => 'vcap', 'instances' => 2, 'memory_in_mb' => 1024, 'disk_in_mb' => 1024, @@ -865,6 +869,7 @@ 'revision' => nil }, 'command' => 'rackup', + 'user' => 'vcap', 'instances' => 5, 'memory_in_mb' => 10, 'disk_in_mb' => 20, @@ -1230,6 +1235,7 @@ }, 'type' => 'web', 'command' => '[PRIVATE DATA HIDDEN IN LISTS]', + 'user' => 'vcap', 'instances' => 2, 'memory_in_mb' => 1024, 'disk_in_mb' => 1024, @@ -1273,6 +1279,7 @@ }, 'type' => 'worker', 'command' => '[PRIVATE DATA HIDDEN IN LISTS]', + 'user' => 'vcap', 'instances' => 1, 'memory_in_mb' => 100, 'disk_in_mb' => 200, @@ -1401,6 +1408,7 @@ }, 'type' => 'web', 'command' => 'rackup', + 'user' => 'vcap', 'instances' => 2, 'memory_in_mb' => 1024, 'disk_in_mb' => 1024, @@ -1495,6 +1503,7 @@ update_request = { command: 'new command', + user: 'containeruser', health_check: { type: 'http', data: { @@ -1523,6 +1532,7 @@ }, 'type' => 'web', 'command' => 'new command', + 'user' => 'containeruser', 'instances' => 2, 'memory_in_mb' => 1024, 'disk_in_mb' => 1024, @@ -1570,6 +1580,7 @@ process.reload expect(process.command).to eq('new command') + expect(process.user).to eq('containeruser') expect(process.health_check_type).to eq('http') expect(process.health_check_timeout).to eq(20) expect(process.health_check_http_endpoint).to eq('/healthcheck') @@ -1591,6 +1602,7 @@ 'process_type' => 'web', 'request' => { 'command' => '[PRIVATE DATA HIDDEN]', + 'user' => 'containeruser', 'health_check' => { 'type' => 'http', 'data' => { @@ -1652,6 +1664,7 @@ 'revision' => nil }, 'command' => 'rackup', + 'user' => 'vcap', 'instances' => 5, 'memory_in_mb' => 10, 'disk_in_mb' => 20, diff --git a/spec/unit/actions/deployment_create_spec.rb b/spec/unit/actions/deployment_create_spec.rb index c19b98fa147..5f2f6cfa7fd 100644 --- a/spec/unit/actions/deployment_create_spec.rb +++ b/spec/unit/actions/deployment_create_spec.rb @@ -181,6 +181,7 @@ module VCAP::CloudController expect(deploying_web_process.state).to eq(ProcessModel::STARTED) expect(deploying_web_process.instances).to eq(1) expect(deploying_web_process.command).to eq(web_process.command) + expect(deploying_web_process.user).to eq(web_process.user) expect(deploying_web_process.memory).to eq(web_process.memory) expect(deploying_web_process.file_descriptors).to eq(web_process.file_descriptors) expect(deploying_web_process.disk_quota).to eq(web_process.disk_quota) diff --git a/spec/unit/actions/process_update_spec.rb b/spec/unit/actions/process_update_spec.rb index f2aab34c2b4..4c383739e66 100644 --- a/spec/unit/actions/process_update_spec.rb +++ b/spec/unit/actions/process_update_spec.rb @@ -25,6 +25,7 @@ module VCAP::CloudController let(:message) do ProcessUpdateMessage.new( command: 'new', + user: 'vcap', health_check: health_check, readiness_health_check: readiness_health_check, metadata: { @@ -44,6 +45,7 @@ module VCAP::CloudController :process, type: 'web', command: 'initial command', + user: nil, health_check_type: 'port', health_check_timeout: 10, health_check_interval: 5, @@ -63,6 +65,7 @@ module VCAP::CloudController process.reload expect(process.command).to eq('new') + expect(process.user).to eq('vcap') expect(process.health_check_type).to eq('process') expect(process.health_check_timeout).to eq(20) expect(process.health_check_interval).to eq(7) @@ -373,6 +376,7 @@ module VCAP::CloudController user_audit_info, { 'command' => 'new', + 'user' => 'vcap', 'health_check' => { 'type' => 'process', 'data' => { 'timeout' => 20, 'interval' => 7 } @@ -398,6 +402,7 @@ module VCAP::CloudController user_audit_info, { 'command' => 'new', + 'user' => 'vcap', 'health_check' => { 'type' => 'process', 'data' => { 'timeout' => 20, 'interval' => 7 } diff --git a/spec/unit/lib/cloud_controller/diego/buildpack/desired_lrp_builder_spec.rb b/spec/unit/lib/cloud_controller/diego/buildpack/desired_lrp_builder_spec.rb index 47b7305696c..54b1f60c342 100644 --- a/spec/unit/lib/cloud_controller/diego/buildpack/desired_lrp_builder_spec.rb +++ b/spec/unit/lib/cloud_controller/diego/buildpack/desired_lrp_builder_spec.rb @@ -21,6 +21,7 @@ module Buildpack checksum_algorithm: 'checksum-algorithm', checksum_value: 'checksum-value', start_command: 'dd if=/dev/random of=/dev/null', + action_user: 'vcap', additional_container_env_vars: additional_env_vars } end 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 8cc7f47ea79..d057e726757 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 @@ -224,6 +224,7 @@ module Buildpack checksum_algorithm: 'sha256', checksum_value: droplet.sha256_checksum, start_command: 'go go go', + action_user: 'vcap', additional_container_env_vars: [] } end diff --git a/spec/unit/lib/cloud_controller/diego/cnb/desired_lrp_builder_spec.rb b/spec/unit/lib/cloud_controller/diego/cnb/desired_lrp_builder_spec.rb index 124c9b38532..ce92a2aee45 100644 --- a/spec/unit/lib/cloud_controller/diego/cnb/desired_lrp_builder_spec.rb +++ b/spec/unit/lib/cloud_controller/diego/cnb/desired_lrp_builder_spec.rb @@ -21,6 +21,7 @@ module CNB checksum_algorithm: 'checksum-algorithm', checksum_value: 'checksum-value', start_command: 'dd if=/dev/random of=/dev/null', + action_user: 'vcap', additional_container_env_vars: additional_env_vars } 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 baa057d8f3f..bf676b596fd 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 @@ -214,6 +214,7 @@ module CNB checksum_algorithm: 'sha256', checksum_value: droplet.sha256_checksum, start_command: 'go go go', + action_user: 'vcap', additional_container_env_vars: [] } end diff --git a/spec/unit/lib/cloud_controller/diego/docker/desired_lrp_builder_spec.rb b/spec/unit/lib/cloud_controller/diego/docker/desired_lrp_builder_spec.rb index 9bab579ebc3..b9ca976c848 100644 --- a/spec/unit/lib/cloud_controller/diego/docker/desired_lrp_builder_spec.rb +++ b/spec/unit/lib/cloud_controller/diego/docker/desired_lrp_builder_spec.rb @@ -11,6 +11,7 @@ module Docker docker_image: 'user/repo:tag', execution_metadata: execution_metadata, start_command: 'dd if=/dev/random of=/dev/null', + action_user: 'my-action-user', additional_container_env_vars: additional_env_vars } end @@ -170,16 +171,8 @@ module Docker end describe '#action_user' do - it 'returns "root"' do - expect(builder.action_user).to eq('root') - end - - context 'when the execution metadata has a specified user' do - let(:execution_metadata) { { user: 'foobar' }.to_json } - - it 'uses the user from the execution metadata' do - expect(builder.action_user).to eq('foobar') - end + it 'returns the passed in action user' do + expect(builder.action_user).to eq('my-action-user') end end diff --git a/spec/unit/lib/cloud_controller/diego/docker/lifecycle_protocol_spec.rb b/spec/unit/lib/cloud_controller/diego/docker/lifecycle_protocol_spec.rb index e60eb8afe62..62681d83d0e 100644 --- a/spec/unit/lib/cloud_controller/diego/docker/lifecycle_protocol_spec.rb +++ b/spec/unit/lib/cloud_controller/diego/docker/lifecycle_protocol_spec.rb @@ -64,13 +64,14 @@ module Docker execution_metadata: 'foobar' }) end - let(:process) { ProcessModel.make(app: app, diego: true, command: 'go go go', metadata: {}) } + let(:process) { ProcessModel.make(app: app, diego: true, command: 'go go go', user: 'ContainerUser', metadata: {}) } let(:builder_opts) do { ports: [8080], docker_image: 'the-image', execution_metadata: 'foobar', start_command: 'go go go', + action_user: 'ContainerUser', additional_container_env_vars: [] } end diff --git a/spec/unit/messages/process_update_message_spec.rb b/spec/unit/messages/process_update_message_spec.rb index 20b6c88e6e3..771aaf84ff6 100644 --- a/spec/unit/messages/process_update_message_spec.rb +++ b/spec/unit/messages/process_update_message_spec.rb @@ -108,6 +108,59 @@ module VCAP::CloudController 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 + context 'when endpoint is too long' do let(:params) { { health_check: { type: 'http', data: { endpoint: 'a' * 256 } } } } diff --git a/spec/unit/models/runtime/constraints/process_user_policy_spec.rb b/spec/unit/models/runtime/constraints/process_user_policy_spec.rb new file mode 100644 index 00000000000..697b3ed9bc6 --- /dev/null +++ b/spec/unit/models/runtime/constraints/process_user_policy_spec.rb @@ -0,0 +1,57 @@ +require 'spec_helper' + +RSpec.describe ProcessUserPolicy do + subject(:validator) { ProcessUserPolicy.new(process, allowed_users) } + + let(:process) { VCAP::CloudController::ProcessModelFactory.make } + let(:process_user) { 'invalid' } + let(:allowed_users) { Set.new(%w[vcap ContainerUser]) } + + before do + # need to stub the user method because validations keep invalid processes from being saved + allow(process).to receive(:user).and_return(process_user) + end + + context 'when user is nil' do + let(:process_user) { nil } + + it 'is valid' do + expect(validator).to validate_without_error(process) + end + end + + context 'when user is empty' do + let(:process_user) { '' } + + it 'is valid' do + expect(validator).to validate_without_error(process) + end + end + + context 'when user is an allowed user' do + let(:process_user) { 'ContainerUser' } + + it 'is valid' do + expect(validator).to validate_without_error(process) + end + end + + context 'when user is not an allowed user' do + let(:process_user) { 'vcarp' } + + it 'is not valid' do + expect(validator).to validate_with_error(process, :user, sprintf(ProcessUserPolicy::ERROR_MSG, requested_user: "'vcarp'", allowed_users: "'vcap', 'ContainerUser'")) + end + end + + describe 'case insensitivity' do + context 'when user is allowed, but does not match case' do + let(:process_user) { 'vCaP' } + let(:allowed_users) { Set.new(['VcAp']) } + + it 'is valid' do + expect(validator).to validate_without_error(process) + 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 30d74b50408..66ced4eff0e 100644 --- a/spec/unit/models/runtime/process_model_spec.rb +++ b/spec/unit/models/runtime/process_model_spec.rb @@ -198,6 +198,7 @@ def expect_no_validator(validator_class) expect_validator(HealthCheckPolicy) expect_validator(ReadinessHealthCheckPolicy) expect_validator(DockerPolicy) + expect_validator(ProcessUserPolicy) end describe 'org and space quota validator policies' do @@ -331,6 +332,49 @@ def expect_no_validator(validator_class) end end + describe 'user' do + subject(:process) { ProcessModelFactory.make(user: process_user) } + let(:process_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 { process.save }.not_to raise_error + end + end + + context 'when user is a permitted user' do + let(:process_user) { 'some_user' } + + it 'does not raise an error' do + expect { process.save }.not_to raise_error + end + end + + context 'when user is nil' do + let(:process_user) { nil } + + it 'does not raise an error' do + expect { process.save }.not_to raise_error + end + end + + context 'when user is not permitted' do + let(:process_user) { 'some-random-user' } + + it 'does raises an error' do + expect { process.save }.to raise_error(/user invalid/) + end + end + end + describe 'quota' do subject(:process) { ProcessModelFactory.make } let(:log_rate_limit) { 1024 } @@ -630,6 +674,61 @@ def act_as_cf_admin end end + describe '#run_action_user' do + subject(:process) { ProcessModelFactory.make } + + 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"}' } + + before do + process.desired_droplet.update(execution_metadata: droplet_execution_metadata) + end + + context 'when the process has a user specified' do + before do + process.update(user: 'ContainerUser') + end + + it 'returns the user' do + expect(process.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(process.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":["/cnb/lifecycle/launcher"]}' } + + 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 + context 'when the process has a user specified' do + before do + process.update(user: 'ContainerUser') + end + + it 'returns the user' do + expect(process.run_action_user).to eq('ContainerUser') + end + end + + context 'when the process DOES NOT have a user specified' do + it 'returns the default "vcap" user' do + expect(process.run_action_user).to eq('vcap') + end + end + end + end + describe '#specified_or_detected_command' do subject(:process) { ProcessModelFactory.make } diff --git a/spec/unit/presenters/v3/process_presenter_spec.rb b/spec/unit/presenters/v3/process_presenter_spec.rb index 764066b0f5f..8e6e3f38f79 100644 --- a/spec/unit/presenters/v3/process_presenter_spec.rb +++ b/spec/unit/presenters/v3/process_presenter_spec.rb @@ -130,6 +130,28 @@ module VCAP::CloudController::Presenters::V3 end end + describe 'user' do + let(:custom_user) { nil } + + before do + process.update(user: custom_user) + end + + context 'when the process has a user set' do + let(:custom_user) { 'ContainerUser' } + + it 'displays the user' do + expect(result[:user]).to eq('ContainerUser') + end + end + + context 'when the process does not have a user set' do + it 'displays the default "vcap" user' do + expect(result[:user]).to eq('vcap') + end + end + end + describe '#revisions' do context('when the process has a revision') do let(:revision) { VCAP::CloudController::RevisionModel.make }