From 3d239c5e4a620f14cf7b4882174e5a3dea856ed9 Mon Sep 17 00:00:00 2001 From: Raphael Philipe Mendes da Silva Date: Fri, 11 Aug 2023 10:20:18 -0700 Subject: [PATCH] Fix: don't overwrite .env file (#2261) * Fix: don't overwrite .env file Do not overwrite the .env file in case ctl commands are executed. This is specially important if extra environment variables are passed in the .env file Signed-off-by: Raphael Silva * Add tests for control script Signed-off-by: Raphael Silva * Fix typo in github workflow Signed-off-by: Raphael Silva * Fix shell linting errors Signed-off-by: Raphael Silva * Update .github/workflows/PR-build.yml * Make debian aligned with RPM Signed-off-by: Raphael Silva * properly install collector deb Signed-off-by: Raphael Silva * Use dpkg to install collector Signed-off-by: Raphael Silva * Properly use cache Signed-off-by: Raphael Silva * Move control script tests to the CI Signed-off-by: Raphael Silva * Add the pr-build tests back Signed-off-by: Raphael Silva --------- Signed-off-by: Raphael Silva Co-authored-by: bryan-aguilar <46550959+bryan-aguilar@users.noreply.github.com> --- .github/scripts/test-collector-ctl.sh | 49 +++++++++++++++++++++++ .github/workflows/CI.yml | 21 ++++++++++ .github/workflows/PR-build.yml | 28 +++++++++++++ tools/ctl/linux/aws-otel-collector-ctl.sh | 21 +++++++--- tools/packaging/debian/create_deb.sh | 4 +- 5 files changed, 116 insertions(+), 7 deletions(-) create mode 100755 .github/scripts/test-collector-ctl.sh diff --git a/.github/scripts/test-collector-ctl.sh b/.github/scripts/test-collector-ctl.sh new file mode 100755 index 000000000..f671b3dc2 --- /dev/null +++ b/.github/scripts/test-collector-ctl.sh @@ -0,0 +1,49 @@ +#!/bin/bash -x + +set -e + +# This script is supposed to run during the CI to validate that the control +# script is working as expected +# Important: this script requires root permission. + +COLLECTOR_DEB=$1 +COLLECTOR_CONFIG=$2 + +ADOT_CTL=/opt/aws/aws-otel-collector/bin/aws-otel-collector-ctl +ENV_FILE="/opt/aws/aws-otel-collector/etc/.env" +CONFIG_FILE="/opt/aws/aws-otel-collector/etc/config.yaml" + +setup() { + dpkg -i "$COLLECTOR_DEB" +} + +test_collector_ctl_does_not_overwrite_env() { + $ADOT_CTL -a start + if [ ! -e $CONFIG_FILE ]; then + echo "Config file does not exist" + exit 1 + fi + + echo "EXTRA_ENV=\"some value\"" | tee -a $ENV_FILE > /dev/null + + # We don't need the collector to succeed, just that the control script is execised + $ADOT_CTL -a start -c "http://example.com" + + if ! grep -q EXTRA_ENV "$ENV_FILE"; then + echo "Env file was overwritten" + exit 1 + fi + + $ADOT_CTL -a start -c "$COLLECTOR_CONFIG" + if ! grep -q EXTRA_ENV "$ENV_FILE"; then + ehco "Env file was overwritten" + exit 1 + fi + echo "${FUNCNAME[0]} ... OK" +} + + +setup + +## Tests +test_collector_ctl_does_not_overwrite_env diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml index 0c93f1f9c..c384a1190 100644 --- a/.github/workflows/CI.yml +++ b/.github/workflows/CI.yml @@ -379,6 +379,27 @@ jobs: run: | gpg --fingerprint --with-colons aws-otel-collector@amazon.com grep "^fpr" | sed -n 's/^fpr:::::::::\([[:alnum:]]\+\):/\1/p' | xargs gpg --batch --yes --delete-secret-keys gpg --list-secret-keys + + test-control-stript: + runs-on: ubuntu-latest + needs: [packaging-deb] + steps: + - name: Checkout + if: ${{ needs.changes.outputs.changed == 'true' }} + uses: actions/checkout@v3 + + - name: Restore cached Debs + if: ${{ needs.changes.outputs.changed == 'true' }} + uses: actions/cache@v3 + with: + key: "cached_debs_${{ github.run_id }}" + path: ${{ env.PACKAGING_ROOT }} + + - name: Execute tests + if: ${{ needs.changes.outputs.changed == 'true' }} + run: | + sudo ./.github/scripts/test-collector-ctl.sh $PACKAGING_ROOT/debian/amd64/aws-otel-collector.deb ./config.yaml + packaging-ssm: runs-on: ubuntu-latest needs: [packaging-rpm, packaging-deb, packaging-msi] diff --git a/.github/workflows/PR-build.yml b/.github/workflows/PR-build.yml index 2fdff0bce..5273a1785 100644 --- a/.github/workflows/PR-build.yml +++ b/.github/workflows/PR-build.yml @@ -247,12 +247,40 @@ jobs: key: "cached_binaries_${{ github.run_id }}" path: build + - name: Cache built debian + if: ${{ needs.changes.outputs.changed == 'true' }} + uses: actions/cache@v3 + with: + key: "cached_deb_${{ github.run_id }}" + path: build/packages/debian + - name: Build Debs if: ${{ needs.changes.outputs.changed == 'true' }} run: | ARCH=amd64 DEST=build/packages/debian/amd64 tools/packaging/debian/create_deb.sh ARCH=arm64 DEST=build/packages/debian/arm64 tools/packaging/debian/create_deb.sh + test-control-stript: + runs-on: ubuntu-latest + needs: [changes, packaging-deb] + steps: + - name: Checkout + if: ${{ needs.changes.outputs.changed == 'true' }} + uses: actions/checkout@v3 + + - name: Restore cached deb + if: ${{ needs.changes.outputs.changed == 'true' }} + uses: actions/cache@v3 + with: + key: "cached_deb_${{ github.run_id }}" + path: build/packages/debian + + - name: Execute tests + if: ${{ needs.changes.outputs.changed == 'true' }} + run: | + file ./build/packages/debian/amd64/aws-otel-collector.deb + sudo ./.github/scripts/test-collector-ctl.sh ./build/packages/debian/amd64/aws-otel-collector.deb ./config.yaml + get-test-cases: runs-on: ubuntu-latest outputs: diff --git a/tools/ctl/linux/aws-otel-collector-ctl.sh b/tools/ctl/linux/aws-otel-collector-ctl.sh index 061d165a5..ad90b26bb 100644 --- a/tools/ctl/linux/aws-otel-collector-ctl.sh +++ b/tools/ctl/linux/aws-otel-collector-ctl.sh @@ -53,7 +53,7 @@ UsageString=" aoc_config_remote_uri() { config="${1:-}" - echo "config=\"--config ${config}\"" > $ENV_FILE + sed -i 's#^config=.*$#config="--config '"${config}"'"#' $ENV_FILE } @@ -63,12 +63,18 @@ aoc_config_local_uri() { # Strip the file scheme in case it is present config="${config#file:}" - echo "config=\"--config /opt/aws/aws-otel-collector/etc/config.yaml\"" > $ENV_FILE + sed -i 's#^config=.*$#config="--config /opt/aws/aws-otel-collector/etc/config.yaml"#' $ENV_FILE if [ -n "$config" ] && [ -f "$config" ]; then cp "$config" $CONFDIR/config.yaml + else + echo "File $config does not exist" + exit 1 fi +} +# Used in case the collector starts for the first time without a configuration parameter +aoc_ensure_default_config() { if [ ! -f $CONFDIR/config.yaml ]; then cp $DFT_CONFDIR/.config.yaml $CONFDIR/config.yaml fi @@ -87,10 +93,15 @@ is_remote_uri() { aoc_start() { config="${1:-}" - if is_remote_uri "$config"; then - aoc_config_remote_uri "$config" + # The previous configuration should be used if no configuration parameter is passed + if [ -z "$config" ]; then + aoc_ensure_default_config else - aoc_config_local_uri "$config" + if is_remote_uri "$config"; then + aoc_config_remote_uri "$config" + else + aoc_config_local_uri "$config" + fi fi if [ "${SYSTEMD}" = 'true' ]; then diff --git a/tools/packaging/debian/create_deb.sh b/tools/packaging/debian/create_deb.sh index 326afcec2..60b4bb221 100755 --- a/tools/packaging/debian/create_deb.sh +++ b/tools/packaging/debian/create_deb.sh @@ -46,7 +46,8 @@ cp LICENSE "${AOC_ROOT}/opt/aws/aws-otel-collector/" cp VERSION "${AOC_ROOT}/opt/aws/aws-otel-collector/bin/" cp "build/linux/${ARCH}/aoc" "${AOC_ROOT}/opt/aws/aws-otel-collector/bin/aws-otel-collector" cp tools/ctl/linux/aws-otel-collector-ctl.sh "${AOC_ROOT}/opt/aws/aws-otel-collector/bin/aws-otel-collector-ctl" -cp config.yaml "${AOC_ROOT}/opt/aws/aws-otel-collector/etc" +# default configuration +cp config.yaml "${AOC_ROOT}/opt/aws/aws-otel-collector/var/.config.yaml" cp .env "${AOC_ROOT}/opt/aws/aws-otel-collector/etc" cp extracfg.txt "${AOC_ROOT}/opt/aws/aws-otel-collector/etc" cp tools/packaging/linux/aws-otel-collector.service "${AOC_ROOT}/etc/systemd/system/" @@ -54,7 +55,6 @@ cp tools/packaging/linux/aws-otel-collector.conf "${AOC_ROOT}/etc/init/" chmod ug+rx "${AOC_ROOT}/opt/aws/aws-otel-collector/bin/aws-otel-collector" chmod ug+rx "${AOC_ROOT}/opt/aws/aws-otel-collector/bin/aws-otel-collector-ctl" -chmod ug+rx "${AOC_ROOT}/opt/aws/aws-otel-collector/etc/config.yaml" chmod ug+rx "${AOC_ROOT}/opt/aws/aws-otel-collector/etc/.env" chmod ug+rx "${AOC_ROOT}/opt/aws/aws-otel-collector/etc/extracfg.txt"