Skip to content

Commit 67eb5cf

Browse files
authored
Can't push apps with app manifests if routes were shared (#4344)
* Can't push apps with app manifests if routes were shared Check if route shared with space and don't throw error if shared * Can't push apps with app manifests if routes were shared Fix robocop errors * Can't push apps with app manifests if routes were shared Fix rubocop error * Can't push apps with app manifests if routes were shared Move app and route space match to common method * Can't push apps with app manifests if routes were shared Fix rubocop errors * Can't push apps with app manifests if routes were shared Rename method based on code review
1 parent 0a8b92a commit 67eb5cf

File tree

5 files changed

+48
-2
lines changed

5 files changed

+48
-2
lines changed

app/actions/manifest_route_update.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ def find_or_create_valid_route(app, manifest_route, user_audit_info)
9292
domain: existing_domain,
9393
manifest_triggered: true
9494
)
95-
elsif route.space.guid != app.space_guid
95+
elsif !route.available_in_space?(app.space)
9696
raise InvalidRoute.new('Routes cannot be mapped to destinations in different spaces')
9797
elsif manifest_route[:options] && route[:options] != manifest_route[:options]
9898
# remove nil values from options

app/controllers/v3/routes_controller.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -362,7 +362,7 @@ def validate_app_guids!(apps_hash, desired_app_guids)
362362
end
363363

364364
def validate_app_spaces!(apps_hash, route)
365-
return unless apps_hash.values.any? { |app| app.space != route.space && route.shared_spaces.exclude?(app.space) }
365+
return unless apps_hash.values.any? { |app| !route.available_in_space?(app.space) }
366366

367367
unprocessable!("Routes destinations must be in either the route's space or the route's shared spaces")
368368
end

app/models/runtime/route.rb

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,10 @@ def self.user_visibility_filter(user)
194194
}
195195
end
196196

197+
def available_in_space?(other_space)
198+
other_space == space || shared_spaces.include?(other_space)
199+
end
200+
197201
delegate :in_suspended_org?, to: :space
198202

199203
def tcp?

spec/unit/actions/manifest_route_update_spec.rb

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,18 @@ module VCAP::CloudController
121121
'Routes cannot be mapped to destinations in different spaces')
122122
end
123123
end
124+
125+
context 'when the route is shared' do
126+
let!(:route_share) { RouteShare.new }
127+
let!(:outside_app) { AppModel.make }
128+
let!(:shared_route) { route_share.create(route, [outside_app.space], user_audit_info) }
129+
130+
it 'succeeds after route share' do
131+
expect do
132+
ManifestRouteUpdate.update(outside_app.guid, message, user_audit_info)
133+
end.not_to raise_error
134+
end
135+
end
124136
end
125137
end
126138

spec/unit/models/runtime/route_spec.rb

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1587,5 +1587,35 @@ def assert_invalid_path(path)
15871587
end
15881588
end
15891589
end
1590+
1591+
describe 'app spaces and route shared spaces' do
1592+
let!(:domain) { SharedDomain.make }
1593+
1594+
context 'when app and route space not shared' do
1595+
let!(:app) { AppModel.make }
1596+
let!(:route) { Route.make(host: 'potato', domain: domain, path: '/some-path') }
1597+
1598+
it 'no space match and not shared and returns false' do
1599+
expect(route.available_in_space?(app.space)).to be(false)
1600+
end
1601+
1602+
it 'match space and returns true' do
1603+
route.space = app.space
1604+
expect(route.available_in_space?(app.space)).to be(true)
1605+
end
1606+
end
1607+
1608+
context 'when app and route space shared' do
1609+
let!(:app) { AppModel.make }
1610+
let!(:route_share) { RouteShare.new }
1611+
let(:user_audit_info) { instance_double(UserAuditInfo).as_null_object }
1612+
let!(:route) { Route.make(host: 'potato', domain: domain, path: '/some-path') }
1613+
let!(:shared_route) { route_share.create(route, [app.space], user_audit_info) }
1614+
1615+
it 'shared space match and returns true' do
1616+
expect(route.available_in_space?(app.space)).to be(true)
1617+
end
1618+
end
1619+
end
15901620
end
15911621
end

0 commit comments

Comments
 (0)