Code review comment for lp:~axwalk/juju-core/manual-auto-generate-storageauthkey

Reviewers: mp+203693_code.launchpad.net,

Message:
Please take a look.

Description:
manual: generate storage-auth-key in Prepare

https://code.launchpad.net/~axwalk/juju-core/manual-auto-generate-storageauthkey/+merge/203693

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/58210043/

Affected files (+68, -19 lines):
   A [revision details]
   M cmd/jujud/machine_test.go
   M provider/null/config_test.go
   M provider/null/environ_test.go
   A provider/null/export_test.go
   M provider/null/provider.go
   A provider/null/provider_test.go

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: tarmac-20140129060438-5kse5gkrbnxkwu27
+New revision: <email address hidden>

Index: cmd/jujud/machine_test.go
=== modified file 'cmd/jujud/machine_test.go'
--- cmd/jujud/machine_test.go 2014-01-29 04:59:18 +0000
+++ cmd/jujud/machine_test.go 2014-01-29 10:08:01 +0000
@@ -31,8 +31,8 @@

charmtesting "launchpad.net/juju-core/state/apiserver/charmrevisionupdater/testing"
   statetesting "launchpad.net/juju-core/state/testing"
   "launchpad.net/juju-core/state/watcher"
+ "launchpad.net/juju-core/testing"
   coretesting "launchpad.net/juju-core/testing"
- "launchpad.net/juju-core/testing"
   jc "launchpad.net/juju-core/testing/checkers"
   "launchpad.net/juju-core/testing/testbase"
   "launchpad.net/juju-core/tools"

Index: provider/null/config_test.go
=== modified file 'provider/null/config_test.go'
--- provider/null/config_test.go 2013-12-23 06:06:40 +0000
+++ provider/null/config_test.go 2014-01-29 10:08:01 +0000
@@ -21,7 +21,7 @@

  var _ = gc.Suite(&configSuite{})

-func minimalConfigValues() map[string]interface{} {
+func MinimalConfigValues() map[string]interface{} {
   return map[string]interface{}{
    "name": "test",
    "type": provider.Null,
@@ -34,8 +34,8 @@
   }
  }

-func minimalConfig(c *gc.C) *config.Config {
- minimal := minimalConfigValues()
+func MinimalConfig(c *gc.C) *config.Config {
+ minimal := MinimalConfigValues()
   testConfig, err := config.New(config.UseDefaults, minimal)
   c.Assert(err, gc.IsNil)
   return testConfig
@@ -50,7 +50,7 @@
  }

  func (s *configSuite) TestValidateConfig(c *gc.C) {
- testConfig := minimalConfig(c)
+ testConfig := MinimalConfig(c)
   testConfig, err :=
testConfig.Apply(map[string]interface{}{"bootstrap-host": ""})
   c.Assert(err, gc.IsNil)
   _, err = nullProvider{}.Validate(testConfig, nil)
@@ -61,7 +61,7 @@
   _, err = nullProvider{}.Validate(testConfig, nil)
   c.Assert(err, gc.ErrorMatches, "storage-auth-key: expected string, got
nothing")

- testConfig = minimalConfig(c)
+ testConfig = MinimalConfig(c)
   valid, err := nullProvider{}.Validate(testConfig, nil)
   c.Assert(err, gc.IsNil)

@@ -73,7 +73,7 @@
  }

  func (s *configSuite) TestConfigMutability(c *gc.C) {
- testConfig := minimalConfig(c)
+ testConfig := MinimalConfig(c)
   valid, err := nullProvider{}.Validate(testConfig, nil)
   c.Assert(err, gc.IsNil)
   unknownAttrs := valid.UnknownAttrs()
@@ -88,7 +88,7 @@
    "storage-listen-ip": "10.0.0.123",
    "storage-port": int64(1234),
   } {
- testConfig = minimalConfig(c)
+ testConfig = MinimalConfig(c)
    testConfig, err = testConfig.Apply(map[string]interface{}{k: v})
    c.Assert(err, gc.IsNil)
    _, err := nullProvider{}.Validate(testConfig, oldConfig)
@@ -99,7 +99,7 @@
  }

  func (s *configSuite) TestBootstrapHostUser(c *gc.C) {
- values := minimalConfigValues()
+ values := MinimalConfigValues()
   testConfig := getEnvironConfig(c, values)
   c.Assert(testConfig.bootstrapHost(), gc.Equals, "hostname")
   c.Assert(testConfig.bootstrapUser(), gc.Equals, "")
@@ -111,7 +111,7 @@
  }

  func (s *configSuite) TestStorageParams(c *gc.C) {
- values := minimalConfigValues()
+ values := MinimalConfigValues()
   testConfig := getEnvironConfig(c, values)
   c.Assert(testConfig.storageAddr(), gc.Equals, "hostname:8040")
   c.Assert(testConfig.storageListenAddr(), gc.Equals, ":8040")

Index: provider/null/environ_test.go
=== modified file 'provider/null/environ_test.go'
--- provider/null/environ_test.go 2014-01-08 05:49:24 +0000
+++ provider/null/environ_test.go 2014-01-29 10:08:01 +0000
@@ -29,15 +29,15 @@
  var _ = gc.Suite(&environSuite{})

  func (s *environSuite) SetUpTest(c *gc.C) {
- envConfig := getEnvironConfig(c, minimalConfigValues())
+ envConfig := getEnvironConfig(c, MinimalConfigValues())
   s.env = &nullEnviron{cfg: envConfig}
  }

  func (s *environSuite) TestSetConfig(c *gc.C) {
- err := s.env.SetConfig(minimalConfig(c))
+ err := s.env.SetConfig(MinimalConfig(c))
   c.Assert(err, gc.IsNil)

- testConfig := minimalConfig(c)
+ testConfig := MinimalConfig(c)
   testConfig, err =
testConfig.Apply(map[string]interface{}{"bootstrap-host": ""})
   c.Assert(err, gc.IsNil)
   err = s.env.SetConfig(testConfig)

Index: provider/null/export_test.go
=== added file 'provider/null/export_test.go'
--- provider/null/export_test.go 1970-01-01 00:00:00 +0000
+++ provider/null/export_test.go 2014-01-29 10:08:01 +0000
@@ -0,0 +1,8 @@
+// Copyright 2014 Canonical Ltd.
+// Licensed under the AGPLv3, see LICENCE file for details.
+
+package null
+
+var (
+ ProviderInstance = nullProvider{}
+)

Index: provider/null/provider.go
=== modified file 'provider/null/provider.go'
--- provider/null/provider.go 2013-10-15 05:11:29 +0000
+++ provider/null/provider.go 2014-01-29 10:08:01 +0000
@@ -22,7 +22,18 @@
  var errNoBootstrapHost = errors.New("bootstrap-host must be specified")

  func (p nullProvider) Prepare(cfg *config.Config) (environs.Environ,
error) {
- // TODO(rog) 2013-10-07 generate storage-auth-key if not set.
+ if _, ok := cfg.UnknownAttrs()["storage-auth-key"].(string); !ok {
+ uuid, err := utils.NewUUID()
+ if err != nil {
+ return nil, err
+ }
+ cfg, err = cfg.Apply(map[string]interface{}{
+ "storage-auth-key": uuid.String(),
+ })
+ if err != nil {
+ return nil, err
+ }
+ }
   return p.Open(cfg)
  }

@@ -112,11 +123,7 @@
      # bootstrap machine's Juju storage server will listen
      # on. It defaults to ` + fmt.Sprint(defaultStoragePort) + `
      # storage-port: ` + fmt.Sprint(defaultStoragePort) + `
-
- # storage-auth-key holds the key used to authenticate
- # to the storage servers. It will become unnecessary to
- # give this option.
- storage-auth-key: {{rand}}
+

  `[1:]
  }

Index: provider/null/provider_test.go
=== added file 'provider/null/provider_test.go'
--- provider/null/provider_test.go 1970-01-01 00:00:00 +0000
+++ provider/null/provider_test.go 2014-01-29 10:08:01 +0000
@@ -0,0 +1,32 @@
+// Copyright 2014 Canonical Ltd.
+// Licensed under the AGPLv3, see LICENCE file for details.
+
+package null_test
+
+import (
+ gc "launchpad.net/gocheck"
+
+ "launchpad.net/juju-core/environs/config"
+ "launchpad.net/juju-core/provider/null"
+ jc "launchpad.net/juju-core/testing/checkers"
+ "launchpad.net/juju-core/testing/testbase"
+ "launchpad.net/juju-core/utils"
+)
+
+type providerSuite struct {
+ testbase.LoggingSuite
+}
+
+var _ = gc.Suite(&providerSuite{})
+
+func (s *providerSuite) TestPrepare(c *gc.C) {
+ minimal := null.MinimalConfigValues()
+ delete(minimal, "storage-auth-key")
+ testConfig, err := config.New(config.UseDefaults, minimal)
+ c.Assert(err, gc.IsNil)
+ env, err := null.ProviderInstance.Prepare(testConfig)
+ c.Assert(err, gc.IsNil)
+ cfg := env.Config()
+ key, _ := cfg.UnknownAttrs()["storage-auth-key"].(string)
+ c.Assert(key, jc.Satisfies, utils.IsValidUUIDString)
+}