search

ral-facilities / object-storage-api

Apache License 2.0
0 stars 0 forks source link

Investigate pinning sub dependencies #3

Open joelvdavies opened 1 month ago

joelvdavies commented 1 month ago

On ims-api and ldap-jwt-auth we don't pin sub-dependencies. There are cases where this could cause issues e.g. docker builds installing different dependencies when tags are pushed, or depndabot failing to notify us of security issues. However using a requirments.txt also has issues e.g. when sub dependencies are added/removed in new versions leading to an invalid requirements.txt. Equally where others suggest using poetry, we are not very keen on it complicating setup.

In #2 I am adding a requirements.txt to test it out, but we should re-evaluate this later and decide whether to revert the change, find a different solution or apply it to ims-api and ldap-jwt-auth.

joelvdavies commented 1 month ago

Another thing I just noticed FastAPI has

"starlette>=0.37.2,<0.39.0",

but the requirements.txt is now being updated by dependabot - so it wont care when it should.

Pip would probably complain during the build though.

joelvdavies commented 1 month ago

Also see https://github.com/ral-facilities/object-storage-api/pull/10#issuecomment-2368049630. Some dependencies may come from just the OS its being installed on (in this case it seems its an optional one https://stackoverflow.com/questions/70731019/is-it-impossible-developing-with-fastapi-uvloop-windows) but there could be cases where one requirements file for all OS's doesn't work.

joelvdavies commented 1 month ago

Another thing I just noticed FastAPI has

"starlette>=0.37.2,<0.39.0",

but the requirements.txt is now being updated by dependabot - so it wont care when it should.

Pip would probably complain during the build though.

On https://github.com/ral-facilities/object-storage-api/pull/13, it could be tested by looking at the unit test CI. (Once some unit tests are added)

joelvdavies commented 1 month ago

It does complain see: https://github.com/ral-facilities/object-storage-api/pull/24 image It errored on the unit tests during install.

joelvdavies commented 2 weeks ago

Today had this repo picking up a starlette cve: https://avd.aquasec.com/nvd/2024/cve-2024-47874/. ims-api did not show the same due to github not seeing it as a dependency, but it was picked up by Trivy on Harbor (which is where I first noticed it).