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

Fix check_remote task revision resolution #1995

Merged
merged 1 commit into from
Feb 1, 2020
Merged

Conversation

jonom
Copy link
Contributor

@jonom jonom commented Jan 31, 2020

I noticed some other issues with this task so have done my best to tidy it up. I'm not a git expert but I've tested my changes and think there are some solid improvements here.

  • Support branch & tag if specified
  • Support shortened commit hashes
  • Skip on first deployment
  • Get true current ref from git
Q A
Bug fix? Yes
New feature? No
BC breaks? No
Deprecations? No
Fixed tickets #1994

@jonom jonom force-pushed the fixes branch 2 times, most recently from e168608 to 8c4d2b8 Compare January 31, 2020 22:06
* Support branch & tag if specified
* Support shortened commit hashes
* Skip on first deployment
* Get true current ref from git

Fixes deployphp#1994
@antonmedv antonmedv merged commit c0bf1d7 into deployphp:master Feb 1, 2020
@cemalkilic
Copy link

Hey @jonom,
Thanks for fixing the check_remote task. But there is one thing I noticed.

What about running the git remote-ls command on the remote machine instead of running it locally? The line I mention is here.

Because on my production server(s) I use custom host name for GitHub and I set the repository setting of deployer according to it(let's say instead of [email protected]:owner-repo, I use [email protected]:owner-repo). Trying to reach that host on the local machine fails, obviously.

What do you think about it? Does it make any harm you think of?

I'd like to open a PR for the fix, if it is OK :)

Thanks in advance.

@jonom
Copy link
Contributor Author

jonom commented Apr 6, 2020

Hi @cemalkilic, running that command remotely was tagged as a bug: #1755 (comment) I think for me, changing runLocally to run would break this function, because my production server doesn't have access to my repo, I only have access from my dev machine and deploy is done through agent forwarding. Don't ask me how that works... 🤷‍♂ but it looks like agent forwarding is turned on by default, so I assume you're explicitly disabling it? I wonder if we need to do a check here to see whether forwardAgent is true, if so then use runLocally, otherwise use run?

@cemalkilic
Copy link

Hello again @jonom,

Yes, as you said agent forwarding does all the job in the production server including cloning the repo even if your production server does not have access the repo. At its simplest, agent forwarding simply uses the ssh keys (in the remote server) from your local machine when it is enabled.

So it means if you can git clone a repo (which means you have access) in the production server, then you should be able to also run git ls-remote without any issues.

Things are a bit more complicated on my side, I use a virtual machine for all PHP-related ops but not for git.

@jonom
Copy link
Contributor Author

jonom commented Apr 6, 2020

So it means if you can git clone a repo (which means you have access) in the production server, then you should be able to also run git ls-remote without any issues.

That's the problem though, on my (average?) setup, the production server doesn't have access to the repo. I've just tried swapping runLocally for run and it fails with:

Host key verification failed. fatal: Could not read from remote repository. Please make sure you have the correct access rights and the repository exists.

So presumably run doesn't use agent forwarding. Maybe there is a different command or a setting to make run execute with agent forwarding?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants