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

Experimental new docker system that will work with volume_mounts #3

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Experimental new docker system that will work with volume_mounts #3

wants to merge 7 commits into from

Conversation

PaulGWebster
Copy link
Contributor

There are security implications to this!

Make sure to see how the /script directory works!

Also I had to change to MD5 in the pg_hba.conf though I imagine someone with more postgres experience could resolve that!

@wlad wlad changed the base branch from master to feature/test-experimental-docker-with-volume-mounts March 17, 2021 11:21
@wlad wlad changed the base branch from feature/test-experimental-docker-with-volume-mounts to master March 17, 2021 11:43
@wlad
Copy link
Contributor

wlad commented Mar 17, 2021

@PaulGWebster thank you for this PR.
I'm going to push an new image tagged experimental to docker hub for further testing and will check that it works well in our CI pipelines.

If everything works out well I'd like some other colleges to review mentioned security implications.

wlad added a commit to ehrbase/ehrbase that referenced this pull request Mar 17, 2021
wlad added a commit to ehrbase/ehrbase that referenced this pull request Mar 17, 2021
…by Docker Hub [skip-ci]

- to adapt changes suggested in PR ehrbase/docker#3

# Stop database before proceeding
su - postgres -c "pg_ctl -D ${PGDATA} -w stop"
psql -U "${POSTGRES_USER}" -d "${POSTGRES_DB}" <<-EOSQL
Copy link
Contributor

Choose a reason for hiding this comment

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

please use verbose form of cli options --username and --dbname

@@ -3,56 +3,65 @@ FROM postgres:11.5-alpine
# Set default values for database user and passwords
ARG EHRBASE_USER="ehrbase"
ARG EHRBASE_PASSWORD="ehrbase"
ENV EHRBASE_USER=${EHRBASE_USER}
ENV EHRBASE_PASSWORD=${EHRBASE_PASSWORD}
ENV EHRBASE_USER=${EHRBASE_USER:-ehrbase}
Copy link
Contributor

Choose a reason for hiding this comment

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

what is this change good for?

I see one test suite failing and I guess this change may be the reason(?)
image

check --> https://app.circleci.com/pipelines/github/ehrbase/ehrbase/1714/workflows/e1b3273c-0909-4b14-bb27-9f0fc217c809/jobs/20683

wlad added a commit that referenced this pull request Mar 22, 2021
to create and push experimental docker image
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