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: set host vars in docker compose #194

Merged
merged 2 commits into from
Mar 27, 2024
Merged

Conversation

cjenkscybercom
Copy link
Contributor

When starting the ecosystem / services with docker compose, the backend fails to communicate with elastic and mongo db.

This is because in the compose networking environment the service hosts are the names of the services (elastic, mongo, etc) rather than localhost.

This fixes that configuration issue for local development, and also adds localhost:3000 as an allowed cors origin so that local frontend development can speak to the server as well.

@cjenkscybercom cjenkscybercom self-assigned this Mar 26, 2024
@@ -9,6 +9,10 @@ services:
depends_on:
- mongo
- elastic
environment:
cors.allowed.origins: http://localhost:3000
Copy link
Contributor

@thelarsjohansson thelarsjohansson Mar 26, 2024

Choose a reason for hiding this comment

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

It is reasonable to have mongo.host and elasticsearch.network.host environment properties in docker compose file.

But it is not reasonable to have cors environment property there when reason is for localhost value. Have local docker compose file for such purpose. Git ignore container docker-compose-local* which means that you can have local docker compose file named docker-compose-local.yml for such purpose.

In addition, mongo.host and elasticsearch.network.host environment properties are missing from docker-compose-integrationtest-yml.

Please adjust.
Otherwise it's fine!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, can do.

Though I'll note having tried this that it appears the OlogIT etc tests don't appear to run successfully regardless and I don't see them running in the CI/CD here?

So that at least I think should be addressed separately. But at least given how the tests are run, using the same args in the integration tests seems reasonable.

Except cors allowed origins 🤦
I realized upon inspection that it already defaults to localhost:3000 in the java configuration files when the value is unset.

Copy link
Contributor

@thelarsjohansson thelarsjohansson left a comment

Choose a reason for hiding this comment

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

Thank you for changes. Looks fine!

@thelarsjohansson thelarsjohansson merged commit 5b56a51 into master Mar 27, 2024
5 checks passed
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