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

SFTP event source #2693

Merged
merged 10 commits into from
Jul 16, 2023
Merged

SFTP event source #2693

merged 10 commits into from
Jul 16, 2023

Conversation

dillonstreator
Copy link
Contributor

@dillonstreator dillonstreator commented Jul 7, 2023

Fixes #1652

This PR introduces an SFTP event source with support for fsnotify.Remove and fsnotify.Create operations.

A couple of caveats/questions on the implementation:

  • Changes in the watched SFTP Server path which are negated within the pollIntervalDuration window will not be captured and result in events. This should be understood by consumers given the nature of polling.
  • The SFTP event source is not stateful. Any changes that occur during a period of event source outage or restart are not captured. This could be implemented via the persistence interface that exists in the calendar event source, if deemed necessary.
  • This PR uses fsnotify.Op and fsnotify.Event but doesn't directly match the fsnotify interface. A file rename, different modification time, or size difference are all captured as fsnotify.Create.

Are we okay with these?
If yes, Is there a good place for me to document these caveats?
If no, I'm open to feedback on how we can adjust to support needs.

@dillonstreator dillonstreator force-pushed the sftp-event-source branch 2 times, most recently from e0cc6a0 to e47ae42 Compare July 7, 2023 22:04
@dillonstreator dillonstreator changed the title sftp event source SFTP event source Jul 7, 2023
@whynowy
Copy link
Member

whynowy commented Jul 8, 2023

@dillonstreator - thanks! could you run make codegen?

dillonstreator and others added 6 commits July 9, 2023 17:51
Signed-off-by: dillonstreator <dillonstreator@gmail.com>
Signed-off-by: dillon <dillonstreator@gmail.com>
Signed-off-by: dillon <dillonstreator@gmail.com>
Signed-off-by: dillon <dillonstreator@gmail.com>
…roj#2696)

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: dillon <dillonstreator@gmail.com>
….4.3 (argoproj#2694)

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: dillon <dillonstreator@gmail.com>
Signed-off-by: dillonstreator <dillonstreator@gmail.com>
Signed-off-by: dillon <dillonstreator@gmail.com>
// PollIntervalDuration the interval at which to poll the SFTP server
// defaults to 10 seconds
// +optional
PollIntervalDuration string `json:"pollIntervalDuration" protobuf:"varint,8,opt,name=pollIntervalDuration"`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use *string to avoid parsing empty string?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@whynowy
Copy link
Member

whynowy commented Jul 11, 2023

Could you also add a doc in https://github.com/argoproj/argo-events/tree/master/docs/eventsources/setup?

Copy link
Member

@whynowy whynowy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!


sftpConfig := &ssh.ClientConfig{
User: username,
Auth: []ssh.AuthMethod{ssh.Password(password)},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would be good to have option for sshkey based auth instead of password based too: https://pkg.go.dev/golang.org/x/crypto/ssh#PublicKeys

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agree with @tooptoop4, the possibility to authenticate with a private key would be very valuable. in our specific case, we're unable to use the sftp event source in production due to our sftp instance being protected by private ssh key.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@whynowy whynowy merged commit 5b0e16c into argoproj:master Jul 16, 2023
whynowy pushed a commit that referenced this pull request Aug 31, 2023
Signed-off-by: dillonstreator <dillonstreator@gmail.com>
Signed-off-by: dillon <dillonstreator@gmail.com>
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
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.

Support SFTP as input source
4 participants