redwoodjs / redwood

The App Framework for Startups
https://redwoodjs.com
MIT License
17.35k stars 999 forks source link

[Bug?]: Setting --verbose when deploying to baremetal "fails" during cleanup step. SshExecutor.js does not check that "args" is defined. #10654

Closed hjalmarhoglund closed 6 months ago

hjalmarhoglund commented 6 months ago

What's not working?

When deploying to baremetal and setting the verbose flag,

yarn rw deploy baremetal production --verbose

all goes well until the last step, the cleanup step. The following output is produced:

[STARTED] myapp.com
[STARTED] Connecting...
...
[COMPLETED] Restarting api process...
[STARTED] Cleaning up old deploys...
[FAILED] Cannot read properties of undefined (reading 'join')
[FAILED] Cannot read properties of undefined (reading 'join')

Notably, the deploy doesn't actually fail. The app is re-deployed and everything works. If you deploy without the verbose flag,

yarn rw deploy baremetal production

no error occurs.

I believe that the error comes from packages/cli/src/commands/deploy/baremetal/SshExecutor.js, where on lines 21 and 31 no check if args is undefined is done. I believe this is the case since the error only occurs when using the verbose flag.

How do we reproduce the bug?

Run yarn rw deploy baremetal production --verbose towards your baremetal server.

What's your environment? (If it applies)

On the system I am deploying from:

  System:
    OS: macOS 14.4.1
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 20.11.1 - /private/var/folders/_0/_dkb2pjx2910hmkb_3822c5w0000gn/T/xfs-72799c62/node
    Yarn: 3.7.0 - /private/var/folders/_0/_dkb2pjx2910hmkb_3822c5w0000gn/T/xfs-72799c62/yarn
  Databases:
    SQLite: 3.43.2 - /usr/bin/sqlite3
  Browsers:
    Safari: 17.4.1
  npmPackages:
    @redwoodjs/core: 7.5.1 => 7.5.1 
  redwood.toml:
    [web]
      title = "MyApp"
      port = 8910
      apiUrl = "/.redwood/functions"
      includeEnvironmentVariables = []
    [api]
      port = 8915
    [browser]
      open = true
    [notifications]
      versionUpdates = ["latest"]

On the system I am deploying to:

  System:
    OS: Linux 5.15 Ubuntu 22.04.4 LTS 22.04.4 LTS (Jammy Jellyfish)
    Shell: 5.1.16 - /bin/bash
  Binaries:
    Node: 20.13.1 - /tmp/xfs-6d0da2fd/node
    Yarn: 3.7.0 - /tmp/xfs-6d0da2fd/yarn
  npmPackages:
    @redwoodjs/cli-data-migrate: 7.5.1 => 7.5.1 
    @redwoodjs/core: 7.5.1 => 7.5.1 
  redwood.toml:
    [web]
      title = "MyApp"
      port = 8910
      apiUrl = "/.redwood/functions"
      includeEnvironmentVariables = []
    [api]
      port = 8915
    [browser]
      open = true
    [notifications]
      versionUpdates = ["latest"]

Are you interested in working on this?

ahaywood commented 6 months ago

@hjalmarhoglund Thanks for reporting this. I'm going to tag in another team member who specializes in deployment and will get back o you!

Josh-Walker-GM commented 6 months ago

Hey @hjalmarhoglund, would you mind trying out v7.6.1 and confirm that the fix works?

hjalmarhoglund commented 6 months ago

Hey @Josh-Walker-GM! Sure, I've upgraded and tested deploying. I believe another error has been introduced. This time, the error affects both the verbose and the none-verbose baremetal deploys.

Running,

yarn rw deploy baremetal production --verbose

gives the output

...
[STARTED] Cloning `main` branch...
SshExecutor::exec running command `gitclone --branch=main --depth=1 git@github.com:hjalmarhoglund/myapp.git 20240522150326` in /var/www/myapp
[FAILED] Error while running command `gitclone --branch=main --depth=1 git@github.com:hjalmarhoglund/myapp.git 20240522150326` in /var/www/myapp
[FAILED] bash: line 1: gitclone: command not found

Running,

yarn rw deploy baremetal production

gives the output

Error while running command `gitclone --branch=main --depth=1 git@github.com:hjalmarhoglund/myapp.git 20240522150713` in /var/www/myapp
bash: line 1: gitclone: command not found
...

The problem

The problem is that git clone [...] becomes gitclone [...].

Looking at the new code for SshExecutor.js we find on lines 13 and 14,

const argsString = args?.join(' ') || ''
const sshCommand = command + argsString

which does not correctly set a space between the command and the first arg.

Proposed fix

I think this could be solved by changing line 13 to:

const argsString = args ? ' ' + args.join(' ') : ''

Testing this in a local Node.js v20.11.1 session:

function testFix(command, args) {
    const argsString = args ? ' ' + args.join(' ') : ''
    const sshCommand = command + argsString
    // See what we end up with
    console.log(sshCommand)
}
const command = 'git'
const args1 = ['clone', '--branch=main']
const args2 = undefined
// Now running the proposed expression of argsString towards both args1 and args2
testFix(command, args1)
testFix(command, args2)

Gives the output

git clone --branch=main
git

Testing if this solves deploying

I try implementing my fix into node_modules/@redwoodjs/cli/dist/commands/deploy/baremetal/SshExecutor.js and run the deploy scripts, both verbose and non-verbose. For me, everything works.

...
async exec(path, command, args) {
    //const argsString = args?.join(" ") || "";
    const argsString = args ? ' ' + args.join(' ') : ''
    const sshCommand = command + argsString;
...

Hope this is of help, and thank you for getting to this as quickly as you did! Let me know if you want me to test anything again.

Josh-Walker-GM commented 6 months ago

Thanks for testing this so quickly! I agree we've messed up and introduced an even worse error! I'll get a fix and patch out again for this today

Josh-Walker-GM commented 6 months ago

Okay v7.6.2 is out with the further fix. Hopefully we didn't break it even more haha!

hjalmarhoglund commented 6 months ago

Fantastic! I've tested deploying on v7.6.2. Both verbose and non-verbose works like a charm!