Lock Dependencies in Synapse

Hi folks,

Right now most of Synapse's dependencies only declare a minimum version bound, like Twisted>=18.9.0 (cite). This means that every time we build release artifacts or install Synapse from source, we unconditionally pull in the latest versions of our dependencies at that moment.

This creates unnecessary risk, as our dependencies can change out from under us without warning. For example, installing Synapse 1.49.0 may work today but fail tomorrow if one of our dependencies releases a new version overnight.

This exact scenario bit us with the release of attrs 21.1.0 (synapse#9936) on May 6th. We were forced to release Synapse 1.33.1 less than a day after 1.33.0 as the attrs release broke our ability to install Synapse from source, build working Debian packages, or create functioning Docker images, even though nothing in our repositories had changed.

The absence of locked dependencies also impedes our ability to pursue continuous delivery, maintain LTS releases, or easily backport security fixes as we cannot recreate older release artifacts without also implicitly updating all of the dependencies included therein.

Definition of Done

For any given Synapse commit, it is possible to repeatably create identical virtualenvs.

Further Discussion / Constraints

Resolving this implies that it must be possible to enumerate exact versions of all dependencies included in any given upstream release of Synapse, using only a clone of the Synapse repository. This is important for auditing, as it allows us to easily answer questions like "did we ever ship a release with a vulnerable version of that dependency?"

Moreover, any solution must record hash digests to protect against remote tampering, such as with pip's hash-checking mode.

To ease maintenance burden (and avail of GitHub's supply chain security features), it would be nice if whatever solution we arrived at integrated with Dependabot. Supported package ecosystems for Python are requirements.txt (pip / pip-tools), pipfile.lock (Pipenv), and poetry.lock (Poetry).

"did we ever ship a release with a vulnerable version of that dependency?"

this appears to assume that our releases include their dependencies. We do not vendor our dependencies into the pypi release, with good reason.

I think what you mean is "did we ever ship docker images for a release with a vulnerable version of that dependency?".

It's a bit of a terminology nit, but an important one imho: Synapse releases are distinct from those of our dependencies.

I think what you mean is "did we ever ship docker images for a release with a vulnerable version of that dependency?".

Sure, Docker images or Debian packages. Our built release artifacts / things we're responsible for as a team. Downstream packagers and people directly consuming a Synapse wheel or sdist are outside of the scope of this proposal.

@richvdh

For any given Synapse commit, it is possible to repeatably create identical virtualenvs.

From previous conversations my understanding was that it was important to be able to create an identical virtualenv for any release of Synapse, not any commit. (I think this is related to what @richvdh is saying though.) Those aren't quite the same as described though.

This is fine, and as you note has many advantages, but I do have a counter-requirement, which is:

We should not fall into the trap of only supporting the pinned version of a dependency. To take an arbitrary example, we currently require cryptography>=3.4.7. That's eight months old now, and being a crypto thing, if we were pinning, we would probably pin to the most recent version. However, doing so would make packaging Synapse for distributions effectively impossible.

I therefore think that any "pinned" versions should be in addition to the current "minimum" versions, where the minimum versions are also subject to a certain amount of testing (as we have presently).

(Exactly how you you would choose whether to use the "pinned" version or the "minimum" version is very much up for debate)

@clokep

my understanding was that it was important to be able to create an identical virtualenv for any release of Synapse, not any commit.

@clokep: Is this a distinction without a difference? Even if we were only dumping pip freeze into the first commit of a release branch, we'd end up with that frozen requirements.txt file back on develop once the release branch merged back in. This would nominally satisfy the requirement of being able to create identical virtualenvs for any commit from that point on.

Functionally, I suspect we'll prefer to actively maintain the lockfile with the rest of our source during development, rather than only touching it when making release branches, but we'll see how things shake out.

We should not fall into the trap of only supporting the pinned version of a dependency. [...] I therefore think that any "pinned" versions should be in addition to the current "minimum" versions, where the minimum versions are also subject to a certain amount of testing (as we have presently).

@richvdh Happy to go along with that here. The standard practice, across languages, is to separate declared minimums from locked versions (setup.py vs requirements.txt, package.json vs package-lock.json, Cargo.toml vs Cargo.lock, Gemfile vs Gemfile.lock, etc.), so we'll be able to do that.

The question of what our minimums should be (or when to raise them) is a separate question, and one we already face regardless of this proposal.

@clokep

my understanding was that it was important to be able to create an identical virtualenv for any release of Synapse, not any commit.

@clokep: Is this a distinction without a difference? Even if we were only dumping pip freeze into the first commit of a release branch, we'd end up with that frozen requirements.txt file back on develop once the release branch merged back in. This would nominally satisfy the requirement of being able to create identical virtualenvs for any commit from that point on.

It is different, yes. There's an assumption being made that develop should have this on it. I'm unsure if I agree with that.

Functionally, I suspect we'll prefer to actively maintain the lockfile with the rest of our source during development, rather than only touching it when making release branches, but we'll see how things shake out.

Perhaps, but that doesn't seem to be one of the requirements.

Perhaps, but that doesn't seem to be one of the requirements.

Let's take the requirement as stated: any commit, not just releases, must have enough information (a lockfile) to reliably recreate identical virtualenvs.

This is a slight leap, but one I believe is inevitable as maintaining a lockfile on the main branch is a prerequisite for availing of Dependabot's features, which are genuinely compelling.

Enabling repeatable builds for arbitrary commits is also necessary for continuous delivery, but that's a distinct concern best left for the future.

There's general consensus that we're fine going ahead with this. The question is how. Next month we'll pick a ~~victim~~ volunteer to research Poetry, Pipenv, and pip-tools and make a recommendation.

We'd likely benefit from proactively simplifying setup.py and synapse/python_dependencies.py in advance as Poetry replaces setuptools entirely, while pip-tools and Pipenv overlap with it.

(Braindump) A few questions to ask of a given solution:

How does it handle locking dependencies with environment markers like when we specify psycopg2>=2.8;platform_python_implementation!='PyPy'?
- Are those dependencies resolved and present in the lockfile even if they're not currently installed or wouldn't be installed in the current environment?
  This is important if we have conditional dependencies (including indirect ones!) since their absence would mean our lockfile wasn't complete and deterministic.
What about groups like lint, dev, etc?
Does it help us know whether we're up to date with upstream releases?
How easy is it to determine where a transitive dependency is coming from?
Is the lockfile reasonably legible / intelligible? What about the diffs when it gets updated?

Also worth noting that PyCharm supports both Pipenv and Poetry

(PDM and Conda are also in the packaging space, but still somewhat esoteric / specialized)

Other writings on the topic of Pipenv vs Poetry:

https://venthur.de/2021-06-26-python-packaging.html
https://dev.to/frostming/a-review-pipenv-vs-poetry-vs-pdm-39b4 by the author of PDM
https://remastr.com/blog/pip-pipenv-poetry-comparison --- didn't find much useful info here
https://www.reddit.com/r/Python/comments/limd9t/poetry_vs_pipenv_vs_piptools_what_do_you_use/
https://johnfraney.ca/posts/2019/11/19/pipenv-poetry-benchmarks-ergonomics-2/
https://news.ycombinator.com/item?id=26093926 which discusses https://muttdata.ai/blog/2020/08/21/a-poetic-apology.html
https://jacobian.org/2019/nov/11/python-environment-2020/
https://news.ycombinator.com/item?id=27559655
https://www.activestate.com/blog/which-python-dependency-manager-should-i-choose/ but this is probably an avert for activestate's offering
- mentions Poetry doesn't handle transitive dependencies correctly if they are Python version dependant python-poetry/poetry#2405 and Infinite dependency loop for azure-core python-poetry/poetry#4054
- of pipenv, claims "reproducibility by using Pipfile.lock" yet also claims
  
  The pipenv lock command does not capture dependencies of Python wheels provided in a Pipfile. This can result in non-deterministic builds in which each run may not pull in the same dependencies, causing inconsistencies between runtime environments.
https://twitter.com/hynek/status/1068144037507592193
https://chriswarrick.com/blog/2018/07/17/pipenv-promises-a-lot-delivers-very-little/ pulls no punches

While reading around I also came across pyflow, micropipenv and pip-tools.

Frankly I feel paralysed by too much choice. I'll try to see if I can summarise this into something digestible.

Frankly I feel paralysed by too much choice. I'll try to see if I can summarise this into something digestible.

If integrating with GitHub's Depenabot security stuff is a hard requirement (and it might as well be), our only choices are Poetry, Pipenv, and pip-tools. I'd recommend stepping back from others' opinions and blog posts and just diving in on your own:

Package up a Hello World example with each tool.
Then try packaging a smaller library like signedjson.
Then try Sydent or Sygnal.

Don't worry about CI or anything... just get a feel for how each tool works, how its docs are, etc.

After, go and try to answer the questions from this comment for each.

Nuke the tox `packaging` job

Remove `dockerfile-pgtests`

Merge tag 'v1.58.0rc1' into babolivier/dinsic_1.58

Merge tag 'v1.58.0' into babolivier/dinsic_1.58

Merge tag 'v1.57.0' into develop

This partially made its way into Synapse 1.57 (which had a parallel pyproject.toml/poetry.lock and setup.py). Synapse 1.58 has removed the setup.py completely. I'm going to close this issue as done and file new ones for any final fixups.

See also the project https://github.com/orgs/matrix-org/projects/54.

chat/matrix-synapse: Update to 1.58.1

Just tried to build v1.61.0 on Linux and Windows and both fail.

I know this worked a few weeks ago and I basically think that OP was right with the version pinning.

Motivation: I want to bisect and thus build v1.61.0 .. v1.62.0 for identifying a patch which destroyed the generate call for us (I'll open a new ticket for that one).

Here is the error:

git checkout tags/v1.61.0
DOCKER_BUILDKIT=1 docker build -f docker/Dockerfile -t fred .
[+] Building 76.9s (14/25)
 => [internal] load build definition from Dockerfile                                                                                                                                                                                    0.1s
 => => transferring dockerfile: 5.28kB                                                                                                                                                                                                  0.0s
 => [internal] load .dockerignore                                                                                                                                                                                                       0.1s
 => => transferring context: 170B                                                                                                                                                                                                       0.0s
 => resolve image config for docker.io/docker/dockerfile:1                                                                                                                                                                              1.3s
 => [auth] docker/dockerfile:pull token for registry-1.docker.io                                                                                                                                                                        0.0s
 => docker-image://docker.io/docker/dockerfile:1@sha256:9ba7531bd80fb0a858632727cf7a112fbfd19b17e94c4e84ced81e24ef1a0dbc                                                                                                                1.0s
 => => resolve docker.io/docker/dockerfile:1@sha256:9ba7531bd80fb0a858632727cf7a112fbfd19b17e94c4e84ced81e24ef1a0dbc                                                                                                                    0.0s
 => => sha256:9ba7531bd80fb0a858632727cf7a112fbfd19b17e94c4e84ced81e24ef1a0dbc 2.00kB / 2.00kB                                                                                                                                          0.0s
 => => sha256:ad87fb03593d1b71f9a1cfc1406c4aafcb253b1dabebf569768d6e6166836f34 528B / 528B                                                                                                                                              0.0s
 => => sha256:1e8a16826fd1c80a63fa6817a9c7284c94e40cded14a9b0d0d3722356efa47bd 2.37kB / 2.37kB                                                                                                                                          0.0s
 => => sha256:1328b32c40fca9bcf9d70d8eccb72eb873d1124d72dadce04db8badbe7b08546 9.94MB / 9.94MB                                                                                                                                          0.7s
 => => extracting sha256:1328b32c40fca9bcf9d70d8eccb72eb873d1124d72dadce04db8badbe7b08546                                                                                                                                               0.2s
 => [internal] load build definition from Dockerfile                                                                                                                                                                                    0.0s
 => [internal] load .dockerignore                                                                                                                                                                                                       0.0s
 => [internal] load metadata for docker.io/library/python:3.9-slim                                                                                                                                                                      0.0s
 => [internal] load build context                                                                                                                                                                                                       0.3s
 => => transferring context: 7.41MB                                                                                                                                                                                                     0.2s
 => [stage-2 1/5] FROM docker.io/library/python:3.9-slim                                                                                                                                                                                0.2s
 => [builder 2/7] RUN    --mount=type=cache,target=/var/cache/apt,sharing=locked    --mount=type=cache,target=/var/lib/apt,sharing=locked  apt-get update && apt-get install -y     build-essential     libffi-dev     libjpeg-dev     51.4s
 => [stage-2 2/5] RUN    --mount=type=cache,target=/var/cache/apt,sharing=locked    --mount=type=cache,target=/var/lib/apt,sharing=locked   apt-get update && apt-get install -y     curl     gosu     libjpeg62-turbo     libpq5      70.1s
 => [requirements 2/6] RUN    --mount=type=cache,target=/var/cache/apt,sharing=locked    --mount=type=cache,target=/var/lib/apt,sharing=locked  apt-get update && apt-get install -y git     && rm -rf /var/lib/apt/lists/*            60.6s
 => ERROR [requirements 3/6] RUN --mount=type=cache,target=/root/.cache/pip   pip install --user "poetry-core==1.1.0a7" "git+https://github.com/python-poetry/poetry.git@fb13b3a676f476177f7937ffa480ee5cff9a90a5"                     13.0s
------
 > [requirements 3/6] RUN --mount=type=cache,target=/root/.cache/pip   pip install --user "poetry-core==1.1.0a7" "git+https://github.com/python-poetry/poetry.git@fb13b3a676f476177f7937ffa480ee5cff9a90a5":
#13 3.856 Collecting git+https://github.com/python-poetry/poetry.git@fb13b3a676f476177f7937ffa480ee5cff9a90a5
#13 3.858   Cloning https://github.com/python-poetry/poetry.git (to revision fb13b3a676f476177f7937ffa480ee5cff9a90a5) to /tmp/pip-req-build-tufn8q4p
#13 3.866   Running command git clone --filter=blob:none --quiet https://github.com/python-poetry/poetry.git /tmp/pip-req-build-tufn8q4p
#13 7.242   Running command git rev-parse -q --verify 'sha^fb13b3a676f476177f7937ffa480ee5cff9a90a5'
#13 7.250   Running command git fetch -q https://github.com/python-poetry/poetry.git fb13b3a676f476177f7937ffa480ee5cff9a90a5
#13 7.839   Running command git checkout -q fb13b3a676f476177f7937ffa480ee5cff9a90a5
#13 9.253   Resolved https://github.com/python-poetry/poetry.git to commit fb13b3a676f476177f7937ffa480ee5cff9a90a5
#13 9.720   Installing build dependencies: started
#13 12.36   Installing build dependencies: finished with status 'done'
#13 12.37   Getting requirements to build wheel: started
#13 12.46   Getting requirements to build wheel: finished with status 'done'
#13 12.46   Preparing metadata (pyproject.toml): started
#13 12.77   Preparing metadata (pyproject.toml): finished with status 'error'
#13 12.77   error: subprocess-exited-with-error
#13 12.77
#13 12.77   × Preparing metadata (pyproject.toml) did not run successfully.
#13 12.77   │ exit code: 1
#13 12.77   ╰─> [26 lines of output]
#13 12.77       Traceback (most recent call last):
#13 12.77         File "/usr/local/lib/python3.9/site-packages/pip/_vendor/pep517/in_process/_in_process.py", line 363, in <module>
#13 12.77           main()
#13 12.77         File "/usr/local/lib/python3.9/site-packages/pip/_vendor/pep517/in_process/_in_process.py", line 345, in main
#13 12.77           json_out['return_val'] = hook(**hook_input['kwargs'])
#13 12.77         File "/usr/local/lib/python3.9/site-packages/pip/_vendor/pep517/in_process/_in_process.py", line 164, in prepare_metadata_for_build_wheel
#13 12.77           return hook(metadata_directory, config_settings)
#13 12.77         File "/tmp/pip-build-env-yp6fwb9j/overlay/lib/python3.9/site-packages/poetry/core/masonry/api.py", line 40, in prepare_metadata_for_build_wheel
#13 12.77           poetry = Factory().create_poetry(Path(".").resolve(), with_groups=False)
#13 12.77         File "/tmp/pip-build-env-yp6fwb9j/overlay/lib/python3.9/site-packages/poetry/core/factory.py", line 62, in create_poetry
#13 12.77           package = self.configure_package(
#13 12.77         File "/tmp/pip-build-env-yp6fwb9j/overlay/lib/python3.9/site-packages/poetry/core/factory.py", line 156, in configure_package
#13 12.77           cls._add_package_group_dependencies(
#13 12.77         File "/tmp/pip-build-env-yp6fwb9j/overlay/lib/python3.9/site-packages/poetry/core/factory.py", line 102, in _add_package_group_dependencies
#13 12.77           cls.create_dependency(
#13 12.77         File "/tmp/pip-build-env-yp6fwb9j/overlay/lib/python3.9/site-packages/poetry/core/factory.py", line 366, in create_dependency
#13 12.77           dependency = Dependency(name, constraint, groups=groups)
#13 12.77         File "/tmp/pip-build-env-yp6fwb9j/overlay/lib/python3.9/site-packages/poetry/core/packages/dependency.py", line 66, in __init__
#13 12.77           self.constraint = constraint  # type: ignore[assignment]
#13 12.77         File "/tmp/pip-build-env-yp6fwb9j/overlay/lib/python3.9/site-packages/poetry/core/packages/dependency.py", line 110, in constraint
#13 12.77           self._constraint = parse_constraint(constraint)
#13 12.77         File "/tmp/pip-build-env-yp6fwb9j/overlay/lib/python3.9/site-packages/poetry/core/semver/helpers.py", line 31, in parse_constraint
#13 12.77           constraint_objects.append(parse_single_constraint(constraint))
#13 12.77         File "/tmp/pip-build-env-yp6fwb9j/overlay/lib/python3.9/site-packages/poetry/core/semver/helpers.py", line 147, in parse_single_constraint
#13 12.77           raise ParseConstraintError(f"Could not parse version constraint: {constraint}")
#13 12.77       poetry.core.semver.exceptions.ParseConstraintError: Could not parse version constraint: (>=20.4.3
#13 12.77       [end of output]
#13 12.77
#13 12.77   note: This error originates from a subprocess, and is likely not a problem with pip.
#13 12.78 error: metadata-generation-failed
#13 12.78
#13 12.78 × Encountered error while generating package metadata.
#13 12.78 ╰─> See above for output.
#13 12.78
#13 12.78 note: This is an issue with the package mentioned above, not pip.
#13 12.78 hint: See above for details.
#13 12.79 WARNING: You are using pip version 22.0.4; however, version 22.2.2 is available.
#13 12.79 You should consider upgrading via the '/usr/local/bin/python -m pip install --upgrade pip' command.
------
executor failed running [/bin/sh -c pip install --user "poetry-core==1.1.0a7" "git+https://github.com/python-poetry/poetry.git@fb13b3a676f476177f7937ffa480ee5cff9a90a5"]: exit code: 1

qknight's issue was pulled out to #13849.

For completeness: there was some discussion about how to get tox and poetry to interact, starting roughly #11537 (comment)

I note that Poetry's docs (now?) has a section on tox configurations. TL;DR: if you want to get tox to install locked versions, you have to get it to invoke poetry.

Navigation Menu

Repository navigation

Lock Dependencies in Synapse #11537

Definition of Done

Further Discussion / Constraints

35 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Lock Dependencies in Synapse #11537

Description

Definition of Done

Further Discussion / Constraints

Activity

richvdh commented on Dec 8, 2021

callahad commented on Dec 8, 2021

clokep commented on Dec 8, 2021

richvdh commented on Dec 8, 2021

callahad commented on Dec 8, 2021

clokep commented on Dec 8, 2021

callahad commented on Dec 8, 2021

callahad commented on Dec 9, 2021

callahad commented on Dec 9, 2021

callahad commented on Dec 14, 2021

DMRobertson commented on Jan 12, 2022

callahad commented on Jan 12, 2022

35 remaining items

DMRobertson commented on May 5, 2022

qknight commented on Sep 20, 2022

DMRobertson commented on Feb 6, 2023

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions