search

deployphp / deployer

The PHP deployment tool with support for popular frameworks out of the box
https://deployer.org
MIT License
10.62k stars 1.48k forks source link

Failed first deployment using Laravel #2733

Closed lorisleiva closed 3 years ago

lorisleiva commented 3 years ago

Hi there 👋

I've just installed Deployer on a new project and I get the following error when deploying for the first time.

❯ dep deploy
[prod] info deploying HEAD
task deploy:setup
task deploy:lock
task deploy:release
task deploy:update_code
task deploy:check_env
task deploy:shared
task deploy:writable
task deploy:vendors
task artisan:storage:link
task artisan:view:cache
task artisan:config:cache
task artisan:migrate
[prod]  error  in laravel.php on line 70:
[prod] run /usr/bin/php ~/proctorlink.proctorwealthassociates.com/releases/1/artisan migrate --force
[prod] In Connection.php line 703:
[prod] SQLSTATE[HY000] [1045] Access denied for user 'homestead'@'localhost' (usin
[prod] g password: YES) (SQL: select * from information_schema.tables where table_
[prod] schema = homestead and table_name = migrations and table_type = 'BASE TABLE
[prod] ')
[prod] In Connector.php line 70:
[prod] SQLSTATE[HY000] [1045] Access denied for user 'homestead'@'localhost' (usin
[prod] g password: YES)
[prod] exit code 1 (General error)
task deploy:failed
task deploy:unlock

This is because, when deploying initially, there is no production .env file to use and therefore it's expected to fail.

Prior to the latest version, we fixed this by using the skipIfNoEnv option on the artisan:migrate task.

https://github.com/deployphp/deployer/blob/20ca4718f2065c30b54a0b943112d50349141763/recipe/laravel.php#L112

This would allow us to create an empty .env and give us the chance to update it before running dep deploy one more time.

However, I've noticed we've recently added a more automatic way to handle the missing .env file by copying it from .env.example (using the deploy:check_env task). Whilst this is a good practice in a local machine, I don't think it is valuable in a production environment and actually breaks the flow on your first deployment by not allowing you to deploy at all.

Moreover, I don't see any use case where developers would set up their production environment variables and secrets in a .env.example file and commit this to their repository as this would be a big no-no security-wise.

In conclusion, I think it's okay to offer the new deploy:check_env task but I don't think it's a good idea to use it by default using a before hook.

https://github.com/deployphp/deployer/blob/20ca4718f2065c30b54a0b943112d50349141763/recipe/laravel.php#L237

What do you think? I'm happy to submit a PR if you agree.

Schrank commented 3 years ago

I have the same problem with shopware and Magento and just go with it 🤷 first deployment fails.

Although my dream is to setup all secrets, credentials and hosts as gitlab/bitbucket/github action/pipeline secrets and have a task which creates the .env/whatever config is needed.

I hope this answer is at least a little helpful, even if pretty offtopic 🙈

antonmedv commented 3 years ago

I don't think it is valuable in a production environment and actually breaks the flow on your first deployment by not allowing you to deploy at all.

Yes, I agree. Let's now remove those tasks.

antonmedv commented 3 years ago

Will add a doc about missing .env file and how to create it.

antonmedv commented 3 years ago

Or maybe some general task for creating .env files? For example a task what will copy local .env.example and upload it to remote host?

lorisleiva commented 3 years ago

Hmm maybe. I guess you'd be calling this "Copy local env" task after a first deploy anyway right? Otherwise, you wouldn't have the structure in place to paste it there.

Do you want me to make a PR tomorrow? If so, should I remove both the hooks and the tasks for the laravel, magento and shopware recipes?

antonmedv commented 3 years ago

I already removed deploy:check_env from laravel recipe.

For creating .env in the shared folder we don't need anything.

antonmedv commented 3 years ago

BTW, Should 'artisan:migrate' be included by default to Laravel deploy?

I just created a new Laravel + Jetstram app. Was able to configure VPS and deploy it in ~5min.

lorisleiva commented 3 years ago

Nice! Yeah it 100% should. It's even in the deploy script of both Forge and Ploi so I think it's expected to be there by default.

Btw I saw you managed to provision your servers in a way that does need php fpm reloading. How is it working being the scenes? 🙂

antonmedv commented 3 years ago

How is it working being the scenes?

I’m going to write a doc about it later today 😀

And yes, dep provision setups webserver correctly and no reloading is needed.

lorisleiva commented 3 years ago

Just closing this for housekeeping since it's been fixed and deployed. Thank you! ❤️