user_update_10000 fails on role with no data

Problem/Motivation

The update hook user_update_10000 "Remove[s] non-existent permissions created by migrations" by loading all "user.role.*" configurations and then using array_diff() to compare each role's permissions against an array of existing permissions. However, if a role's configuration is lacking a "permissions" key, $role_config->get('permissions') returns NULL instead of an empty array, which is then passed as the first parameter to array_diff(). This results in an error that halts the processing of updates:

[error] array_diff(): Argument #1 ($array) must be of type array, null given

Presumably, all role configurations should have a "permissions" key, so the above scenario should never occur. One response to this issue could be that the update hook "works as designed" and that if you are experiencing the above problem, it's your own configuration's fault. In that case, this issue will at least serve to help others with this problem to figure out their own solution.

If we want to help site admins smooth out the Drupal 10 upgrade process, then we can add some extra input checks, and perhaps also some helpful log output.

Steps to reproduce

This is somewhat tricky because I don't know exactly how to replicate our current situation. I work on Drupal sites that are hosted by others in a large multisite configuration, where a base set of modules are managed by others, and I only deal with additional modules in our site repository. The errant role appears to be the result of an incomplete or improperly executed config update by those managing the base platform. The malconfigured role is present and impacting Drupal 10 upgrades for multiple sites that I manage, as well as sites managed by others on the same platform. Other sites with complex configuration histories may find themselves in the same situation.

You can manually replicate the problem by adding a row to the "config" table with a "name" of "user.role.my_test_role" and "data" set to "a:0:{}". This is an empty configuration, missing (among other things) a "properties" key. Run the user_update_10000 update hook, and you should get the above array_diff() error.

Proposed resolution

At a minimum, we can add a check that get('permissions') is not returning a NULL value, and if it is, instead pass an empty array to array_diff. So, instead of the current:
$removed_permissions = array_diff($role_config->get('permissions'), $existing_permissions);
you could simply add a null coalescing operator:
$removed_permissions = array_diff($role_config->get('permissions') ?? [], $existing_permissions);

This ensures array_diff() is happy, but it doesn't do anything about the errant configuration in the first place. If we wanted to be really helpful, we could add some test logic on whether $role_config->get('permissions') returns NULL and if so, provide the user with helpful information for cleanup purposes.

In my case, this errant misconfigured role doesn't seem to have been causing any problems and has gone unnoticed until this update. The role does not appear in the list of roles (/admin/people/roles). If you enable devel and use the config editor, the errant role configuration DOES appear, HOWEVER if you click on the associated "edit" button you get an error message that "Config user.role.badly_configured_role does not exist in the system." And while the role exists in the database config table, it does not get exported to config files, so there is no way to manage it with config import/export. There may be other config editing tools available, but short of that, you have to either manually edit the database or write your own update hook to clean it out.

Remaining tasks

Decide whether this warrants a fix, and if so, how much work to put into it. The proposed quick and dirty fix with the null coalescing operator would solve the problem silently, leaving the errant role in place.

Addition of error/warning output would at least alert the site administrators to problems worthy of further investigation and cleanup.

You could also remove as malformed any role missing a "permissions" key as part of the update, but that seems like a potentially hazardous road to go down unless you're absolutely certain this would have no unforeseen negative impacts.

User interface changes

None

API changes

None

Data model changes

None

Release notes snippet

Not needed?