Introduce the format 1.16 for agent confg.

agent/format-1.16.go

Index: agent/format-1.16.go
=== added file 'agent/format-1.16.go'
--- agent/format-1.16.go	1970-01-01 00:00:00 +0000
+++ agent/format-1.16.go	2013-08-30 04:47:00 +0000
@@ -0,0 +1,196 @@
+// Copyright 2013 Canonical Ltd.
+// Licensed under the AGPLv3, see LICENCE file for details.
+
+package agent
+
+import (
+	"encoding/base64"
+	"io/ioutil"
+	"os"
+	"path"
+
+	"launchpad.net/goyaml"
+)
+
+const (
+	format116 = "format 1.16"
+	// Old environment variables that are now stored in agent config.
+	JujuLxcBridge         = "JUJU_LXC_BRIDGE"
+	JujuProviderType      = "JUJU_PROVIDER_TYPE"
+	JujuStorageDir        = "JUJU_STORAGE_DIR"
+	JujuStorageAddr       = "JUJU_STORAGE_ADDR"
+	JujuSharedStorageDir  = "JUJU_SHARED_STORAGE_DIR"
+	JujuSharedStorageAddr = "JUJU_SHARED_STORAGE_ADDR"
+)
+
+// formatter116 is the formatter for the 1.16 format.
+type formatter116 struct {

[jameinel, 2013/09/02 06:44:16]

I think this needs to be renamed to formatter_1_16 in keeping with what you did
for the 1_12 formats.

+}
+
+// format116Serialization holds information for a given agent.
+type format116Serialization struct {
+	Tag   string
+	Nonce string
+	// CACert is base64 encoded
+	CACert         string
+	StateAddresses []string `yaml:",omitempty"`
+	StatePassword  string   `yaml:",omitempty"`
+
+	APIAddresses []string `yaml:",omitempty"`
+	APIPassword  string   `yaml:",omitempty"`
+
+	OldPassword string
+	Values      map[string]string
+
+	// Only state server machines have these next three items
+	StateServerCert string `yaml:",omitempty"`
+	StateServerKey  string `yaml:",omitempty"`
+	APIPort         int    `yaml:",omitempty"`
+}
+
+// Ensure that the formatter116 struct implements the formatter interface.
+var _ formatter = (*formatter116)(nil)
+
+func (*formatter116) configFile(dirName string) string {
+	return path.Join(dirName, "agent.conf")
+}
+
+// decode makes sure that for an empty string we have a nil slice,
+// not an empty slice, which is what the DecodeString returns.

[jameinel, 2013/09/02 06:44:16]

I think the doc string isn't quite complete, as this actually does base64
decoding. We may also want to mention that for an empty string we end up with a
nil slice rather than an empty slice.

However, should we call this decode64 or something to make it clearer that it
isn't a generic decode function, but is about decoding base64 encoded content?

+func (*formatter116) decode(value string) (result []byte, err error) {
+	if value != "" {
+		result, err = base64.StdEncoding.DecodeString(value)
+	}
+	return
+}
+
+func (formatter *formatter116) read(dirName string) (*configInternal, error) {

[jameinel, 2013/09/02 06:44:16]

I take it your 1.16 format doesn't actually split the content into separate
files. It just is about moving the Certs into base64 encoded strings and pulling
Values from ENV variables into the Values map in the data stream.

+	data, err := ioutil.ReadFile(formatter.configFile(dirName))
+	if err != nil {
+		return nil, err
+	}
+	var format format116Serialization
+	if err := goyaml.Unmarshal(data, &format); err != nil {
+		return nil, err
+	}
+	caCert, err := formatter.decode(format.CACert)
+	if err != nil {
+		return nil, err
+	}
+	stateServerCert, err := formatter.decode(format.StateServerCert)
+	if err != nil {
+		return nil, err
+	}
+	stateServerKey, err := formatter.decode(format.StateServerKey)
+	if err != nil {
+		return nil, err
+	}
+	config := &configInternal{
+		tag:             format.Tag,
+		nonce:           format.Nonce,
+		caCert:          caCert,
+		oldPassword:     format.OldPassword,
+		stateServerCert: stateServerCert,
+		stateServerKey:  stateServerKey,
+		apiPort:         format.APIPort,
+		values:          format.Values,
+	}
+	if len(format.StateAddresses) > 0 {
+		config.stateDetails = &connectionDetails{
+			format.StateAddresses,
+			format.StatePassword,
+		}
+	}
+	if len(format.APIAddresses) > 0 {
+		config.apiDetails = &connectionDetails{
+			format.APIAddresses,
+			format.APIPassword,
+		}
+	}
+	return config, nil
+}
+
+func (formatter *formatter116) makeFormat(config *configInternal) *format116Serialization {
+	format := &format116Serialization{
+		Tag:             config.tag,
+		Nonce:           config.nonce,
+		CACert:          base64.StdEncoding.EncodeToString(config.caCert),
+		OldPassword:     config.oldPassword,
+		StateServerCert: base64.StdEncoding.EncodeToString(config.stateServerCert),
+		StateServerKey:  base64.StdEncoding.EncodeToString(config.stateServerKey),
+		APIPort:         config.apiPort,
+		Values:          config.values,
+	}
+	if config.stateDetails != nil {
+		format.StateAddresses = config.stateDetails.addresses
+		format.StatePassword = config.stateDetails.password
+	}
+	if config.apiDetails != nil {
+		format.APIAddresses = config.apiDetails.addresses
+		format.APIPassword = config.apiDetails.password
+	}
+	return format
+}
+
+func (formatter *formatter116) write(config *configInternal) error {
+	dirName := config.Dir()
+	conf := formatter.makeFormat(config)
+	data, err := goyaml.Marshal(conf)
+	if err != nil {
+		return err
+	}

[jameinel, 2013/09/02 06:44:16]

You seem to atomically create the contents of agent.conf but you universally
splat the format file down.

At the very least, we should probably do:
write agent.conf-new, write format file, rename agent.conf-new => target.

That gives us a smaller vulnerability. One could argue that you write the format
file last (in case permissions are such that you can't rename over the
agent.conf file itself.)

But otherwise I would expect writing to fail before renaming.

+	if err := writeFormatFile(dirName, format116); err != nil {
+		return err
+	}
+	newFile := path.Join(dirName, "agent.conf-new")
+	if err := ioutil.WriteFile(newFile, data, 0600); err != nil {
+		return err
+	}
+	if err := os.Rename(newFile, formatter.configFile(dirName)); err != nil {
+		return err
+	}
+	return nil
+}
+
+func (formatter *formatter116) writeCommands(config *configInternal) ([]string, error) {
+	dirName := config.Dir()
+	conf := formatter.makeFormat(config)
+	data, err := goyaml.Marshal(conf)
+	if err != nil {
+		return nil, err
+	}
+	commands := writeCommandsForFormat(dirName, format116)
+	commands = append(commands,
+		writeFileCommands(formatter.configFile(dirName), string(data), 0600)...)
+	return commands, nil
+}
+
+func (*formatter116) migrate(config *configInternal) {
+	for _, name := range []struct {
+		environment string
+		config      string
+	}{{
+		JujuProviderType,
+		ProviderType,
+	}, {
+		JujuLxcBridge,
+		LxcBridge,
+	}, {
+		JujuStorageDir,
+		StorageDir,
+	}, {
+		JujuStorageAddr,
+		StorageAddr,
+	}, {
+		JujuSharedStorageDir,
+		SharedStorageDir,
+	}, {
+		JujuSharedStorageAddr,
+		SharedStorageAddr,
+	}} {
+		value := os.Getenv(name.environment)
+		if value != "" {
+			config.values[name.config] = value
+		}
+	}
+}