Fix bash shell cleanup to support variable expansion in paths

What does this MR do?

Switch RmDir and RmFile methods to use CommandArgExpand instead of Command to enable proper shell variable expansion in file paths.

This problem was exposed in !5912 (merged) because the external Git config file was not cleaned up when an instance executor were used with the default build path, which is a relative path.

Previously, these methods used Command() which applies POSIX shell escaping ($'...'), preventing variable expansion. This caused issues when cleanup paths contained shell variables like $PWD ,as they would be treated as literal strings rather than being expanded to their actual values.

By switching to CommandArgExpand(), which uses double-quote escaping ("..."), shell variables in paths are now properly expanded during cleanup operations. The change affects:

  • RmDir: Both the chmod command (when FF_SET_PERMISSIONS_BEFORE_CLEANUP is enabled) and the rm command now support variable expansion.
  • RmFile: The rm command now supports variable expansion.

This ensures cleanup operations work correctly when paths are dynamically constructed using shell variables.

Why was this MR needed?

This is related to #39130 (closed). !5961 (merged) fixed the issue for shell executors, but it did not fix the issue for instance executors, which defaulted to a relative builds path.

What's the best way to test this MR?

  1. Set up an instance executor. Here is a sample TOML:
[session_server]
  session_timeout = 1800

[[runners]]
  name = "test-runner"
  url = "https://gitlab.example.com/"
  id = 46
  token = "REDACTED"
  token_obtained_at = 2022-12-15T08:08:46Z
  token_expires_at = 0001-01-01T00:00:00Z
  executor = 'instance'
  [runners.docker]
    tls_verify = false
    image                = "ubuntu:latest"
    helper_image_flavor  = "ubi-fips"
    privileged = true
    disable_entrypoint_overwrite = false
    oom_kill_disable = false
    disable_cache = false
    volumes = ["/cache"]
    shm_size = 0

  [runners.autoscaler]
  plugin = "aws"

  capacity_per_instance = 1
  max_use_count = 10
  max_instances = 2

    [runners.autoscaler.plugin_config] # plugin specific configuration (see plugin documentation)
    name             = "aws-autoscaling-group1"               
    profile          = "default"                     
    credentials_file = "/path/to/raws-creds"

    [runners.autoscaler.connector_config]
      username          = "ubuntu"
      protocol = "ssh"
      use_external_addr = true
      use_static_credentials = false

    [[runners.autoscaler.policy]]
      idle_count = 1
      idle_time = "20m0s"
  1. Run a CI job twice at least. It should succeed both times. Setting CI_DEBUG_TRACE: "true" will show that in the non-working case, the $PWD has been escaped:
++ rm -f '$PWD/builds/iK6vKq4R7/0/root/simple-ci.tmp/gitlab_runner_env'
++ rm -f '$PWD/builds/iK6vKq4R7/0/root/simple-ci.tmp/masking.db'
++ rm -f '$PWD/builds/iK6vKq4R7/0/root/simple-ci.tmp/.gitlab-runner.ext.conf'
++ rm -f '$PWD/builds/iK6vKq4R7/0/root/simple-ci.tmp/CI_SERVER_TLS_CA_FILE'
++ rm -f builds/iK6vKq4R7/0/root/simple-ci/.git/index.lock
++ rm -f builds/iK6vKq4R7/0/root/simple-ci/.git/shallow.lock
++ rm -f builds/iK6vKq4R7/0/root/simple-ci/.git/HEAD.lock
++ rm -f builds/iK6vKq4R7/0/root/simple-ci/.git/hooks/post-checkout
++ rm -f builds/iK6vKq4R7/0/root/simple-ci/.git/config.lock

Whereas in the working case:

++ rm -f /home/ubuntu/builds/iK6vKq4R7/0/root/simple-ci.tmp/gitlab_runner_env
++ rm -f /home/ubuntu/builds/iK6vKq4R7/0/root/simple-ci.tmp/masking.db
++ rm -f /home/ubuntu/builds/iK6vKq4R7/0/root/simple-ci.tmp/.gitlab-runner.ext.conf
++ rm -f /home/ubuntu/builds/iK6vKq4R7/0/root/simple-ci.tmp/CI_SERVER_TLS_CA_FILE
++ rm -f builds/iK6vKq4R7/0/root/simple-ci/.git/index.lock
++ rm -f builds/iK6vKq4R7/0/root/simple-ci/.git/shallow.lock
++ rm -f builds/iK6vKq4R7/0/root/simple-ci/.git/HEAD.lock
++ rm -f builds/iK6vKq4R7/0/root/simple-ci/.git/hooks/post-checkout
++ rm -f builds/iK6vKq4R7/0/root/simple-ci/.git/config.lock
  1. SSH into the instance executor:
aws ec2-instance-connect send-ssh-public-key --instance-id $INSTANCE_ID --instance-os-user ubuntu --ssh-public-key file://~/.ssh/id_rsa.pub

# Get public IP
aws ec2 describe-instances \
    --instance-ids $INSTANCE_ID \
    --query 'Reservations[0].Instances[0].PublicIpAddress' \
    --output text

Verify the files in the builds/*.tmp/ dir are really gone.

What are the relevant issue numbers?

Related to #39130 (closed)

Edited by Stan Hu

Merge request reports

Loading
Loading