Fix destroy skipped on zero resource count when terraform state exists; reduce Run cyclomatic complexity

.gitignore

@@ -18,6 +18,7 @@ test/results/
 *.kubeconfig
 me
 out
+/step
 me.yml
 me.yaml
 tmp/

cmd/step/main.go

@@ -70,6 +70,7 @@ func main() {
 	flags.StringVarP(&step.Shell, "shell", "s", "/bin/sh", "The shell to execute the command in")
 	flags.StringVar(&step.FailureFile, "is-failure", "", "The path of the file used to indicate failure above")
 	flags.StringSliceVarP(&step.UploadFile, "upload", "u", []string{}, "Upload file as a kubernetes secret")
+	flags.StringSliceVar(&step.UploadOnErrorFile, "upload-on-error", []string{}, "Upload file as a kubernetes secret even when the command has failed")
 	flags.StringVar(&step.WaitFile, "wait-on", "", "The path to a file to indicate this step can be run")
 	flags.StringSliceVarP(&step.Commands, "command", "c", []string{}, "Command to execute")
 	flags.IntVar(&step.RetryAttempts, "retry-attempts", 0, "Number of times to retry the commands")
@@ -82,6 +83,11 @@ func main() {
 	}
 }
 
+// uploadRetryAttempts is the fixed number of retry attempts used for all secret upload
+// operations (both success-path and error-path). A small conservative value is used
+// intentionally: uploads are best-effort and should not block the primary operation.
+const uploadRetryAttempts = 2
+
 // calculateBackoff returns a duration that includes the minimum backoff plus a random jitter
 func calculateBackoff(minBackoff, maxJitter time.Duration) time.Duration {
 	if maxJitter <= 0 {
@@ -92,135 +98,162 @@ func calculateBackoff(minBackoff, maxJitter time.Duration) time.Duration {
 	return minBackoff + jitter
 }
 
-// Run is called to implement the action
-func Run(ctx context.Context, step Step) error {
-	if err := step.IsValid(); err != nil {
+// waitForSignal waits for a signal file to appear before allowing execution to proceed.
+// It returns an error if a failure file is found or the timeout expires.
+func waitForSignal(ctx context.Context, step Step) error {
+	log.WithFields(log.Fields{
+		"is-failure": step.FailureFile,
+		"on-error":   step.ErrorFile,
+		"on-wait":    step.WaitFile,
+		"timeout":    step.Timeout.String(),
+	}).Info("waiting for signal to execute")
+
+	return utils.RetryWithTimeout(ctx, step.Timeout, time.Second, func() (bool, error) {
+		if step.FailureFile != "" {
+			if found, _ := utils.FileExists(step.FailureFile); found {
+				return false, errors.New("found error signal file, refusing to execute")
+			}
+		}
+		if found, _ := utils.FileExists(step.WaitFile); found {
+			return true, nil
+		}
+
+		return false, nil
+	})
+}
+
+// runCommand executes a single attempt of a command and returns any error.
+func runCommand(ctx context.Context, step Step, index, attempt int, command string) error {
+	//nolint:gosec
+	cmd := exec.CommandContext(ctx, step.Shell, "-c", command)
+	cmd.Env = os.Environ()
+
+	logger := log.WithFields(log.Fields{
+		"command-index": index,
+		"attempt":       attempt,
+	})
+
+	stdout, err := cmd.StdoutPipe()
+	if err != nil {
+		logger.WithError(err).Error("failed to acquire stdout pipe on command")
+
 		return err
 	}
 
-	var cc client.Client
-	if len(step.UploadFile) > 0 {
-		ci, err := kubernetes.NewRuntimeClient(nil)
-		if err != nil {
-			return err
-		}
-		cc = ci
+	stderr, err := cmd.StderrPipe()
+	if err != nil {
+		logger.WithError(err).Error("failed to acquire stderr pipe on command")
+
+		return err
 	}
 
-	if step.Comment != "" {
-		fmt.Printf(`
-=======================================================
-%s
-=======================================================
-`, strings.ToUpper(step.Comment))
+	//nolint:errcheck
+	go io.Copy(os.Stdout, stdout)
+	//nolint:errcheck
+	go io.Copy(os.Stdout, stderr)
+
+	if err := cmd.Start(); err != nil {
+		logger.WithError(err).Error("failed to execute the command")
+
+		return err
 	}
 
-	if step.WaitFile != "" {
-		log.WithFields(log.Fields{
-			"is-failure": step.FailureFile,
-			"on-error":   step.ErrorFile,
-			"on-wait":    step.WaitFile,
-			"timeout":    step.Timeout.String(),
-		}).Info("waiting for signal to execute")
-
-		err := utils.RetryWithTimeout(ctx, step.Timeout, time.Second, func() (bool, error) {
-			if step.FailureFile != "" {
-				if found, _ := utils.FileExists(step.FailureFile); found {
-					return false, errors.New("found error signal file, refusing to execute")
-				}
-			}
-			if found, _ := utils.FileExists(step.WaitFile); found {
-				return true, nil
-			}
+	if err := cmd.Wait(); err != nil {
+		logger.WithError(err).Error("command execution failed")
 
-			return false, nil
-		})
-		if err != nil {
-			return err
-		}
+		return err
 	}
 
-	for i, command := range step.Commands {
-		attempt := 0
-		var lastErr error
-
-		for attempt <= step.RetryAttempts {
-			if attempt > 0 {
-				backoff := calculateBackoff(step.RetryMinBackoff, step.RetryMaxJitter)
-				log.WithFields(log.Fields{
-					"attempt":       attempt,
-					"command-index": i,
-					"backoff":       backoff,
-				}).Info("retrying command")
-
-				select {
-				case <-ctx.Done():
-					return ctx.Err()
-				case <-time.After(backoff):
-				}
-			}
+	return nil
+}
 
-			//nolint:gosec
-			cmd := exec.CommandContext(ctx, step.Shell, "-c", command)
-			cmd.Env = os.Environ()
+// runCommandWithRetries runs a command with retry logic, returning the number of
+// attempts made and any error after all retries are exhausted.
+func runCommandWithRetries(ctx context.Context, step Step, index int, command string) (int, error) {
+	attempt := 0
+	var lastErr error
 
-			logger := log.WithFields(log.Fields{
-				"command-index": i,
+	for attempt <= step.RetryAttempts {
+		if attempt > 0 {
+			backoff := calculateBackoff(step.RetryMinBackoff, step.RetryMaxJitter)
+			log.WithFields(log.Fields{
 				"attempt":       attempt,
-			})
-
-			stdout, err := cmd.StdoutPipe()
-			if err != nil {
-				logger.WithError(err).Error("failed to acquire stdout pipe on command")
-				return err
-			}
-			stderr, err := cmd.StderrPipe()
-			if err != nil {
-				logger.WithError(err).Error("failed to acquire stderr pipe on command")
-				return err
+				"command-index": index,
+				"backoff":       backoff,
+			}).Info("retrying command")
+
+			select {
+			case <-ctx.Done():
+				return attempt, ctx.Err()
+			case <-time.After(backoff):
 			}
+		}
 
-			//nolint:errcheck
-			go io.Copy(os.Stdout, stdout)
-			//nolint:errcheck
-			go io.Copy(os.Stdout, stderr)
+		if err := runCommand(ctx, step, index, attempt, command); err != nil {
+			lastErr = err
+			attempt++
 
-			if err := cmd.Start(); err != nil {
-				logger.WithError(err).Error("failed to execute the command")
-				lastErr = err
-				attempt++
-				continue
-			}
+			continue
+		}
 
-			// @step: wait for the command to finish
-			if err := cmd.Wait(); err != nil {
-				logger.WithError(err).Error("command execution failed")
-				lastErr = err
-				attempt++
-				continue
-			}
+		return attempt, nil
+	}
+
+	return attempt, lastErr
+}
+
+// handleCommandError processes a command failure: touches the error file if configured,
+// performs best-effort upload of any on-error files, and returns a wrapped error.
+func handleCommandError(ctx context.Context, step Step, cc client.Client, attempts int, lastErr error) error {
+	if step.ErrorFile != "" {
+		if err := utils.TouchFile(step.ErrorFile); err != nil {
+			log.WithError(err).WithField("file", step.ErrorFile).Error("failed to create error file")
 
-			// Command succeeded, break the retry loop
-			lastErr = nil
-			break
+			return err
 		}
+	}
 
-		// If we exhausted all retries and still have an error
-		if lastErr != nil {
-			if step.ErrorFile != "" {
-				if err := utils.TouchFile(step.ErrorFile); err != nil {
-					log.WithError(err).WithField("file", step.ErrorFile).Error("failed to create error file")
-					return err
-				}
+	// @step: attempt a best-effort upload of any files configured to upload on error
+	for name, path := range step.UploadOnErrorKeyPairs() {
+		if found, err := utils.FileExists(path); err != nil {
+			log.WithError(err).WithFields(log.Fields{
+				"path":   path,
+				"secret": name,
+			}).Warn("failed to check if upload-on-error file exists, skipping")
+
+			continue
+		} else if !found {
+			log.WithFields(log.Fields{
+				"path":   path,
+				"secret": name,
+			}).Warn("skipping upload-on-error as file does not exist")
+
+			continue
+		}
+
+		// @step: attempt a best-effort upload with a conservative fixed retry count rather than
+		// the command's RetryAttempts, since this is a best-effort operation that should not
+		// block error reporting.
+		if err := utils.Retry(ctx, uploadRetryAttempts, true, 5*time.Second, func() (bool, error) {
+			err := uploadSecret(ctx, cc, step.Namespace, name, path)
+			if err == nil {
+				return true, nil
 			}
-			return fmt.Errorf("command failed after %d attempts: %w", attempt, lastErr)
+			log.WithError(err).WithField("secret", name).Error("failed to upload secret on error")
+
+			return false, nil
+		}); err != nil {
+			log.WithError(err).WithField("secret", name).Error("failed to upload secret on error, continuing")
 		}
 	}
-	log.Info("successfully executed the step")
 
-	// @step: upload any files as kubernetes secrets
+	return fmt.Errorf("command failed after %d attempts: %w", attempts, lastErr)
+}
+
+// uploadSuccessFiles uploads all configured files as Kubernetes secrets after a successful run.
+func uploadSuccessFiles(ctx context.Context, step Step, cc client.Client) error {
 	for name, path := range step.UploadKeyPairs() {
-		err := utils.Retry(ctx, 2, true, 5*time.Second, func() (bool, error) {
+		err := utils.Retry(ctx, uploadRetryAttempts, true, 5*time.Second, func() (bool, error) {
 			err := uploadSecret(ctx, cc, step.Namespace, name, path)
 			if err == nil {
 				return true, nil
@@ -234,6 +267,51 @@ func Run(ctx context.Context, step Step) error {
 		}
 	}
 
+	return nil
+}
+
+// Run is called to implement the action
+func Run(ctx context.Context, step Step) error {
+	if err := step.IsValid(); err != nil {
+		return err
+	}
+
+	var cc client.Client
+	if len(step.UploadFile) > 0 || len(step.UploadOnErrorFile) > 0 {
+		ci, err := kubernetes.NewRuntimeClient(nil)
+		if err != nil {
+			return err
+		}
+		cc = ci
+	}
+
+	if step.Comment != "" {
+		fmt.Printf(`
+=======================================================
+%s
+=======================================================
+`, strings.ToUpper(step.Comment))
+	}
+
+	if step.WaitFile != "" {
+		if err := waitForSignal(ctx, step); err != nil {
+			return err
+		}
+	}
+
+	for i, command := range step.Commands {
+		attempts, err := runCommandWithRetries(ctx, step, i, command)
+		if err != nil {
+			return handleCommandError(ctx, step, cc, attempts, err)
+		}
+	}
+
+	log.Info("successfully executed the step")
+
+	if err := uploadSuccessFiles(ctx, step, cc); err != nil {
+		return err
+	}
+
 	// @step: everything was good - lets touch the file
 	if step.SuccessFile != "" {
 		if err := utils.TouchFile(step.SuccessFile); err != nil {

cmd/step/types.go

@@ -50,6 +50,8 @@ type Step struct {
 	Timeout time.Duration
 	// UploadFile is file to upload on success of the command
 	UploadFile []string
+	// UploadOnErrorFile is a list of files to upload even when the command has failed
+	UploadOnErrorFile []string
 	// WaitFile is the path to a file which is wait for to run
 	WaitFile string
 }
@@ -83,6 +85,16 @@ func (s Step) IsValid() error {
 		}
 	}
 
+	if len(s.UploadOnErrorFile) > 0 && s.Namespace == "" {
+		return errors.New("namespace must be specified when uploading files on error")
+	}
+
+	for _, x := range s.UploadOnErrorFile {
+		if e := strings.Split(x, "="); len(e) != 2 {
+			return fmt.Errorf("upload-on-error file %q must be in the format 'key=path'", x)
+		}
+	}
+
 	return nil
 }
 
@@ -101,3 +113,19 @@ func (s Step) UploadKeyPairs() map[string]string {
 
 	return keys
 }
+
+// UploadOnErrorKeyPairs returns a map of key pairs to upload even when the command has failed
+func (s Step) UploadOnErrorKeyPairs() map[string]string {
+	if len(s.UploadOnErrorFile) == 0 {
+		return nil
+	}
+
+	keys := make(map[string]string)
+	for _, x := range s.UploadOnErrorFile {
+		if e := strings.Split(x, "="); len(e) == 2 {
+			keys[e[0]] = e[1]
+		}
+	}
+
+	return keys
+}