search

snipe / snipe-it

A free open source IT asset/license management system
https://snipeitapp.com
GNU Affero General Public License v3.0
11.03k stars 3.18k forks source link

Please support MySQL 8+ (and upgrade to Laravel 5.7) #6701

Closed lieszkol closed 5 years ago

lieszkol commented 5 years ago

Your requirements documentation states "MySQL or MariaDB" - so I went ahead and tried to install on my server with MySQL 8.0.14

I'd like to

  1. Request that you upgrade snipe-it to support MySQL 8, since this necessitates upgrading to Laravel 5.6+, I'd recommend you make the effort and update to 5.7 and update all of your dependencies (which are getting outdated)
  2. Share my experiences doing #1 above

Environment:

I went ahead and spent a few hours getting SnipeIT to work in the above environment, and it IS working...mostly. But of course you will have to do way more than I to do a proper upgrade. Nonetheless, I wanted to share my experiences, it may help you in your own migration path.

DISCLAIMER: I don't recommend anyone to actually follow what I describe below, I would just like to share it in case it might be useful for the SnipeIT team when doing the actual upgrade. They will have to follow these steps and more for sure! Also, I'm a YII guy and this is my first time diving into Laravel so I'm sure I'm making some mistakes.

So I installed snipe-it and got going with /setup/migrate until I hit my first issue:

Issue no. 1: unescaped reserved word table name "groups" in migration script

This is described originally in issue #6023 and #6181

Syntax error or access violation: 1064 You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'groups set permissions = ? where id = ?' at line 1 (SQL: update groups set permissions = {"admin":1,"users":1,"reports":1} where id = 1)

Caused by This upgrade script: /database/migrations/2014_11_04_231416_update_group_field_for_reporting.php Includes two lines like:

DB::update('update '.DB::getTablePrefix().'groups set permissions = ? where id = ?', ['{"admin":1,"users":1,"reports":1}', 1]); DB::update('update '.DB::getTablePrefix().'groups set permissions = ? where id = ?', ['{"users":1,"reports":1}', 2]);

However "groups" has become a reserved word in MySQL 8+ So it needs to be properly escaped.

Solution Update the script to use Laravel's query builder instead of running a raw SQL query. The query builder will properly escape your table names for you.

So replace the above with:

DB::table(DB::getTablePrefix() . 'groups') ->where('id', 1) ->update(['permissions' => '{"admin":1,"users":1,"reports":1}']); DB::table(DB::getTablePrefix() . 'groups') ->where('id', 2) ->update(['permissions' => '{"users":1,"reports":1}']);

Issue no. 2: Laravel 5.5 does not support changed MySQL schema

So now you will hit this issue:

Undefined property: stdClass::$column_name ...in MySqlProcessor.php line 16 ...in 2016_02_19_070119_add_remember_token_to_users_table.php line 15

Caused by Starting MySQL 8, information_schema.tables has field names in uppercase, whereas Laravel 5.4 expects this field to be lowercase. This has been reported here and the solution was implemented in Laravel 5.5. So basically you could create a temporary hack to resolve this by automatically patching the relevant file in /vendor or you could just do the right thing and migrate to the latest stable Laravel version.

Solution Update to Laravel 5.5+

Issues with upgrading to Laravel 5.7

So of course you can't just update one component in composer.json. To get snipe-it working I had to update the following lines:

"barryvdh/laravel-debugbar": "^2.4", >> "^3.2.2" "fideloper/proxy": "^3.3", >> "^4.0" "laravel/passport": "^3.0", >> "^7.0" "spatie/laravel-backup": "3.11.0", >> "^6.0.0" "codeception/codeception": "2.3.6", >> "2.5.3"

Additionally, I had to remove this part from composer.json:

"platform": { "php": "5.6.4" }

Undefined class constant 'HEADER_CLIENT_IP' So first you will get the dreaded "Class does not exist" error! Follow the instructions in the link to get to the original exception and you will see that the actual issue is this:

Undefined class constant 'HEADER_CLIENT_IP'

You will get this even if you try a sudo php artisan config:clear.

Similar issues have been dealt with here and here, of course the proper solution when doing an upgrade would be to follow the official upgrade guide unfortunately they do not go into much detail.

The actual problem is caused by a change in how trusted proxies need to be configured. More info on that in the official docs.

Solution Migrate your trusted proxy configuration as follows:

Delete /config/trustedproxy.php Upload the following to /app/Http/Middleware/TrustProxies.php:

namespace App\Http\Middleware; use Illuminate\Http\Request; use Fideloper\Proxy\TrustProxies as Middleware; class TrustProxies extends Middleware { protected $proxies = env('APP_TRUSTED_PROXIES') !== null ? explode(',', env('APP_TRUSTED_PROXIES')) : '*'; protected $headers = Request::HEADER_X_FORWARDED_ALL; }

Call to undefined method Monolog\Logger::getMonolog() You will hit this is you even try to do a sudo php artisan config:clear.

sudo php artisan config:clear PHP Fatal error: Uncaught Error: Class 'Log' not found in .../app/Exceptions/Handler.php:39 Stack trace:

0 .../vendor/laravel/framework/src/Illuminate/Foundation/Console/Kernel.php(353): App\Exceptions\Handler->report(Object(Symfony\Component\Debug\Exception\FatalThrowableError))

The issue is simple, getMonolog() needs to be replaced with Log::getLogger() as described here. Or read more about logging in the official docs.

The error is coming from the following file, however it may be worth looging over all of your files: /app/Providers/AppServiceProvider.php Change:

$monolog = Log::getMonolog();

To:

$monolog = Log::getLogger();

laravel Undefined index: App\Models\CustomField Reload the page and migration continues, until it hits the next error:

at HandleExceptions->handleError(8, 'Undefined index: App\Models\CustomField', '/var/www/assets.tipton/vendor/laravel/framework/src/Illuminate/Database/Eloquent/Model.php', 242, array()) in Model.php line 242 ..in 2016_03_22_125911_fix_custom_fields_regexes.php line 15

This is because Laravel 5.7 initializeTraits breaks models that override the boot method

Solution Call parent::boot(); anywhere where you extend Eloquent\Model! The specific error can be overcome by fixing /app/Models/CustomField.php: Old:

public static function boot() { self::created(function ($custom_field) {

New:

public static function boot() { parent::boot(); self::created(function ($custom_field) {

App\Models\CustomField::rules must return a relationship instance. So now if you retry the db migration, you get stuck with this error:

App\Models\CustomField::rules must return a relationship instance. ...in 2016_03_22_125911_fix_custom_fields_regexes.php line 34

I forget how I got to the solution :-( in any case this seems to work: update /app/Models/CustomField.php: as follows: just add the following anywhere: (too many colons, I know):

protected $rules = [ ];

!! I am not too comfortable with this as there already is a public function validationRules() further down in that class but it does get us moving.

Table 'tipton_assets.oauth_clients' doesn't exist

SQLSTATE[42S02]: Base table or view not found: 1146 Table 'tipton_assets.oauth_clients' doesn't exist (SQL: insert into oauth_clients (user_id, name, secret, redirect, personal_access_client, password_client, revoked, updated_at, created_at) values (, Snipe-IT Personal Access Client, nRsiu0hDj6jAoFmDQZ8GBn2lXNe5NyUY8eoeqYz1, http://localhost, 1, 0, 0, 2019-02-09 11:08:10, 2019-02-09 11:08:10))

Luckily this can be overcome by running sudo php artisan migrate as [described here] (https://github.com/snipe/snipe-it/issues/4439) (this is required because we updated laravel/passport in composer.json)

Undefined variable: color We are getting close!

Undefined variable: color (View: /var/www/assets.tipton/resources/views/vendor/mail/html/button.blade.php) (View: /var/www/assets.tipton/resources/views/vendor/mail/html/button.blade.php) ...at CompilerEngine->get('/var/www/assets.tipton/resources/views/notifications/FirstAdmin.blade.php',

This now has to do with migrating to Laravel 5.7: L5.7 Undefined variable for Mail Vendor

Of course the right thing would be to properly follow the official 5.6 to 5.7 migration guide!

The solution is to replace "or" with "??" in all of the files under /resources/views/vendor/mail/html. The error is specifically coming from button.blade.php which can be fixed as follows: Old:

...$color or 'blue'...

New:

...$color ?? 'blue'...

laravel An Error has occured! Unauthenticated. So now the platform loads, and you are able to insert records, but you can't see them! If you get your browser debugger up and check the network traffic when you try to reload any table, you'll see this error. Long story short, you need to make sure to upgrade the laravel passport version in composer.php:

"laravel/passport": "^3.0", >> "^7.0"

TODO:

Well I can now use most of the platform but things are still not perfect...for example for some off reason whenever I browse to "deployed assets", the SQL being executed is this:

select assets.* from assets where assets.status_id = '' and assets.company_id = '' and assets.order_number = '' and assets.assigned_to > '0' and assets.deleted_at is null order by name asc limit 20 offset 0 700μs /app/Http/Controllers/Api/AssetsController.php:275

I've run out of time so I can't figure out why it's adding "company_id" and "order_number" as search parameters so if anyone has any ideas please share!

snipe commented 5 years ago

Per the v5 update, the upcoming version of Snipe-IT will be using Laravel 5.7 (and all dependencies are pulled forward accordingly.) You probably could have saved yourself a lot of time by reading the pinned issue, or opening up an issue asking about laravel 5.7 before spending all the time manually dragging it forward. V5 has been in development for 6 months.

lieszkol commented 5 years ago

Hmm. Well, on the one hand that's great! I'm happy development is ongoing and this great project isn't going stale. On the other hand I do feel quite silly now, and a bit resentful about your reply. Hardly makes me want to contribute to this project...not that you need any help obviously.

On the more pragmatic side, I would like to humbly offer some suggestions:

  1. You aim to let your users know of the upcoming release in a pinned issue. But github's pinned issue feature isn't particularly attention grabbing, I obviously didn't notice it. Yes, stupid me. Other forum software do a much better job at actually "pinning" issues to the top of the issues list and usually highlighting it in some way. Even now when I opened the issues list it took me a bit to find what you're talking about. My point is, you may want to put a link to this issue in your README.md so that people more easily see that changes are taking place under the hood!
  2. The first line of my post still holds valid, "Your requirements documentation states "MySQL or MariaDB" - which at this point in time is NOT correct, the current version of snipe-it supports UP TO MySQL 5.7 only, so I would recommend adding a note to your documentation page to this effect, also mentioning that the upcoming release will have MySQL 8 support! Would have saved me all the effort!
  3. When I saw that MySQL 8 wasn't working, I googled the problem. I saw that there are already two issues (#6023 and #6181) related to this, read through them, and since nowhere in those issues has anyone taken the time to explain that the current version does not support MySQL 8 but the upcoming release will, I tried to solve the problem myself. I've now added a short note to this effect to those issues in case anyone else goes down the same road as me.

End of rant :-)

Good luck with your project! I look forward to testing it all out again once the v5 is released.

breisig commented 5 years ago

MySQL 8 support is a definite requirement!