Fix lp:1168744 validate config

Fix lp:1168744 validate config

Add new function to environs, ValidateConfig, which makes a new config by applying config args to oldConfig and validating via environ provider specific validator.

https://code.launchpad.net/~waigani/juju-core/validate-environ-config/+merge/205880

(do not edit description out of merge proposal)

[waigani, 2014-02-12 21:09:58]

Please take a look.

[waigani, 2014-02-12 22:55:30]

Please take a look.

[axw, 2014-02-13 08:59:41]

https://codereview.appspot.com/62400043/diff/10001/environs/config.go
File environs/config.go (right):

https://codereview.appspot.com/62400043/diff/10001/environs/config.go#newcode267
environs/config.go:267: func ValidateConfig(oldConfig *config.Config, args
map[string]interface{}) (newProviderConfig *config.Config, err error) {
This doesn't look quite right to me. I think it should just have the same
signature as EnvironProvider.Validate.

https://codereview.appspot.com/62400043/diff/10001/environs/config.go#newcode273
environs/config.go:273: env, err := New(oldConfig)
This is way overkill for getting a provider. Instead, just get the provider type
from the config and use environs.Provider().

https://codereview.appspot.com/62400043/diff/10001/state/apiserver/client/cli...
File state/apiserver/client/client.go (right):

https://codereview.appspot.com/62400043/diff/10001/state/apiserver/client/cli...
state/apiserver/client/client.go:787: //Make new config by applying config args
to oldConfig and validating via environ provider specific validator
I don't think this comment is worthwhile. It's just restating what
ValidateConfig says it does.

https://codereview.appspot.com/62400043/diff/10001/state/apiserver/keymanager...
File state/apiserver/keymanager/keymanager.go (right):

https://codereview.appspot.com/62400043/diff/10001/state/apiserver/keymanager...
state/apiserver/keymanager/keymanager.go:143: //Make new config by applying
config args to currentConfig and validating via environ provider specific
validator
ditto

[fwereade, 2014-02-14 09:32:41]

On 2014/02/13 08:59:41, axw wrote:
> https://codereview.appspot.com/62400043/diff/10001/environs/config.go
> File environs/config.go (right):
> 
>
https://codereview.appspot.com/62400043/diff/10001/environs/config.go#newcode267
> environs/config.go:267: func ValidateConfig(oldConfig *config.Config, args
> map[string]interface{}) (newProviderConfig *config.Config, err error) {
> This doesn't look quite right to me. I think it should just have the same
> signature as EnvironProvider.Validate.
> 
>
https://codereview.appspot.com/62400043/diff/10001/environs/config.go#newcode273
> environs/config.go:273: env, err := New(oldConfig)
> This is way overkill for getting a provider. Instead, just get the provider
type
> from the config and use environs.Provider().
> 
>
https://codereview.appspot.com/62400043/diff/10001/state/apiserver/client/cli...
> File state/apiserver/client/client.go (right):
> 
>
https://codereview.appspot.com/62400043/diff/10001/state/apiserver/client/cli...
> state/apiserver/client/client.go:787: //Make new config by applying config
args
> to oldConfig and validating via environ provider specific validator
> I don't think this comment is worthwhile. It's just restating what
> ValidateConfig says it does.
> 
>
https://codereview.appspot.com/62400043/diff/10001/state/apiserver/keymanager...
> File state/apiserver/keymanager/keymanager.go (right):
> 
>
https://codereview.appspot.com/62400043/diff/10001/state/apiserver/keymanager...
> state/apiserver/keymanager/keymanager.go:143: //Make new config by applying
> config args to currentConfig and validating via environ provider specific
> validator
> ditto

I don't think this is quite the right approach: it's fundamentally racy, and
that can't be fixed unless we do it inside state (well... we *could* have a
retry loop at the API level I guess? but I think that would suck a bit).

Can we chat about this live? Axw is doing some relevant work that would mesh
nicely, I think; I'll be online my sunday night/your monday morning to chat to
Tim.

[waigani, 2014-02-15 20:04:41]

Please take a look.

https://codereview.appspot.com/62400043/diff/10001/environs/config.go
File environs/config.go (right):

https://codereview.appspot.com/62400043/diff/10001/environs/config.go#newcode267
environs/config.go:267: func ValidateConfig(oldConfig *config.Config, args
map[string]interface{}) (newProviderConfig *config.Config, err error) {
On 2014/02/13 08:59:42, axw wrote:
> This doesn't look quite right to me. I think it should just have the same
> signature as EnvironProvider.Validate.

Changed func name after discussing with Ian.

https://codereview.appspot.com/62400043/diff/10001/environs/config.go#newcode273
environs/config.go:273: env, err := New(oldConfig)
On 2014/02/13 08:59:42, axw wrote:
> This is way overkill for getting a provider. Instead, just get the provider
type
> from the config and use environs.Provider().

Done.

[waigani, 2014-02-15 20:34:27]

https://codereview.appspot.com/62400043/diff/10001/state/apiserver/client/cli...
File state/apiserver/client/client.go (right):

https://codereview.appspot.com/62400043/diff/10001/state/apiserver/client/cli...
state/apiserver/client/client.go:787: //Make new config by applying config args
to oldConfig and validating via environ provider specific validator
On 2014/02/13 08:59:42, axw wrote:
> I don't think this comment is worthwhile. It's just restating what
> ValidateConfig says it does.

Done.

https://codereview.appspot.com/62400043/diff/10001/state/apiserver/keymanager...
File state/apiserver/keymanager/keymanager.go (right):

https://codereview.appspot.com/62400043/diff/10001/state/apiserver/keymanager...
state/apiserver/keymanager/keymanager.go:143: //Make new config by applying
config args to currentConfig and validating via environ provider specific
validator
On 2014/02/13 08:59:42, axw wrote:
> ditto

Done.