Skip to content

Commit b140fa8

Browse files
committed
Implemented requests from reviews.
- Corrected grammar in doc.go. - Obfuscating parse failures with a permissions error. These errors should primarily be checked on the client. - Updated unit tests to conform to new param structure.
1 parent d763b17 commit b140fa8

File tree

4 files changed

+20
-15
lines changed

4 files changed

+20
-15
lines changed

api/leadership/client_test.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -63,8 +63,8 @@ func (s *clientSuite) TestClaimLeadershipTranslation(c *gc.C) {
6363
}}
6464

6565
c.Assert(typedP.Params, gc.HasLen, 1)
66-
c.Check(typedP.Params[0].ServiceTag.Id(), gc.Equals, StubServiceNm)
67-
c.Check(typedP.Params[0].UnitTag.Id(), gc.Equals, StubUnitNm)
66+
c.Check(typedP.Params[0].ServiceTag, gc.Equals, names.NewServiceTag(StubServiceNm).String())
67+
c.Check(typedP.Params[0].UnitTag, gc.Equals, names.NewUnitTag(StubUnitNm).String())
6868

6969
return nil
7070
},
@@ -130,8 +130,8 @@ func (s *clientSuite) TestReleaseLeadershipTranslation(c *gc.C) {
130130
typedR.Results = []params.ErrorResult{{}}
131131

132132
c.Assert(typedP.Params, gc.HasLen, 1)
133-
c.Check(typedP.Params[0].ServiceTag.Id(), gc.Equals, StubServiceNm)
134-
c.Check(typedP.Params[0].UnitTag.Id(), gc.Equals, StubUnitNm)
133+
c.Check(typedP.Params[0].ServiceTag, gc.Equals, names.NewServiceTag(StubServiceNm).String())
134+
c.Check(typedP.Params[0].UnitTag, gc.Equals, names.NewUnitTag(StubUnitNm).String())
135135

136136
return nil
137137
},

apiserver/leadership/leadership.go

+7-2
Original file line numberDiff line numberDiff line change
@@ -173,17 +173,22 @@ func callWithIds(fn func(string, string) error) func(st, ut names.Tag) params.Er
173173
}
174174
}
175175

176+
// parseServiceAndUnitTags takes in string representations of service
177+
// and unit tags and parses them into structs.
178+
//
179+
// NOTE: we return permissions errors when parsing fails to obfuscate
180+
// the parse-failure. This is for security purposes.
176181
func parseServiceAndUnitTags(
177182
serviceTagString, unitTagString string,
178183
) (serviceTag names.ServiceTag, unitTag names.UnitTag, _ *params.Error) {
179184
serviceTag, err := names.ParseServiceTag(serviceTagString)
180185
if err != nil {
181-
return serviceTag, unitTag, common.ServerError(err)
186+
return serviceTag, unitTag, common.ServerError(common.ErrPerm)
182187
}
183188

184189
unitTag, err = names.ParseUnitTag(unitTagString)
185190
if err != nil {
186-
return serviceTag, unitTag, common.ServerError(err)
191+
return serviceTag, unitTag, common.ServerError(common.ErrPerm)
187192
}
188193

189194
return serviceTag, unitTag, nil

apiserver/leadership/leadership_test.go

+8-8
Original file line numberDiff line numberDiff line change
@@ -96,8 +96,8 @@ func (s *leadershipSuite) TestClaimLeadershipTranslation(c *gc.C) {
9696
results, err := ldrSvc.ClaimLeadership(params.ClaimLeadershipBulkParams{
9797
Params: []params.ClaimLeadershipParams{
9898
params.ClaimLeadershipParams{
99-
ServiceTag: names.NewServiceTag(StubServiceNm),
100-
UnitTag: names.NewUnitTag(StubUnitNm),
99+
ServiceTag: names.NewServiceTag(StubServiceNm).String(),
100+
UnitTag: names.NewUnitTag(StubUnitNm).String(),
101101
},
102102
},
103103
})
@@ -119,8 +119,8 @@ func (s *leadershipSuite) TestReleaseLeadershipTranslation(c *gc.C) {
119119
results, err := ldrSvc.ClaimLeadership(params.ClaimLeadershipBulkParams{
120120
Params: []params.ClaimLeadershipParams{
121121
params.ClaimLeadershipParams{
122-
ServiceTag: names.NewServiceTag(StubServiceNm),
123-
UnitTag: names.NewUnitTag(StubUnitNm),
122+
ServiceTag: names.NewServiceTag(StubServiceNm).String(),
123+
UnitTag: names.NewUnitTag(StubUnitNm).String(),
124124
},
125125
},
126126
})
@@ -153,8 +153,8 @@ func (s *leadershipSuite) TestClaimLeadershipFailOnAuthorizerErrors(c *gc.C) {
153153
results, err := ldrSvc.ClaimLeadership(params.ClaimLeadershipBulkParams{
154154
Params: []params.ClaimLeadershipParams{
155155
params.ClaimLeadershipParams{
156-
ServiceTag: names.NewServiceTag(StubServiceNm),
157-
UnitTag: names.NewUnitTag(StubUnitNm),
156+
ServiceTag: names.NewServiceTag(StubServiceNm).String(),
157+
UnitTag: names.NewUnitTag(StubUnitNm).String(),
158158
},
159159
},
160160
})
@@ -174,8 +174,8 @@ func (s *leadershipSuite) TestReleaseLeadershipFailOnAuthorizerErrors(c *gc.C) {
174174
results, err := ldrSvc.ClaimLeadership(params.ClaimLeadershipBulkParams{
175175
Params: []params.ClaimLeadershipParams{
176176
params.ClaimLeadershipParams{
177-
ServiceTag: names.NewServiceTag(StubServiceNm),
178-
UnitTag: names.NewUnitTag(StubUnitNm),
177+
ServiceTag: names.NewServiceTag(StubServiceNm).String(),
178+
UnitTag: names.NewUnitTag(StubUnitNm).String(),
179179
},
180180
},
181181
})

featuretests/doc.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ tested.
88
99
Rules:
1010
11-
1) Do NOT mirror the architecture/namespaces of Juju. This shouldbe a
11+
1) Do NOT mirror the architecture/namespaces of Juju. This should be a
1212
very flat folder.
1313
1414
2) Whenever possible, do not mock anything. The goal is to test the

0 commit comments

Comments
 (0)