Provide AWS_REGION variable to cargohold #314
Conversation
We do this for all the other servers, too, but I'm actually not sure why. The integration tests themselves don't call AWS. We should probably just remove it (+ from the other services)
| env: | ||
| # these dummy values are necessary for Amazonka's "Discover" | ||
| - name: AWS_ACCESS_KEY_ID | ||
| value: "dummy" | ||
| - name: AWS_SECRET_ACCESS_KEY | ||
| value: "dummy" | ||
| - name: AWS_REGION | ||
| value: "eu-west-1" |
There was a problem hiding this comment.
This is what we also do for the other services, but unless I'm missing something it is completely unnecessary?
The integration test binaries don't talk to AWS directly, so they should not need these variables.
We should probably remove them (from all integration pods, in a separate PR).
There was a problem hiding this comment.
I cannot remember right now but... amazonka might complain/throw errors if they are not defined?
There was a problem hiding this comment.
But this pod only runs the cargohold-integration binary, right? And that doesn't depend on amazonka.
There was a problem hiding this comment.
I also verified this locally for gundeck (from where I copied the config). I can run the binary without providing the environment variables.
Well, for now I'll merge this and then I can make a separate PR removing the config from all -integration pods.
There was a problem hiding this comment.
cargohold-integration depends on cargohold lib which does have a dependency on amazonka. I can't tell for sure if it's a problem or not; it may be different for other services because brig-integration and galley-integration do talk directly to AWS (they check the queues)
See https://github.com/zinfra/backend-issues/issues/488
Now with cargohold using V4 signatures, the region is part of the Authorization header and needs to configured correctly. We already do that for the other services, so I just copied the code from there.