cloud-init: rework user-data handling

Merge the user data specified by explicit terraform directives into
the raw data provided by the user. The raw data has priority over the
values specified using older directives.

Signed-off-by: Flavio Castelli <fcastelli@suse.com>

3 files changed, 224 insertions, 18 deletions

diff --git a/docs/providers/libvirt/r/cloudinit.html.markdown b/docs/providers/libvirt/r/cloudinit.html.markdown
index a2bddfab..e30cf55a 100644
--- a/docs/providers/libvirt/r/cloudinit.html.markdown
+++ b/docs/providers/libvirt/r/cloudinit.html.markdown
@@ -33,8 +33,8 @@ The following arguments are supported:
 * `ssh_authorized_key` - (Optional) A public ssh key that will be accepted by
   the `root` user.
 * `user_data` - (Optional) Raw cloud-init user data. This content will
-be merged automatically with the values specified in other arguments
-(like `local_hostname`, `ssh_authorized_key`, etc), but they cannot be
-specified in both places at the same time.
+  be merged automatically with the values specified in other arguments
+  (like `local_hostname`, `ssh_authorized_key`, etc). The contents of
+  `user_data` will take precedence over the ones defined by the other keys.
 
 Any change of the above fields will cause a new resource to be created.
diff --git a/libvirt/cloudinit_def.go b/libvirt/cloudinit_def.go
index 7ff16288..f681488b 100644
--- a/libvirt/cloudinit_def.go
+++ b/libvirt/cloudinit_def.go
@@ -14,6 +14,7 @@ import (
 
 	libvirt "github.com/dmacvicar/libvirt-go"
 	"github.com/hooklift/iso9660"
+	"github.com/imdario/mergo"
 	"github.com/mitchellh/packer/common/uuid"
 	"gopkg.in/yaml.v2"
 )
@@ -22,6 +23,10 @@ import (
 const USERDATA string = "user-data"
 const METADATA string = "meta-data"
 
+type UserDataStruct struct {
+	SSHAuthorizedKeys []string `yaml:"ssh_authorized_keys"`
+}
+
 type defCloudInit struct {
 	Name     string
 	PoolName string
@@ -30,9 +35,7 @@ type defCloudInit struct {
 		InstanceID    string `yaml:"instance-id"`
 	}
 	UserDataRaw string `yaml:"user_data"`
-	UserData    struct {
-		SSHAuthorizedKeys []string `yaml:"ssh_authorized_keys"`
-	}
+	UserData    UserDataStruct
 }
 
 // Creates a new cloudinit with the defaults
@@ -183,19 +186,11 @@ func (ci *defCloudInit) createFiles() (string, error) {
 	}
 
 	// Create files required by ISO file
-	userdata := ""
-	if len(ci.UserDataRaw) > 0 {
-		userdata = ci.UserDataRaw
-	} else {
-		userdata = "#cloud-config\n"
-	}
-
-	// append the extra user data flags
-	if userdata_extra, err := yaml.Marshal(&ci.UserData); err != nil {
-		return "", fmt.Errorf("Error dumping cloudinit's user data: %s", err)
-	} else {
-		userdata = fmt.Sprintf("%s\n%s", userdata, string(userdata_extra))
+	mergedUserData, err := mergeUserDataIntoUserDataRaw(ci.UserData, ci.UserDataRaw)
+	if err != nil {
+		return "", fmt.Errorf("Error merging UserData with UserDataRaw: %v", err)
 	}
+	userdata := fmt.Sprintf("#cloud-config\n%s", mergedUserData)
 
 	if err = ioutil.WriteFile(
 		filepath.Join(tmpDir, USERDATA),
@@ -338,3 +333,41 @@ func downloadISO(virConn *libvirt.VirConnection, volume libvirt.VirStorageVol) (
 
 	return file, nil
 }
+
+// Convert a UserData instance to a map with string as key and interface as value
+func convertUserDataToMap(data UserDataStruct) (map[string]interface{}, error) {
+	userDataMap := make(map[string]interface{})
+
+	// This is required to get the right names expected by cloud-init
+	// For example: SSHKeys -> ssh_authorized_keys
+	tmp, err := yaml.Marshal(&data)
+	if err != nil {
+		return userDataMap, err
+	}
+
+	err = yaml.Unmarshal([]byte(tmp), &userDataMap)
+	return userDataMap, err
+}
+
+func mergeUserDataIntoUserDataRaw(userData UserDataStruct, userDataRaw string) (string, error) {
+	userDataMap, err := convertUserDataToMap(userData)
+	if err != nil {
+		return "", err
+	}
+
+	userDataRawMap := make(map[string]interface{})
+	if err = yaml.Unmarshal([]byte(userDataRaw), &userDataRawMap); err != nil {
+		return "", err
+	}
+
+	if err = mergo.Merge(&userDataRawMap, userDataMap); err != nil {
+		return "", err
+	}
+
+	out, err := yaml.Marshal(userDataRawMap)
+	if err != nil {
+		return "", err
+	}
+
+	return string(out[:]), nil
+}
diff --git a/libvirt/cloudinit_def_test.go b/libvirt/cloudinit_def_test.go
new file mode 100644
index 00000000..2472e019
--- /dev/null
+++ b/libvirt/cloudinit_def_test.go
@@ -0,0 +1,173 @@
+package libvirt
+
+import (
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"gopkg.in/yaml.v2"
+)
+
+func TestNewCloudInitDef(t *testing.T) {
+	ci := newCloudInitDef()
+
+	if ci.Metadata.InstanceID == "" {
+		t.Error("Expected metadata InstanceID not to be empty")
+	}
+}
+
+func TestTerraformKeyOps(t *testing.T) {
+	ci := newCloudInitDef()
+
+	volKey := "volume-key"
+
+	terraformId := ci.buildTerraformKey(volKey)
+	if terraformId == "" {
+		t.Error("key should not be empty")
+	}
+
+	actualKey, _ := getCloudInitVolumeKeyFromTerraformID(terraformId)
+	if actualKey != volKey {
+		t.Error("wrong key returned")
+	}
+}
+
+func TestCreateFiles(t *testing.T) {
+	ci := newCloudInitDef()
+
+	dir, err := ci.createFiles()
+	if err != nil {
+		t.Errorf("Unexpected error %v", err)
+	}
+	defer os.RemoveAll(dir)
+
+	for _, file := range []string{USERDATA, METADATA} {
+		check, err := exists(filepath.Join(dir, file))
+		if !check {
+			t.Errorf("%s not found: %v", file, err)
+		}
+	}
+}
+
+func TestCreateISONoExteralTool(t *testing.T) {
+	path := os.Getenv("PATH")
+	defer os.Setenv("PATH", path)
+
+	os.Setenv("PATH", "/")
+
+	ci := newCloudInitDef()
+
+	iso, err := ci.createISO()
+	if err == nil {
+		t.Errorf("Expected error")
+	}
+
+	if iso != "" {
+		t.Errorf("Expected iso to be empty")
+	}
+}
+
+func TestConvertUserDataToMapPreservesCloudInitNames(t *testing.T) {
+	ud := UserDataStruct{
+		SSHAuthorizedKeys: []string{"key1"},
+	}
+
+	actual, err := convertUserDataToMap(ud)
+	if err != nil {
+		t.Errorf("Unexpectd error %v", err)
+	}
+
+	_, ok := actual["ssh_authorized_keys"]
+	if !ok {
+		t.Error("Could not found ssh_authorized_keys key")
+	}
+}
+
+func TestMergeEmptyUserDataIntoUserDataRaw(t *testing.T) {
+	ud := UserDataStruct{}
+
+	var userDataRaw = `
+new-key: new-value-set-by-extra
+ssh_authorized_keys:
+  - key2-from-extra-data
+`
+
+	res, err := mergeUserDataIntoUserDataRaw(ud, userDataRaw)
+	if err != nil {
+		t.Errorf("Unexpectd error %v", err)
+	}
+
+	actual := make(map[string]interface{})
+	err = yaml.Unmarshal([]byte(res), &actual)
+	if err != nil {
+		t.Errorf("Unexpectd error %v", err)
+	}
+
+	if _, ok := actual["ssh_authorized_keys"]; !ok {
+		t.Error("ssh_authorized_keys missing")
+	}
+
+	if _, ok := actual["new-key"]; !ok {
+		t.Error("new-key missing")
+	}
+}
+
+func TestMergeUserDataIntoEmptyUserDataRaw(t *testing.T) {
+	ud := UserDataStruct{
+		SSHAuthorizedKeys: []string{"key1"},
+	}
+	var userDataRaw string
+
+	res, err := mergeUserDataIntoUserDataRaw(ud, userDataRaw)
+	if err != nil {
+		t.Errorf("Unexpectd error %v", err)
+	}
+
+	actual := make(map[string]interface{})
+	err = yaml.Unmarshal([]byte(res), &actual)
+	if err != nil {
+		t.Errorf("Unexpectd error %v", err)
+	}
+
+	if _, ok := actual["ssh_authorized_keys"]; !ok {
+		t.Error("ssh_authorized_keys missing")
+	}
+}
+
+func TestMergeUserDataIntoUserDataRawGivesPrecedenceToRawData(t *testing.T) {
+	ud_key := "user-data-key"
+	ud := UserDataStruct{
+		SSHAuthorizedKeys: []string{ud_key},
+	}
+
+	var userDataRaw = `
+new-key: new-value-set-by-extra
+ssh_authorized_keys:
+  - key2-from-extra-data
+`
+
+	res, err := mergeUserDataIntoUserDataRaw(ud, userDataRaw)
+	if err != nil {
+		t.Errorf("Unexpectd error %v", err)
+	}
+
+	if strings.Contains(res, ud_key) {
+		t.Error("Should not have found string defined by user data")
+	}
+
+	if !strings.Contains(res, "key2-from-extra-data") {
+		t.Error("Should have found string defined by raw data")
+	}
+}
+
+func exists(path string) (bool, error) {
+	_, err := os.Stat(path)
+	if err == nil {
+		return true, nil
+	}
+	if os.IsNotExist(err) {
+		return false, nil
+	}
+	return true, err
+}