ROX-8830: Respect local overwrites of variables in BASH_ENV

[vikin91]

This PR:

• Fixes variable shadowing problem (see description of the problem below)
• Adds regression tests for variable shadowing problem
• Changes CI step "Create commit in stackrox that updates the base image" to update the existing branch instead of overwriting it with force.

This is (a part of) the proper fix for the workaround introduced in stackrox/stackrox#180

Variable shadowing problem

This PR fixes the problem defined as follows:

$ cat /etc/bash.env
export FOO="bar"
export MAIN="3"

$ cat ~/test.sh
#!/usr/bin/env bash

echo "FOO: $FOO"
echo "MAIN: $MAIN"

$ export BASH_ENV=/etc/bash.env
$ ~/test.sh
FOO: bar
MAIN: 3

$ FOO=x ~/test.sh
FOO: bar  # <<< error, should be FOO=x
MAIN: 3

This faulty behavior results in broken master as described in ROX-8821

Fix

The fix is short and its intention should be clear when looking at the diff.

We modify cci-export in such a way, so that it respects the existing value of env variable. This changes the priority hierarchy from:

1. cci-export FOO bar (i.e., contents of $BASH_ENV)
2. export FOO=bar
3. FOO=x ./script.sh

Into:

1. FOO=x ./script.sh
2. export FOO=bar
3. cci-export FOO bar (i.e., contents of $BASH_ENV)

We obtain this by changing the contents of $BASH_ENV produced by

export CIRCLECI=true 
cci-export FOO bar

from:

export FOO=bar

into

export FOO="${FOO:-bar}"

however, there can be maximally one export VAR line for each VAR, because othwerwise, subsequent cci-export calls would not work as expected.

How tested

• Manually by replaying the scenario from this PR description
• In ROX-8830,RS-404: Update rox-ci-image to v0.3.22 stackrox#190 with label ci-upgrade-tests
• In unit-tests added in this PR
• In collector ROX-8830: Bump ci-image to test recent changes collector#537
• In scanner ROX-8830: Bump ci-image to test recent changes scanner#572

[msugakov]

From my limited understanding of the issue, the problem is that BASH_ENV contents may override whatever is set in the environment before bash process is launched.

1. Parent process exports environment variables.
2. Parent process starts real bash (through our wrappers but that doesn't matter).
3. bash sees BASH_ENV and sources that file.
4. BASH_ENV overwrites environment variables if their names match the ones set by the parent process.

If we assume, we don't want overwriting only for values that we put into $BASH_ENV with our cci-export function, could we simply modify this specific line from

echo "export ${key}=$(printf '%q' "$value")" >> "$BASH_ENV"

to

echo "export ${key}=\${${key}:-$(printf '%q' "$value")}" >> "$BASH_ENV"

Warning: this needs testing if escaping works as it should.

[vikin91]

This is correct, I added this in line 39.

This solution requires handling one edge-case: cci-exporting the same key twice with different values. In order to make this work properly, I needed to add line 35: remove_lines_starting_with "$BASH_ENV" "$(printf 'export %q="${%q:-' "$key" "$key")". There are unit tests in this PR to cover this edge-case.

[msugakov]

Good point. I did not even think about it. Let me take another look at the current approach.

[msugakov]

Will look into tests after we converge on the main change.

[vikin91]

Thanks for the comments, I addressed some of them so far, but I will ping you when I am ready with the rest.
I want to move now to RS-382 for an hour or two (as long as I have the topic fresh in my head).

[vikin91]

Okay, I think that #101 may take a bit longer and it is not crucial at all at the moment.
Okay to merge this now and move the priority back to the helm topic, @msugakov ?

I would then continue on #101 when we have less time pressure.

[viswajithiii]

Great work!

[msugakov]

I'm certainly ok to merge this without #101. I'm a bit 😿 about postponing #101 because I thought it would not be that difficult.

[vikin91]

Let's see how much we postpone :) Maybe only several hours :)