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.

Sign up for GitHub

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

fix: set host vars in docker compose #194

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
10bbce5
fix: set host vars in docker compose
cjenkscybercom Mar 26, 2024
099e3df
remove duplicate cors arg, add args to int test
cjenkscybercom Mar 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions docker-compose.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ services:
depends_on:
- mongo
- elastic
environment:
cors.allowed.origins: http://localhost:3000
Copy link
Contributor

@thelarsjohansson thelarsjohansson Mar 26, 2024 •
edited
Loading

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.

mongo.host: mongo
elasticsearch.network.host: elastic
# Ensure that the elastic server is up before starting olog.
command: >
/bin/bash -c "
Expand Down
Loading