search

cds-snc / notification-planning

Project planning for GC Notify Team
5 stars 0 forks source link

Update Notify package management #658

Closed jzbahrai closed 1 year ago

jzbahrai commented 2 years ago

Description

We can backport some changes from GDS and their improved way of doing package management.

CDS documentation on patch management: https://github.com/cds-snc/platform-sre-security-support/wiki/Patch-management

Please look at this thread for discussion: https://gcdigital.slack.com/archives/CV38DBNVA/p1652794882038499

GDS has introduced some improvements by using pip-tools and pyup that we can also replicate. These would allow for transitive dependencies traces.

There is also poetry which could an option instead of pip-tools / pyup. To evaluate and consider.

Acceptance Criteria

Explore in the ADR the following items:

QA

Additional Context

The current Notify setup makes use of a Make target named freeze-requirements. This is an in-house GDS target to resolve transitive dependencies based on an intake and desired list of dependencies. The intake currently sits as requirements-app.txt file while the resolved dependencies will get resolved as the requirements.txt file, sitting at the root of the project.

With the new changes in, the dependency intake would actually be in a pyproject.toml file instead and the resolved dependencies are stored in the poetry.lock file.

The pyup tool most likely would not be adopted as we have similar tooling (Renovate) that detects vulnerabilities and automatically opens PRs with potential fixes and Renovate natively supports Poetry. We might want to check if that offers anything new... but also, this might conflict with new solutions being brought in by the SRE team (ask @mohamed-cds ).

The Poetry tool seems quite mature and we instill best practices.

yaelberger-commits commented 2 years ago

Hey team! Please add your planning poker estimate with ZenHub @andrewleith @iokpala @jimleroyer @jzbahrai @sastels

jimleroyer commented 2 years ago

@sastels and @andrewleith Are you OK if we move forward without an ADR and implement the suggestions listed in the acceptance criteria?

sastels commented 2 years ago

Sounds reasonable, especially if GDS has had success. If it ends up not working then we can explore alternative approaches through an ADR.

andrewleith commented 2 years ago

I agree!

jimleroyer commented 2 years ago

Sounds good then! @sastels and @andrewleith , if you can estimate on the issue then as-is, not for the ADR but the actual changes, it would be much appreciated. 🙏

yaelberger-commits commented 2 years ago

please also see link in description to CDS documentation for patch management

mohdnr commented 2 years ago

If you want to see the new patch management setup in action take a look at https://github.com/cds-snc/scan-websites/issues/322. This is a dashboard that the new tool "renovate" will maintain to help you keep track of what needs to be patched.

How to add it to you repo (my team can help):

  1. Give the Renovate GitHub app permission on the repo; and
  2. Submit a PR to the repo to add the Renovate config. This uses a default config that was setup for all of CDS.

Here’s the spot in the GitHub app that you add repos: image

andrewleith commented 2 years ago

Just a note that we are meeting with SRE this week re:renovate which will hopefully take care of at least a portion of this work.

patheard commented 2 years ago

I did some research on Renovate's support for transitive dependency trace comments and it doesn't provide this. However, it is able to consume a pip-compile requirements.in dependency file using the following config:

{
  "pip-compile": {
    "fileMatch": ["(^|/)requirements\\.in$"]
  }
}
yaelberger-commits commented 2 years ago

Some overlap with Renovate dependency tracker

whabanks commented 1 year ago

I threw together a quick doc on Pip-tools vs Poetry based on my findings thus far.

yaelberger-commits commented 1 year ago

Could also look into Renovate as well

whabanks commented 1 year ago

Regarding the question: Can we leverage Renovate in place of Poetry / in conjunction with pip instead of moving to Poetry?

Renovate has little to no overlap with package managers. It provides DevOps oriented tooling, that notifies us via PR's with suggested version updates for dependencies.

Renovate does this by reading package files generated by package managers like poetry, pip, pip-tools or npm and searches PyPi to check if deps listed in the package file are out of date.

All this to say: Renovate cannot do what tools like pip or Poetry do for us and depends on the usage of a package manager in a project to fulfill its function. We can use these tools in conjunction with one another but neither is a replacement for the other.

yaelberger-commits commented 1 year ago

Small draft PR with Poetry for anyone who wants to play around with it

whabanks commented 1 year ago

Updated the Pip-tools vs Poetry doc with some comparisons between migrating projects to both Pip-tools and Poetry. Added a short blurb on the level of risk associated with migrating to each tool.

yaelberger-commits commented 1 year ago

Will working today, doing some work on github actions workflows from pip to poetry. Start migrating other projects over to Poetry as well

andrewleith commented 1 year ago
andrewleith commented 1 year ago
yaelberger-commits commented 1 year ago

Document download API PR is ready to go Utils API admin still some work to clean up, should be ready today for review

smcmurtry commented 1 year ago
YedidaZalik commented 1 year ago

Document download API successfully deployed, running with Poetry Small problem with docker file Working on possible solutions and hoping for 1 deployment

smcmurtry commented 1 year ago

@whabanks could you add an update to this card?

smcmurtry commented 1 year ago

Yesterday successfully deployed Admin changes to staging. Hoping to get the notification-api changes deployed today.

smcmurtry commented 1 year ago

API ready for another test deployment to staging. Pat identified a compatibility issue with Renovate causing errors. Will troubleshooting that today.

whabanks commented 1 year ago

Moving to blocked: Renovate is currently broken for Admin and is expected to be broken in API as well. It looks like until we can self host Renovate to increase the execution timeout this issue will persist. This is just a soft block, other work such as deploying and testing API and Utils can still continue.

smcmurtry commented 1 year ago

Issues with the lambda API image, looks like we may need to self-host renovate to make it work with Poetry.

sastels commented 1 year ago

lambda api issues fixed, poetry PR merged to staging and tested.

yaelberger-commits commented 1 year ago

Relevant Slack thread for current state https://gcdigital.slack.com/archives/CV38DBNVA/p1677075866073539

andrewleith commented 1 year ago
patheard commented 1 year ago

The problem is that the werkzeug update to 2.2.3 is causing a dependency conflict since notification-utils is pinned to werkzeug==2.2.2. This then triggers an infinite loop in Poetry:

I chatted with @whabanks and I'm going to submit a Renovate config change to ignore werkzeug updates to see if this is the only dependency conflict we're hitting.

Ideally there will be an upstream fix to Poetry that allows it to fail gracefully on conflicts.

patheard commented 1 year ago

After having Werkzeug ignored by Renovate, things are working again! image

Until Poetry resolves the bug, we'll have to periodically check that Renovate is completing since unfortunately the temporary-error on the Renovate side doesn't get surfaced so it's easy to miss that it's happening.

andrewleith commented 1 year ago

Great news!

Will other package bumps that both admin and utils depend on lead to this situation? There are several that the two projects have that need to stay in sync when doing dep bumps.

patheard commented 1 year ago

Yup, it's possible that we'll hit this problem with other products if you're using Poetry to manage the dependencies.