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

refactor(ssh-keys): Update ssh-keys register, show and delete to egos… #639

Merged
merged 5 commits into from
Nov 4, 2024
Merged

refactor(ssh-keys): Update ssh-keys register, show and delete to egos… #639

merged 5 commits into from
Nov 4, 2024

Conversation

mlec1
Copy link
Contributor

@mlec1 mlec1 commented Sep 20, 2024

Update ssh-keys register, show and delete to egoscale v3

Checklist

(For exoscale contributors)

  • Changelog updated (under Unreleased block)
  • Testing

Testing

root@65df3c390733:/cli# go run . compute ssh-key register test ./id_test.pub 
 ✔ Registering SSH key "test"... 0s
┼─────────────┼─────────────────────────────────────────────────┼
│   SSH KEY   │                                                 │
┼─────────────┼─────────────────────────────────────────────────┼
│ Name        │ test                                            │
│ Fingerprint │ 6c:e0:04:f9:0b:fd:38:d1:c2:96:14:f9:ed:99:ff:01 │
┼─────────────┼─────────────────────────────────────────────────┼
root@65df3c390733:/cli# go run . compute ssh-key show test
┼─────────────┼─────────────────────────────────────────────────┼
│   SSH KEY   │                                                 │
┼─────────────┼─────────────────────────────────────────────────┼
│ Name        │ test                                            │
│ Fingerprint │ 6c:e0:04:f9:0b:fd:38:d1:c2:96:14:f9:ed:99:ff:01 │
┼─────────────┼─────────────────────────────────────────────────┼
root@65df3c390733:/cli# go run . compute ssh-key delete test
[+] Are you sure you want to delete SSH key test? [yN]: y
 ✔ Deleting SSH key test... 0s

pierre-emmanuelJ
pierre-emmanuelJ requested changes Oct 14, 2024
View reviewed changes
cmd/ssh_key_delete.go Outdated Show resolved Hide resolved
cmd/ssh_key_delete.go Outdated Show resolved Hide resolved
@pierre-emmanuelJ
Copy link
Member

Probably will be easier to format your code if you are using gofmt :)

@pierre-emmanuelJ
Copy link
Member

All these suggestions are suggestion and to be tested

@mlec1 mlec1 force-pushed the update-ssh-keys-register-delete-v3 branch from 016d7f7 to 8203e03 Compare October 14, 2024 20:34
@mlec1
Copy link
Contributor Author

mlec1 commented Oct 14, 2024

All these suggestions are suggestion and to be tested

Change Tested

@mlec1
Copy link
Contributor Author

mlec1 commented Oct 14, 2024

Probably will be easier to format your code if you are using gofmt :)

I run make fmt or even gofmt -w cmd/ but it doesn't remove the line you mentionned

pierre-emmanuelJ
pierre-emmanuelJ requested changes Oct 16, 2024
View reviewed changes
Copy link
Member

@pierre-emmanuelJ pierre-emmanuelJ left a comment

Choose a reason for hiding this comment

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

Thank you

cmd/ssh_key_delete.go Outdated Show resolved Hide resolved
cmd/ssh_key_register.go Outdated Show resolved Hide resolved
cmd/ssh_key_register.go Outdated Show resolved Hide resolved
cmd/ssh_key_register.go Outdated Show resolved Hide resolved
cmd/ssh_key_register.go Outdated Show resolved Hide resolved
@mlec1
Copy link
Contributor Author

mlec1 commented Oct 19, 2024 

root@ca43ab0f2fa4:/exo# go run . compute ssh-key register test /root/.ssh/id_ed25519.pub 
 ✔ Registering SSH key "test"... 0s
┼─────────────┼─────────────────────────────────────────────────┼
│   SSH KEY   │                                                 │
┼─────────────┼─────────────────────────────────────────────────┼
│ Name        │ test                                            │
│ Fingerprint │ f0:bf:b8:70:d9:0e:58:f0:4a:15:ef:08:89:11:55:a5 │
┼─────────────┼─────────────────────────────────────────────────┼
root@ca43ab0f2fa4:/exo# go run . compute ssh-key show test                               
┼─────────────┼─────────────────────────────────────────────────┼
│   SSH KEY   │                                                 │
┼─────────────┼─────────────────────────────────────────────────┼
│ Name        │ test                                            │
│ Fingerprint │ f0:bf:b8:70:d9:0e:58:f0:4a:15:ef:08:89:11:55:a5 │
┼─────────────┼─────────────────────────────────────────────────┼
root@ca43ab0f2fa4:/exo# go run . compute ssh-key delete test
[+] Are you sure you want to delete SSH key test? [yN]: y
 ✔ Deleting SSH key test... 0s

Change tested

@elkezza elkezza self-requested a review October 29, 2024 10:08
elkezza
elkezza reviewed Oct 29, 2024
View reviewed changes
cmd/ssh_key_register.go Outdated
err = decorateAsyncOperations(fmt.Sprintf("Registering SSH key %q...", c.Name), func() error {
op, err := client.RegisterSSHKey(ctx, registerKeyRequest)
if err != nil {
return err
Copy link
Contributor

Choose a reason for hiding this comment

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

in the function, we usually add more error wrapping the same way you did in this file

elkezza
elkezza reviewed Oct 29, 2024
View reviewed changes
cmd/ssh_key_delete.go Outdated
return decorateAsyncOperations(fmt.Sprintf("Deleting SSH key %s...", c.Name), func() error {
op, err := globalstate.EgoscaleV3Client.DeleteSSHKey(ctx, c.Name)
if err != nil {
return err
Copy link
Contributor

Choose a reason for hiding this comment

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

Same thing here.

elkezza
elkezza requested changes Oct 29, 2024
View reviewed changes
Copy link
Contributor

@elkezza elkezza left a comment

Choose a reason for hiding this comment

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

other than the error wrapping everything looks good to me : )

@mlec1 mlec1 requested a review from elkezza November 1, 2024 23:00
elkezza
elkezza approved these changes Nov 3, 2024
View reviewed changes
Copy link
Contributor

@elkezza elkezza left a comment

Choose a reason for hiding this comment

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

lgtm, thank you for the nice work!

@pierre-emmanuelJ pierre-emmanuelJ merged commit f0aea5f into exoscale:master Nov 4, 2024
1 check passed
@mlec1 mlec1 deleted the update-ssh-keys-register-delete-v3 branch November 5, 2024 16:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@elkezza elkezza elkezza approved these changes

@pierre-emmanuelJ pierre-emmanuelJ pierre-emmanuelJ approved these changes

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

Successfully merging this pull request may close these issues.
3 participants
@mlec1 @pierre-emmanuelJ @elkezza