ROX-8830: Respect local overwrites of variables in BASH_ENV

.circleci/config.yml

@@ -51,7 +51,7 @@ jobs:
     <<: *defaults
     steps:
       - build-and-push-image:
-          dockerfile-path: images/Dockerfile.rox
+          dockerfile-path: images/rox.Dockerfile
 
   create-or-update-dependent-rox-pr:
     <<: *defaults
@@ -80,24 +80,43 @@ jobs:
           git config user.name "RoxBot"
 
           branch_name="roxbot/update-ci-image-from-${CIRCLE_PULL_REQUEST##*/}"
-          git checkout "${branch_name}" || git checkout -b "${branch_name}"
-          git reset --hard origin/HEAD
+          if git fetch --quiet origin "${branch_name}"; then
+            git checkout "${branch_name}"
+            git pull --quiet --set-upstream origin "${branch_name}"
+          else
+            git checkout -b "${branch_name}"
+            git push --set-upstream origin "${branch_name}"
+          fi
 
           sed -i '[email protected]/rhacs-eng/apollo-ci:.*@'"${pushed_image}"' # TODO(do not merge): After upstream PR is merged, cut a tag and update this@g' .circleci/config.yml
           git commit -am "Bump base image to ${pushed_image##:}"
-          git push origin "${branch_name}" --force
+          git push origin "${branch_name}"
 
     - run:
         name: Create PR
         command: |
           branch_name="roxbot/update-ci-image-from-${CIRCLE_PULL_REQUEST##*/}"
           .circleci/create_update_pr_rox.sh "${branch_name}"
 
+  run-test-cci-export:
+    <<: *defaults
+    resource_class: medium
+    steps:
+    - checkout
+    - setup_remote_docker
+    - run:
+        name: Test cci-export inside Docker
+        command: |
+          export TAG="$(git describe --tags --abbrev=10)"
+          docker build images/ -f images/test.cci-export.Dockerfile \
+            -t "quay.io/rhacs-eng/apollo-ci:test-cci-export-$TAG"
+          docker run "quay.io/rhacs-eng/apollo-ci:test-cci-export-${TAG}"
+
   build-and-push-env-check:
     <<: *defaults
     steps:
       - build-and-push-image:
-          dockerfile-path: images/Dockerfile.env-check
+          dockerfile-path: images/env-check.Dockerfile
           tag-prefix: "env-check-"
 
   unit-test-env-check:
@@ -171,25 +190,31 @@ jobs:
     <<: *defaults
     steps:
       - build-and-push-image:
-          dockerfile-path: images/Dockerfile.collector
+          dockerfile-path: images/collector.Dockerfile
           tag-prefix: "collector-"
 
   build-and-push-jenkins-plugin:
     <<: *defaults
     steps:
       - build-and-push-image:
-          dockerfile-path: images/Dockerfile.jenkins-plugin
+          dockerfile-path: images/jenkins-plugin.Dockerfile
           tag-prefix: "jenkins-plugin-"
 
 workflows:
   version: 2
   build:
     jobs:
+    - run-test-cci-export:
+        filters:
+          tags:
+            only: /.*/
     - build-and-push-rox:
         context: quay-rhacs-eng-readwrite
         filters:
           tags:
             only: /.*/
+        requires:
+        - run-test-cci-export
     - create-or-update-dependent-rox-pr:
         filters:
           branches:

images/static-contents/bin/bash-wrapper

@@ -4,23 +4,36 @@
 # cci-export is a function which can be used to export environment variables in a way that is persistent
 # across CircleCI steps.
 cci-export() {
-	if [ "$#" -ne 2 ]; then
-		echo >&2 "Usage: $0 KEY VALUE"
-		return 1
-	fi
-
-	key="$1"
-	value="$2"
-
-	export "${key}=${value}"
-
-	if [[ "$CIRCLECI" == "true" ]]; then
-		if [[ -z "${BASH_ENV}" ]]; then
-			echo >&2 "Env var BASH_ENV not properly set"
-			return 1
-		fi
-		echo "export ${key}=$(printf '%q' "$value")" >> "$BASH_ENV"
-	fi
+  if [ "$#" -ne 2 ]; then
+    echo >&2 "Usage: $0 KEY VALUE"
+    return 1
+  fi
+
+  key="$1"
+  value="$2"
+
+  export "${key}=${value}"
+
+  if [[ "$CIRCLECI" == "true" ]]; then
+    if [[ -z "${BASH_ENV}" ]]; then
+      echo >&2 "Env var BASH_ENV not properly set"
+      return 1
+    fi
+
+    # Use export with default value: 'export FOO="${FOO:-bar}"', so that variables set in the environment are not overwritten by `$BASH_ENV`
+    key_part="export ${key}"
+    # shellcheck disable=SC2016 # we must produce literal ${} symbols
+    value_part="$(printf '"${%q:-"%q"}"\n' "$key" "$value")"
+
+    # Remove all export-lines for the same exported variable, to 'forget' about past cci-export calls,
+    # otherwise the first call to cci-export would define a default value for the variable, so that
+    # second and subsequent calls to cci-export would have no effect.
+    if [[ -f "$BASH_ENV" ]]; then
+      filtered_envfile="$(mktemp -t "bash.env-XXXX")"
+      grep --invert-match --fixed-strings "${key_part}=" "$BASH_ENV" > "${filtered_envfile}" && mv "${filtered_envfile}" "$BASH_ENV"
+    fi
+    echo "${key_part}=${value_part}" >> "$BASH_ENV"
+  fi
 }
 
 export -f cci-export

images/test.cci-export.Dockerfile

@@ -0,0 +1,63 @@
+FROM ubuntu:20.04 as base
+ARG DEBIAN_FRONTEND=noninteractive
+SHELL ["/bin/bash", "-o", "pipefail", "-c"]
+
+# Configure all necessary apt repositories
+RUN set -ex \
+  && apt-get update \
+  && apt-get install --no-install-recommends -y \
+      apt-transport-https \
+      ca-certificates \
+      gnupg2 \
+      wget \
+      git \
+      sudo \
+      nodejs \
+  && wget --no-verbose -O - https://deb.nodesource.com/setup_lts.x | bash - \
+  && wget --no-verbose -O - https://dl.yarnpkg.com/debian/pubkey.gpg | apt-key add - \
+  && echo "deb https://dl.yarnpkg.com/debian/ stable main" | tee /etc/apt/sources.list.d/yarn.list \
+  && apt-get remove -y \
+      apt-transport-https \
+      gnupg2 \
+      wget \
+  && apt-get autoremove -y \
+  && rm -rf /var/lib/apt/lists/*
+
+RUN set -ex \
+ && apt-get update \
+ && apt-get install --no-install-recommends -y \
+      nodejs \
+ && rm -rf /var/lib/apt/lists/*
+
+RUN set -ex \
+  && npm install -g [email protected] [email protected] [email protected] \
+  && bats -v
+
+RUN set -ex \
+ && groupadd --gid 3434 circleci \
+ && useradd --uid 3434 --gid circleci --shell /bin/bash --create-home circleci \
+ && echo 'circleci ALL=NOPASSWD: ALL' > /etc/sudoers.d/50-circleci
+
+# Function-under-test setup
+FROM base as image_under_test
+
+COPY ./static-contents/ /static-tmp
+RUN set -e \
+  && for file in $(find /static-tmp -type f); do \
+    dir="$(dirname "${file}")"; new_dir="${dir#/static-tmp}"; mkdir -p "${new_dir}"; cp "${file}" "${new_dir}"; \
+  done \
+  && rm -r /static-tmp
+
+RUN \
+	mv /bin/bash /bin/real-bash && \
+	mv /bin/bash-wrapper /bin/bash
+
+# Test setup
+FROM image_under_test as tester
+
+USER circleci
+WORKDIR /home/circleci/test
+COPY --chown=circleci:circleci test/ .
+ENV CIRCLECI=true
+
+CMD ["bats", "--print-output-on-failure", "--verbose-run", "/home/circleci/test/bats/"]

images/test/bats/FILE

@@ -0,0 +1 @@
+1.2.3

images/test/bats/cci-export.bats

@@ -0,0 +1,140 @@
+#!/usr/bin/env bats
+
+# To run the test locally do:
+# docker build -t apollo-cci:test -f images/test.cci-export.Dockerfile images && docker run -it apollo-cci:test
+
+bats_helpers_root="/usr/lib/node_modules"
+load "${bats_helpers_root}/bats-support/load.bash"
+load "${bats_helpers_root}/bats-assert/load.bash"
+
+setup() {
+  bash_env="$(mktemp)"
+  export BASH_ENV="$bash_env"
+  # ensure clean start of every test case
+  unset FOO
+  echo "" > "$bash_env"
+  run echo $BASH_ENV
+  assert_output "$bash_env"
+  run cat $BASH_ENV
+  assert_output ""
+  run "$HOME/test/foo-printer.sh"
+  assert_output "FOO: "
+}
+
+@test "cci-export BASH_ENV does not exist" {
+  run rm -f "${BASH_ENV}"
+  run test -f "${BASH_ENV}"
+  assert_failure
+
+  run cci-export FOO cci1
+  assert_success
+  run "$HOME/test/foo-printer.sh"
+  assert_output "FOO: cci1"
+  refute_output "FOO: "
+}
+
+@test "cci-export sanity check single value" {
+  run cci-export FOO cci1
+  assert_success
+  run "$HOME/test/foo-printer.sh"
+  assert_output "FOO: cci1"
+  refute_output "FOO: "
+
+  run cci-export FOO cci2
+  assert_success
+  run "$HOME/test/foo-printer.sh"
+  assert_output "FOO: cci2"
+  refute_output "FOO: cci1"
+}
+
+@test "cci-export should escape special characters in values" {
+  run cci-export FOO 'quay.io/rhacs-"eng"/super scanner:2.21.0-15-{{g44}(8f)2dc8fa}'
+  assert_success
+  run "$HOME/test/foo-printer.sh"
+  assert_output 'FOO: quay.io/rhacs-"eng"/super scanner:2.21.0-15-{{g44}(8f)2dc8fa}'
+  refute_output "FOO: "
+}
+
+@test "cci-export sanity check many values" {
+  export _FILE="$HOME/test/bats/FILE"
+  run cat "${_FILE}"
+  assert_output "1.2.3"
+
+  export VAR=placeholder
+  run cci-export VAR1 "text/$VAR/text:$(cat "${_FILE}")"
+  run cci-export VAR2 "text/$VAR/text:$(cat "${_FILE}")"
+  run cci-export IMAGE3 "text/$VAR/text:$(cat "${_FILE}")"
+
+  run "$HOME/test/foo-printer.sh" "VAR1"
+  assert_output "VAR1: text/$VAR/text:$(cat "${_FILE}")"
+  assert_output "VAR1: text/placeholder/text:1.2.3"
+
+  run "$HOME/test/foo-printer.sh" VAR2
+  assert_output "VAR2: text/$VAR/text:$(cat "${_FILE}")"
+  assert_output "VAR2: text/placeholder/text:1.2.3"
+
+  run "$HOME/test/foo-printer.sh" IMAGE3
+  assert_output "IMAGE3: text/$VAR/text:$(cat "${_FILE}")"
+  assert_output "IMAGE3: text/placeholder/text:1.2.3"
+}
+
+@test "cci-export potentially colliding variable names" {
+  run cci-export PART1 "value1"
+  run cci-export PART1_PART2 "value_joined"
+  run cci-export PART1 "value2"
+
+  run "$HOME/test/foo-printer.sh" PART1
+  assert_output "PART1: value2"
+  refute_output "PART1: value1"
+  run "$HOME/test/foo-printer.sh" PART1_PART2
+  assert_output "PART1_PART2: value_joined"
+}
+
+@test "exported variable should be respected in a script" {
+  export FOO=bar
+  run "$HOME/test/foo-printer.sh"
+  assert_output "FOO: bar"
+  refute_output "FOO: "
+}
+
+@test "shadowed variable should be respected in a script" {
+  FOO=bar run "$HOME/test/foo-printer.sh"
+  assert_output "FOO: bar"
+  refute_output "FOO: "
+}
+
+@test "exported variable should have priority over the cci-exported one" {
+  run cci-export FOO cci
+  export FOO=bar
+  run "$HOME/test/foo-printer.sh"
+  assert_output "FOO: bar"
+  refute_output "FOO: cci"
+  refute_output "FOO: "
+}
+
+@test "shadowed variable should have priority over the cci-exported one" {
+  run cci-export FOO cci
+  FOO=bar run "$HOME/test/foo-printer.sh"
+  assert_output "FOO: bar"
+  refute_output "FOO: cci"
+  refute_output "FOO: "
+}
+
+@test "shadowed variable should have priority over both: the exported and the cci-exported one" {
+  export FOO=bar-export
+  run cci-export FOO cci
+  FOO=bar-shadow run "$HOME/test/foo-printer.sh"
+  assert_output "FOO: bar-shadow"
+  refute_output "FOO: bar-export"
+  refute_output "FOO: cci"
+  refute_output "FOO: "
+
+
+  run cci-export FOO cci2
+  export FOO=bar-export2
+  FOO=bar-shadow2 run "$HOME/test/foo-printer.sh"
+  assert_output "FOO: bar-shadow2"
+  refute_output "FOO: bar-export2"
+  refute_output "FOO: cci2"
+  refute_output "FOO: "
+}

images/test/foo-printer.sh

@@ -0,0 +1,7 @@
+#!/usr/bin/env bash
+
+if [[ -n "$1" ]]; then
+  echo "$1: ${!1}"
+else
+  echo "FOO: $FOO"
+fi