Skip to content

Commit d9a7db1

Browse files
committed
Stop unnecessarily scan the 'job.MF' file from job blob when rendering ERBs
Indeed we already know the job name and template list, as these data are persisted when a release is uploaded to the director. This makes unnecessary to verify that the job name is consistent at ERB rendering time. And this consistency check is more relevant when done at the time a release job is uploaded to the director.
1 parent 977d6b7 commit d9a7db1

File tree

6 files changed

+130
-53
lines changed

6 files changed

+130
-53
lines changed

src/bosh-director-core/lib/bosh/director/core/templates/job_template_loader.rb

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -35,21 +35,13 @@ def initialize(logger, template_blob_cache, link_provider_intents, dns_encoder)
3535
# @return [JobTemplateRenderer] Object that can render the templates
3636
def process(instance_job)
3737
template_dir = extract_template(instance_job)
38-
manifest = Psych.load_file(File.join(template_dir, 'job.MF'), aliases: true)
3938

4039
monit_erb_file = File.read(File.join(template_dir, 'monit'))
4140
monit_source_erb = SourceErb.new('monit', 'monit', monit_erb_file, instance_job.name)
4241

4342
source_erbs = []
4443

45-
job_name_from_manifest = manifest.fetch('name', {})
46-
if job_name_from_manifest != instance_job.name
47-
raise Bosh::Director::JobTemplateUnpackFailed,
48-
"Inconsistent name in extracted job.MF manifest " +
49-
"(exptected: '#{instance_job.name}', got: '#{job_name_from_manifest}')"
50-
end
51-
52-
manifest.fetch('templates', {}).each_pair do |src_name, dest_name|
44+
instance_job.model.spec.fetch('templates', {}).each_pair do |src_name, dest_name|
5345
erb_file = File.read(File.join(template_dir, 'templates', src_name))
5446
source_erbs << SourceErb.new(src_name, dest_name, erb_file, instance_job.name)
5547
end

src/bosh-director-core/spec/bosh/director/core/templates/job_template_loader_spec.rb

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,10 @@ module Bosh::Director::Core::Templates
9595
release: release
9696
)
9797

98+
job_model = double('Bosh::Director::Models::Template')
99+
expect(job).to receive(:model).and_return(job_model)
100+
expect(job_model).to receive(:spec).and_return({ "templates" => { "test" => "test_dst" } })
101+
98102
monit_erb = instance_double(SourceErb)
99103
job_template_erb = instance_double(SourceErb)
100104
fake_renderer = instance_double(JobTemplateRenderer)
@@ -137,6 +141,10 @@ module Bosh::Director::Core::Templates
137141
release: release,
138142
)
139143

144+
job_model = double('Bosh::Director::Models::Template')
145+
expect(job).to receive(:model).and_return(job_model)
146+
expect(job_model).to receive(:spec).and_return({ "templates" => {} })
147+
140148
monit_erb = instance_double(SourceErb)
141149
fake_renderer = instance_double(JobTemplateRenderer)
142150

@@ -182,6 +190,10 @@ module Bosh::Director::Core::Templates
182190
model: double('Bosh::Director::Models::Template', provides: [])
183191
)
184192

193+
job_model = double('Bosh::Director::Models::Template')
194+
expect(job).to receive(:model).and_return(job_model)
195+
expect(job_model).to receive(:spec).and_return({ "templates" => {} })
196+
185197
job_template_renderer = job_template_loader.process(job)
186198
expect(job_template_renderer.source_erbs).to eq([])
187199
end

src/bosh-director/lib/bosh/director/errors.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,7 @@ def self.err(error_code, response_code = BAD_REQUEST)
134134
JobInvalidLinkSpec = err(80013)
135135
JobDuplicateLinkName = err(80014)
136136
JobWithExportedFromMismatch = err(80015)
137+
JobInvalidName = err(80016)
137138

138139
ResourceError = err(100001)
139140
ResourceNotFound = err(100002, NOT_FOUND)

src/bosh-director/lib/bosh/director/jobs/release/release_job.rb

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ def update
1515

1616
job_manifest = load_manifest
1717

18+
validate_name(job_manifest)
1819
validate_templates(job_manifest)
1920
validate_monit
2021
validate_logs(job_manifest)
@@ -75,6 +76,13 @@ def load_manifest
7576
YAML.load_file(manifest_file, aliases: true)
7677
end
7778

79+
def validate_name(job_manifest)
80+
unless name == job_manifest['name']
81+
raise JobInvalidName, "Inconsistent name for job '#{name}'" +
82+
"(exptected: '#{name}', got: '#{job_manifest['name']}')"
83+
end
84+
end
85+
7886
def validate_templates(job_manifest)
7987
if job_manifest['templates']
8088
job_manifest['templates'].each_key do |relative_path|

src/bosh-director/spec/unit/job_renderer_spec.rb

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,13 +41,21 @@ def perform
4141

4242
before do
4343
release_version = DeploymentPlan::ReleaseVersion.parse(deployment_model, 'name' => 'fake-release', 'version' => '123')
44+
4445
job1 = DeploymentPlan::Job.new(release_version, 'dummy')
45-
job1.bind_existing_model(
46-
Models::Template.make(blobstore_id: 'my-blobstore-id', sha1: '16baf0c24e2dac2a21ccdcd4655be403a602f573'),
47-
)
46+
job1.bind_existing_model(Models::Template.make(
47+
name: 'dummy',
48+
spec_json: '{ "templates": { "ctl.erb": "bin/dummy_ctl" } }',
49+
blobstore_id: 'my-blobstore-id',
50+
sha1: '16baf0c24e2dac2a21ccdcd4655be403a602f573'
51+
))
4852

4953
job2 = DeploymentPlan::Job.new(release_version, 'dummy')
50-
job2.bind_existing_model(Models::Template.make(blobstore_id: 'my-blobstore-id'))
54+
job2.bind_existing_model(Models::Template.make(
55+
name: 'dummy',
56+
spec_json: '{ "templates": { "ctl.erb": "bin/dummy_ctl" } }',
57+
blobstore_id: 'my-blobstore-id')
58+
)
5159

5260
allow(instance_plan).to receive_message_chain(:spec, :as_template_spec).and_return('template' => 'spec')
5361
allow(instance_plan).to receive(:templates).and_return([job1, job2])

0 commit comments

Comments
 (0)