Merge pull request #581 from cloudfoundry/refactor_allow_newlines_in_var

Try to improve code readability of var_kv

director/template/var_kv.go

@@ -12,33 +12,41 @@ type VarKV struct {
 	Value interface{}
 }
 
+func trimWrappingQuotes(s string) string {
+	if (s[0] == '"' && s[len(s)-1] == '"') || (s[0] == '\'' && s[len(s)-1] == '\'') {
+		s = s[1 : len(s)-1]
+		s = trimWrappingQuotes(s)
+	}
+	return s
+}
 func (a *VarKV) UnmarshalFlag(data string) error {
 	pieces := strings.SplitN(data, "=", 2)
+	const nameIndex, valueIndex = 0, 1
 	if len(pieces) != 2 {
 		return bosherr.Errorf("Expected var '%s' to be in format 'name=value'", data)
 	}
 
-	if len(pieces[0]) == 0 {
+	if len(pieces[nameIndex]) == 0 {
 		return bosherr.Errorf("Expected var '%s' to specify non-empty name", data)
 	}
 
-	if len(pieces[1]) == 0 {
+	if len(pieces[valueIndex]) == 0 {
 		return bosherr.Errorf("Expected var '%s' to specify non-empty value", data)
 	}
 
 	var vars interface{}
 
-	err := yaml.Unmarshal([]byte(pieces[1]), &vars)
-	if err != nil {
-		return bosherr.WrapErrorf(err, "Deserializing variables '%s'", data)
-	}
-
+	err := yaml.Unmarshal([]byte(pieces[valueIndex]), &vars)
+	//If for whatever reason YAML unmarshal fails, treat the second part of pieces as a string
 	//yaml.Unmarshal returns a string if the input is not valid yaml.
-	//in that case, we pass through the string itself as the Unmarshal process strips newlines.
-	if _, ok := vars.(string); ok {
-		*a = VarKV{Name: pieces[0], Value: pieces[1]}
+	if _, ok := vars.(string); ok || err != nil {
+		//in that case, we return the initial flag value as the Unmarshal
+		//process strips newlines. Stripping the quotes is required to keep
+		//backwards compability (YAML unmarshal also removed wrapping quotes from the value).
+		*a = VarKV{Name: pieces[nameIndex], Value: trimWrappingQuotes(pieces[valueIndex])}
 	} else {
-		*a = VarKV{Name: pieces[0], Value: vars}
+		//otherwise, return the parsed YAML object
+		*a = VarKV{Name: pieces[nameIndex], Value: vars}
 	}
 
 	return nil

director/template/var_kv_test.go

@@ -23,6 +23,33 @@ var _ = Describe("VarKV", func() {
 			Expect(arg).To(Equal(VarKV{Name: "name", Value: "val"}))
 		})
 
+		It("reverts to the old (incorrect) bevaviour and removes the wrapping double quotes if the value is a string", func() {
+			err := (&arg).UnmarshalFlag(`name="val"`)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(arg).To(Equal(VarKV{Name: "name", Value: "val"}))
+		})
+		It("reverts to the old (incorrect) bevaviour and removes the wrapping single quotes if the value is a string", func() {
+			err := (&arg).UnmarshalFlag(`name='val'`)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(arg).To(Equal(VarKV{Name: "name", Value: "val"}))
+		})
+		It("only removes wrapping quotes", func() {
+			err := (&arg).UnmarshalFlag(`name="'val""val'"`)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(arg).To(Equal(VarKV{Name: "name", Value: `val""val`}))
+
+			err = (&arg).UnmarshalFlag(`name="val''val"`)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(arg).To(Equal(VarKV{Name: "name", Value: `val''val`}))
+
+			err = (&arg).UnmarshalFlag(`name='"some-data'"`)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(arg).To(Equal(VarKV{Name: "name", Value: `'"some-data'"`}))
+
+			err = (&arg).UnmarshalFlag(`name="'some-data"'`)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(arg).To(Equal(VarKV{Name: "name", Value: `"'some-data"'`}))
+		})
 		It("sets name and value when value contains a `=`", func() {
 			err := (&arg).UnmarshalFlag("public_key=ssh-rsa G4/+VHa1aw==")
 			Expect(err).ToNot(HaveOccurred())