Accept string or numeric spec.host.files.perm (#248)

Author: kke

config/cluster/uploadfile.go

@@ -1,15 +1,60 @@
 package cluster
 
 import (
+	"fmt"
 	"path/filepath"
+	"strconv"
 )
 
 // UploadFile describes a file to be uploaded for the host
 type UploadFile struct {
-	Name           string `yaml:"name,omitempty"`
-	Source         string `yaml:"src" validate:"required"`
-	DestinationDir string `yaml:"dstDir" validate:"required"`
-	PermMode       string `yaml:"perm" default:"0755"`
+	Name           string      `yaml:"name,omitempty"`
+	Source         string      `yaml:"src" validate:"required"`
+	DestinationDir string      `yaml:"dstDir" validate:"required"`
+	PermMode       interface{} `yaml:"perm" default:"0755"`
+	PermString     string      `yaml:"-"`
+}
+
+// UnmarshalYAML sets in some sane defaults when unmarshaling the data from yaml
+func (u *UploadFile) UnmarshalYAML(unmarshal func(interface{}) error) error {
+	type uploadFile UploadFile
+	yu := (*uploadFile)(u)
+
+	if err := unmarshal(yu); err != nil {
+		return err
+	}
+
+	switch t := u.PermMode.(type) {
+	case int:
+		if t < 0 {
+			return fmt.Errorf("invalid uploadFile permission: %d: must be a positive value", t)
+		}
+		if t == 0 {
+			return fmt.Errorf("invalid nil uploadFile permission")
+		}
+		u.PermString = fmt.Sprintf("%#o", t)
+	case string:
+		u.PermString = t
+	default:
+		return fmt.Errorf("invalid value for uploadFile perm, must be a string or a number")
+	}
+
+	for i, c := range u.PermString {
+		n, err := strconv.Atoi(string(c))
+		if err != nil {
+			return fmt.Errorf("failed to parse uploadFile permission %s: %w", u.PermString, err)
+		}
+
+		// These could catch some weird octal conversion mistakes
+		if i == 1 && n < 4 {
+			return fmt.Errorf("invalid uploadFile permission %s: owner would have unconventional access", u.PermString)
+		}
+		if n > 7 {
+			return fmt.Errorf("invalid uploadFile permission %s: octal value can't have numbers over 7", u.PermString)
+		}
+	}
+
+	return nil
 }
 
 func (u *UploadFile) Resolve() ([]string, error) {

config/cluster/uploadfile_test.go

@@ -0,0 +1,66 @@
+package cluster
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	"gopkg.in/yaml.v2"
+)
+
+func TestPermStringUnmarshalWithOctal(t *testing.T) {
+	u := UploadFile{}
+	yml := []byte(`
+src: /tmp
+dstDir: /tmp
+perm: 0755
+`)
+
+	require.NoError(t, yaml.Unmarshal(yml, &u))
+	require.Equal(t, "0755", u.PermString)
+}
+
+func TestPermStringUnmarshalWithString(t *testing.T) {
+	u := UploadFile{}
+	yml := []byte(`
+src: /tmp
+dstDir: /tmp
+perm: "0755"
+`)
+
+	require.NoError(t, yaml.Unmarshal(yml, &u))
+	require.Equal(t, "0755", u.PermString)
+}
+
+func TestPermStringUnmarshalWithInvalidString(t *testing.T) {
+	u := UploadFile{}
+	yml := []byte(`
+src: /tmp
+dstDir: /tmp
+perm: u+rwx
+`)
+
+	require.Error(t, yaml.Unmarshal(yml, &u))
+}
+
+func TestPermStringUnmarshalWithInvalidNumber(t *testing.T) {
+	u := UploadFile{}
+	yml := []byte(`
+src: /tmp
+dstDir: /tmp
+perm: 0800
+`)
+
+	require.Error(t, yaml.Unmarshal(yml, &u))
+}
+
+func TestPermStringUnmarshalWithZero(t *testing.T) {
+	u := UploadFile{}
+	yml := []byte(`
+src: /tmp
+dstDir: /tmp
+perm: 0
+`)
+	t.Logf("what the hell: %+v %+v", u.PermMode, u.PermString)
+
+	require.Error(t, yaml.Unmarshal(yml, &u))
+}

phase/uploadfiles.go

@@ -50,7 +50,7 @@ func (p *UploadFiles) uploadFiles(h *cluster.Host) error {
 			return err
 		}
 
-		if err := h.Execf("install -d %s -m %s", f.DestinationDir, f.PermMode, exec.Sudo(h)); err != nil {
+		if err := h.Execf("install -d %s -m %s", f.DestinationDir, f.PermString, exec.Sudo(h)); err != nil {
 			return err
 		}
 
@@ -62,7 +62,7 @@ func (p *UploadFiles) uploadFiles(h *cluster.Host) error {
 				return err
 			}
 
-			if err := h.Configurer.Chmod(h, destination, f.PermMode); err != nil {
+			if err := h.Configurer.Chmod(h, destination, f.PermString); err != nil {
 				return err
 			}
 		}