Skip to content

Commit

Permalink
Use Rack Lint in specs to check compliance with rack spec
Browse files Browse the repository at this point in the history
[#150958544](https://www.pivotaltracker.com/n/projects/1456570/stories/150958544)

Signed-off-by: David Ansari <[email protected]>
Signed-off-by: Felix Riegger <[email protected]>
  • Loading branch information
beyhan authored and friegger committed Oct 23, 2017
1 parent f59a586 commit adfeab6
Show file tree
Hide file tree
Showing 25 changed files with 44 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -161,11 +161,13 @@ def initialize(config)

payload = json_decode(request.body.read)
@resurrector_manager.set_pause_for_instance(deployment, params[:job], params[:index_or_id], payload['resurrection_paused'])
status(200)
end

put '/:deployment/instance_groups/:instancegroup/:id/ignore', consumes: :json do
payload = json_decode(request.body.read)
@instance_ignore_manager.set_ignore_state_for_instance(deployment, params[:instancegroup], params[:id], payload['ignore'])
status(200)
end

post '/:deployment/jobs/:job/:index_or_id/snapshots' do
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,8 @@ class EventsController < BaseController
error: error,
context: context
})

status(200)
end

private
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ module Bosh::Director
module Api::Controllers
class PackagesController < BaseController
post '/matches', :consumes => :yaml do
manifest = YAML.load(request.body)
manifest = YAML.load(request.body.read)

unless manifest.is_a?(Hash) && manifest['packages'].is_a?(Array)
raise BadManifest, "Manifest doesn't have a usable packages section"
Expand All @@ -22,7 +22,7 @@ class PackagesController < BaseController
end

post '/matches_compiled', :consumes => :yaml do
manifest = YAML.load(request.body)
manifest = YAML.load(request.body.read)

unless manifest.is_a?(Hash) && manifest['compiled_packages'].is_a?(Array)
raise BadManifest, "Manifest doesn't have a usable packages section"
Expand Down
7 changes: 7 additions & 0 deletions src/bosh-director/spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,13 @@ def check_event_log(task_id)
end
end

def linted_rack_app(app)
Rack::Builder.new do
use Rack::Lint
run app
end
end

module ManifestHelper
class << self
def default_deployment_manifest(overrides = {})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ module Api
describe Controllers::BackupsController do
include Rack::Test::Methods

subject(:app) { described_class.new(config) }
subject(:app) { linted_rack_app(described_class.new(config)) }
let(:config) { Config.load_hash(SpecHelper.spec_get_director_config) }
let(:temp_dir) { Dir.mktmpdir}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ module Api
describe Controllers::CleanupController do
include Rack::Test::Methods

subject(:app) { described_class.new(config) }
subject(:app) { linted_rack_app(described_class.new(config)) }
let(:config) { Config.load_hash(SpecHelper.spec_get_director_config) }

before do
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ module Bosh::Director
describe Api::Controllers::CloudConfigsController do
include Rack::Test::Methods

subject(:app) { Api::Controllers::CloudConfigsController.new(config) }
subject(:app) { linted_rack_app(described_class.new(config)) }
let(:config) do
config = Config.load_hash(SpecHelper.spec_get_director_config)
identity_provider = Support::TestIdentityProvider.new(config.get_uuid_provider)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ module Bosh::Director
describe Api::Controllers::CpiConfigsController do
include Rack::Test::Methods

subject(:app) { Api::Controllers::CpiConfigsController.new(config) }
subject(:app) { linted_rack_app(described_class.new(config)) }
let(:config) do
config = Config.load_hash(SpecHelper.spec_get_director_config)
identity_provider = Support::TestIdentityProvider.new(config.get_uuid_provider)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ module Api
include IpUtil
include Rack::Test::Methods

subject(:app) { described_class.new(config) }
subject(:app) { linted_rack_app(described_class.new(config)) }

let(:config) do
config = Config.load_hash(SpecHelper.spec_get_director_config)
Expand Down Expand Up @@ -307,7 +307,7 @@ def manifest_with_errand(deployment_name='errand')
variable_set: Models::VariableSet.create(deployment: deployment)
)
Models::PersistentDisk.create(instance: instance, disk_cid: 'disk_cid')
put "#{path}", manifest, {'CONTENT_TYPE' => 'text/yaml', 'CONTENT_LENGTH' => 0}
put "#{path}", manifest, {'CONTENT_TYPE' => 'text/yaml', 'CONTENT_LENGTH' => '0'}
match = last_response.location.match(%r{/tasks/no_content_length})
expect(match).to_not be_nil
end
Expand Down Expand Up @@ -380,7 +380,7 @@ def manifest_with_errand(deployment_name='errand')
create(:deployment => deployment, :job => 'dea',
:index => '0', :state => 'started', :variable_set => Models::VariableSet.create(deployment: deployment))

put '/foo/jobs/dea/0?state=started', "}}}i'm not really yaml, hah!", {'CONTENT_TYPE' => 'text/yaml', 'CONTENT_LENGTH' => 0}
put '/foo/jobs/dea/0?state=started', "}}}i'm not really yaml, hah!", {'CONTENT_TYPE' => 'text/yaml', 'CONTENT_LENGTH' => '0'}

expect(last_response.status).to eq(302)
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ module Api
describe Controllers::DisksController do
include Rack::Test::Methods

subject(:app) { described_class.new(config) }
subject(:app) { linted_rack_app(described_class.new(config)) }
let(:config) { Config.load_hash(SpecHelper.spec_get_director_config) }

let(:orphaned_at) { Time.now.utc }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ module Api
describe Controllers::EventsController do
include Rack::Test::Methods

subject(:app) { described_class.new(config) }
subject(:app) { linted_rack_app(described_class.new(config)) }

let(:config) { Config.load_hash(SpecHelper.spec_get_director_config) }
let(:timestamp) { Time.now }

Expand Down Expand Up @@ -502,6 +503,8 @@ def perform

it 'stores event' do
expect { perform }.not_to raise_exception
expect(last_response.status).to eq(200)
expect(last_response.body).to eq('')
event = Models::Event.first
expect(event.id).to eq(1)
expect(event.parent_id).to eq(nil)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ module Api
describe Controllers::InfoController do
include Rack::Test::Methods

subject(:app) { described_class.new(config) }
subject(:app) { linted_rack_app(described_class.new(config)) }
let(:config) { Config.load_hash(test_config) }
let(:base_config) { SpecHelper.spec_get_director_config }
let(:test_config) { base_config }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ module Api
describe Controllers::JobsController do
include Rack::Test::Methods

subject(:app) { described_class.new(config) }
subject(:app) { linted_rack_app(described_class.new(config)) }
let(:config) do
config = Config.load_hash(SpecHelper.spec_get_director_config)
identity_provider = Support::TestIdentityProvider.new(config.get_uuid_provider)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ module Bosh::Director
describe Api::Controllers::LocksController do
include Rack::Test::Methods

subject(:app) { described_class.new(config) }
subject(:app) { linted_rack_app(described_class.new(config)) }
let(:config) do
config = Config.load_hash(SpecHelper.spec_get_director_config)
identity_provider = Support::TestIdentityProvider.new(config.get_uuid_provider)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ module Api
describe Controllers::OrphanDisksController do
include Rack::Test::Methods

subject(:app) { described_class.new(config) }
subject(:app) { linted_rack_app(described_class.new(config)) }
let(:config) { Config.load_hash(SpecHelper.spec_get_director_config) }

let(:orphaned_at) { Time.now.utc }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ module Api
describe Controllers::PackagesController do
include Rack::Test::Methods

subject(:app) { described_class.new(config) }
subject(:app) { linted_rack_app(described_class.new(config)) }
before { allow(Api::ResourceManager).to receive(:new) }
let(:config) { Config.load_hash(SpecHelper.spec_get_director_config) }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ module Api
describe Controllers::ReleasesController do
include Rack::Test::Methods

subject(:app) { described_class.new(config) }
subject(:app) { linted_rack_app(described_class.new(config)) }
let(:config) do
config = Config.load_hash(SpecHelper.spec_get_director_config)
identity_provider = Support::TestIdentityProvider.new(config.get_uuid_provider)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ module Api

let(:blobstore) { double('client') }
let(:resource_manager) { ResourceManager.new(blobstore) }
subject(:app) { described_class.new(config, resource_manager) }
subject(:app) { linted_rack_app(described_class.new(config, resource_manager)) }

let(:config) do
config = SpecHelper.spec_get_director_config
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ module Api
describe Controllers::RestoreController do
include Rack::Test::Methods

subject(:app) { described_class.new(config) }
subject(:app) { linted_rack_app(described_class.new(config)) }
let(:config) { Config.load_hash(test_config) }
let(:test_config) do
config = YAML.load(spec_asset('test-director-config.yml'))
Expand All @@ -20,7 +20,11 @@ module Api
config
end

before { App.new(config) }
before do
allow(File).to receive(:read).and_call_original
allow(File).to receive(:read).with('/path/to/server_ca_path').and_return('whatever makes you happy')
App.new(config)
end

it 'requires auth' do
post '/', 'fake-data', { 'CONTENT_TYPE' => 'multipart/form-data' }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ module Api
describe Controllers::ResurrectionController do
include Rack::Test::Methods

subject(:app) { described_class.new(config) }
subject(:app) { linted_rack_app(described_class.new(config)) }
let(:config) { Config.load_hash(SpecHelper.spec_get_director_config) }
before { basic_authorize 'admin', 'admin' }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ module Bosh::Director
describe Api::Controllers::RuntimeConfigsController do
include Rack::Test::Methods

subject(:app) { Api::Controllers::RuntimeConfigsController.new(config) }
subject(:app) { linted_rack_app(described_class.new(config)) }
let(:config) do
config = Config.load_hash(SpecHelper.spec_get_director_config)
identity_provider = Support::TestIdentityProvider.new(config.get_uuid_provider)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ module Bosh::Director
describe Api::Controllers::StemcellsController do
include Rack::Test::Methods

subject(:app) { described_class.new(config) }
subject(:app) { linted_rack_app(described_class.new(config)) }
let(:config) do
config = Config.load_hash(SpecHelper.spec_get_director_config)
identity_provider = Support::TestIdentityProvider.new(config.get_uuid_provider)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ module Api
describe Controllers::TaskController do
include Rack::Test::Methods

subject(:app) { described_class.new(config) }
subject(:app) { linted_rack_app(described_class.new(config)) }
let(:config) { Config.load_hash(SpecHelper.spec_get_director_config) }

it 'requires auth' do
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ module Api
describe Controllers::TasksController do
include Rack::Test::Methods

subject(:app) { described_class.new(config) }
subject(:app) { linted_rack_app(described_class.new(config)) }

let(:temp_dir) { Dir.mktmpdir }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ module Api
describe Controllers::VmsController do
include Rack::Test::Methods

subject(:app) { described_class.new(config) }
subject(:app) { linted_rack_app(described_class.new(config)) }
let(:config) { Config.load_hash(SpecHelper.spec_get_director_config) }

before do
Expand Down

0 comments on commit adfeab6

Please sign in to comment.