Skip to content

Commit 6381ed8

Browse files
committed
Remove name field from access rules, add read-only relationships per RFC updates
Update /v3/access_rules API to align with latest RFC changes: - Remove 'name' field from RouteAccessRule model and API - Add database migration to drop name column and unique index - Use labels/annotations for metadata instead of name field - Add read-only relationships (app, space, organization) to responses extracted from selector (cf:app:X, cf:space:X, cf:org:X, cf:any) - Replace 'names' filter with 'guids' filter - Add 'selector_resource_guids' filter for text-match against selectors - Update include support: add individual app, space, organization (in addition to existing selector_resource and route) - Remove name-based uniqueness validation (keep selector uniqueness) - Update all tests to remove name references Breaking changes: - POST /v3/access_rules no longer accepts 'name' field - GET /v3/access_rules responses no longer include 'name' field - Filter parameter 'names' removed, use 'guids' instead - Access rule responses now include app/space/organization relationships
1 parent e641586 commit 6381ed8

9 files changed

Lines changed: 125 additions & 73 deletions

app/controllers/v3/access_rules_controller.rb

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -52,13 +52,11 @@ def create
5252
unprocessable!("Cannot add 'cf:any' selector when other access rules already exist for this route.") if message.selector == 'cf:any' && existing_selectors.any?
5353
unprocessable!("Cannot add selector '#{message.selector}': route already has a 'cf:any' rule.") if existing_selectors.include?('cf:any') && message.selector != 'cf:any'
5454

55-
# Uniqueness: name and selector must be unique per route
56-
unprocessable!("An access rule with name '#{message.name}' already exists for this route.") if route.access_rules.any? { |r| r.name == message.name }
55+
# Uniqueness: selector must be unique per route
5756
unprocessable!("An access rule with selector '#{message.selector}' already exists for this route.") if existing_selectors.include?(message.selector)
5857

5958
access_rule = VCAP::CloudController::RouteAccessRule.new(
6059
guid: SecureRandom.uuid,
61-
name: message.name,
6260
selector: message.selector,
6361
route_id: route.id,
6462
created_at: Time.now.utc,
@@ -127,9 +125,18 @@ def build_dataset(message)
127125
select_all(:route_access_rules)
128126
end
129127

130-
dataset = dataset.where(name: message.names) if message.requested?(:names)
128+
dataset = dataset.where(guid: message.guids) if message.requested?(:guids)
131129
dataset = dataset.where(selector: message.selectors) if message.requested?(:selectors)
132130

131+
if message.requested?(:selector_resource_guids)
132+
# Text-match against selector string for resource GUIDs
133+
# Handles cf:app:<guid>, cf:space:<guid>, cf:org:<guid>
134+
conditions = message.selector_resource_guids.map do |guid|
135+
Sequel.like(:selector, "%#{guid}%")
136+
end
137+
dataset = dataset.where(Sequel.|(*conditions))
138+
end
139+
133140
dataset
134141
end
135142
end

app/decorators/include_access_rule_selector_resource_decorator.rb

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,15 @@ class IncludeAccessRuleSelectorResourceDecorator
66
SELECTOR_REGEX = /\Acf:(app|space|org):([0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12})\z/
77

88
def self.match?(include_params)
9-
include_params&.include?('selector_resource')
9+
return false unless include_params
10+
11+
# Match if any of: selector_resource, app, space, organization
12+
(include_params & %w[selector_resource app space organization]).any?
1013
end
1114

1215
def self.decorate(hash, access_rules)
1316
hash[:included] ||= {}
14-
17+
1518
# Collect all GUIDs by type
1619
app_guids = []
1720
space_guids = []
@@ -38,7 +41,7 @@ def self.decorate(hash, access_rules)
3841
hash[:included][:apps] = fetch_and_present_apps(app_guids.uniq)
3942
hash[:included][:spaces] = fetch_and_present_spaces(space_guids.uniq)
4043
hash[:included][:organizations] = fetch_and_present_organizations(org_guids.uniq)
41-
44+
4245
hash
4346
end
4447

app/messages/access_rule_create_message.rb

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,13 @@ class AccessRuleCreateMessage < MetadataBaseMessage
55
SELECTOR_REGEX = /\A(cf:(app|space|org):[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}|cf:any)\z/
66

77
register_allowed_keys %i[
8-
name
98
selector
109
relationships
1110
]
1211

1312
validates_with NoAdditionalKeysValidator
1413
validates_with RelationshipValidator
1514

16-
validates :name, presence: true, string: true
1715
validates :selector, presence: true, string: true
1816

1917
validate :selector_format_valid

app/messages/access_rules_list_message.rb

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,20 +3,22 @@
33
module VCAP::CloudController
44
class AccessRulesListMessage < ListMessage
55
register_allowed_keys %i[
6+
guids
67
route_guids
78
space_guids
8-
names
99
selectors
10+
selector_resource_guids
1011
include
1112
]
1213

1314
validates_with NoAdditionalParamsValidator
14-
validates_with IncludeParamValidator, valid_values: ['selector_resource', 'route']
15+
validates_with IncludeParamValidator, valid_values: %w[selector_resource route app space organization]
1516

1617
validates :space_guids, array: true, allow_nil: true
18+
validates :selector_resource_guids, array: true, allow_nil: true
1719

1820
def self.from_params(params)
19-
super(params, %w[route_guids space_guids names selectors include])
21+
super(params, %w[guids route_guids space_guids selectors selector_resource_guids include])
2022
end
2123
end
2224
end

app/models/runtime/route_access_rule.rb

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ class RouteAccessRule < Sequel::Model(:route_access_rules)
77
without_guid_generation: true
88

99
def validate
10-
validates_presence :name
1110
validates_presence :selector
1211
validates_presence :route_id
1312
end

app/presenters/v3/access_rule_presenter.rb

Lines changed: 43 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,15 +12,12 @@ def to_hash
1212
guid: access_rule.guid,
1313
created_at: access_rule.created_at,
1414
updated_at: access_rule.updated_at,
15-
name: access_rule.name,
1615
selector: access_rule.selector,
17-
relationships: {
18-
route: {
19-
data: {
20-
guid: access_rule.route.guid
21-
}
22-
}
16+
metadata: {
17+
labels: hashified_labels(access_rule.labels),
18+
annotations: hashified_annotations(access_rule.annotations)
2319
},
20+
relationships: build_relationships,
2421
links: build_links
2522
}
2623
end
@@ -31,6 +28,45 @@ def access_rule
3128
@resource
3229
end
3330

31+
def build_relationships
32+
relationships = {
33+
route: {
34+
data: {
35+
guid: access_rule.route.guid
36+
}
37+
}
38+
}
39+
40+
# Extract resource GUID from selector and populate read-only relationships
41+
selector_match = access_rule.selector.match(/\Acf:(app|space|org):([0-9a-f-]+)\z/)
42+
if selector_match
43+
resource_type = selector_match[1]
44+
resource_guid = selector_match[2]
45+
46+
case resource_type
47+
when 'app'
48+
relationships[:app] = { data: { guid: resource_guid } }
49+
relationships[:space] = { data: nil }
50+
relationships[:organization] = { data: nil }
51+
when 'space'
52+
relationships[:app] = { data: nil }
53+
relationships[:space] = { data: { guid: resource_guid } }
54+
relationships[:organization] = { data: nil }
55+
when 'org'
56+
relationships[:app] = { data: nil }
57+
relationships[:space] = { data: nil }
58+
relationships[:organization] = { data: { guid: resource_guid } }
59+
end
60+
else
61+
# cf:any or malformed - all relationships are null
62+
relationships[:app] = { data: nil }
63+
relationships[:space] = { data: nil }
64+
relationships[:organization] = { data: nil }
65+
end
66+
67+
relationships
68+
end
69+
3470
def build_links
3571
{
3672
self: {
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
Sequel.migration do
2+
up do
3+
alter_table :route_access_rules do
4+
drop_index %i[route_id name], name: :route_access_rules_route_id_name_index
5+
drop_column :name
6+
end
7+
end
8+
9+
down do
10+
alter_table :route_access_rules do
11+
add_column :name, String, size: 255
12+
add_index %i[route_id name], unique: true, name: :route_access_rules_route_id_name_index
13+
end
14+
end
15+
end

0 commit comments

Comments
 (0)