libcluster_ec2 icon indicating copy to clipboard operation
libcluster_ec2 copied to clipboard

Keep hackney, remove Tesla

Open tzumby opened this issue 1 year ago • 2 comments

Hey @kyleaa,

Thanks for creating this cool library. Hope it's ok I'm bringing this PR in, I'm trying to achieve two things:

  1. There are currently 2 http clients in this app: hackney and Tesla. Since ex_aws is already using hackney by default, I updated the calls that were using Tesla to also go through hackney. This requires explicit mocking with Mox, but at least now there's only one common http client used
  2. If your Elixir release uses :longnames, the full private DNS will be used for the node name, I added a :private_dns setting to the ip type to support that.

tzumby avatar Sep 15 '23 20:09 tzumby

Hi @tzumby, apologies for the delay here. There was an earlier PR for IMDSv2 that I had suggested we switch to using functions from ExAws core to accomplish - I just took a stab at reimplementation there, and in the process completely removed all direct HTTP calls from the library entirely. See https://github.com/kyleaa/libcluster_ec2/pull/33

If you're willing to take a peek at that implementation, I am going to attempt to set up a better test environment this weekend before I consider a release of the IMDSv2 unless anyone beats me to testing it out. I think the private DNS changes then would make sense to apply on top of that.

kyleaa avatar Jan 26 '24 06:01 kyleaa

Hey @kyleaa , that makes a lot more sense to use the metadata API. In that case I will close this one and re-open another one just with the private dns change. Looking back now, it would have made sense to introduce the private dns change separately anyway

tzumby avatar Jan 26 '24 15:01 tzumby