Skip to content
This repository was archived by the owner on Jun 2, 2021. It is now read-only.

Commit cf1ed14

Browse files
Return errors for malformed manifests in manifest_diff
- also fixes bug in apply_manifest with an improper 400 [finishes #173688915] [finsihes #173818750] Co-authored-by: James Palmer <jpalmer@pivotal.io> Co-authored-by: Merric de Launey <mdelauney@pivotal.io>
1 parent b794d87 commit cf1ed14

File tree

4 files changed

+57
-41
lines changed

4 files changed

+57
-41
lines changed

app/controllers/v3/space_manifests_controller.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ def check_version_is_supported!
120120
def parsed_app_manifests
121121
check_version_is_supported!
122122
parsed_applications = params[:body]['applications']
123-
raise invalid_request!('Invalid app manifest') unless parsed_applications.present?
123+
raise unprocessable!("Cannot parse manifest with no 'applications' field.") unless parsed_applications.present?
124124

125125
parsed_applications.map(&:to_unsafe_h)
126126
end

docs/v3/source/includes/resources/manifests/_create_diff.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,8 @@ Content-Type: application/json
4343

4444
Create a manifest diff for apps in the provided manifest and their underlying processes. Currently manifest_diff only supports version 1 manifests.
4545

46+
Manifests require the `applications` field.
47+
4648
#### Definition
4749
`GET /v3/spaces/:guid/manifest_diff`
4850

spec/request/space_manifests_spec.rb

Lines changed: 50 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -573,53 +573,67 @@
573573
end
574574
it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS
575575
end
576-
context 'the manifest is unparseable' do
577-
let(:yml_manifest) do
578-
{
579-
89 => [
580-
{
581-
'name' => 'new-app',
582-
},
583-
]
584-
}.to_yaml
585-
end
586-
576+
context 'when the request is invalid' do
587577
before do
588578
space.organization.add_user(user)
589579
space.add_developer(user)
590580
end
591581

592-
it 'returns an appropriate error' do
593-
post "/v3/spaces/#{space.guid}/manifest_diff", yml_manifest, yml_headers(user_header)
594-
parsed_response = MultiJson.load(last_response.body)
582+
context 'when there is a manifest without \'applications\'' do
583+
let(:yml_manifest) do
584+
{
585+
'not-applications' => [
586+
{
587+
'name' => 'new-app',
588+
},
589+
]
590+
}.to_yaml
591+
end
595592

596-
expect(last_response).to have_status_code(400)
597-
expect(parsed_response['errors'].first['detail']).to eq('The request is invalid')
598-
end
599-
end
600-
context 'the manifest is a not supported version' do
601-
let(:yml_manifest) do
602-
{
603-
'version' => 1234567,
604-
'applications' => [
605-
{
606-
'name' => 'new-app',
607-
},
608-
]
609-
}.to_yaml
593+
it 'returns an appropriate error' do
594+
post "/v3/spaces/#{space.guid}/manifest_diff", yml_manifest, yml_headers(user_header)
595+
parsed_response = MultiJson.load(last_response.body)
596+
597+
expect(last_response).to have_status_code(422)
598+
expect(parsed_response['errors'].first['detail']).to eq("Cannot parse manifest with no 'applications' field.")
599+
end
610600
end
611601

612-
before do
613-
space.organization.add_user(user)
614-
space.add_developer(user)
602+
context 'the manifest is unparseable' do
603+
let(:yml_manifest) do
604+
{
605+
'key' => 'this is json, not yaml'
606+
}
607+
end
608+
609+
it 'returns an appropriate error' do
610+
post "/v3/spaces/#{space.guid}/manifest_diff", yml_manifest, yml_headers(user_header)
611+
parsed_response = MultiJson.load(last_response.body)
612+
613+
expect(last_response).to have_status_code(400)
614+
expect(parsed_response['errors'].first['detail']).to eq('Request invalid due to parse error: invalid request body')
615+
end
615616
end
616617

617-
it 'returns an appropriate error' do
618-
post "/v3/spaces/#{space.guid}/manifest_diff", yml_manifest, yml_headers(user_header)
619-
parsed_response = MultiJson.load(last_response.body)
618+
context 'the manifest is a not supported version' do
619+
let(:yml_manifest) do
620+
{
621+
'version' => 1234567,
622+
'applications' => [
623+
{
624+
'name' => 'new-app',
625+
},
626+
]
627+
}.to_yaml
628+
end
620629

621-
expect(last_response).to have_status_code(422)
622-
expect(parsed_response['errors'].first['detail']).to eq('Unsupported manifest schema version. Currently supported versions: [1].')
630+
it 'returns an appropriate error' do
631+
post "/v3/spaces/#{space.guid}/manifest_diff", yml_manifest, yml_headers(user_header)
632+
parsed_response = MultiJson.load(last_response.body)
633+
634+
expect(last_response).to have_status_code(422)
635+
expect(parsed_response['errors'].first['detail']).to eq('Unsupported manifest schema version. Currently supported versions: [1].')
636+
end
623637
end
624638
end
625639
context 'the space does not exist' do

spec/unit/controllers/v3/space_manifests_controller_spec.rb

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -115,18 +115,18 @@
115115
context 'when the yaml is missing an applications array' do
116116
let(:request_body) { { 'name' => 'blah', 'instances' => 4 } }
117117

118-
it 'returns a 400' do
118+
it 'returns a 422' do
119119
post :apply_manifest, params: { guid: space.guid }.merge(request_body), as: :yaml
120-
expect(response.status).to eq(400)
120+
expect(response.status).to eq(422)
121121
end
122122
end
123123

124124
context 'when the requested applications array is empty' do
125125
let(:request_body) { { 'applications' => [] } }
126126

127-
it 'returns a 400' do
127+
it 'returns a 422' do
128128
post :apply_manifest, params: { guid: space.guid }.merge(request_body), as: :yaml
129-
expect(response.status).to eq(400)
129+
expect(response.status).to eq(422)
130130
end
131131
end
132132

0 commit comments

Comments
 (0)