WIP replace path and args with command field in Execute customization

Signed-off-by: Paul Mars <[email protected]>

internal/imagedefinition/README.rst

@@ -250,18 +250,18 @@ The following specification defines what is supported in the YAML:
             # The location of the rootfs will be prepended to this
             # path automatically.
             path: <string>
-        # Chroots into the rootfs and executes an executable file.
+        # Chroots into the rootfs and executes comamnds and executables.
         # This customization state is run after the copy-files state,
         # so files that have been copied into the rootfs are valid
         # targets to be executed.
         execute: (optional)
           -
-            # Path inside the rootfs.
+            # Path to a executable inside the rootfs. This field only 
+            # supports a single path and not arguments. This field is
+            # deprecated. Use command instead.
             path: <string>
-            # Arguments to give to the command
-            args: (optional)
-              - <string>
-              - <string>
+            # Full command to execute, including arguments
+            command: <string>
             # Environment variables to set before executing the command
             # Format: ENV=VALUE
             env: (optional)

internal/imagedefinition/image_definition.go

@@ -148,9 +148,9 @@ type CopyFile struct {
 
 // Execute allows users to execute a script/command in the rootfs of an image
 type Execute struct {
-	ExecutePath string   `yaml:"path" json:"ExecutePath"`
-	ExecuteArgs []string `yaml:"args" json:"ExecuteArgs,omitempty"`
-	Env         []string `yaml:"env"  json:"Env,omitempty"`
+	ExecutePath    string   `yaml:"path" json:"ExecutePath,omitempty"`
+	ExecuteCommand string   `yaml:"command" json:"ExecuteCommand,omitempty"`
+	Env            []string `yaml:"env"  json:"Env,omitempty"`
 }
 
 // TouchFile allows users to touch a file in the rootfs of an image
@@ -310,6 +310,46 @@ type DependentKeyError struct {
 	gojsonschema.ResultErrorFields
 }
 
+// ExclusiveKeyError implements gojsonschema.ErrorType.
+// It is used for custom errors for keys that cannot be
+// used with others
+type ExclusiveKeyError struct {
+	gojsonschema.ResultErrorFields
+}
+
+// NewExclusiveKeyError fails the image definition parsing when one
+// field depends on another being specified
+func NewExclusiveKeyError(context *gojsonschema.JsonContext, value interface{}, details gojsonschema.ErrorDetails) *ExclusiveKeyError {
+	err := ExclusiveKeyError{}
+	err.SetContext(context)
+	err.SetType("exclusive_key_error")
+	err.SetDescriptionFormat("Key {{.key1}} cannot be used with key {{.key2}}")
+	err.SetValue(value)
+	err.SetDetails(details)
+
+	return &err
+}
+
+// MissingCommandError implements gojsonschema.ErrorType.
+// It is used for custom errors for missing command or path
+// in Execute object
+type MissingCommandError struct {
+	gojsonschema.ResultErrorFields
+}
+
+// NewMissingCommandError fails the image definition parsing when
+// a Execute object does not have a path or a command defined
+func NewMissingCommandError(context *gojsonschema.JsonContext, value interface{}, details gojsonschema.ErrorDetails) *MissingCommandError {
+	err := MissingCommandError{}
+	err.SetContext(context)
+	err.SetType("missing_path_or_command_error")
+	err.SetDescriptionFormat("No path or command where defined in the manual execute entry")
+	err.SetValue(value)
+	err.SetDetails(details)
+
+	return &err
+}
+
 func (i ImageDefinition) securityMirror() string {
 	if i.Architecture == "amd64" || i.Architecture == "i386" {
 		return "http://security.ubuntu.com/ubuntu/"

internal/statemachine/classic.go

@@ -241,6 +241,7 @@ func validateCustomization(imageDefinition *imagedefinition.ImageDefinition, res
 		validateManualMakeDirs(imageDefinition, result, jsonContext)
 		validateManualCopyFile(imageDefinition, result, jsonContext)
 		validateManualTouchFile(imageDefinition, result, jsonContext)
+		validateManualExecute(imageDefinition, result, jsonContext)
 	}
 
 	return nil
@@ -297,6 +298,50 @@ func validateManualTouchFile(imageDefinition *imagedefinition.ImageDefinition, r
 	}
 }
 
+// validateManualExecute validates the Customization.Manual.Execute section of the image definition
+func validateManualExecute(imageDefinition *imagedefinition.ImageDefinition, result *gojsonschema.Result, jsonContext *gojsonschema.JsonContext) {
+	if imageDefinition.Customization.Manual.Execute == nil {
+		return
+	}
+
+	for _, execute := range imageDefinition.Customization.Manual.Execute {
+		if len(execute.ExecutePath) != 0 {
+			fmt.Print("WARNING: customization.manual.path was used. This option will soon be deprecated. Please use the \"command\" field.\n")
+		}
+
+		if len(execute.ExecutePath) != 0 && len(execute.ExecuteCommand) != 0 {
+			errDetail := gojsonschema.ErrorDetails{
+				"key1": "customization:manual:execute:execute-path",
+				"key2": "customization:manual:execute:execute-command",
+			}
+			result.AddError(
+				imagedefinition.NewExclusiveKeyError(
+					gojsonschema.NewJsonContext("exclusiveKeyError",
+						jsonContext),
+					52,
+					errDetail,
+				),
+				errDetail,
+			)
+		}
+
+		if len(execute.ExecutePath) == 0 && len(execute.ExecuteCommand) == 0 {
+			errDetail := gojsonschema.ErrorDetails{
+				"execute": "customization:manual:execute",
+			}
+			result.AddError(
+				imagedefinition.NewMissingCommandError(
+					gojsonschema.NewJsonContext("missingPathOrCommandError",
+						jsonContext),
+					52,
+					errDetail,
+				),
+				errDetail,
+			)
+		}
+	}
+}
+
 // validateAbsolutePath validates the
 func validateAbsolutePath(path string, errorKey string, result *gojsonschema.Result, jsonContext *gojsonschema.JsonContext) {
 	// XXX: filepath.IsAbs() does returns true for paths like ../../../something

internal/statemachine/helper.go

@@ -893,10 +893,12 @@ func manualCopyFile(customizations []*imagedefinition.CopyFile, confDefPath stri
 	return nil
 }
 
-// manualExecute executes executable files in the chroot
+// manualExecute executes commands or executables in the chroot
 func manualExecute(customizations []*imagedefinition.Execute, targetDir string, debug bool) error {
 	for _, c := range customizations {
-		executeCmd := execCommand("chroot", append([]string{targetDir, c.ExecutePath}, c.ExecuteArgs...)...)
+		args := toExecute(c)
+
+		executeCmd := execCommand("chroot", append([]string{targetDir}, args...)...)
 		if debug {
 			fmt.Printf("Executing command \"%s\"\n", executeCmd.String())
 		}
@@ -910,6 +912,20 @@ func manualExecute(customizations []*imagedefinition.Execute, targetDir string,
 	return nil
 }
 
+// toExecute returns the slice of commands/parameters to execute
+func toExecute(e *imagedefinition.Execute) []string {
+	// We already checked both cannot be set at the same time
+	if len(e.ExecutePath) != 0 {
+		return []string{e.ExecutePath}
+	}
+
+	if len(e.ExecuteCommand) != 0 {
+		return strings.Split(e.ExecuteCommand, " ")
+	}
+
+	return nil
+}
+
 // manualTouchFile touches files in the chroot
 func manualTouchFile(customizations []*imagedefinition.TouchFile, targetDir string, debug bool) error {
 	for _, c := range customizations {

internal/statemachine/helper_test.go

@@ -857,9 +857,8 @@ func TestStateMachine_manualExecute(t *testing.T) {
 			args: args{
 				customizations: []*imagedefinition.Execute{
 					{
-						ExecutePath: "/execute/path",
-						ExecuteArgs: []string{"arg1", "arg2"},
-						Env:         []string{"VAR1=value1", "VAR2=value2"},
+						ExecuteCommand: "/execute/path arg1 arg2",
+						Env:            []string{"VAR1=value1", "VAR2=value2"},
 					},
 				},
 				targetDir: "test",
@@ -876,28 +875,69 @@ func TestStateMachine_manualExecute(t *testing.T) {
 			},
 		},
 		{
-			name: "3 commands",
+			name: "3 commands with path",
 			args: args{
 				customizations: []*imagedefinition.Execute{
 					{
 						ExecutePath: "/execute/path1",
-						ExecuteArgs: []string{"arg1", "arg2"},
 						Env:         []string{"VAR1=value1", "VAR2=value2"},
 					},
 					{
 						ExecutePath: "/execute/path2",
-						ExecuteArgs: []string{"arg21", "arg22"},
 						Env:         []string{"VAR1=value21", "VAR2=value22"},
 					},
 					{
 						ExecutePath: "/execute/path3",
-						ExecuteArgs: []string{"arg31", "arg32"},
 						Env:         []string{"VAR1=value31", "VAR2=value32"},
 					},
 				},
 				targetDir: "test",
 				debug:     true,
 			},
+			expectedCmds: []expectedCmd{
+				{
+					cmd: "/usr/sbin/chroot test /execute/path1",
+					env: []string{
+						"VAR1=value1",
+						"VAR2=value2",
+					},
+				},
+				{
+					cmd: "/usr/sbin/chroot test /execute/path2",
+					env: []string{
+						"VAR1=value21",
+						"VAR2=value22",
+					},
+				},
+				{
+					cmd: "/usr/sbin/chroot test /execute/path3",
+					env: []string{
+						"VAR1=value31",
+						"VAR2=value32",
+					},
+				},
+			},
+		},
+		{
+			name: "3 commands with command",
+			args: args{
+				customizations: []*imagedefinition.Execute{
+					{
+						ExecuteCommand: "/execute/path1 arg1 arg2",
+						Env:            []string{"VAR1=value1", "VAR2=value2"},
+					},
+					{
+						ExecuteCommand: "/execute/path2 arg21 arg22",
+						Env:            []string{"VAR1=value21", "VAR2=value22"},
+					},
+					{
+						ExecuteCommand: "/execute/path3 arg31 arg32",
+						Env:            []string{"VAR1=value31", "VAR2=value32"},
+					},
+				},
+				targetDir: "test",
+				debug:     true,
+			},
 			expectedCmds: []expectedCmd{
 				{
 					cmd: "/usr/sbin/chroot test /execute/path1 arg1 arg2",

internal/statemachine/testdata/image_definitions/test_amd64.yaml

@@ -33,10 +33,7 @@ customization:
     touch-file:
       - path: /etc/touchfilefeature
     execute:
-      - path: /usr/bin/bash
-        args:
-          - "-c"
-          - "echo -n 'test' > $ENV_VAR_TEST"
+      - command: /usr/bin/bash -c "echo -n 'test' > $ENV_VAR_TEST"
         env: 
         - "ENV_VAR_TEST=/etc/executefeature"
     add-user: