-
Notifications
You must be signed in to change notification settings - Fork 127
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
add support for managing ssh keys #47
Conversation
1bdec44
to
941242d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a couple of minor 🤔 otherwise LGTM 👍
scaleway/resource_ssh_key.go
Outdated
return nil | ||
} | ||
|
||
mu.Lock() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do you want to move this lock up / re-retrieve the list post-lock, in-case a secondary key gets added in the meantime?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done!
scaleway/resource_ssh_key.go
Outdated
}) | ||
} | ||
} | ||
mu.Lock() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
again - do we want to move this lock up?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done :)
var testAccCheckScalewaySSHKeyConfig = ` | ||
resource "scaleway_ssh_key" "test" { | ||
key = "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDYpDmIzRs5c+xs0jmljMbNYVcgV8fRruMCRDA4HKjGN2lqLTZhngGDXsdt/2kTNQQPAq2sR4N8mfX5wMRT/+jNb+8esPyY5WlElni0zmD7oLoPW4lYRES6f7EeAv6NttLfkDO42r15OtMnglcgWk1u4o3lOXuLbhzJT1qdicpDja22X3uR/xUy1AYhKBOoiSlQbkb7NhL0lA1xQNwerdaJJS8tFB+wViVDyP0f1HaIRxViFlTGuTbTuIJNR/7VJ9VBBuTnYXaRkPxz64sUXrtdVK8U0+4KsisyXwmgQKnvZBDj91wxz12OOzFSQ52iFprIj1JbkzuBmNWXUGKYzXJZ test" | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
given the comments above, it might be worth a test adding multiple keys?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added testcases for multiple keys including removal.
}, | ||
{ | ||
"checksumSHA1": "hfABw6DX9B4Ma+88qDDGz9qY45s=", | ||
"path": "golang.org/x/crypto/internal/chacha20", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
assuming go build
is happy with the internal
vendors :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 it is
1bdb2f5
to
ac04d5d
Compare
this PR adds support for managing user ssh keys on scaleway.
the new resource looks like this:
The implementation relies on
golang.org/x/crypto/ssh
to parse the SSH key to retrieve the correct fingerprint; also included are docs.SSH keys can be imported like this:
closes #45