Skip to content

Commit 3faab23

Browse files
rkosterrkoster
committed
Remove access rule names per RFC updates
Per RFC commits 882b69a and 11752f2, access rules no longer have user-provided names. They are identified by their selector only, with labels/annotations used for metadata instead. Changes: - Removed RULE_NAME argument from add-access-rule command - Removed Name field from AccessRule API resource - Updated access-rules list to show 4 columns (route, selector, scope, source) - SourceName now represents resolved app/space/org name from selector - Updated remove-access-rule to use --selector flag instead of rule name - Renamed DeleteAccessRule() to DeleteAccessRuleBySelector() - Updated all tests to remove Name field references All tests passing.
1 parent c69bbdb commit 3faab23

12 files changed

Lines changed: 134 additions & 165 deletions

‎actor/actionerror/access_rule_not_found_error.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,9 @@ package actionerror
33
import "fmt"
44

55
type AccessRuleNotFoundError struct {
6-
Name string
6+
Selector string
77
}
88

99
func (e AccessRuleNotFoundError) Error() string {
10-
return fmt.Sprintf("Access rule '%s' not found.", e.Name)
10+
return fmt.Sprintf("Access rule with selector '%s' not found.", e.Selector)
1111
}

‎actor/v7action/access_rule.go‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import (
66
"code.cloudfoundry.org/cli/v9/resources"
77
)
88

9-
func (actor Actor) AddAccessRule(ruleName, domainName, selector, hostname, path string) (Warnings, error) {
9+
func (actor Actor) AddAccessRule(domainName, selector, hostname, path string) (Warnings, error) {
1010
allWarnings := Warnings{}
1111

1212
// Get the domain to ensure it exists and supports access rules
@@ -35,7 +35,6 @@ func (actor Actor) AddAccessRule(ruleName, domainName, selector, hostname, path
3535

3636
// Create the access rule
3737
accessRule := resources.AccessRule{
38-
Name: ruleName,
3938
Selector: selector,
4039
RouteGUID: route.GUID,
4140
}
@@ -87,7 +86,7 @@ func (actor Actor) GetAccessRulesByRoute(domainName, hostname, path string) ([]r
8786
return rules, allWarnings, err
8887
}
8988

90-
func (actor Actor) DeleteAccessRule(ruleName, domainName, hostname, path string) (Warnings, error) {
89+
func (actor Actor) DeleteAccessRuleBySelector(domainName, selector, hostname, path string) (Warnings, error) {
9190
allWarnings := Warnings{}
9291

9392
// Get the domain
@@ -114,7 +113,7 @@ func (actor Actor) DeleteAccessRule(ruleName, domainName, hostname, path string)
114113

115114
route := routes[0]
116115

117-
// Get access rules for this route to find the one with matching name
116+
// Get access rules for this route to find the one with matching selector
118117
accessRules, _, apiWarnings, err := actor.CloudControllerClient.GetAccessRules(
119118
ccv3.Query{Key: ccv3.RouteGUIDFilter, Values: []string{route.GUID}},
120119
)
@@ -123,17 +122,17 @@ func (actor Actor) DeleteAccessRule(ruleName, domainName, hostname, path string)
123122
return allWarnings, err
124123
}
125124

126-
// Find the rule with matching name
125+
// Find the rule with matching selector
127126
var ruleGUID string
128127
for _, rule := range accessRules {
129-
if rule.Name == ruleName {
128+
if rule.Selector == selector {
130129
ruleGUID = rule.GUID
131130
break
132131
}
133132
}
134133

135134
if ruleGUID == "" {
136-
return allWarnings, actionerror.AccessRuleNotFoundError{Name: ruleName}
135+
return allWarnings, actionerror.AccessRuleNotFoundError{Selector: selector}
137136
}
138137

139138
// Delete the access rule
@@ -143,6 +142,7 @@ func (actor Actor) DeleteAccessRule(ruleName, domainName, hostname, path string)
143142
return allWarnings, err
144143
}
145144

145+
146146
// GetRoutesByDomain gets routes for a domain with optional hostname and path filters
147147
func (actor Actor) GetRoutesByDomain(domainGUID, hostname, path string) ([]resources.Route, Warnings, error) {
148148
queries := []ccv3.Query{

‎actor/v7action/access_rule_test.go‎

Lines changed: 37 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -122,13 +122,12 @@ var _ = Describe("Access Rule Actions", func() {
122122
"get-domain-warning-1",
123123
"get-domain-warning-2",
124124
"get-app-warning",
125-
))
125+
))
126126

127-
Expect(results).To(HaveLen(2))
127+
Expect(results).To(HaveLen(2))
128128

129-
// First rule
129+
// First rule
130130
Expect(results[0].GUID).To(Equal("rule-guid-1"))
131-
Expect(results[0].Name).To(Equal("rule-1"))
132131
Expect(results[0].Selector).To(Equal("cf:app:app-guid-1"))
133132
Expect(results[0].Route.GUID).To(Equal("route-guid-1"))
134133
Expect(results[0].Route.Host).To(Equal("app1"))
@@ -139,7 +138,6 @@ var _ = Describe("Access Rule Actions", func() {
139138

140139
// Second rule
141140
Expect(results[1].GUID).To(Equal("rule-guid-2"))
142-
Expect(results[1].Name).To(Equal("rule-2"))
143141
Expect(results[1].Selector).To(Equal("cf:any"))
144142
Expect(results[1].Route.GUID).To(Equal("route-guid-2"))
145143
Expect(results[1].Route.Host).To(Equal("app2"))
@@ -498,8 +496,8 @@ var _ = Describe("Access Rule Actions", func() {
498496
It("returns the access rules", func() {
499497
Expect(executeErr).ToNot(HaveOccurred())
500498
Expect(rules).To(HaveLen(2))
501-
Expect(rules[0].Name).To(Equal("rule-1"))
502-
Expect(rules[1].Name).To(Equal("rule-2"))
499+
Expect(rules[0].Selector).To(Equal("cf:app:app-guid-1"))
500+
Expect(rules[1].Selector).To(Equal("cf:app:app-guid-2"))
503501
Expect(warnings).To(ConsistOf(
504502
"get-domains-warning",
505503
"get-routes-warning",
@@ -538,7 +536,6 @@ var _ = Describe("Access Rule Actions", func() {
538536

539537
Describe("AddAccessRule", func() {
540538
var (
541-
ruleName string
542539
domainName string
543540
selector string
544541
hostname string
@@ -549,15 +546,14 @@ var _ = Describe("Access Rule Actions", func() {
549546
)
550547

551548
BeforeEach(func() {
552-
ruleName = "my-rule"
553549
domainName = "example.com"
554550
selector = "cf:app:app-guid-1"
555551
hostname = "myapp"
556552
path = ""
557553
})
558554

559555
JustBeforeEach(func() {
560-
warnings, executeErr = actor.AddAccessRule(ruleName, domainName, selector, hostname, path)
556+
warnings, executeErr = actor.AddAccessRule(domainName, selector, hostname, path)
561557
})
562558

563559
When("creating the access rule succeeds", func() {
@@ -594,16 +590,15 @@ var _ = Describe("Access Rule Actions", func() {
594590
Expect(executeErr).ToNot(HaveOccurred())
595591
Expect(warnings).To(ConsistOf(
596592
"get-domains-warning",
597-
"get-routes-warning",
598-
"create-rule-warning",
599-
))
600-
601-
Expect(fakeCloudControllerClient.CreateAccessRuleCallCount()).To(Equal(1))
602-
rule := fakeCloudControllerClient.CreateAccessRuleArgsForCall(0)
603-
Expect(rule.Name).To(Equal("my-rule"))
604-
Expect(rule.Selector).To(Equal("cf:app:app-guid-1"))
605-
Expect(rule.RouteGUID).To(Equal("route-guid-1"))
606-
})
593+
"get-routes-warning",
594+
"create-rule-warning",
595+
))
596+
597+
Expect(fakeCloudControllerClient.CreateAccessRuleCallCount()).To(Equal(1))
598+
rule := fakeCloudControllerClient.CreateAccessRuleArgsForCall(0)
599+
Expect(rule.Selector).To(Equal("cf:app:app-guid-1"))
600+
Expect(rule.RouteGUID).To(Equal("route-guid-1"))
601+
})
607602
})
608603

609604
When("the route does not exist", func() {
@@ -633,9 +628,9 @@ var _ = Describe("Access Rule Actions", func() {
633628
})
634629
})
635630

636-
Describe("DeleteAccessRule", func() {
631+
Describe("DeleteAccessRuleBySelector", func() {
637632
var (
638-
ruleName string
633+
selector string
639634
domainName string
640635
hostname string
641636
path string
@@ -645,14 +640,14 @@ var _ = Describe("Access Rule Actions", func() {
645640
)
646641

647642
BeforeEach(func() {
648-
ruleName = "my-rule"
643+
selector = "cf:any"
649644
domainName = "example.com"
650645
hostname = "myapp"
651646
path = ""
652647
})
653648

654649
JustBeforeEach(func() {
655-
warnings, executeErr = actor.DeleteAccessRule(ruleName, domainName, hostname, path)
650+
warnings, executeErr = actor.DeleteAccessRuleBySelector(domainName, selector, hostname, path)
656651
})
657652

658653
When("the access rule exists", func() {
@@ -680,7 +675,7 @@ var _ = Describe("Access Rule Actions", func() {
680675

681676
fakeCloudControllerClient.GetAccessRulesReturns(
682677
[]resources.AccessRule{
683-
{GUID: "rule-guid-1", Name: "my-rule", Selector: "cf:any"},
678+
{GUID: "rule-guid-1", Selector: "cf:any"},
684679
},
685680
ccv3.IncludedResources{},
686681
ccv3.Warnings{"get-access-rules-warning"},
@@ -732,24 +727,24 @@ var _ = Describe("Access Rule Actions", func() {
732727
nil,
733728
)
734729

735-
fakeCloudControllerClient.GetAccessRulesReturns(
736-
[]resources.AccessRule{
737-
{GUID: "rule-guid-other", Name: "other-rule", Selector: "cf:any"},
738-
},
739-
ccv3.IncludedResources{},
740-
ccv3.Warnings{"get-access-rules-warning"},
741-
nil,
742-
)
743-
})
730+
fakeCloudControllerClient.GetAccessRulesReturns(
731+
[]resources.AccessRule{
732+
{GUID: "rule-guid-other", Selector: "cf:app:other-guid"},
733+
},
734+
ccv3.IncludedResources{},
735+
ccv3.Warnings{"get-access-rules-warning"},
736+
nil,
737+
)
738+
})
744739

745-
It("returns an AccessRuleNotFoundError", func() {
746-
Expect(executeErr).To(MatchError(actionerror.AccessRuleNotFoundError{Name: "my-rule"}))
747-
Expect(warnings).To(ConsistOf(
748-
"get-domains-warning",
749-
"get-routes-warning",
750-
"get-access-rules-warning",
751-
))
752-
})
740+
It("returns an AccessRuleNotFoundError", func() {
741+
Expect(executeErr).To(MatchError(actionerror.AccessRuleNotFoundError{Selector: "cf:any"}))
742+
Expect(warnings).To(ConsistOf(
743+
"get-domains-warning",
744+
"get-routes-warning",
745+
"get-access-rules-warning",
746+
))
753747
})
754748
})
755749
})
750+
})

‎command/flag/arguments.go‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -415,11 +415,9 @@ type TaskArgs struct {
415415
}
416416

417417
type AddAccessRuleArgs struct {
418-
RuleName string `positional-arg-name:"RULE_NAME" required:"true" description:"The access rule name"`
419-
Domain string `positional-arg-name:"DOMAIN" required:"true" description:"The domain name"`
418+
Domain string `positional-arg-name:"DOMAIN" required:"true" description:"The domain name"`
420419
}
421420

422421
type RemoveAccessRuleArgs struct {
423-
RuleName string `positional-arg-name:"RULE_NAME" required:"true" description:"The access rule name"`
424-
Domain string `positional-arg-name:"DOMAIN" required:"true" description:"The domain name"`
422+
Domain string `positional-arg-name:"DOMAIN" required:"true" description:"The domain name"`
425423
}

‎command/v7/access_rules_command.go‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,6 @@ func (cmd AccessRulesCommand) Execute(args []string) error {
6464
// Build table data
6565
table := [][]string{
6666
{
67-
cmd.UI.TranslateText("name"),
6867
cmd.UI.TranslateText("route"),
6968
cmd.UI.TranslateText("selector"),
7069
cmd.UI.TranslateText("scope"),
@@ -74,7 +73,6 @@ func (cmd AccessRulesCommand) Execute(args []string) error {
7473

7574
for _, ruleWithRoute := range rulesWithRoutes {
7675
table = append(table, []string{
77-
ruleWithRoute.Name,
7876
formatRoute(ruleWithRoute.Route, ruleWithRoute.DomainName),
7977
ruleWithRoute.Selector,
8078
ruleWithRoute.ScopeType,

‎command/v7/access_rules_command_test.go‎

Lines changed: 11 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,6 @@ var _ = Describe("access-rules Command", func() {
114114
{
115115
AccessRule: resources.AccessRule{
116116
GUID: "rule-guid-1",
117-
Name: "rule-1",
118117
Selector: "cf:app:app-guid-1",
119118
},
120119
Route: resources.Route{
@@ -129,7 +128,6 @@ var _ = Describe("access-rules Command", func() {
129128
{
130129
AccessRule: resources.AccessRule{
131130
GUID: "rule-guid-2",
132-
Name: "rule-2",
133131
Selector: "cf:any",
134132
},
135133
Route: resources.Route{
@@ -139,7 +137,7 @@ var _ = Describe("access-rules Command", func() {
139137
},
140138
DomainName: "test.com",
141139
ScopeType: "any",
142-
SourceName: "(any app)",
140+
SourceName: "",
143141
},
144142
}, v7action.Warnings{"warning-1"}, nil)
145143
})
@@ -148,9 +146,9 @@ var _ = Describe("access-rules Command", func() {
148146
Expect(executeErr).ToNot(HaveOccurred())
149147

150148
Expect(testUI.Out).To(Say(`Getting access rules in org some-org / space some-space as steve\.\.\.`))
151-
Expect(testUI.Out).To(Say(`name\s+route\s+selector\s+scope\s+source`))
152-
Expect(testUI.Out).To(Say(`rule-1\s+myapp\.example\.com/api\s+cf:app:app-guid-1\s+my-app`))
153-
Expect(testUI.Out).To(Say(`rule-2\s+webapp\.test\.com\s+cf:any\s+\(any app\)`))
149+
Expect(testUI.Out).To(Say(`route\s+selector\s+scope\s+source`))
150+
Expect(testUI.Out).To(Say(`myapp\.example\.com/api\s+cf:app:app-guid-1\s+app\s+my-app`))
151+
Expect(testUI.Out).To(Say(`webapp\.test\.com\s+cf:any\s+any`))
154152

155153
Expect(testUI.Err).To(Say("warning-1"))
156154

@@ -185,7 +183,6 @@ var _ = Describe("access-rules Command", func() {
185183
{
186184
AccessRule: resources.AccessRule{
187185
GUID: "rule-guid-1",
188-
Name: "rule-1",
189186
Selector: "cf:any",
190187
},
191188
Route: resources.Route{
@@ -194,7 +191,7 @@ var _ = Describe("access-rules Command", func() {
194191
},
195192
DomainName: "example.com",
196193
ScopeType: "any",
197-
SourceName: "(any app)",
194+
SourceName: "",
198195
},
199196
}, v7action.Warnings{}, nil)
200197
})
@@ -271,7 +268,6 @@ var _ = Describe("access-rules Command", func() {
271268
{
272269
AccessRule: resources.AccessRule{
273270
GUID: "rule-guid-1",
274-
Name: "filtered-rule",
275271
Selector: "cf:app:app-guid-1",
276272
},
277273
Route: resources.Route{
@@ -302,8 +298,8 @@ var _ = Describe("access-rules Command", func() {
302298
Expect(executeErr).ToNot(HaveOccurred())
303299

304300
Expect(testUI.Out).To(Say(`Getting access rules in org some-org / space some-space as steve\.\.\.`))
305-
Expect(testUI.Out).To(Say(`name\s+route\s+selector\s+scope\s+source`))
306-
Expect(testUI.Out).To(Say(`filtered-rule\s+myapp\.example\.com/api\s+cf:app:app-guid-1\s+my-app`))
301+
Expect(testUI.Out).To(Say(`route\s+selector\s+scope\s+source`))
302+
Expect(testUI.Out).To(Say(`myapp\.example\.com/api\s+cf:app:app-guid-1\s+app\s+my-app`))
307303
})
308304
})
309305

@@ -313,7 +309,6 @@ var _ = Describe("access-rules Command", func() {
313309
{
314310
AccessRule: resources.AccessRule{
315311
GUID: "rule-guid-1",
316-
Name: "no-host-rule",
317312
Selector: "cf:any",
318313
},
319314
Route: resources.Route{
@@ -323,12 +318,11 @@ var _ = Describe("access-rules Command", func() {
323318
},
324319
DomainName: "example.com",
325320
ScopeType: "any",
326-
SourceName: "(any app)",
321+
SourceName: "",
327322
},
328323
{
329324
AccessRule: resources.AccessRule{
330325
GUID: "rule-guid-2",
331-
Name: "no-path-rule",
332326
Selector: "cf:any",
333327
},
334328
Route: resources.Route{
@@ -338,7 +332,7 @@ var _ = Describe("access-rules Command", func() {
338332
},
339333
DomainName: "test.com",
340334
ScopeType: "any",
341-
SourceName: "(any app)",
335+
SourceName: "",
342336
},
343337
}, v7action.Warnings{}, nil)
344338
})
@@ -347,10 +341,10 @@ var _ = Describe("access-rules Command", func() {
347341
Expect(executeErr).ToNot(HaveOccurred())
348342

349343
// No host, with path: "example.com/api"
350-
Expect(testUI.Out).To(Say(`no-host-rule\s+example\.com/api`))
344+
Expect(testUI.Out).To(Say(`example\.com/api`))
351345

352346
// With host, no path: "myapp.test.com"
353-
Expect(testUI.Out).To(Say(`no-path-rule\s+myapp\.test\.com`))
347+
Expect(testUI.Out).To(Say(`myapp\.test\.com`))
354348
})
355349
})
356350
})

0 commit comments

Comments
 (0)