From f5ba67de9e86de2cc104b7d39fc50dff72c04a58 Mon Sep 17 00:00:00 2001 From: Leonardo Di Giovanna Date: Fri, 22 Nov 2024 14:38:51 +0100 Subject: [PATCH 1/3] fix(decl/scripts): fix shell script SIGUSR1 handling Fix SIGUSR1 handling by resetting the associated exit code (138) after handler execution. Comment shell script. Signed-off-by: Leonardo Di Giovanna Co-authored-by: Aldo Lacuku --- pkg/test/script/shell/shell.go | 40 +++++++++++++++++++++++++++------- 1 file changed, 32 insertions(+), 8 deletions(-) diff --git a/pkg/test/script/shell/shell.go b/pkg/test/script/shell/shell.go index 796de40c..569d95b3 100644 --- a/pkg/test/script/shell/shell.go +++ b/pkg/test/script/shell/shell.go @@ -47,35 +47,59 @@ const ( // defines three placeholders: a placeholder for the "before" script, one for the signalingToken, and one for the // "after" script. scriptTemplate = ` +# __SLEEP_PID is the PID of the sleep process used to block the current script execution. __SLEEP_PID= + +# __cleanup is used as handler for SIGTERM and SIGINT signals, as well as handler for script EXIT pseudo-signal. __cleanup() { [ -n "$__SLEEP_PID" ] && kill -9 $__SLEEP_PID exit } +# __usr1 is used as handler for SIGUSR1. __usr1() { kill -9 $__SLEEP_PID && __SLEEP_PID= } +# Register signal handler to delete resources and exit upon SIGTERM and SIGINT or upon script EXIT. trap __cleanup TERM INT EXIT; -%s; + +# Launch "before" script. +%s + +# Launch sleep process and register SIGUSR1 signal handler. The signal handler is responsible to kill the sleep process +# to unblock the current process waiting for it to exit (see "wait $__SLEEP_PID"" below). sleep infinity & __SLEEP_PID=$! trap __usr1 USR1 + +# Send token to parent process to signal "before" script execution completion. echo %s -wait $PID; -%s; -` + +# Wait for sleep process to exit. +wait $__SLEEP_PID + +# The SIGUSR1 signal triggers __usr1 handler invocation and sets the exit code to 138. Existing from wait, we check if +# the exit code value is as expected and reset it to 0; otherwise, we return the obtained exit code. +if [ $? != 138 ]; then + echo "Wait on sleep unblocked due to a reason different from USR1 signal" >&2 + exit $? +else + true # Reset exit code +fi + +# Launch "after" script. +%s` ) // New creates a new shell script by merging beforeScript and afterScript. Since the "before" and "after" are part of // the same shell script, it is possible to reuse the declarations/definitions of the "before" part in the "after" part. func New(logger logr.Logger, beforeScript, afterScript *string) test.Script { var before, after string - if beforeScript != nil { - before = strings.TrimSpace(*beforeScript) + if beforeScript != nil && *beforeScript != "" { + before = strings.TrimSpace(*beforeScript) + ";" } - if afterScript != nil { - after = strings.TrimSpace(*afterScript) + if afterScript != nil && *afterScript != "" { + after = strings.TrimSpace(*afterScript) + ";" } script := fmt.Sprintf(scriptTemplate, before, signalingToken, after) s := &shellScript{ From 42caec140ccbdfaef68d0f4f7c63c8263c74b794 Mon Sep 17 00:00:00 2001 From: Leonardo Di Giovanna Date: Fri, 22 Nov 2024 14:41:14 +0100 Subject: [PATCH 2/3] fix(decl/loader): fix potential nil ptr deref in context validation Signed-off-by: Leonardo Di Giovanna Co-authored-by: Aldo Lacuku --- pkg/test/loader/loader.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkg/test/loader/loader.go b/pkg/test/loader/loader.go index 1fb924bc..dcfdc671 100644 --- a/pkg/test/loader/loader.go +++ b/pkg/test/loader/loader.go @@ -116,6 +116,10 @@ func (t *Test) validateNameUniqueness() error { // validateContext validates that names used for test resources and steps are unique. func (t *Test) validateContext() error { + if t.Context == nil { + return nil + } + processes := t.Context.Processes processesLen := len(processes) if processesLen <= 1 { From 6c557d3ccfd1675aa2d1b30a8472d20c8549bf0b Mon Sep 17 00:00:00 2001 From: Leonardo Di Giovanna Date: Mon, 25 Nov 2024 19:33:35 +0100 Subject: [PATCH 3/3] chore(decl/scripts): conditionally disable shell script execution Disable shell script execution if both "before" and "after" scripts are not provided. Signed-off-by: Leonardo Di Giovanna Co-authored-by: Aldo Lacuku --- pkg/test/script/shell/shell.go | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/pkg/test/script/shell/shell.go b/pkg/test/script/shell/shell.go index 569d95b3..fad27eb2 100644 --- a/pkg/test/script/shell/shell.go +++ b/pkg/test/script/shell/shell.go @@ -101,7 +101,10 @@ func New(logger logr.Logger, beforeScript, afterScript *string) test.Script { if afterScript != nil && *afterScript != "" { after = strings.TrimSpace(*afterScript) + ";" } - script := fmt.Sprintf(scriptTemplate, before, signalingToken, after) + var script string + if before != "" || after != "" { + script = fmt.Sprintf(scriptTemplate, before, signalingToken, after) + } s := &shellScript{ logger: logger, script: script, @@ -109,9 +112,16 @@ func New(logger logr.Logger, beforeScript, afterScript *string) test.Script { return s } +var noopAfterFunc = func(context.Context) error { return nil } + func (s *shellScript) RunBefore(ctx context.Context) (func(context.Context) error, error) { + script := s.script + if s.script == "" { + return noopAfterFunc, nil + } + processCtx, cancel := context.WithCancel(context.Background()) - cmd := exec.CommandContext(processCtx, "sh", "-c", s.script) //nolint:gosec // Disable G204 + cmd := exec.CommandContext(processCtx, "sh", "-c", script) //nolint:gosec // Disable G204 cmd.Cancel = func() error { return cmd.Process.Signal(unix.SIGTERM) }