Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Investigate pinning sub dependencies #3

Open
joelvdavies opened this issue Sep 19, 2024 · 6 comments
Open

Investigate pinning sub dependencies #3

joelvdavies opened this issue Sep 19, 2024 · 6 comments

Comments

@joelvdavies
Copy link
Collaborator

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
Copy link
Collaborator Author

joelvdavies commented Sep 19, 2024

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
Copy link
Collaborator Author

joelvdavies commented Sep 23, 2024

Also see #10 (comment). 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
Copy link
Collaborator Author

joelvdavies commented Sep 24, 2024

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 #13, it could be tested by looking at the unit test CI. (Once some unit tests are added)

@joelvdavies
Copy link
Collaborator Author

It does complain see: #24
image
It errored on the unit tests during install.

@joelvdavies
Copy link
Collaborator Author

joelvdavies commented Oct 22, 2024

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).

@joelvdavies
Copy link
Collaborator Author

joelvdavies commented Nov 28, 2024

#71 worked successfully without having to recreate the pip env/manually edit the requirements to ensure all versions were compatible with each other like a lot of the others due to pydantic having a newer version that fastapi didn't support.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Backlog
Development

No branches or pull requests

1 participant