Merge lp:~dimitern/juju-core/openstack-stub-provider-config into lp:~juju/juju-core/trunk

[Dimiter Naydenov]

Implement environs/openstack/config, tests and minimal stubbed OpenStack provider.

Implemented the base of a juju openstack provider.

Right now the configuration is parsed and validated, and the minimum amount of code needed in the provider is implemented, the rest is stubbed out for now.

https://codereview.appspot.com/6858052/

[Dimiter Naydenov (dimitern)]

Reviewers: mp+135454_code.launchpad.net,

Message:
Please take a look.

Description:
Implemented the base of a juju openstack provider.

Right now the configuration is parsed and validated, and the minimum
amount of code needed in the provider is implemented, the rest is
stubbed out for now.

https://code.launchpad.net/~dimitern/juju-core/openstack-stub-provider-config/+merge/135454

(do not edit description out of merge proposal)

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

Affected files:
   A [revision details]
   A environs/openstack/config.go
   A environs/openstack/config_test.go
   A environs/openstack/provider.go

[Roger Peppe (rogpeppe)]

looks great, with only minor comments below.

https://codereview.appspot.com/6858052/diff/1/environs/openstack/config.go
File environs/openstack/config.go (right):

https://codereview.appspot.com/6858052/diff/1/environs/openstack/config.go#newcode79
environs/openstack/config.go:79: ecfg.username() != "" ||
perhaps these checks should be in dummyEnvAuth, and you could just
return err here. then the error message can be more useful.

https://codereview.appspot.com/6858052/diff/1/environs/openstack/config_test.go
File environs/openstack/config_test.go (right):

https://codereview.appspot.com/6858052/diff/1/environs/openstack/config_test.go#newcode18
environs/openstack/config_test.go:18: // Hook up gocheck into the gotest
runner.
this is ubiquitous enough that it probably doesn't need a comment.

https://codereview.appspot.com/6858052/diff/1/environs/openstack/config_test.go#newcode111
environs/openstack/config_test.go:111: s.savedUsername =
os.Getenv("OS_USERNAME")
there are perhaps enough of these that you could have a variable above,
say envVars:

var envVars = map[string]string{
     "OS_USERNAME" : "testuser",
     etc
}

then have ConfigSuite.savedVars defined as map[string]string

and below:

for v, val := range envVars {
    s.savedVargs[v] = os.Getenv(v)
    os.Setenv(v, val)
}

and restore in TearDownTest.

YMMV, just a thought.

https://codereview.appspot.com/6858052/diff/1/environs/openstack/provider.go
File environs/openstack/provider.go (right):

https://codereview.appspot.com/6858052/diff/1/environs/openstack/provider.go#newcode21
environs/openstack/provider.go:21: //
-------------------------------------------------------------------
d

we don't tend to bother with markers like this.
i'm not sure the comment below helps much either. it should be fairly
obvious that this is implementing environs.Environ.

https://codereview.appspot.com/6858052/diff/1/environs/openstack/provider.go#newcode66
environs/openstack/provider.go:66: // TODO: setup the goose client
auth/compute, etc. here
TODO(dimitern)

(we usually attach user names to TODOs)

https://codereview.appspot.com/6858052/diff/1/environs/openstack/provider.go#newcode119
environs/openstack/provider.go:119: // EnvironProvider interface
as for Environ comment above.

https://codereview.appspot.com/6858052/diff/1/environs/openstack/provider.go#newcode121
environs/openstack/provider.go:121: func (p environProvider) Open(cfg
*config.Config) (environs.Environ, error) {
i think it's worth putting these methods at the top of the file, as in
ec2 - an EnvironProvider is a precursor to the Environ, and it has many
less methods and much less code.

https://codereview.appspot.com/6858052/

[Martin Packman (gz)]

Have made a selection of comments on specifics, some of which is just
for things to worry about in future.

https://codereview.appspot.com/6858052/diff/1/environs/openstack/config.go
File environs/openstack/config.go (right):

https://codereview.appspot.com/6858052/diff/1/environs/openstack/config.go#newcode16
environs/openstack/config.go:16: "identity-url": schema.String(),
Where possible, use the existing keys names that python juju does, so
'auth-url' rather than 'identity-url' here.

https://codereview.appspot.com/6858052/diff/1/environs/openstack/config.go#newcode46
environs/openstack/config.go:46: func (c *environConfig) tenantId()
string {
Using tenant-id vs. tenant-name is something we'll need to watch out for
later.

https://codereview.appspot.com/6858052/diff/1/environs/openstack/config.go#newcode50
environs/openstack/config.go:50: func (c *environConfig) identityURL()
string {
As above, 'auth-url' instead.

https://codereview.appspot.com/6858052/diff/1/environs/openstack/config.go#newcode66
environs/openstack/config.go:66: func (p environProvider) Validate(cfg,
old *config.Config) (valid *config.Config, err error) {
Having one big function in the style of the current go code will get
painful with the various different authentication schemes openstack
uses, but we will want to delegate as much as possible to goose.

https://codereview.appspot.com/6858052/diff/1/environs/openstack/config.go#newcode115
environs/openstack/config.go:115: // TODO: temporarily here, until goose
client handles this
You should be able to use Credentials from the identity package, the
client has now been refactored to look at that.

https://codereview.appspot.com/6858052/diff/1/environs/openstack/config.go#newcode120
environs/openstack/config.go:120: func dummyEnvAuth() (dummyAuth, error)
{
Just use CredentialsFromEnv from the goose identity package as well.

https://codereview.appspot.com/6858052/diff/1/environs/openstack/config_test.go
File environs/openstack/config_test.go (right):

https://codereview.appspot.com/6858052/diff/1/environs/openstack/config_test.go#newcode40
environs/openstack/config_test.go:40: func (t configTest) check(c *C) {
This function is already big and scary and will presumably only get
worse when we add more authentication variations. Is there not a better
way?

https://codereview.appspot.com/6858052/diff/1/environs/openstack/config_test.go#newcode110
environs/openstack/config_test.go:110: func (s *ConfigSuite) SetUpTest(c
*C) {
This is kind of ugly, and we'll need to do better for testing envvars as
there are a bunch of different ones beyond these. The tests for this in
the python code used a change_environment helper that you passed a dict
of new values to within the test which were then stashed and restored at
the end.

https://codereview.appspot.com/6858052/diff/1/environs/openstack/config_test.go#newcode132
environs/openstack/config_test.go:132: "region": "lcy01",
I'd prefer not to bundle real canonistack values in here.

https://codereview.appspot.com/6858052/diff/1/environs/openstack/provider.go
File environs/openstack/provider.go (right):

https://codereview.appspot.com/6858052/diff/1/environs/openstack/provider.go#newcode1...

[Martin Packman (gz)]

Remembered that jam has already written a neat way of handling envvar
isolation in goose, that may want pulling out somewhere common so it can
be used here too.

https://codereview.appspot.com/6858052/diff/1/environs/openstack/config_test.go
File environs/openstack/config_test.go (right):

https://codereview.appspot.com/6858052/diff/1/environs/openstack/config_test.go#newcode110
environs/openstack/config_test.go:110: func (s *ConfigSuite) SetUpTest(c
*C) {
See also what goose/testing/envsuite provides, which is a neat
alternative.

https://codereview.appspot.com/6858052/

[John A Meinel (jameinel)]

LGTM
Worth landing with some tweaks, and iterating from here.

https://codereview.appspot.com/6858052/diff/1/environs/openstack/config.go
File environs/openstack/config.go (right):

https://codereview.appspot.com/6858052/diff/1/environs/openstack/config.go#newcode46
environs/openstack/config.go:46: func (c *environConfig) tenantId()
string {
On 2012/11/21 17:28:20, gz wrote:
> Using tenant-id vs. tenant-name is something we'll need to watch out
for later.

Just to follow up on this, "tenant-name" is what we'll get from the
environment, and what we would expect users to write themselves.
"tenant-id" would be something that we get from talking to the API.
So if we need to use it/store it we can, but ideally we would just go
with tenant name in the config.

https://codereview.appspot.com/6858052/diff/1/environs/openstack/config_test.go
File environs/openstack/config_test.go (right):

https://codereview.appspot.com/6858052/diff/1/environs/openstack/config_test.go#newcode18
environs/openstack/config_test.go:18: // Hook up gocheck into the gotest
runner.
On 2012/11/21 17:26:12, rog wrote:
> this is ubiquitous enough that it probably doesn't need a comment.

I would also mention that I don't usually see 1-line functions. They
pretty much are always wrapped to 3 lines. Am I mistaken about that?

https://codereview.appspot.com/6858052/diff/1/environs/openstack/config_test.go#newcode110
environs/openstack/config_test.go:110: func (s *ConfigSuite) SetUpTest(c
*C) {
On 2012/11/21 17:28:20, gz wrote:
> This is kind of ugly, and we'll need to do better for testing envvars
as there
> are a bunch of different ones beyond these. The tests for this in the
python
> code used a change_environment helper that you passed a dict of new
values to
> within the test which were then stashed and restored at the end.

If we switch to using CredentialsFromEnv, do we need a lot of env
variable testing here? It seems like permutation testing can be done at
the goose level, and then some sort of smoke test can be done here.

https://codereview.appspot.com/6858052/diff/1/environs/openstack/config_test.go#newcode130
environs/openstack/config_test.go:130: {
If we are going to go with table based tests instead of a bunch of named
tests, I'd like us to add summary: to the definitions, so that we have
some sort of name identifying what this permutation is trying to do.
Table based tests aren't that different from using a TestSuite that has
a common SetUp and TearDown and then you get a named test case for each
one. Hopefully these have less infrastructure making them easier to use,
but they do have the downside of not having human-sensible names unless
we put them there.

As a thought, I wonder if we might try writing this as TestSuite with
some helper setup functions, rather than as one big 'check' function.
And see which one is clearer. I can see where table based tests have
nice definitions. But you also tend to end up with a 'check' function
that mixes setup and assertions.

The 'check' function can become overloaded with lots of if checks,
because some cases want to test a failure mode, some want to test
setting a value, etc.

So there are bits I like (simple table definitio...

[Dimiter Naydenov (dimitern)]

Please take a look.

https://codereview.appspot.com/6858052/

[Martin Packman (gz)]

Changes all look good to me.

https://codereview.appspot.com/6858052/

[Gustavo Niemeyer (niemeyer)]

Nice. LGTM with trivials only:

https://codereview.appspot.com/6858052/diff/5002/environs/openstack/config.go
File environs/openstack/config.go (right):

https://codereview.appspot.com/6858052/diff/5002/environs/openstack/config.go#newcode54
environs/openstack/config.go:54: func (c *environConfig) container()
string {
As we discussed, let's use controlBucket/"control-bucket" for this
setting, for compatibility with the ec2 provider.

https://codereview.appspot.com/6858052/diff/5002/environs/openstack/config.go#newcode75
environs/openstack/config.go:75: ecfg.authURL() == "" {
Can we please have these in the same line? We have lines around the same
width around here, and it ends up more reasonable as it doesn't align
with the internal block.

https://codereview.appspot.com/6858052/diff/5002/environs/openstack/config.go#newcode79
environs/openstack/config.go:79: return nil, fmt.Errorf("environment has
no username, password, tenant-name, or auth-url")
"OpenStack environment requires options username, password, tenant-name,
and auth-url"

https://codereview.appspot.com/6858052/diff/5002/environs/openstack/config.go#newcode112
environs/openstack/config.go:112: type dummyAuth struct {
Why dummy? This looks quite real. I think it should be envAuth or
something like that.

https://codereview.appspot.com/6858052/diff/5002/environs/openstack/config.go#newcode116
environs/openstack/config.go:116: func dummyEnvAuth() (dummyAuth, error)
{
getEnvAuth?

https://codereview.appspot.com/6858052/diff/5002/environs/openstack/config.go#newcode126
environs/openstack/config.go:126: auth.authURL == "" {
Same line as well, please.

https://codereview.appspot.com/6858052/diff/5002/environs/openstack/config.go#newcode127
environs/openstack/config.go:127: return auth, fmt.Errorf("missing
username, password, tenant-name, or auth-url")
This function can be simplified to return (auth envAuth, ok bool)
instead. This error is pretty much the same one found above, and is
being discarded, which means the call site above is inferring what error
it is.

https://codereview.appspot.com/6858052/diff/5002/environs/openstack/config_test.go
File environs/openstack/config_test.go (right):

https://codereview.appspot.com/6858052/diff/5002/environs/openstack/config_test.go#newcode133
environs/openstack/config_test.go:133: var configTests = []configTest{
Nice tests.

https://codereview.appspot.com/6858052/

[Dimiter Naydenov (dimitern)]

https://codereview.appspot.com/6858052/diff/5002/environs/openstack/config.go
File environs/openstack/config.go (right):

https://codereview.appspot.com/6858052/diff/5002/environs/openstack/config.go#newcode54
environs/openstack/config.go:54: func (c *environConfig) container()
string {
On 2012/11/22 12:54:14, niemeyer wrote:
> As we discussed, let's use controlBucket/"control-bucket" for this
setting, for
> compatibility with the ec2 provider.

Done.

https://codereview.appspot.com/6858052/diff/5002/environs/openstack/config.go#newcode75
environs/openstack/config.go:75: ecfg.authURL() == "" {
On 2012/11/22 12:54:14, niemeyer wrote:
> Can we please have these in the same line? We have lines around the
same width
> around here, and it ends up more reasonable as it doesn't align with
the
> internal block.

Done.

https://codereview.appspot.com/6858052/diff/5002/environs/openstack/config.go#newcode79
environs/openstack/config.go:79: return nil, fmt.Errorf("environment has
no username, password, tenant-name, or auth-url")
On 2012/11/22 12:54:14, niemeyer wrote:
> "OpenStack environment requires options username, password,
tenant-name, and
> auth-url"

Done.

https://codereview.appspot.com/6858052/diff/5002/environs/openstack/config.go#newcode112
environs/openstack/config.go:112: type dummyAuth struct {
On 2012/11/22 12:54:14, niemeyer wrote:
> Why dummy? This looks quite real. I think it should be envAuth or
something like
> that.
This is here only because the goose identity package will handle this
eventually, but for now it's like this (hence the TODO), so we don't
couple the unstable code here. Anyway, renamed as suggested.

https://codereview.appspot.com/6858052/diff/5002/environs/openstack/config.go#newcode116
environs/openstack/config.go:116: func dummyEnvAuth() (dummyAuth, error)
{
On 2012/11/22 12:54:14, niemeyer wrote:
> getEnvAuth?

Done.

https://codereview.appspot.com/6858052/diff/5002/environs/openstack/config.go#newcode126
environs/openstack/config.go:126: auth.authURL == "" {
On 2012/11/22 12:54:14, niemeyer wrote:
> Same line as well, please.

Done.

https://codereview.appspot.com/6858052/

[Dimiter Naydenov (dimitern)]

Please take a look.

https://codereview.appspot.com/6858052/

[William Reade (fwereade)]

LGTM

https://codereview.appspot.com/6858052/

[Dimiter Naydenov (dimitern)]

On 2012/11/22 14:22:10, fwereade wrote:
> LGTM

As discussed, added checks for empty region and invalid auth-url.

https://codereview.appspot.com/6858052/

[Dimiter Naydenov (dimitern)]

Please take a look.

https://codereview.appspot.com/6858052/

[Roger Peppe (rogpeppe)]

On 2012/11/22 15:03:10, dimitern wrote:
> Please take a look.

LGTM

https://codereview.appspot.com/6858052/

[Dimiter Naydenov (dimitern)]

*** Submitted:

Implemented the base of a juju openstack provider.

Right now the configuration is parsed and validated, and the minimum
amount of code needed in the provider is implemented, the rest is
stubbed out for now.

R=rog, gz, john.meinel, niemeyer, fwereade
CC=
https://codereview.appspot.com/6858052

https://codereview.appspot.com/6858052/

=== added directory 'environs/openstack'
=== added file 'environs/openstack/config.go'
--- environs/openstack/config.go	1970-01-01 00:00:00 +0000
+++ environs/openstack/config.go	2012-11-22 15:03:19 +0000
@@ -0,0 +1,141 @@
+package openstack
+
+import (
+	"fmt"
+	"launchpad.net/juju-core/environs/config"
+	"launchpad.net/juju-core/schema"
+	"net/url"
+	"os"
+)
+
+var configChecker = schema.StrictFieldMap(
+	schema.Fields{
+		"username":       schema.String(),
+		"password":       schema.String(),
+		"tenant-name":    schema.String(),
+		"auth-url":       schema.String(),
+		"region":         schema.String(),
+		"control-bucket": schema.String(),
+	},
+	schema.Defaults{
+		"username":       "",
+		"password":       "",
+		"tenant-name":    "",
+		"auth-url":       "",
+		"region":         "",
+		"control-bucket": "",
+	},
+)
+
+type environConfig struct {
+	*config.Config
+	attrs map[string]interface{}
+}
+
+func (c *environConfig) region() string {
+	return c.attrs["region"].(string)
+}
+
+func (c *environConfig) username() string {
+	return c.attrs["username"].(string)
+}
+
+func (c *environConfig) password() string {
+	return c.attrs["password"].(string)
+}
+
+func (c *environConfig) tenantName() string {
+	return c.attrs["tenant-name"].(string)
+}
+
+func (c *environConfig) authURL() string {
+	return c.attrs["auth-url"].(string)
+}
+
+func (c *environConfig) controlBucket() string {
+	return c.attrs["control-bucket"].(string)
+}
+
+func (p environProvider) newConfig(cfg *config.Config) (*environConfig, error) {
+	valid, err := p.Validate(cfg, nil)
+	if err != nil {
+		return nil, err
+	}
+	return &environConfig{valid, valid.UnknownAttrs()}, nil
+}
+
+func (p environProvider) Validate(cfg, old *config.Config) (valid *config.Config, err error) {
+	v, err := configChecker.Coerce(cfg.UnknownAttrs(), nil)
+	if err != nil {
+		return nil, err
+	}
+	ecfg := &environConfig{cfg, v.(map[string]interface{})}
+
+	if ecfg.authURL() != "" {
+		parts, err := url.Parse(ecfg.authURL())
+		if err != nil || parts.Host == "" || parts.Scheme == "" {
+			return nil, fmt.Errorf("invalid auth-url value %q", ecfg.authURL())
+		}
+	}
+
+	if ecfg.username() == "" || ecfg.password() == "" || ecfg.tenantName() == "" || ecfg.authURL() == "" {
+		// TODO(dimitern): get goose client to handle this
+		auth, ok := getEnvAuth()
+		if !ok {
+			return nil, fmt.Errorf("OpenStack environment has no username, password, tenant-name, or auth-url")
+		}
+		ecfg.attrs["username"] = auth.username
+		ecfg.attrs["password"] = auth.password
+		ecfg.attrs["tenant-name"] = auth.tenantName
+		ecfg.attrs["auth-url"] = auth.authURL
+	}
+	// We cannot validate the region name, since each OS installation
+	// can have its own region names - only after authentication the
+	// region names are known (from the service endpoints)
+	if ecfg.region() == "" {
+		region := os.Getenv("OS_REGION_NAME")
+		if region != "" {
+			ecfg.attrs["region"] = region
+		} else {
+			return nil, fmt.Errorf("OpenStack environment has no region")
+		}
+	}
+
+	if old != nil {
+		attrs := old.UnknownAttrs()
+		if region, _ := attrs["region"].(string); ecfg.region() != region {
+			return nil, fmt.Errorf("cannot change region from %q to %q", region, ecfg.region())
+		}
+		if controlBucket, _ := attrs["control-bucket"].(string); ecfg.controlBucket() != controlBucket {
+			return nil, fmt.Errorf("cannot change control-bucket from %q to %q", controlBucket, ecfg.controlBucket())
+		}
+	}
+
+	switch cfg.FirewallMode() {
+	case config.FwDefault:
+		ecfg.attrs["firewall-mode"] = config.FwInstance
+	case config.FwInstance, config.FwGlobal:
+	default:
+		return nil, fmt.Errorf("unsupported firewall mode: %q", cfg.FirewallMode())
+	}
+
+	return cfg.Apply(ecfg.attrs)
+}
+
+// TODO(dimitern): temporarily here, until goose client handles this
+type envAuth struct {
+	username, password, tenantName, authURL string
+}
+
+func getEnvAuth() (auth envAuth, ok bool) {
+	auth = envAuth{
+		username:   os.Getenv("OS_USERNAME"),
+		password:   os.Getenv("OS_PASSWORD"),
+		tenantName: os.Getenv("OS_TENANT_NAME"),
+		authURL:    os.Getenv("OS_AUTH_URL"),
+	}
+	if auth.username == "" || auth.password == "" || auth.tenantName == "" || auth.authURL == "" {
+		return auth, false
+	}
+	return auth, true
+}

=== added file 'environs/openstack/config_test.go'
--- environs/openstack/config_test.go	1970-01-01 00:00:00 +0000
+++ environs/openstack/config_test.go	2012-11-22 15:03:19 +0000
@@ -0,0 +1,287 @@
+package openstack
+
+import (
+	. "launchpad.net/gocheck"
+	"launchpad.net/goyaml"
+	"launchpad.net/juju-core/environs"
+	"launchpad.net/juju-core/environs/config"
+	"os"
+	"testing"
+)
+
+type ConfigSuite struct {
+	savedVars map[string]string
+}
+
+var envVars = map[string]string{
+	"OS_USERNAME":    "testuser",
+	"OS_PASSWORD":    "testpass",
+	"OS_TENANT_NAME": "testtenant",
+	"OS_AUTH_URL":    "http://somehost",
+	"OS_REGION_NAME": "testreg",
+}
+
+var _ = Suite(&ConfigSuite{})
+
+func Test(t *testing.T) {
+	TestingT(t)
+}
+
+// configTest specifies a config parsing test, checking that env when
+// parsed as the openstack section of a config file matches
+// baseConfigResult when mutated by the mutate function, or that the
+// parse matches the given error.
+type configTest struct {
+	summary       string
+	config        attrs
+	change        attrs
+	region        string
+	controlBucket string
+	username      string
+	password      string
+	tenantName    string
+	authURL       string
+	firewallMode  config.FirewallMode
+	err           string
+}
+
+type attrs map[string]interface{}
+
+func (t configTest) check(c *C) {
+	envs := attrs{
+		"environments": attrs{
+			"testenv": attrs{
+				"type": "openstack",
+			},
+		},
+	}
+	testenv := envs["environments"].(attrs)["testenv"].(attrs)
+	for k, v := range t.config {
+		testenv[k] = v
+	}
+	if _, ok := testenv["control-bucket"]; !ok {
+		testenv["control-bucket"] = "x"
+	}
+	data, err := goyaml.Marshal(envs)
+	c.Assert(err, IsNil)
+
+	es, err := environs.ReadEnvironsBytes(data)
+	c.Check(err, IsNil)
+
+	e, err := es.Open("testenv")
+	if t.change != nil {
+		c.Assert(err, IsNil)
+
+		// Testing a change in configuration.
+		var old, changed, valid *config.Config
+		osenv := e.(*environ)
+		old = osenv.ecfg().Config
+		changed, err = old.Apply(t.change)
+		c.Assert(err, IsNil)
+
+		// Keep err for validation below.
+		valid, err = providerInstance.Validate(changed, old)
+		if err == nil {
+			err = osenv.SetConfig(valid)
+		}
+	}
+	if t.err != "" {
+		c.Check(err, ErrorMatches, t.err)
+		return
+	}
+	c.Assert(err, IsNil)
+
+	ecfg := e.(*environ).ecfg()
+	c.Assert(ecfg.Name(), Equals, "testenv")
+	c.Assert(ecfg.controlBucket(), Equals, "x")
+	if t.region != "" {
+		c.Assert(ecfg.region(), Equals, t.region)
+	}
+	if t.username != "" {
+		c.Assert(ecfg.username(), Equals, t.username)
+		c.Assert(ecfg.password(), Equals, t.password)
+		c.Assert(ecfg.tenantName(), Equals, t.tenantName)
+		c.Assert(ecfg.authURL(), Equals, t.authURL)
+		expected := map[string]interface{}{
+			"username":    t.username,
+			"password":    t.password,
+			"tenant-name": t.tenantName,
+		}
+		c.Assert(err, IsNil)
+		actual, err := e.Provider().SecretAttrs(ecfg.Config)
+		c.Assert(err, IsNil)
+		c.Assert(expected, DeepEquals, actual)
+	}
+	if t.firewallMode != "" {
+		c.Assert(ecfg.FirewallMode(), Equals, t.firewallMode)
+	}
+}
+
+func (s *ConfigSuite) SetUpTest(c *C) {
+	s.savedVars = make(map[string]string)
+	for v, val := range envVars {
+		s.savedVars[v] = os.Getenv(v)
+		os.Setenv(v, val)
+	}
+}
+
+func (s *ConfigSuite) TearDownTest(c *C) {
+	for v, val := range envVars {
+		os.Setenv(v, val)
+	}
+}
+
+var configTests = []configTest{
+	{
+		summary: "setting region",
+		config: attrs{
+			"region": "somereg",
+		},
+		region: "somereg",
+	}, {
+		summary: "setting region (2)",
+		config: attrs{
+			"region": "configtest",
+		},
+		region: "configtest",
+	}, {
+		summary: "changing region",
+		config: attrs{
+			"region": "configtest",
+		},
+		change: attrs{
+			"region": "somereg",
+		},
+		err: `cannot change region from "configtest" to "somereg"`,
+	}, {
+		summary: "invalid region",
+		config: attrs{
+			"region": 666,
+		},
+		err: ".*expected string, got 666",
+	}, {
+		summary: "invalid username",
+		config: attrs{
+			"username": 666,
+		},
+		err: ".*expected string, got 666",
+	}, {
+		summary: "invalid password",
+		config: attrs{
+			"password": 666,
+		},
+		err: ".*expected string, got 666",
+	}, {
+		summary: "invalid tenant-name",
+		config: attrs{
+			"tenant-name": 666,
+		},
+		err: ".*expected string, got 666",
+	}, {
+		summary: "invalid auth-url type",
+		config: attrs{
+			"auth-url": 666,
+		},
+		err: ".*expected string, got 666",
+	}, {
+		summary: "invalid auth-url format",
+		config: attrs{
+			"auth-url": "invalid",
+		},
+		err: `invalid auth-url value "invalid"`,
+	}, {
+		summary: "invalid control-bucket",
+		config: attrs{
+			"control-bucket": 666,
+		},
+		err: ".*expected string, got 666",
+	}, {
+		summary: "changing control-bucket",
+		change: attrs{
+			"control-bucket": "new-x",
+		},
+		err: `cannot change control-bucket from "x" to "new-x"`,
+	}, {
+		summary: "valid auth args",
+		config: attrs{
+			"username":    "jujuer",
+			"password":    "open sesame",
+			"tenant-name": "juju tenant",
+			"auth-url":    "http://some/url",
+		},
+		username:   "jujuer",
+		password:   "open sesame",
+		tenantName: "juju tenant",
+		authURL:    "http://some/url",
+	}, {
+		summary: "admin-secret given",
+		config: attrs{
+			"admin-secret": "Futumpsh",
+		},
+	}, {
+		summary:      "default firewall-mode",
+		config:       attrs{},
+		firewallMode: config.FwInstance,
+	}, {
+		summary: "unset firewall-mode",
+		config: attrs{
+			"firewall-mode": "",
+		},
+		firewallMode: config.FwInstance,
+	}, {
+		summary: "instance firewall-mode",
+		config: attrs{
+			"firewall-mode": "instance",
+		},
+		firewallMode: config.FwInstance,
+	}, {
+		summary: "global firewall-mode",
+		config: attrs{
+			"firewall-mode": "global",
+		},
+		firewallMode: config.FwGlobal,
+	},
+}
+
+func (s *ConfigSuite) TestConfig(c *C) {
+	for i, t := range configTests {
+		c.Logf("test %d: %s (%v)", i, t.summary, t.config)
+		t.check(c)
+	}
+}
+
+func (s *ConfigSuite) TestMissingRegion(c *C) {
+	os.Setenv("OS_REGION_NAME", "")
+	test := configTests[0]
+	delete(test.config, "region")
+	test.err = ".*environment has no region"
+	test.check(c)
+}
+
+func (s *ConfigSuite) TestMissingUsername(c *C) {
+	os.Setenv("OS_USERNAME", "")
+	test := configTests[0]
+	test.err = ".*environment has no username, password, tenant-name, or auth-url"
+	test.check(c)
+}
+
+func (s *ConfigSuite) TestMissingPassword(c *C) {
+	os.Setenv("OS_PASSWORD", "")
+	test := configTests[0]
+	test.err = ".*environment has no username, password, tenant-name, or auth-url"
+	test.check(c)
+}
+
+func (s *ConfigSuite) TestMissinTenant(c *C) {
+	os.Setenv("OS_TENANT_NAME", "")
+	test := configTests[0]
+	test.err = ".*environment has no username, password, tenant-name, or auth-url"
+	test.check(c)
+}
+
+func (s *ConfigSuite) TestMissingAuthUrl(c *C) {
+	os.Setenv("OS_AUTH_URL", "")
+	test := configTests[0]
+	test.err = ".*environment has no username, password, tenant-name, or auth-url"
+	test.check(c)
+}

=== added file 'environs/openstack/provider.go'
--- environs/openstack/provider.go	1970-01-01 00:00:00 +0000
+++ environs/openstack/provider.go	2012-11-22 15:03:19 +0000
@@ -0,0 +1,145 @@
+// Stub provider for OpenStack, using goose will be implemented here
+
+package openstack
+
+import (
+	"launchpad.net/juju-core/environs"
+	"launchpad.net/juju-core/environs/config"
+	"launchpad.net/juju-core/log"
+	"launchpad.net/juju-core/state"
+	"sync"
+)
+
+type environProvider struct{}
+
+var _ environs.EnvironProvider = (*environProvider)(nil)
+
+var providerInstance environProvider
+
+func init() {
+	environs.RegisterProvider("openstack", environProvider{})
+}
+
+func (p environProvider) Open(cfg *config.Config) (environs.Environ, error) {
+	log.Printf("environs/openstack: opening environment %q", cfg.Name())
+	e := new(environ)
+	err := e.SetConfig(cfg)
+	if err != nil {
+		return nil, err
+	}
+	return e, nil
+}
+
+func (p environProvider) SecretAttrs(cfg *config.Config) (map[string]interface{}, error) {
+	m := make(map[string]interface{})
+	ecfg, err := providerInstance.newConfig(cfg)
+	if err != nil {
+		return nil, err
+	}
+	m["username"] = ecfg.username()
+	m["password"] = ecfg.password()
+	m["tenant-name"] = ecfg.tenantName()
+	return m, nil
+}
+
+func (p environProvider) PublicAddress() (string, error) {
+	panic("not implemented")
+}
+
+func (p environProvider) PrivateAddress() (string, error) {
+	panic("not implemented")
+}
+
+type environ struct {
+	name string
+
+	ecfgMutex    sync.Mutex
+	ecfgUnlocked *environConfig
+}
+
+var _ environs.Environ = (*environ)(nil)
+
+func (e *environ) ecfg() *environConfig {
+	e.ecfgMutex.Lock()
+	ecfg := e.ecfgUnlocked
+	e.ecfgMutex.Unlock()
+	return ecfg
+}
+
+func (e *environ) Name() string {
+	return e.name
+}
+
+func (e *environ) Bootstrap(uploadTools bool, stateServerPEM []byte) error {
+	panic("not implemented")
+}
+
+func (e *environ) StateInfo() (*state.Info, error) {
+	panic("not implemented")
+}
+
+func (e *environ) Config() *config.Config {
+	panic("not implemented")
+}
+
+func (e *environ) SetConfig(cfg *config.Config) error {
+	ecfg, err := providerInstance.newConfig(cfg)
+	if err != nil {
+		return err
+	}
+	e.ecfgMutex.Lock()
+	defer e.ecfgMutex.Unlock()
+	e.name = ecfg.Name()
+	e.ecfgUnlocked = ecfg
+
+	// TODO(dimitern): setup the goose client auth/compute, etc. here
+	return nil
+}
+
+func (e *environ) StartInstance(machineId int, info *state.Info, tools *state.Tools) (environs.Instance, error) {
+	panic("not implemented")
+}
+
+func (e *environ) StopInstances([]environs.Instance) error {
+	panic("not implemented")
+}
+
+func (e *environ) Instances(ids []string) ([]environs.Instance, error) {
+	panic("not implemented")
+}
+
+func (e *environ) AllInstances() ([]environs.Instance, error) {
+	panic("not implemented")
+}
+
+func (e *environ) Storage() environs.Storage {
+	panic("not implemented")
+}
+
+func (e *environ) PublicStorage() environs.StorageReader {
+	panic("not implemented")
+}
+
+func (e *environ) Destroy(insts []environs.Instance) error {
+	panic("not implemented")
+}
+
+func (e *environ) AssignmentPolicy() state.AssignmentPolicy {
+	panic("not implemented")
+}
+
+func (e *environ) OpenPorts(ports []state.Port) error {
+	panic("not implemented")
+}
+
+func (e *environ) ClosePorts(ports []state.Port) error {
+	panic("not implemented")
+}
+
+func (e *environ) Ports() ([]state.Port, error) {
+	panic("not implemented")
+}
+
+func (e *environ) Provider() environs.EnvironProvider {
+	return &providerInstance
+}

=== modified file 'juju/bootstrap.go'
--- juju/bootstrap.go	2012-11-21 00:49:11 +0000
+++ juju/bootstrap.go	2012-11-22 15:03:19 +0000
@@ -74,8 +74,8 @@
 		SubjectKeyId:          bigIntHash(priv.N),
 		KeyUsage:              x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign,
 		BasicConstraintsValid: true,
-		IsCA:       true,
-		MaxPathLen: 0, // Disallow delegation for now.
+		IsCA:                  true,
+		MaxPathLen:            0, // Disallow delegation for now.
 	}
 	certDER, err := x509.CreateCertificate(rand.Reader, template, template, &priv.PublicKey, priv)
 	if err != nil {