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

remove DNS instructions for solr, activemq, and blazegraph #61

Merged
merged 2 commits into from
Sep 24, 2024

Conversation

joshdentremont
Copy link
Contributor

Since these three services are not exposed via traefik they don't need to have their DNS records set

Copy link
Contributor

@joecorall joecorall left a comment

Choose a reason for hiding this comment

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

If we're removing this from the README, should we also remove the relevant config from docker-compose.yml for these services?

Copy link
Contributor Author

@joshdentremont joshdentremont left a comment

Choose a reason for hiding this comment

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

If it doesn't hurt anything I would be in favour of leaving them in, since it makes them easier to toggle if someone wanted to expose them.

They have been in the docker-compose.yml all along, with the traefik-disabled label, so this doesn't really change anything. Just specifies that there is no need to update DNS records.

That said, I don't feel strongly one way or another. We could remove the extra lines from docker-compose.yml and just document how to expose it.

@joecorall
Copy link
Contributor

I'm in favor of removing the config and documenting how to use SSH port forwarding to access these services if/when needed and/or having traefik limit what IP ranges can access the services. Leaving these services open to the WWW in production is a security risk, and we shouldn't have the recommended solution for needing to access the UIs for these services be opening them up world wide.

@joshdentremont
Copy link
Contributor Author

We just discussed this at the tech call and everyone is in favour of this. I will push another update to make those changes.

@joecorall joecorall merged commit 821de3e into Islandora-Devops:main Sep 24, 2024
1 check 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