Jeroen T. Vermeulen has proposed merging lp:~jtv/juju-core/mpv-fwreade-15 into
lp:~maas-maintainers/juju-core/maas-provider-skeleton.
Commit message:
Address fwereade's review comment #15: validate config transition in
SetConfig().
Requested reviews:
MAAS Maintainers (maas-maintainers)
For more details, see:
https://code.launchpad.net/~jtv/juju-core/mpv-fwreade-15/+merge/158521
I don't see the other providers doing this yet, but since we were already
halfway done implementing thumper's global Validate(), it makes sense to
validate not just the new config that's being set but the resulting changes as
well.
This small branch took surprising amounts of debugging.
Jeroen
--
https://code.launchpad.net/~jtv/juju-core/mpv-fwreade-15/+merge/158521
Your team MAAS Maintainers is requested to review the proposed merge of
lp:~jtv/juju-core/mpv-fwreade-15 into
lp:~maas-maintainers/juju-core/maas-provider-skeleton.
=== modified file 'environs/maas/environ.go'
--- environs/maas/environ.go 2013-04-11 11:59:26 +0000
+++ environs/maas/environ.go 2013-04-12 03:41:23 +0000
@@ -238,14 +238,25 @@
// SetConfig is specified in the Environ interface.
func (env *maasEnviron) SetConfig(cfg *config.Config) error {
+ env.ecfgMutex.Lock()
+ defer env.ecfgMutex.Unlock()
+
+ // The new config has already been validated by itself, but now we
+ // validate the transition from the old config to the new.
+ var oldCfg *config.Config
+ if env.ecfgUnlocked != nil {
+ oldCfg = env.ecfgUnlocked.Config
+ }
+ cfg, err := env.Provider().Validate(cfg, oldCfg)
+ if err != nil {
+ return err
+ }
+
ecfg, err := env.Provider().(*maasEnvironProvider).newConfig(cfg)
if err != nil {
return err
}
- env.ecfgMutex.Lock()
- defer env.ecfgMutex.Unlock()
-
env.name = cfg.Name()
env.ecfgUnlocked = ecfg
=== modified file 'environs/maas/environ_test.go'
--- environs/maas/environ_test.go 2013-04-11 12:05:50 +0000
+++ environs/maas/environ_test.go 2013-04-12 03:41:23 +0000
@@ -71,20 +71,41 @@
envtesting.PutFakeTools(c, storage)
}
+func (EnvironSuite) TestSetConfigValidatesFirst(c *C) {
+ // SetConfig() validates the config change and disallows, for example,
+ // changes in the environment name.
+ server := "http://maas.example.com";
+ oauth := "a:b:c"
+ secret := "pssst"
+ oldCfg := getTestConfig("old-name", server, oauth, secret)
+ newCfg := getTestConfig("new-name", server, oauth, secret)
+ env, err := NewEnviron(oldCfg)
+ c.Assert(err, IsNil)
+
+ // SetConfig() fails, even though both the old and the new config are
+ // individually.
+ err = env.SetConfig(newCfg)
+ c.Assert(err, NotNil)
+ c.Check(err, ErrorMatches, ".*cannot change name.*")
+
+ // The old config is still in place. The new config never took effect.
+ c.Check(env.Name(), Equals, "old-name")
+}
+
func (EnvironSuite) TestSetConfigUpdatesConfig(c *C) {
- cfg := getTestConfig("test env", "http://maas2.example.com";, "a:b:c", "secret")
+ name := "test env"
+ cfg := getTestConfig(name, "http://maas2.example.com";, "a:b:c", "secret")
env, err := NewEnviron(cfg)
c.Check(err, IsNil)
c.Check(env.name, Equals, "test env")
- anotherName := "another name"
anotherServer := "http://maas.example.com";
anotherOauth := "c:d:e"
anotherSecret := "secret2"
- cfg2 := getTestConfig(anotherName, anotherServer, anotherOauth, anotherSecret)
+ cfg2 := getTestConfig(name, anotherServer, anotherOauth, anotherSecret)
errSetConfig := env.SetConfig(cfg2)
c.Check(errSetConfig, IsNil)
- c.Check(env.name, Equals, anotherName)
+ c.Check(env.name, Equals, name)
authClient, _ := gomaasapi.NewAuthenticatedClient(anotherServer, anotherOauth, apiVersion)
maas := gomaasapi.NewMAAS(*authClient)
MAASServer := env.maasClientUnlocked
_______________________________________________
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : [email protected]
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
