From c33dd079887dbb82d3906bf93fcd2043a3c1233a Mon Sep 17 00:00:00 2001 From: Tim Downey Date: Fri, 20 Jun 2025 15:58:17 -0600 Subject: [PATCH 1/2] Add ability to specify the process user in manifests - Updated manifest generation to include users --- app/messages/app_manifest_message.rb | 1 + .../manifest_process_update_message.rb | 7 +++ .../process_properties_presenter.rb | 1 + .../resources/manifests/_object.md.erb | 2 + .../messages/app_manifest_message_spec.rb | 6 ++- .../manifest_process_update_message_spec.rb | 53 ++++++++++++++++++ .../process_properties_presenter_spec.rb | 54 +++++++++++++++++++ 7 files changed, 123 insertions(+), 1 deletion(-) diff --git a/app/messages/app_manifest_message.rb b/app/messages/app_manifest_message.rb index e9f313d054d..97802f9e361 100644 --- a/app/messages/app_manifest_message.rb +++ b/app/messages/app_manifest_message.rb @@ -262,6 +262,7 @@ def process_update_attributes_from_app_level def process_update_attributes_from_process(params) mapping = {} mapping[:command] = params[:command] || 'null' if params.key?(:command) + mapping[:user] = params[:user] if params.key?(:user) mapping[:health_check_http_endpoint] = params[:health_check_http_endpoint] if params.key?(:health_check_http_endpoint) mapping[:health_check_timeout] = params[:health_check_timeout] if params.key?(:health_check_timeout) mapping[:health_check_invocation_timeout] = params[:health_check_invocation_timeout] if params.key?(:health_check_invocation_timeout) diff --git a/app/messages/manifest_process_update_message.rb b/app/messages/manifest_process_update_message.rb index ea3a6aff6e8..0a1b6eff9a2 100644 --- a/app/messages/manifest_process_update_message.rb +++ b/app/messages/manifest_process_update_message.rb @@ -5,6 +5,7 @@ module VCAP::CloudController class ManifestProcessUpdateMessage < BaseMessage register_allowed_keys %i[ command + user health_check_http_endpoint health_check_invocation_timeout health_check_type @@ -34,6 +35,12 @@ def self.readiness_health_check_endpoint_and_type_requested? length: { in: 1..4096, message: 'must be between 1 and 4096 characters' }, if: proc { |a| a.requested?(:command) } + validates :user, + string: true, + allow_nil: true, + length: { in: 1..255, message: 'must be between 1 and 255 characters' }, + if: proc { |a| a.requested?(:user) } + validates :health_check_type, inclusion: { in: [HealthCheckTypes::PORT, HealthCheckTypes::PROCESS, HealthCheckTypes::HTTP], diff --git a/app/presenters/v3/app_manifest_presenters/process_properties_presenter.rb b/app/presenters/v3/app_manifest_presenters/process_properties_presenter.rb index e062a3c289d..12bfc845b14 100644 --- a/app/presenters/v3/app_manifest_presenters/process_properties_presenter.rb +++ b/app/presenters/v3/app_manifest_presenters/process_properties_presenter.rb @@ -16,6 +16,7 @@ def process_hash(process) 'disk_quota' => add_units(process.disk_quota), 'log-rate-limit-per-second' => add_units_log_rate_limit(process.log_rate_limit), 'command' => process.command, + 'user' => process.user, 'health-check-type' => process.health_check_type, 'health-check-http-endpoint' => process.health_check_http_endpoint, 'health-check-invocation-timeout' => process.health_check_invocation_timeout, diff --git a/docs/v3/source/includes/resources/manifests/_object.md.erb b/docs/v3/source/includes/resources/manifests/_object.md.erb index 8a24c279be8..9c993b3e3ce 100644 --- a/docs/v3/source/includes/resources/manifests/_object.md.erb +++ b/docs/v3/source/includes/resources/manifests/_object.md.erb @@ -54,6 +54,7 @@ applications: memory: 500M log-rate-limit-per-second: 1KB timeout: 10 + user: vcap - type: worker command: start-worker.sh disk_quota: 1G @@ -122,6 +123,7 @@ Name | Type | Description ---- | ---- | ----------- **type** | _string_ | **(Required)** The identifier for the processes to be configured **command** | _string_ | The command used to start the process; this overrides start commands from [Procfiles](#procfiles) and buildpacks +**user** | _string_ | The user under which the process runs **disk_quota** | _string_ | The disk limit for all instances of the web process;
this attribute requires a unit of measurement: `B`, `K`, `KB`, `M`, `MB`, `G`, `GB`, `T`, or `TB` in upper case or lower case **health-check-http-endpoint** | _string_ | Endpoint called to determine if the app is healthy **health-check-interval** | _integer_ | The interval in seconds between health check requests diff --git a/spec/unit/messages/app_manifest_message_spec.rb b/spec/unit/messages/app_manifest_message_spec.rb index e41ad73bb78..dc1143172e9 100644 --- a/spec/unit/messages/app_manifest_message_spec.rb +++ b/spec/unit/messages/app_manifest_message_spec.rb @@ -721,6 +721,7 @@ module VCAP::CloudController 'readiness_health_check_http_endpoint' => 'potato potahto', 'readiness_health_check_interval' => 'yucca', 'command' => '', + 'user' => '', 'timeout' => 'yam' } end @@ -738,6 +739,7 @@ module VCAP::CloudController 'readiness_health_check_invocation_timeout' => 'cat-jicima', 'readiness_health_check_interval' => -1, 'command' => '', + 'user' => '', 'timeout' => 'yam' } end @@ -751,10 +753,11 @@ module VCAP::CloudController it 'includes the type of the process in the error message' do message = AppManifestMessage.create_from_yml(params_from_yaml) expect(message).not_to be_valid - expect(message.errors).to have(27).items + expect(message.errors).to have(29).items expected_errors = [ 'Process "type1": Command must be between 1 and 4096 characters', + 'Process "type1": User must be between 1 and 255 characters', 'Process "type1": Disk quota must use a supported unit: B, K, KB, M, MB, G, GB, T, or TB', 'Process "type1": Log rate limit per second is not a number', 'Process "type1": Instances must be greater than or equal to 0', @@ -770,6 +773,7 @@ module VCAP::CloudController 'Process "type1": Readiness health check http endpoint must be a valid URI path', 'Process "type1": Readiness health check type must be "http" to set a health check HTTP endpoint', 'Process "type2": Command must be between 1 and 4096 characters', + 'Process "type2": User must be between 1 and 255 characters', 'Process "type2": Disk quota must use a supported unit: B, K, KB, M, MB, G, GB, T, or TB', 'Process "type2": Log rate limit per second must use a supported unit: B, K, KB, M, MB, G, GB, T, or TB', 'Process "type2": Instances is not a number', diff --git a/spec/unit/messages/manifest_process_update_message_spec.rb b/spec/unit/messages/manifest_process_update_message_spec.rb index 58f312cb59d..f2d06e2de86 100644 --- a/spec/unit/messages/manifest_process_update_message_spec.rb +++ b/spec/unit/messages/manifest_process_update_message_spec.rb @@ -62,6 +62,59 @@ module VCAP::CloudController expect(message.errors[:command]).to include('must be between 1 and 4096 characters') end end + + context 'when command is just right' do + let(:params) { { command: './start.sh' } } + + it 'is valid' do + expect(message).to be_valid + end + end + end + + describe 'user' do + context 'when user is not a string' do + let(:params) { { user: 32.77 } } + + it 'is valid' do + 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 not valid' do + expect(message).to be_valid + end + end + + context 'when user is too long' do + let(:params) { { user: 'a' * 256 } } + + it 'is not valid' do + 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 + 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) { { command: 'vcap' } } + + it 'is valid' do + expect(message).to be_valid + end + end end describe 'health_check_type' do diff --git a/spec/unit/presenters/v3/app_manifest_presenters/process_properties_presenter_spec.rb b/spec/unit/presenters/v3/app_manifest_presenters/process_properties_presenter_spec.rb index 1fab8ac5db0..38f41409dd4 100644 --- a/spec/unit/presenters/v3/app_manifest_presenters/process_properties_presenter_spec.rb +++ b/spec/unit/presenters/v3/app_manifest_presenters/process_properties_presenter_spec.rb @@ -67,6 +67,60 @@ module VCAP::CloudController::Presenters::V3::AppManifestPresenters 'timeout' => 30 }) end + + context 'nullable fields' do + context 'when command is present' do + let(:process) do + VCAP::CloudController::ProcessModel.make( + command: './start-command' + ) + end + + it 'includes command in the hash' do + hash = subject.process_hash(process) + expect(hash).to include('command' => './start-command') + end + end + + context 'when command is not present' do + let(:process) do + VCAP::CloudController::ProcessModel.make( + command: nil + ) + end + + it 'does not include command in the hash' do + hash = subject.process_hash(process) + expect(hash).not_to include('command') + end + end + + context 'when user is present' do + let(:process) do + VCAP::CloudController::ProcessModel.make( + user: 'ContainerUser' + ) + end + + it 'includes user in the hash' do + hash = subject.process_hash(process) + expect(hash).to include('user' => 'ContainerUser') + end + end + + context 'when user is not present' do + let(:process) do + VCAP::CloudController::ProcessModel.make( + user: nil + ) + end + + it 'does not include user in the hash' do + hash = subject.process_hash(process) + expect(hash).not_to include('user') + end + end + end end describe '#add_units_log_rate_limit' do From 9a4591e9ac70df3c02d92a4093684ceaed27754e Mon Sep 17 00:00:00 2001 From: Tim Downey Date: Mon, 23 Jun 2025 14:02:32 -0600 Subject: [PATCH 2/2] Add user attribute to manifest diffs --- app/actions/space_diff_manifest.rb | 1 + spec/unit/actions/space_diff_manifest_spec.rb | 6 ++++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/app/actions/space_diff_manifest.rb b/app/actions/space_diff_manifest.rb index d3a52224a49..d46b2757ea7 100644 --- a/app/actions/space_diff_manifest.rb +++ b/app/actions/space_diff_manifest.rb @@ -98,6 +98,7 @@ def filter_manifest_app_hash(manifest_app_hash) hash.slice( 'type', 'command', + 'user', 'disk_quota', 'log-rate-limit-per-second', 'health-check-http-endpoint', diff --git a/spec/unit/actions/space_diff_manifest_spec.rb b/spec/unit/actions/space_diff_manifest_spec.rb index d7c9f3fde27..0189c596ff8 100644 --- a/spec/unit/actions/space_diff_manifest_spec.rb +++ b/spec/unit/actions/space_diff_manifest_spec.rb @@ -84,14 +84,16 @@ module VCAP::CloudController end context 'processes' do - context 'when processes are added' do + context 'when processes are updated' do before do default_manifest['applications'][0]['processes'][0]['memory'] = '2048M' + default_manifest['applications'][0]['processes'][0]['user'] = 'ContainerUser' end it 'returns the correct diff' do expect(subject).to contain_exactly( - { 'op' => 'replace', 'path' => '/applications/0/processes/0/memory', 'was' => "#{process1.memory}M", 'value' => '2048M' } + { 'op' => 'replace', 'path' => '/applications/0/processes/0/memory', 'was' => "#{process1.memory}M", 'value' => '2048M' }, + { 'op' => 'add', 'path' => '/applications/0/processes/0/user', 'value' => 'ContainerUser' } ) end end