source-sftp: ssh key authentication #1202
Conversation
| Password string `json:"password" jsonschema:"title=Password" jsonschema_extras:"secret=true,order=2"` | ||
| Directory string `json:"directory" jsonschema:"title=Directory" jsonschema_extras:"order=3"` | ||
| MatchFiles string `json:"matchFiles,omitempty" jsonschema:"title=Match Files Regex" jsonschema_extras:"order=4"` | ||
| Password string `json:"password,omitempty" jsonschema:"title=Password" jsonschema_extras:"secret=true,order=2"` |
There was a problem hiding this comment.
I think it would be good to have password and SSH key as a "one of" option in the config, to make it clear that you don't need to provide both of them. This would probably require writing out the spec by hand since IIRC there's not a way to represent that in the struct tags, and also some kind of backward compatibility with existing specs that have "password" in the top-level. I wish I would have anticipated needing to add another authentication method and originally made this a "one of" to begin with.
It's probably fine having them both as top-level options until/if we need more than 2 authentication options, so I'll leave it up to you as to wether its worth the effort or not to make it a "one of" right now. If nothing else, we should update the descriptions to say "may be left blank if using the other thing".
There was a problem hiding this comment.
@williamhbaker yeah I thought about oneOf, but I think there is a lot of effort necessary for that. At the moment I don't see this connector wanting another authentication method so we can keep it this way for now for simplicity's sake. Added to the description of SSHKey and Password about this
Description:
Workflow steps:
(How does one use this feature, and how has it changed)
Documentation links affected:
(list any documentation links that you created, or existing ones that you've identified as needing updates, along with a brief description)
Notes for reviewers:
(anything that might help someone review this PR)
This change is