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 (whenFF_SET_PERMISSIONS_BEFORE_CLEANUPis 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?
- 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"
- 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$PWDhas 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
- 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)