Fix certificates chain generation regression introduced with 12.4.0-rc1

What does this MR do?

Fixes a regression introduced with !1581 (merged).

Why was this MR needed?

Usage of github.com/zakjan/cert-chain-resolver/certUtil added in !1581 (merged) allowed us to finally upgrade Runner from Go 1.8. But the implementation contains a bug, that in some cases may end with a panic that crashes the running Runner process:

panic: runtime error: index out of range
goroutine 87 [running]:
gitlab.com/gitlab-org/gitlab-runner/vendor/github.com/zakjan/cert-chain-resolver/certUtil.AddRootCA(0x0, 0x0, 0x0, 0x0, 0x181daa0, 0xc43a667aa0, 0x2, 0x4)
        /builds/gitlab-org/gitlab-runner/.gopath/src/gitlab.com/gitlab-org/gitlab-runner/vendor/github.com/zakjan/cert-chain-resolver/certUtil/chain.go:57 +0x2f8
gitlab.com/gitlab-org/gitlab-runner/network.(*client).getCAChain(0xc42009c480, 0xc439c61e40, 0x181ef80, 0x2254160)
        /builds/gitlab-org/gitlab-runner/.gopath/src/gitlab.com/gitlab-org/gitlab-runner/network/client.go:191 +0x33e
gitlab.com/gitlab-org/gitlab-runner/network.(*client).doJSON(0xc42009c480, 0xc4223e6280, 0xe, 0x16cb258, 0x3, 0xc8, 0x13e42c0, 0xc42b23f970, 0x0, 0x0, ...)
        /builds/gitlab-org/gitlab-runner/.gopath/src/gitlab.com/gitlab-org/gitlab-runner/network/client.go:320 +0x251
gitlab.com/gitlab-org/gitlab-runner/network.(*GitLabClient).doJSON(0xc4201cda80, 0x1841e20, 0xc428373d08, 0x16cb258, 0x3, 0xc4223e6280, 0xe, 0xc8, 0x13e42c0, 0xc42b23f970, ...)
        /builds/gitlab-org/gitlab-runner/.gopath/src/gitlab.com/gitlab-org/gitlab-runner/network/gitlab.go:168 +0x193
gitlab.com/gitlab-org/gitlab-runner/network.(*GitLabClient).UpdateJob(0xc4201cda80, 0xc4203d3760, 0x12, 0xc8, 0x0, 0xd, 0xc4203d37c0, 0x13, 0xc42042dd70, 0x1e, ...)
        /builds/gitlab-org/gitlab-runner/.gopath/src/gitlab.com/gitlab-org/gitlab-runner/network/gitlab.go:308 +0x2ae
gitlab.com/gitlab-org/gitlab-runner/network.(*clientJobTrace).sendUpdate(0xc42dd40b40, 0x0)
        /builds/gitlab-org/gitlab-runner/.gopath/src/gitlab.com/gitlab-org/gitlab-runner/network/trace.go:217 +0x191
gitlab.com/gitlab-org/gitlab-runner/network.(*clientJobTrace).finalStatusUpdate(0xc42dd40b40)
        /builds/gitlab-org/gitlab-runner/.gopath/src/gitlab.com/gitlab-org/gitlab-runner/network/trace.go:114 +0x2f
gitlab.com/gitlab-org/gitlab-runner/network.(*clientJobTrace).finish(0xc42dd40b40)
        /builds/gitlab-org/gitlab-runner/.gopath/src/gitlab.com/gitlab-org/gitlab-runner/network/trace.go:133 +0x6f
gitlab.com/gitlab-org/gitlab-runner/network.(*clientJobTrace).Fail(0xc42dd40b40, 0x0, 0x0, 0x0, 0x0)
        /builds/gitlab-org/gitlab-runner/.gopath/src/gitlab.com/gitlab-org/gitlab-runner/network/trace.go:56 +0xdc
gitlab.com/gitlab-org/gitlab-runner/network.(*clientJobTrace).Success(0xc42dd40b40)
        /builds/gitlab-org/gitlab-runner/.gopath/src/gitlab.com/gitlab-org/gitlab-runner/network/trace.go:38 +0x38
gitlab.com/gitlab-org/gitlab-runner/common.(*Build).setTraceStatus(0xc437420e00, 0x1849120, 0xc42dd40b40, 0x0, 0x0)
        /builds/gitlab-org/gitlab-runner/.gopath/src/gitlab.com/gitlab-org/gitlab-runner/common/build.go:485 +0x47c
gitlab.com/gitlab-org/gitlab-runner/common.(*Build).Run.func1(0xc437420e00, 0x1849120, 0xc42dd40b40, 0xc42b1aaa00, 0xc42b1aaa20)
        /builds/gitlab-org/gitlab-runner/.gopath/src/gitlab.com/gitlab-org/gitlab-runner/common/build.go:529 +0x55
gitlab.com/gitlab-org/gitlab-runner/common.(*Build).Run(0xc437420e00, 0xc420132790, 0x1849120, 0xc42dd40b40, 0xc4268a9ac0, 0x8)
        /builds/gitlab-org/gitlab-runner/.gopath/src/gitlab.com/gitlab-org/gitlab-runner/common/build.go:578 +0x713
gitlab.com/gitlab-org/gitlab-runner/commands.(*RunCommand).processRunner(0xc4200e6780, 0x18, 0xc42056a160, 0xc420087aa0, 0x0, 0x0)
        /builds/gitlab-org/gitlab-runner/.gopath/src/gitlab.com/gitlab-org/gitlab-runner/commands/multi.go:228 +0x5c7
gitlab.com/gitlab-org/gitlab-runner/commands.(*RunCommand).processRunners(0xc4200e6780, 0x18, 0xc42041a3c0, 0xc420087aa0)
        /builds/gitlab-org/gitlab-runner/.gopath/src/gitlab.com/gitlab-org/gitlab-runner/commands/multi.go:261 +0x24f
created by gitlab.com/gitlab-org/gitlab-runner/commands.(*RunCommand).startWorkers
        /builds/gitlab-org/gitlab-runner/.gopath/src/gitlab.com/gitlab-org/gitlab-runner/commands/multi.go:284 +0x84

This MR fixes this behavior. Additionally - because we needed to start checking errors from the former getCAChain() method, it adds a little refactoring which have a positive side effect - certificates fetching is done only when it's needed - on RequestJob() calls.

The MR contains also tests added for certificates chain building, that cover the regression failure.

Are there points in the code the reviewer needs to double check?

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?

changed milestone to %12.4

added [Deprecated] Category:Runner devopsverify grouprunner regression typebug labels

@steveazz Could you review this?

assigned to @steveazz

After merging this needs to be picked into 12-4-stable branch. I've created an extended title for this MR - Fix certificates chain generation regression introduced with 12.4.0-rc1 - so we will know what to remove from the final CHANGELOG for 12.4.0 (this is an "in RC" fix).

changed title from Fix certificates chain generation to Fix certificates chain generation regression introduced with 12.4.0-rc1

changed the description

mentioned in issue #4756 (closed)

added regression:11.4 label

approved this merge request

Nice work @tmaczukin the code looks good to me and it seems to work as expected. Especially for the scenario !1581 (merged) is trying to fix, which can be seen https://gitlab.com/steveazz/go-lang-tls-connection-state/-/jobs/325155105

merged

mentioned in commit e30ce19f

mentioned in commit 9b02d0ba

mentioned in issue gl-retrospectives/runner#2 (closed)

mentioned in issue #4805 (closed)

mentioned in issue #4890