From 38423ed88373b909b864fc0bdf5d7268137b242e Mon Sep 17 00:00:00 2001 From: "James E. Blair" Date: Mon, 15 May 2023 14:18:23 -0700 Subject: Fix JWT auth test and alembic migration A new version of pyjwt was released which alters an internal call it makes to urllib which we have mocked in the unit tests. Our mock must be updated to match. The previous version would call urlopen with a string url argument, newer versions call it with a Request object (urllib accepts both). This change updates the mock to accept both. There has also been a recent alembic release which alters the function signature of alter_column so that most arguments are now required to be keyword arguments. This causes one of our migrations to fail since it was not passing the new type in as a positional argument. Notably, the type was not the next positional argument in the signature ("nullable" is), so this explains why we had two "change patchset to string" migrations: the first one did not actually work. This change alters the migration to do what it effectively did rather that what it was intended to do. The later migration continues to correct the error. Change-Id: I10cdffa43147f7f72e431e7c64e10e9416dfb295 --- tests/unit/test_auth.py | 3 +++ .../alembic/versions/cfc0dc45f341_change_patchset_to_string.py | 10 +++++++++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/tests/unit/test_auth.py b/tests/unit/test_auth.py index 95ccdb4a2..6d35d761e 100644 --- a/tests/unit/test_auth.py +++ b/tests/unit/test_auth.py @@ -70,6 +70,9 @@ def mock_get(url, params=None, **kwargs): def mock_urlopen(url, *args, **kwargs): + if hasattr(url, 'full_url'): + # Like a urllib.Request object + url = url.full_url if url == ("https://my.oidc.provider/auth/realms/realm-one/" "protocol/openid-connect/certs"): io = StringIO() diff --git a/zuul/driver/sql/alembic/versions/cfc0dc45f341_change_patchset_to_string.py b/zuul/driver/sql/alembic/versions/cfc0dc45f341_change_patchset_to_string.py index 3fde8e545..e8226837b 100644 --- a/zuul/driver/sql/alembic/versions/cfc0dc45f341_change_patchset_to_string.py +++ b/zuul/driver/sql/alembic/versions/cfc0dc45f341_change_patchset_to_string.py @@ -17,11 +17,19 @@ import sqlalchemy as sa BUILDSET_TABLE = 'zuul_buildset' +# Note: the following migration does not do what it was intended to do +# (change the column type to a string). It was introduced with an +# error which instead caused the nullable flag to be set to true (even +# though it was already set), effectively making it a no-op. To +# maintain compatibility with later versions of alembic, this has been +# updated to explicitly do now what it implicitly did then. A later +# migration (19d3a3ebfe1d) corrects the error and changes the type. + def upgrade(table_prefix=''): op.alter_column(table_prefix + BUILDSET_TABLE, 'patchset', - sa.String(255), + nullable=True, existing_nullable=True, existing_type=sa.Integer) -- cgit v1.2.1