Fix: don't overwrite .env file #2261

rapphil · 2023-08-09T17:15:41Z

Description: 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

Link to tracking Issue: #2243

Testing: Manual

(23-08-09 17:05:54) <130> [~/workplace/aws-otel-collector]
 % echo 'EXTRA_ENV2="foo"' | sudo tee -a  /opt/aws/aws-otel-collector/etc/.env
EXTRA_ENV2="foo"

(23-08-09 17:06:03) <0> [~/workplace/aws-otel-collector]
 % sudo cat /opt/aws/aws-otel-collector/etc/.env
config="--config http://example123.com"
EXTRA_ENV="foo"
EXTRA_ENV2="foo"

(23-08-09 17:06:07) <0> [~/workplace/aws-otel-collector]
 % sudo /opt/aws/aws-otel-collector/bin/aws-otel-collector-ctl -a start -c http://example.com
Redirecting to /bin/systemctl restart aws-otel-collector.service

(23-08-09 17:06:16) <0> [~/workplace/aws-otel-collector]
 % sudo cat /opt/aws/aws-otel-collector/etc/.env
config="--config http://example.com"
EXTRA_ENV="foo"
EXTRA_ENV2="foo"

(23-08-09 17:06:18) <0> [~/workplace/aws-otel-collector]
 % sudo /opt/aws/aws-otel-collector/bin/aws-otel-collector-ctl -a start -c /opt/aws/aws-otel-collector/var/.config.yaml
Redirecting to /bin/systemctl restart aws-otel-collector.service

(23-08-09 17:06:25) <0> [~/workplace/aws-otel-collector]
 % sudo cat /opt/aws/aws-otel-collector/etc/.env
config="--config /opt/aws/aws-otel-collector/etc/config.yaml"
EXTRA_ENV="foo"
EXTRA_ENV2="foo"

New tests were also added to validate the regression. I'm adding to both the CI and PR build since this test is very cheap to execute and does not require any special permissions.

Documentation:

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

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 <rapphil@gmail.com>

Signed-off-by: Raphael Silva <rapphil@gmail.com>

rapphil · 2023-08-11T00:07:54Z

.github/workflows/PR-build.yml

@@ -253,6 +253,27 @@ jobs:
          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:


Using PR-build for now to test that this works.

I decided to leave this permanently since these tests are very cheap to execute.

Signed-off-by: Raphael Silva <rapphil@gmail.com>

.github/workflows/PR-build.yml

Signed-off-by: Raphael Silva <rapphil@gmail.com>

rapphil · 2023-08-11T04:24:28Z

tools/packaging/debian/create_deb.sh

@@ -46,15 +46,15 @@ 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


adding this to this pr since this is a one liner.

This change will make the .deb file in parity with the RPM file and how the control script works.

Signed-off-by: Raphael Silva <rapphil@gmail.com>

* 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 <rapphil@gmail.com> * Add tests for control script Signed-off-by: Raphael Silva <rapphil@gmail.com> * Fix typo in github workflow Signed-off-by: Raphael Silva <rapphil@gmail.com> * Fix shell linting errors Signed-off-by: Raphael Silva <rapphil@gmail.com> * Update .github/workflows/PR-build.yml * Make debian aligned with RPM Signed-off-by: Raphael Silva <rapphil@gmail.com> * properly install collector deb Signed-off-by: Raphael Silva <rapphil@gmail.com> * Use dpkg to install collector Signed-off-by: Raphael Silva <rapphil@gmail.com> * Properly use cache Signed-off-by: Raphael Silva <rapphil@gmail.com> * Move control script tests to the CI Signed-off-by: Raphael Silva <rapphil@gmail.com> * Add the pr-build tests back Signed-off-by: Raphael Silva <rapphil@gmail.com> --------- Signed-off-by: Raphael Silva <rapphil@gmail.com> Co-authored-by: bryan-aguilar <46550959+bryan-aguilar@users.noreply.github.com> * Fix control script to avoid sed special characters Signed-off-by: Raphael Silva <rapphil@gmail.com> * Fix typo in tests Signed-off-by: Raphael Silva <rapphil@gmail.com> * Update release notes Signed-off-by: Raphael Silva <rapphil@gmail.com> --------- Signed-off-by: Raphael Silva <rapphil@gmail.com> Co-authored-by: bryan-aguilar <46550959+bryan-aguilar@users.noreply.github.com>

* bump version to v0.32.0 * Add windows version update * Port fix of ctl script to v0.32.0 release (#2269) * 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 <rapphil@gmail.com> * Add tests for control script Signed-off-by: Raphael Silva <rapphil@gmail.com> * Fix typo in github workflow Signed-off-by: Raphael Silva <rapphil@gmail.com> * Fix shell linting errors Signed-off-by: Raphael Silva <rapphil@gmail.com> * Update .github/workflows/PR-build.yml * Make debian aligned with RPM Signed-off-by: Raphael Silva <rapphil@gmail.com> * properly install collector deb Signed-off-by: Raphael Silva <rapphil@gmail.com> * Use dpkg to install collector Signed-off-by: Raphael Silva <rapphil@gmail.com> * Properly use cache Signed-off-by: Raphael Silva <rapphil@gmail.com> * Move control script tests to the CI Signed-off-by: Raphael Silva <rapphil@gmail.com> * Add the pr-build tests back Signed-off-by: Raphael Silva <rapphil@gmail.com> --------- Signed-off-by: Raphael Silva <rapphil@gmail.com> Co-authored-by: bryan-aguilar <46550959+bryan-aguilar@users.noreply.github.com> * Fix control script to avoid sed special characters Signed-off-by: Raphael Silva <rapphil@gmail.com> * Fix typo in tests Signed-off-by: Raphael Silva <rapphil@gmail.com> * Update release notes Signed-off-by: Raphael Silva <rapphil@gmail.com> --------- Signed-off-by: Raphael Silva <rapphil@gmail.com> Co-authored-by: bryan-aguilar <46550959+bryan-aguilar@users.noreply.github.com> --------- Signed-off-by: Raphael Silva <rapphil@gmail.com> Co-authored-by: Raphael Philipe Mendes da Silva <rapphil@gmail.com> Co-authored-by: bryan-aguilar <46550959+bryan-aguilar@users.noreply.github.com>

rapphil requested a review from a team as a code owner August 9, 2023 17:15

Fix: don't overwrite .env file

22be499

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 <rapphil@gmail.com>

rapphil force-pushed the rapphil-dont-overwrite-env branch from 44b8f0a to 22be499 Compare August 9, 2023 17:16

rapphil requested review from Aneurysm9 and bryan-aguilar August 9, 2023 17:44

rapphil added 3 commits August 9, 2023 21:59

Merge branch 'main' into rapphil-dont-overwrite-env

db738ea

Merge branch 'main' into rapphil-dont-overwrite-env

0d998d2

Add tests for control script

8017a80

Signed-off-by: Raphael Silva <rapphil@gmail.com>

rapphil commented Aug 11, 2023

View reviewed changes

rapphil added 2 commits August 11, 2023 00:08

Fix typo in github workflow

e9e0063

Signed-off-by: Raphael Silva <rapphil@gmail.com>

Fix shell linting errors

a7c47d0

Signed-off-by: Raphael Silva <rapphil@gmail.com>

rapphil commented Aug 11, 2023

View reviewed changes

.github/workflows/PR-build.yml Outdated Show resolved Hide resolved

rapphil added 5 commits August 10, 2023 18:11

Update .github/workflows/PR-build.yml

aede7e9

Make debian aligned with RPM

c15775b

Signed-off-by: Raphael Silva <rapphil@gmail.com>

properly install collector deb

5c2378a

Signed-off-by: Raphael Silva <rapphil@gmail.com>

Use dpkg to install collector

75405a4

Signed-off-by: Raphael Silva <rapphil@gmail.com>

Properly use cache

72cedec

Signed-off-by: Raphael Silva <rapphil@gmail.com>

rapphil commented Aug 11, 2023

View reviewed changes

rapphil added 2 commits August 11, 2023 04:43

Move control script tests to the CI

e81ce7a

Signed-off-by: Raphael Silva <rapphil@gmail.com>

Add the pr-build tests back

da125f3

Signed-off-by: Raphael Silva <rapphil@gmail.com>

bryan-aguilar approved these changes Aug 11, 2023

View reviewed changes

Merge branch 'main' into rapphil-dont-overwrite-env

bb4277e

bryan-aguilar merged commit 3d239c5 into main Aug 11, 2023
27 checks passed

bryan-aguilar deleted the rapphil-dont-overwrite-env branch August 11, 2023 17:20

rapphil mentioned this pull request Aug 12, 2023

Port fix of ctl script to v0.32.0 release #2269

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix: don't overwrite .env file #2261

Fix: don't overwrite .env file #2261

rapphil commented Aug 9, 2023 •

edited

Loading

rapphil Aug 11, 2023

rapphil Aug 11, 2023

rapphil Aug 11, 2023

Fix: don't overwrite .env file #2261

Fix: don't overwrite .env file #2261

Conversation

rapphil commented Aug 9, 2023 • edited Loading

rapphil Aug 11, 2023

Choose a reason for hiding this comment

rapphil Aug 11, 2023

Choose a reason for hiding this comment

rapphil Aug 11, 2023

Choose a reason for hiding this comment

rapphil commented Aug 9, 2023 •

edited

Loading