fix: Improve bind imports, support lower case 'source' in yaml, add go test

pkg/types/layer.go

@@ -2,7 +2,6 @@
 
 import (
 	"encoding/json"
-	"fmt"
 	"io/fs"
 	"os"
 	"path/filepath"
@@ -73,40 +72,6 @@
 	Packages []Package `yaml:"packages" json:"packages,omitempty"`
 }
 
-func validateDataAsBind(i interface{}) (map[interface{}]interface{}, error) {
-	bindMap, ok := i.(map[interface{}]interface{})
-	if !ok {
-		return nil, errors.Errorf("unable to cast into map[interface{}]interface{}: %T", i)
-	}
-
-	// validations
-	bindSource, ok := bindMap["Source"]
-	if !ok {
-		return nil, errors.Errorf("bind source missing: %v", i)
-	}
-
-	_, ok = bindSource.(string)
-	if !ok {
-		return nil, errors.Errorf("unknown bind source type, expected string: %T", i)
-	}
-
-	bindDest, ok := bindMap["Dest"]
-	if !ok {
-		return nil, errors.Errorf("bind dest missing: %v", i)
-	}
-
-	_, ok = bindDest.(string)
-	if !ok {
-		return nil, errors.Errorf("unknown bind dest type, expected string: %T", i)
-	}
-
-	if bindSource == "" || bindDest == "" {
-		return nil, errors.Errorf("empty source or dest: %v", i)
-	}
-
-	return bindMap, nil
-}
-
 func getStringOrStringSlice(data interface{}, xform func(string) ([]string, error)) ([]string, error) {
 	// The user didn't supply run: at all, so let's not do anything.
 	if data == nil {
@@ -125,14 +90,6 @@
 			switch v := i.(type) {
 			case string:
 				s = v
-			case interface{}:
-				bindMap, err := validateDataAsBind(i)
-				if err != nil {
-					return nil, err
-				}
-
-				// validations passed, return as string in form: source -> dest
-				s = fmt.Sprintf("%s -> %s", bindMap["Source"], bindMap["Dest"])
 			default:
 				return nil, errors.Errorf("unknown run array type: %T", i)
 			}
@@ -205,87 +162,77 @@
 	return nil
 }
 
-type Bind struct {
-	Source string `yaml:"source" json:"source"`
-	Dest   string `yaml:"dest" json:"dest"`
+type bindType struct {
+	Source string `yaml:"source" json:"source,omitempty"`
+	Dest   string `yaml:"dest" json:"dest,omitempty"`
 }
 
-type Binds []Bind
-
-func (bs *Bind) MarshalJSON() ([]byte, error) {
-	var sb strings.Builder
+// toBind - copy to Bind type and check.
+func (b *bindType) toBind(bs *Bind) error {
+	if b.Source == "" {
+		return errors.Errorf("unexpected 'bind': missing required field 'source': %#v", b)
+	}
+	bs.Source = b.Source
+	bs.Dest = b.Dest
 	if bs.Dest == "" {
-		sb.WriteString(fmt.Sprintf("%q", bs.Source))
-	} else {
-		var sbt strings.Builder
-		sbt.WriteString(fmt.Sprintf("%s -> %s", bs.Source, bs.Dest))
-		sb.WriteString(fmt.Sprintf("%q", sbt.String()))
+		bs.Dest = bs.Source
 	}
-
-	return []byte(sb.String()), nil
+	return nil
 }
 
-func (bs *Binds) UnmarshalJSON(data []byte) error {
-	var rawBinds []string
-
-	if err := json.Unmarshal(data, &rawBinds); err != nil {
-		return err
-	}
-
-	*bs = Binds{}
-	for _, bind := range rawBinds {
-		parts := strings.Split(bind, "->")
-		if len(parts) != 1 && len(parts) != 2 {
-			return errors.Errorf("invalid bind mount %s", bind)
-		}
-
-		source := strings.TrimSpace(parts[0])
-		target := source
-
-		if len(parts) == 2 {
-			target = strings.TrimSpace(parts[1])
-		}
-
-		*bs = append(*bs, Bind{Source: source, Dest: target})
-	}
+func (b *bindType) toBindFromString(bind *Bind, asStr string) error {
+	toks := strings.Fields(asStr)
+	if len(toks) == 1 {
+		bind.Source = toks[0]
+		bind.Dest = toks[0]
+		return nil
+	} else if len(toks) == 3 && toks[1] == "->" {
+		bind.Source = toks[0]
+		bind.Dest = toks[2]
+		return nil
+	}
+	return errors.Errorf("invalid Bind: %s", string(asStr))
+}
 
-	return nil
+type Bind struct {
+	Source string `yaml:"source,omitempty" json:"source,omitempty"`
+	Dest   string `yaml:"dest,omitempty" json:"dest,omitempty"`
 }
 
-func (bs *Binds) UnmarshalYAML(unmarshal func(interface{}) error) error {
-	var data interface{}
-	err := unmarshal(&data)
-	if err != nil {
-		return errors.WithStack(err)
-	}
+type Binds []Bind
 
-	xform := func(s string) ([]string, error) {
-		return []string{s}, nil
+func (bs *Bind) UnmarshalJSON(data []byte) error {
+	btype := bindType{}
+	if err := json.Unmarshal(data, &btype); err == nil {
+		return btype.toBind(bs)
 	}
 
-	rawBinds, err := getStringOrStringSlice(data, xform)
+	asStr := ""
+	err := json.Unmarshal(data, &asStr)
 	if err != nil {
-		return err
+		return errors.Errorf("invalid Bind: %s", string(data))
 	}
 
-	*bs = Binds{}
-	for _, bind := range rawBinds {
-		parts := strings.Split(bind, "->")
-		if len(parts) != 1 && len(parts) != 2 {
-			return errors.Errorf("invalid bind mount %s", bind)
-		}
+	return btype.toBindFromString(bs, asStr)
+}
 
-		source := strings.TrimSpace(parts[0])
-		target := source
+func (bs *Bind) UnmarshalYAML(unmarshal func(interface{}) error) error {
+	btype := bindType{}
+	if err := unmarshal(&btype); err == nil {
+		return btype.toBind(bs)
+	}
 
-		if len(parts) == 2 {
-			target = strings.TrimSpace(parts[1])
-		}
+	asStr := ""
+	if err := unmarshal(&asStr); err == nil {
+		return btype.toBindFromString(bs, asStr)
+	}
 
-		*bs = append(*bs, Bind{Source: source, Dest: target})
+	var data interface{}
+	if err := unmarshal(&data); err != nil {
+		return errors.Errorf("unexpected error unmarshaling bind yaml: %v", err)
 	}
 
-	return nil
+	return errors.Errorf("unexpected 'bind' data of type: %s: %#v", reflect.TypeOf(data), data)
 }
 
 type Layer struct {

pkg/types/layer_bind_test.go

@@ -0,0 +1,84 @@
+package types
+
+import (
+	"encoding/json"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"gopkg.in/yaml.v2"
+)
+
+func TestUnmarshalBindsYamlAndJSON(t *testing.T) {
+	assert := assert.New(t)
+	tables := []struct {
+		desc     string
+		yblob    string
+		jblob    string
+		expected Binds
+		errstr   string
+	}{
+		{desc: "proper array of source/dest bind allowed",
+			yblob: "- source: src1\n  dest: dest1\n",
+			jblob: `[{"source": "src1", "dest": "dest1"}]`,
+			expected: Binds{
+				Bind{Source: "src1", Dest: "dest1"},
+			}},
+		{desc: "array of bind ascii art",
+			yblob: "- src1 -> dest1\n- src2 -> dest2",
+			jblob: `["src1 -> dest1", "src2 -> dest2"]`,
+			expected: Binds{
+				Bind{Source: "src1", Dest: "dest1"},
+				Bind{Source: "src2", Dest: "dest2"},
+			}},
+		{desc: "example mixed valid ascii art and dict",
+			yblob: "- src1 -> dest1\n- source: src2\n  dest: dest2\n",
+			jblob: `["src1 -> dest1", {"source": "src2", "dest": "dest2"}]`,
+			expected: Binds{
+				Bind{Source: "src1", Dest: "dest1"},
+				Bind{Source: "src2", Dest: "dest2"},
+			}},
+		// golang encoding/json is case insensitive
+		{desc: "capital Source/Dest is not allowed as yaml",
+			yblob:    "- Source: src1\n  Dest: dest1\n",
+			expected: Binds{},
+			errstr:   "xpected 'bind'"},
+		{desc: "source is required",
+			yblob:    "- Dest: dest1\n",
+			jblob:    `[{"Dest": "dest1"}]`,
+			expected: Binds{},
+			errstr:   "xpected 'bind'"},
+		{desc: "must be an array",
+			yblob:    "source: src1\ndest: dest1\n",
+			jblob:    `{"source": "src1", "dest": "dest1"}`,
+			expected: Binds{},
+			errstr:   "unmarshal"},
+	}
+	var err error
+	found := Binds{}
+	for _, t := range tables {
+		err = yaml.Unmarshal([]byte(t.yblob), &found)
+		if t.errstr == "" {
+			if !assert.NoError(err, t.desc) {
+				continue
+			}
+			assert.Equal(t.expected, found)
+		} else {
+			assert.ErrorContains(err, t.errstr, t.desc)
+		}
+	}
+
+	for _, t := range tables {
+		if t.jblob == "" {
+			continue
+		}
+		err = json.Unmarshal([]byte(t.jblob), &found)
+		if t.errstr == "" {
+			if !assert.NoError(err, t.desc) {
+				continue
+			}
+			assert.Equal(t.expected, found)
+		} else {
+			assert.ErrorContains(err, t.errstr, t.desc)
+		}
+	}
+}

test/binds.bats

@@ -43,22 +43,32 @@ bind-test:
         type: oci
         url: ${{CENTOS_OCI}}
     binds:
-        - Source: ${{bind_path}}
-          Dest: /root/tree1/foo
+        - source: ${{bind_path1}}
+          dest: /root/tree1/foo
+        - source: ${{bind_path2}}
     run: |
         touch /root/tree1/foo/bar
+        [ -f "${{bind_path2}}/file1" ]
 EOF
-    mkdir -p tree1/foo
+    mkdir -p tree1/foo tree2/bar
+    touch tree2/bar/file1
 
     # since we are creating directory as
     # real root and then `touch`-ing a file
     # where in user NS, need to have rw persmission
     # for others
     chmod +666 tree1/foo
 
-    bind_path=$(realpath tree1/foo)
+    bind_path1=$(realpath tree1/foo)
+    bind_path2=$(realpath tree2/bar)
 
-    out=$(stacker build --substitute bind_path=$bind_path  --substitute CENTOS_OCI=$CENTOS_OCI)
+    out=$(stacker build \
+        "--substitute=bind_path1=${bind_path1}" \
+        "--substitute=bind_path2=${bind_path2}" \
+        "--substitute=CENTOS_OCI=$CENTOS_OCI" ) || {
+             printf "%s\n" "$out" 1>&2
+             exit 1
+        }
     [[ "${out}" =~ ^(.*filesystem bind-test built successfully)$ ]]
 
     stat tree1/foo/bar