Skip to content

Commit dfa6eff

Browse files
wallyworldwallyworld
committed
Fix region handling in model-defaults --reset
1 parent 4410306 commit dfa6eff

File tree

8 files changed

+120
-38
lines changed

8 files changed

+120
-38
lines changed

‎cmd/juju/cloud/updatecredential_test.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,13 @@ type updateCredentialSuite struct {
2424
var _ = gc.Suite(&updateCredentialSuite{})
2525

2626
func (s *updateCredentialSuite) TestBadArgs(c *gc.C) {
27-
cmd := cloud.NewUpdateCredentialCommand()
27+
store := &jujuclienttesting.MemStore{
28+
Controllers: map[string]jujuclient.ControllerDetails{
29+
"controller": {},
30+
},
31+
CurrentControllerName: "controller",
32+
}
33+
cmd := cloud.NewUpdateCredentialCommandForTest(store, nil)
2834
_, err := testing.RunCommand(c, cmd)
2935
c.Assert(err, gc.ErrorMatches, "Usage: juju update-credential <cloud-name> <credential-name>")
3036
_, err = testing.RunCommand(c, cmd, "cloud", "credential", "extra")

‎cmd/juju/model/defaultscommand.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,12 @@ Examples:
4040
juju model-defaults http-proxy
4141
juju model-defaults -m mymodel type
4242
juju model-defaults ftp-proxy=10.0.0.1:8000
43+
juju model-defaults aws/us-east-1 ftp-proxy=10.0.0.1:8000
44+
juju model-defaults us-east-1 ftp-proxy=10.0.0.1:8000
4345
juju model-defaults -m othercontroller:mymodel default-series=yakkety test-mode=false
4446
juju model-defaults --reset default-series test-mode
47+
juju model-defaults aws/us-east-1 --reset http-proxy
48+
juju model-defaults us-east-1 --reset http-proxy
4549
4650
See also:
4751
models
@@ -596,7 +600,7 @@ func formatDefaultConfigTabular(writer io.Writer, value interface{}) error {
596600
}
597601
sort.Strings(valueNames)
598602

599-
w.Println("ATTRIBUTE", "DEFAULT", "CONTROLLER")
603+
w.Println("Attribute", "Default", "Controller")
600604

601605
for _, name := range valueNames {
602606
info := defaultValues[name]

‎cmd/juju/model/defaultscommand_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -307,7 +307,7 @@ func (s *DefaultsCommandSuite) TestGetSingleValue(c *gc.C) {
307307

308308
output := strings.TrimSpace(testing.Stdout(context))
309309
expected := "" +
310-
"ATTRIBUTE DEFAULT CONTROLLER\n" +
310+
"Attribute Default Controller\n" +
311311
"attr2 - bar\n" +
312312
" dummy-region dummy-value -"
313313
c.Assert(output, gc.Equals, expected)
@@ -353,7 +353,7 @@ func (s *DefaultsCommandSuite) TestGetAllValuesTabular(c *gc.C) {
353353

354354
output := strings.TrimSpace(testing.Stdout(context))
355355
expected := "" +
356-
"ATTRIBUTE DEFAULT CONTROLLER\n" +
356+
"Attribute Default Controller\n" +
357357
"attr foo -\n" +
358358
"attr2 - bar\n" +
359359
" dummy-region dummy-value -"

‎cmd/juju/model/grantrevoke.go

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -81,9 +81,9 @@ See also:
8181
type accessCommand struct {
8282
modelcmd.ControllerCommandBase
8383

84-
User string
85-
ModelNames []string
86-
ModelAccess string
84+
User string
85+
ModelNames []string
86+
Access string
8787
}
8888

8989
// Init implements cmd.Command.
@@ -98,16 +98,23 @@ func (c *accessCommand) Init(args []string) error {
9898

9999
c.User = args[0]
100100
c.ModelNames = args[2:]
101-
c.ModelAccess = args[1]
101+
c.Access = args[1]
102102
// Special case for backwards compatibility.
103-
if c.ModelAccess == "addmodel" {
104-
c.ModelAccess = "add-model"
103+
if c.Access == "addmodel" {
104+
c.Access = "add-model"
105105
}
106106
if len(c.ModelNames) > 0 {
107-
err := permission.ValidateModelAccess(permission.Access(c.ModelAccess))
108-
if err != nil {
109-
return err
107+
if err := permission.ValidateControllerAccess(permission.Access(c.Access)); err == nil {
108+
return errors.Errorf("You have specified a controller access permission %q.\n"+
109+
"If you intended to change controller access, do not specify any model names.\n"+
110+
"See 'juju help grant'.", c.Access)
110111
}
112+
return permission.ValidateModelAccess(permission.Access(c.Access))
113+
}
114+
if err := permission.ValidateModelAccess(permission.Access(c.Access)); err == nil {
115+
return errors.Errorf("You have specified a model access permission %q.\n"+
116+
"If you intended to change model access, you need to specify one or more model names.\n"+
117+
"See 'juju help grant'.", c.Access)
111118
}
112119
return nil
113120
}
@@ -171,7 +178,7 @@ func (c *grantCommand) runForController() error {
171178
}
172179
defer client.Close()
173180

174-
return block.ProcessBlockedError(client.GrantController(c.User, c.ModelAccess), block.BlockChange)
181+
return block.ProcessBlockedError(client.GrantController(c.User, c.Access), block.BlockChange)
175182
}
176183

177184
func (c *grantCommand) runForModel() error {
@@ -185,7 +192,7 @@ func (c *grantCommand) runForModel() error {
185192
if err != nil {
186193
return err
187194
}
188-
return block.ProcessBlockedError(client.GrantModel(c.User, c.ModelAccess, models...), block.BlockChange)
195+
return block.ProcessBlockedError(client.GrantModel(c.User, c.Access, models...), block.BlockChange)
189196
}
190197

191198
// NewRevokeCommand returns a new revoke command.
@@ -247,7 +254,7 @@ func (c *revokeCommand) runForController() error {
247254
}
248255
defer client.Close()
249256

250-
return block.ProcessBlockedError(client.RevokeController(c.User, c.ModelAccess), block.BlockChange)
257+
return block.ProcessBlockedError(client.RevokeController(c.User, c.Access), block.BlockChange)
251258
}
252259

253260
func (c *revokeCommand) runForModel() error {
@@ -261,5 +268,5 @@ func (c *revokeCommand) runForModel() error {
261268
if err != nil {
262269
return err
263270
}
264-
return block.ProcessBlockedError(client.RevokeModel(c.User, c.ModelAccess, models...), block.BlockChange)
271+
return block.ProcessBlockedError(client.RevokeModel(c.User, c.Access, models...), block.BlockChange)
265272
}

‎cmd/juju/model/grantrevoke_test.go

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
package model_test
55

66
import (
7+
"strings"
8+
79
"github.com/juju/cmd"
810
jc "github.com/juju/testing/checkers"
911
gc "gopkg.in/check.v1"
@@ -127,7 +129,7 @@ func (s *grantSuite) TestInitGrantAddModel(c *gc.C) {
127129
// The backwards-compatible case, addmodel.
128130
err = testing.InitCommand(wrappedCmd, []string{"bob", "addmodel"})
129131
c.Check(err, jc.ErrorIsNil)
130-
c.Assert(grantCmd.ModelAccess, gc.Equals, "add-model")
132+
c.Assert(grantCmd.Access, gc.Equals, "add-model")
131133
}
132134

133135
type revokeSuite struct {
@@ -171,7 +173,21 @@ func (s *grantSuite) TestInitRevokeAddModel(c *gc.C) {
171173
// The backwards-compatible case, addmodel.
172174
err = testing.InitCommand(wrappedCmd, []string{"bob", "addmodel"})
173175
c.Check(err, jc.ErrorIsNil)
174-
c.Assert(revokeCmd.ModelAccess, gc.Equals, "add-model")
176+
c.Assert(revokeCmd.Access, gc.Equals, "add-model")
177+
}
178+
179+
func (s *grantSuite) TestModelAccessForController(c *gc.C) {
180+
wrappedCmd, _ := model.NewRevokeCommandForTest(s.fake, s.store)
181+
err := testing.InitCommand(wrappedCmd, []string{"bob", "write"})
182+
msg := strings.Replace(err.Error(), "\n", "", -1)
183+
c.Check(msg, gc.Matches, `You have specified a model access permission "write".*`)
184+
}
185+
186+
func (s *grantSuite) TestControllerAccessForModel(c *gc.C) {
187+
wrappedCmd, _ := model.NewRevokeCommandForTest(s.fake, s.store)
188+
err := testing.InitCommand(wrappedCmd, []string{"bob", "superuser", "default"})
189+
msg := strings.Replace(err.Error(), "\n", "", -1)
190+
c.Check(msg, gc.Matches, `You have specified a controller access permission "superuser".*`)
175191
}
176192

177193
type fakeGrantRevokeAPI struct {

‎cmd/modelcmd/controller.go

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -270,10 +270,6 @@ func (w *sysCommandWrapper) Init(args []string) error {
270270
store = QualifyingClientStore{store}
271271
w.SetClientStore(store)
272272

273-
return w.ControllerCommand.Init(args)
274-
}
275-
276-
func (w *sysCommandWrapper) Run(ctx *cmd.Context) error {
277273
if w.setControllerFlags {
278274
if w.controllerName == "" && w.useDefaultController {
279275
store := w.ClientStore()
@@ -292,7 +288,7 @@ func (w *sysCommandWrapper) Run(ctx *cmd.Context) error {
292288
return translateControllerError(w.ClientStore(), err)
293289
}
294290
}
295-
return w.ControllerCommand.Run(ctx)
291+
return w.ControllerCommand.Init(args)
296292
}
297293

298294
func translateControllerError(store jujuclient.ClientStore, err error) error {

‎cmd/modelcmd/controller_test.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,7 @@ type ControllerCommandSuite struct {
2323
var _ = gc.Suite(&ControllerCommandSuite{})
2424

2525
func (s *ControllerCommandSuite) TestControllerCommandNoneSpecified(c *gc.C) {
26-
cmd, _, err := initTestControllerCommand(c, nil)
27-
c.Assert(err, jc.ErrorIsNil)
28-
err = cmd.Run(nil)
26+
_, _, err := initTestControllerCommand(c, nil)
2927
c.Assert(errors.Cause(err), gc.Equals, modelcmd.ErrNoControllersDefined)
3028
}
3129

‎featuretests/cmd_juju_model_test.go

Lines changed: 66 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ import (
1616

1717
"github.com/juju/juju/cmd/juju/commands"
1818
"github.com/juju/juju/core/description"
19+
"github.com/juju/juju/environs"
20+
"github.com/juju/juju/environs/config"
1921
"github.com/juju/juju/feature"
2022
jujutesting "github.com/juju/juju/juju/testing"
2123
"github.com/juju/juju/permission"
@@ -106,25 +108,78 @@ func (s *cmdModelSuite) TestModelUsersCmd(c *gc.C) {
106108

107109
}
108110

109-
func (s *cmdModelSuite) TestGet(c *gc.C) {
111+
func (s *cmdModelSuite) TestModelConfigGet(c *gc.C) {
110112
err := s.State.UpdateModelConfig(map[string]interface{}{"special": "known"}, nil, nil)
111113
c.Assert(err, jc.ErrorIsNil)
112114

113115
context := s.run(c, "model-config", "special")
114116
c.Assert(testing.Stdout(context), gc.Equals, "known\n")
115117
}
116118

117-
func (s *cmdModelSuite) TestSet(c *gc.C) {
119+
func (s *cmdModelSuite) TestModelConfigSet(c *gc.C) {
118120
s.run(c, "model-config", "special=known")
119-
s.assertEnvValue(c, "special", "known")
121+
s.assertModelValue(c, "special", "known")
120122
}
121123

122-
func (s *cmdModelSuite) TestUnset(c *gc.C) {
124+
func (s *cmdModelSuite) TestModelConfigReset(c *gc.C) {
123125
err := s.State.UpdateModelConfig(map[string]interface{}{"special": "known"}, nil, nil)
124126
c.Assert(err, jc.ErrorIsNil)
125127

126128
s.run(c, "model-config", "--reset", "special")
127-
s.assertEnvValueMissing(c, "special")
129+
s.assertModelValueMissing(c, "special")
130+
}
131+
132+
func (s *cmdModelSuite) TestModelDefaultsGet(c *gc.C) {
133+
err := s.State.UpdateModelConfigDefaultValues(map[string]interface{}{"special": "known"}, nil, nil)
134+
c.Assert(err, jc.ErrorIsNil)
135+
136+
context := s.run(c, "model-defaults", "special")
137+
c.Assert(testing.Stdout(context), gc.Equals, `
138+
Attribute Default Controller
139+
special - known
140+
141+
`[1:])
142+
}
143+
144+
func (s *cmdModelSuite) TestModelDefaultsSet(c *gc.C) {
145+
s.run(c, "model-defaults", "special=known")
146+
defaults, err := s.State.ModelConfigDefaultValues()
147+
c.Assert(err, jc.ErrorIsNil)
148+
value, found := defaults["special"]
149+
c.Assert(found, jc.IsTrue)
150+
c.Assert(value.Controller, gc.Equals, "known")
151+
}
152+
153+
func (s *cmdModelSuite) TestModelDefaultsSetRegion(c *gc.C) {
154+
s.run(c, "model-defaults", "dummy/dummy-region", "special=known")
155+
defaults, err := s.State.ModelConfigDefaultValues()
156+
c.Assert(err, jc.ErrorIsNil)
157+
value, found := defaults["special"]
158+
c.Assert(found, jc.IsTrue)
159+
c.Assert(value.Controller, gc.IsNil)
160+
c.Assert(value.Regions, jc.SameContents, []config.RegionDefaultValue{{"dummy-region", "known"}})
161+
}
162+
163+
func (s *cmdModelSuite) TestModelDefaultsReset(c *gc.C) {
164+
err := s.State.UpdateModelConfigDefaultValues(map[string]interface{}{"special": "known"}, nil, nil)
165+
c.Assert(err, jc.ErrorIsNil)
166+
167+
s.run(c, "model-defaults", "--reset", "special")
168+
defaults, err := s.State.ModelConfigDefaultValues()
169+
c.Assert(err, jc.ErrorIsNil)
170+
_, found := defaults["special"]
171+
c.Assert(found, jc.IsFalse)
172+
}
173+
174+
func (s *cmdModelSuite) TestModelDefaultsResetRegion(c *gc.C) {
175+
err := s.State.UpdateModelConfigDefaultValues(map[string]interface{}{"special": "known"}, nil, &environs.RegionSpec{"dummy", "dummy-region"})
176+
c.Assert(err, jc.ErrorIsNil)
177+
178+
s.run(c, "model-defaults", "dummy-region", "--reset", "special")
179+
defaults, err := s.State.ModelConfigDefaultValues()
180+
c.Assert(err, jc.ErrorIsNil)
181+
_, found := defaults["special"]
182+
c.Assert(found, jc.IsFalse)
128183
}
129184

130185
func (s *cmdModelSuite) TestRetryProvisioning(c *gc.C) {
@@ -170,17 +225,17 @@ func (s *cmdModelSuite) TestDumpModelDB(c *gc.C) {
170225
c.Assert(modelMap["name"], gc.Equals, "controller")
171226
}
172227

173-
func (s *cmdModelSuite) assertEnvValue(c *gc.C, key string, expected interface{}) {
174-
envConfig, err := s.State.ModelConfig()
228+
func (s *cmdModelSuite) assertModelValue(c *gc.C, key string, expected interface{}) {
229+
modelConfig, err := s.State.ModelConfig()
175230
c.Assert(err, jc.ErrorIsNil)
176-
value, found := envConfig.AllAttrs()[key]
231+
value, found := modelConfig.AllAttrs()[key]
177232
c.Assert(found, jc.IsTrue)
178233
c.Assert(value, gc.Equals, expected)
179234
}
180235

181-
func (s *cmdModelSuite) assertEnvValueMissing(c *gc.C, key string) {
182-
envConfig, err := s.State.ModelConfig()
236+
func (s *cmdModelSuite) assertModelValueMissing(c *gc.C, key string) {
237+
modelConfig, err := s.State.ModelConfig()
183238
c.Assert(err, jc.ErrorIsNil)
184-
_, found := envConfig.AllAttrs()[key]
239+
_, found := modelConfig.AllAttrs()[key]
185240
c.Assert(found, jc.IsFalse)
186241
}

0 commit comments

Comments
 (0)