Skip to content

Commit ea99dbb

Browse files
committed
Implement include=selector_resource for /v3/access_rules endpoint
- Add include parameter support to AccessRulesListMessage - Refactor IncludeAccessRuleSelectorResourceDecorator to match RFC format: - Return separate arrays for apps, spaces, organizations instead of selector_resources - Include full resource details using appropriate presenters - Batch resource fetching by type with eager loading - Auto-deduplicate resources - Gracefully handle stale/missing resources - Wire up decorator to AccessRulesController - Add comprehensive request specs for include=selector_resource Fixes: uninitialized constant error by adding proper require statement
1 parent 568bb0b commit ea99dbb

4 files changed

Lines changed: 169 additions & 16 deletions

File tree

app/controllers/v3/access_rules_controller.rb

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
require 'messages/access_rule_update_message'
33
require 'messages/access_rules_list_message'
44
require 'presenters/v3/access_rule_presenter'
5+
require 'decorators/include_access_rule_selector_resource_decorator'
56

67
class AccessRulesController < ApplicationController
78
def index
@@ -10,11 +11,15 @@ def index
1011

1112
dataset = build_dataset(message)
1213

14+
decorators = []
15+
decorators << IncludeAccessRuleSelectorResourceDecorator if IncludeAccessRuleSelectorResourceDecorator.match?(message.include)
16+
1317
render status: :ok, json: Presenters::V3::PaginatedListPresenter.new(
1418
presenter: Presenters::V3::AccessRulePresenter,
1519
paginated_result: SequelPaginator.new.get_page(dataset, message.try(:pagination_options)),
1620
path: '/v3/access_rules',
17-
message: message
21+
message: message,
22+
decorators: decorators
1823
)
1924
end
2025

app/decorators/include_access_rule_selector_resource_decorator.rb

Lines changed: 46 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,12 @@ def self.match?(include_params)
1010
end
1111

1212
def self.decorate(hash, access_rules)
13-
included = []
13+
hash[:included] ||= {}
14+
15+
# Collect all GUIDs by type
16+
app_guids = []
17+
space_guids = []
18+
org_guids = []
1419

1520
access_rules.each do |rule|
1621
match = SELECTOR_REGEX.match(rule.selector)
@@ -19,22 +24,49 @@ def self.decorate(hash, access_rules)
1924
resource_type = match[1]
2025
resource_guid = match[2]
2126

22-
resource = case resource_type
23-
when 'app'
24-
VCAP::CloudController::AppModel.find(guid: resource_guid)
25-
when 'space'
26-
VCAP::CloudController::Space.find(guid: resource_guid)
27-
when 'org'
28-
VCAP::CloudController::Organization.find(guid: resource_guid)
29-
end
30-
31-
next if resource.nil?
32-
33-
included << { type: resource_type, guid: resource.guid }
27+
case resource_type
28+
when 'app'
29+
app_guids << resource_guid
30+
when 'space'
31+
space_guids << resource_guid
32+
when 'org'
33+
org_guids << resource_guid
34+
end
3435
end
3536

36-
hash[:included] = { selector_resources: included }
37+
# Fetch and present resources
38+
hash[:included][:apps] = fetch_and_present_apps(app_guids.uniq)
39+
hash[:included][:spaces] = fetch_and_present_spaces(space_guids.uniq)
40+
hash[:included][:organizations] = fetch_and_present_organizations(org_guids.uniq)
41+
3742
hash
3843
end
44+
45+
private_class_method def self.fetch_and_present_apps(guids)
46+
return [] if guids.empty?
47+
48+
apps = VCAP::CloudController::AppModel.where(guid: guids).
49+
order(:created_at, :guid).
50+
eager(VCAP::CloudController::Presenters::V3::AppPresenter.associated_resources).all
51+
apps.map { |app| VCAP::CloudController::Presenters::V3::AppPresenter.new(app).to_hash }
52+
end
53+
54+
private_class_method def self.fetch_and_present_spaces(guids)
55+
return [] if guids.empty?
56+
57+
spaces = VCAP::CloudController::Space.where(guid: guids).
58+
order(:created_at, :guid).
59+
eager(VCAP::CloudController::Presenters::V3::SpacePresenter.associated_resources).all
60+
spaces.map { |space| VCAP::CloudController::Presenters::V3::SpacePresenter.new(space).to_hash }
61+
end
62+
63+
private_class_method def self.fetch_and_present_organizations(guids)
64+
return [] if guids.empty?
65+
66+
orgs = VCAP::CloudController::Organization.where(guid: guids).
67+
order(:created_at, :guid).
68+
eager(VCAP::CloudController::Presenters::V3::OrganizationPresenter.associated_resources).all
69+
orgs.map { |org| VCAP::CloudController::Presenters::V3::OrganizationPresenter.new(org).to_hash }
70+
end
3971
end
4072
end

app/messages/access_rules_list_message.rb

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,14 @@ class AccessRulesListMessage < ListMessage
66
route_guids
77
names
88
selectors
9+
include
910
]
1011

1112
validates_with NoAdditionalParamsValidator
13+
validates_with IncludeParamValidator, valid_values: ['selector_resource', 'route']
1214

1315
def self.from_params(params)
14-
super(params, %w[route_guids names selectors])
16+
super(params, %w[route_guids names selectors include])
1517
end
1618
end
1719
end

spec/request/access_rules_spec.rb

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -300,6 +300,120 @@ def expected_rule_json(rule)
300300
expect(parsed['resources'].length).to eq(1)
301301
expect(parsed['resources'][0]['selector']).to eq('cf:any')
302302
end
303+
304+
context 'with include=selector_resource' do
305+
let!(:app) { VCAP::CloudController::AppModel.make(space: space, name: 'frontend-app') }
306+
let!(:other_space) { VCAP::CloudController::Space.make(organization: org, name: 'other-space') }
307+
let!(:other_org) { VCAP::CloudController::Organization.make(name: 'other-org') }
308+
309+
let!(:app_rule) do
310+
VCAP::CloudController::RouteAccessRule.create(
311+
guid: SecureRandom.uuid,
312+
name: 'app-rule',
313+
selector: "cf:app:#{app.guid}",
314+
route_id: mtls_route.id
315+
)
316+
end
317+
318+
let!(:space_rule) do
319+
VCAP::CloudController::RouteAccessRule.create(
320+
guid: SecureRandom.uuid,
321+
name: 'space-rule',
322+
selector: "cf:space:#{other_space.guid}",
323+
route_id: mtls_route.id
324+
)
325+
end
326+
327+
let!(:org_rule) do
328+
VCAP::CloudController::RouteAccessRule.create(
329+
guid: SecureRandom.uuid,
330+
name: 'org-rule',
331+
selector: "cf:org:#{other_org.guid}",
332+
route_id: mtls_route.id
333+
)
334+
end
335+
336+
it 'includes resolved selector resources' do
337+
get '/v3/access_rules?include=selector_resource', nil, admin_header
338+
339+
expect(last_response.status).to eq(200)
340+
parsed = Oj.load(last_response.body)
341+
342+
# Check included structure
343+
expect(parsed['included']).to be_a(Hash)
344+
expect(parsed['included']['apps']).to be_an(Array)
345+
expect(parsed['included']['spaces']).to be_an(Array)
346+
expect(parsed['included']['organizations']).to be_an(Array)
347+
348+
# Check app is included with full details
349+
app_included = parsed['included']['apps'].find { |a| a['guid'] == app.guid }
350+
expect(app_included).to be_present
351+
expect(app_included['name']).to eq('frontend-app')
352+
expect(app_included['guid']).to eq(app.guid)
353+
354+
# Check space is included
355+
space_included = parsed['included']['spaces'].find { |s| s['guid'] == other_space.guid }
356+
expect(space_included).to be_present
357+
expect(space_included['name']).to eq('other-space')
358+
359+
# Check org is included
360+
org_included = parsed['included']['organizations'].find { |o| o['guid'] == other_org.guid }
361+
expect(org_included).to be_present
362+
expect(org_included['name']).to eq('other-org')
363+
end
364+
365+
it 'handles stale resources (missing GUIDs) gracefully' do
366+
stale_guid = '99999999-9999-9999-9999-999999999999'
367+
VCAP::CloudController::RouteAccessRule.create(
368+
guid: SecureRandom.uuid,
369+
name: 'stale-rule',
370+
selector: "cf:app:#{stale_guid}",
371+
route_id: mtls_route.id
372+
)
373+
374+
get '/v3/access_rules?include=selector_resource', nil, admin_header
375+
376+
expect(last_response.status).to eq(200)
377+
parsed = Oj.load(last_response.body)
378+
379+
# Stale resource should not appear in included
380+
stale_app = parsed['included']['apps'].find { |a| a['guid'] == stale_guid }
381+
expect(stale_app).to be_nil
382+
end
383+
384+
it 'includes only unique resources when multiple rules reference the same resource' do
385+
# Create another rule referencing the same app
386+
VCAP::CloudController::RouteAccessRule.create(
387+
guid: SecureRandom.uuid,
388+
name: 'another-app-rule',
389+
selector: "cf:app:#{app.guid}",
390+
route_id: VCAP::CloudController::Route.make(space: space, domain: mtls_domain).id
391+
)
392+
393+
get '/v3/access_rules?include=selector_resource', nil, admin_header
394+
395+
expect(last_response.status).to eq(200)
396+
parsed = Oj.load(last_response.body)
397+
398+
# App should appear only once
399+
app_count = parsed['included']['apps'].count { |a| a['guid'] == app.guid }
400+
expect(app_count).to eq(1)
401+
end
402+
403+
it 'does not include resources for cf:any selectors' do
404+
VCAP::CloudController::RouteAccessRule.create(
405+
guid: SecureRandom.uuid,
406+
name: 'any-rule',
407+
selector: 'cf:any',
408+
route_id: VCAP::CloudController::Route.make(space: space, domain: mtls_domain).id
409+
)
410+
411+
get '/v3/access_rules?include=selector_resource', nil, admin_header
412+
413+
expect(last_response.status).to eq(200)
414+
# Should succeed without error even with cf:any selector
415+
end
416+
end
303417
end
304418

305419
describe 'DELETE /v3/access_rules/:guid' do

0 commit comments

Comments
 (0)