Use docker secrets for EDL credentials#134
Conversation
hrodmn
left a comment
There was a problem hiding this comment.
The flexibility of being able to use .netrc or the EARTHDATA_* env vars is nice, but my preference would be to just require the env vars for simplicity's sake. The (gitignored) plain text files for credentials feels clunky to me, but I trust your judgement on this @chuckwondo!
| earthdata-username: | ||
| file: ./secrets/earthdata-username.txt | ||
| earthdata-password: | ||
| file: ./secrets/earthdata-password.txt |
There was a problem hiding this comment.
This could all be a lot simpler if we simply instructed users to set the EARTHDATA_USERNAME and EARTHDATA_PASSWORD env vars. Then we wouldn't need the secret-writing script.
| earthdata-username: | |
| file: ./secrets/earthdata-username.txt | |
| earthdata-password: | |
| file: ./secrets/earthdata-password.txt | |
| earthdata-username: | |
| environment: "EARTHDATA_USERNAME" | |
| earthdata-password: | |
| environment: "EARTHDATA_PASSWORD" |
There was a problem hiding this comment.
Unfortunately, using the env vars for the secrets is unreliable. There's a known issue for docker compose where it does not properly pass through the env vars to the secrets for docker, which is why I ended up going the route of the files, which is completely reliable, if a bit clunky.
It is possible for the env vars to be defined, but when building the image via docker compose, the secrets are populated as empty strings. In our case, that means the .netrc is written with blank values for the username and password, so the image is built without error, but when the app starts, it bombs because the creds are blank.
I had originally taken the approach you suggested above, but ran into this issue, which drove me nuts until I found the various issues around this. Some are marked as closed, but there still seems to be a problem, which might be macOS-specific, but I really don't know.
Unfortunately, the only reliable alternative was the file-based approach.
There was a problem hiding this comment.
However, if you feel this is perhaps too annoying, I'd be willing to avoid dealing with docker secrets since this is only for local development, so there's no serious security concern if we stick with env vars.
Let me know if that's what you'd prefer and I'll just close this PR without merging, and mark the related issue as "not planned".
789a077 to
3e45d37
Compare
Fixes #126