Prevent Executors from modifying Runner configuration
What does this MR do?
Prevent the executors from modifying the Runner configuration by performing a deep-copy on the Runner config when passing it into the Executor create/prepare.
Why was this MR needed?
Executors, specifically the Docker-Machine one, modify the Config object. This can lead to credentials changing under the hood causing strange side effects like this one.
Are there points in the code the reviewer needs to double check?
- I added a test to prove that this occurs, you can see the result in the first commit's pipeline
- I can't think of a way to cause JSON marshal and unmarshal to fail, so I don't have any tests for those failures.
Does this MR meet the acceptance criteria?
-
Documentation created/updated -
Added tests for this feature/bug -
In case of conflicts with master- branch was rebased
What are the relevant issue numbers?
Benchmark results
Check !1134 (comment 130360628) for detail.
BenchmarkBuildRunCanModifyConfig 1000 2016537 ns/op 1514351 B/op 3664 allocs/op
BenchmarkBuildRunModifyConfig 1000 1774828 ns/op 1518470 B/op 3688 allocs/opUX
When we fail deep copying the config the user will see the following in the job log and will also see specific logs inside of the runner logs.
Edited by Steve Xuereb