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

feat: commit signing using ssh and gpg #1146

Merged
merged 50 commits into from
Oct 17, 2024
Merged

Conversation

divanshu-go
Copy link
Contributor

@divanshu-go divanshu-go commented Sep 24, 2024

feat: commit signing using ssh and gpg

Description

add support for commit signing when adding git provider(s)

  • This change requires a documentation update
  • I have made corresponding changes to the documentation

Related Issue(s)

Closes #370
/claim #370

Screenshots

image

Notes

In order to sign your commits with ssh . ssh keys need to be present on your local system and should be added to your github or gitlab account. my pr simplifies the work for manually doing setup for ssh commit signing in the gitconfig file of the project.
for the ssh key field : paste the same key that you pasted in the add ssh key in github or gitlab

@divanshu-go divanshu-go requested review from a team as code owners September 24, 2024 16:22
@algora-pbc algora-pbc bot mentioned this pull request Sep 24, 2024
@divanshu-go
Copy link
Contributor Author

@Tpuljak
image
can you please assist here. I am succesfull in adding git provider but these values are not getting stored and am not able to understand what to do

@divanshu-go divanshu-go marked this pull request as draft September 24, 2024 16:34
@Tpuljak
Copy link
Member

Tpuljak commented Sep 25, 2024

@Tpuljak image can you please assist here. I am succesfull in adding git provider but these values are not getting stored and am not able to understand what to do

@bryans-go from what I can see, you don't pass the properties from SetGitProviderConfig to GitProviderConfig here https://github.com/daytonaio/daytona/blob/main/pkg/api/controllers/gitprovider/gitprovider.go#L134

@divanshu-go divanshu-go force-pushed the commits-git branch 3 times, most recently from 0660fde to 25fea9e Compare September 27, 2024 03:54
@divanshu-go divanshu-go marked this pull request as ready for review September 27, 2024 04:09
@divanshu-go
Copy link
Contributor Author

divanshu-go commented Sep 27, 2024

asciinema

@divanshu-go
Copy link
Contributor Author

divanshu-go commented Sep 27, 2024

image

image
now the ssh signing is working perfectly @Tpuljak
Some of the git providers does not show verified commits bagde even on signed commits by git while some also dont have proper docs on this . what should we do there ?

pkg/db/dto/git_provider.go Outdated Show resolved Hide resolved
pkg/db/dto/git_provider.go Outdated Show resolved Hide resolved
pkg/git/service.go Outdated Show resolved Hide resolved
pkg/gitprovider/types.go Outdated Show resolved Hide resolved
pkg/gitprovider/types.go Outdated Show resolved Hide resolved
pkg/server/gitproviders/service.go Outdated Show resolved Hide resolved
pkg/server/gitproviders/service.go Outdated Show resolved Hide resolved
@Tpuljak
Copy link
Member

Tpuljak commented Sep 27, 2024

Some of the git providers does not show verified commits bagde even on signed commits by git while some also dont have proper docs on this . what should we do there ?

Could you let us know which providers don't show the verified badge but should?

P.S. Nice work on the solution!

@divanshu-go
Copy link
Contributor Author

https://jira.atlassian.com/browse/BCLOUD-3166. Bitbucket doesn't have this feature yet and gitness don't have a proper docs on ssh and gpg signing while

@Tpuljak
Copy link
Member

Tpuljak commented Sep 27, 2024

https://jira.atlassian.com/browse/BCLOUD-3166. Bitbucket doesn't have this feature yet and gitness don't have a proper docs on ssh and gpg signing while

Then let's not show that input at all if the user selects Bitbucket or Gitness

@divanshu-go
Copy link
Contributor Author

Ok

@divanshu-go
Copy link
Contributor Author

@Tpuljak Done

@Tpuljak Tpuljak mentioned this pull request Sep 30, 2024
6 tasks
pkg/gitprovider/types.go Outdated Show resolved Hide resolved
pkg/views/gitprovider/select.go Outdated Show resolved Hide resolved
@divanshu-go
Copy link
Contributor Author

@bryans-go I had something else in mind with https://github.com/daytonaio/daytona/pull/1146/files#r1778545525.

"none" shouldn't be defined as an option here.

The solution I was aiming for included setting up a variable inside GitProviderSelectionView that would be a helper variable for selecting the signing method. It would include ssh gpg and none as options. After the form runs inside that function, you would check the value of that variable and assign GitProviderView to the appropriate value (if none then nil).

demo
@Tpuljak I did what you said but problem is that due to the fact that signing method is a enum . the following problem has arisen. please assist with it

@Tpuljak
Copy link
Member

Tpuljak commented Sep 30, 2024

@bryans-go I had something else in mind with https://github.com/daytonaio/daytona/pull/1146/files#r1778545525.
"none" shouldn't be defined as an option here.
The solution I was aiming for included setting up a variable inside GitProviderSelectionView that would be a helper variable for selecting the signing method. It would include ssh gpg and none as options. After the form runs inside that function, you would check the value of that variable and assign GitProviderView to the appropriate value (if none then nil).

demo demo @Tpuljak I did what you said but problem is that due to the fact that signing method is a enum . the following problem has arisen. please assist with it

@bryans-go please try to do some debugging yourself. Not completely sure but this might be part of the issue. The signing method is a string when passed here when it should be a pointer to a string.
Also, make sure to wipe your config between these changes since these are all breaking to one another.

@divanshu-go
Copy link
Contributor Author

divanshu-go commented Sep 30, 2024

gitProviderAddView.SigningMethod = nil
I added the following line by explicitly setting the value as nil, and now everything works perfectly.

PS: I tried setting signing method as pointer to string but it didn't worked as expected but the following method was successful .

pkg/views/gitprovider/select.go Outdated Show resolved Hide resolved
pkg/views/gitprovider/select.go Outdated Show resolved Hide resolved
pkg/cmd/gitprovider/add.go Outdated Show resolved Hide resolved
pkg/cmd/gitprovider/delete.go Outdated Show resolved Hide resolved
pkg/views/gitprovider/select.go Outdated Show resolved Hide resolved
pkg/views/gitprovider/select.go Outdated Show resolved Hide resolved
pkg/api/controllers/gitprovider/dto/dto.go Outdated Show resolved Hide resolved
pkg/api/controllers/gitprovider/dto/dto.go Outdated Show resolved Hide resolved
pkg/db/dto/git_provider.go Outdated Show resolved Hide resolved
pkg/cmd/gitprovider/add.go Outdated Show resolved Hide resolved
Signed-off-by: bryans-go <[email protected]>
Signed-off-by: bryans-go <[email protected]>
pkg/cmd/workspace/code.go Outdated Show resolved Hide resolved
pkg/cmd/workspace/code.go Outdated Show resolved Hide resolved
pkg/cmd/workspace/create.go Outdated Show resolved Hide resolved
pkg/cmd/workspace/ssh.go Outdated Show resolved Hide resolved
pkg/cmd/workspace/ssh.go Outdated Show resolved Hide resolved
pkg/cmd/workspace/start.go Outdated Show resolved Hide resolved
pkg/cmd/workspace/start.go Outdated Show resolved Hide resolved
pkg/cmd/workspace/create.go Outdated Show resolved Hide resolved
Signed-off-by: bryans-go <[email protected]>
@divanshu-go
Copy link
Contributor Author

divanshu-go commented Oct 14, 2024

@Tpuljak Done !

Copy link
Member

@Tpuljak Tpuljak left a comment

Choose a reason for hiding this comment

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

Nice work! Everything works except the one comment I left below.

pkg/views/gitprovider/select.go Show resolved Hide resolved
Signed-off-by: bryans-go <[email protected]>
Signed-off-by: bryans-go <[email protected]>
Signed-off-by: bryans-go <[email protected]>
@divanshu-go
Copy link
Contributor Author

asciicast
Its working now

Signed-off-by: bryans-go <[email protected]>
Tpuljak
Tpuljak previously approved these changes Oct 15, 2024
Copy link
Member

@Tpuljak Tpuljak left a comment

Choose a reason for hiding this comment

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

Nice work!

Signed-off-by: bryans-go <[email protected]>
Signed-off-by: bryans-go <[email protected]>
@divanshu-go
Copy link
Contributor Author

image
added logic for displaying signing method.

@divanshu-go
Copy link
Contributor Author

@Tpuljak do we require any further changes ?

Copy link
Member

@Tpuljak Tpuljak left a comment

Choose a reason for hiding this comment

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

One more comment to resolve before I can approve

cmd/daytona/config/ssh_file.go Outdated Show resolved Hide resolved
Signed-off-by: bryans-go <[email protected]>
@lbrecic lbrecic requested a review from idagelic October 16, 2024 11:50
@divanshu-go
Copy link
Contributor Author

@Tpuljak The PR has both approvals now.
If possible, could you merge it today? Would really appreciate it. Thanks! 😊

@Tpuljak
Copy link
Member

Tpuljak commented Oct 17, 2024

Screenshot 2024-10-17 at 11 08 46

@bryans-go as you can see above your message, one more review was requested. No need to ping us for this. It will be merged once it's ready.

@idagelic idagelic merged commit 7f8462c into daytonaio:main Oct 17, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Git commit signing
4 participants