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.

Sign up for GitHub

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

Jump to bottom

Feat/gpg agent forwarding #599

Merged
merged 30 commits into from
Oct 27, 2023
Merged

Feat/gpg agent forwarding #599

merged 30 commits into from
Oct 27, 2023

Conversation

89luca89
Copy link
Contributor

@89luca89 89luca89 commented Aug 9, 2023 •
edited
Loading

This PR Will

  • migrate to our ssh server library github.com/loft-sh/ssh
  • add reverse-forward-ports to ssh command
  • add gpg-agent-forwarding to up command

Before merging I'd like to have feedbacks from non-linux users, as I've been able to test this only on a series of linux machines

Fixes #580
Resolves ENG-1809

@89luca89 89luca89 marked this pull request as draft August 9, 2023 13:58
@89luca89 89luca89 force-pushed the feat/gpg_agent_forwarding branch from 176e7cd to 6357fbe Compare August 9, 2023 14:30
FabianKramm
FabianKramm requested changes Aug 10, 2023
View reviewed changes
cmd/up.go Outdated Show resolved Hide resolved
cmd/up.go Outdated Show resolved Hide resolved
cmd/up.go Outdated Show resolved Hide resolved
pkg/ssh/forward.go Outdated Show resolved Hide resolved
pkg/ssh/forward.go Outdated Show resolved Hide resolved
@89luca89 89luca89 requested a review from FabianKramm August 10, 2023 11:57
@89luca89 89luca89 force-pushed the feat/gpg_agent_forwarding branch 9 times, most recently from 5d28694 to 3c98f78 Compare August 11, 2023 11:37
@89luca89 89luca89 marked this pull request as ready for review August 11, 2023 13:15
@89luca89
Copy link
Contributor Author

Tested working on Macos also 👍

pbialon
pbialon previously requested changes Sep 22, 2023
View reviewed changes
pkg/ssh/forward.go Outdated Show resolved Hide resolved
pkg/ssh/forward.go Show resolved Hide resolved
cmd/ssh.go Outdated Show resolved Hide resolved
cmd/ssh.go Outdated Show resolved Hide resolved
cmd/ssh.go Outdated Show resolved Hide resolved
cmd/ssh.go Outdated Show resolved Hide resolved
cmd/ssh.go Show resolved Hide resolved
cmd/agent/workspace/setup_gpg.go Show resolved Hide resolved
@89luca89 89luca89 requested a review from pbialon September 26, 2023 08:44
pascalbreuninger
pascalbreuninger reviewed Sep 26, 2023
View reviewed changes
cmd/agent/workspace/setup_gpg.go Outdated Show resolved Hide resolved
cmd/ssh.go Outdated Show resolved Hide resolved
cmd/agent/workspace/setup_gpg.go Show resolved Hide resolved
cmd/ssh.go Outdated Show resolved Hide resolved
pbialon
pbialon reviewed Sep 27, 2023
View reviewed changes
vendor/github.com/loft-sh/ssh/tcpip.go Show resolved Hide resolved
vendor/github.com/loft-sh/ssh/streamlocal.go Show resolved Hide resolved
vendor/github.com/loft-sh/ssh/streamlocal.go Show resolved Hide resolved
vendor/github.com/loft-sh/ssh/streamlocal.go Show resolved Hide resolved
vendor/github.com/loft-sh/ssh/streamlocal.go Show resolved Hide resolved
cmd/ssh.go Outdated Show resolved Hide resolved
pkg/ssh/forward.go Show resolved Hide resolved
cmd/ssh.go Show resolved Hide resolved
cmd/ssh.go Outdated Show resolved Hide resolved
cmd/ssh.go Outdated Show resolved Hide resolved
FabianKramm
FabianKramm requested changes Oct 23, 2023
View reviewed changes
Copy link
Member

@FabianKramm FabianKramm left a comment

Choose a reason for hiding this comment

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

@89luca89 can we add a unit or e2e test for this? Since this is something thats default disabled and we have changes on the roadmap for the underlying ssh connection, we need to make sure those future changes are not breaking this

@89luca89
Copy link
Contributor Author

@89luca89 can we add a unit or e2e test for this? Since this is something thats default disabled and we have changes on the roadmap for the underlying ssh connection, we need to make sure those future changes are not breaking this

👍 sure thing

@FabianKramm
Copy link
Member

FabianKramm commented Oct 23, 2023 •
edited
Loading

Besides that it looks good, but we need a rebase here, as soon as we have the test / rebased, we can merge. Also please make sure we squash this and don't merge 30 commits

@89luca89
chore: migrate ssh server library to loft-sh/ssh
aee1836 
Signed-off-by: Luca Di Maio <luca.dimaio1@gmail.com>
@89luca89
feat: add reverse forwarding of ports
31e3c22 
Signed-off-by: Luca Di Maio <luca.dimaio1@gmail.com>
89luca89 added 13 commits October 25, 2023 09:54
@89luca89
docs: update docs for new gpg forwarding feature
25808e1 
Signed-off-by: Luca Di Maio <luca.dimaio1@gmail.com>
@89luca89
fix: force creation of the socket for conseguent runs
10a47a9 
Signed-off-by: Luca Di Maio <luca.dimaio1@gmail.com>
@89luca89
fix: unstuck ssh commands on different systems
e67d5a5 
Signed-off-by: Luca Di Maio <luca.dimaio1@gmail.com>
@89luca89
docs: fix typo
1199ad5 
Signed-off-by: Luca Di Maio <luca.dimaio1@gmail.com>
@89luca89
fix: deduplicate coding between Forward and ReverseForward
1d2755f 
Signed-off-by: Luca Di Maio <luca.dimaio1@gmail.com>
@89luca89
fix: remove unused argument from setupGpgAgent
d04ef47 
Signed-off-by: Luca Di Maio <luca.dimaio1@gmail.com>
@89luca89
fix: split remote gpg-agent setup in smaller steps
6fdcad3 
Signed-off-by: Luca Di Maio <luca.dimaio1@gmail.com>
@89luca89
fix: compatibility with older go versions
257782b 
Signed-off-by: Luca Di Maio <luca.dimaio1@gmail.com>
@89luca89
fix: move gpg setup to own package
119bdf7 
Signed-off-by: Luca Di Maio <luca.dimaio1@gmail.com>
@89luca89
fix: double forwarding bug
2cfe47d 
Signed-off-by: Luca Di Maio <luca.dimaio1@gmail.com>
@89luca89
chore: extract config file path to function
f93a96d 
Signed-off-by: Luca Di Maio <luca.dimaio1@gmail.com>
@89luca89
chore: fix naming, comments, debug leftovers
358db46 
Signed-off-by: Luca Di Maio <luca.dimaio1@gmail.com>
@89luca89
chore: extract timeout parsing to function
d32bf62 
Signed-off-by: Luca Di Maio <luca.dimaio1@gmail.com>
@89luca89 89luca89 force-pushed the feat/gpg_agent_forwarding branch 11 times, most recently from 1c28f53 to 18edd1f Compare October 25, 2023 10:59
@89luca89
tests: add e2e test for gpg agent forwarding
127db61 
Signed-off-by: Luca Di Maio <luca.dimaio1@gmail.com>
@89luca89 89luca89 force-pushed the feat/gpg_agent_forwarding branch from 18edd1f to 127db61 Compare October 25, 2023 11:09
@89luca89 89luca89 requested a review from FabianKramm October 25, 2023 11:51
@FabianKramm FabianKramm merged commit a94e5b4 into loft-sh:main Oct 27, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@FabianKramm FabianKramm FabianKramm approved these changes

@ThomasK33 ThomasK33 Awaiting requested review from ThomasK33

@pascalbreuninger pascalbreuninger Awaiting requested review from pascalbreuninger

@pbialon pbialon Awaiting requested review from pbialon

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Request: sync gpg signing keys in addition to git & docker credentials
4 participants
@89luca89 @FabianKramm @pbialon @pascalbreuninger