Add Docker support + Docker secrets with fallback (fixes #92)#98
Conversation
|
I was also thinking of adding a section to the |
|
@RicYaben I removed the artifacts referencing redis and stuff like that as discussed, please have a look when you get time and let met know if any other changes are required |
RicYaben
left a comment
RicYaben
left a comment
There was a problem hiding this comment.
I cannot merge this PR. As it is right now, it mixes concepts from the previous versions into the new one. The goal was to get rid of unnecessary things, not to add them again. On top of that, this PR touches other files unrelated to attempting to read secrets from a secure location instead of having them just lying around. On that note, it still loads the API secrets into the app at runtime, so there is no much of a difference anyway.
I recommend going back to the drawing board to do just what is needed, no other files touched.
There was a problem hiding this comment.
This file should not be touched by your PR, stick to the docker support
| block_scanners: Optional[bool] = typer.Option( | ||
| None, | ||
| "--block-scanners/--no-block-scanners", | ||
| help="Block known scanner IPs. Defaults to BLOCK_SCANNERS env var.", | ||
| ), | ||
| integrity_check: Optional[bool] = typer.Option( | ||
| None, | ||
| "--integrity-check/--no-integrity-check", | ||
| help="Enable DICOM file integrity checks. Defaults to INTEGRITY_CHECK env var.", | ||
| ), |
There was a problem hiding this comment.
what is this? why is this here?
| hp = new_dicomhawk(config) | ||
| hp.run() No newline at end of file | ||
| hp = new_dicomhawk(logger, [], config) | ||
| hp.start() No newline at end of file |
There was a problem hiding this comment.
why did you change it from run to start?
| EXCEPTIONS_LOG_DIR: str | ||
|
|
||
| # TODO: we shouldn't store api keys. This can be loaded from secrets | ||
| ABUSE_IP_API_KEY: str |
There was a problem hiding this comment.
The TODO refers to not load API credentials in memory. Values can be read when needed (just-in-time)
| try: | ||
| with open(f"{base}/{name}") as f: | ||
| return f.read().strip() | ||
| except (FileNotFoundError, PermissionError): |
There was a problem hiding this comment.
silent exception? why is this not handled? if there are no permissions to read, why would we want to continue executing? exit, log the error
| networks: | ||
| - main_network |
There was a problem hiding this comment.
is there any other network?
| abuse_ip_key: | ||
| file: ./abuse_ip_key | ||
| ip_quality_score_key: | ||
| file: ./ip_quality_score_key | ||
| virus_total_key: | ||
| file: ./virus_total_key |
| tcia_data: | ||
| tcia_stagger: |
| networks: | ||
| main_network: | ||
| driver: bridge No newline at end of file |
There was a problem hiding this comment.
the default network is already a bridge
| @@ -0,0 +1,30 @@ | |||
| FROM python:3.12-slim | |||
|
|
|||
| WORKDIR /app | |||
|
Hi @RicYaben, apologies for going out of scope earlier. I’ve now kept the changes minimal and focused only on the essentials: this update adds a Dockerfile, a minimal docker-compose configuration with the three secrets (tcia_username, tcia_password, and honey_url) mounted, and two small helper functions in config.py to read from those secret files with an environment variable fallback for local development. No other changes were made. Please let me know if anything needs adjustment. |
2 participants
Hey @RicYaben,
As we discussed in #92, I’ve implemented Docker file-based secrets on the v3.0-dev branch (thank you again for telling me to work on top of it!).
v3.0-dev didn't have any Docker support yet (no Dockerfile, no compose file), so I first created a clean, minimal setup by taking reference from the older branches (mainly v3.0) and adapting everything to the new refactored structure (
src/,dicomhawkCLI, etc.).Then I implemented the Docker secrets feature exactly as per our conversation.
What was done
Dockerfileanddocker-compose.ymlso the project can run in containersSECRETS_BASE_PATHenv var (defaults to/run/secrets)get_secret()+secret_or_env()helpers insrc/dicomhawk/config.pyreadSecret()helper inAPI/threatIntelligence.jsEverything has been tested:
/run/secrets/and are readable inside containersdocker inspectPlease take a look whenever you have time and let me know your thoughts if anything should be changed, scoped differently, or improved, I'm more than happy to work on it.
Thanks again for the direction really appreciate it!