Skip to content

Commit ab9669a

Browse files
committed
Support overriding user for CNB tasks
1 parent 762dfd0 commit ab9669a

File tree

8 files changed

+78
-6
lines changed

8 files changed

+78
-6
lines changed

app/models/runtime/app_model.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ class AppModel < Sequel::Model(:apps)
88
include Serializer
99
APP_NAME_REGEX = /\A[[:alnum:][:punct:][:print:]]+\Z/
1010
DEFAULT_CONTAINER_USER = 'vcap'.freeze
11+
DEFAULT_DOCKER_CONTAINER_USER = 'root'.freeze
1112

1213
many_to_many :routes, join_table: :route_mappings, left_key: :app_guid, left_primary_key: :guid, right_primary_key: :guid, right_key: :route_guid
1314
one_to_many :route_mappings, class: 'VCAP::CloudController::RouteMappingModel', key: :app_guid, primary_key: :guid

app/models/runtime/droplet_model.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ def docker_user
138138
end
139139
end
140140

141-
container_user.presence || 'root'
141+
container_user.presence || AppModel::DEFAULT_DOCKER_CONTAINER_USER
142142
end
143143

144144
def staging?

app/models/runtime/process_model.rb

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -576,7 +576,9 @@ def permitted_users
576576
end
577577

578578
def docker_run_action_user
579-
desired_droplet.docker_user.presence || AppModel::DEFAULT_CONTAINER_USER
579+
return AppModel::DEFAULT_CONTAINER_USER unless docker?
580+
581+
desired_droplet&.docker_user.presence || AppModel::DEFAULT_DOCKER_CONTAINER_USER
580582
end
581583

582584
def non_unique_process_types

app/models/runtime/task_model.rb

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,10 +47,17 @@ def after_destroy
4747
def run_action_user
4848
return user if user.present?
4949

50-
docker? ? docker_run_action_user : AppModel::DEFAULT_CONTAINER_USER
50+
if docker?
51+
docker_run_action_user
52+
elsif cnb?
53+
'root' # TODO: Why do CNB tasks default to this user instead of vcap?
54+
else
55+
AppModel::DEFAULT_CONTAINER_USER
56+
end
5157
end
5258

5359
delegate :docker?, to: :droplet
60+
delegate :cnb?, to: :droplet
5461

5562
private
5663

lib/cloud_controller/diego/cnb/lifecycle_protocol.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ def staging_action_builder(config, staging_details)
1515
end
1616

1717
def task_action_builder(config, task)
18-
VCAP::CloudController::Diego::Buildpack::TaskActionBuilder.new(config, task, task_lifecycle_data(task), 'root', ['--', task.command], 'cnb')
18+
VCAP::CloudController::Diego::Buildpack::TaskActionBuilder.new(config, task, task_lifecycle_data(task), task.run_action_user, ['--', task.command], 'cnb')
1919
end
2020

2121
def desired_lrp_builder(config, process)

spec/unit/lib/cloud_controller/diego/cnb/lifecycle_protocol_spec.rb

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,31 @@ module CNB
175175
'cnb')
176176
end
177177

178+
context 'when the task has a user specified' do
179+
let(:task) { TaskModel.make(:cnb, user: 'TestUser') }
180+
181+
it 'sets the appropriate user' do
182+
task.app.update(cnb_lifecycle_data: CNBLifecycleDataModel.make(stack: 'potato-stack'))
183+
184+
task_action_builder = instance_double(Buildpack::TaskActionBuilder)
185+
allow(Buildpack::TaskActionBuilder).to receive(:new).and_return task_action_builder
186+
187+
expect(lifecycle_protocol.task_action_builder(config, task)).to be task_action_builder
188+
189+
expect(Buildpack::TaskActionBuilder).to have_received(:new).with(
190+
config,
191+
task,
192+
{
193+
droplet_uri: 'droplet-download-url',
194+
stack: 'potato-stack'
195+
},
196+
'TestUser',
197+
['--', task.command],
198+
'cnb'
199+
)
200+
end
201+
end
202+
178203
context 'when the blobstore_url_generator returns nil' do
179204
let(:droplet_download_url) { nil }
180205

spec/unit/models/runtime/process_model_spec.rb

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -679,7 +679,7 @@ def act_as_cf_admin
679679

680680
context 'when the process belongs to a Docker lifecycle app' do
681681
subject(:process) { ProcessModelFactory.make({ docker_image: 'example.com/image' }) }
682-
let(:droplet_execution_metadata) { '{"entrypoint":["/image-entrypoint.sh"],"user":"cnb"}' }
682+
let(:droplet_execution_metadata) { '{"entrypoint":["/image-entrypoint.sh"],"user":"some-user"}' }
683683

684684
before do
685685
process.desired_droplet.update(execution_metadata: droplet_execution_metadata)
@@ -699,7 +699,7 @@ def act_as_cf_admin
699699

700700
context 'when the droplet execution metadata specifies a user' do
701701
it 'returns the specified user' do
702-
expect(process.run_action_user).to eq('cnb')
702+
expect(process.run_action_user).to eq('some-user')
703703
end
704704
end
705705

@@ -734,6 +734,17 @@ def act_as_cf_admin
734734
expect(process.run_action_user).to eq('root')
735735
end
736736
end
737+
738+
context 'when the app does not have a droplet assigned' do
739+
before do
740+
process.app.update(droplet: nil)
741+
process.reload
742+
end
743+
744+
it 'defaults the user to root' do
745+
expect(process.run_action_user).to eq('root')
746+
end
747+
end
737748
end
738749

739750
context 'when the process DOES NOT belong to a Docker lifecycle app' do

spec/unit/models/runtime/task_model_spec.rb

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,32 @@ module VCAP::CloudController
168168
let(:task) { TaskModel.make(app: parent_app, droplet: droplet, state: TaskModel::SUCCEEDED_STATE) }
169169
let(:droplet) { DropletModel.make }
170170

171+
context 'when the task belongs to a CNB lifecycle app' do
172+
let(:parent_app) { AppModel.make(:cnb) }
173+
let(:task) { TaskModel.make(app: parent_app, droplet: droplet, state: TaskModel::SUCCEEDED_STATE) }
174+
let(:droplet) { DropletModel.make(:cnb) }
175+
176+
context 'when the task has a user specified' do
177+
before do
178+
task.update(user: 'TestUser')
179+
end
180+
181+
it 'returns the user' do
182+
expect(task.run_action_user).to eq('TestUser')
183+
end
184+
end
185+
186+
context 'when the task does not have a user specified' do
187+
before do
188+
task.update(user: nil)
189+
end
190+
191+
it 'defaults the user to root' do
192+
expect(task.run_action_user).to eq('root')
193+
end
194+
end
195+
end
196+
171197
context 'when the task belongs to a Docker lifecycle app' do
172198
let(:parent_app) { AppModel.make(:docker) }
173199
let(:task) { TaskModel.make(app: parent_app, droplet: droplet, state: TaskModel::SUCCEEDED_STATE) }

0 commit comments

Comments
 (0)