Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(mae-consumer): fix regression on base64 encoding #6061

Merged
merged 3 commits into from
Sep 29, 2022
Merged

fix(mae-consumer): fix regression on base64 encoding #6061

merged 3 commits into from
Sep 29, 2022

Conversation

codesorcery
Copy link
Contributor

Pull request #5827 introduced a regression by removing coreutils from the mae-consumer Dockerfile (coreutils was added in #3723). This broke the base64 call in the startup script s.th. the Elasticsearch auth header will not be correctly set when username and password are provided.

To make sure that the startup script fails on these errors in the future, set -euo pipefail which lets the bash script fail on errors and unset variables. Also refactor the startup script to make it more stable and readable.

@github-actions github-actions bot added the devops PR or Issue related to DataHub backend & deployment label Sep 27, 2022
@github-actions
Copy link

github-actions bot commented Sep 27, 2022

Unit Test Results (build & test)

584 tests  ±0   580 ✔️ ±0   12m 56s ⏱️ +2s
143 suites ±0       4 💤 ±0 
143 files   ±0       0 ±0 

Results for commit 7d777a6. ± Comparison against base commit 164bc1a.

♻️ This comment has been updated with latest results.

@pedro93
Copy link
Collaborator

pedro93 commented Sep 27, 2022

Thank you for this PR @codesorcery, this was an oversight on my part! Weird that during testing this was not caught... The smoke tests don't seem to be passing. Are you able to take a look?

@codesorcery
Copy link
Contributor Author

Hey @pedro93 , found some small mistakes within my changes. Shouldn't really have anything to do with the failing smoke tests, but let's see if this already fixes the tests.

Pull request #5827 introduced a regression by removing coreutils
from the mae-consumer Dockerfile (coreutils was added in #3723).
This broke the base64 call in the startup script s.th. the Elasticsearch
auth header will not be correctly set when username and password are
provided.

To make sure that the startup script fails on these errors in the
future, set "-euo pipefail" which lets the bash script fail on errors
and unset variables.
Also refactor the startup script to make it more stable and readable.
@maggiehays maggiehays added the community-contribution PR or Issue raised by member(s) of DataHub Community label Sep 28, 2022
@pedro93 pedro93 merged commit 9e7bd1a into datahub-project:master Sep 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community-contribution PR or Issue raised by member(s) of DataHub Community devops PR or Issue related to DataHub backend & deployment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants