search

google / osdfir-infrastructure

Helm charts for running open source digital forensic tools in Kubernetes
Apache License 2.0
65 stars 16 forks source link

Github rate limit hit pulling Turbinia releases information #161

Open hacktobeer opened 1 month ago

hacktobeer commented 1 month ago

Name and Version

charts/turbinia 1.1.1

What environment are you using?

Minikube

What steps will reproduce the bug?

Helm deploy Turbinia and spinup a heavy processing job that start many workers. Each worker will try to pull from api.github.com here https://github.com/google/osdfir-infrastructure/blob/main/charts/turbinia/templates/init-configmap.yaml#L29

As the rate limit for unauthenticated users is 60 requests per hour it will prevent containers from pulling the Turbinia configuration file once that limit is hit (as it can't resolve the release version through the github api call). https://docs.github.com/en/rest/using-the-rest-api/rate-limits-for-the-rest-api?apiVersion=2022-11-28#primary-rate-limit-for-unauthenticated-users

Are you using any custom parameters or values?

No

What is the expected behavior?

To not fail the release github api call and succesful download of the turbinia configuration template file.

What do you see instead?

wget: server returned error: HTTP/1.1 403 rate limit exceeded

Additional information

No response

wajihyassine commented 1 month ago

Thanks nice find. This one will be a bit difficult to solve with how we are pulling the config files to not have to check them in by default and maintain their versions. Will have to think about it if you have any ideas please feel free to share. In the meantime at least we can pull the config file locally and skip the API calls entirely as a workaround

hacktobeer commented 1 month ago

Agree, as we do not manage the configs at deployement but pull them at boot time I don't see an easy way atm. What we could look into is not using the versioned config (for which you call the github api) but using the raw latest version from github (which has no rate limiting). In theory we could break something if a new config is not backward compatible, but that is something we have never done before (famous last words...).

hacktobeer commented 1 month ago

So we pull from below instead of the whole release version dance. https://github.com/google/turbinia/raw/master/turbinia/config/turbinia_config_tmpl.py

wajihyassine commented 13 hours ago

In theory we could break something if a new config is not backward compatible, but that is something we have never done before (famous last words...).

Hehe that is actually why this logic was put in the first place, because of this issue happening... but agree this was a rare occurrence so may be worth the risk vs. pods failing to deploy due to rate limiting and that harder for folks to debug. I think this would be an issue with the containers as well... I can remove it in the next PR and if we can think of a better way adjust from there. Worse case scenario we can always provide the config instead through a configmap.