Skip to content

enable globbing when using 'tsh scp'#23789

Merged
capnspacehook merged 2 commits intomasterfrom
capnspacehook/tsh-scp-globbing
Apr 11, 2023
Merged

enable globbing when using 'tsh scp'#23789
capnspacehook merged 2 commits intomasterfrom
capnspacehook/tsh-scp-globbing

Conversation

@capnspacehook
Copy link
Copy Markdown
Contributor

@capnspacehook capnspacehook commented Mar 29, 2023 •
edited
Loading

Also fixes copying symlinked files.

Fixes #22748.

@capnspacehook capnspacehook changed the base branch from master to capnspacehook/sftp-backend-refactor March 29, 2023 16:45
@capnspacehook capnspacehook force-pushed the capnspacehook/tsh-scp-globbing branch from ac7bbcf to b288667 Compare March 29, 2023 20:06
@github-advanced-security
Copy link
Copy Markdown

github-advanced-security AI commented Mar 29, 2023

You have successfully added a new Trivy configuration .github/workflows/trivy.yaml:trivy. As part of the setup process, we have scanned this repository and found no existing alerts. In the future, you will see all code scanning alerts on the repository Security tab.

@capnspacehook capnspacehook force-pushed the capnspacehook/sftp-backend-refactor branch from 665074c to d1cb50e Compare March 29, 2023 20:21
@capnspacehook capnspacehook force-pushed the capnspacehook/tsh-scp-globbing branch from b288667 to b9b0dae Compare March 29, 2023 20:22
@capnspacehook capnspacehook mentioned this pull request Mar 30, 2023
Closed
Base automatically changed from capnspacehook/sftp-backend-refactor to master April 7, 2023 02:12
@capnspacehook capnspacehook added sftp Issues related to Teleport's SFTP implementation and removed backport/branch/v10 labels Apr 7, 2023
@capnspacehook capnspacehook marked this pull request as ready for review April 7, 2023 02:56
@github-actions github-actions Bot requested review from EdwardDowling and r0mant April 7, 2023 02:56
@capnspacehook capnspacehook mentioned this pull request Apr 7, 2023
Merged
@capnspacehook
enable globbing when using 'tsh scp'
09286bc 
Also fix copying symlinked files.
@capnspacehook capnspacehook force-pushed the capnspacehook/tsh-scp-globbing branch from 2789295 to 09286bc Compare April 7, 2023 03:05
r0mant
r0mant approved these changes Apr 7, 2023
View reviewed changes
Copy link
Copy Markdown
Collaborator

@r0mant r0mant left a comment

Choose a reason for hiding this comment

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

lgtm with a few suggestions

Comment thread lib/sshutils/sftp/local.go
}

func (l localFS) Stat(ctx context.Context, path string) (os.FileInfo, error) {
func (l *localFS) Glob(ctx context.Context, pattern string) ([]string, error) {
Copy link
Copy Markdown
Collaborator

@r0mant r0mant Apr 7, 2023

Choose a reason for hiding this comment

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

Nit: Can you add godocs on all exported names?

Copy link
Copy Markdown
Contributor Author

@capnspacehook capnspacehook Apr 10, 2023

Choose a reason for hiding this comment

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

I can, but I thought it was unnecessary due to the fact that localFS's methods are never called directly, only as a FileSystem interface which has all of its methods documented. For future reference, in this case is it still necessary to add comments to every method of an implementation of an interface that is (should) never be used directly?

Comment thread lib/sshutils/sftp/local.go Outdated
Comment thread lib/sshutils/sftp/local.go Outdated
Comment thread lib/sshutils/sftp/remote.go Outdated
Comment thread lib/sshutils/sftp/sftp.go Outdated
Comment thread lib/sshutils/sftp/sftp.go Outdated
Comment thread lib/sshutils/sftp/sftp.go Outdated
Comment thread lib/sshutils/sftp/sftp.go Outdated
@capnspacehook
Copy link
Copy Markdown
Contributor Author

capnspacehook commented Apr 10, 2023

Friendly ping @EdwardDowling

@capnspacehook capnspacehook added this pull request to the merge queue Apr 11, 2023
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Apr 11, 2023
@capnspacehook capnspacehook added this pull request to the merge queue Apr 11, 2023
Merged via the queue into master with commit f29d839 Apr 11, 2023
@capnspacehook capnspacehook deleted the capnspacehook/tsh-scp-globbing branch April 11, 2023 15:11
@capnspacehook capnspacehook mentioned this pull request Apr 12, 2023
Merged
This was referenced Apr 19, 2023
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@r0mant r0mant r0mant approved these changes

+1 more reviewer

@EdwardDowling EdwardDowling EdwardDowling approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

sftp Issues related to Teleport's SFTP implementation size/md

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

tsh scp doesn't recognize wildcard patterns

4 participants

@capnspacehook @github-advanced-security @r0mant @EdwardDowling