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

Upgrade Inventory to use CKAN 2.11.0 #4861

Closed
3 tasks
btylerburton opened this issue Aug 22, 2024 · 21 comments
Closed
3 tasks

Upgrade Inventory to use CKAN 2.11.0 #4861

btylerburton opened this issue Aug 22, 2024 · 21 comments
Assignees
Labels

Comments

@btylerburton
Copy link
Contributor

User Story

Upgrade Inventory to use CKAN 2.11.0

Acceptance Criteria

[ACs should be clearly demoable/verifiable whenever possible. Try specifying them using BDD.]

Sketch

[Notes or a checklist reflecting our understanding of the selected approach]

@Bagesary Bagesary moved this to 📔 Product Backlog in data.gov team board Aug 22, 2024
@btylerburton btylerburton moved this from 📔 Product Backlog to 📟 Sprint Backlog [7] in data.gov team board Oct 3, 2024
@FuhuXia FuhuXia self-assigned this Oct 8, 2024
@FuhuXia
Copy link
Member

FuhuXia commented Nov 4, 2024

Inventory Branch ckan-2-11-0 is pushed with ckan core 2.11.0 and its requirements filled in. The general functionality of Inventory are working. We can add datasets with a publisher. We can export data.json file.

Things noticed that are not working:

  • For anonymous user, /dataset page loads with empty dataset instead of a permission error.
  • Loading individual dataset page raise ckan.exceptions.HelperError: Helper 'popular' has not been defined
  • New plugin activity cant be used.
  • SAML login is not working.

@btylerburton
Copy link
Contributor Author

Tracing SAML error stack that results from any attempted login and this is the last line:

  2024-12-12T15:34:45.86-0600 [APP/PROC/WEB/0] ERR Message: Exception("The auth 'site_read' is not found for chained auth")
   2024-12-12T15:34:45.86-0600 [APP/PROC/WEB/0] ERR Arguments: ()

Then I found this in the 2.11 changelog: The site_read authz function has been removed since it always returned True. (https://github.com/ckan/ckan/pull/7544)

This is the PR: ckan/ckan#7544

@btylerburton
Copy link
Contributor Author

@btylerburton
Copy link
Contributor Author

New error related to NameID.

  2024-12-12T17:39:39.18-0600 [APP/PROC/WEB/0] ERR File "/home/vcap/deps/1/python/lib/python3.10/site-packages/itsdangerous/serializer.py", line 316, in dumps
   2024-12-12T17:39:39.18-0600 [APP/PROC/WEB/0] ERR payload = want_bytes(self.dump_payload(obj))
   2024-12-12T17:39:39.18-0600 [APP/PROC/WEB/0] ERR File "/home/vcap/deps/1/python/lib/python3.10/site-packages/itsdangerous/url_safe.py", line 56, in dump_payload
   2024-12-12T17:39:39.18-0600 [APP/PROC/WEB/0] ERR json = super().dump_payload(obj)
   2024-12-12T17:39:39.18-0600 [APP/PROC/WEB/0] ERR File "/home/vcap/deps/1/python/lib/python3.10/site-packages/itsdangerous/serializer.py", line 278, in dump_payload
   2024-12-12T17:39:39.18-0600 [APP/PROC/WEB/0] ERR return want_bytes(self.serializer.dumps(obj, **self.serializer_kwargs))
   2024-12-12T17:39:39.18-0600 [APP/PROC/WEB/0] ERR File "/home/vcap/deps/1/python/lib/python3.10/site-packages/flask/json/tag.py", line 323, in dumps
   2024-12-12T17:39:39.18-0600 [APP/PROC/WEB/0] ERR return dumps(self.tag(value), separators=(",", ":"))
   2024-12-12T17:39:39.18-0600 [APP/PROC/WEB/0] ERR File "/home/vcap/deps/1/python/lib/python3.10/site-packages/flask/json/__init__.py", line 41, in dumps
   2024-12-12T17:39:39.18-0600 [APP/PROC/WEB/0] ERR return current_app.json.dumps(obj, **kwargs)
   2024-12-12T17:39:39.18-0600 [APP/PROC/WEB/0] ERR File "/home/vcap/deps/1/python/lib/python3.10/site-packages/flask/json/provider.py", line 179, in dumps
   2024-12-12T17:39:39.18-0600 [APP/PROC/WEB/0] ERR return json.dumps(obj, **kwargs)
   2024-12-12T17:39:39.18-0600 [APP/PROC/WEB/0] ERR File "/home/vcap/deps/1/python/lib/python3.10/json/__init__.py", line 238, in dumps
   2024-12-12T17:39:39.18-0600 [APP/PROC/WEB/0] ERR **kw).encode(obj)
   2024-12-12T17:39:39.18-0600 [APP/PROC/WEB/0] ERR File "/home/vcap/deps/1/python/lib/python3.10/json/encoder.py", line 199, in encode
   2024-12-12T17:39:39.18-0600 [APP/PROC/WEB/0] ERR chunks = self.iterencode(o, _one_shot=True)
   2024-12-12T17:39:39.18-0600 [APP/PROC/WEB/0] ERR File "/home/vcap/deps/1/python/lib/python3.10/json/encoder.py", line 257, in iterencode
   2024-12-12T17:39:39.18-0600 [APP/PROC/WEB/0] ERR return _iterencode(o, 0)
   2024-12-12T17:39:39.18-0600 [APP/PROC/WEB/0] ERR File "/home/vcap/deps/1/python/lib/python3.10/site-packages/flask/json/provider.py", line 121, in _default
   2024-12-12T17:39:39.18-0600 [APP/PROC/WEB/0] ERR raise TypeError(f"Object of type {type(o).__name__} is not JSON serializable")
   2024-12-12T17:39:39.18-0600 [APP/PROC/WEB/0] ERR TypeError: Object of type NameID is not JSON serializable

This SO article points to modifications we might need to make to saml2auth to make the class serializable:

https://stackoverflow.com/questions/69234799/typeerror-object-of-type-type-is-not-json-serializable

@btylerburton
Copy link
Contributor Author

Spoke with Fuhu on Monday and we decided to unarchive the GSA's fork of saml2auth plugin, make the tests pass with CKAN 2.11 and then work on integrating it into inventory.

To that end, the tests are now configured to be run on demand here: https://github.com/GSA/ckanext-saml2auth/actions/workflows/ci.yml

On the latest run, 2.11 fails to start the tests, complaining that Plugin already registered under a different name

https://github.com/GSA/ckanext-saml2auth/actions/runs/12364216015/job/34507036045

ValueError: Plugin already registered under a different name: 
ckan.tests.pytest_ckan.ckan_setup=<module 'ckan.tests.pytest_ckan.ckan_setup' 
from '/home/runner/work/ckanext-saml2auth/ckanext-
saml2auth/ckan/ckan/tests/pytest_ckan/ckan_setup.py'>

@btylerburton
Copy link
Contributor Author

Updates:

  • I updated the gsa/saml2Auth fork to download its copy of ckan from GSA/ckan fork
  • Created 2 new branches on the GSA/ckan fork: upgrade-test-2.10, upgrade-test-2.11
  • Synched those branches to their respective release branches on ckan/ckan: https://github.com/ckan/ckan/tree/2.10, https://github.com/ckan/ckan/tree/2.11
  • Created 2 new tags on GSA/ckan to correspond to those 2 new versions: ckan-2.10, ckan-2.11
  • Ran another test and confirmed we got same result as from ckan/ckan. Tests passing in 2.10, and finding the error above on 2.11
  • https://github.com/GSA/ckanext-saml2auth/actions/runs/12422104682/job/34683142727

@btylerburton
Copy link
Contributor Author

This is the PR that breaks ckanext-saml2Auth from running 2.11.1:
ckan/ckan#8507

Removing conftest.py from the extension, as suggested, results in the --ckan-ini option not being recognized:

pytest: error: unrecognized arguments: --ckan-ini=subdir/test.ini
  inifile: None
  rootdir: /home/runner/work/ckanext-saml2auth/ckanext-saml2auth

@btylerburton
Copy link
Contributor Author

From action run: https://github.com/GSA/ckanext-saml2auth/actions/runs/12586625210/job/35080860043

  • CKAN is in correct workdir
  • CKAN sees the test.ini config file
  • CKAN 2.11.0 complains that it can't find configurations specified in config file, whereas 2.10.x has no issues
2025-01-02 18:01:29,800 WARNI [ckan.common] Option ckanext.saml2auth.idp_metadata.local_path is not declared
2025-01-02 18:01:29,800 WARNI [ckan.common] Option ckanext.saml2auth.user_firstname is not declared
2025-01-02 18:01:29,800 WARNI [ckan.common] Option ckanext.saml2auth.user_lastname is not declared
2025-01-02 18:01:29,800 WARNI [ckan.common] Option ckanext.saml2auth.user_fullname is not declared
2025-01-02 18:01:29,800 WARNI [ckan.common] Option ckanext.saml2auth.acs_endpoint is not declared

@btylerburton
Copy link
Contributor Author

Passing/Failing in 2.11

image

Posting here since turning on the -s streaming output means this isn't output.

@btylerburton
Copy link
Contributor Author

Following up on this: #4861 (comment)

More than likely, the config declaration warnings are due to the newly enforced strict mode, so just a warning and not a red flag: https://docs.ckan.org/en/2.11/maintaining/configuration.html#config-declaration

@FuhuXia FuhuXia moved this from 📡 Blocked to 🏗 In Progress [8] in data.gov team board Jan 8, 2025
@FuhuXia
Copy link
Member

FuhuXia commented Jan 8, 2025

Since ckanext-saml2auth is passing the test, giving its integration with ckan-2-11-0 another try.

@FuhuXia
Copy link
Member

FuhuXia commented Jan 8, 2025

saml2auth is working with inventory 2.11.0
image

@btylerburton btylerburton removed their assignment Jan 15, 2025
@btylerburton
Copy link
Contributor Author

Check out this branch for cypress upgrades: GSA/catalog.data.gov#1502

@FuhuXia
Copy link
Member

FuhuXia commented Jan 30, 2025

Branch https://github.com/GSA/inventory-app/tree/ckan-2-11-0 passed all tests and pushed to Inventory develop.

Doing manual QAs. Found saml2 logout issue, it returns 500 on logging out.

@FuhuXia
Copy link
Member

FuhuXia commented Jan 30, 2025

saml logout issue fixed. Pushed to inventory develop for further testing.

@FuhuXia
Copy link
Member

FuhuXia commented Feb 4, 2025

Noticed this datastore/xloader related db error when setting up a fresh instance of inventory with ckan 2.10, or when running 2.11 on existing inventory 2.10 DBs.

(psycopg2.errors.UndefinedTable) relation "_table_metadata" does not exist

The cause of it is that ckan 2.10/2.11 has updated set-permissions.sql script for datastore, but it was fine if the database was originally for ckan 2.9 and later upgraded to 2.10. The error is gone after run updated set-permissions.sql.

UPDATE:
Issue with datastore/xloader query error for is resolved by this commit:

  • use ckan db not datastore db for xloader
  • xloader needs an API token
  • update set-permissions.sql

Next, focusing on db migration and dataset/resource QA for existing datasets.

@FuhuXia
Copy link
Member

FuhuXia commented Feb 5, 2025

A fresh Inventory with ckan 2.10 was installed on development then get upgraded to 2.11. All inventory functions on the 2.11 are fine. Resources and their API results are as good as existing Inventory (ckan 2.10).

Inventory on Prod and Staging are on ckan 2.10 with 2.9 datastore DB. Cant replicate the same setup on Development. Next will upgrade staging to make sure it is fine before prod upgrade.

@FuhuXia
Copy link
Member

FuhuXia commented Feb 5, 2025

Inventory staging upgraded to 2.11 by running cf push command. Verifying.

@btylerburton
Copy link
Contributor Author

Inventory staging upgraded to 2.11 by running cf push command. Verifying.

Great news!!!!

@Bagesary
Copy link

Bagesary commented Feb 7, 2025

Deployment will be performed as planned on 2/7

@FuhuXia
Copy link
Member

FuhuXia commented Feb 7, 2025

Inventory 2.11 verified on development and staging. Release ticket created.

@FuhuXia FuhuXia closed this as completed Feb 7, 2025
@github-project-automation github-project-automation bot moved this from 🏗 In Progress [8] to ✔ Done in data.gov team board Feb 7, 2025
@hkdctol hkdctol moved this from ✔ Done to 🗄 Closed in data.gov team board Feb 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

No branches or pull requests

3 participants