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

✨ Add SSH overrides for user, host, and port - N8N-3040 #2333

Closed
wants to merge 1 commit into from

Conversation

pemontto
Copy link
Contributor

@pemontto pemontto commented Oct 17, 2021

This PR adds the ability to override the SSH host, username, and port.

I'm not sure why SSH credentials are so tightly coupled to username, host, and port. Often one set of credentials can be used to access multiple hosts, especially with SSH keys.

A couple of questions:

  1. Is there a reason SSH credentials require host and port, but not username/password/private key? I would've assumed the opposite?
    n8n_io_-_Workflow_Automation
  2. What's the best way to set displayOptions on nested items within a collection? In this case we only want options.fileName to display based on those displayOptions I commented out, however we always want the new override options visible. Setting displayOptions on options.fileName directly causes errors in the browser.

Result showing connection to multiple users, and gracefully handling errors when using Continue On Fail:
n8n_-_▶️_Test_Data

@RicardoE105
Copy link
Contributor

@pemontto Thanks for your contribution

Is there a reason SSH credentials require host and port, but not username/password/private key? I would've assumed the opposite?

This seems to be an overlook.

What's the best way to set displayOptions on nested items within a collection? In this case we only want options.fileName to display based on those displayOptions I commented out, however we always want the new override options visible. Setting displayOptions on options.fileName directly causes errors in the browser.

Not sure if I fully understand what you want to do. But, to control what is shown within a collection based on a field in the root level you can / in the displayOptions. Check the example below in the Airtable node.

https://github.com/n8n-io/n8n/blob/master/packages/nodes-base/nodes/Airtable/Airtable.node.ts#L423

With regards to adding those fields, let me discuss them with a coworker. Will keep you posted.

@pemontto
Copy link
Contributor Author

@RicardoE105 thanks for the pointer! I've updated and tested the nested displayOptions for options.fileName.

Will look at reviewing the creds in another PR.

@RicardoE105
Copy link
Contributor

@pemontto ok, this is something that you can do using expressions in the credentials. Did you try that? Since this can already be done, and even if we included it, we would have to add it to other nodes as well (we try as much as we can to keep consistency in the nodes), sadly we are not going to merge it. We Appreciate the contribution. I hope that makes sense. Let me know if you have further questions.

@pemontto
Copy link
Contributor Author

@RicardoE105 ahh I wasn't aware that was possible. While that's useful it doesn't actually let the node connect to multiple hosts, as the ssh.connect is currently done outside the loop.

@RicardoE105
Copy link
Contributor

RicardoE105 commented Oct 26, 2021

You have to provide the hosts as the input and process them one by one (you need the split batches node), then in the ssh node's credentials, you reference the hots using expressions. Why? because using expressions in the credentials only workss with the first item.

@ivov ivov added node/improvement New feature or request community Authored by a community member labels Dec 15, 2021
@RicardoE105
Copy link
Contributor

Closing this PR as it is something that we can partially do, as explained above. Also, at some point, we will add a feature to be able to reference credentials dynamically without the current limitations. @pemontto feel free to open this PR at any time if you have further questions. And, thanks very much for the contribution.

@RicardoE105 RicardoE105 closed this Apr 4, 2022
@RicardoE105 RicardoE105 changed the title ✨ Add SSH overrides for user, host, and port ✨ Add SSH overrides for user, host, and port - N8N-3040 Apr 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community Authored by a community member node/improvement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants