crashappsec / chalk

Chalk allows you to follow code from development, through builds and into production.
https://crashoverride.com/
GNU General Public License v3.0
322 stars 11 forks source link

fix: fetch GCP metadata from 169.254.169.254 directly #293

Closed nettrino closed 1 month ago

nettrino commented 1 month ago

Issue

Fixes #292

nettrino commented 1 month ago

We should probably define 169.254.169.254 as a const in this file and use that everywhere, but I don't mind leaving that till later.

Agreed - not sure though if there are lists of IPs that differ slightly per provider? this plugin could be split / refactored in the future so left it as such for now as I treated it as a nit as well

ee7 commented 1 month ago

not sure though if there are lists of IPs that differ slightly per provider

For this particular case, I think we'd be pretty safe even if we added more cloud providers. My understanding is that 169.254.0.0/16 is reserved for link-local addressing, and it's pretty standard for the instance metadata service to be available via 169.254.169.254. At least also for:

thc202 commented 1 month ago

Alibaba Cloud uses a different address: https://www.alibabacloud.com/help/en/ecs/user-guide/view-instance-metadata (not arguing against or for, just providing an example).

ee7 commented 1 month ago

Cool, thanks.

Yeah, I've got no context for Alibaba Cloud. Looks like it's different for Tencent Cloud too.

ee7 commented 1 month ago

I suggest merging with the edited shorter PR title - commit title would be longer than 72 chars otherwise, which overflows e.g. GitHub's commit view.