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(docker): Fix issues with base64 not working on some platforms #3723

Merged
merged 1 commit into from
Dec 13, 2021

Conversation

dexter-mh-lee
Copy link
Contributor

Got reports that base64 version may be different in different platforms. Adding coreutils which downloads the gnu base64 making it consistent across platforms.

Checklist

  • The PR conforms to DataHub's Contributing Guideline (particularly Commit Message Format)
  • Links to related issues (if applicable)
  • Tests for the changes have been added/updated (if applicable)
  • Docs related to the changes have been added/updated (if applicable)

@github-actions
Copy link

Unit Test Results

     44 files       44 suites   41m 23s ⏱️
   623 tests    571 ✔️ 52 💤 0
1 385 runs  1 311 ✔️ 74 💤 0

Results for commit 3469a12.

Copy link
Contributor

@shirshanka shirshanka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@shirshanka shirshanka merged commit d4d6218 into datahub-project:master Dec 13, 2021
pedro93 added a commit that referenced this pull request Sep 29, 2022
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.

Co-authored-by: Pedro Silva <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants