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 Dockerfile for Angular 13 updates #1613

Merged
merged 1 commit into from
Apr 27, 2022

Conversation

tdonohue
Copy link
Member

References

Related to discussion on Slack where @kshepherd noticed our dspace-angular Docker container was only listening on localhost. This caused the UI to be unresponsive unless you accessed it from within the running Docker container (via wget or similar)

Description

During the Angular 13 upgrade (#1567), the yarn serve command (which is used to run the app in dev mode) was updated to use ng serve (Angular) instead of node. This had the unfortunate side effect of limiting the dspace-angular docker container to listening only on localhost instead of on 0.0.0.0 (default for node) because ng serve defaults to localhost, see https://angular.io/cli/serve

This small PR fixes our Dockerfile to specify --host 0.0.0.0 to ensure it listens to all IP addresses, and not only to localhost.

Instructions for Reviewers

Using this PR, try the following:

  • Rebuild your local dspace-angular Docker container: docker-compose -p d7 -f docker/docker-compose.yml build
  • Spin it up & verify the UI now works: docker-compose -p d7 -f docker/docker-compose.yml up -d

@tdonohue tdonohue added bug high priority backend: Docker related to DSpace deployment via Docker labels Apr 26, 2022
@tdonohue tdonohue added this to the 7.3 milestone Apr 26, 2022
@tdonohue tdonohue requested a review from kshepherd April 26, 2022 17:24
@tdonohue tdonohue self-assigned this Apr 26, 2022
@tdonohue
Copy link
Member Author

NOTE: I've just realized with additional testing that Dev Mode isn't fully working in Angular 13.. see #1614 . So, while this PR "works", it causes Docker to use config.prod.yml instead of config.dev.yml.

So, this PR is a partial fix, and it appears to be dependent on first fixing #1614

@kshepherd
Copy link
Member

kshepherd commented Apr 27, 2022

@tdonohue just a note, in my local fix i instead updated package.json serve profile like this:
"serve": "ng serve -c development --host 0.0.0.0", (just adding the --host flag)
And it seemed to work, and I think development mode seems to work OK - I can log in, when I copy over new files the source is recompiled, etc.
It also doesn't require the docker image to be rebuilt and works for both dev and production. However, it would affect servers running normally, not just docker services and could be a security or configuration problem.

Happy to just +1 and merge this if the Dockerfile is a better approach -- since the package.json fix would also affect non-Docker instances, it could be a security issue. But thought I'd note it here.

(what if we added a new serve:dev or something, that could include the --host 0.0.0.0 line in the ng serve command?)

Copy link
Member

@kshepherd kshepherd left a comment

Choose a reason for hiding this comment

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

+1, tested and working. Will merge this now as it is high priority and allows yarn/ng to bind properly, though as per the last comment this might only be a partial fix or could be substituted for a different fix in future

@kshepherd kshepherd merged commit 13dac1a into DSpace:main Apr 27, 2022
@tdonohue tdonohue deleted the fix_docker_build branch December 7, 2023 22:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend: Docker related to DSpace deployment via Docker bug high priority
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants