environs: split out config validation

environs: split out config validation

This enables the validation of environment configurations
independently from the creation of environment instances.

The way in which this was done facilitates:

- the implementation of set-env 
- storage of validated and provider-enriched configs in state
- provider-specific config change rejection

https://code.launchpad.net/~niemeyer/juju-core/config-3000-validation/+merge/115879

Requires: https://code.launchpad.net/~niemeyer/juju-core/schema-field-map-defaults/+merge/115863

(do not edit description out of merge proposal)

[niemeyer, 2012-07-20 13:56:07]

Please take a look.

[fwereade, 2012-07-20 14:25:08]

This is *great*, thank you very much. LGTM aside from control-bucket.

https://codereview.appspot.com/6416055/diff/4001/environs/ec2/config.go
File environs/ec2/config.go (right):

https://codereview.appspot.com/6416055/diff/4001/environs/ec2/config.go#newco...
environs/ec2/config.go:74: return nil, fmt.Errorf("invalid region name %q",
ecfg.region())
If it's worth checking for a missing endpoint (as we do in Open) I maintain it's
worth checking for here rather than there :).

https://codereview.appspot.com/6416055/diff/4001/environs/ec2/config.go#newco...
environs/ec2/config.go:80: return nil, fmt.Errorf("can't change environment
region from %q to %q", region, ecfg.region())
I don't think we can safely change control-bucket either.

https://codereview.appspot.com/6416055/diff/4001/environs/ec2/ec2.go
File environs/ec2/ec2.go (left):

https://codereview.appspot.com/6416055/diff/4001/environs/ec2/ec2.go#oldcode117
environs/ec2/ec2.go:117: return fmt.Errorf("environment %q references unknown
EC2 region %q", config.name, config.region)
...but I guess it's not worth checking for. I always did wonder about that :).

[niemeyer, 2012-07-20 15:59:52]

Please take a look.

[fwereade, 2012-07-20 16:13:59]

LGTM with reinstatement of the changed test (and keep the new one, ofc).

https://codereview.appspot.com/6416055/diff/6002/environs/ec2/config_test.go
File environs/ec2/config_test.go (right):

https://codereview.appspot.com/6416055/diff/6002/environs/ec2/config_test.go#...
environs/ec2/config_test.go:159: err: `can't change control-bucket from "x" to
"new-x"`,
Why did we lose the 666 test for public-bucket?

[niemeyer, 2012-07-20 18:14:45]

*** Submitted:

environs: split out config validation

This enables the validation of environment configurations
independently from the creation of environment instances.

The way in which this was done facilitates:

- the implementation of set-env 
- storage of validated and provider-enriched configs in state
- provider-specific config change rejection

R=fwereade
CC=
https://codereview.appspot.com/6416055

https://codereview.appspot.com/6416055/diff/4001/environs/ec2/config.go
File environs/ec2/config.go (right):

https://codereview.appspot.com/6416055/diff/4001/environs/ec2/config.go#newco...
environs/ec2/config.go:74: return nil, fmt.Errorf("invalid region name %q",
ecfg.region())
On 2012/07/20 14:25:08, fwereade wrote:
> If it's worth checking for a missing endpoint (as we do in Open) I maintain
it's
> worth checking for here rather than there :).

As we talked I don't think it makes sense to check for invalid aws.Regions. We
didn't do this for S3 either, for example.

https://codereview.appspot.com/6416055/diff/4001/environs/ec2/config.go#newco...
environs/ec2/config.go:80: return nil, fmt.Errorf("can't change environment
region from %q to %q", region, ecfg.region())
On 2012/07/20 14:25:08, fwereade wrote:
> I don't think we can safely change control-bucket either.

Good point, that's fixed.

https://codereview.appspot.com/6416055/diff/4001/environs/ec2/ec2.go
File environs/ec2/ec2.go (left):

https://codereview.appspot.com/6416055/diff/4001/environs/ec2/ec2.go#oldcode117
environs/ec2/ec2.go:117: return fmt.Errorf("environment %q references unknown
EC2 region %q", config.name, config.region)
On 2012/07/20 14:25:08, fwereade wrote:
> ...but I guess it's not worth checking for. I always did wonder about that :).

Agreed.

https://codereview.appspot.com/6416055/diff/6002/environs/ec2/config_test.go
File environs/ec2/config_test.go (right):

https://codereview.appspot.com/6416055/diff/6002/environs/ec2/config_test.go#...
environs/ec2/config_test.go:159: err: `can't change control-bucket from "x" to
"new-x"`,
On 2012/07/20 16:13:59, fwereade wrote:
> Why did we lose the 666 test for public-bucket?

Oops. Copy & paste error. That's what I get for not self-reviewing.