From ff6dc5c9ea91adab9a5767599234298a36e75f46 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Fri, 5 Apr 2024 17:58:00 +0200 Subject: [PATCH] WIP replace path and args with command field in Execute customization Signed-off-by: Paul Mars --- internal/imagedefinition/README.rst | 12 ++--- internal/imagedefinition/image_definition.go | 46 ++++++++++++++-- internal/statemachine/classic.go | 45 ++++++++++++++++ internal/statemachine/helper.go | 20 ++++++- internal/statemachine/helper_test.go | 54 ++++++++++++++++--- .../image_definitions/test_amd64.yaml | 5 +- 6 files changed, 160 insertions(+), 22 deletions(-) diff --git a/internal/imagedefinition/README.rst b/internal/imagedefinition/README.rst index b6b9c388..91e4207f 100644 --- a/internal/imagedefinition/README.rst +++ b/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: - # 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: - # Arguments to give to the command - args: (optional) - - - - + # Full command to execute, including arguments + command: # Environment variables to set before executing the command # Format: ENV=VALUE env: (optional) diff --git a/internal/imagedefinition/image_definition.go b/internal/imagedefinition/image_definition.go index 22984024..eef54a00 100644 --- a/internal/imagedefinition/image_definition.go +++ b/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/" diff --git a/internal/statemachine/classic.go b/internal/statemachine/classic.go index 27b70229..82fb97c8 100644 --- a/internal/statemachine/classic.go +++ b/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 diff --git a/internal/statemachine/helper.go b/internal/statemachine/helper.go index 4a26f6ea..e6ecc947 100644 --- a/internal/statemachine/helper.go +++ b/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 { diff --git a/internal/statemachine/helper_test.go b/internal/statemachine/helper_test.go index 32050d87..8d665ec8 100644 --- a/internal/statemachine/helper_test.go +++ b/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", diff --git a/internal/statemachine/testdata/image_definitions/test_amd64.yaml b/internal/statemachine/testdata/image_definitions/test_amd64.yaml index 2b8ec327..8afe2358 100644 --- a/internal/statemachine/testdata/image_definitions/test_amd64.yaml +++ b/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: