caprover / caprover-cli

Command Line Interface for https://github.com/caprover/caprover
72 stars 39 forks source link

caprover deploy: Cannot find hash of last commit on branch "master" caused by git as root error #122

Closed Ziip-dev closed 2 years ago

Ziip-dev commented 2 years ago

Hello there, and thanks a ton for caprover :)

I have found the other issues related to "cannot find hash of last commit" but none of what I have tried so far could solve my issue.

At this point I cannot deploy at all using caprover deploy, I get this same error message even with a fresh new test sample app. However, I could successfully send my app to the server using the tarball mechanism caprover deploy -t app.tar Here what I get server-side afterward: image

Deployment used to work last year when I last tried it, and git rev-parse master returns the hash of my last commit. Current versions: caprover-cli v2.2.3 and caprover server v1.10.1

I guess there is an issue on my side, but I don't know what to look for anymore

githubsaturn commented 2 years ago

Are you on windows, Mac or Linux?

Ziip-dev commented 2 years ago

Sorry did not mention it... Linux, Ubuntu 18.04

githubsaturn commented 2 years ago

What are these outputs?

node -v
npm -v
git --version

Also, can you please screenshot the error that you're getting when you run caprover deploy ?

Ziip-dev commented 2 years ago
> node -v
v14.19.1

> npm -v
8.10.0

> git --version
git version 2.36.0

And here is the screenshot of the error: image

githubsaturn commented 2 years ago

Here is the function that throws that error.

https://github.com/caprover/caprover-cli/blob/cf4f970ef897de684bd10ac56dd46f6d18d63c41/src/utils/ValidationsHandler.ts#L166-L182

Are you sure that you're actually on branch master?

What does this print?

git rev-parse master 2>/dev/null || echo "did not work"
Ziip-dev commented 2 years ago

Yep I found it already but that did not give me a clue ^^ I am absolutely sure to be on master, this is the only branch I have and I triple checked.

The command returns the last commit hash as expected, this is getting odd :/

zibuntu ~/D/C/S/P/personality-bfi/ git rev-parse master 2>/dev/null || echo "did not work"
f352d5b2ce00ab8c6411365bd2b0118ddf7214ba

Is there any way I can debug the command execution interactively?

githubsaturn commented 2 years ago

That is really bizarre!

Yes, try this:

## find where is the caprover command, most likely: /usr/local/bin/caprover
where caprover

## find the actual file:
## most likely it's in /usr/local/lib/node_modules/caprover/built/commands/caprover.js
ls -lah `where caprover`

## once you find the location, you can edit the validation handler file and add more debugging to it:
## edit this file: /usr/local/lib/node_modules/caprover/built/utils/ValidationsHandler.js
nano /usr/local/lib/node_modules/caprover/built/utils/ValidationsHandler.js
## or
code /usr/local/lib/node_modules/caprover/built/utils/ValidationsHandler.js
## etc...
Ziip-dev commented 2 years ago

Okay, think I get it, thanks to your pointers :)

caprover deploy is running with sudo. And this line:

https://github.com/caprover/caprover-cli/blob/cf4f970ef897de684bd10ac56dd46f6d18d63c41/src/utils/ValidationsHandler.ts#L174

translates to:

: child_process_1.execSync(`git rev-parse ${value} 2>/dev/null`)

in javascript.

So I guess that a child process shall runs as root as well. Yet, when I try the following in my terminal:

zibuntu ~/D/C/S/P/personality-bfi/ sudo git rev-parse master
fatal: dépôt non sécurisé ('/h/Z/D/C/S/P/personality-bfi' appartient à quelqu'un d'autre)
Pour ajouter une exception pour ce dépôt, lancez :

    git config --global --add safe.directory /h/Z/D/C/S/P/personality-bfi

... which is confirmed by removing the /dev/null filter in ValidationsHandler.js:

//: child_process_1.execSync(`git rev-parse ${value} 2>/dev/null`)
: child_process_1.execSync(`git rev-parse ${value}`)
zibuntu ~/D/C/S/P/personality-bfi/ sudo caprover deploy

Preparing deployment to CapRover...

**** Protip ****
You seem to have deployed bfi from this directory in the past, use --default flag to avoid having to re-enter the information.

? select the CapRover machine name you want to deploy to: Linode
Ensuring authentication...
? select the app name you want to deploy to: dashboard
? git branch name to be deployed: master
fatal: dépôt non sécurisé ('/h/Z/D/C/S/P/personality-bfi' appartient à quelqu'un d'autre)
Pour ajouter une exception pour ce dépôt, lancez :

    git config --global --add safe.directory /h/Z/D/C/S/P/personality-bfi
? git branch name to be deployed: master
>> Cannot find hash of last commit on branch "master".

We were landing on the misleading error because of git :) However I can't explain this change of behaviour from last year... Maybe a check for git errors should be added in getErrorForBranchName? Or stderr redirected to a log file with a message? What do you think would be a neat way to fix this? I don't feel like running everything as root nor adding git exceptions everywhere would be a particularly desirable solution

githubsaturn commented 2 years ago

Hmm interesting... Good find! Maybe a simple sudo check should be added to the code.

But... Why would you run caprover deploy as sudo anyways?

Ziip-dev commented 2 years ago

Well, because of this:

zibuntu ~/D/C/S/P/personality-bfi/ caprover deploy

/usr/lib/node_modules/caprover/node_modules/configstore/index.js:52
            throw error;
            ^

Error: EACCES: permission denied, open '/home/Ziip/.config/configstore/caprover.json'
You don't have access to this file.

    at Object.openSync (fs.js:497:3)
    at Object.readFileSync (fs.js:393:35)
    at Configstore.get all [as all] (/usr/lib/node_modules/caprover/node_modules/configstore/index.js:34:25)
    at Configstore.get (/usr/lib/node_modules/caprover/node_modules/configstore/index.js:77:27)
    at StorageHelper.getMachines (/usr/lib/node_modules/caprover/built/utils/StorageHelper.js:18:53)
    at CliHelper.getMachinesAsOptions (/usr/lib/node_modules/caprover/built/utils/CliHelper.js:45:18)
    at new List (/usr/lib/node_modules/caprover/built/commands/logout.js:23:51)
    at Object.<anonymous> (/usr/lib/node_modules/caprover/built/commands/caprover.js:30:5)
    at Module._compile (internal/modules/cjs/loader.js:1085:14)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1114:10)
    at Module.load (internal/modules/cjs/loader.js:950:32)
    at Function.Module._load (internal/modules/cjs/loader.js:790:12)
    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:75:12)
    at internal/main/run_main_module.js:17:47 {
  errno: -13,
  syscall: 'open',
  code: 'EACCES',
  path: '/home/Ziip/.config/configstore/caprover.json'
}
zibuntu ~/D/C/S/P/personality-bfi/ ll ~/.config/configstore/caprover.json
-rw------- 1 root root 1.5K May 20 15:38 /home/Ziip/.config/configstore/caprover.json

Well, as npm i -g caprover was complaining about permissions to install globally, I believed I installed with sudo at that time. It was working fine so I didn't look further to be honest ^^'

I guess this comes from npm install? Is changing the permissions of caprover.json is enough?

githubsaturn commented 2 years ago

I guess this comes from npm install?

No. sudo npm i -g caprover is totally fine.

CapRover uses configstore package to store the creds etc. The entire point of that package (or one of the most important aspect of it) is to make sure the permissions are set to the current user, not root.

You probably ran sudo caprover something post installation and that caused that file to be created by root.

Ziip-dev commented 2 years ago

You probably ran sudo caprover something post installation and that caused that file to be created by root.

Oow yep, this is what happened most probably! Well, I am closing the issue as changing the permissions of caprover.json did the trick!

I think it would be nice to add a small check for git errors in ValidationsHandler.js as an external program called in a child process, at least to avoid falling on a misleading error.

Despite my limited skills in JS, I would be happy to help with testing or anything, feel free to ask :) In my mind, something like this should do the trick:

export function getErrorForBranchName(value: string): true | string {
    ...
    try {
        const cmd = isWindows
            ? execSync(`git rev-parse ${value}`)
            : execSync(`git rev-parse ${value} 2>&1`)
            // if git error:
                // return 'caprover encountered an error during a git command:' + error
    ...
    return `Cannot find hash of last commit on branch "${value}".`
}

Thanks a lot for your guidance :+1: