bazaah / aur-ceph

Public workspace for ceph packages in the Archlinux AUR
https://aur.archlinux.org/pkgbase/ceph
6 stars 1 forks source link

rebuild for v18.2.2 #22

Closed bazaah closed 5 months ago

bazaah commented 6 months ago

Just released yesterday:

Nothing major, but encoder fix may be nice to have.

bazaah commented 6 months ago

The release of 18.2.2 is imminent, and should happen this weekend.

With it comes a backport of https://github.com/ceph/ceph/pull/55689, plus a forked python-bcrypt to reenable the majority of mgr modules, including the dashboard. The orch module remains broken, as it depends on python-cryptography, which cannot safely be used sub-interpreter contexts due to its rampant use of static globals.

However, I broke the mgr test suite for this release, which means check() will always fail for the ~5 mgr + dashboard test suites. Some of that isn't my fault (python-xmlsec is completely broken when trying to install from pip), but some is related to the changes I've needed to do to get a functional mgr again.

I will address these problems in a later pkgrel, as my integration testing found no issues.

bazaah commented 6 months ago

To expand on the choice to use an internal fork of python-bcrypt.

The change is tiny, basically switching off the safeguards via feature flag: git.st8l.com/luxolus/pyo3@44d6919, and is accompanied by effectively this diff on the ceph end:

Patches ```diff diff --git a/PKGBUILD b/PKGBUILD index 5989cba..4585647 100644 --- a/PKGBUILD +++ b/PKGBUILD @@ -38,12 +38,16 @@ makedepends=( 'python-pecan' 'python-prettytable' 'python-pyjwt' 'python-pyopenssl' 'python-requests' 'python-scipy' 'python-setuptools' 'python-sphinx' 'python-typing_extensions' 'python-werkzeug' 'python-yaml' + + # python-bcrypt makedepends + 'python-build' 'python-installer' 'python-setuptools-rust' 'python-wheel' ) checkdepends=( 'inetutils' 'xmlstarlet' 'python-nose' 'python-pycodestyle' 'python-pylint' 'python-pytest' 'python-pytest-cov' ) +__bcrypt_version='4.1.2' # Despite the upstream suggesting that LTO is now possible, I still am unable # to set this. I get SEGVs in tests, and repeated mentions of C++ One Definition Rule @@ -115,6 +119,15 @@ source=( # Fix a change in behavior between python 3.11.5 and 3.11.8, which prevents # importing type stub (.pyi) files directly, without a .py skeleton 'ceph-18.2.2-mgr-ceph-module-stub.patch' + + # ===== ceph-python-bcrypt sources ===== # + "python-bcrypt-${__bcrypt_version}.tar.gz::https://github.com/pyca/bcrypt/archive/${__bcrypt_version}.tar.gz" + + # Rename bcrypt -> ceph_bcrypt + 'python-bcrypt-prefix-ceph.patch' + + # Use our fork of pyo3, reenabling subinterpreter support + 'python-bcrypt-allow-subinterpreters.patch' ) ``` ```diff diff --git a/src/_bcrypt/Cargo.toml b/src/_bcrypt/Cargo.toml index a9c7f7c..02317c8 100644 --- a/src/_bcrypt/Cargo.toml +++ b/src/_bcrypt/Cargo.toml @@ -6,7 +6,7 @@ edition = "2018" publish = false [dependencies] -pyo3 = { version = "0.20.0", features = ["abi3"] } +pyo3 = { git = "https://git.st8l.com/luxolus/pyo3", tag = "v0.20.3-subint+1", features = ["abi3", "unsafe-allow-subinterpreters"] } bcrypt = "0.15" bcrypt-pbkdf = "0.10.0" base64 = "0.21.5" ``` ```diff diff --git a/pyproject.toml b/pyproject.toml index e365c8c..6e27a0d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -10,7 +10,7 @@ requires = [ build-backend = "setuptools.build_meta" [project] -name = "bcrypt" +name = "ceph_bcrypt" # When updating this, also update lib.rs version = "4.1.2" authors = [ @@ -41,7 +41,7 @@ homepage = "https://github.com/pyca/bcrypt/" [tool.setuptools] zip-safe = false package-dir = {"" = "src"} -packages = ["bcrypt"] +packages = ["ceph_bcrypt"] [tool.setuptools.dynamic] readme = {file = "README.rst", content-type = "text/x-rst"} @@ -57,7 +57,7 @@ select = ['E', 'F', 'I', 'N', 'W', 'UP', 'RUF'] line-length = 79 [tool.ruff.isort] -known-first-party = ["bcrypt", "tests"] +known-first-party = ["ceph_bcrypt", "tests"] [tool.mypy] show_error_codes = true diff --git a/setup.py b/setup.py index 13694c4..160abdd 100644 --- a/setup.py +++ b/setup.py @@ -35,7 +35,7 @@ try: setup( rust_extensions=[ RustExtension( - "bcrypt._bcrypt", + "ceph_bcrypt._bcrypt", "src/_bcrypt/Cargo.toml", py_limited_api="auto", rust_version=( diff --git a/tests/test_bcrypt.py b/tests/test_bcrypt.py index 68c00fb..0661573 100644 --- a/tests/test_bcrypt.py +++ b/tests/test_bcrypt.py @@ -1,6 +1,6 @@ import pytest -import bcrypt +import ceph_bcrypt as bcrypt _test_vectors = [ ( ```

Which is fairly easy to maintain, and high value, as otherwise the mgr is completely borked. We do have to adapt our PKGBUILD to also build the forked python-bcrypt, but that is relatively easy.


From my perspective, there was 3 ways I could go about doing this.

  1. Create an AUR package for python-ceph-bcrypt
  2. Create an internal fork of bcrypt
  3. Do nothing

Each has pros and cons.

. 1

This allows people to choose to use the package, as I could make it provides=('python-bcrypt') and advise users to install it from AUR themselves.

It would greatly simplify the build, and be more in line with KISS. However, it would also basically stray into requiring AUR packages to build ceph, which I have explicitly avoided, and its quite likely such a package would violate the AUR guidelines around similarly purposed packages.

. 3

This was my choice until now, as it was the only viable path before https://github.com/ceph/ceph/pull/55689 and seemingly only broke a single part of the software stack. However, picking this result in a broken dashboard for months.

Unfortunately, it appears bcrypt is required by a common module all other mgr modules import, effectively rendering all python originating functionality dead; including a lot of core functions. SO we can no longer realistically take this path.

. 2

The least intrusive for consumers, we effectively patch the mgr to instead use ceph_bcrypt which is our separately built fork of python-bcrypt.

This is good because it effectively is invisible to most consumers and I can remove it later just as invisibly. However, there are several major drawbacks:

  1. bcrypt is security related software, and if there is a vulnerability in it, we will not directly inherit the fix from Archlinux's build
  2. The cadence for ceph releases is slower than bcrypt, and we do rely on the system bcrypt, meaning that a major soname bump could cause issues
  3. It is yet another thing I need to watch and trigger rebuilds for, adding to the maintenance load of this package
  4. Currently we rely on the fact that python-bcrypt is well written and does not contain any static globals with python objects (it doesn't use globals at all -- nice!), but this may not remain true going forward, we'll need to check before we up the version

Overall, I think my choice was the best of the options, but still; there are some issues with it.

bazaah commented 5 months ago

Closed in https://github.com/bazaah/aur-ceph/commit/b9a598f37529ebfd98fd5a0ead5a66518626acb4